Fix the six recurring test failures on main (title-precedence + 59nb capture race + hsrh session-init) - #805
Merged
Merged
Conversation
…s (invariants probe-phase family to :1234); stranded 8186d9f merges onto the zombie, so Task 5 applies by test name
…third fabricated-title assertion + cfg(test) provider_display_name, sidebar provider-label rung for fabricated rows, 59nb per-test terminal-id renames, accurate display-precedence ladders + contract scope-table row, titleSetByUser side-effect analysis, hsrh uncommitted 200ms red repro, e2e-lane gate in Task 6 (npm test runs no Playwright; CLOUD_SKIP census = 2 of 4)
…aude-r (slice(0,8) verified); cap-unenforceable row fixed to real message + root field (no path) with corrupt-dir-exempt trap note; honest NOT-coordinator-gated e2e lane wording + by-hand gate check; base-comparison triage step (2c) for non-campaign failures; hsrh three-beat repro (green under forced interleave, then revert sleep); provider_display_name + join_tests parity test DELETED (not cfg(test)); 59nb scope extended to the two thread-local set_default captures (lock_failure_capture, create_dedupe DIAG-01) + census/commit/audit updates; {events:?} failure dumps narrowed to filtered hits at six sites; Sidebar.tsx:486-489 guard citations in both places; no-import set_default wording
… the base's 54850ac pre-registration deferral (verified: ancestor of dbbfd07, NOT of 8186d9f; consumer broadcasts the init frame on the Deferred arm, row writes ~25ms later) — either-order frame drain kept VERBATIM + NEW await_claude_session_init_binding_row row barrier (bindings-vec poll, was_recorded cannot see the blank lineage row), row-sourced assertions, false premise comments replaced, pre-insert sleep placement forces BOTH orderings; 59nb: three terminal.rs with_default filter-shape captures recorded as deliberate exceptions (claim softened, Step 6 grep sees with_default), shared-lock poisoning handled (on_event PoisonError recovery + collect-hits-then-assert), whole mod lock_log_capture deleted (not just the fn), E0308-safe .map(String::as_str) filters (oversize + DIAG-01, rustc-verified), DIAG-01 test name + record_u64→record_debug carry-over reasoning fixed; known-open 3fxd/y5fw load-flake allowance added to the x3 loop and Task 6
…tles; fabricated live-terminal rows carry no title
… deploy-tab-diff divergence identity
…le ladder, mirror diagnosis corrected by execution evidence)
…y auto-title (D3) The cwd-leaf banner expectation rotted with e78c25c: registry auto-titles own terminal panes (terminal.rs mode_label -> terminals.changed -> recordTerminalTitleForReplay -> initLayout replay), so the paned child-target pane renders 'Pane: OpenCode', never the cwd-leaf. The banner assertion and both rotted comments now document the ladder + writer path; rail-flow assertions unchanged. Fresh focused run green (reports/t2-rail-reshape.log). Plan Task 2 Step 3 ticked complete — all four campaign families green.
…s (kata 59nb) — fixes callsite Interest-cache poisoning
…ing zombie (kata hsrh part 1) — superseded by the r27 F4 lineage-row reshape
…delta review Minor 2, plan D5) D3 already showed the title:None change fixes none of the six campaign failures, and it ships real user-visible regressions: HistoryView's main label for terminal:<id> rows degrades to the literal 'terminal' (HistoryView.tsx:483), the server's title-search tier stops matching placeholder rows, and the sidebar label changes. The remaining value was cosmetic honesty; the mirror-fold case the removed comment cited was the D3-disproven subagent mechanism, and for shapes that do reach the client the fabricated title equals the registry auto-title the pane ladder itself crowns. session_directory.rs is byte-identical to dbbfd07 again (provider_display_name + parity test + the three base assertions restored); the sessionTitleMirror fabricated-row pin is deleted (the no-title skip keeps its pre-existing coverage); the contract doc's fabricated-row paragraph is replaced by the surviving title-less-running- row display rule. The client titleSetByUser fold and the sidebar display fallback are KEPT (the fallback's name order is fixed in the next commit). Verified: the four campaign e2e families green focused, the Task-1 unit files green, session_directory tests green.
… order (delta review Minor 1) The provider-label rung skipped the name order the client-side fallback row already uses (pane title -> terminal title -> provider label), so the label changed once the server's placeholder row arrived (the fallback row showed the registry title, e.g. 'Codex CLI'; the server row showed 'Codex'), and getProviderLabel without extension data rendered Opencode/Codex/Claude as the whole label. The rung now composes the same order — pane title, then the terminal's registry title, then the provider label — so the label is stable when the server row replaces the fallback row and the provider label is only the last rung. New unit test pins the order (red-first: 'expected Codex to be My pane title'); the existing title-less-running test keeps its expectations (provider label with no local pane/terminal info; the 'claude-r' id-prefix pin) with comments updated to the post-D5 wire shape — server placeholder rows carry the provider display name again, so a title-less running row is a real session's pre-transcript window.
…(delta review Minor 3 disposition) The CHILD_PANE_DIR_LEAF button check is live coverage, not dead: the paned child-target row it guards is the pane-content fallback row (pushFallbackItem / collectFallbackItemsFromNode in sidebarSelectors.ts), which takes its cwd from the pane content's initialCwd — NOT the terminal's cwd. Verified empirically (receipt reports/delta-r1-minor3-rail-badge-proof.log): the exact e2e store shape builds a row titled 'OpenCode' badged 'railsubagentpane', hidden only by the isSubagent classification — a regression there is exactly what the check catches. The comment now names the mechanism and the terminal-cwd-badged shapes (server fabricated row, manufactured row) that CHILD_TERMINAL_CWD_LEAF negates. Check and doc comment otherwise unchanged; spec re-run green focused after the round-1 fixes.
… standing-reds classification + honest Task-6 gate, D5 fabricated-title revert Major: Task 4 Step 5 and Task 5 Step 3 now record their actual results (1144 passed / 10 failed per whole-crate run; the 10 named; base + focused receipts cited) instead of the misstated 'Expected: PASS'. D4 records the classification decision: the 10 are pre-existing standing reds on main (fail identically-or-worse at dbbfd07, fail focused too — deterministic, not load flakes, not regressions of this branch), out of scope under no-scope-creep. Task 6's gate is redesigned honestly: the Rust lane WILL show the 10; its pass criterion is green EXCLUDING the recorded set (the e2e campaign-ledger model), Step 3 carves the recorded set out of the 'persists => genuinely new' rule, and Step 4 files one kata per family at wrap (kata list verified: none on file; three families). The Goal and verification summary carry the same qualifier. D5 records the Minor 2 decision (server-side title:None REVERTED; reasoning + kept client halves), with AMENDED pointers in the Architecture, Resolution, Task-1 Files list, and the rail bullet.
…rion; kata closures 59nb+hsrh; D4-family filings 7ad4/17eh/p6q2 (+ gate-discovered 84nb); e2e four CLEARED
…nner (D6 landing blocker)
Owner
Author
|
Landing-blocker root cause + fix (plan amendment D6): the rust-tests CI job (new in #803) never installed npm deps, so the codex fake-app-server fixture (imports |
…blocking rust-gate
… dropped from handle_send PR #795's ext-r8 handle_send rework silently dropped the send-time freshAgent.session.metadata convergence broadcast (present at bf9b8d3, gone by 38939b5, no compensating emission). A settings-bearing send that CHANGES the live session again converges every device's model surfaces — the same frame handle_configure emits; a device that missed the configure still lands here. Red test: claude::tests::a_settings_changing_send_broadcasts_session_metadata (failed claude.rs:22715 pre-fix, green post-fix, assertions unchanged). D7 group (c) of the main-green-sixpack campaign.
…ntics (D7 a+b+d) #795's r30 F1 (45ff519) deliberately refuses to publish a sidecar whose pid is already dead at the create's commit; the stale-family fixtures still scripted die-at-thread/start creates and never re-ran after that gate landed (the suite was red at merge — rust-gate only became required with #803). Each test's assertion intent is preserved exactly; only the fixture setup changes: - (a) The 8 codex a_stale_{fence,generation}_* tests now create with the sidecar ALIVE ("{}") and build the crashed state by hand — the manual exited flip each test already performed plus its existing begin_handoff/fail generation advance/vacate. The stale refusal under test happens at the coordinator claim, BEFORE any respawn, so the fencing semantics still face a dead-but-materialized session and the no-recreation asserts stay honest. None of the 8 requires death-at-create; all 8 red before (codex.rs:19241 freshAgent.create.failed), green after, assertions unchanged. - (b) opencode_ws::a_stale_fence_map_hit_attach: fenced_state_with_map_row now seeds a MATERIALIZED row (real_session_id: Some) per gate-C (7824b5a) — unmaterialized placeholders are observation-only on attach, so the r15 F1 fenced map-hit adopt claim the test covers is only consulted for a materialized row. Both create-path siblings sharing the fixture stay green. - (d) fork_in_flight_guard_covers_the_respawn_rekeyed_parent_id: same alive-at-commit create, then the post-crash state by hand — exited flip, mark_thread_dead, and clear_binding (the exact release cleanup_confirmed_codex_teardown performs on a confirmed death). The old fixture depended on the exit-watcher's reap-confirmation machinery, which fails closed on the CI runner: the retained binding answered the fork's lease claim BoundLive→Recovered and the mint-new respawn (with its OLD→NEW materialized broadcast) never ran — the rust-gate Elapsed. No such dependency remains. Whole-crate freshell-freshagent --lib: 1154 passed, 0 failed. D7 groups (a)+(b)+(d) of the main-green-sixpack campaign.
… the fake app-server (D7 e) a_codex_handoff_on_an_old_rebound_reference_resolves_the_permanent_alias spawned the handoff's codex TARGET through whatever CODEX_CMD names — unset, that is the host's real codex binary, so the test passed only where codex is installed and failed on the rust-gate runner with TARGET_SPAWN_FAILED (codex app-server spawn failed (codex): No such file or directory). Route the spawn through the committed fake app-server — the same CODEX_CMD wrapper codex.rs's own fixtures use, now pub(crate) for sibling modules — under codex.rs's ENV_LOCK (its process-global surface; the guard snapshots/restores every mutated variable on drop, the mismatch-refusal sibling's pattern). The fake's thread/resume echoes the requested id, so the canonical-key target resumes cleanly and the alias-resolution assertions are unchanged. D7 group (e) of the main-green-sixpack campaign.
…ily cleared, katas 7ad4/17eh/p6q2 closed)
…cise the fence again (D7 r2) The focused round-1 review found D7's alive-at-commit reshape had quietly changed what two of the eight codex stale-family tests verify: with the sidecar alive, begin_handoff captured a Live prior and fail(.., true) RESTORED it at the advanced generation, so a_stale_generation_crashed_attach… was refused by the tracked arm's adopt guard (never reaching ensure_session_alive) and a_stale_generation_send… could not tell a fenced claim from an unfenced one (AdoptLive → has_live_session=false → the same SESSION_RESERVED). Witness: with the claim's fence temporarily dropped to None (the pre-r8 bug), both tests still passed. Fix: vacate like the six siblings — fail(.., false), the crash shape (the row is marked exited, so the failed unwind cannot confirm the prior live and never restores it; identical terminal state to the original pre-D7 fixture) — plus harness sanity asserts pinning Vacant at the advanced generation so a future restored-Live mistake fails loudly, refreshed docstrings naming the exact guard under test, and the family comment corrected (only the exited flag flips; the sidecar process stays alive). Proof the fence is now exercised: the same temporary fence-drop makes both reshaped tests FAIL (receipts d7-r2-*.log). Also: fake_codex_app_server_cmd visibility reverted to module-private (only same-module configure_fake_codex_cmd calls it), and the restored send-time metadata broadcast gains its negative test — a_settings_unchanged_send_broadcasts_no_metadata (an unchanged pair converges nothing and writes no second configure), mirroring the idempotent-configure test. The (d)/(e) evidence gap the review flagged is closed with receipts: the rust-gate run 35436750812 CI red is captured into reports/, (e) is proven deterministically under a codex-less PATH (pre-fix shape fails byte-identically to CI; the fix passes), and (d)'s fail-closed reap-confirmation mechanism is demonstrated by simulation (old fixture + forced fail-closed confirmation reproduces the CI Elapsed byte-identically; the committed fixture passes under the same simulation). The plan addendum records the honest residual: which /proc sub-condition fails on the actual runner is not determinable from the captured CI output. Whole-crate freshell-freshagent --lib: 1155 passed, 0 failed; clippy -D warnings + fmt clean.
…runner's reap-confirmation (D8) The rust-gate run 3 (post-D7) flipped 7 previously-green die-at- thread/start recovery fixtures red on the 2-core runner, all SESSION_RESERVED "Another resume for this session is in flight" (attach_after_unrequested_crash_recovers..., concurrent_send_and_attach _single_flight..., fork_after_a_mint_new_respawn_keeps_the_create _provenance_chain, fork_after_unrequested_crash_respawns..., fork_on_a_mint_new_respawn_keys_mid_flight..., fork_on_a_mint_new _respawn_keys_the_forked_reply..., send_after_crash_mint_new_thread _broadcasts...). Root cause (proven, not the product): those fixtures crash the sidecar for real, so the exit-watcher runs the reap-confirmation machinery D7 group (d) recorded as FAILING CLOSED on the runner. Unconfirmed, the watcher retains a condemned prior; the test's single foreground action then enters ensure_session_alive, whose fail-closed condemned-prior guard answers Reserved -> the typed SESSION_RESERVED. Proven by the D7-r2-standard forced fail-closed A/B: the pre-fix fixtures under a forced-false kill_and_confirm_recorded_tree_dead fail byte-identically to CI (6/7 at the same panic lines; the attach test's CI shape differs only because the real confirm's latency orders the error frame ahead of the consumer's trailing snapshot). The D7 delta exposed the latent race by load-shape -- the now-green stale/(d)/(e) families spawn-and-hold real sidecars instead of failing fast, shifting the runner's per-instance confirm timing (run 2 passed all 7; run-3 same-fixture siblings passed seconds apart from the failing ones). The product is correct as-is: the per-thread resuming lock already JOINS concurrent same-session resumes, and the fail-closed refusal is the deliberate never-respawn-beside-an- unverified-writer guard (retryable, never lost; the claude lane answers the same shape). Fix (same reshape as D7 (a)/(d)): the 7 fixtures now create with the sidecar ALIVE and produce the post-crash state via the new simulate_unrequested_crash helper -- flip exited (the lazy-restart flag the watcher's self-heal sets) + clear the completed lease binding (the documented watcher release) -- so the recovery pipeline under test (respawn, resume/mint-new, snapshot/materialized broadcasts, the fork handoff, spawn counts, op-log RPC orders) still runs end-to-end for real; only the crash DETECTION is simulated (the onexit_self_heal_* family keeps covering the real watcher flow). No sleeps, no wall-clock raises; the rejected alternative (an event-driven wait for the reservation to settle) fails because the escalation emits no completion frame and never settles under a permanently fail-closed capture. Receipts (reports/d8-*.log): red A/B (pre-fix + simulated runner -> CI-identical), green A/B (post-fix under the SAME simulation 7/7), taskset before/after (-c 0 --test-threads=1 / -c 0 / -c 0,1 -- all green on both sides; plain CPU starvation does not reproduce the runner's /proc condition), focused plain 7/7, whole-crate 1155/0, clippy -D warnings + fmt clean. Honest residual recorded in the plan: the 8 remaining wait_for_self_heal callers share the latent fixture dependence and pass only by winning the same per-instance race -- out of this task's scope (not among the failing 7).
added 2 commits
September 19, 2026 06:35
…convert the 6 exposed crash fixtures (D8 r2) Round-3 review found the D8 reshape left two holes: Major 1: attach_after_unrequested_crash_recovers_and_emits_a_snapshot passed whether or not the attach recovered -- every create leaves a create-time snapshot event on the bus (thread/start -> thread/started -> the consumer, released after freshAgent.created), wait_for_self_heal used to consume it, simulate_unrequested_crash reads nothing, so the test's generous materialized-or-snapshot assert latched the stale frame (D8's own red A/B already showed the pre-fix test passing under the forced fail-closed confirm). Redesigned, name = contract: attach_after_unrequested_crash_respawns_the_sidecar_and_emits_a_post_ recovery_snapshot -- drains the create-time snapshot first (new drain_create_time_snapshot helper), asserts a post-attach snapshot naming the session, pins spawn_count == 2, and refuses error frames both bare and freshAgent.event-wrapped (the Reserved loser answer rides the event envelope; a bare-type-only assert degrades to silence-then- timeout). Major 2: six recovery tests still used the exact die-at-thread/start fixture the runner fails closed on (run 3's own log: near-identical twins split seconds apart -- a per-instance coin-flip). Converted to simulate_unrequested_crash, assertion intent preserved per test: fork provenance carry (resume path), send same-id resume acceptance, send not-found mint-new fallback, send transient CODEX_RESPAWN_FAILED + stays-exited, attach same-id snapshot (drain added), compact respawn-then-compact (spawn counts + two-connection op log). The two real-crash wait_for_self_heal callers stay, justified: diag01 (subject IS the detection machinery) and handle_create_replay (never drives a foreground recovery -- the Reserved window cannot fire; both passed run 3's runner). Minor + nits: the D8 plan entry's "Product verdict: correct as-is" softened to what the A/B proves vs. the undetermined /proc sub-condition, with the stuck-Reserved production exposure recorded as open follow-up kata hcax (commit message left as history, correction noted); the D7-(d) fork-guard test now calls the helper instead of inlining its body; the helper's doc states the two real differences from a genuine crash (unwired-registry-only validity; the still-alive old sidecar's teardown ordering); the run-3 CI receipt copied into the evidence base as reports/d8-red-ci-run-35442055157-excerpts.log and cited there. Red-proof (D7-standard targeted regression, reverted after receipts): ensure_session_alive short-circuited to answer Reserved -- the runner's fail-closed foreground shape -- makes ALL SEVEN reshaped/converted tests fail, six fast at the precise assert naming the SESSION_RESERVED frame, the mint-new fallback via its materialized-wait Elapsed (its green path legitimately emits a wrapped THREAD_MEMORY_LOST degradation, so it keeps no blanket event-error assert). Receipts (reports/d8-r2-*.log): red short-circuit A/B 7/7 red, green focused 16/16, taskset -c 0 spot-check 7/7 solo + a 201-test unrequested_crash-filtered run green, whole-crate 1155/0, clippy -D warnings + fmt clean.
…es (D8 r3) The round-4 focused review passed the D7+D8 delta with four test-strength/doc-accuracy findings; all four are mechanical: Minor 1: concurrent_send_and_attach_single_flight_recovery_for_the_ same_crashed_session promised "no user-facing error frame from either racing caller" but its loop only rejected bare error frames -- the SESSION_RESERVED loser answer rides the freshAgent.event envelope, so a regression of the per-thread resuming join (loser -> lease Held -> Reserved) passed blind whenever the attach lost the race (~half the runs; the send-losing half only went red via an anonymous timeout). The loop now also refuses event-wrapped freshAgent.error frames, and a spawn_count == 2 pin (create + exactly one recovery spawn) enforces the "never two independent respawns" half of the doc promise the D8 plan cites as the single-flight proof. Minor 2: the fork-guard test's comment blamed the old CI Elapsed on a BoundLive->Recovered lease-claim answer -- unreachable for this fixture: a fresh create binds no lease (finish_create runs with lease_guard = None; bindings come only from FreshSessionLeaseGuard:: complete) and handle_fork's ensure_session_alive answers Reserved at its condemned-prior check, before any lease claim. Rewritten to the proven mechanism: the runner's fail-closed reap retained the condemned prior -> Reserved -> the mint-new respawn (and its OLD->NEW materialized broadcast) never ran. Nit 1: the mint-new fallback test's materialized-wait loop now also refuses wrapped freshAgent.error frames. The old "keeps no blanket event-error assert" reasoning does not hold: the legitimate wrapped THREAD_MEMORY_LOST is broadcast strictly AFTER the materialized frame, and the loop returns on materialized, so the assert can never see it -- it turns a Reserved-regression red from a 15s Elapsed into a named-frame failure (D8-r2 plan entry corrected in place, r4 note). Nit 2: the redesigned attach test's doc claimed "BOTH asserts below fire" under the SESSION_RESERVED shape; only the loop's wrapped-error assert can (its panic ends the test before the spawn-count check). Now reads "either assert alone would catch it". Receipts (reports/d8-r3-*.log): the four touched tests green solo (d8-r3-green-focused-plain.log), the concurrent test 4/4 repeat solo runs green (d8-r3-concurrent-x4-determinism.log -- the loser is joined, never Reserved), whole-crate cargo test -p freshell-freshagent --locked --lib 1155/0 (d8-r3-whole-crate.log), clippy -D warnings + fmt clean (d8-r3-clippy-freshagent.log, d8-r3-fmt.log). No new red A/B this round: the identical wrapped-assert pattern's red is already receipted for the seven siblings (d8-r2-red-recovery-shortcircuit.log), and the review verified the join works in product code today.
This was referenced Sep 19, 2026
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.
What this does (all six root-caused first, then fixed)
The four recurring e2e families — investigation showed three were deterministic reds from the Sep 15 tab-title campaign, not load flakes:
deploy-tab-diff:81— stale expectation: untitled tabs now follow the pane's live title per the title pipeline (tab=Codex CLIafter restart). Test hardened to assert the stable tabKey identity alongside its original (now-correct)tab=workname check.remote-tab-linkage:107— product fix: the session-title mirror instantly renamed REST-created tabs. Theui.command{tab.create}fold now marks an explicit caller-providedtitleas user-set (titleSetByUser: true), so creator-named tabs keep their names; the mirror still titles untitled tabs (documented precedence: explicit creator/user title > session mirror > auto labels). Stale spec NOTE corrected + preservation pin.rest-tab-persistence:117— same root cause: the server's auto "Amplifier" mode-label outranked the explicit REST-provided title; fixed by the same fold.sidebar-opencode-rail:143— investigation's mirror diagnosis was corrected during execution (the fabricated subagent row is dropped server-side; the real writer is the terminal-directory title replay from e78c25c): per the pane ladder (registry auto-titles own terminal panes), the spec's cwd-leaf expectation rotted — reshaped to assert the canonicalPane: OpenCodewith the ladder documented.59nb (Rust) — the pane_ledger invariants-capture race, proven at the tracing-core level: sibling tests executing a shared log site first can register it as
Interest::neveragainst no subscriber, so the thread-local capture sees zero events. Fix: migrate to the repo's established e08g pattern — one process-globalOnceLockcapture with per-test-unique field filtering at all 28 sites, forced worst-case-order regression proof (was red 1-in-30 at base; now 30/30 green pinned), lock-poisoning recovery, narrowed failure messages, dead capture module deleted. The 3 sanctionedwith_defaultfilter-shape tests are documented exceptions.hsrh (Rust) — two fixes: (a) deleted the zombie test a merge accidentally resurrected (it was deliberately retired last week and reshaped into the stronger lineage-row test); (b) landed the either-order combined frame drain + a binding-row barrier on the three live siblings (the init frame can arrive before the drain listens, and after 54850ac the frame no longer proves the row landed) — proven by forced-interleave red→green with the sleep harness in place.
Evidence (receipts under
.worktrees/.the-usual-logs/main-green-sixpack/)--no-verifyfor that reason. rust-gate on the clean CI runner arbitrates.