diff --git a/crates/freshell-freshagent/src/claude.rs b/crates/freshell-freshagent/src/claude.rs index 05120bdce..91f7a9782 100644 --- a/crates/freshell-freshagent/src/claude.rs +++ b/crates/freshell-freshagent/src/claude.rs @@ -13950,6 +13950,40 @@ rl.on('line', (line) => { } } + /// Wait until the fresh create's adoption has COMMITTED ownership under + /// `durable_id`. The adoption publishes the `cli_index` alias and then + /// writes the identity binding BEFORE its commit (ext r27 F4: a failed + /// binding abandons it pre-commit), and the commit-to-Live plus the + /// retained release stamp are one critical section under the stamps + /// lock (ext r30 F1) — so neither the alias nor `was_recorded` proves + /// the commit: both can be observed while the claim is still Starting + /// and the stamp is absent. The stamp is the real condition. + async fn await_create_ownership_committed(st: &FreshClaudeState, durable_id: &str) { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); + while crate::ownership_lane::peek_retained_stamp(&st.ownership_stamps, durable_id).is_none() + { + assert!( + tokio::time::Instant::now() < deadline, + "the create's adoption never committed ownership (no retained stamp)" + ); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + } + + /// [`await_create_ownership_committed`] for a create with a RECORDABLE + /// binding, then pins the binding-first order. + async fn await_create_adoption_committed( + st: &FreshClaudeState, + sink: &crate::identity_sink::FakeIdentitySink, + durable_id: &str, + ) { + await_create_ownership_committed(st, durable_id).await; + assert!( + sink.was_recorded("claude", durable_id), + "the adoption records the binding before it commits ownership" + ); + } + /// b8ke delta round-3 F1: a coordinator-REFUSED kill mutates NOTHING /// durable — the pane-ledger row stays BOUND, the session stays live, /// and the refusal broadcast is TYPED (code + message, never @@ -14051,20 +14085,7 @@ rl.on('line', (line) => { .await; let created = await_claude_created(&mut rx, "req-e3r1-f1").await; let placeholder = created["sessionId"].as_str().unwrap().to_string(); - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); - while st - .cli_index - .lock() - .await - .get(FRESH_CREATE_DURABLE_ID) - .is_none() - { - assert!( - tokio::time::Instant::now() < deadline, - "the create's adoption never published" - ); - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + await_create_ownership_committed(&st, FRESH_CREATE_DURABLE_ID).await; // THE COMPLETED HANDOFF (simulated at the coordinator level): the // stamp is consumed and a TERMINAL owner is committed under the @@ -14134,14 +14155,7 @@ rl.on('line', (line) => { st.handle_create(create, None).await; let created = await_claude_created(&mut rx, "req-e3r1-f2").await; let placeholder = created["sessionId"].as_str().unwrap().to_string(); - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); - while !sink.was_recorded("claude", FRESH_CREATE_DURABLE_ID) { - assert!( - tokio::time::Instant::now() < deadline, - "the create's adoption never recorded the binding" - ); - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + await_create_adoption_committed(&st, &sink, FRESH_CREATE_DURABLE_ID).await; assert!( crate::ownership_lane::peek_retained_stamp( &st.ownership_stamps, @@ -14218,14 +14232,7 @@ rl.on('line', (line) => { st.handle_create(create, None).await; let created = await_claude_created(&mut rx, "req-e3r3-f8").await; let placeholder = created["sessionId"].as_str().unwrap().to_string(); - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); - while !sink.was_recorded("claude", FRESH_CREATE_DURABLE_ID) { - assert!( - tokio::time::Instant::now() < deadline, - "the create's adoption never recorded the binding" - ); - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + await_create_adoption_committed(&st, &sink, FRESH_CREATE_DURABLE_ID).await; let stamp = crate::ownership_lane::peek_retained_stamp( &st.ownership_stamps, FRESH_CREATE_DURABLE_ID, @@ -14329,14 +14336,7 @@ rl.on('line', (line) => { st.handle_create(create, None).await; let created = await_claude_created(&mut rx, "req-e3r4-f1").await; let placeholder = created["sessionId"].as_str().unwrap().to_string(); - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); - while !sink.was_recorded("claude", FRESH_CREATE_DURABLE_ID) { - assert!( - tokio::time::Instant::now() < deadline, - "the create's adoption never recorded the binding" - ); - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + await_create_adoption_committed(&st, &sink, FRESH_CREATE_DURABLE_ID).await; // THE NO-STAMP SHAPE: consume the retained stamp (the handoff's // kill_for_handoff consumption) — the kill runs the no-stamp @@ -14444,14 +14444,7 @@ rl.on('line', (line) => { st.handle_create(create, None).await; let created = await_claude_created(&mut rx, "req-e3r2-f1").await; let placeholder = created["sessionId"].as_str().unwrap().to_string(); - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); - while !sink.was_recorded("claude", FRESH_CREATE_DURABLE_ID) { - assert!( - tokio::time::Instant::now() < deadline, - "the create's adoption never recorded the binding" - ); - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + await_create_adoption_committed(&st, &sink, FRESH_CREATE_DURABLE_ID).await; // THE MID-HANDOFF WINDOW: kill_for_handoff consumed the stamp // while the coordinator record stays Handoff. @@ -14607,14 +14600,7 @@ rl.on('line', (line) => { st.handle_create(create, None).await; let created = await_claude_created(&mut rx, "req-e3r2-f4").await; let placeholder = created["sessionId"].as_str().unwrap().to_string(); - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); - while !sink.was_recorded("claude", FRESH_CREATE_DURABLE_ID) { - assert!( - tokio::time::Instant::now() < deadline, - "the create's adoption never recorded the binding" - ); - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + await_create_adoption_committed(&st, &sink, FRESH_CREATE_DURABLE_ID).await; // THE RUNTIME EXITS (the sidecar's recorded pid is killed outright // — it is dead by the time the kill's unwind verdict runs). @@ -14691,20 +14677,7 @@ rl.on('line', (line) => { .await; let created = await_claude_created(&mut rx, "req-e3r1-f3").await; let _placeholder = created["sessionId"].as_str().unwrap().to_string(); - let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); - while st - .cli_index - .lock() - .await - .get(FRESH_CREATE_DURABLE_ID) - .is_none() - { - assert!( - tokio::time::Instant::now() < deadline, - "the create's adoption never published" - ); - tokio::time::sleep(std::time::Duration::from_millis(10)).await; - } + await_create_ownership_committed(&st, FRESH_CREATE_DURABLE_ID).await; // THE UNOWNED-LIVE shape: the coordinator key goes Vacant (a // confirmed-kill release) while the lane map still holds the live @@ -16699,6 +16672,10 @@ rl.on('line', (line) => { .await; let created = await_claude_created(&mut rx, "req-postcap-f3").await; let _placeholder = created["sessionId"].as_str().unwrap().to_string(); + // The first create's adoption must COMMIT before the pre-commit + // park arms — otherwise it parks there too, and both the `reached` + // flag and the stale-start recovery below see the WRONG claim. + await_create_ownership_committed(&st, FRESH_CREATE_DURABLE_ID).await; // The pre-commit park: a SECOND session (fresh map key — the // ordinary-create shape, no prior durable id) whose adoption diff --git a/crates/freshell-freshagent/src/claude_snapshot.rs b/crates/freshell-freshagent/src/claude_snapshot.rs index f06d5e20a..6773a7dd4 100644 --- a/crates/freshell-freshagent/src/claude_snapshot.rs +++ b/crates/freshell-freshagent/src/claude_snapshot.rs @@ -2732,11 +2732,15 @@ pub(crate) mod tests { /// under the `CLAUDE_CONFIG_DIR` candidate, and agrees with /// [`locate_transcript`] on the same ordered candidates (one definition /// of "the transcript exists"). Env vars are process-global — serialize - /// under the shared claude env lock. + /// under the shared claude env lock. Only the claude-specific roots are + /// cleared: `HOME` is never touched, because tests that do not take the + /// claude lock read it concurrently (the OpenCode home harness asserts + /// the parent's `HOME` is unchanged) and the platform home candidate + /// resolves through the passwd entry without it anyway. #[tokio::test] async fn locate_transcript_selected_answers_root_path_and_cwd_under_config_dir() { let _guard = crate::claude::tests::CLAUDE_ENV_LOCK.lock().await; - let _restore = EnvVarsRestore::remove_all(&["CLAUDE_CONFIG_DIR", "CLAUDE_HOME", "HOME"]); + let _restore = EnvVarsRestore::remove_all(&["CLAUDE_CONFIG_DIR", "CLAUDE_HOME"]); let store = temp_home(); std::env::set_var("CLAUDE_CONFIG_DIR", store.path()); let id = "66666666-6666-4666-8666-666666666666"; diff --git a/crates/freshell-freshagent/src/lib.rs b/crates/freshell-freshagent/src/lib.rs index b6f5b9212..386ed76b9 100644 --- a/crates/freshell-freshagent/src/lib.rs +++ b/crates/freshell-freshagent/src/lib.rs @@ -2221,10 +2221,11 @@ pub struct FreshAgentState { /// b8ke focused round-4 R4-2 test seam: when set, the REST opencode /// drive PARKS at the exact interleaving point under review — after /// the gate-held ownership/condemned re-check, BEFORE the prompt POST - /// — until the Notify fires. Never armed in production. Installed via + /// — until released (see [`RestTurnTestPause`]). Never armed in + /// production. Installed via /// [`Self::set_rest_turn_test_pause_for_test`]. #[cfg(test)] - rest_turn_test_pause: Arc>>>, + rest_turn_test_pause: Arc>>>, /// The ONE server-wide runtime-ownership coordinator (kata b8ke Task 3), /// wired from freshell-server::main next to `fresh_agent_leases`. /// `None` (every pre-existing test) = the lane skips coordinator @@ -2323,6 +2324,24 @@ pub(crate) struct RestOpencodeTurn { pub(crate) route: Option, } +/// b8ke focused round-4 R4-2 test seam (cfg(test) only): the handshake a +/// test uses to hold the REST opencode drive at the interleaving point +/// under review and to observe a lifecycle quiesce queueing behind it. +/// Every signal is `notify_one` (a stored permit), so a waiter that +/// arrives late still sees it — no step depends on scheduling order. +#[cfg(test)] +#[derive(Default)] +pub(crate) struct RestTurnTestPause { + /// Signalled by the drive once it is parked — the gate held, the + /// ownership/condemned re-check done, the prompt POST not issued. + pub(crate) parked: tokio::sync::Notify, + /// Awaited by the parked drive before it issues its prompt POST. + pub(crate) release: tokio::sync::Notify, + /// Signalled by a lifecycle quiesce that found the session's + /// dispatch/condemn gate held and is about to wait on it. + pub(crate) quiesce_blocked: tokio::sync::Notify, +} + /// A Slice-1 terminal pane's record: just enough to dispatch `send-keys` / /// `capture` / `wait-for` to the right `terminal_id` in the shared registry. #[derive(Clone)] @@ -3113,16 +3132,31 @@ impl FreshAgentState { /// b8ke focused round-4 R4-2 test seam installer: see /// [`Self::rest_turn_test_pause`]. #[cfg(test)] - pub(crate) fn set_rest_turn_test_pause_for_test( - &self, - pause: Option>, - ) { + pub(crate) fn set_rest_turn_test_pause_for_test(&self, pause: Option>) { *self .rest_turn_test_pause .lock() .expect("rest_turn_test_pause mutex") = pause; } + /// R4-2 test seam probe, called by the lifecycle quiesce paths right + /// before they wait on the session's dispatch/condemn gate: when the + /// seam is installed and the gate is held (the parked drive owns it), + /// signal [`RestTurnTestPause::quiesce_blocked`]. + #[cfg(test)] + pub(crate) fn note_rest_turn_gate_wait_for_test(&self, gate: &tokio::sync::Mutex<()>) { + let pause = self + .rest_turn_test_pause + .lock() + .expect("rest_turn_test_pause mutex") + .clone(); + if let Some(pause) = pause { + if gate.try_lock().is_err() { + pause.quiesce_blocked.notify_one(); + } + } + } + /// The REST resume probe's bounded `get_session` budget, in milliseconds: /// cfg(test) state override > `FRESHELL_OPENCODE_GET_SESSION_TIMEOUT_MS` /// env parse > 10_000ms default — resolved purely by @@ -6845,7 +6879,8 @@ async fn send_keys( .expect("rest_turn_test_pause mutex") .clone(); if let Some(pause) = pause { - pause.notified().await; + pause.parked.notify_one(); + pause.release.notified().await; } } let rx = manager.subscribe(&durable_id); diff --git a/crates/freshell-freshagent/src/opencode_ws.rs b/crates/freshell-freshagent/src/opencode_ws.rs index 7d5069822..0c70cf52a 100644 --- a/crates/freshell-freshagent/src/opencode_ws.rs +++ b/crates/freshell-freshagent/src/opencode_ws.rs @@ -3473,6 +3473,8 @@ impl FreshOpencodeState { // dispatch/condemn gate, exactly like quiesce_rest_opencode_turn. let rest_turn = { let gate = self.fresh_agent.rest_turn_gate(session_id); + #[cfg(test)] + self.fresh_agent.note_rest_turn_gate_wait_for_test(&gate); let _gate = gate.lock().await; let rest_turn = self .fresh_agent @@ -3536,6 +3538,8 @@ impl FreshOpencodeState { // Bind before the await: the guard must never live across it. let witness = { let gate = self.fresh_agent.rest_turn_gate(canonical); + #[cfg(test)] + self.fresh_agent.note_rest_turn_gate_wait_for_test(&gate); let _gate = gate.lock().await; let witness = self .fresh_agent @@ -7229,6 +7233,23 @@ impl FreshOpencodeState { // claim and its own commit (under-ticket mode). handoff: Option<(&str, u64)>, ) -> Result>, ResumeOpencodeError> { + // A `freshopencode-*` placeholder is minted by Freshell for a pane + // that has not materialized yet. Opencode has never seen it, so no + // daemon can resume it: answer NotFound before ANY side effect — no + // coordinator claim, no lease, no daemon start. Pre-fix the claim + // below held the placeholder's key across the shared daemon's cold + // start (`get_session` starts the serve only to 404). After a server + // restart the reconnect attach did exactly that, and the pane's own + // first send inside that window had its materialize claim refused + // (SESSION_RESERVED), which lost the message. + if session_id.starts_with(crate::OPENCODE_PLACEHOLDER_PREFIX) { + tracing::info!(target: "freshell_freshagent::opencode", + session_id = %session_id, + "freshagent.opencode.resume_placeholder_not_found: a placeholder id has no \ + durable session to resume; nothing is claimed or started" + ); + return Err(ResumeOpencodeError::NotFound); + } // D8 (focused-ep1-r5 Finding 2): "meaningful provenance" only — a // HOLLOW `Some` (a partially initialized client's hello) behaves like // `None` on every decision below: the park falls through to the @@ -12886,6 +12907,159 @@ mod tests { } } + /// A [`RealisticServeHttp`] whose `/global/health` probe parks until the + /// test opens `latch`: the shared daemon's COLD START held open (the serve + /// process spawned but not yet answering). Each probe reports its arrival. + struct ColdStartGatedHttp { + inner: RealisticServeHttp, + health_arrived: tokio::sync::mpsc::UnboundedSender<()>, + latch: Arc, + } + impl ServeHttp for ColdStartGatedHttp { + fn request<'a>( + &'a self, + req: ServeHttpRequest, + ) -> std::pin::Pin< + Box< + dyn std::future::Future> + + Send + + 'a, + >, + > { + if req.url.contains("/global/health") { + let arrived = self.health_arrived.clone(); + let latch = Arc::clone(&self.latch); + return Box::pin(async move { + let _ = arrived.send(()); + let _open = latch.acquire().await.expect("the cold-start latch opens"); + Ok(ServeHttpResponse::new(200, b"{}".to_vec())) + }); + } + self.inner.request(req) + } + } + + /// The zero-turn restart race behind the `[freshopencode] pending rename + /// survives a zero-turn restart` e2e flake. After a server restart the + /// client's reconnect attach names the pane's never-materialized + /// `freshopencode-*` placeholder, which the new process has never seen. + /// Freshell mints placeholders; opencode never knows them, so no daemon + /// can resume one. The attach nevertheless claimed the placeholder's + /// coordinator key and held that claim across the shared daemon's cold + /// start (`get_session` → `ensure_started`). The reconcile's re-create and + /// the user's first send landed inside that window: the send's + /// materialize claim on the same key was refused Blocked and answered + /// SESSION_RESERVED, the client's reservation re-drive re-sent the + /// attach (not the message), and the pane never materialized. + #[tokio::test(flavor = "multi_thread")] + async fn an_untracked_placeholder_attach_never_holds_the_key_across_a_daemon_cold_start() { + let (tx, mut rx) = tokio::sync::broadcast::channel::(256); + let registry = Arc::new(freshell_ownership::RuntimeOwnershipRegistry::new()); + let fresh_agent = FreshAgentState::new(Arc::new("tok".to_string()), Arc::new(tx)) + .with_ownership(Arc::clone(®istry)); + let (health_tx, mut health_rx) = tokio::sync::mpsc::unbounded_channel::<()>(); + let latch = Arc::new(tokio::sync::Semaphore::new(0)); + let deps = ServeDeps { + spawner: Arc::new(TrackedSpawner { + killed: Arc::new(std::sync::atomic::AtomicBool::new(false)), + }), + http: Arc::new(ColdStartGatedHttp { + inner: RealisticServeHttp::new(), + health_arrived: health_tx, + latch: Arc::clone(&latch), + }), + ports: Arc::new(FakeAllocator), + events: Arc::new(NoopEventSource), + }; + // NOT started: the daemon is cold, exactly as right after a restart. + fresh_agent + .set_manager_for_test(OpencodeServeManager::new(deps, ServeConfig::default())) + .await; + let st = FreshOpencodeState::new(fresh_agent); + let placeholder = "freshopencode-restart-race"; + + // The reconnect attach. Either it settles at once, or it parks in the + // daemon's cold start (the defect) — in which case the next steps + // run inside that window. + let st_attach = st.clone(); + let mut attach = + tokio::spawn(async move { st_attach.handle_attach(attach_msg(placeholder)).await }); + tokio::select! { + joined = &mut attach => joined.expect("the attach task joins"), + arrived = health_rx.recv() => { + arrived.expect("the attach parked in the daemon's cold start"); + } + } + + // The reconcile's re-create (same createRequestId) and the user's + // first send, while the daemon is still cold. + st.handle_create(create_msg("restart-race"), None).await; + let st_send = st.clone(); + let send = tokio::spawn(async move { + st_send + .handle_send(send_msg(placeholder, "Materialize after the restart")) + .await + }); + // Open the cold start only once the send has either given up or holds + // its own materialize claim on the placeholder. + let deadline = tokio::time::Instant::now() + Duration::from_secs(15); + loop { + let send_claims = matches!( + registry.observe(PROVIDER, placeholder).state, + freshell_ownership::OwnershipState::Starting { ref operation_id, .. } + if operation_id.starts_with("send-materialize-") + ); + if send.is_finished() || send_claims { + break; + } + assert!( + tokio::time::Instant::now() < deadline, + "the send neither finished nor claimed the placeholder" + ); + tokio::time::sleep(Duration::from_millis(5)).await; + } + latch.add_permits(tokio::sync::Semaphore::MAX_PERMITS / 2); + tokio::time::timeout(Duration::from_secs(15), send) + .await + .expect("the send settles once the daemon is up") + .expect("the send task joins"); + if !attach.is_finished() { + tokio::time::timeout(Duration::from_secs(15), attach) + .await + .expect("the attach settles once the daemon is up") + .expect("the attach task joins"); + } + + let mut frames = Vec::new(); + while let Ok(raw) = rx.try_recv() { + frames.push(serde_json::from_str::(&raw).expect("JSON frame")); + } + assert!( + !frames.iter().any(|f| f["type"] == "freshAgent.event" + && f["sessionId"] == placeholder + && f["event"]["code"] == "SESSION_RESERVED"), + "the first send after the restart must not be refused SESSION_RESERVED: {frames:?}" + ); + assert!( + frames.iter().any(|f| f["type"] == "freshAgent.event" + && f["sessionId"] == placeholder + && f["event"]["code"] == "INVALID_SESSION_ID"), + "the untracked placeholder attach answers lost: {frames:?}" + ); + let session = st + .sessions + .lock() + .await + .get(placeholder) + .cloned() + .expect("the re-created placeholder session is tracked"); + assert_eq!( + session.lock().await.real_session_id.as_deref(), + Some("ses_1"), + "the first send after the restart materializes the durable session" + ); + } + /// Decision-table row: NOT tracked locally + serve genuinely has no record of the id /// (a real 404) -> `lost_session_frame` (`INVALID_SESSION_ID`) is still correct. #[tokio::test] diff --git a/crates/freshell-freshagent/src/session_handoff/tests.rs b/crates/freshell-freshagent/src/session_handoff/tests.rs index d558f0ef8..91d0a3b74 100644 --- a/crates/freshell-freshagent/src/session_handoff/tests.rs +++ b/crates/freshell-freshagent/src/session_handoff/tests.rs @@ -281,7 +281,8 @@ impl FakeOpencodeServeEnv { } assert!( tokio::time::Instant::now() < deadline, - "audit row did not appear within budget" + "audit row did not appear within budget; rows: {:?}", + self.audit_rows() ); tokio::time::sleep(Duration::from_millis(50)).await; } @@ -8046,15 +8047,33 @@ async fn opencode_handoff_stop_waits_for_a_mid_dispatch_rest_drive_under_the_gat axum::Json(json!({ "text": "first REST turn", "timeout": 1 })), )); let rest_response = first.await.expect("drive #1 completed"); + let first_status = rest_response.status(); + let first_body = axum::body::to_bytes(rest_response.into_body(), usize::MAX) + .await + .expect("drive #1 body"); assert_eq!( - rest_response.status(), + first_status, axum::http::StatusCode::OK, - "drive #1 answers approx: {rest_response:?}" + "drive #1 answers approx: {}", + String::from_utf8_lossy(&first_body) + ); + + let is_prompt_row = |r: &Value| { + r["method"] == "POST" + && r["path"] + .as_str() + .is_some_and(|p| p == "/session/ses_1/prompt_async") + }; + let prompt_rows = |rows: &[Value]| rows.iter().filter(|r| is_prompt_row(r)).count(); + assert_eq!( + prompt_rows(&env.audit_rows()), + 1, + "drive #1 issued exactly one prompt POST" ); // Park drive #2 mid-dispatch — AFTER the gate-held re-check, BEFORE // the prompt POST (the R4-2 test seam; never armed in production). - let pause = Arc::new(tokio::sync::Notify::new()); + let pause = Arc::new(crate::RestTurnTestPause::default()); rig.fresh_agent .set_rest_turn_test_pause_for_test(Some(Arc::clone(&pause))); let second = tokio::spawn(crate::send_keys( @@ -8063,14 +8082,18 @@ async fn opencode_handoff_stop_waits_for_a_mid_dispatch_rest_drive_under_the_gat headers(()), axum::Json(json!({ "text": "second REST turn", "timeout": 5 })), )); - // The drive reached the park: its retained-witness settle (R4-3) - // aborted drive #1's daemon-side turn first — that abort row is the - // drive's progress marker up to the park point. + // On its way to the park the drive's retained-witness settle (R4-3) + // aborts drive #1's daemon-side turn first. env.await_audit_row(Duration::from_secs(20), |r| { r["event"] == "abort-received" && r["id"] == json!("ses_1") }) .await; - tokio::time::sleep(Duration::from_millis(100)).await; + // The drive reached the park, holding the dispatch gate (the seam's + // own signal — never a fixed sleep: a release sent before the drive + // parked would be lost and the stop would block until REAP_TIMEOUT). + tokio::time::timeout(Duration::from_secs(20), pause.parked.notified()) + .await + .expect("drive #2 parks after its gate-held re-check, before its prompt POST"); // Handoff opencode -> terminal while the drive is parked BEFORE its // prompt POST: the stop's quiesce blocks on the dispatch gate. @@ -8082,26 +8105,30 @@ async fn opencode_handoff_stop_waits_for_a_mid_dispatch_rest_drive_under_the_gat // THE R4-2 regression: the handoff must NOT complete while the drive // is parked mid-section — pre-fix the quiesce condemned the UNARMED // witness and the handoff committed the terminal owner BEFORE the - // drive's POST (a daemon-side writer started after the reap). - assert!( - tokio::time::timeout(Duration::from_millis(250), &mut completion) - .await - .is_err(), - "the handoff completed while the REST drive was parked before its \ - prompt dispatch — the stop must block on the dispatch gate" + // drive's POST (a daemon-side writer started after the reap). The + // stop's quiesce reaches the gate the parked drive holds and queues + // behind it; until the release, the handoff cannot complete. + tokio::select! { + biased; + done = &mut completion => panic!( + "the handoff completed while the REST drive was parked before its \ + prompt dispatch — the stop must block on the dispatch gate: {done:?}" + ), + () = pause.quiesce_blocked.notified() => {} + () = tokio::time::sleep(Duration::from_secs(20)) => panic!( + "the stop's quiesce never reached the dispatch gate the parked drive holds" + ), + } + assert_eq!( + prompt_rows(&env.audit_rows()), + 1, + "the parked drive has not issued its prompt POST yet" ); // Release the drive: the prompt POST issues UNDER the gate, the gate // opens, and the stop takes+condemns the now-ARMED witness and its // abort confirms the daemon-side turn BEFORE the reap. - pause.notify_waiters(); - env.await_audit_row(Duration::from_secs(20), |r| { - r["method"] == "POST" - && r["path"] - .as_str() - .is_some_and(|p| p == "/session/ses_1/prompt_async") - }) - .await; + pause.release.notify_one(); let result = completion.await.expect("handoff completed"); assert_eq!(result["ok"], json!(true), "handoff to terminal: {result}"); let terminal_id = result["owner"]["terminalId"].as_str().unwrap().to_string(); @@ -8112,20 +8139,19 @@ async fn opencode_handoff_stop_waits_for_a_mid_dispatch_rest_drive_under_the_gat } other => panic!("expected Live terminal owner, got {other:?}"), } - // Ordering proof: the drive's prompt POST row precedes a SECOND - // abort row for ses_1 — the quiesce's confirmed abort of the armed - // witness. (Pre-fix the quiesce aborted nothing: no armed witness - // existed and the drive's POST armed an orphan AFTER the handoff.) + // Ordering proof: drive #2's prompt POST row (the SECOND prompt row — + // drive #1 issued the first) precedes a further abort row for ses_1 — + // the quiesce's confirmed abort of the armed witness. (Pre-fix the + // quiesce aborted nothing: no armed witness existed and the drive's + // POST armed an orphan AFTER the handoff.) let rows = env.audit_rows(); let prompt_idx = rows .iter() - .position(|r| { - r["method"] == "POST" - && r["path"] - .as_str() - .is_some_and(|p| p == "/session/ses_1/prompt_async") - }) - .expect("the drive's prompt row"); + .enumerate() + .filter(|(_, r)| is_prompt_row(r)) + .map(|(i, _)| i) + .nth(1) + .expect("drive #2's prompt row"); let abort_after_prompt = rows[prompt_idx..] .iter() .any(|r| r["event"] == "abort-received" && r["id"] == json!("ses_1")); diff --git a/crates/freshell-freshagent/src/session_lease.rs b/crates/freshell-freshagent/src/session_lease.rs index ab1d24b2b..fbe8a4c5e 100644 --- a/crates/freshell-freshagent/src/session_lease.rs +++ b/crates/freshell-freshagent/src/session_lease.rs @@ -598,15 +598,40 @@ pub(crate) fn proc_starttime(pid: i32) -> Option { proc_starttime_observation(pid).ok().flatten() } +/// Whether a failed `/proc//…` access proves the process is gone: the +/// entry is absent (`NotFound`), or the task exited and was reaped between the +/// open and the read — procfs then answers `ESRCH` on the already-open +/// descriptor. Both mean the observed incarnation no longer exists; neither is +/// "unavailable" evidence. (A sweep reads exactly the processes it is killing, +/// so a member reaped mid-read is the expected race, not an unknown writer.) +#[cfg(target_os = "linux")] +fn proc_entry_gone(error: &std::io::Error) -> bool { + error.kind() == std::io::ErrorKind::NotFound || error.raw_os_error() == Some(libc::ESRCH) +} + /// Read a process start time while preserving unavailable observations as a /// distinct result from a process that is gone. #[cfg(target_os = "linux")] fn proc_starttime_observation(pid: i32) -> Result, ()> { - let stat = match std::fs::read_to_string(format!("/proc/{pid}/stat")) { - Ok(stat) => stat, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + match std::fs::File::open(format!("/proc/{pid}/stat")) { + Ok(stat) => proc_starttime_from_open_stat(stat), + Err(error) if proc_entry_gone(&error) => Ok(None), + Err(_) => Err(()), + } +} + +/// [`proc_starttime_observation`] past the open: the read of an +/// already-open `/proc//stat` (split out so the open→read window — +/// the process reaped in between — is testable deterministically). +#[cfg(target_os = "linux")] +fn proc_starttime_from_open_stat(mut stat_file: std::fs::File) -> Result, ()> { + use std::io::Read; + let mut stat = String::new(); + match stat_file.read_to_string(&mut stat) { + Ok(_) => {} + Err(error) if proc_entry_gone(&error) => return Ok(None), Err(_) => return Err(()), - }; + } // comm (field 2) may contain spaces/parens: split at the LAST ')' — the remainder // starts at field 3 (state), so starttime (field 22) is index 19 there. let rest = stat.rsplit(')').next().ok_or(())?; @@ -698,7 +723,7 @@ fn scan_tagged_pids( let status_path = std::path::Path::new("/proc").join(name).join("status"); let status = match std::fs::read_to_string(status_path) { Ok(status) => status, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) if proc_entry_gone(&error) => continue, Err(_) => { if process_is_known_owned_process(pid, owned_root_pid) { complete = false; @@ -724,7 +749,7 @@ fn scan_tagged_pids( Ok(bytes) => bytes, // A process disappearing between the directory and environ read // is an expected race, not unknown surviving-writer evidence. - Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) if proc_entry_gone(&error) => continue, Err(_) => { // Same-user processes can still make environ unreadable (for // example a non-dumpable user service). An unrelated process @@ -767,7 +792,7 @@ fn process_is_descendant_of(mut pid: i32, root_pid: i32) -> Result { } let status = match std::fs::read_to_string(format!("/proc/{pid}/status")) { Ok(status) => status, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(error) if proc_entry_gone(&error) => return Ok(false), Err(_) => return Err(()), }; let parent = status @@ -1082,6 +1107,41 @@ mod tests { const TTL: u64 = FRESH_AGENT_SESSION_LEASE_TTL_MS; + /// A process reaped between the open and the read of its `/proc` files + /// reads as GONE — procfs answers ESRCH on the already-open descriptor — + /// never as unavailable evidence. The expired-lease kill sweep reads + /// exactly the processes it is killing; an "unavailable" verdict there + /// left the kill unconfirmed and the lease held closed, so the contending + /// create was refused SESSION_RESERVED although the tree was dead. + #[cfg(target_os = "linux")] + #[test] + fn a_process_reaped_between_proc_open_and_read_reads_as_gone() { + use std::io::Read; + let mut child = std::process::Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn a child to observe"); + let pid = child.id(); + let stat = std::fs::File::open(format!("/proc/{pid}/stat")).expect("open the live stat"); + let mut status = + std::fs::File::open(format!("/proc/{pid}/status")).expect("open the live status"); + child.kill().expect("kill the child"); + child.wait().expect("reap the child"); + + assert_eq!( + proc_starttime_from_open_stat(stat), + Ok(None), + "a start-time read racing the reap answers gone, not unavailable" + ); + let error = status + .read_to_string(&mut String::new()) + .expect_err("procfs refuses a reaped task's open descriptor"); + assert!( + proc_entry_gone(&error), + "the scan's status/environ reads classify the reap race as gone: {error}" + ); + } + #[test] fn first_claim_acquires_second_is_held() { let leases = FreshAgentSessionLeases::new(); diff --git a/crates/freshell-sessions/src/session_watcher_tests.rs b/crates/freshell-sessions/src/session_watcher_tests.rs index 04e5775d7..7a89f9143 100644 --- a/crates/freshell-sessions/src/session_watcher_tests.rs +++ b/crates/freshell-sessions/src/session_watcher_tests.rs @@ -3197,19 +3197,32 @@ async fn metadata_late_subagent_named_mkdir_arms_via_deferred_recheck_without_a_ .await .expect("the deferred probe recheck arms the drift root within seconds of its metadata"); writer.join().unwrap(); + // And the row is promptly visible — the watch-then-scan scoped mark + // drove a scoped re-parse, end to end. The arm is observable BEFORE + // the row: the flush releases the book lock (the arm) before its + // scoped mark drains into `mark_dirty`, and the mark's re-parse then + // publishes from a background refresh (a snapshot read while that + // sweep holds the refresh lock serves the previous generation). So + // the row is awaited under the same seconds-not-reconcile budget as + // the arm, never read once right after it. + let visible = |snap: &[crate::directory_index::IndexedSession]| { + snap.iter() + .any(|s| s.provider == "amplifier" && s.session_id == "late-drift-root") + }; + tokio::time::timeout(Duration::from_secs(5), async { + while !visible(&index.snapshot().await) { + tokio::time::sleep(Duration::from_millis(20)).await; + } + }) + .await + .expect("the drift root's scoped mark made the row visible within seconds"); + // Checked AFTER the row is visible: the row arrived without any full + // provider discover — scoped marks only, for the mkdir AND its row. assert_eq!( counted.discover_calls.load(Ordering::SeqCst), baseline_discovers, "the mkdir must never pay a full provider discover (scoped marks only)" ); - // And the row is promptly visible — the watch-then-scan scoped mark - // drove a scoped re-parse, end to end. - let snap = index.snapshot().await; - assert!( - snap.iter() - .any(|s| s.provider == "amplifier" && s.session_id == "late-drift-root"), - "the drift root's scoped mark made the row visible without a discover" - ); watcher.stop(); let _ = tokio::time::timeout(Duration::from_secs(1), handle).await; std::fs::remove_dir_all(&home).ok(); diff --git a/crates/freshell-ws/tests/freshagent_session_lease.rs b/crates/freshell-ws/tests/freshagent_session_lease.rs index b7cfc4b72..1e9360727 100644 --- a/crates/freshell-ws/tests/freshagent_session_lease.rs +++ b/crates/freshell-ws/tests/freshagent_session_lease.rs @@ -143,10 +143,19 @@ rl.on('line', (line) => { } if (process.env.FAKE_SIDECAR_SPAWN_GRANDCHILD === '1') { // Detached decoy grandchild inheriting env (so /proc//environ carries the - // ownership tag the kill sweep scans for). - const gc = spawn('sleep', ['300'], { detached: true, stdio: 'ignore' }) + // ownership tag the kill sweep scans for). It IGNORES SIGTERM (an ignored signal + // stays ignored across exec), so the claude lane's own one-shot SIGTERM reap of + // its tagged tree cannot end it: only the expired-lease sweep's SIGKILL + // escalation can. It is logged only once the shell has armed the ignore. + const gc = spawn('sh', ['-c', 'trap "" TERM; echo armed; exec sleep 300 >/dev/null'], { + detached: true, + stdio: ['ignore', 'pipe', 'ignore'], + }) + gc.stdout.once('data', () => { + gc.stdout.destroy() + logReq({ type: 'grandchild', grandchildPid: gc.pid }) + }) gc.unref() - logReq({ type: 'grandchild', grandchildPid: gc.pid }) } counter += 1 const sessionId = `fake-claude-session-${process.pid}-${counter}` @@ -205,6 +214,50 @@ impl FakeLeaseSidecarEnv { .collect() } + /// SIGKILL every decoy grandchild this env's sidecars logged that is still + /// alive. The decoy is detached on purpose (it must outlive a raw + /// single-pid kill of its sidecar), so nothing else stops it when a test + /// ends before the ownership sweep ran — or when a later sidecar spawned + /// one of its own. Each pid is pinned through a pidfd and must carry this + /// env's unique request-log path in `/proc//environ`, so a recycled + /// pid is never signalled. + #[cfg(target_os = "linux")] + fn kill_logged_decoys(&self) { + let tag = format!( + "FAKE_SIDECAR_REQUEST_LOG={}", + self.dir.join("requests.jsonl").display() + ); + for row in self.request_log_rows() { + if row["msg"]["type"] != "grandchild" { + continue; + } + let Some(pid) = row["msg"]["grandchildPid"].as_i64() else { + continue; + }; + // SAFETY: plain syscalls on an owned descriptor; the pidfd is + // closed below on every path. + let pidfd = unsafe { libc::syscall(libc::SYS_pidfd_open, pid as libc::pid_t, 0) }; + if pidfd < 0 { + continue; // already gone + } + let pidfd = pidfd as libc::c_int; + let ours = std::fs::read(format!("/proc/{pid}/environ")) + .is_ok_and(|env| env.split(|b| *b == 0).any(|kv| kv == tag.as_bytes())); + if ours { + unsafe { + libc::syscall( + libc::SYS_pidfd_send_signal, + pidfd, + libc::SIGKILL, + std::ptr::null::(), + 0, + ); + } + } + unsafe { libc::close(pidfd) }; + } + } + /// Poll the request log until `pred` matches a row (or the budget expires). async fn await_log_row(&self, budget: Duration, pred: impl Fn(&Value) -> bool) -> Value { let deadline = tokio::time::Instant::now() + budget; @@ -223,6 +276,8 @@ impl FakeLeaseSidecarEnv { impl Drop for FakeLeaseSidecarEnv { fn drop(&mut self) { + #[cfg(target_os = "linux")] + self.kill_logged_decoys(); for var in [ "FRESHELL_CLAUDE_SIDECAR", "FRESHELL_CLAUDE_NODE", @@ -662,7 +717,9 @@ async fn codex_loser_create_resume_adopts_and_never_clobbers_the_winner() { /// An EXPIRED lease must be TREE-killed before release: the decoy grandchild (which a /// raw single-pid SIGKILL of the sidecar would orphan) must be dead BEFORE the second /// client's create converges -- i.e. the ownership sweep ran and confirmed empty prior -/// to force-release. +/// to force-release. The decoy ignores SIGTERM, so the claude lane's own SIGTERM reap +/// (run when A's sidecar dies mid-create) cannot stand in for the sweep: only the +/// sweep's SIGKILL escalation ends it. #[cfg(target_os = "linux")] #[tokio::test] async fn expired_lease_kill_sweeps_the_sidecar_tree_before_release() { @@ -691,14 +748,35 @@ async fn expired_lease_kill_sweeps_the_sidecar_tree_before_release() { std::path::Path::new(&format!("/proc/{gpid}")).exists(), "decoy grandchild must be alive before expiry" ); + // A's sidecar captured its environment at spawn. Client B's replacement sidecar + // must be a plain one: no decoy of its own (nothing would ever reap it — the + // detached `sleep 300` outlived every run) and no artificial create delay on the + // path whose convergence this test times. + std::env::remove_var("FAKE_SIDECAR_SPAWN_GRANDCHILD"); + std::env::remove_var("FAKE_SIDECAR_CREATE_DELAY_MS"); // Let the TTL expire, then client B contends -> ExpiredNeedsKill -> tree kill. tokio::time::sleep(Duration::from_millis(300)).await; send_json(&mut ws_b, &claude_create_resume("req-ttl-b", durable)).await; - // The DECOY GRANDCHILD dies (ownership sweep / tree-kill) BEFORE client B's created - // lands. NOTE: a dead-but-unreaped ZOMBIE keeps its /proc entry (it reparents to the - // container's pid 1, which never reaps it), so "dead" is read from the stat STATE — - // gone, Z, or X — not from /proc existence. + // B's create settles (force-release + re-claim ONCE). A refusal (e.g. an + // unconfirmed tree kill holding the lease closed) answers create.failed — fail + // on it at once with its code instead of timing out on a created that never comes. + let outcome = await_frame(&mut ws_b, Duration::from_secs(20), |v| { + (v["type"] == "freshAgent.created" || v["type"] == "freshAgent.create.failed") + && v["requestId"] == "req-ttl-b" + }) + .await; + assert_eq!( + outcome["type"], "freshAgent.created", + "client B's create must converge after the expired holder's tree kill: {outcome}" + ); + assert!(outcome["sessionId"].is_string()); + + // The DECOY GRANDCHILD was dead (ownership sweep / tree-kill) BEFORE client B's + // created landed: the release follows the sweep's confirmation, so it is already + // dead now — no polling. NOTE: a dead-but-unreaped ZOMBIE keeps its /proc entry (it + // reparents to the container's pid 1, which never reaps it), so "dead" is read from + // the stat STATE — gone, Z, or X — not from /proc existence. let grandchild_dead = |pid: i64| -> bool { let Ok(stat) = std::fs::read_to_string(format!("/proc/{pid}/stat")) else { return true; // gone entirely @@ -710,24 +788,10 @@ async fn expired_lease_kill_sweeps_the_sidecar_tree_before_release() { Some("Z") | Some("X") | None ) }; - let deadline = tokio::time::Instant::now() + Duration::from_secs(10); - loop { - if grandchild_dead(gpid) { - break; - } - assert!( - tokio::time::Instant::now() < deadline, - "grandchild survived: a raw single-pid kill orphans the writer (V6)" - ); - tokio::time::sleep(Duration::from_millis(50)).await; - } - - // ...and only then does B's own resume converge (force-release + re-claim ONCE). - let created = await_frame(&mut ws_b, Duration::from_secs(20), |v| { - v["type"] == "freshAgent.created" && v["requestId"] == "req-ttl-b" - }) - .await; - assert!(created["sessionId"].is_string()); + assert!( + grandchild_dead(gpid), + "grandchild survived B's convergence: a raw single-pid kill orphans the writer (V6)" + ); } // ── Task 13: lease at the attach-resume seams ──────────────────────────────────────── diff --git a/src/components/TerminalView.tsx b/src/components/TerminalView.tsx index 872a57d48..a6eebd23a 100644 --- a/src/components/TerminalView.tsx +++ b/src/components/TerminalView.tsx @@ -937,6 +937,20 @@ function TerminalView({ tabId, paneId, paneContent, hidden, focusEpoch = 0 }: Te const freshAgentOwnerDivergence = derivePaneOwnerDivergence(terminalRuntimeOwner, 'terminal') const freshAgentOwnerDivergenceRef = useRef(freshAgentOwnerDivergence) freshAgentOwnerDivergenceRef.current = freshAgentOwnerDivergence + // The terminal id whose attach the fresh-agent divergence gate declined + // (see attachTerminal). A pane switched onto its handoff's NEW terminal by + // the handoff's HTTP answer can mount before the committed owner frame + // folds — the answer and the WS frame race — so the gate declines while + // the record still names the fresh-agent runtime. Nothing else re-fires + // the lifecycle effect, so the commit that names THIS terminal re-drives + // the attach (the effect below). + const ownerDivergenceDeclinedAttachRef = useRef(null) + const committedOwnerTerminalId = terminalRuntimeOwner + && !terminalRuntimeOwner.fenced + && terminalRuntimeOwner.transition !== 'handoff-started' + && terminalRuntimeOwner.ownerKind === 'terminal' + ? terminalRuntimeOwner.terminalId + : undefined // b8ke ext r11 F2: the SAME-KIND TERMINAL convergence — a handoff // committed a NEW terminal owner for this pane's canonical session while @@ -1019,6 +1033,28 @@ function TerminalView({ tabId, paneId, paneContent, hidden, focusEpoch = 0 }: Te terminalOwnerConvergenceGeneration, ]) + // The declined-attach re-drive (see ownerDivergenceDeclinedAttachRef): + // once the committed owner record names THIS pane's terminal, an attach + // the fresh-agent divergence gate declined re-fires through the + // reconcileEpoch bump — the lifecycle effect's re-fire signal. Scoped to + // the pane's own terminal on purpose: a vacant record or another + // terminal never re-attaches the declined handle (a dead handle draws + // INVALID_TERMINAL_ID and the reconnect recovery would relaunch the + // stopped session — the delta F1 hazard). + const paneTerminalId = terminalContent?.terminalId + useEffect(() => { + if (!isTerminal) return + const declined = ownerDivergenceDeclinedAttachRef.current + if (!declined || declined !== paneTerminalId) return + if (committedOwnerTerminalId !== declined) return + ownerDivergenceDeclinedAttachRef.current = null + log.info('re-driving the attach the fresh-agent owner gate declined: the committed owner names this terminal', { + paneId, + terminalId: declined, + }) + dispatch(bumpPaneReconcileEpoch({ tabId, paneId })) + }, [isTerminal, paneId, tabId, dispatch, paneTerminalId, committedOwnerTerminalId]) + // Register live terminal text reader for Stream Deck previews (classic tile style) useTerminalTextRegistration(terminalContent?.terminalId, termRef) @@ -3495,6 +3531,7 @@ function TerminalView({ tabId, paneId, paneContent, hidden, focusEpoch = 0 }: Te paneId: paneIdRef.current, intent, }) + ownerDivergenceDeclinedAttachRef.current = tid return } // Never attach a terminal the layouts no longer reference: the layout-diff diff --git a/test/e2e-browser/helpers/unified-agent-names.ts b/test/e2e-browser/helpers/unified-agent-names.ts index ffce65c86..d1e2fcde1 100644 --- a/test/e2e-browser/helpers/unified-agent-names.ts +++ b/test/e2e-browser/helpers/unified-agent-names.ts @@ -1216,6 +1216,9 @@ export async function renameThroughTabEdit(page: Page, tabId: string, name: stri const tab = page.locator(`[data-context="tab"][data-tab-id="${tabId}"]`) await tab.dblclick({ timeout: RENAME_ACTION_TIMEOUT }) const input = tab.locator('input') + // A failed session-owned tab rename renders its error as a role=alert + // sibling of the tab (TabBar) and keeps the editor open. + const renameError = tab.locator('xpath=following-sibling::*[@role="alert"]') await expect(input).toBeVisible({ timeout: 5_000 }) await input.fill(name, { timeout: RENAME_ACTION_TIMEOUT }) await input.press('Enter', { timeout: RENAME_ACTION_TIMEOUT }) @@ -1223,12 +1226,23 @@ export async function renameThroughTabEdit(page: Page, tabId: string, name: stri // the editor opened) keeps the editor open with the conflict folded and // a REFRESHED capture, and a transient 429 drains the shared bucket: // the user's resubmit from the still-open editor is the documented - // path (the shared pane-editor discipline). + // path (the shared pane-editor discipline). An open editor with NO shown + // error is an IN-FLIGHT commit (the session-owned rename keeps the editor + // open while its request runs): resubmitting then races the healthy + // request — it can close the editor between the visibility check and the + // resubmit's Enter, leaving the press waiting on a detached input until + // its 30s timeout. Resubmit ONLY on the shown error state, exactly like + // commitPaneRenameEditor. for (const backoff of RENAME_RESUBMIT_BACKOFFS) { await page.waitForTimeout(backoff) if (!(await input.isVisible().catch(() => false))) break - await input.fill(name, { timeout: RENAME_ACTION_TIMEOUT }) - await input.press('Enter', { timeout: RENAME_ACTION_TIMEOUT }) + if (!(await renameError.isVisible().catch(() => false))) continue + try { + await input.fill(name, { timeout: RENAME_ACTION_TIMEOUT }) + await input.press('Enter', { timeout: RENAME_ACTION_TIMEOUT }) + } catch (error) { + console.error(`[unified-agent-names] tab rename resubmit failed: ${String(error).slice(0, 200)}`) + } } await expect(input).toBeHidden({ timeout: 10_000 }) await expect(tab).toContainText(name, { timeout: 10_000 }) diff --git a/test/e2e-browser/specs/fresh-agent-rollback-rust.spec.ts b/test/e2e-browser/specs/fresh-agent-rollback-rust.spec.ts index c6d2133ce..88ae27c58 100644 --- a/test/e2e-browser/specs/fresh-agent-rollback-rust.spec.ts +++ b/test/e2e-browser/specs/fresh-agent-rollback-rust.spec.ts @@ -854,6 +854,17 @@ test.describe('fresh-agent /undo + /redo conversation rollback (rust, kata 1wxv) expect(reverts[0].params?.beforeTurnId).toBeTruthy() expect(ops.map((o) => o.method)).not.toContain('thread/rollback') // deprecated path never used + // The undo's CLIENT-side completion: this pane's rolledBack ack refills + // the composer with the first removed prompt (decision 4) and posts the + // refill notice. The REST snapshot above can show the reverted thread + // before the pane processes that ack; a /redo typed in that window is + // overwritten by the late refill, whose notice then REPLACES the redo + // notice (the observed "Redo is not available" timeouts). Wait for the + // refill — the claude undo test's same gate — before typing. + await expect + .poll(() => composerLocator(page).inputValue(), { timeout: 15_000 }) + .toBe('codex turn one') + // Typed /redo never reaches the wire: the composer's reserved-name // interception answers with the pinned codex undo-only copy. await typeSlash(page, '/redo') diff --git a/test/e2e-browser/specs/layout-sync-authoritative.spec.ts b/test/e2e-browser/specs/layout-sync-authoritative.spec.ts index 9022bf40b..1ce7c99b8 100644 --- a/test/e2e-browser/specs/layout-sync-authoritative.spec.ts +++ b/test/e2e-browser/specs/layout-sync-authoritative.spec.ts @@ -18,6 +18,9 @@ import { test, expect } from '../helpers/fixtures.js' import type { E2eServerInfo } from '../helpers/server-fixture-support.js' const LAYOUT_SYNC_DEBOUNCE_MS = 1000 +/** @dnd-kit/core's AbstractPointerSensor removes its post-drag document + * click blocker on `setTimeout(..., 50)` after the drag ends. */ +const DND_KIT_POST_DRAG_CLICK_BLOCK_MS = 50 type PaneNode = { type: 'leaf' | 'split' @@ -197,6 +200,18 @@ test.describe('AUTO-01 — ui.layout.sync is the authoritative layout', () => { await expect .poll(async () => (await harness.getState()).tabs.tabs[0].title, { timeout: 10_000 }) .toBe('Beta') + // The reorder is in the store at pointerup, but dnd-kit's pointer sensor + // keeps a capture-phase click blocker on the document until a 50ms timer + // after the drag ends (AbstractPointerSensor.detach — it swallows the + // click the drop itself would fire). A click sent inside that window is + // eaten and the tab never activates (the observed activeTabId timeouts: + // Chrome runs input ahead of a starved timer under load). A page timer of + // at least that delay, scheduled after the drop, fires after dnd-kit's — + // the blocker is gone once it resolves. + await page.evaluate( + (ms) => new Promise((resolve) => setTimeout(resolve, ms)), + DND_KIT_POST_DRAG_CLICK_BLOCK_MS, + ) // ── select the "Alpha" tab by clicking it ── await page.locator('[data-context="tab"]').filter({ hasText: 'Alpha' }).first().click() diff --git a/test/unit/client/components/TerminalView.lifecycle.test.tsx b/test/unit/client/components/TerminalView.lifecycle.test.tsx index 70eca362b..1e57bd8c3 100644 --- a/test/unit/client/components/TerminalView.lifecycle.test.tsx +++ b/test/unit/client/components/TerminalView.lifecycle.test.tsx @@ -4365,6 +4365,92 @@ describe('TerminalView lifecycle updates', () => { }) }) + it('a handoff pane mounted on its new terminal before the committed owner frame attaches once the frame names that terminal', async () => { + // Fresh Agent -> CLI on THIS device: the handoff's HTTP answer switches + // the pane onto the new terminal while the canonical record still names + // the fresh-agent runtime (the answer and the WS owner frame race). The + // attach gate declines while the record says fresh-agent; the committed + // frame naming THIS pane's terminal must re-drive the attach. Pre-fix + // nothing re-fired the lifecycle effect and the pane never attached (no + // output, the server terminal reported hasClients:false) until a reload. + const { store } = setupTypedPane({ + content: { status: 'running', terminalId: 't-handoff' }, + seed: (seededStore) => { + act(() => { + seededStore.dispatch(applyRuntimeOwner(runtimeOwnerFrame({ + generation: 1, + ownerKind: 'fresh-agent', + terminalId: undefined, + }))) + }) + }, + }) + + await waitFor(() => { + expect(messageHandler).not.toBeNull() + }) + const attachesTo = (terminalId: string) => sentMessages().filter((msg) => ( + msg?.type === 'terminal.attach' && msg.terminalId === terminalId + )) + // The gate declined: no attach while the record names the fresh agent. + expect(attachesTo('t-handoff')).toHaveLength(0) + + act(() => { + store.dispatch(applyRuntimeOwner(runtimeOwnerFrame({ + generation: 2, + ownerKind: 'terminal', + terminalId: 't-handoff', + }))) + }) + + await waitFor(() => { + expect(attachesTo('t-handoff')).toHaveLength(1) + }) + }) + + it('a declined attach is never re-driven when the owner frame does not name the pane\'s own terminal', async () => { + // The re-drive is scoped to a commit naming THIS terminal: a vacant + // record (the session stopped) or another terminal must not re-attach + // the declined handle (a dead handle draws INVALID_TERMINAL_ID and the + // reconnect recovery would relaunch the stopped session). + const { store } = setupTypedPane({ + content: { status: 'running', terminalId: 't-declined' }, + seed: (seededStore) => { + act(() => { + seededStore.dispatch(applyRuntimeOwner(runtimeOwnerFrame({ + generation: 1, + ownerKind: 'fresh-agent', + terminalId: undefined, + }))) + }) + }, + }) + + await waitFor(() => { + expect(messageHandler).not.toBeNull() + }) + act(() => { + store.dispatch(applyRuntimeOwner(runtimeOwnerFrame({ + generation: 2, + ownerKind: 'vacant', + terminalId: undefined, + transition: 'stopped', + }))) + }) + act(() => { + store.dispatch(applyRuntimeOwner(runtimeOwnerFrame({ + generation: 3, + ownerKind: 'terminal', + terminalId: 't-someone-else', + }))) + }) + + // act() flushed every effect the two folds triggered. + expect(sentMessages().some((msg) => ( + msg?.type === 'terminal.attach' && msg.terminalId === 't-declined' + ))).toBe(false) + }) + it('b8ke ext r11 F2: a still-RUNNING pane is never stolen off its own terminal by a committed same-kind owner', async () => { const { store } = setupTypedPane({ content: {