Skip to content

fix(server): make audit lifecycle durable under capacity pressure - #72

Merged
khaliqgant merged 20 commits into
mainfrom
codex/relayauth-audit-capacity-core
Jul 31, 2026
Merged

fix(server): make audit lifecycle durable under capacity pressure#72
khaliqgant merged 20 commits into
mainfrom
codex/relayauth-audit-capacity-core

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

This composes the reviewed RelayAuth Core changes needed to prevent the production audit-capacity incident from recurring at the storage-contract boundary.

  • makes token mint, refresh rotation, and revoke audit persistence atomic so success cannot acknowledge only half of a security-sensitive write;
  • exposes durable token and audit storage contracts for the Cloud adapter rather than copying platform-specific contracts into the OSS package;
  • adds bounded audit archive continuations and fixed work-budget accounting so mixed hot/archive pagination can resume without duplicate or skipped entries;
  • binds continuation cursors to the exact org-scoped filter and ordering that produced them;
  • retains compact archive receipts and an outbox migration while leaving Cloud responsible for the Cloudflare adapter, migration deferral, and dormant activation gates;
  • hardens refresh-token reuse handling and repairs the composed audit contract tests.

This is the upstream half of the durable RelayAuth capacity fix associated with #71. It does not itself change a deployed Cloudflare binding or activate the Cloud archive path.

Exact reviewed source

  • Head: 5d70d2bfb4472b889cd85f6b4c94837fc35dabc7
  • Base observed immediately before PR creation: 6c339afe5918b6985e30f43ed8e4f4715211fd56
  • Relationship at creation: 9 commits ahead, 0 behind

Verification

The branch has been exercised with the focused audit/archive and storage-contract suites plus the monorepo build and typecheck. The PR must additionally pass the repository live gates before any merge:

  • Build & Test
  • SDK Contract Check
  • full package build, typecheck, and tests
  • clean physical package/cold-import proof for @relayauth/server/storage/interface
  • independent exact-head review

Required release sequence

After a separately authorized merge, the required consumer package is @relayauth/server@0.2.23 only. It must be produced through the owner-gated Publish Packages workflow from merged main, first as a dry run, then verified from the real npm registry before Cloud updates its manifest, lockfile, and Worker re-bundle marker.

Explicit boundary

Opening this PR does not authorize or perform any merge, npm publication, publish.yml dispatch, tag or GitHub release creation, Cloud dependency repin, deployment, migration, provider mutation, database write, feature activation, or cutover. All runtime paths remain unchanged until their separately gated downstream steps occur.

Review in cubic

@cursor

cursor Bot commented Jul 31, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Audit queries now support archive continuation cursors, partial results, and work-budget metadata. Token routes now use atomic storage operations for issuance, rotation, revocation, and audit recording. The SDK and tests support the new result shapes.

Changes

Audit continuation and SDK

Layer / File(s) Summary
Archive storage and cursor contracts
packages/server/src/db/migrations/0005_audit_hot_outbox.sql, packages/server/src/storage/interface.ts
Adds archive tables, cursor types, structured results, timestamp validation, and continuation-key builders.
Audit, dashboard, and identity routes
packages/server/src/routes/audit-query.ts, packages/server/src/routes/audit-export.ts, packages/server/src/routes/dashboard-stats.ts, packages/server/src/routes/identity-activity.ts
Adds archive cursor validation, budget-exhausted responses, continuation encoding, and work-budget metadata.
TypeScript SDK audit pages
packages/sdk/typescript/src/client.ts, packages/sdk/typescript/src/index.ts
Adds typed audit page and work-budget exports. Audit methods now return structured pages.
Continuation test coverage
packages/server/src/__tests__/audit-query-api.test.ts, packages/server/src/__tests__/dashboard-stats-api.test.ts, packages/server/src/__tests__/identity-activity-api.test.ts, packages/server/src/__tests__/storage-sqlite.test.ts, packages/sdk/typescript/src/__tests__/client-audit.test.ts
Tests cover pagination, cursor binding, timestamp normalization, budget exhaustion, and gap-free resumption.

Atomic token storage

Layer / File(s) Summary
Token storage implementation
packages/server/src/storage/interface.ts, packages/server/src/storage/sqlite.ts
Adds token records, audited persistence, rotation, lookup, listing, revocation, rollback, and memory-backend selection.
Token route integration
packages/server/src/routes/tokens.ts
Uses AuthStorage directly for token workflows and atomic audit persistence.
Token and regression tests
packages/server/src/__tests__/tokens-route.test.ts, packages/server/src/__tests__/sqlite-storage.test.ts, packages/server/src/__tests__/token-storage-contract.test.ts, packages/server/src/__tests__/audit-logger.test.ts, packages/server/src/__tests__/e2e/audit.test.ts, packages/server/src/__tests__/e2e/rbac.test.ts
Covers atomic failures, rotation, revocation, introspection, token validation, audit behavior, and unchanged RBAC flows.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Poem

