Skip to content

Keep the snapshot a ref resolves against where every replica can read it - #46

Open
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/gateway-snapshot-cross-replica
Open

Keep the snapshot a ref resolves against where every replica can read it#46
kevin9327 wants to merge 1 commit into
CopilotKit:mainfrom
kevin9327:fix/gateway-snapshot-cross-replica

Conversation

@kevin9327

Copy link
Copy Markdown

What this changes

The gateway resolves the opaque ref in an acting call into the element it points at, from the
snapshot 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 Map in the process.

With more than one server process the snapshot lands on one and the click that uses its refs on
another, so the Map is empty on the replica that answers the click: the ref resolves to nothing,
the policy decides with no element in its context, and the audit row can only record
"not in the current snapshot". A deny rule written against the element then fails closed and
refuses 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 every
snapshot, read on the action path — so a ref resolves on whichever replica the click lands on. A ref
resolves only when the snapshotId it carries matches the stored generation, so a ref from a
superseded 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

  • New state that outlives a request? The last snapshot per computer. It lives in Postgres,
    table computer_snapshot, keyed by computer_id — not a Map or a closure.
  • What happens on the second replica? The click loads the snapshot from Postgres and resolves
    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.
  • Anything serialised? One row per computer, primary key computer_id, written with
    INSERT … 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.
  • Anything fanned out to a browser? No. This is server-side resolution state, read on the
    action path, never pushed to a socket.
  • New listener, port, or schedule? None. Same ingress, same Postgres.

Boundary and audit

  • Every acting call still goes through the gateway: resolve, decide, audit, then act. Only
    resolve changed — it reads the shared snapshot and matches the generation.
  • New refusals and new failures each write a row. A superseded ref is unchanged: no element
    resolves, the row says so, and the computer refuses it on arrival.
  • Nothing new is trusted from the client. The snapshotId only narrows resolution: it yields the
    true 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 typecheck across 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.
  • The rest of the non-integration suite passes. The only local failures are the channel-store and
    credential 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 migrate applies 0001_computer_snapshot, and
computer-snapshot-store.integration.test.ts (modelled on policy-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 new
computer/snapshot-store.ts, the computer_snapshot table, and the composition root only.

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

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.

1 participant