A snapshot save in flight across a reset brings the wiped page back, and a restarted computer's snapshots stop landing

save in server/src/computer/snapshot-store.ts is an INSERT ... ON CONFLICT DO UPDATE, and its only-ever-forward guard is setWhere: lt(computerSnapshot.snapshotId, values.snapshotId) at line 108. setWhere qualifies the update branch, so it decides nothing when there is no row to conflict with.

resetComputer deletes the row (server/src/computer/gateway.ts:577) precisely because the refs the last snapshot handed out describe a page that no longer exists and a fresh computer counts generations from one again. Between the computer answering /snapshot and the save landing (gateway.ts:313) there is a window, and a reset inside it leaves the delete already done. The in-flight save then inserts unconditionally.

What follows:

Session A reaches generation 7. The row holds A's page.
A snapshot is in flight.
The reset lands. The profile is wiped, the row is deleted.
The in-flight save arrives, finds no row, and inserts. The wiped session's page is back at generation 7.
The fresh computer counts from one. Its saves take the conflict path, lt(7, 1) is false, and the update is skipped without error.

So refs resolve against a page from a session that was deleted, which is the thing step 3 exists to prevent, and no fresh snapshot lands until the generation climbs past the dead one's.

The larger half

Step 5 does not need the race. agent-computer/src/index.ts:124 keeps the generation in an in-process Map, sessionFor mints snapshotId: 0 for any Bot it is not already holding (:148), and forgetIdleSessions drops sessions whose profile is not live once the map passes 32 (:140-143). Generations therefore restart at one after any container restart, redeploy, crash, or eviction, and none of those clear the server's row. A restarted computer's snapshots are then silently dropped until its counter climbs past whatever the row still holds.

To be clear about what I ran: the server-side half is executed against Postgres, including a generation-1 save being dropped while the row holds 7. The computer restarting its counter I have read at those lines but not executed, since agent-computer/src/index.ts imports Playwright at module scope.

Reproducing it

Two cases against a real database, both failing on be18bc9, with the six existing cases in the file still passing. They drop into server/tests/computer-snapshot-store.integration.test.ts:

test("a save still in flight when the computer was wiped does not bring the old page back", async () => {
  const store = createSnapshotStore(database);
  await store.save("default", snapshot(7, [{ ref: "e9", role: "button", name: "Submit order" }], "https://old.example/order"));

  await store.clear("default");
  // The old session's last snapshot, arriving after the wipe.
  await store.save("default", snapshot(7, [{ ref: "e9", role: "button", name: "Submit order" }], "https://old.example/order"));

  expect(await createSnapshotStore(database).load("default")).toBeUndefined();
});

test("a wiped computer's fresh snapshots are not blocked by the generation the old one reached", async () => {
  const store = createSnapshotStore(database);
  await store.save("default", snapshot(7, [{ ref: "e9", role: "button", name: "Submit order" }], "https://old.example/order"));
  await store.clear("default");
  await store.save("default", snapshot(7, [{ ref: "e9", role: "button", name: "Submit order" }], "https://old.example/order"));

  await store.save("default", snapshot(1, [{ ref: "e1", role: "link", name: "Sign in" }], "https://fresh.example/start"));

  const loaded = await createSnapshotStore(database).load("default");
  expect(loaded?.url).toBe("https://fresh.example/start");
  expect(loaded?.snapshotId).toBe(1);
});

The file's existing "an older snapshot arriving late does not overwrite the newer one" covers the same guard through the conflict path, which is why the insert path read as covered.

Why there is no PR with this

I had a fix ready to write, an epoch owned by the server, bumped on reset and ordered against the generation. Pricing the plumbing is what turned up the restart case, and an epoch bumped on reset does not cover it. Whatever the ordering keys on has to change on any new computer session rather than only on a wipe, which points at the computer stamping a session identity at startup and the server ordering on (session, generation). That spans both services and is your call on where session identity should live, so I stopped rather than guess at it.

