Workspaces 5/10: hold derived events in the transfer gap; drift cleanup - #618
Workspaces 5/10: hold derived events in the transfer gap; drift cleanup#618nedtwigg wants to merge 12 commits into
Conversation
pty:data in the gap is dropped because the replay carries the bytes, but terminal:semanticEvents and terminal:protocolEvents are derived once at the sidecar's parse site and ride no replay, so a prompt mark, cwd change, or notification landing mid-transfer never reached the target. Rust now holds them per id, bounded, and emits them to the new owner right behind the replay that lifts the suppression; a hand-back or exit drops them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u
- The three updater comments still described the reverted every-window grant; they now match capabilities/main-only.json, and the specs name the capability as written, updater:default. - layout.md claimed every Workspace verb has a dor counterpart; reorder, transfer, and tear-out do not yet. - A source test pins that main.js hands pty-core the shared sliceSince: without it recovery capture reads an empty buffer with no error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u
Deploying mouseterm with
|
| Latest commit: |
e0f73c2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://85030685.mouseterm.pages.dev |
| Branch Preview URL: | https://workspaces-harden.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.
The routing change rests on the premise that these events are in no replay. For terminal:semanticEvents in the standalone host that isn't true: the replay carries the raw OSC bytes and tauri-adapter.ts re-derives the semantic events from them itself, so the flushed queue applies them a second time. Mechanism and the narrower fix are in the thread on routing.rs. The other threads are the held queue's lifecycle and two spec claims.
…replay The hold queued both derived streams on the premise that neither is in any replay. Semantic events are: the replay carries the raw bytes, OSCs included, and the target's `pty:replay` listener re-parses them. The flushed queue then re-applied `commandStart` on top of state the replay had just rebuilt, and `commandStart` is not idempotent, so a transfer that split a `commandLine` from its `commandStart` left the arriving window with a derived title for a command whose real line the replay had recovered. Semantic events now route to `Drop` while suppressed; only `terminal:protocolEvents`, which no replay path rebuilds, are held. What the replay path genuinely did not rebuild was the AlertManager's half, so both adapters' replay listeners now feed it too, and the tests pin a watched command coming back from a replay alone. `clear_suppression` and `mint` cleared `awaiting_replay` and left `held` behind, so a queue could survive until the shell exited and be flushed ahead of the next transfer's own gap. One helper, `lift_suppression`, now takes both halves together, and every site goes through it. The hand-back comment claimed the source saw the gap's bytes live; it did not, since suppression is by id and ownership had already moved. The comment and the Arrival queue spec now say the gap is lost on a hand-back, the one path nothing recovers on this branch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RChsJ5rMUMyfu22UZDfUus
Carries the review fixes from the earlier stages up the stack. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RChsJ5rMUMyfu22UZDfUus
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.
The semantic-events half of the fix doesn't survive the arrival path it was written for: planArrival calls alertSeed for every transferred pane right after collectLivePtys resolves, and AlertManager.seed nulls commandExitWatch, so the WATCHING state the replay just rebuilt is gone a turn later. Mechanism, and why this is a regression against the previous Hold, are in the thread on standalone/src/tauri-adapter.ts. The other thread is the hand-back comment against the spec line it cites.
Everything else from the last round reads as addressed — the routing split, the lift_suppression consolidation, the HELD_EVENTS_MAX bound in the table, and the layout.md reorder line.
| // them are gone from its pane. A later stage recovers the gap | ||
| // (docs/specs/standalone.md -> "Arrival queue"). |
There was a problem hiding this comment.
This points at a spec line that says the opposite. docs/specs/standalone.md → "Arrival queue" now reads "The gap is lost on a hand-back: suppressed from the invoke with no replay to follow, it is the one path nothing recovers", and no spec carries a ## Future item for recovering it — standalone.md has no ## Future section at all, and there is no **Scope: workspaces** anywhere under docs/specs/. So a reader who follows the pointer to check what the later stage is finds the claim withdrawn, and nothing will bring it back when a stage does land.
Either drop the promise here, or record it as a ## Future item with a Reserved: line in the body per AGENTS.md → "Spec lifecycle". The suggestion is the former, matching the spec's own wording.
| // them are gone from its pane. A later stage recovers the gap | |
| // (docs/specs/standalone.md -> "Arrival queue"). | |
| // them are gone from its pane, the one path nothing recovers. |
# Conflicts: # scripts/spec-word-budgets.json
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark it ready when you want the full review.
The reorder does close last round's thread: alertSeed now runs before adopt_ready, so the replay listener's applyTerminalSemanticEvents is the last writer of alert state on the arrival path, and alert.md -> "Public State" already guarantees the pre-registration seed survives (Must retain host Activity before xterm initialization). One decision the reorder raises, plus the thread on workspace-move.ts.
The seed now runs before the point where planArrival can still throw. On the live.timedOut path the Workspace is handed back and handleArrivalFailed only clears the source's transferring mark, so this window keeps an AlertManager entry — and, through onStateChange -> setTerminalActivity, an activity-store entry — for panes that stay with the source and never mount here. Nothing renders them that I can find: getLivePersistedAlertState returns null for an unregistered id so they are not persisted, and a second arrival of the same ids seeds over them. So it is retained state rather than a wrong display, but it is new with this commit — previously the throw came first. Either clear it on the throw, or say in standalone.md -> "Arrival queue" that a refused arrival leaves its seeded alerts behind, so the next reader does not have to re-derive that it is harmless.
| platform: PlatformAdapter, | ||
| payload: MovePayload, | ||
| ): Promise<WallBootPlans[string]> { | ||
| // Seed the older persisted state before replay re-derives a running watch. |
There was a problem hiding this comment.
The clause this replaces was the only statement of why the seed exists: the AlertManager is per webview, so the source's went with its window. The spec sentence rewritten in the same commit dropped "seeds each persisted TODO into its own AlertManager" as well, and the nearest sibling seed says the opposite of what the arrival path needs — restoreTerminal's in lib/src/lib/session-restore.ts is commented "Restore-only: a live resume still has the manager's own state". That holds where the manager lives in the extension host; it is false for standalone, where the manager is the arriving webview's. Keeping only the ordering rule leaves an editor free to read this loop as redundant on a live resume and delete it. Keep both halves:
| // Seed the older persisted state before replay re-derives a running watch. | |
| // The AlertManager is per webview, so a persisted TODO has to be seeded into | |
| // this one — the source's went with its window. Before the replay, which | |
| // re-derives the running command's watch over it. |
2 participants
terminal:semanticEvents/terminal:protocolEventslanding while an id is suppressed are held in Rust and delivered behind the replay, instead of dropped. Stale updater comments and theupdater:*vsupdater:defaultspec mismatch corrected; a source test pins thatmain.jshands pty-core the sharedsliceSince.Based on
workspaces-dor.🤖 Generated with Claude Code
https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u