Skip to content

Walk the audit trail without skipping rows written in the same millisecond - #586

Merged
davidmckayv merged 2 commits into
CopilotKit:mainfrom
kevin9327:audit-cursor-microseconds
Sep 16, 2026
Merged

davidmckayv merged 2 commits into
CopilotKit:mainfrom
kevin9327:audit-cursor-microseconds

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What this changes

GET /api/admin/audit-events pages on (created_at, id), and its nextCursor carries the last row's createdAt. That value came from the row as the driver returns it, a JavaScript Date, which keeps milliseconds. audit_events.created_at is a plain timestamptz defaulted to now(), which keeps microseconds.

So the cursor names a moment slightly before the row it was taken from. The next page asks for created_at < cursor OR (created_at = cursor AND id < cursor.id). A row written earlier in the same millisecond, or at the same instant (one statement or one transaction shares now()), is newer than the truncated cursor. It fails both halves and lands on no page. The walk carries on or ends with nothing to show a row was missed.

The schema already expects this case. The comment on the audit indexes says "two rows written in the same millisecond are ordered by id", and the cursor is meant to handle that. It could not, because both rows compare as newer than the cursor.

The fix, in createAuditReader.list:

  • The query also selects created_at formatted to the microsecond in UTC (to_char(... 'YYYY-MM-DD"T"HH24:MI:SS.US"Z"')), and nextCursor carries that string. The field is removed before the events are returned, so the response body is unchanged.
  • The cursor's timestamp is bound as text and cast with ::timestamptz rather than passed through new Date(...), which would truncate it again.

A cursor issued before this change still decodes and still pages as it did. decodeCursor is unchanged: the microsecond ISO string passes its Date.parse check. Ordering, filters, the limit and the response body are unchanged.

The channel roster cursor (channels/routes.ts) has the same shape, but its timestamps rarely share a millisecond: last_message_at is written from a JavaScript Date, and created_at only decides for a channel with no messages. I left it alone to keep this change to the audit trail.

Where it runs

  • New state that outlives a request? None. The cursor is still a string handed to the caller.
  • What happens on the second replica? The same. The timestamp is formatted by PostgreSQL, so every replica writes the same cursor for the same row.
  • Anything serialised? No.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act. Reading the trail is not an acting call.
  • New refusals and new failures each write a row. None are added. Rows that were already written can now be read.
  • Nothing new is trusted from the client that the server can resolve itself. The cursor is validated as before and bound as a parameter.

Changelog

  • A line in CHANGELOG.md under Unreleased.

Proof

Two cases added to server/tests/audit-cursor.integration.test.ts. Each writes three rows in one INSERT, so they share now(), at date_trunc('milliseconds', now()) plus a few microseconds. It then walks createAuditReader(database).list with limit: 1, following nextCursor. The first case puts the rows microseconds apart. The second puts them at one instant, so only the id orders them.

I have no local PostgreSQL on this machine, so this ran on GitHub Actions in my fork. The job used the same pgvector/pgvector:pg17 service, bun install --frozen-lockfile, drizzle-kit migrate and Bun 1.3.14 as ci.yml.

With server/src/audit.ts from main (only the test added):

(fail) a cursor over rows written within one millisecond > reaches every row, newest first, when they are microseconds apart
error: expect(received).toEqual(expected)
  [
    "7fe6dd78-7912-44be-ba12-203bcaacd781",
-   "76614afb-0501-471a-99eb-97a44ae69478",
-   "1c8a7fa0-0cbc-4205-9935-78d7f4dd83b1",
  ]
(fail) a cursor over rows written within one millisecond > reaches every row when they share one instant
 3 pass
 2 fail

The walk returned the first row and a cursor, and the next page was empty.

With the fix:

  • bun test --coverage server/tests/audit-cursor.integration.test.ts: 5 pass, 0 fail. Every changed line in audit.ts executes. The uncovered lines the report lists are all outside this change.
  • bun test on audit-cursor.test.ts, audit.test.ts, audit-limit.test.ts, audit-initiator.integration.test.ts, audit-retention.integration.test.ts, computer-policy-dry-run-limit.test.ts and policy-dry-run.test.ts: 83 pass, 0 fail.
  • Locally: bun run typecheck (app, server, worker) exits 0, bunx biome check on the changed files is clean, and the three non-database audit test files pass 30/30.

This PR adds its entry at the top of ## Unreleased, where other open PRs add theirs, so the changelog may conflict. I can rebase if it does.

🤖 Generated with Claude Code

…ame millisecond are not skipped

created_at keeps microseconds and a JavaScript Date keeps milliseconds, so
the cursor built from a row named a moment just before that row. The rows
the next page should start with, written earlier in the same millisecond or
at the same instant, then compared as newer than the cursor and were on no
page at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@davidmckayv davidmckayv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code-verified clean; CI green on this sha.

@davidmckayv
davidmckayv merged commit a911bb9 into CopilotKit:main Sep 16, 2026
17 checks passed
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