Workspaces 2/10: standalone persistence and agent recovery

[nedtwigg]

Per-Workspace persistence through the Window aggregator, restore per Workspace off one PTY list, agent recovery lifted into lib/src/host and shared with VS Code, the dev state root, the orphan temp sweep.

Based on workspaces-window.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u

[dormouse-bot]

Feedback on work in progress — not a merge verdict. Mark the PR ready when you want the full review.

The quit budget is now derived arithmetic spread across three files, and nothing pins it. QUIT_TEARDOWN_CEILING_MS works out to 12 300 ms ((1300+1500) + 1500 + (2000+1500) + 1500 + 2000, +1000 slack), and its own doc comment says it is "exported for the test that pins it above the sum" — but no test imports it, so neither that relationship nor "stays under Rust's QUIT_PHASE_TIMEOUT_MS" is checked anywhere. SIDECAR_ROUND_TRIP_MARGIN_MS = 1500 is likewise a hand-copy of the timeout + 1500 in capture_agent_recovery and pty_graceful_kill_all.

The drift this guards against already happened inside the diff: Rust's comment on QUIT_PHASE_TIMEOUT_MS still claims a 10 s webview ceiling, and quit.test.ts says the same. The real margin is 1 700 ms, not 4 000 — and that comment is exactly what a later editor would read when deciding whether 14 000 is still enough. lib/src/lib/mirrored-constants.test.ts is the existing home for this shape ("a 'must stay in sync' claim names the test that pins it"), and it already pins OPEN_PORT_TIMEOUT_MS across the same TS↔Rust boundary. Two source-reading assertions there — QUIT_TEARDOWN_CEILING_MS < QUIT_PHASE_TIMEOUT_MS, and SIDECAR_ROUND_TRIP_MARGIN_MS against the two timeout + N call sites — would make the whole derivation self-checking, and let both comments drop their literals for good. Inline suggestions below just remove the two stale numbers.

flushWindowSession() is a teardown step with no bound of its own and no term in STEP_BUDGET_TOTAL_MS. The comment above the constants says the ceiling is derived from the sum of the step bounds precisely so it cannot abort the final save, and docs/specs/standalone.md states it as a rule ("derived from the sum of those bounds, never a literal"). Today the step is free — both saveWindowState implementations return void, so await inFlight resolves on a null — but that is a property of the current adapters, not of the step. A writer that ever returns a slow promise eats the 1 000 ms slack and then drainSessionSaves, which is the failure the derivation exists to prevent. Either bound it like its neighbours and add its term, or say in the comment that it is deliberately unbounded because the writer is synchronous.

[cloudflare-workers-and-pages]

Deploying mouseterm with    Cloudflare Pages

Latest commit:	e1a6f2a
Status:	✅  Deploy successful!
Preview URL:	https://79d6e422.mouseterm.pages.dev
Branch Preview URL:	https://workspaces-persist.mouseterm.pages.dev

View logs

[dormouse-bot]

Feedback on work in progress — not a merge verdict. Mark the PR ready when you want the full review.

The derivation is pinned and the Window write is bounded, so both of last round's points are closed. Two things about what the new pin actually holds.

The margin to Rust's watchdog is down to 700 ms, and the pin asserts only a sign. Adding WINDOW_WRITE_MS moves QUIT_TEARDOWN_CEILING_MS to 13 300 ms against Rust's 14 000, so the gap is 700 ms where it was 1 700 — and keeps the webview ceiling under the Rust per-phase watchdog passes for any positive gap. One more 500 ms step term would leave 200 ms, inside Rust's own QUIT_POLL_STEP_MS, so the watchdog can force the exit before the webview's own guard has reached quit_proceed — the aborted final save the whole derivation exists to prevent, arrived at through a green test. The inline suggestion pins the gap above the poll step instead; it holds today at 700 > 500. The comment this commit rewrote in lib.rs still says the watchdog Comfortably exceeds the ceiling, and at 700 ms that is the sentence a later editor reads when deciding whether 14 000 is still enough.

The pin reads the sum's value, not its completeness. expect(ceiling).toBeGreaterThan(valueOf('STEP_BUDGET_TOTAL_MS')) compares the sum against the same sum plus the slack term, so it can only fail if that slack goes to zero — nothing checks that every bound runQuitTeardown actually passes is a term in STEP_BUDGET_TOTAL_MS, which is the defect the last round found by hand. The same source-reading style reaches it: each step's bound is a literal *_MS identifier in the call (adapter.requestSessionFlush(POST_KILL_FLUSH_MS), withTimeout(..., WINDOW_WRITE_MS, ...)), so pulling those out of runQuitTeardown's body and asserting each is a term would make the next unbudgeted step fail the test rather than wait for a reviewer.

[dormouse-bot]

A bare toBeLessThan still passes at a 200 ms gap, which is inside Rust's own QUIT_POLL_STEP_MS — the watchdog could then fire before the webview's guard has reached quit_proceed. Pinning the gap above the poll step keeps the claim the Rust comment makes, and holds today (700 > 500).

Suggested change

	expect(ceiling).toBeLessThan(rustMs('QUIT_PHASE_TIMEOUT_MS'));
	// The webview still has to reach `quit_proceed` after its own guard fires,
	// so the gap must clear Rust's poll granularity, not merely be positive.
	expect(rustMs('QUIT_PHASE_TIMEOUT_MS') - ceiling).toBeGreaterThan(rustMs('QUIT_POLL_STEP_MS'));

[dormouse-bot]

"Comfortably" described a 4 000 ms gap, then a 1 700 ms one, and now a 700 ms one — it is the one part of this comment nothing checks. Dropping it leaves the pin as the claim.

Suggested change

	// sum of all teardown work. Comfortably exceeds the webview's own teardown
	// sum of all teardown work. Sits above the webview's own teardown

[dormouse-bot]

At exactly WINDOW_WRITE_MS the work and withTimeout's guard land on the same tick, and the work only wins because its timer was registered first (the argument is evaluated before withTimeout is entered). The other steps here don't race — the fake adapter doesn't enforce the budget it's passed — so this is the one line where a future reordering flips the assertion. One tick inside the bound is unambiguous and still the worst case the webview can observe with the work winning.

Suggested change

	mocks.flushWindowSession.mockImplementation(slow("flushWindow", 1000));
	mocks.flushWindowSession.mockImplementation(slow("flushWindow", 999));