diff --git a/CHANGELOG.md b/CHANGELOG.md index 200b8ef..c3d6112 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.9.19] - 2026-05-26 + +### Fixed + +- **Codex stale resume sessions recover cleanly.** If Codex reports + `Session not found for thread_id` for a persisted thread id, CoreRoom now + retries that turn once with a fresh Codex thread instead of rendering the + raw adapter error as an `@host` reply. +- **Root host turns no longer double-label `@host`.** The transcript now + suppresses the root host `TurnDispatched` handoff banner while preserving + real cross-role handoff banners. +- **`@user` is visibly styled in scrollback.** The user tag now uses the + prompt green instead of off-white, so user-authored transcript lines are + visually distinct from body text and role output. + ## [0.9.18] - 2026-05-26 ### Fixed @@ -1700,7 +1715,8 @@ API stability, not feature completeness. - **No timestamps in CREP events.** `cr cost --since` honors the log file's mtime only; per-event timestamps land in v0.2. -[Unreleased]: https://github.com/spytensor/CoreRoom/compare/v0.9.18...HEAD +[Unreleased]: https://github.com/spytensor/CoreRoom/compare/v0.9.19...HEAD +[0.9.19]: https://github.com/spytensor/CoreRoom/compare/v0.9.18...v0.9.19 [0.9.18]: https://github.com/spytensor/CoreRoom/compare/v0.9.17...v0.9.18 [0.9.17]: https://github.com/spytensor/CoreRoom/compare/v0.9.16...v0.9.17 [0.9.16]: https://github.com/spytensor/CoreRoom/compare/v0.9.15...v0.9.16 diff --git a/Cargo.lock b/Cargo.lock index 02fa4ed..0831e75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -286,7 +286,7 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" [[package]] name = "coreroom" -version = "0.9.18" +version = "0.9.19" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index 80d6ebe..cbde5a5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "coreroom" -version = "0.9.18" +version = "0.9.19" edition = "2021" rust-version = "1.88" authors = ["Charlie Zhu "] diff --git a/README.md b/README.md index 2602957..9b1df1c 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,7 @@ Disable that with `COREROOM_NO_UPDATE_CHECK=1` or Don't have npm? Direct binary install. ```bash -TAG=v0.9.18 +TAG=v0.9.19 ARCH=$(uname -m); case "$ARCH" in arm64|aarch64) ARCH=aarch64 ;; *) ARCH=x86_64 ;; esac OS=$(uname -s | tr '[:upper:]' '[:lower:]') curl -fsSL "https://github.com/spytensor/CoreRoom/releases/download/${TAG}/cr-${TAG}-${OS}-${ARCH}.tar.gz" \ diff --git a/data/splash_content.toml b/data/splash_content.toml index a402dfe..e8c50e2 100644 --- a/data/splash_content.toml +++ b/data/splash_content.toml @@ -15,6 +15,14 @@ items = [ "/journal captures today's lessons-learned", ] +[[whats_new]] +version = "0.9.19" +items = [ + "stale Codex resume sessions retry fresh instead of printing Session not found", + "@user now has a visible identity color in scrollback", + "root host turns no longer double-print @host handoff banners", +] + [[whats_new]] version = "0.9.18" items = [ diff --git a/npm/package.json b/npm/package.json index e202f7a..c5d0585 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "@spytensor/coreroom", - "version": "0.9.18", + "version": "0.9.19", "description": "CoreRoom is the Engineering Control Room for AI Agents: host-led, GitHub-gated AI-assisted software engineering control.", "keywords": [ "cli", diff --git a/src/adapter/codex.rs b/src/adapter/codex.rs index cecc653..1a361ae 100644 --- a/src/adapter/codex.rs +++ b/src/adapter/codex.rs @@ -1272,7 +1272,7 @@ async fn write_loop( let approval_policy = codex_approval_policy(permission_mode); let sandbox = codex_sandbox(permission_mode); let mut thread_id = initial_thread_id.filter(|id| !id.trim().is_empty()); - while let Some(msg) = rx.recv().await { + 'prompt_loop: while let Some(msg) = rx.recv().await { if runtime.base.stopping.load(Ordering::SeqCst) { break; } @@ -1290,150 +1290,225 @@ async fn write_loop( let turn_id = prompt.turn_id.clone(); let thread_id_for_events = prompt.thread_id.clone(); let prompt = degrade_image_refs_for_codex(&prompt.text); - let params = codex_tool_call_params( - &prompt, - &priors_text, - approval_policy, - sandbox, - thread_id.as_deref(), - ); - - // Allocate the tools/call request id up front and publish it - // into the cancel-tracker before sending. The drainer reads - // this lock to know exactly which id to target with - // `notifications/cancelled` — no inference from `pending`, - // so initialize ids and stale-during-cleanup ids cannot win. - let request_id = client.alloc_id().await; - *current_tools_call.lock().await = Some(request_id); - let (outcome, partial_text, event_thread_id) = client - .request_with_id_capture( - "tools/call", - params, - request_id, - Some((turn_id.clone(), thread_id_for_events.clone())), - ) - .await; - *current_tools_call.lock().await = None; - - // Take the halted-id marker if it matches THIS request. A - // stale marker from a previous turn — or a marker for some - // future turn — stays in place; we only consume what we - // know belongs to us. This is the fix for the previous - // AtomicBool design that could leak a halt signal into the - // next turn's legitimate error. - let halted_match = { - let mut guard = halted_request_id.lock().await; - if *guard == Some(request_id) { - *guard = None; - true - } else { - false - } - }; + let mut retried_stale_session = false; + loop { + let attempted_thread_id = thread_id.clone(); + let params = codex_tool_call_params( + &prompt, + &priors_text, + approval_policy, + sandbox, + attempted_thread_id.as_deref(), + ); - match outcome { - Ok(result) => { - if runtime.base.stopping.load(Ordering::SeqCst) { - break; + // Allocate the tools/call request id up front and publish it + // into the cancel-tracker before sending. The drainer reads + // this lock to know exactly which id to target with + // `notifications/cancelled` — no inference from `pending`, + // so initialize ids and stale-during-cleanup ids cannot win. + let request_id = client.alloc_id().await; + *current_tools_call.lock().await = Some(request_id); + let (outcome, partial_text, event_thread_id) = client + .request_with_id_capture( + "tools/call", + params, + request_id, + Some((turn_id.clone(), thread_id_for_events.clone())), + ) + .await; + *current_tools_call.lock().await = None; + + // Take the halted-id marker if it matches THIS request. A + // stale marker from a previous turn — or a marker for some + // future turn — stays in place; we only consume what we + // know belongs to us. This is the fix for the previous + // AtomicBool design that could leak a halt signal into the + // next turn's legitimate error. + let halted_match = { + let mut guard = halted_request_id.lock().await; + if *guard == Some(request_id) { + *guard = None; + true + } else { + false } - if halted_match { - // Codex acked the cancel after the model already - // produced a final result — rare but possible. - // Honour the user's halt: emit TurnInterrupted - // and discard the late result so the REPL drain - // sees the boundary. - let _ = runtime - .base - .events - .send(turn_interrupted_event( - &runtime.base.role, + }; + + match outcome { + Ok(result) => { + if runtime.base.stopping.load(Ordering::SeqCst) { + break 'prompt_loop; + } + if halted_match { + // Codex acked the cancel after the model already + // produced a final result — rare but possible. + // Honour the user's halt: emit TurnInterrupted + // and discard the late result so the REPL drain + // sees the boundary. + emit_turn_interrupted( + &runtime, &partial_text, &turn_id, &thread_id_for_events, - )) + ) .await; - continue; - } - if let Some(next_thread_id) = - extract_thread_id_from_tool_result(&result).or(event_thread_id) - { - if thread_id.as_deref() != Some(next_thread_id.as_str()) { - thread_id = Some(next_thread_id.clone()); - let _ = runtime - .base - .events - .send(CrepEvent::RoleSessionUpdated { - role: runtime.base.role.clone(), - priors_hash: String::new(), - session_id: next_thread_id, - }) - .await; + break; + } + let text = extract_text_from_tool_result(&result); + if attempted_thread_id.is_some() + && !retried_stale_session + && is_stale_codex_session_text(&text) + { + warn!( + role = %runtime.base.role, + "codex resume thread id is stale; retrying turn with a fresh thread" + ); + retried_stale_session = true; + thread_id = None; + continue; + } + if let Some(next_thread_id) = + extract_thread_id_from_tool_result(&result).or(event_thread_id) + { + if thread_id.as_deref() != Some(next_thread_id.as_str()) { + thread_id = Some(next_thread_id.clone()); + let _ = runtime + .base + .events + .send(CrepEvent::RoleSessionUpdated { + role: runtime.base.role.clone(), + priors_hash: String::new(), + session_id: next_thread_id, + }) + .await; + } + } + for event in crate::adapter::role_spoke_events_from_text_with_ids( + &runtime.base.role, + &text, + 0.0, + 0, + &turn_id, + &thread_id_for_events, + ) { + let _ = runtime.base.events.send(event).await; } - } - let text = extract_text_from_tool_result(&result); - for event in crate::adapter::role_spoke_events_from_text_with_ids( - &runtime.base.role, - &text, - 0.0, - 0, - &turn_id, - &thread_id_for_events, - ) { - let _ = runtime.base.events.send(event).await; - } - } - Err(error) => { - warn!(role = %runtime.base.role, %error, "codex tools/call failed"); - if runtime.base.stopping.load(Ordering::SeqCst) { - break; - } - if matches!(&error, RpcError::Timeout) { - runtime.base.stopping.store(true, Ordering::SeqCst); - let _ = runtime.internal_stop.try_send(StopReason::TimedOut); break; } - // If the user halted *this* turn, the RPC error is - // expected (codex closed the pending request after - // our `notifications/cancelled` reached it). Emit - // `TurnInterrupted` instead of an error `RoleSpoke`. - // The id-bound marker means a stale halt cannot leak - // into a later turn's legitimate failure. - if halted_match { - let _ = runtime - .base - .events - .send(turn_interrupted_event( - &runtime.base.role, + Err(error) => { + warn!(role = %runtime.base.role, %error, "codex tools/call failed"); + if runtime.base.stopping.load(Ordering::SeqCst) { + break 'prompt_loop; + } + if matches!(&error, RpcError::Timeout) { + runtime.base.stopping.store(true, Ordering::SeqCst); + let _ = runtime.internal_stop.try_send(StopReason::TimedOut); + break 'prompt_loop; + } + if attempted_thread_id.is_some() + && !retried_stale_session + && is_stale_codex_session_error(&error) + { + warn!( + role = %runtime.base.role, + "codex resume thread id failed; retrying turn with a fresh thread" + ); + retried_stale_session = true; + thread_id = None; + continue; + } + // If the user halted *this* turn, the RPC error is + // expected (codex closed the pending request after + // our `notifications/cancelled` reached it). Emit + // `TurnInterrupted` instead of an error `RoleSpoke`. + // The id-bound marker means a stale halt cannot leak + // into a later turn's legitimate failure. + if halted_match { + emit_turn_interrupted( + &runtime, &partial_text, &turn_id, &thread_id_for_events, - )) + ) .await; - continue; + break; + } + emit_codex_error_event(&runtime, &error, turn_id, thread_id_for_events).await; + break; } - let text = format!("[codex error: {error}]"); - let _ = runtime - .base - .events - .send(CrepEvent::RoleSpoke { - role: runtime.base.role.clone(), - priors_hash: String::new(), - text, - mentions: Vec::new(), - cost_usd: 0.0, - cache_read: 0, - turn_id, - thread_id: thread_id_for_events, - outcome: crate::crep::TurnOutcome::Continue, - phase_block: None, - }) - .await; } } } debug!(role = %runtime.base.role, "codex write loop exiting"); } +fn is_stale_codex_session_error(error: &RpcError) -> bool { + matches!(error, RpcError::Server(message) if is_stale_codex_session_text(message)) +} + +fn is_stale_codex_session_text(text: &str) -> bool { + if is_stale_codex_session_message(text) { + return true; + } + serde_json::from_str::(text) + .ok() + .and_then(|value| { + value + .get("message") + .and_then(Value::as_str) + .map(is_stale_codex_session_message) + }) + .unwrap_or(false) +} + +fn is_stale_codex_session_message(text: &str) -> bool { + text.trim_start() + .starts_with("Session not found for thread_id:") +} + +async fn emit_turn_interrupted( + runtime: &CodexWriteRuntime, + partial_text: &str, + turn_id: &str, + thread_id: &str, +) { + let _ = runtime + .base + .events + .send(turn_interrupted_event( + &runtime.base.role, + partial_text, + turn_id, + thread_id, + )) + .await; +} + +async fn emit_codex_error_event( + runtime: &CodexWriteRuntime, + error: &RpcError, + turn_id: String, + thread_id: String, +) { + let text = format!("[codex error: {error}]"); + let _ = runtime + .base + .events + .send(CrepEvent::RoleSpoke { + role: runtime.base.role.clone(), + priors_hash: String::new(), + text, + mentions: Vec::new(), + cost_usd: 0.0, + cache_read: 0, + turn_id, + thread_id, + outcome: crate::crep::TurnOutcome::Continue, + phase_block: None, + }) + .await; +} + fn turn_interrupted_event( role: &str, partial_text: &str, @@ -1812,6 +1887,19 @@ mod tests { assert!(params["arguments"].get("base-instructions").is_none()); } + #[test] + fn stale_codex_session_errors_are_detected_in_tool_text_and_rpc_errors() { + let text = "Session not found for thread_id: 4028d717-299e-4039-b333-c615a041c197"; + assert!(is_stale_codex_session_text(text)); + assert!(is_stale_codex_session_error(&RpcError::Server( + serde_json::json!({"message": text}).to_string() + ))); + assert!(!is_stale_codex_session_text("ordinary model reply")); + assert!(!is_stale_codex_session_text( + "quoted text: Session not found for thread_id: fake" + )); + } + #[test] fn display_model_uses_configured_model_before_server_name() { assert_eq!( diff --git a/src/console_room_runtime.rs b/src/console_room_runtime.rs index dc61bfb..be48742 100644 --- a/src/console_room_runtime.rs +++ b/src/console_room_runtime.rs @@ -322,8 +322,7 @@ impl RoomRuntimeState { fn push_user_line(&mut self, line: &str) { // Style the @user tag consistently with the colored role - // identities around it. Off-white bold reads as "you" without - // colliding with any role slot in the palette. + // identities around it. self.push_scrollback(Line::from(vec![ Span::styled( "@user".to_owned(), @@ -1658,12 +1657,10 @@ fn strip_leading_role_header(rendered: &str, role: &str) -> String { } } -/// Display color for the `@user` tag in scrollback. Picked to read as -/// "you" without collding with any role slot in the palette (host -/// lavender, engineer/backend sky, reviewer blossom, security coral, -/// qa honey, sre teal, frontend rose, product jade). `EM` from the -/// crossterm palette is `RGB(0xf0, 0xf0, 0xf0)` — warm off-white. -const USER_TAG_COLOR: Color = Color::Rgb(0xf0, 0xf0, 0xf0); +/// Display color for the `@user` tag in scrollback. Use the prompt +/// green so user-authored text is visibly distinct from both body text +/// and every role slot. +const USER_TAG_COLOR: Color = Color::Rgb(0x58, 0xc3, 0x9c); #[cfg(test)] fn spinner_line(snapshot: &SpinnerSnapshot, host_role: &str) -> Line<'static> { diff --git a/src/lib.rs b/src/lib.rs index 2b10e49..31e1b2c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,7 +4,7 @@ //! primary consumer. See `docs/architecture.md` for the v0.1 constitution and //! `docs/v0.2-trust-and-interrupt.md` for the v0.2 amendment. -#![doc(html_root_url = "https://docs.rs/coreroom/0.9.18")] +#![doc(html_root_url = "https://docs.rs/coreroom/0.9.19")] #![warn(missing_debug_implementations)] #![warn(rust_2018_idioms)] diff --git a/src/repl/render.rs b/src/repl/render.rs index a318ec7..c8857da 100644 --- a/src/repl/render.rs +++ b/src/repl/render.rs @@ -167,9 +167,13 @@ pub(super) fn render_event_line_at_width( // with banners. CrepEvent::TurnDispatched { role, + parent_turn_id, queue_position, .. } => { + if parent_turn_id.is_none() && role == host_role { + return String::new(); + } if *queue_position == 0 { handoff_banner(role, host_role, "starting", width) } else { diff --git a/src/repl/tests.rs b/src/repl/tests.rs index 467ff36..c9ba9b9 100644 --- a/src/repl/tests.rs +++ b/src/repl/tests.rs @@ -640,17 +640,17 @@ fn snapshot_boot_dashboard_at_80() { .trim_start_matches('\n') .to_owned(); insta::assert_snapshot!(rendered, @r" -┌─ CoreRoom v0.9.18 ───────────────────────────────────────────────────────────┐ +┌─ CoreRoom v0.9.19 ───────────────────────────────────────────────────────────┐ │ │ │ welcome back, Ada tips for getting started │ │ • type @role to send a task to a sp… │ │ ◉ @host cc · 1M · ask • /halt @role interrupts a turn; Ct… │ │ ◇ @backend cc · 1M · ask • /journal captures today's … │ │ ◆ @security codex · default · bypass │ -│ what's new in 0.9.18 │ -│ 3.3k base tokens loaded • Claude Agent delegation is disabl… │ -│ /repo/CoreRoom • cr cost now normalizes Claude Cod… │ -│ • terminal text selection works by … │ +│ what's new in 0.9.19 │ +│ 3.3k base tokens loaded • stale Codex resume sessions retry… │ +│ /repo/CoreRoom • @user now has a visible identity … │ +│ • root host turns no longer double-… │ │ │ │ /help for commands │ │ │ @@ -862,6 +862,19 @@ fn turn_dispatched_renders_as_full_width_handoff_banner() { assert_eq!(rendered, " @frontend queued · 2 ahead"); } +#[test] +fn root_host_turn_dispatched_is_not_rendered_as_handoff_banner() { + let dispatched = CrepEvent::TurnDispatched { + role: "host".into(), + priors_hash: String::new(), + turn_id: String::new(), + thread_id: String::new(), + parent_turn_id: None, + queue_position: 0, + }; + assert_eq!(render_event_line_at_width(&dispatched, "host", 80), ""); +} + #[test] fn turn_dispatched_banner_pads_to_exact_width_in_tight_fits() { // Width budget exactly equal to fixed + 2 (1 dash position) used