Workspaces 8/10: carry the whole transcript and pins across a move

[nedtwigg]

The transfer splits each PTY at a pty:marked line the sidecar stamps in the stream: the source serializes its buffer there (@xterm/addon-serialize) and hands it over as the arrival's content, the target writes it ahead of a since-mark replay, and notepad pins are re-registered at their lines. Tear-out windows are built once the content lands.

Based on workspaces-durability.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u

[cloudflare-workers-and-pages]

Deploying mouseterm with    Cloudflare Pages

Latest commit:	363ea2a
Status:	✅  Deploy successful!
Preview URL:	https://0981ef1b.mouseterm.pages.dev
Branch Preview URL:	https://workspaces-fidelity.mouseterm.pages.dev

View logs

[dormouse-bot]

Feedback on work in progress — not a merge verdict. The two-phase split reads well and the mark ordering argument holds (one sidecar writer, one Rust reader, one webview queue), but three things in the new failure/timing paths look wrong.

The tear-out's build_window failure now strands the source. open_workspace_window returns Ok before the window exists, so handOff has already run markWorkspaceTransferring by the time transfer_workspace_content tries to build it. The silent hand-back inside that failure branch is carried over from when the failure arrived on the first invoke, where the mark had not been set yet — its comment's premise ("a transferring mark the source still holds and expects") is now inverted. If build_window fails, the source's catch only logs, no workspace-arrival-failed is emitted, and the Workspace stays transferring forever: still mounted, but omitted from every snapshot this window writes, so it is gone on the next restart along with its shells. Inline suggestion routes it through hand_back_arrival, which is the same unstage + reassign plus the emit.

marksFor subscribes after the invoke resolves, but the mark request goes out before it returns. begin_arrival sends pty:mark to the sidecar and then does two session-file writes before the command replies, so the sidecar's marked lines have a real head start on the invoke's own reply. The adapter's listener is registered at init, but markedHandlers is empty until handOff resumes — a marked event that lands first is dropped, and then every transfer eats the full MARK_TIMEOUT_MS and ships an unmarked id, whose serialized buffer and whole-buffer replay paint the tail twice. That is the documented worst case, but on the ordinary path rather than an exceptional one. The suite can't see it: the fake stamps marks from setTimeout(…, 0) inside the invoke mock, so they are always strictly after the resolve.

