fix(tabs-sync): quality closes on continuity work + opencode terminal-signal research

[danshapiro]

Track 5 of the post-continuity quality wave: the missing independent review of e7a4add, the two known minors from that review loop, and the opencode terminal-signal research deliverable.

Task A — Independent review of e7a4add (verdict: clean beyond the known minors)

Adversarial review of e7a4addb ("fix(tabs-sync): custom CLI modes, cross-client ordering, and bounded restore marker"), the final fix round that never got re-reviewed. Reviewed as a senior engineer against the three risk areas named in the task:

Cross-client capturedAt ordering — no findings.

• cross_client_capture_order is a total order over queue heads (capturedAt → revision → clientInstanceId → path); paths are unique, so max_by/min_by selection is fully deterministic despite HashMap queue collection order (tabs_persist.rs:152-175).
• The k-way merge (order_generations_newest_first) compares only per-client queue HEADS, preserving within-client revision monotonicity under clock rollback — the non-transitivity trap of mixing both comparison rules in one sort is correctly avoided, and the doc comment says exactly why.
• Retention eviction (enforce_device_file_cap) mirrors the merge with oldest-first per-client queues; file_count bookkeeping guarantees min_by always has a nonempty head; a client's higher-revision-but-older-clock file is correctly protected behind its queue head.
• union_of_newest_per_client label tie-break is deterministic (clientInstanceId is unique per candidate by construction).

Restore-marker caps — no findings.

• Boundary semantics are consistently inclusive-at-cap across all four caps (> MAX rejects); boundary tests cover at/over for bytes, pane count, and all three identifiers.
• read_marker_doc TOCTOU between metadata() and read_to_string is re-checked post-read.
• Error-before-side-effects verified: validate_restore_projection preflight runs before the pane loop, and restore_rejects_over_limit_marker_plan_before_side_effects proves no marker file and no pane on rejection.
• Write-ahead ordering preserved: per-pane persist_marker still precedes the create/deliver (tabs_snapshots.rs:799), and write_ahead_marker_is_durable_before_tab_create_is_sent fences it.

Any-nonempty-mode acceptance — no regression.

• Persistence-time validation now only requires a nonempty mode; actual spawning remains guarded by mode_is_known (terminal_tabs.rs:588, 400 for unregistered modes) and cli_spec_known (terminal.rs:762). pane_to_create_body passes mode through with no seven-mode enum assumption; restore of an unregistered mode fails loudly as a failed entry, never a silent shell fallback.

Non-blocking notes (recorded, deliberately not restyled):

• MarkerSizeWriter's over-cap message reads "serialized byte limit is X (more than X)" — repeats the cap instead of the actual size. Cosmetic; the fail-loud path still identifies the file and cause.
• validate_restore_projection sizes projected terminal ids with a 36-byte UUID placeholder; a hypothetical future id format >36 bytes would pass preflight and fail loudly post-side-effect instead. Exact today, safe-by-fail-loud tomorrow.
• The commit found (and this review confirms) one REAL defect adjacent to its diff — confirm_delivery — fixed here as Task B1.

Task B — Known minors

1. confirm_delivery dropped-sender bug (fixed, TDD): the ack wait matched Ok(_) on the timed-out oneshot receiver, so Ok(Err(RecvError)) — a dropped sender, e.g. the pending entry cancelled/purged before the client answered — read as delivery confirmation and marked the pane restored. Now only Ok(Ok(_)) confirms. Regression test (red first: it reproduced deliveryConfirmed:true + restored) drives the restore endpoint with a client whose ack entry is purged instead of answered and asserts the delivery-unconfirmed failure path plus the retained in-progress write-ahead marker.
2. Retention wording (docs only): plan doc and main.rs comment claimed "last 5 generations per device"; actual (correct, tested) behavior is MAX_SNAPSHOT_GENERATIONS = 5 per (device, client), MAX_SNAPSHOT_FILES_PER_DEVICE = 40 per device. Wording fixed in both places (and the doc's embedded copy of the comment).

Task C — OpenCode terminal-signal research

docs/plans/2026-07-23-opencode-terminal-signals.md — evidence-backed findings from running the real opencode 1.18.3 CLI in a throwaway HOME (run + TUI under script), schema/row inspection, socket/lock inspection, and bounded read-only sampling of the real 4.4 GB DB. Headlines: the standalone TUI has no socket, no lock/heartbeat, and emits zero bare BELs (the claude/codex turn-complete pipeline cannot be extended to it); the durable signal is the SQLite message.data.time.completed null→set edge (with error absent for positive completion); subagent activity is separable via session.parent_id; session_input is structurally a prompt queue but its on-disk visibility while queued is honestly UNVERIFIED. Recommendation: full truly-idle support via bounded opencode.db polling keyed off the existing OpencodeLocator binding; unbound panes stay status-inert; no grace-window heuristics; effort 2-4 days.

Verification

• cargo fmt --check: only the pre-existing directory_index.rs drift (outside this diff, already noted as pre-existing in e7a4add's own message)
• cargo clippy --workspace --all-targets: zero warnings in files touched by this PR (background warnings pre-existing)
• cargo test --workspace: 1524 passed, 0 failed
• Coordinated FRESHELL_TEST_SUMMARY="track5 quality closes pre-PR" npm test: full-suite success at 51f5719 (coordinator-recorded)

🤖 Generated with Amplifier