Skip to content

Record a computer reset as soon as the profile is gone - #404

Merged
davidmckayv merged 3 commits into
CopilotKit:mainfrom
kevin9327:fix/reset-audit-order
Sep 6, 2026
Merged

Record a computer reset as soon as the profile is gone#404
davidmckayv merged 3 commits into
CopilotKit:mainfrom
kevin9327:fix/reset-audit-order

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What this changes

resetComputer says of itself:

The most destructive button we have. Every login the Bot had is gone and no undo exists, so the
row is written whatever happens next.

The row is written last:

const result = await provider.reset(botId);   // the profile is gone HERE
await snapshots.clear(botId);                 // Postgres delete
await pageFrames?.clear(botId);               // Postgres delete
await writeControlEvent(auditStore, "computer.reset", {});

Both clears are database deletes. A connection reset, a failover or a statement timeout in either
throws before the row is reached, and the throw propagates. The profile is already destroyed by
then — provider.reset ran first and there is no undo — so the outcome is a computer with every
login wiped and nothing on the trail saying who did it or when. That is precisely what the note
above rules out, and it is the one action where the trail is the only record that survives.

Measured, with a snapshot store whose clear rejects:

Expected to contain: "computer.reset"
Received: []

and the same for the screenshot store.

Fix

The row is written as soon as provider.reset returns, which is the point of no return, and the two
clears follow. Nothing else moves: the reason string still comes from result.cleared, both clears
still run in the same order, and a failure in either still propagates, so the caller is still told
the clears did not finish.

Writing it before the deletes rather than wrapping them is deliberate. Swallowing the delete failure
would make the row a claim the deployment cannot back — the screenshots are the part the comment
below snapshots.clear says made "every login is gone" untrue in the first place — so the honest
record is a reset that happened, with the failure surfaced to whoever pressed the button.

Where it runs

  • New state that outlives a request? None. The same row, written earlier in the same call.
  • What happens on the second replica? No change. The audit row goes to Postgres as before,
    and both clears are keyed by botId, so a second replica sees the same reset with the same
    row whichever process served it.
  • Anything serialised? Nothing new. provider.reset remains the single destructive step and
    the deletes are unconditional by botId, so two resets racing converge on the same end state.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? None.

Boundary and audit

  • Every acting call still goes through the gateway: this is the gateway, and the order of its
    own steps is what changed.
  • New refusals and new failures each write a row: this is the point. A reset followed by a
    failed delete used to write no row at all; now it writes one and still reports the failure.
  • Nothing new is trusted from the client: nothing new is read.

Changelog

  • A line in CHANGELOG.md under Unreleased.

Tests

Two in server/tests/computer-gateway.test.ts, beside the existing reset tests:

  • a snapshot store whose clear rejects: the reset still records computer.reset against the bot,
    and the failure still reaches the caller
  • a screenshot store whose clear rejects: same

Against main both fail with Received: [].

How I tested

Windows 11, Bun 1.3.14. bun test server/tests/computer-gateway.test.ts is 59 passed, 0 failed, up
from 57 by the two new tests. Neighbours unchanged: computer-client 18, computer-sandbox 8,
computer-provider 12, computer-session-guard 5, audit 7, all passing.
bun run --filter server typecheck and bunx biome check are clean.

@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.

Deep-reviewed against live code (correctness, governance, no vendor/secret/scale issues). Composed build+tests green. CHANGELOG/format rebase on CI-validated substance.

@davidmckayv
davidmckayv merged commit d97641e into CopilotKit:main Sep 6, 2026
14 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