One smaller thing found alongside: createInMemorySnapshotStore (snapshot-store.ts:144) has no monotonic guard at all, so the two implementations disagree about the property #46 established.

@beardthelion

Hey @beardthelion,

Thanks for this one, and particularly for stopping where you did. The line references are exact all the way through, and spotting that createInMemorySnapshotStore has no guard at all is the kind of thing that stays invisible until a test quietly proves the wrong thing.

I took the in-memory half in #159. It now applies the same rule the table does, >= to match lt(stored, incoming) exactly, with a test for both the late-arrival and equal-generation cases.

On the insert path, your mechanism is right and I stopped short of the shape, because a premise underneath it does not hold. It is not your premise, to be fair, it is ours: snapshot-store.ts:61 and gateway.ts:576 both say a fresh computer counts generations from one again, and whether that happens depends on which provider is deployed.

agent-computer's /computers/reset handler calls profiles.reset(botId) at src/index.ts:684 and then releases control, and never touches sessions. session.snapshotId is written at :148, :259 and :709 only, none of them the reset path. So on the direct provider path the session outlives the wipe and the fresh browser's first snapshot is the next generation rather than 1. Through the supervisor it is different: supervisor/src/docker.ts:519 removes the container, so that one really does restart at 1.

And your forgetIdleSessions note turns out to matter more than either of us put it. Once the profile is gone the session is evictable at :137-144, so the direct path can restart at 1 too, depending on whether anything pushed the map past 32 in between. That is the strongest form of the argument: both 1 and the next generation are reachable on both paths, so the generation carries no information about which case the store is looking at.

Which means after wiping a row at generation 7, the store can be handed 8 because an old save was in flight, or 8 because the fresh page is genuinely there, and only the page contents tell them apart. Every generation-keyed guard on the insert path picks one to break:

>= blocks the resurrection and also blocks the fresh browser's every later snapshot on the direct path, forever. No row inserts again and refs stop resolving.
< only, which is what the two tests you supplied specify, is the same rule with the same effect on a real reset.
!= passes both tests and leaves the realistic case alone, which is just gaming the tests.

So the tests are not wrong about the bug, they are describing the case where the counter did restart. I left them out rather than bank something that has to be undone.

The one shape that fixes it inside server/ is a server-owned epoch per computer, bumped by clear in the same transaction as the delete, captured by gateway.snapshot before the /snapshot call and carried into save, with the write conditional in SQL on the captured epoch still being current. Postgres decides it in one statement, and it does not care what the generation does.

But it would be replaced by the thing you already described. Ordering on (session, generation) subsumes the insert race completely, so landing an epoch first means removing it later. And your instinct that it spans both services is right, with one thing worth adding: the ordinary reset path needs it too, not only restart, redeploy and eviction. The counter surviving a plain /computers/reset on the direct path is the same problem without the container going anywhere.

If you want to write it, it is yours, and I would rather have your version than an epoch. BotSession at agent-computer/src/index.ts:111-124 looks like the natural home for the identity, minted in sessionFor at :148, with the reset handler at :683 re-minting it, which it does not do for anything today. Worth holding both provider paths in mind, since only one of them recreates the container.

Either way the two comments are wrong and want fixing with whatever lands.

