Keep the snapshot a ref resolves against where every replica can read it - #46
Open
kevin9327 wants to merge 1 commit into
Open
Keep the snapshot a ref resolves against where every replica can read it#46kevin9327 wants to merge 1 commit into
kevin9327 wants to merge 1 commit into
Conversation
The gateway turns the opaque ref in an acting call into the element it points at, from the snapshot this server took, and then decides and records against that element. That mapping lived in a Map in the process. OpenBot runs several server processes behind a load balancer, and the process that answers a snapshot is rarely the one that answers the click that uses its refs, so on every other replica the Map was empty: the ref resolved to nothing, the policy decided with no element in front of it, and the audit row could not name what was touched. Neither outcome is loud. A deny rule written against the element fails closed and refuses every click on the replicas that did not snapshot, which reads as the computer being flaky. A rule that does not name the element lets the click through unresolved and unrecorded, which reads as the boundary being quiet because nothing matched. Either way the boundary is not doing what the operator wrote, and nothing says so. CopilotKit#21 took two features back for this exact shape and noted this cache had it too. So the snapshot goes through Postgres, the way channel activity and the policy already do: one row per computer, upserted on every snapshot, read on the action path. A ref resolves on whichever replica the click lands on. Staleness, the reason a persisted snapshot cache is rightly regarded with suspicion, is answered by the generation the far-side computer stamps on every snapshot. A ref resolves only when its generation matches the stored one, so a ref from a superseded page resolves to nothing rather than to whatever now holds it: it cannot resolve to a name that is no longer on screen, because a ref from an old screen no longer matches. The computer makes the same generation check when the action reaches it; this keeps the policy decision and the audit row honest first, on whichever replica the action landed. The client-supplied snapshotId only ever narrows resolution. It can yield the true element of the current snapshot or nothing, never a different element than the ref already names, so nothing new is trusted from the client that the server does not resolve itself. Without a database the gateway still keeps snapshots in memory, so a single-process test does not need Postgres, exactly as the policy store does not. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kevin9327
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
August 20, 2026 11:16
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
The gateway resolves the opaque
refin an acting call into the element it points at, from thesnapshot this server took, and then decides and audits against that element — step 1 of
resolve → decide → record → act, the one the file header calls "fatal to skip". That mapping lived
in a
Mapin the process.With more than one server process the snapshot lands on one and the click that uses its refs on
another, so the
Mapis empty on the replica that answers the click: the ref resolves to nothing,the policy decides with no
elementin its context, and the audit row can only record"not in the current snapshot". A deny rule written against the element then fails closed andrefuses every click on those replicas, which reads as a flaky computer; a rule that does not name the
element lets the click through unresolved and unrecorded, which reads as the boundary being quiet
because nothing matched. Either way the boundary stops doing what the operator wrote, and nothing
says so. This is the cache #21 flagged as still having the same problem, after taking two features
back for exactly this shape.
This moves the snapshot into Postgres — one row per computer (
computer_snapshot), upserted on everysnapshot, read on the action path — so a ref resolves on whichever replica the click lands on. A ref
resolves only when the
snapshotIdit carries matches the stored generation, so a ref from asuperseded page resolves to nothing rather than to whatever now holds it: the concern that a
persisted cache would "decide on fiction" is answered by the generation, not by keeping the cache in
memory. The agent-computer still makes the same generation check when the action arrives; this keeps
the policy decision and the audit row honest first, on whichever replica the action landed.
Where it runs
table
computer_snapshot, keyed bycomputer_id— not aMapor a closure.the ref, so the policy sees the real element and the audit row names it. Before this change the
ref resolved to nothing there and the boundary was blind.
computer_id, written withINSERT … ON CONFLICT (computer_id) DO UPDATE. Two processes snapshotting the same computer:last write wins, which is the correct meaning, because the newest generation is the current
page. No check-then-write.
action path, never pushed to a socket.
Boundary and audit
resolvechanged — it reads the shared snapshot and matches the generation.resolves, the row says so, and the computer refuses it on arrival.
snapshotIdonly narrows resolution: it yields thetrue element of the current snapshot or nothing, never a different element than the ref already
names. The ref-to-element mapping stays server-held.
Proof
Local (Windows, Bun 1.3.14, no Postgres):
bun run typecheckacross app/server/worker — clean.biome check(format, lint, organizeImports) on every changed file — clean.bun test server/tests/computer-gateway.test.ts server/tests/computer-snapshot-store.test.ts— 24 pass. The three new gateway tests build two gateways over one shared store (two replicas, one
database) and prove a click is resolved and refused on the replica that never took the snapshot,
that the resolved element label is available there, and that a ref only resolves against its own
generation.
channel-storeandcredential tests that need Postgres and the agent-computer symlink tests that need Windows symlink
permission — both environmental, neither in the changed code.
CI covers what this machine cannot:
drizzle-kit migrateapplies0001_computer_snapshot, andcomputer-snapshot-store.integration.test.ts(modelled onpolicy-durability.integration.test.ts)proves a snapshot saved by one store is resolvable by a second store on the same database, that a
newer snapshot supersedes the last with one row per computer, and that the JSON round-trip keeps each
element's
type.This is independent of my open #45 (fleet listing); it touches
computer/gateway.ts, a newcomputer/snapshot-store.ts, thecomputer_snapshottable, and the composition root only.