A rabbit follows cursors through the night,
Archive pages keep each entry in sight.
Tokens rotate with audits tied,
Atomic records safely reside.
Partial or complete, the results stay true. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.52% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary server-side change: durable audit handling under capacity pressure.
Description check ✅ Passed The description directly explains the audit durability, storage-contract, pagination, cursor, and token lifecycle changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/relayauth-audit-capacity-core

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d70d2bfb4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/server/src/routes/tokens.ts Outdated
Comment on lines +756 to +760
previousRefreshToken: {
id: presentedJti,
identityId: identity.id,
expiresAt: verification.claims.exp,
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle the losing refresh rotation as token reuse

When two requests present the same active refresh token concurrently, both can pass the revocation and active-token checks, but only the first transaction can commit; the second rotateIssuedPairWithAudit call throws refresh_token_not_active. Because this await is not caught, the global error handler returns 500 and the reuse-cascade branch above never runs, leaving the first request's newly issued session tokens active despite the concurrent reuse event. Catch this rotation conflict, cascade-revoke the session, and return the normal revoked-token response.

Useful? React with 👍 / 👎.

Comment on lines +105 to +109
export type AuditQueryWorkBudget = {
d1Pages: number;
d1Rows: number;
partitions: number;
r2Reads: number;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep work-budget fields provider-neutral

Replace the exported d1Pages, d1Rows, and r2Reads counters with provider-neutral accounting. These names make both the public server storage contract and the SDK response type depend on Cloudflare D1/R2 concepts, forcing Node/SQLite and other adapters to expose irrelevant provider metrics and locking Cloudflare terminology into the published API; the repository explicitly requires these contracts to use plain Node/platform-neutral terms and keep Cloudflare adapter details in the cloud repo.

AGENTS.md reference: AGENTS.md:L187-L193

Useful? React with 👍 / 👎.

Comment thread packages/server/src/storage/sqlite.ts Outdated
Comment on lines +3598 to +3602
query.cursor.inclusive && !query.cursor.chunk
? new Date(
new Date(query.cursor.timestamp).getTime() + 60_000,
).toISOString()
: query.cursor.timestamp,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject impossible archive cursor timestamps before arithmetic

A client-crafted archive cursor whose timestamp matches the ISO-shaped regex but is not a real date, such as 2026-99-99T99:99:99Z, passes parseArchiveCursor; when inclusive is true and no chunk is present, this new toISOString() call throws a RangeError, turning an invalid cursor into a 500 response. Validate that the decoded timestamp parses to a finite date and reject it with the existing invalid cursor 400 response before performing this arithmetic.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (13)
packages/server/src/routes/tokens.ts (2)

891-891: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

deferTask is now dead in issueTokenPair. The whole body (Lines 911-1013) persists atomically and never schedules deferred work, yet every call site still threads deferTask: c.get("deferTask"). Dropping the option (and the now-unused scheduleDeferredTask import) removes misleading plumbing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/tokens.ts` at line 891, Remove the unused
deferTask option from issueTokenPair and update every caller to stop passing
c.get("deferTask"). Delete the now-unused scheduleDeferredTask import, while
preserving the existing atomic token issuance behavior.

1317-1330: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant audience branch. The second check (exactly ["relayauth"] for refresh tokens) fully subsumes the first, so Lines 1319-1324 are unreachable in effect. The preceding comment also claims access tokens must list relayauth, which this function never enforces — worth trimming both.

♻️ Suggested simplification
-  // Refresh tokens are spec'd single-audience `["relayauth"]`; access tokens
-  // must list `relayauth` in their audience list for this issuer endpoint.
-  if (
-    !claims.aud.includes(REFRESH_AUDIENCE) &&
-    claims.token_type === "refresh"
-  ) {
-    return { ok: false, error: "Invalid token" };
-  }
+  // Refresh tokens are spec'd single-audience `["relayauth"]`.
   if (
     claims.token_type === "refresh" &&
     (claims.aud.length !== 1 || claims.aud[0] !== REFRESH_AUDIENCE)
   ) {
     return { ok: false, error: "Invalid token" };
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/tokens.ts` around lines 1317 - 1330, In the token
validation logic around the refresh-token audience checks, remove the redundant
includes-based branch and trim the inaccurate comment about access-token
audiences. Retain the existing exact-audience validation requiring refresh
tokens to have only REFRESH_AUDIENCE, preserving the current invalid-token
response.
packages/server/src/__tests__/e2e/rbac.test.ts (1)

24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reaching into the SDK's src by relative path couples this test to another package's internal layout. Importing from the @relayauth/sdk package entry (as the other SDK usages in this suite do) keeps the boundary intact and survives any SDK file reorganization.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/__tests__/e2e/rbac.test.ts` around lines 24 - 31, Update
the imports in the RBAC test to use the public `@relayauth/sdk` package entry
instead of relative paths into the SDK's src directory, importing matchesAny,
validateSubset, parseScope, and validateScope through the package boundary.
packages/server/src/__tests__/token-storage-contract.test.ts (1)

14-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Regex assertions over interface.ts source text are brittle. Each pattern requires the signature to stay on a single line with exact spacing; the formatting sweep in this same PR is precisely the kind of change that would break these without any contract regression. Prefer compile-time assertions against TokenStorage (which fail loudly and precisely on real contract drift):

♻️ Type-level alternative
import type { TokenStorage } from "../storage/interface.js";

test("TokenStorage owns the complete issued-token hot-path contract", () => {
  const _contract: Pick<
    TokenStorage,
    | "persistIssued"
    | "persistIssuedPairWithAudit"
    | "persistIssuedWithAudit"
    | "rotateIssuedPairWithAudit"
    | "getById"
    | "listActiveByIdentityId"
    | "listActiveBySessionId"
  > | undefined = undefined;
  assert.equal(_contract, undefined);
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/__tests__/token-storage-contract.test.ts` around lines 14
- 31, Replace the source-text regex checks in the “TokenStorage owns the
complete issued-token hot-path contract” test with a compile-time
Pick<TokenStorage, ...> assertion covering the same seven method names. Import
the TokenStorage type directly, retain the runtime assertion that the
placeholder is undefined, and remove the brittle interface-source matching.
packages/server/src/storage/compat.ts (2)

18-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

| unknown collapses this union to unknown. PolicyStorage and Pick<AuthStorage, "policies"> carry no type information here, so resolvePolicyStorage accepts any value with no checking. If the permissiveness is intentional, type PolicyStorageSource = unknown; states it honestly; otherwise drop the unknown member.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/storage/compat.ts` around lines 18 - 21, Update the
PolicyStorageSource type used by resolvePolicyStorage so it does not combine
specific storage types with unknown, which collapses the union and removes type
checking. Either remove unknown to preserve validation against PolicyStorage and
Pick<AuthStorage, "policies">, or explicitly define PolicyStorageSource as
unknown if unrestricted input is intentional.

142-150: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Keep explicit return types on the unsupported members. These now infer Promise<never>, which still satisfies AuditStorage, but the annotation is what catches signature drift in query/getActionCounts — note writeIdentitySuspendedEvent below retained its Promise<void>.

♻️ Restore annotations
-    async query(_query: AuditQueryInput, _options?: AuditQueryOptions) {
+    async query(
+      _query: AuditQueryInput,
+      _options?: AuditQueryOptions,
+    ): Promise<AuditQueryResult> {
       throw new Error("D1 audit storage adapter does not support query()");
     },
 
-    async getActionCounts(_orgId: string, _query: DashboardAuditQuery) {
+    async getActionCounts(
+      _orgId: string,
+      _query: DashboardAuditQuery,
+    ): Promise<DashboardAuditCountsResult> {
       throw new Error(
         "D1 audit storage adapter does not support getActionCounts()",
       );
     },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/storage/compat.ts` around lines 142 - 150, Restore
explicit Promise-based return types on the unsupported query and getActionCounts
methods, matching the AuditStorage contract and the retained Promise<void>
annotation pattern used by writeIdentitySuspendedEvent. Keep their existing
parameters and thrown errors unchanged.
packages/server/src/routes/dashboard-stats.ts (1)

68-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the exported cursor type instead of restating it.

This inline shape duplicates AuditArchivePartitionCursor from packages/server/src/storage/interface.ts (Lines 74-101), which this file already imports from. DashboardAuditQuery there is exactly { from?, to?, cursor?: AuditArchivePartitionCursor } — the local copy will silently drift from the contract it's passed to via getActionCounts.

♻️ Proposed change
-import { createDashboardAuditContinuationFilterKey } from "../storage/interface.js";
+import {
+  createDashboardAuditContinuationFilterKey,
+  type DashboardAuditQuery,
+} from "../storage/interface.js";
-type DashboardStatsQuery = {
-  from?: string;
-  to?: string;
-  cursor?: {
-    kind: "archive_partition";
-    orgId: string;
-    timestamp: string;
-    inclusive?: boolean;
-    chunk?: {
-      key: string;
-      sha256: string;
-    };
-    entryCursor?: {
-      timestamp: string;
-      id: string;
-    };
-    filterKey: string;
-  };
-};
+type DashboardStatsQuery = DashboardAuditQuery;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/dashboard-stats.ts` around lines 68 - 86, Replace
the inline cursor shape in DashboardStatsQuery with the imported
AuditArchivePartitionCursor type. Keep the query’s from and to fields unchanged,
and preserve cursor as an optional property while ensuring getActionCounts
receives the shared exported contract.
packages/server/src/routes/identity-activity.ts (1)

113-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Leftover local encodeCursor duplicates the now-shared encodeAuditCursor.

The budget branch uses the imported encodeAuditCursor (Line 91) while the normal branch still calls the file-local encodeCursor. They currently emit identical strings for entry cursors, so there's no behavior change — but keeping two encoders for one wire format is precisely the drift risk the shared helper removes, and it contradicts the summary's claim that normal results now use encodeAuditCursor.

♻️ Proposed change
-        nextCursor: hasMore ? encodeCursor(page[page.length - 1]) : null,
+        nextCursor: hasMore
+          ? encodeAuditCursor({
+              kind: "entry",
+              timestamp: page[page.length - 1]?.timestamp ?? "",
+              id: page[page.length - 1]?.id ?? "",
+            })
+          : null,

Then delete the local encodeCursor helper (Lines 268-279).

Also applies to: 268-279

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/identity-activity.ts` at line 113, Update the
normal pagination response in the route handler to call the shared
encodeAuditCursor helper for nextCursor, matching the budget branch. Remove the
now-unused local encodeCursor helper from the same module.
packages/sdk/typescript/src/client.ts (1)

37-49: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

AuditQueryWorkBudget is now declared three times across packages.

The same four-field shape exists here, in packages/server/src/storage/interface.ts (Lines 105-110), and inline in packages/server/src/routes/dashboard-stats.ts (Lines 29-34). It's a wire contract crossing the HTTP boundary, so the natural home is @relayauth/types — which both the server and this SDK already depend on — with the other two sites importing it. Otherwise a counter added server-side won't surface in the SDK type and the mismatch is invisible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sdk/typescript/src/client.ts` around lines 37 - 49, Move the shared
AuditQueryWorkBudget wire type into `@relayauth/types`, then import and reuse it
in AuditQueryPage in client.ts, the server storage interface, and the
dashboard-stats route instead of maintaining local or inline declarations.
Preserve the existing four-field shape and ensure all HTTP contract consumers
reference the shared type.
packages/server/src/storage/sqlite.ts (2)

3595-3619: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

One archive-cursor predicate, three hand-maintained copies. The inclusive && !chunk ? timestamp + 60_000 : timestamp upper bound plus the entryCursor tiebreak is implemented independently in the storage SQL builder, the storage in-memory matcher, and the route SQL builder. These three must agree exactly or a continuation silently duplicates or skips audit rows, and the SQL copies express the tiebreak as id < ? while the in-memory copy inverts it to id >= ? — correct today, but two inversions of one rule maintained in parallel. Extract a single helper (for example resolveArchiveCursorBounds(cursor) exported from packages/server/src/storage/interface.ts, next to the AuditArchivePartitionCursor doc comment that defines the semantics) and have all three call it; the helper is also the right place to guard against a non-ISO timestamp reaching new Date(...).

  • packages/server/src/storage/sqlite.ts#L3595-L3619: replace the inline bound/tiebreak computation in buildAuditQuerySql with the shared helper.
  • packages/server/src/storage/sqlite.ts#L3750-L3781: derive upper and the entryCursor comparison in matchesAuditQuery from the same helper instead of restating them.
  • packages/server/src/routes/audit-query.ts#L242-L268: consume the shared helper in buildAuditQuery rather than duplicating the storage predicate in the route layer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/storage/sqlite.ts` around lines 3595 - 3619, Extract and
export a shared archive-cursor bounds helper, such as
resolveArchiveCursorBounds, from interface.ts beside
AuditArchivePartitionCursor; centralize the inclusive non-chunk timestamp
adjustment, entryCursor tiebreak, and ISO timestamp validation there. Update
buildAuditQuerySql and matchesAuditQuery in
packages/server/src/storage/sqlite.ts at lines 3595-3619 and 3750-3781, and
buildAuditQuery in packages/server/src/routes/audit-query.ts at lines 242-268,
to consume the helper instead of maintaining independent predicates, preserving
equivalent SQL and in-memory behavior.

1643-1660: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move the rotation revocation SQL into a module-level constant.

rotateIssuedPairWithAudit inlines a revocation UPDATE while the rest of the file uses prepared-statement constants. Existing UPDATE_TOKEN_STATUS_SQL lacks the AND status = 'active' predicate used in this atomic changes !== 1 reuse check, so add a sibling constant with that predicate rather than duplicating the template literal.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/storage/sqlite.ts` around lines 1643 - 1660, Extract the
rotation revocation UPDATE from rotateIssuedPairWithAudit into a module-level
SQL constant, alongside UPDATE_TOKEN_STATUS_SQL. Preserve the identity and
token-matching conditions plus the status = 'active' predicate, and have the
method prepare and execute the new constant while retaining the existing changes
!== 1 validation.
packages/server/src/routes/audit-export.ts (1)

68-83: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Partial entries in the 409 body are unreachable for SDK callers.

exportAudit requests responseType: "text" and, on a non-2xx, _request throws via createRequestError, which only mines code/error/message from the payload (packages/sdk/typescript/src/client.ts Lines 472-473, 531-542). So entries, partial, and workBudget on this 409 are discarded — only the error code survives. Consider either dropping entries from the body (a 409 that discards data is cleaner than one that ships half of it), or surfacing nextCursor/workBudget on the thrown SDK error so clients can actually resume.

Also, 409 Conflict is a slightly odd fit for "budget exhausted"; 503 with Retry-After, or 206 with the partial payload, communicates the retry/resume affordance more directly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/audit-export.ts` around lines 68 - 83, Update the
budget-exhausted branch in exportAudit so the 409 response does not include
partial fields that SDK callers discard, removing entries, partial, workBudget,
and nextCursor unless the SDK error path is updated to expose them. Keep the
audit error code and existing status behavior unchanged.
packages/server/src/storage/interface.ts (1)

152-186: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Exclude limit from the audit archive continuation filter key.

The cursor contract says filterKey is the “org-scoped query scope” and the ordering guard already covers filters and ordering. limit is client pagination, so including it prevents an identical archive scan from being resumed with a different page size (for example, /v1/audit default 50 cannot be resumed by /v1/audit/export default 10000), and changing limit between pages fails as invalid cursor instead of producing the next page. Also remove limit from the parameter Pick<...> used by this key.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/storage/interface.ts` around lines 152 - 186, Update
createAuditQueryContinuationFilterKey to exclude limit from the serialized
continuation filter key, and remove "limit" from its query parameter
Pick<AuditQueryInput>. Preserve all other scope, ordering, and cursor fields
unchanged so cursors remain reusable across page sizes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/server/src/routes/audit-query.ts`:
- Around line 364-391: Update encodeAuditCursor to UTF-8 encode the archive
cursor JSON payload before passing it to toBase64Url, including filterKey values
containing characters above U+00FF. Preserve decoding compatibility with
existing ASCII-only cursors and apply the same encoding change to the related
cursor encoding path.
- Around line 91-113: Trim budget_exhausted results to parsed.value.limit before
returning them, matching the complete-response behavior. Apply this in the
budget_exhausted handler in packages/server/src/routes/audit-query.ts (lines
91-113) and the corresponding handler in
packages/server/src/routes/identity-activity.ts (lines 87-105); preserve the
continuation and metadata fields.

In `@packages/server/src/routes/dashboard-stats.ts`:
- Around line 129-137: Update the budget_exhausted response construction around
encodeAuditCursor by hoisting its result into a local constant and only adding
partial, nextCursor, and hasMore when the encoded cursor is present; otherwise
omit the continuation metadata (or surface an error) instead of falling back to
an empty cursor.

---

Nitpick comments:
In `@packages/sdk/typescript/src/client.ts`:
- Around line 37-49: Move the shared AuditQueryWorkBudget wire type into
`@relayauth/types`, then import and reuse it in AuditQueryPage in client.ts, the
server storage interface, and the dashboard-stats route instead of maintaining
local or inline declarations. Preserve the existing four-field shape and ensure
all HTTP contract consumers reference the shared type.

In `@packages/server/src/__tests__/e2e/rbac.test.ts`:
- Around line 24-31: Update the imports in the RBAC test to use the public
`@relayauth/sdk` package entry instead of relative paths into the SDK's src
directory, importing matchesAny, validateSubset, parseScope, and validateScope
through the package boundary.

In `@packages/server/src/__tests__/token-storage-contract.test.ts`:
- Around line 14-31: Replace the source-text regex checks in the “TokenStorage
owns the complete issued-token hot-path contract” test with a compile-time
Pick<TokenStorage, ...> assertion covering the same seven method names. Import
the TokenStorage type directly, retain the runtime assertion that the
placeholder is undefined, and remove the brittle interface-source matching.

In `@packages/server/src/routes/audit-export.ts`:
- Around line 68-83: Update the budget-exhausted branch in exportAudit so the
409 response does not include partial fields that SDK callers discard, removing
entries, partial, workBudget, and nextCursor unless the SDK error path is
updated to expose them. Keep the audit error code and existing status behavior
unchanged.

In `@packages/server/src/routes/dashboard-stats.ts`:
- Around line 68-86: Replace the inline cursor shape in DashboardStatsQuery with
the imported AuditArchivePartitionCursor type. Keep the query’s from and to
fields unchanged, and preserve cursor as an optional property while ensuring
getActionCounts receives the shared exported contract.

In `@packages/server/src/routes/identity-activity.ts`:
- Line 113: Update the normal pagination response in the route handler to call
the shared encodeAuditCursor helper for nextCursor, matching the budget branch.
Remove the now-unused local encodeCursor helper from the same module.

In `@packages/server/src/routes/tokens.ts`:
- Line 891: Remove the unused deferTask option from issueTokenPair and update
every caller to stop passing c.get("deferTask"). Delete the now-unused
scheduleDeferredTask import, while preserving the existing atomic token issuance
behavior.
- Around line 1317-1330: In the token validation logic around the refresh-token
audience checks, remove the redundant includes-based branch and trim the
inaccurate comment about access-token audiences. Retain the existing
exact-audience validation requiring refresh tokens to have only
REFRESH_AUDIENCE, preserving the current invalid-token response.

In `@packages/server/src/storage/compat.ts`:
- Around line 18-21: Update the PolicyStorageSource type used by
resolvePolicyStorage so it does not combine specific storage types with unknown,
which collapses the union and removes type checking. Either remove unknown to
preserve validation against PolicyStorage and Pick<AuthStorage, "policies">, or
explicitly define PolicyStorageSource as unknown if unrestricted input is
intentional.
- Around line 142-150: Restore explicit Promise-based return types on the
unsupported query and getActionCounts methods, matching the AuditStorage
contract and the retained Promise<void> annotation pattern used by
writeIdentitySuspendedEvent. Keep their existing parameters and thrown errors
unchanged.

In `@packages/server/src/storage/interface.ts`:
- Around line 152-186: Update createAuditQueryContinuationFilterKey to exclude
limit from the serialized continuation filter key, and remove "limit" from its
query parameter Pick<AuditQueryInput>. Preserve all other scope, ordering, and
cursor fields unchanged so cursors remain reusable across page sizes.

In `@packages/server/src/storage/sqlite.ts`:
- Around line 3595-3619: Extract and export a shared archive-cursor bounds
helper, such as resolveArchiveCursorBounds, from interface.ts beside
AuditArchivePartitionCursor; centralize the inclusive non-chunk timestamp
adjustment, entryCursor tiebreak, and ISO timestamp validation there. Update
buildAuditQuerySql and matchesAuditQuery in
packages/server/src/storage/sqlite.ts at lines 3595-3619 and 3750-3781, and
buildAuditQuery in packages/server/src/routes/audit-query.ts at lines 242-268,
to consume the helper instead of maintaining independent predicates, preserving
equivalent SQL and in-memory behavior.
- Around line 1643-1660: Extract the rotation revocation UPDATE from
rotateIssuedPairWithAudit into a module-level SQL constant, alongside
UPDATE_TOKEN_STATUS_SQL. Preserve the identity and token-matching conditions
plus the status = 'active' predicate, and have the method prepare and execute
the new constant while retaining the existing changes !== 1 validation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dfa0d827-bdd4-4a93-83b8-ea32fa4d66b7

📥 Commits

Reviewing files that changed from the base of the PR and between 6c339af and 5d70d2b.

📒 Files selected for processing (21)
  • packages/sdk/typescript/src/__tests__/client-audit.test.ts
  • packages/sdk/typescript/src/client.ts
  • packages/sdk/typescript/src/index.ts
  • packages/server/src/__tests__/audit-logger.test.ts
  • packages/server/src/__tests__/audit-query-api.test.ts
  • packages/server/src/__tests__/dashboard-stats-api.test.ts
  • packages/server/src/__tests__/e2e/audit.test.ts
  • packages/server/src/__tests__/e2e/rbac.test.ts
  • packages/server/src/__tests__/sqlite-storage.test.ts
  • packages/server/src/__tests__/storage-sqlite.test.ts
  • packages/server/src/__tests__/token-storage-contract.test.ts
  • packages/server/src/__tests__/tokens-route.test.ts
  • packages/server/src/db/migrations/0005_audit_hot_outbox.sql
  • packages/server/src/routes/audit-export.ts
  • packages/server/src/routes/audit-query.ts
  • packages/server/src/routes/dashboard-stats.ts
  • packages/server/src/routes/identity-activity.ts
  • packages/server/src/routes/tokens.ts
  • packages/server/src/storage/compat.ts
  • packages/server/src/storage/interface.ts
  • packages/server/src/storage/sqlite.ts

Comment thread packages/server/src/routes/audit-query.ts
Comment thread packages/server/src/routes/audit-query.ts
Comment thread packages/server/src/routes/dashboard-stats.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/server/src/__tests__/identity-activity-api.test.ts`:
- Around line 834-934: Extend the continuation test around the
storage.audit.query mock and response assertions to verify query.orgId,
cursor.timestamp, cursor.inclusive, and the supplied workBudget values. Add
rejection cases for reusing the continuation cursor with a different
organization and with mismatched activity filters, matching the cursor-binding
patterns in sibling API tests, while preserving the existing successful resume
and no-duplication assertions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 930a755d-00a6-4dbd-b2b0-50651d51f243

📥 Commits

Reviewing files that changed from the base of the PR and between 5d70d2b and e4aa5ed.

📒 Files selected for processing (12)
  • packages/sdk/typescript/src/__tests__/client-audit.test.ts
  • packages/sdk/typescript/src/client.ts
  • packages/server/src/__tests__/audit-query-api.test.ts
  • packages/server/src/__tests__/dashboard-stats-api.test.ts
  • packages/server/src/__tests__/identity-activity-api.test.ts
  • packages/server/src/__tests__/tokens-route.test.ts
  • packages/server/src/routes/audit-export.ts
  • packages/server/src/routes/audit-query.ts
  • packages/server/src/routes/dashboard-stats.ts
  • packages/server/src/routes/identity-activity.ts
  • packages/server/src/routes/tokens.ts
  • packages/server/src/storage/interface.ts
🚧 Files skipped from review as they are similar to previous changes (8)
  • packages/server/src/routes/audit-export.ts
  • packages/server/src/routes/identity-activity.ts
  • packages/sdk/typescript/src/tests/client-audit.test.ts
  • packages/server/src/routes/dashboard-stats.ts
  • packages/server/src/storage/interface.ts
  • packages/sdk/typescript/src/client.ts
  • packages/server/src/tests/tokens-route.test.ts
  • packages/server/src/routes/tokens.ts

Comment thread packages/server/src/__tests__/identity-activity-api.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/server/src/routes/dashboard-stats.ts (1)

96-106: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Check the organization context before parsing the query.

parseDashboardStatsQuery receives claims?.org at Line 98. The !claims?.org guard runs at Line 104. If the org claim is missing and the request supplies a cursor, then Line 181 compares decodedCursor.orgId against undefined, and the route answers 400 invalid cursor. The correct answer is 401 missing_org_context. Move the guard above the parse call.

♻️ Proposed fix
   const claims = (c as typeof c & { var: ScopeContextVars }).var.identity;
-  const parsedQuery = parseDashboardStatsQuery(c.req.query(), claims?.org);
-
-  if (!parsedQuery.ok) {
-    return c.json({ error: parsedQuery.error }, 400);
-  }
-
   if (!claims?.org) {
     return c.json({ error: "missing_org_context" }, 401);
   }
+
+  const parsedQuery = parseDashboardStatsQuery(c.req.query(), claims.org);
+  if (!parsedQuery.ok) {
+    return c.json({ error: parsedQuery.error }, 400);
+  }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/dashboard-stats.ts` around lines 96 - 106, Move
the missing organization-context guard in the dashboardStats route handler above
the parseDashboardStatsQuery call. Ensure requests without claims?.org return
401 with missing_org_context before any query or cursor validation occurs, while
preserving the existing parsed-query error handling for requests with
organization context.
packages/server/src/routes/audit-query.ts (1)

365-401: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

encodeAuditCursor does not validate chunk fields, but parseArchiveCursor requires them.

Lines 373-381 validate orgId, filterKey, and entryCursor. Line 389 then serializes cursor.chunk unchecked. parseArchiveCursor at Lines 452-457 rejects a chunk whose key or sha256 is an empty string. A storage backend that returns such a chunk produces a cursor that encodes successfully and fails on the next request with invalid cursor. Validate the chunk fields at encode time so the failure surfaces as a 500 on the producing request.

♻️ Proposed fix
     if (
       cursor.orgId.trim().length === 0 ||
       cursor.filterKey.length === 0 ||
+      (cursor.chunk !== undefined &&
+        (cursor.chunk.key.length === 0 || cursor.chunk.sha256.length === 0)) ||
       (cursor.entryCursor !== undefined &&
         (!isIsoTimestamp(cursor.entryCursor.timestamp) ||
           cursor.entryCursor.id.trim().length === 0))
     ) {
       return null;
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/audit-query.ts` around lines 365 - 401, Update
encodeAuditCursor’s archive_partition validation to reject any present
cursor.chunk whose key or sha256 is empty, matching parseArchiveCursor’s
requirements; keep valid chunk values serialized unchanged so invalid storage
data fails during cursor production.
🧹 Nitpick comments (6)
packages/server/src/routes/dashboard-stats.ts (2)

72-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the shared archive cursor type instead of redeclaring it.

DashboardStatsQuery.cursor restates the archive-partition cursor shape that storage/interface.ts already exports as part of AuditQueryCursor. The route also assigns a decodeAuditCursor result into this field, so the two declarations must stay identical. A future field added to the shared contract will not reach this local copy.

♻️ Proposed fix
+import type { AuditQueryCursor } from "../storage/interface.js";
+
 type DashboardStatsQuery = {
   from?: string;
   to?: string;
-  cursor?: {
-    kind: "archive_partition";
-    orgId: string;
-    timestamp: string;
-    inclusive?: boolean;
-    chunk?: {
-      key: string;
-      sha256: string;
-    };
-    entryCursor?: {
-      timestamp: string;
-      id: string;
-    };
-    filterKey: string;
-  };
+  cursor?: Extract<AuditQueryCursor, { kind: "archive_partition" }>;
 };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/dashboard-stats.ts` around lines 72 - 90, Replace
the locally redeclared cursor shape in DashboardStatsQuery with the shared
AuditQueryCursor type exported from storage/interface.ts. Preserve the existing
optional cursor property and ensure the decodeAuditCursor result remains
assignable without duplicating the archive-partition fields.

140-148: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the non-null assertion by branching on the encoded cursor.

nextCursor! at Line 144 is safe today because Lines 119-121 return early. The assertion depends on that remote guard. Branch on nextCursor directly so the type narrows without an assertion.

♻️ Proposed fix
-    ...(auditResult.kind === "budget_exhausted"
+    ...(nextCursor
       ? {
           partial: true as const,
-          nextCursor: nextCursor!,
+          nextCursor,
           hasMore: true as const,
         }
       : {}),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/dashboard-stats.ts` around lines 140 - 148, Update
the budget_exhausted branch in the audit result response construction to branch
directly on the encoded nextCursor value, allowing TypeScript to narrow it
before assigning nextCursor. Remove the non-null assertion while preserving
partial: true and hasMore: true behavior.
packages/server/src/storage/interface.ts (1)

523-531: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Document or enforce minute alignment of the inclusive cursor timestamp.

The doc comment states "inclusive-minute arithmetic". The implementation adds a fixed 60 seconds to cursor.timestamp. If a producer emits a cursor timestamp that is not aligned to a minute boundary, for example 12:00:59.000Z, the bound becomes 12:01:59.000Z and overlaps the next partition minute.

Either truncate to the minute before adding 60 seconds, or state in the contract that timestamp must be minute-aligned when inclusive is true.

Also applies to: 555-555

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/storage/interface.ts` around lines 523 - 531, Update
getAuditArchiveCursorUpperBound to enforce minute alignment for inclusive
cursors by truncating cursor.timestamp to the start of its minute before adding
the 60-second exclusive upper-bound offset. Preserve existing behavior for
already minute-aligned timestamps and ensure SQL and in-memory callers receive
the same normalized result.
packages/server/src/__tests__/storage-sqlite.test.ts (2)

110-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The in-memory fallback trigger depends on undocumented driver behavior.

The test passes a directory path to createSqliteStorage and relies on better-sqlite3 failing to open it, and on BackendProvider converting that failure into the memory backend. If the driver error message or the provider fallback condition changes, this test silently exercises the SQLite path instead of the memory path, and the assertion still passes.

Assert the selected backend explicitly, or add a supported test-only way to select the memory backend.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/__tests__/storage-sqlite.test.ts` around lines 110 - 122,
Update the test around createSqliteStorage and assertInclusiveArchiveCursorQuery
to select the memory backend through an explicit supported test configuration or
verify the provider’s selected backend before running the assertion. Remove the
dependency on passing a directory and on better-sqlite3 failure behavior, while
preserving the existing cleanup and query coverage.

67-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the remaining getAuditArchiveCursorUpperBound branches.

The new tests cover the inclusive branch and an invalid cursor.timestamp. Three branches stay uncovered:

  • An invalid entryCursor.timestamp, which must raise "archive entry cursor timestamp must be an ISO 8601 timestamp".
  • A cursor with inclusive: false, which must keep the bound at cursor.timestamp.
  • A cursor with chunk set and inclusive: true, which must also keep the bound at cursor.timestamp.

The chunk case is the one most likely to regress, because it overrides the inclusive minute arithmetic.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/__tests__/storage-sqlite.test.ts` around lines 67 - 102,
Add tests covering the remaining getAuditArchiveCursorUpperBound branches:
reject an invalid entryCursor.timestamp with the expected message, verify
inclusive: false preserves cursor.timestamp as the bound, and verify a cursor
with chunk plus inclusive: true also preserves cursor.timestamp rather than
applying inclusive minute arithmetic. Keep the existing storage-opening
assertions and test setup consistent with the current audit cursor tests.
packages/server/src/routes/audit-query.ts (1)

427-493: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer an explicit statement sequence over one large ternary condition.

parseArchiveCursor combines version, kind, org, timestamp, chunk, entry-cursor, and filter-key validation into a single boolean expression, then rebuilds the object with as string casts. The casts are needed only because the narrowing is lost across the expression. Sequential guard clauses remove the casts and make each rejection reason greppable.

This is a readability change only. Behavior stays the same.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server/src/routes/audit-query.ts` around lines 427 - 493, Refactor
parseArchiveCursor to validate the parsed fields through sequential guard
clauses, preserving all existing validation and null-return behavior. Validate
version, kind, orgId, timestamp, inclusive, chunk, entryCursor, and filterKey
individually, then construct the cursor only after narrowing succeeds so the
current as string casts are unnecessary. Keep the returned object shape and
normalization unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/server/src/storage/interface.ts`:
- Around line 551-564: Update getAuditArchiveCursorUpperBound() to parse and
normalize every valid cursor.timestamp to its UTC ISO representation before
returning or applying the one-minute upper-bound adjustment, including
non-inclusive and chunk cursors. Also normalize cursor.entryCursor.timestamp at
this storage contract boundary so both archive cursor values use UTC
consistently for SQL and in-memory comparisons.

---

Outside diff comments:
In `@packages/server/src/routes/audit-query.ts`:
- Around line 365-401: Update encodeAuditCursor’s archive_partition validation
to reject any present cursor.chunk whose key or sha256 is empty, matching
parseArchiveCursor’s requirements; keep valid chunk values serialized unchanged
so invalid storage data fails during cursor production.

In `@packages/server/src/routes/dashboard-stats.ts`:
- Around line 96-106: Move the missing organization-context guard in the
dashboardStats route handler above the parseDashboardStatsQuery call. Ensure
requests without claims?.org return 401 with missing_org_context before any
query or cursor validation occurs, while preserving the existing parsed-query
error handling for requests with organization context.

---

Nitpick comments:
In `@packages/server/src/__tests__/storage-sqlite.test.ts`:
- Around line 110-122: Update the test around createSqliteStorage and
assertInclusiveArchiveCursorQuery to select the memory backend through an
explicit supported test configuration or verify the provider’s selected backend
before running the assertion. Remove the dependency on passing a directory and
on better-sqlite3 failure behavior, while preserving the existing cleanup and
query coverage.
- Around line 67-102: Add tests covering the remaining
getAuditArchiveCursorUpperBound branches: reject an invalid
entryCursor.timestamp with the expected message, verify inclusive: false
preserves cursor.timestamp as the bound, and verify a cursor with chunk plus
inclusive: true also preserves cursor.timestamp rather than applying inclusive
minute arithmetic. Keep the existing storage-opening assertions and test setup
consistent with the current audit cursor tests.

In `@packages/server/src/routes/audit-query.ts`:
- Around line 427-493: Refactor parseArchiveCursor to validate the parsed fields
through sequential guard clauses, preserving all existing validation and
null-return behavior. Validate version, kind, orgId, timestamp, inclusive,
chunk, entryCursor, and filterKey individually, then construct the cursor only
after narrowing succeeds so the current as string casts are unnecessary. Keep
the returned object shape and normalization unchanged.

In `@packages/server/src/routes/dashboard-stats.ts`:
- Around line 72-90: Replace the locally redeclared cursor shape in
DashboardStatsQuery with the shared AuditQueryCursor type exported from
storage/interface.ts. Preserve the existing optional cursor property and ensure
the decodeAuditCursor result remains assignable without duplicating the
archive-partition fields.
- Around line 140-148: Update the budget_exhausted branch in the audit result
response construction to branch directly on the encoded nextCursor value,
allowing TypeScript to narrow it before assigning nextCursor. Remove the
non-null assertion while preserving partial: true and hasMore: true behavior.

In `@packages/server/src/storage/interface.ts`:
- Around line 523-531: Update getAuditArchiveCursorUpperBound to enforce minute
alignment for inclusive cursors by truncating cursor.timestamp to the start of
its minute before adding the 60-second exclusive upper-bound offset. Preserve
existing behavior for already minute-aligned timestamps and ensure SQL and
in-memory callers receive the same normalized result.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f31c895f-5598-4ed0-be04-9b416978b5ce

📥 Commits

Reviewing files that changed from the base of the PR and between e4aa5ed and 950652e.

📒 Files selected for processing (7)
  • packages/server/src/__tests__/dashboard-stats-api.test.ts
  • packages/server/src/__tests__/identity-activity-api.test.ts
  • packages/server/src/__tests__/storage-sqlite.test.ts
  • packages/server/src/routes/audit-query.ts
  • packages/server/src/routes/dashboard-stats.ts
  • packages/server/src/storage/interface.ts
  • packages/server/src/storage/sqlite.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/server/src/tests/identity-activity-api.test.ts
  • packages/server/src/tests/dashboard-stats-api.test.ts
  • packages/server/src/storage/sqlite.ts

Comment thread packages/server/src/storage/interface.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/server/src/storage/interface.ts`:
- Around line 545-566: Update normalizeAuditEntryCursor to return the trimmed
cursor.id alongside the normalized timestamp, while preserving validation and
other cursor fields. Ensure normalizeAuditQueryCursor’s entry and
archive_partition entryCursor paths continue using this canonicalized ID through
normalizeAuditEntryCursor.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5ead68c0-dddb-49d1-9e65-687fcbc5fc85

📥 Commits

Reviewing files that changed from the base of the PR and between 7da6b42 and 3efbe6b.

📒 Files selected for processing (8)
  • packages/server/src/__tests__/audit-query-api.test.ts
  • packages/server/src/__tests__/dashboard-stats-api.test.ts
  • packages/server/src/__tests__/identity-activity-api.test.ts
  • packages/server/src/__tests__/storage-sqlite.test.ts
  • packages/server/src/routes/audit-query.ts
  • packages/server/src/routes/dashboard-stats.ts
  • packages/server/src/storage/interface.ts
  • packages/server/src/storage/sqlite.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • packages/server/src/routes/dashboard-stats.ts
  • packages/server/src/routes/audit-query.ts
  • packages/server/src/tests/identity-activity-api.test.ts
  • packages/server/src/tests/dashboard-stats-api.test.ts
  • packages/server/src/storage/sqlite.ts

Comment thread packages/server/src/storage/interface.ts
@khaliqgant

Copy link
Copy Markdown
Member Author

Canonical Claude review: GREEN at exact head 954c428 / tree a25d1e48b7649432f621a3e0205ea9c5b069d7c1.

Fresh read-only whole-PR review found no blockers. It verified atomic issue/rotate/revoke plus audit transactions in SQLite and memory; bounded audit query and dashboard count contracts; valid-calendar and UTC normalization for ordinary/archive cursors and from/to/filter keys; UTC-minute archive alignment; SQL/memory parity; direct storage boundaries; UTF-8/fail-closed cursor codecs; and both ID-trim paths in the final three-file delta. The regressions catch a one-line-only incomplete fix in SQLite, forced-memory, and codec paths.

Exact CI 30617488021 and contract 30617488009 are successful; CodeRabbit is successful; GitHub is MERGEABLE/CLEAN with zero current threads. Nonblocking residuals: an unused type-only import, redundant idempotent archive normalization, and dormant migration 0005 schema reserved for the Cloud adapter. No source or provider mutation was performed by the reviewer.

@khaliqgant

Copy link
Copy Markdown
Member Author

Canonical Codex review: RED at exact head 954c428 / tree a25d1e48b7649432f621a3e0205ea9c5b069d7c1.

Merge blocker: packages/server/src/routes/identity-activity.ts still uses a private raw btoa(timestamp|id) encoder for ordinary complete-result pagination and does not fail closed when encoding yields null, while archive continuations already use the shared encodeAuditCursor. A valid non-Latin-1 audit ID can throw InvalidCharacterError; malformed provider output can advertise hasMore:true with nextCursor:null.

Required repair: use shared encodeAuditCursor for the ordinary continuation, add the same explicit null guard as /v1/audit, remove the private encoder, and add ordinary UTF-8 plus fail-closed regressions. A fresh isolated two-file repair is active. No merge at this head.

@khaliqgant

Copy link
Copy Markdown
Member Author

RELAYAUTH-RECOVERY-2026-07-31 canonical fresh Claude review: GREEN on exact head 9588b9825765c4dfde2ae7ef3039c853da813988, tree 11d2bfe86147ab29c3250f492f5f5c5a1e453f8b, base 6c339afe5918b6985e30f43ed8e4f4715211fd56.

Substantive findings: the ordinary identity-activity complete-result path now uses shared UTF-8-safe encodeAuditCursor; the null-to-500 guard matches /v1/audit; raw route-local btoa is removed; archive behavior is unchanged; and the UTF-8 round-trip plus malformed-continuation fail-closed regressions are meaningful. Composition with the earlier UTC normalization, ID trimming, bounded query/count, and atomic revoke/audit changes is sound. No merge blocker. Nonblocking: the ?? "" fallbacks are unreachable but harmless; pre-existing Cloudflare simulation test stubs are outside this two-file delta.

Automation reported separately: exact-head CI and SDK Contract are SUCCESS, CodeRabbit SUCCESS, Cubic NEUTRAL, PR MERGEABLE/CLEAN, and there is no current unresolved review thread. Fresh Codex review starts only after this Claude verdict.

@khaliqgant

Copy link
Copy Markdown
Member Author

RELAYAUTH-RECOVERY-2026-07-31 required sequential fresh Codex review: RED on exact head 9588b9825765c4dfde2ae7ef3039c853da813988, tree 11d2bfe86147ab29c3250f492f5f5c5a1e453f8b.

Merge blocker: encodeAuditCursor validates archive org/filter but accepts a present chunk with empty key/hash; decodeAuditCursor rejects the resulting advertised cursor. Exact reproduction: malformed archive chunk: { key: "", sha256: "" } encodes non-null, then decodes null. Malformed storage output therefore bypasses the route null-to-500 guard, advertises an unusable continuation, and replays as 400. The shared normalizeAuditQueryCursor boundary should reject empty/whitespace-only chunk fields. A red-first repair is underway with codec inverse coverage and one /v1/audit route-level fail-closed regression. Merge remains held.

Affected shared-codec consumers: /v1/audit, identity activity, stats, and audit export. Exact-head automation remains green but does not override this reproducible blocker.

@khaliqgant

Copy link
Copy Markdown
Member Author

Fresh Codex RED blocker repaired and live branch fast-forwarded to exact head ea0a6d9a4b217b76880ef4d2557b7818777c636e, tree 4e77b82555d1280acc86cf000982fda7bb9eac5a, parent 9588b9825765c4dfde2ae7ef3039c853da813988.

Red-before: the codec test and route-level malformed budget continuation test both failed (18/20): empty chunk encoded non-null and /v1/audit returned 200 instead of fail-closed 500. Repair: shared normalizeAuditQueryCursor now rejects a present chunk whose key or sha256 is empty/whitespace while preserving valid opaque values byte-for-byte. Green: audit 20/20; composed audit/identity/stats/export 55/55; independent full server replay 456/456; server typecheck and diff check green. Diff is two files, +81/-0. Exact-head hosted CI/bots and fresh sequential Claude→Codex reviews restart now; merge remains held.

@khaliqgant

Copy link
Copy Markdown
Member Author

RELAYAUTH-RECOVERY-2026-07-31 exact-head review update: Claude RED at ea0a6d9a4b217b76880ef4d2557b7818777c636e / tree 4e77b82555d1280acc86cf000982fda7bb9eac5a.

Blocking finding: createAuditQueryContinuationFilterKey includes cursor.entryCursor, even though that value is pagination position rather than query scope. A first bounded archive response emitted for a cursorless query carries a filter key whose entryCursor component is null. Replaying its continuation causes parseAuditQuery to recompute the key with the continuation's non-null mid-page entryCursor, so the request is rejected with 400 invalid cursor.

This affects /v1/audit and identity activity through their shared parser. Existing hosted tests stay green because the Cloud D1/R2 adapter is the out-of-tree producer of entry-cursor-bearing budget_exhausted continuations; current route tests do not replay that shape.

Merge remains held. A fresh isolated red-first repair is adding an end-to-end emit-and-replay regression and removing pagination position from the scope key if the contract audit confirms that interpretation. Exact-head Claude→Codex reviews will restart after the repair.

@khaliqgant

Copy link
Copy Markdown
Member Author

RELAYAUTH-RECOVERY-2026-07-31 repair landed at exact head d2394319d30ac9a8cadf5cd74c40c8a11409e865 / tree dbebce1b1154776d6d4ff324f36a689f09b904d9, parent ea0a6d9a4b217b76880ef4d2557b7818777c636e.

The canonical audit continuation filter key now contains only query scope—resource, ordering, actual filters, canonical time bounds, and limit. Pagination position remains exclusively in the cursor, so a mid-page archive continuation replays under the same scope key.

Regression evidence:

  • red before: audit route 19/20; page-two replay returned 400 instead of 200
  • green after: focused audit route 20/20
  • composed audit/identity-activity/dashboard/export consumers 55/55
  • full server suite 456/456
  • server typecheck and diff check pass
  • route regression emits an entryCursor-bearing budget_exhausted continuation, replays it, verifies the storage boundary receives it, retains scope-mismatch rejection, and proves no duplicate/skipped aggregate entries

Hosted exact-head CI/bots and the mandatory fresh Claude→Codex sequence restart now. Merge/publish remain held.

@khaliqgant

Copy link
Copy Markdown
Member Author

RELAYAUTH-RECOVERY-2026-07-31 final exact-head signoff: head d2394319d30ac9a8cadf5cd74c40c8a11409e865, tree dbebce1b1154776d6d4ff324f36a689f09b904d9, base 6c339afe5918b6985e30f43ed8e4f4715211fd56. Fresh sequential Claude then Codex reviews are both GREEN. They independently verified the scope-only continuation key, cursorless-to-mid-page replay without duplicate/skip, strict mismatch rejection, atomic issue/rotate/revoke behavior, SQLite/memory parity, bounded audit/count consumers, UTF-8/chunk fail-closed behavior, and the additive platform-neutral 0005 migration. Exact hosted Build & Test, SDK contract, and CodeRabbit gates are green; current unresolved non-outdated threads are zero. Nonblocking residual is limited to tightening provider-contract documentation/test hardening.

@khaliqgant
khaliqgant merged commit ccbd1a4 into main Jul 31, 2026
4 checks passed
@khaliqgant
khaliqgant deleted the codex/relayauth-audit-capacity-core branch July 31, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants