fix: close the last three restart-wall pins — the wall is green with zero pins - #577
Merged
Conversation
…n findings Validation round (ledger: .the-usual-logs/wall-pins-closure/load-bearing-ledger.md): 12 assumptions — 8 verified, 4 falsified. Plan changes: - Task 2: correct real-claude semantics (verified v2.1.220: --resume of a not-found id errors+exits — never recreates; --session-id rejected as silent-fresh-hazardous for deleted ids); add Step 5b updating reconcile-client-adoption spec (pins the inverted claude verdict); note the ruler baseline aborts at :1753 so later sub-assertions are unmeasured under composition. - Task 3: replace one-shot marker consumption with idempotent read-only design (delete-at-derive loses the breadcrumb to reconnect churn — client re-sends reconcile on every ready because responses can drop); drops the terminal.rs consumption block and helper; keeps pane-ledger-restart-rust.spec.ts green. - Task 4: cite verified created-vs-attach routing; record accepted residual. - Task 5: fix wrong comparator (freshopencode uses the sidecar fake, not the row gate); cite verified restartAbrupt env continuity. - Task 6: add Step 4b — delete the pre-spawn binding row when a FRESH claude create's spawn fails (ghost ledgerOnly offer otherwise); record verified lock-safety and resume-rewrite benignity. - Task 7: never assert ledgerOnly vs device location (racy snapshot push). - Task 8: add pane-ledger-restart-rust.spec.ts to sibling gates. - Self-Review: add section 4 documenting the validation round.
…-only, ledger excluded)
…ad_session (PIN 1)
…le-down (PIN 1 carve-out derives Respawn there)
The batched-adjudication scenario's dead-session fixtures switch from
claude to codex panes (opened from seeded sidebar history, so the resume
creates record durable pane-ledger bindings). Under the PIN 1 carve-out a
ledger-bound claude id never seen on disk in the new epoch derives
Respawn, so the deleted-while-down claude shape no longer produces the
Dead sessions dialog. Codex has no Absent-arm carve-out: ledger-bound +
Absent stays dead_session{session_not_on_disk}. Batching assertions are
otherwise equivalent (ONE dialog, 2 listitems, per-row adjudication,
counts preserved); the seed writes fixtures only when ~/.codex/sessions
is first created so a restart cannot resurrect the deleted files.
Red before this update (carve-out binary, old spec):
npx playwright test --config test/e2e-browser/playwright.config.ts \
--project=rust-chromium reconcile-client-adoption-rust.spec.ts
1 failed
[rust-chromium] ... dead sessions surface as ONE batched adjudication panel
Error: Timeout 90000ms exceeded while waiting on the predicate
477 | await expect(dialog).toHaveCount(1)
478 | await expect(dialog.getByRole('listitem')).toHaveCount(2)
2 passed
Green after this update (same command): 3 passed (27.0s)
…reen, newly-surfaced red at quiet-client alert assertion (PIN 1) Step 6 contingency (task-2 brief): the ruler still fails expectedly, but NOT on the pinned leg. The claude terminal \$2.2 --resume argv poll at restore-contract-wall-rust.spec.ts:1753 (the pin's only recorded red at baseline c1c6746) now passes under composition with the PIN 1 carve-out. The run proceeds past :1762 into sub-assertions that had never executed under composition and hits a newly-surfaced red; per the brief this takes the contingency path (re-diagnosis in Task 8 Step 2, never re-pin), so the test.fail pin at :1500-1503 stays in place for now. Failing leg error, verbatim (JSON reporter, expectedStatus=failed, status=expected, duration 37552ms): Error: expect(locator).toHaveCount(expected) failed Locator: getByRole('alert') Expected: 0 Received: 2 Timeout: 10000ms Call log: - Expect "toHaveCount" with timeout 10000ms - waiting for getByRole('alert') 14 x locator resolved to 2 elements - unexpected value "2" 1803 | // from the snapshot fetch racing pane creation -- see 1804 | // createFreshclaudePane's note above. > 1805 | await expect(page.getByRole('alert')).toHaveCount(0) | ^ 1806 | } finally { 1807 | await server.stop() 1808 | await fs.rm(sharedRoot, { recursive: true, force: true }) at test/e2e-browser/specs/restore-contract-wall-rust.spec.ts:1805:45 Page snapshot (error-context.md) shows two anonymous role=alert nodes with no text content at the end of the DOM. Repro: cargo build --release -p freshell-server npx playwright test --config test/e2e-browser/playwright.config.ts \ --project=rust-chromium restore-contract-wall-rust.spec.ts -g "THE RULER"
…potent read-only (PIN 3 server)
Step 1 join-key verification: the client's reconcile request builder
includes the pane's current terminalId —
src/lib/pane-reconcile.ts:127:
...(content.terminalId ? { terminalId: content.terminalId } : {}),
Step 5 no-consumption confirmation (delete-at-derive falsified by
load-bearing validation; read is idempotent/read-only):
grep -n "delete_pending" crates/freshell-ws/src/*.rs
pane_ledger.rs:760 (definition)
pane_ledger_tests.rs:436,440 (ledger tests)
terminal.rs:1338 (exit-hook path)
terminal.rs:3792 (locator-resolution/GC path)
NONE in handle_pane_reconcile — the derive path in reconcile.rs is the
READ side's only touch point.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
… identity (PIN 3 client) 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…eadcrumb + re-armed locator re-capture (PIN 3) P1.10 pinning unit test verified landed before the flip: cargo test -p freshell-ws restore_created_pane_without_identity_arms_and_resolves_into_the_ledger -> test opencode_association::tests::restore_created_pane_without_identity_arms_and_resolves_into_the_ledger ... ok -> test codex_association::tests::restore_created_pane_without_identity_arms_and_resolves_into_the_ledger ... ok Flip proven first: the pinned leg failed with 'Expected to fail, but passed.' (breadcrumb now matches the wall probe regex in the DOM). Pin deleted, comment updated to the live contract, and a P1.10 end-to-end re-capture assertion added: open the row gate post-restart, submit, and the re-armed locator must resolve a ses_ identity into the leaf's sessionRef. Flipped leg green twice sequentially. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…cedes argv observability (PIN 2 server)
Step 1 pin-red confirmation: the SIGKILL-within-5s leg still fails as
expected (JSON reporter: expectedStatus=failed, status=failed; the
summary's '1 passed' is the test.fail accounting).
Step 1 instrumentation output (single instrumented run, reverted):
LEDGER BINDINGS AT KILL: [ 'claude' ]
A binding file was present at kill: the write-vs-SIGKILL race did NOT
materialize on this run — Gap B (no recovery surface consuming the row,
Task 7) was the observed blocker. The pre-spawn write still lands: the
window is real at the code level (argv observable at the PTY spawn,
durability previously only ~230 lines later), and the leg's contract is
'kill immediately after argv' — without tightening, green would be
timing-luck.
Note (2) answer — should claude also get a pending marker
(MARKER_MODES)? No. MARKER_MODES correctly still excludes claude:
claude has create-time identity (the preallocation) and no post-spawn
resolver, so a marker for it could never resolve. This pre-spawn
binding row IS its durability story — strictly stronger than a marker.
Step 4b (validated gap): a pre-spawn row for a spawn-FAILED fresh
claude preallocation would surface as a ghost ledgerOnly recovery
offer for ~30 days. The spawn-failure branch now deletes the
just-written row (new PaneLedger::delete_binding, the atomic mirror of
delete_pending) for fresh preallocations only — resume-creates keep
their row (prior epoch, must stay recoverable). Covered by
deleted_binding_row_is_gone_for_recovery_readers.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…correct recovery-offer probe (PIN 2)
…lerts (PIN 1 diagnosis) Task 8 Step 2 diagnosis of the ruler's newly-surfaced red (recorded in 3f776d5): getByRole('alert') at :1805 expected 0, got 2. The two alerts are NOT product alerts and NOT a side effect of the claude respawn path. They are monaco-editor's permanent accessibility scaffold: setARIAContainer (node_modules/monaco-editor/esm/vs/base/browser/ui/aria/aria.js) appends a monaco-aria-container with exactly TWO empty role="alert" divs (.monaco-alert / alertContainer + alertContainer2) the moment the first editor mounts. The ruler composition includes an editor pane (pane-ruler-editor, spec :1590), so a bare getByRole('alert') count is structurally >=2 regardless of restart behavior. The assertion's donor (restore-sync05.spec.ts) has no editor pane, which is why it passes there. This red was latent: the ruler previously died at the claude --resume argv poll (:1753) before :1805 ever executed under composition; the PIN 1 carve-out let the run proceed and exposed it. Fix: count [role="alert"]:not(.monaco-alert) instead. This is a false- positive correction, not a weakening: every product alert (Pane error banner, TerminalExitBanner, fresh-agent banners, ConnectionErrorOverlay, DirectoryPicker error, error-boundary) lacks .monaco-alert and is still counted; the excluded nodes are empty screen-reader announcement slots invisible to users. Evidence: error-context.md from the expected-fail run shows the two alerts as anonymous empty nodes in a trailing container outside the app tree, matching monaco-aria-container exactly. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…now green (PIN 1) Deletes the P0.1 test.fail pin (spec :1500-1503) and trims the pin's explanatory comment to a short "ruler is live" note. The composed all-pane ruler now passes genuinely: - The pinned red (claude terminal §2.2 --resume argv poll under composition) closed with the PIN 1 never-conversed carve-out (e970d9c) — verified green under composition in the deferred-flip run recorded in 3f776d5. - The newly-surfaced red past :1762 (quiet-client alert count) was diagnosed as monaco-editor's structural aria scaffold, not a product alert and not a branch side effect; corrected in 8923b7e. Flip evidence (Task 8 Step 2): with the pin still in place the ruler reported "Expected to fail, but passed" (JSON stats unexpected:1); after deletion, two consecutive un-pinned runs green (28.1s and 28.0s, JSON stats expected:1 unexpected:0 each). Zero test.fail / test.fixme markers remain anywhere in the wall spec. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ocs only Fresh-eyes review found the pre-spawn ledger binding write (PIN 2) was gated on `mode == "claude" && resume_session_id.is_some()`, so it fired for EVERY claude create — resume/restore creates included — while the compensating spawn-failure delete was (correctly) restricted to claude_fresh_prealloc. That asymmetry let a failed or race-losing RESUME create durably rewrite a binding row it does not own (live_terminal_id -> never-spawned terminal, create_request_id -> the failing create, last_observed_at bumped). In the duplicate-live-resume race (the AlreadyExists arm), a loser's write landing after the winner's post-spawn write would leave the durable ledger pointing the live session at a dead terminal id — the D8 ghost-recovery / duplicate-writer shape. - terminal.rs: gate the PIN2_CLAUDE_PRE_SPAWN_BINDING write on claude_fresh_prealloc (the freshly minted UUID is provably exclusive); comment rewritten to document the scoping and the race-loser hazard. Failure-branch delete unchanged. Source-order pin tokens unchanged. - tests/pane_ledger_triggers.rs: new behavioral test failed_claude_resume_create_leaves_prior_binding_row_untouched — seeds a prior-epoch binding row, issues a claude resume create whose PTY spawn fails, asserts PTY_SPAWN_FAILED and the row is byte-for-byte untouched (index + fresh on-disk reader). RED verified pre-fix (row rewritten exactly as the reviewer predicted), GREEN post-fix. Verified: cargo test -p freshell-ws --lib (312 passed, incl. the create-ordering pin), --test pane_ledger_triggers (5 passed), clippy --all-targets clean, fmt clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes the LAST THREE expected-fail pins in the restart contract wall — the wall (test/e2e-browser/specs/restore-contract-wall-rust.spec.ts) is now fully green with zero pins, proven 3x consecutive. Three root-caused fixes (systematic-debugging investigation preceded implementation):
PIN 1 (product bug): claude terminals that were opened but never conversed with were falsely adjudicated DeadSession{session_not_on_disk} after restart — derive_verdicts had an amplifier carve-out for stubbed-never-typed sessions but none for claude's preallocated-but-unconversed shape. Now: valid ledger binding + absent transcript ⇒ Respawn. Hazard guard pinned: genuinely deleted transcript with prior conversation still yields DeadSession. (Load-bearing check corrected the plan against the REAL claude CLI: --resume fails FAST on a missing transcript — loud and actionable — while --session-id silently reuses a deleted id, which is strictly worse; --resume kept deliberately.)
PIN 3 (unbuilt designed behavior): SIGKILL inside the opencode locator window yielded SILENT fresh — the ledger pending markers were written at spawn but never READ. Now reconcile consults pending markers before the generic no-identity Fresh and returns fresh_by_race, folded client-side into a visible breadcrumb (idempotent read — the original consume-once design was caught losing the breadcrumb on reconnect churn); plus the opencode locator re-arms on restore-created panes so the next submit re-captures identity.
PIN 2 (wiring gap): a claude pane created seconds before SIGKILL + full browser loss wasn't recoverable — claude sessions now get their durable ledger binding BEFORE spawn, with cleanup on the spawn-failure branch (ghost-offer prevention) and the write gated to FRESH claude creates only (delta review round 1 caught that resumes could clobber a live session's binding — fixed red-first, eaa25b7).
Verification: full wall 3x 15/15 zero pins; cargo workspace 2208 green; npm 4569 green; test:port 38/38; lint clean; hidden-pane-rebind + reconcile + freshclaude sibling suites green post-refresh onto main@7e29dad1.