The terminal:semanticEvents routing row no longer matches route. The table still says the events are held for the whole of mid-transfer, while the new code emits them to the source until the mark passes (routing.rs, and the PR's own Route::EmitTo("main") assertion for marking_view). Which one is wrong is a design call: payload.workspace.session was snapshotted back at prepareWorkspaceTransfer, so a CWD change, prompt end, or title candidate that fires in the marking window reaches neither the source's payload nor the target — if that is acceptable, the suggestion fixes the row; if not, the fix is Route::Hold during marking.

[dormouse-bot]

Feedback on work in progress — not a merge verdict. The three findings from the last pass are closed, and the semantic/protocol split is the better of the two answers that were open: TauriAdapter's pty:replay listener re-derives semantic events from the replay, so dropping them with their chunk avoids applying a non-idempotent commandStart on top of what the replay rebuilt. One thing in the new hand-back replay looks short by one round trip.

The hand-back decides what to replay from arrival.content, but the source falls silent one round trip earlier. route drops pty:data for an id the moment the pty:marked arm inserts it into awaiting_replay — that is where the source's xterm stops. The content carrying the marks only reaches Rust at the source's next invoke, transfer_workspace_content, after marksFor resolves and captureTransferContent has serialized every pane. A hand-back inside that window sees arrival.content == None, so arrival_marks is empty, hand_back_ids puts every id in unmarked, and reassign(&unmarked, from, false) hands them back unsuppressed with no replay — leaving exactly the gap this commit set out to close, and silently: the pane resumes live output with a hole in its scrollback where the transfer was. WindowEvent::Destroyed routes a target closed mid-serialize straight into hand_back_arrival, so this is the ordinary "the target closed before it took it" path, not an exotic one. The same branch swallows a second case: an id whose marked line lands after MARK_TIMEOUT_MS is serialized without a mark, yet Rust suppressed it when that line did arrive. Both make the new spec line — "An id without a mark — no content yet, or one the sidecar never stamped — missed nothing its source does not hold" — untrue for the first case it names.

The state that answers the question is Rust's own: an id in awaiting_replay is, by construction, one the source stopped receiving. Recording each mark in the pty:marked arm (the value is already in data) and partitioning on the suppression map instead of on arrival.content covers both windows under one rule, and leaves list()'s whole-buffer fallback for an id whose mark was never recorded — repainting a tail rather than losing it, which is the trade the rest of this path already makes. The source half needs the matching move: handedMarks is recorded after captureTransferContent returns, so a handleArrivalFailed inside that same window finds nothing and never arms acceptHandBackReplay.

[dormouse-bot]

Feedback on work in progress — not a merge verdict. Moving the marks into Rust's own state is the right answer to the last pass's finding: transfer_marks is recorded in the pty:marked arm, survives the target's replay, and begin_marking clears it at the head of every transfer, so both windows the hand-back used to fall through are closed. One thing the same commit opened on the source side.

inFlight is now written before the invoke, but every failure path still reads it by presence rather than by identity. A second drop of the same tab is reachable for the whole in-flight window: markWorkspaceTransferring only omits the Workspace from snapshots (transferring.has(workspace.id) in lib/src/lib/window-session-aggregator.ts), nothing hides the tab from the strip until commit, and onDropOnOtherWindow in standalone/src/workspace-drag.ts has no re-entrancy guard. So the second handOff overwrites inFlight, Rust refuses it in begin_arrival ("Workspace '{}' is already in flight"), and the new unconditional inFlight.delete(workspaceId) in the catch evicts the first transfer's record. That transfer then reads !inFlight.has(workspaceId) as "handed back before invoke replied", returns without markWorkspaceTransferring, and never sends transfer_workspace_content — so the arrival sits in Rust until spawn_arrival_watchdog expires it at ARRIVAL_MAX, its panes silent from their marked lines, and the watchdog's hand_back_arrival then reaches a handleArrivalFailed that bails at if (!inFlight.delete(workspaceId)) return before acceptHandBackReplay can arm. The recovery this commit adds is precisely what gets skipped, and the scrollback hole stays. Keying all four checks on inFlight.get(workspaceId) === prepared covers it; suggestions inline.

[dormouse-bot]

The new method landed above the doc comment, so lift_suppression's contract — including the WindowState::suppressed republish the caller owes — now reads as documentation of mark_transfer.

Suggested change

	/// `routing::lift_suppression` over this state's two halves. The caller
	/// republishes `WindowState::suppressed` after it, still under the lock.
	fn mark_transfer(&mut self, id: &str, mark: u64) {
	if self.marking.remove(id).is_some() {
	self.transfer_marks.insert(id.to_string(), mark);
	self.awaiting_replay.insert(id.to_string(), Instant::now());
	}
	}
	fn lift_suppression(&mut self, id: &str) -> Vec<routing::HeldEvent> {
	/// The sidecar's cut for an id still marking: suppression begins here, and
	/// the mark outlives it so a hand-back can replay since it.
	fn mark_transfer(&mut self, id: &str, mark: u64) {
	if self.marking.remove(id).is_some() {
	self.transfer_marks.insert(id.to_string(), mark);
	self.awaiting_replay.insert(id.to_string(), Instant::now());
	}
	}
	/// `routing::lift_suppression` over this state's two halves. The caller
	/// republishes `WindowState::suppressed` after it, still under the lock.
	fn lift_suppression(&mut self, id: &str) -> Vec<routing::HeldEvent> {

[dormouse-bot]

Feedback on work in progress — not a merge verdict. The identity-keyed inFlight is the right shape for the duplicate-drop interleaving, and the new test pins the part that mattered — the first attempt's hand-back replay still lands after the second drop is refused. Two things in the other half.

Retaining the cut past forget_pty does not produce a replay: the sidecar will not answer for an id that has exited. list() in standalone/sidecar/pty-core.js opens with const targets = Array.isArray(ids) ? ids.filter((id) => ptys.has(id)) : [...ptys.keys()], and the PTY's onExit runs ptys.delete(id) — so a pty:requestInit naming an exited id gets neither a list entry nor a replay line. The bytes really are still there, which is the commit's premise: sessions is deleted only by kill/forget, never by onExit. The ptys.has(id) filter is what stands between them and the source. So on a hand-back the retained mark now sorts the id into marked, reassign(&marked, from, true) re-suppresses it and re-inserts an owner for a dead PTY, pty:requestInit goes out and is answered by nothing, and the suppression is cleared only when AWAITING_REPLAY_MAX fails open (the owner entry is never cleared at all) — while the bytes between the mark and the exit stay lost, which is the hole the commit set out to close. forget_pty also runs for pty_kill, and there the gap cannot be closed from Rust at all: the sidecar's kill runs sessions.delete(id), so no buffer survives to replay. Either list has to reach an exited id's sessions entry — reporting it alive: false, which then needs checking against collectLivePtys on the adoption path, since that treats the list as the live set — or forget_pty keeps dropping the mark and the spec's new "retaining them through target replay and PTY exit until settlement" narrows back to the replay half.

begin_arrival hands ownership to the target one lock acquisition before it opens the marking phase. windows.reassign(&arrival.terminal_ids, &arrival.to, false) writes owners[id] = to and releases the routing lock; windows.begin_marking(...) then takes it again. A pty:data chunk dispatched in that gap finds no marking entry and an owner that is the target, so route returns EmitTo(to) — and the target has no xterm for the id yet, so those bytes are painted at neither end. dispatch_sidecar_event cannot be excluded from the gap: it takes the arrivals lock only when suppressed > 0, and nothing is suppressed at this point. Reversing the two calls does not fix it — reassign's suppress == false branch runs routing.marking.remove(id), so it would wipe the entries begin_marking had just written. What closes it is doing both under one acquisition: a WindowState method that, per id, inserts the target owner, lifts suppression, drops any stale transfer_marks, and inserts the source into marking before the guard drops.

[dormouse-bot]

Feedback on work in progress — not a merge verdict. begin_transfer closes the ownership/marking gap the last pass named, and explicit_kill_discards_a_cut_with_its_buffer plus forget_pty clearing transfer_marks close the orphaned-arrival leak that rode with it. The sidecar half of the exited-buffer replay is right too — list now reaches sessions for a marked id and pty-core.test.js pins discovery staying live-only and kill discarding the buffer. What it produces still cannot be delivered.

The retained cut earns a replay the host then drops. route's "pty:exit" | "pty:replay" arm resolves by ownership alone — owner(view.owners, id), which returns Route::Drop for an id no window owns — and forWindow is read only for pty:list. An exited id has no owner: exited_pty runs owners.remove(id) at pty:exit, and hand_back's if let Some(owner) = routing.owners.get_mut(id) deliberately does not put one back, which a_pty_exit_keeps_its_cut_until_the_arrival_settles now asserts (assert!(!routing.owners.contains_key("t1"))). So the hand-back's pty:requestInit goes out with the id and its mark, the sidecar answers with the alive: false list entry and the since-mark replay line this commit added, Rust drops the replay, and acceptHandBackReplay — which subscribes to onPtyReplay and nothing else — writes nothing before its 3 s timer fires. The pane resumes with the same hole between its mark and the exit that the commit set out to close, and transport.md's new "replaying their since-mark tail" is true of the sidecar but not of what reaches an xterm.

The arrival path pays the same routing rule differently: adopt_ready sends arrival_marks, so an id that exited mid-transfer is now listed to the target, its replay is dropped for the same reason, and collectLivePtysOnce finishes only on replay.size >= ptyList.length — so every arrival carrying an exited PTY stalls the whole mount for ARRIVAL_TIMEOUT_MS before mounting with the post-mark tail missing. The serialized content still lands, so that half is a delay and a truncated tail rather than a hole.

Two shapes fit. Routing pty:replay by forWindow where the sidecar echoed one, falling back to owner, matches every existing case — boot, arrival and hand-back all ask as the window that owns the ids — and is the only one that reaches an id with no owner at all. Re-minting the source as owner in hand_back (and the target in begin_transfer) instead keeps the routing table alone as the authority, at the cost of the phantom owner the doc comment currently rules out; route's own contract, "an unowned id is one whose window went away", is the thing being traded either way, so it reads as a design call rather than a mechanical fix.

Smaller, from the same commit: WindowState::reassign now has no production caller — begin_arrival and hand_back_arrival were its only two — and its doc comment still describes the hand-back split it used to serve — "a marked one is re-suppressed by hand_back_arrival" — which hand_back has replaced. cargo test keeps it compiling, so nothing flags it; it is the three test call sites holding it alive.

[dormouse-bot]

Feedback on work in progress — not a merge verdict. The forWindow-first arm is the fix the last pass asked for, and it lands on both halves: hand_back_arrival names arrival.from, list() echoes that address onto every replay line, and the new arm resolves it ahead of owner, so an exited id — which remove_pty deliberately leaves ownerless — reaches the source instead of becoming Route::Drop. adopt_ready names the target the same way, so the arrival path stops sitting out ARRIVAL_TIMEOUT_MS on an id that exited mid-transfer, and resumeTerminal's own dead branch already writes its banner there. Every in-tree asker names the window the owner lookup would have picked, so nothing else changes route, and ("pty:replay", json!({"id":"exited"}), Route::Drop) pins the unaddressed fallback. Moving the watch settlement to the adapter seam in 363ea2a5 is the better home for it than the mount paths — it covers adoption and hand-back from one place, and replayExits can only be reached by a marked request, so an ordinary boot list never enters it.

That leaves the previous commit's copy of the same job in acceptHandBackReplay. TauriAdapter's pty:replay listener now applies commandFinish for a listed-dead id — same pty:list payload, same exitCode ?? -1 fallback — and does it before for (const handler of this.replayHandlers), which is what platform.onPtyReplay(onReplay) registered into. So by the time onReplay runs, the event has already been applied, and its own applyTerminalSemanticEvents line re-applies it. It is not wrong today — the reducer's commandFinish case returns state unchanged once currentCommand is null — but it is a second, weaker implementation of a rule the adapter now owns: the adapter's version also settles the alert watch and the parked dor await waiters through alertManager.onExit, which this one never did, so the two only diverge from here. The exited map still earns its place for the xterm half (REPLAY_MODE_RESET, the banner, entry.exited), which nothing else does. initWorkspaceMoves is armed only on the !BROWSER_DEV_HOST branch of main.tsx, so movePlatform is always the adapter that now carries this — there is no second host still relying on the local copy. Suggestions inline; noUnusedLocals is on in standalone/tsconfig.json, so the import goes with the call.

[dormouse-bot]

TauriAdapter applies this for the same id and requestId before it reaches replayHandlers, so this is a second application of an event the adapter already delivered — and the weaker one, since the adapter's also runs alertManager.onExit, settling the watch and the parked waiters. Harmless today only because the reducer no-ops once currentCommand is null.

Suggested change

	entry.exited = true;
	applyTerminalSemanticEvents(detail.id, [{ type: 'commandFinish', exitCode }]);
	entry.exited = true;

[dormouse-bot]

Unused once the commandFinish call below goes; standalone/tsconfig.json sets noUnusedLocals, so it would fail the build rather than linger.

Suggested change

import { applyTerminalSemanticEvents } from "dormouse-lib/lib/terminal-state-store";