fix: give the in-memory snapshot store the guard the table has (#158) (…

#173 closed the read side, so this is what is left here.

The insert race is untouched by it. clear() still deletes the row, so a save still in flight when the reset lands finds nothing to conflict with and inserts unconditionally. Run against main at d767642 with a real database, the save arriving after clear() re-creates the wiped page's row, whether or not the provider reports a run.

Under a supervisor that is now mostly contained rather than harmful: resolve compares the stored row's run against the live one, so the resurrected row refuses instead of resolving, until a fresh snapshot supersedes it. Degraded rather than wrong, which is the important part.

Where the provider cannot report a run, which is the single shared computer with no supervisor, neither half of this issue is closed. Both executed against main:

the in-flight save re-creates the wiped page, and nothing later refuses it, because resolve skips the comparison when either side is null
a fresh computer's first snapshot is still refused while the row holds a higher generation, since lt(7, 1) is false and the session arm cannot fire with null on both sides, so the row keeps the dead page until the counter climbs back past it

Two smaller things in the same area, from reading main:

stored.session being null also skips the comparison, so a row written while the provider could not answer stays unprotected afterwards even under a supervisor
an action citing a ref that does not resolve is still forwarded with an empty element, and the shipped default (deny: [], allow: ["true"]) permits it, so an element-keyed deny rule does not match and the action goes through

I have the ordering half implemented: the computer stamps the run rather than the provider reporting it, so it covers both deployments, plus a token the reset mints that the save is conditional on, which closes the insert path directly rather than relying on the read side to catch it afterwards. Happy to open it, rework it onto the session column #173 added, or drop it if you would rather take this yourself.

Fixed, now on main (#173 for the session-ordering, #159 for the in-memory guard). A restarted computer carries a new session identity (persisted via migration 0014_snapshot_session.sql), and snapshot-store.ts save now orders across sessions (setWhere: or(lt(snapshotId), <session differs>)), so a fresh gen-1 snapshot from a restarted computer takes the update branch instead of being blocked by the stale higher generation. The in-memory store got the same session-aware rule (#159). The reset race is neutralised at resolve time: a revived stale row carries the old session, and resolve refuses any ref whose session differs from the current run. This is the (session, generation) design the issue asked for. Closing as fixed.

Both halves are closed for the supervisor and the sandbox. They are still open on the one shared
computer, and I have opened #265 for it.

(session, generation) is keyed on ComputerProvider.sessionOf, which is optional
(server/src/computer/provider.ts:94). The Docker supervisor implements it (supervisor.ts:165) and
the sandbox implements it (sandbox.ts:384). createSharedComputerProvider does not, so with no
COMPUTER_SUPERVISOR_URL the run is undefined on every save and every resolve, and both guards go
inert: setWhere reduces to lt on the generation once both sides are null
(snapshot-store.ts:135), and resolve short-circuits on session && stored.session
(gateway.ts:395). Nothing downstream catches the reset half either, since /computers/reset at
agent-computer/src/index.ts:707 releases control without touching session.snapshotId, so the
computer's own citation check at :278 still accepts the dead generation and forwards the action.

Run against main at e8aa344, real Postgres with 0014 applied:

reset, then the save that was in flight, then a click on the wiped page's ref: resolved as
{"role":"button","name":"Confirm transfer"}, no StaleSnapshotError
generation 7 stored, generation 1 saved, both runs null: the row keeps generation 7 and the old
page, in the table and in memory
control, the same sequence with run-1 then run-2: generation 1 lands
the same reset case with a provider that reports a run: refused, element: "not in the current snapshot"

The last two are the ones that matter. Same store, same gateway, changing only whether sessionOf
answers.

#265 takes the shape you described in this thread rather than reading the run off infrastructure:
the session mints one, the reset handler mints a new one, and the computer answers which run a Bot is
on, so it covers the plain reset on the direct path as well as restart, redeploy and eviction. One
thing I found reviewing my own diff and fixed there: the run has to be read before /snapshot, not
after, or a computer replaced mid-snapshot stamps its dead page with the run of the browser that
replaced it. The two comments you flagged are corrected.

Happy for this to stay closed and live on the PR if you would rather.

Navigation Menu

Repository navigation

A snapshot save in flight across a reset brings the wiped page back, and a restarted computer's snapshots stop landing #158

The larger half

Reproducing it

Why there is no PR with this

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

A snapshot save in flight across a reset brings the wiped page back, and a restarted computer's snapshots stop landing #158

Description

The larger half

Reproducing it

Why there is no PR with this

Activity

NathanTarbert commented on Aug 22, 2026

beardthelion commented on Aug 22, 2026

davidmckayv commented on Aug 26, 2026

beardthelion commented on Aug 26, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions