diff --git a/src/commands/help.rs b/src/commands/help.rs index c23c6b52..faba0348 100644 --- a/src/commands/help.rs +++ b/src/commands/help.rs @@ -385,6 +385,10 @@ const START_HELP: &[HelpEntry] = &[ "start --as ", "Reclaim identity (after compaction/resume/clear)", ), + ( + "start --as --relocate", + "Move a stopped top-level Codex identity to this task's directory", + ), ( "start --orphan ", "Recover orphaned PTY process from pidtrack", diff --git a/src/commands/list.rs b/src/commands/list.rs index a04a02ee..3b0a65be 100644 --- a/src/commands/list.rs +++ b/src/commands/list.rs @@ -52,31 +52,182 @@ pub struct ListArgs { pub last: Option, } -/// Get unread message count for a single instance. -fn get_unread_count(db: &HcomDb, name: &str, last_event_id: i64) -> i64 { - db.conn() +/// Diagnostic-only threshold for a pending PTY delivery whose durable gate +/// state says it is blocked. This does not change delivery retries or timeouts. +const DELIVERY_STALLED_AFTER_SECS: i64 = 60; + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +struct UnreadInfo { + count: i64, + oldest_age_seconds: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct DeliveryVisibility { + mode: &'static str, + state: &'static str, + stalled: bool, + detail: String, +} + +/// Get durable unread evidence for a single instance. +/// +/// The cursor/event gap proves that a message is still pending. Event time is +/// used only to age that fact; an unparseable timestamp remains visible as +/// unread but never manufactures a stalled signal. +fn get_unread_info(db: &HcomDb, name: &str, last_event_id: i64, now: i64) -> UnreadInfo { + let (count, oldest_timestamp): (i64, Option) = db + .conn() .query_row( - "SELECT COUNT(*) FROM events WHERE id > ? AND type = 'message' + "SELECT COUNT(*), MIN(timestamp) FROM events + WHERE id > ? AND type = 'message' AND EXISTS (SELECT 1 FROM json_each(json_extract(data, '$.delivered_to')) WHERE value = ?)", rusqlite::params![last_event_id, name], - |row| row.get(0), + |row| Ok((row.get(0)?, row.get(1)?)), ) - .unwrap_or(0) + .unwrap_or((0, None)); + + let oldest_age_seconds = oldest_timestamp + .as_deref() + .and_then(|timestamp| chrono::DateTime::parse_from_rfc3339(timestamp).ok()) + .map(|timestamp| now.saturating_sub(timestamp.timestamp()).max(0)); + + UnreadInfo { + count, + oldest_age_seconds, + } } -/// Get unread counts for all instances in batch. -fn get_unread_counts_batch(db: &HcomDb, instances: &[InstanceRow]) -> HashMap { - let mut counts = HashMap::new(); +/// Get unread evidence for all local instances in batch. +fn get_unread_info_batch(db: &HcomDb, instances: &[InstanceRow]) -> HashMap { + let mut unread = HashMap::new(); + let now = crate::shared::time::now_epoch_i64(); for inst in instances { if is_remote_instance(inst) { continue; } - let count = get_unread_count(db, &inst.name, inst.last_event_id); - if count > 0 { - counts.insert(inst.name.clone(), count); + let info = get_unread_info(db, &inst.name, inst.last_event_id, now); + if info.count > 0 { + unread.insert(inst.name.clone(), info); + } + } + unread +} + +fn gate_block_detail(status_context: &str, status_detail: &str) -> Option { + let reason = status_context.strip_prefix("tui:")?; + if !status_detail.is_empty() && status_detail != "cmd:listen" { + Some(status_detail.to_string()) + } else { + Some(reason.replace('-', " ")) + } +} + +/// Classify only what the durable database state proves. +/// +/// Hook-only sessions have no HCOM-owned input stream, so queued messages are +/// boundary-driven rather than instant. A PTY delivery is called stalled only +/// when both halves are present: an old unread event and a persisted `tui:*` +/// gate reason. This intentionally avoids inferring failure from silence alone. +fn delivery_visibility( + status_context: &str, + status_detail: &str, + is_remote: bool, + hooks_bound: bool, + process_bound: bool, + unread: &UnreadInfo, +) -> DeliveryVisibility { + let mode = if is_remote { + "remote" + } else if process_bound { + "pty" + } else if hooks_bound { + "hook_boundary" + } else { + "manual" + }; + + let gate_detail = gate_block_detail(status_context, status_detail); + let old_enough = unread + .oldest_age_seconds + .is_some_and(|age| age >= DELIVERY_STALLED_AFTER_SECS); + let stalled_detail = if unread.count > 0 && old_enough { + if process_bound { + gate_detail.clone() + } else if !is_remote && !hooks_bound { + Some("no automatic delivery binding".to_string()) + } else { + None + } + } else { + None + }; + + if let Some(detail) = stalled_detail { + return DeliveryVisibility { + mode, + state: "stalled", + stalled: true, + detail, + }; + } + + if unread.count == 0 { + let detail = if mode == "hook_boundary" { + "delivery occurs at a supported hook boundary, not instantly".to_string() + } else { + String::new() + }; + return DeliveryVisibility { + mode, + state: "clear", + stalled: false, + detail, + }; + } + + let (state, detail) = match mode { + "hook_boundary" => ( + "waiting_for_hook_boundary", + "queued until a supported hook boundary; no PTY injection".to_string(), + ), + "pty" if gate_detail.is_some() => ( + "blocked", + gate_detail.unwrap_or_else(|| "PTY delivery gate is blocked".to_string()), + ), + "pty" => ("queued", "queued for PTY delivery".to_string()), + "manual" => ( + "waiting_for_poll", + "unread with no automatic delivery binding".to_string(), + ), + _ => ("queued_remote", "queued for remote delivery".to_string()), + }; + + DeliveryVisibility { + mode, + state, + stalled: false, + detail, + } +} + +fn human_delivery_suffix(delivery: &DeliveryVisibility, unread: &UnreadInfo) -> String { + let age = unread.oldest_age_seconds.map(format_age); + if delivery.stalled { + let age = age.unwrap_or_else(|| "unknown age".to_string()); + format!(" | DELIVERY STALLED {age}: {}", delivery.detail) + } else if delivery.mode == "hook_boundary" { + if unread.count > 0 { + let age = age.map(|age| format!(", oldest {age}")).unwrap_or_default(); + format!(" | delivery: supported hook boundary{age}") + } else { + " | delivery: hook boundary (not instant)".to_string() } + } else if unread.count > 0 && delivery.mode == "manual" { + " | delivery: manual poll required".to_string() + } else { + String::new() } - counts } /// Main entry point for `hcom list` command. @@ -148,6 +299,22 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i match db.get_instance_full(&lookup_name) { Ok(Some(data)) => { + let hooks_bound = db.has_session_binding(&data.name); + let process_bound = db.has_process_binding_for_instance(&data.name); + let unread = get_unread_info( + db, + &data.name, + data.last_event_id, + crate::shared::time::now_epoch_i64(), + ); + let delivery = delivery_visibility( + &data.status_context, + &data.status_detail, + is_remote_instance(&data), + hooks_bound, + process_bound, + &unread, + ); let mut payload = serde_json::json!({ "name": lookup_name, "session_id": data.session_id, @@ -157,6 +324,15 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i "parent_name": data.parent_name, "agent_id": data.agent_id, "tool": data.tool, + "unread_count": unread.count, + "oldest_unread_age_seconds": unread.oldest_age_seconds, + "hooks_bound": hooks_bound, + "process_bound": process_bound, + "delivery_mode": delivery.mode, + "delivery_state": delivery.state, + "delivery_stalled": delivery.stalled, + "delivery_stalled_after_seconds": DELIVERY_STALLED_AFTER_SECS, + "delivery_detail": delivery.detail, }); if is_self @@ -211,7 +387,7 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i } }; - let unread_counts = get_unread_counts_batch(db, &sorted_instances); + let unread_info = get_unread_info_batch(db, &sorted_instances); if names_output { for data in &sorted_instances { @@ -231,6 +407,15 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i // Get binding status let hooks_bound = db.has_session_binding(&data.name); let process_bound = db.has_process_binding_for_instance(&data.name); + let unread = unread_info.get(&data.name).cloned().unwrap_or_default(); + let delivery = delivery_visibility( + &data.status_context, + &data.status_detail, + is_remote_instance(data), + hooks_bound, + process_bound, + &unread, + ); // Parse launch_context JSON let launch_context: serde_json::Value = data @@ -247,7 +432,8 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i "status_detail": data.status_detail, "status_age_seconds": age_seconds, "description": description, - "unread_count": unread_counts.get(&data.name).copied().unwrap_or(0), + "unread_count": unread.count, + "oldest_unread_age_seconds": unread.oldest_age_seconds, "headless": data.background != 0, "session_id": data.session_id.as_deref().unwrap_or(""), "directory": data.directory, @@ -261,6 +447,11 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i "base_name": data.name, "hooks_bound": hooks_bound, "process_bound": process_bound, + "delivery_mode": delivery.mode, + "delivery_state": delivery.state, + "delivery_stalled": delivery.stalled, + "delivery_stalled_after_seconds": DELIVERY_STALLED_AFTER_SECS, + "delivery_detail": delivery.detail, "launch_context": launch_context, }); result_list.push(payload); @@ -358,7 +549,10 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i if is_remote_instance(data) { n += 9; // " [remote]" } - let uc = unread_counts.get(&data.name).copied().unwrap_or(0); + let uc = unread_info + .get(&data.name) + .map(|info| info.count) + .unwrap_or(0); if uc > 0 { n += format!(" +{uc}").len(); } @@ -371,6 +565,8 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i let cs = get_instance_status(data, db); let (status, age_str, description) = (cs.status, cs.age_string, cs.description); let icon = status_icon(&status); + let hooks_bound = db.has_session_binding(&data.name); + let process_bound = db.has_process_binding_for_instance(&data.name); let age_display = if age_str == "now" { age_str.clone() @@ -385,8 +581,6 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i // Tool prefix — binding state encoding: // UPPER = pty+hooks, lower = hooks only, UPPER* = pty only, lower* = no binding let tool_prefix = if show_tool { - let hooks_bound = db.has_session_binding(&data.name); - let process_bound = db.has_process_binding_for_instance(&data.name); let tool_display = if data.tool == "adhoc" { "ad-hoc".to_string() } else if process_bound && hooks_bound { @@ -417,13 +611,23 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i }; // Unread - let unread = unread_counts.get(&data.name).copied().unwrap_or(0); - let unread_str = if unread > 0 { - format!(" +{unread}") + let unread = unread_info.get(&data.name).cloned().unwrap_or_default(); + let unread_str = if unread.count > 0 { + format!(" +{}", unread.count) } else { String::new() }; + let delivery = delivery_visibility( + &data.status_context, + &data.status_detail, + is_remote_instance(data), + hooks_bound, + process_bound, + &unread, + ); + let delivery_suffix = human_delivery_suffix(&delivery, &unread); + // Listening-since suffix: show idle duration for listening agents idle >= 60s let listening_since = if status == ST_LISTENING && cs.age_seconds >= 60 { format!(" since {}", format_age(cs.age_seconds)) @@ -453,8 +657,9 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i }; let name_part = format!("{name}{headless_badge}{remote_badge}{unread_str}"); - let status_text = - format!("{age_display}{desc_sep}{description}{listening_since}{timeout_marker}"); + let status_text = format!( + "{age_display}{desc_sep}{description}{listening_since}{timeout_marker}{delivery_suffix}" + ); println!( "{tool_prefix}{icon} {name_part:) -> i } // Binding status - let hooks_bound = db.has_session_binding(&data.name); - let process_bound = db.has_process_binding_for_instance(&data.name); let bind_str = match (hooks_bound, process_bound) { (true, true) => "hooks, pty", (true, false) => "hooks", @@ -516,6 +719,13 @@ pub fn cmd_list(db: &HcomDb, args: &ListArgs, ctx: Option<&CommandContext>) -> i (false, false) => "none", }; println!(" bindings: {bind_str}"); + println!(" delivery: {} ({})", delivery.state, delivery.mode); + if let Some(age) = unread.oldest_age_seconds { + println!(" oldest unread: {}", format_age(age)); + } + if !delivery.detail.is_empty() { + println!(" delivery note: {}", delivery.detail); + } let transcript = if data.transcript_path.is_empty() { "(none)".to_string() @@ -658,16 +868,31 @@ fn print_instance_details(db: &HcomDb, data: &InstanceRow, display_name: &str) { } } - // Unread Count - let unread = get_unread_count(db, &data.name, data.last_event_id); - if unread > 0 { - let s = if unread == 1 { "" } else { "s" }; - println!(" Unread: {unread} message{s}"); - } - - // Bindings + // Delivery evidence and bindings let hooks_bound = db.has_session_binding(&data.name); let process_bound = db.has_process_binding_for_instance(&data.name); + let unread = get_unread_info( + db, + &data.name, + data.last_event_id, + crate::shared::time::now_epoch_i64(), + ); + let delivery = delivery_visibility( + &data.status_context, + &data.status_detail, + is_remote_instance(data), + hooks_bound, + process_bound, + &unread, + ); + if unread.count > 0 { + let s = if unread.count == 1 { "" } else { "s" }; + println!(" Unread: {} message{s}", unread.count); + if let Some(age) = unread.oldest_age_seconds { + println!(" Oldest: {}", format_age(age)); + } + } + let bind_str = match (hooks_bound, process_bound) { (true, true) => "hooks, pty", (true, false) => "hooks", @@ -675,6 +900,10 @@ fn print_instance_details(db: &HcomDb, data: &InstanceRow, display_name: &str) { (false, false) => "none", }; println!(" Bindings: {bind_str}"); + println!(" Delivery: {} ({})", delivery.state, delivery.mode); + if !delivery.detail.is_empty() { + println!(" Delivery Note: {}", delivery.detail); + } if let Some(pid) = data.pid { println!(" PID: {pid}"); @@ -964,3 +1193,118 @@ fn get_recently_stopped( .filter(|name| !exclude_active.contains(name)) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + use serial_test::serial; + + #[test] + fn hook_only_delivery_is_boundary_driven_not_stalled() { + let unread = UnreadInfo { + count: 2, + oldest_age_seconds: Some(600), + }; + + let delivery = delivery_visibility("", "", false, true, false, &unread); + + assert_eq!(delivery.mode, "hook_boundary"); + assert_eq!(delivery.state, "waiting_for_hook_boundary"); + assert!(!delivery.stalled); + assert!(delivery.detail.contains("supported hook boundary")); + assert!(human_delivery_suffix(&delivery, &unread).contains("hook boundary")); + } + + #[test] + fn pty_gate_becomes_stalled_only_at_bounded_age() { + let young = UnreadInfo { + count: 1, + oldest_age_seconds: Some(DELIVERY_STALLED_AFTER_SECS - 1), + }; + let old = UnreadInfo { + count: 1, + oldest_age_seconds: Some(DELIVERY_STALLED_AFTER_SECS), + }; + + let young_delivery = delivery_visibility( + "tui:prompt-has-text", + "uncommitted text in prompt", + false, + true, + true, + &young, + ); + let old_delivery = delivery_visibility( + "tui:prompt-has-text", + "uncommitted text in prompt", + false, + true, + true, + &old, + ); + + assert_eq!(young_delivery.state, "blocked"); + assert!(!young_delivery.stalled); + assert_eq!(old_delivery.state, "stalled"); + assert!(old_delivery.stalled); + assert_eq!(old_delivery.detail, "uncommitted text in prompt"); + } + + #[test] + fn old_unread_without_any_binding_has_explicit_stalled_signal() { + let unread = UnreadInfo { + count: 1, + oldest_age_seconds: Some(DELIVERY_STALLED_AFTER_SECS), + }; + + let delivery = delivery_visibility("", "", false, false, false, &unread); + + assert_eq!(delivery.mode, "manual"); + assert_eq!(delivery.state, "stalled"); + assert!(delivery.stalled); + assert_eq!(delivery.detail, "no automatic delivery binding"); + } + + #[test] + #[serial] + fn unread_info_uses_durable_recipient_and_event_age() { + let (_dir, _hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + db.conn() + .execute( + "INSERT INTO instances (name, tool, created_at, last_event_id) VALUES ('desktop', 'codex', 1.0, 0)", + [], + ) + .unwrap(); + db.log_event_with_ts( + "message", + "sender", + &serde_json::json!({ + "from": "sender", + "text": "queued", + "delivered_to": ["desktop"] + }), + Some("2026-01-01T00:00:00Z"), + ) + .unwrap(); + db.log_event_with_ts( + "message", + "sender", + &serde_json::json!({ + "from": "sender", + "text": "not for desktop", + "delivered_to": ["someone-else"] + }), + Some("2026-01-01T00:00:30Z"), + ) + .unwrap(); + let now = chrono::DateTime::parse_from_rfc3339("2026-01-01T00:02:00Z") + .unwrap() + .timestamp(); + + let unread = get_unread_info(&db, "desktop", 0, now); + + assert_eq!(unread.count, 1); + assert_eq!(unread.oldest_age_seconds, Some(120)); + } +} diff --git a/src/commands/send.rs b/src/commands/send.rs index 9bfdc8a9..b0641fe7 100644 --- a/src/commands/send.rs +++ b/src/commands/send.rs @@ -248,10 +248,27 @@ fn deliverable_instances(db: &HcomDb) -> Result, String> { let rows = db .conn() .prepare( - "SELECT name, tag FROM instances - WHERE status != 'stopped' - AND status_context != 'launch_failed' - AND NOT (status = 'inactive' AND status_context LIKE 'exit:%')", + "SELECT i.name, i.tag FROM instances i + WHERE i.status != 'stopped' + AND i.status_context != 'launch_failed' + AND ( + NOT (i.status = 'inactive' AND i.status_context LIKE 'exit:%') + OR ( + i.status = 'inactive' + AND i.status_context = 'exit:timeout' + AND i.tool = 'claude' + AND COALESCE(i.origin_device_id, '') = '' + AND EXISTS ( + SELECT 1 FROM session_bindings sb + WHERE sb.instance_name = i.name + AND sb.session_id = i.session_id + ) + AND NOT EXISTS ( + SELECT 1 FROM process_bindings pb + WHERE pb.instance_name = i.name + ) + ) + )", ) .map_err(|e| format!("DB error: {e}"))? .query_map([], |row| { @@ -1702,6 +1719,75 @@ mod tests { cleanup_test_db(path); } + #[test] + #[serial] + fn send_mention_queues_for_hook_only_claude_timeout() { + let (db, path, _env) = setup_test_db(); + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, status, status_context, created_at, last_event_id) + VALUES ('luna', 'sess-luna', 'claude', 'listening', '', 1000.0, 0), + ('risa', 'sess-risa', 'claude', 'inactive', 'exit:timeout', 1000.0, 0)", + [], + ) + .unwrap(); + db.set_session_binding("sess-risa", "risa").unwrap(); + + let sender = SenderIdentity { + kind: SenderKind::Instance, + name: "luna".into(), + instance_data: None, + session_id: Some("sess-luna".into()), + }; + + let delivered = + send_message(&db, &sender, "queued", None, Some(&["risa".to_string()])).unwrap(); + assert_eq!(delivered, vec!["risa".to_string()]); + + let unread = db.get_unread_messages("risa"); + assert_eq!(unread.len(), 1); + assert_eq!(unread[0].text, "queued"); + assert_eq!(db.get_cursor("risa"), 0, "send must not consume the queue"); + + cleanup_test_db(path); + } + + #[test] + #[serial] + fn send_mention_excludes_non_desktop_claude_timeout() { + let (db, path, _env) = setup_test_db(); + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, status, status_context, created_at) + VALUES ('luna', 'sess-luna', 'claude', 'listening', '', 1000.0), + ('risa', 'sess-risa', 'claude', 'inactive', 'exit:timeout', 1000.0)", + [], + ) + .unwrap(); + + let sender = SenderIdentity { + kind: SenderKind::Instance, + name: "luna".into(), + instance_data: None, + session_id: Some("sess-luna".into()), + }; + + let err = + send_message(&db, &sender, "ping", None, Some(&["risa".to_string()])).unwrap_err(); + assert!(err.contains("@risa"), "err={err}"); + + db.set_session_binding("sess-risa", "risa").unwrap(); + db.set_process_binding("proc-risa", "sess-risa", "risa") + .unwrap(); + let err = + send_message(&db, &sender, "ping", None, Some(&["risa".to_string()])).unwrap_err(); + assert!(err.contains("@risa"), "err={err}"); + + cleanup_test_db(path); + } + #[test] fn process_compat_at_with_space() { // "@luna hi" → full text as message, no targets diff --git a/src/commands/start.rs b/src/commands/start.rs index de6f7ae8..6d95fc47 100644 --- a/src/commands/start.rs +++ b/src/commands/start.rs @@ -1,4 +1,4 @@ -//! Start command: `hcom start [--name ] [--as ] [--orphan ]` +//! Start command: `hcom start [--name ] [--as [--relocate]] [--orphan ]` //! //! Runs inside an already-running tool session rather than launching a new one. //! Used for adhoc/manual setup, identity rebinding, and orphan recovery: @@ -9,8 +9,10 @@ //! - `--as`: rebind session identity use anyhow::{Result, bail}; +use rusqlite::OptionalExtension; use serde_json::json; use std::collections::{HashMap, HashSet}; +use std::io::{BufRead, BufReader, Read}; use std::path::PathBuf; use crate::bootstrap; @@ -37,6 +39,9 @@ pub struct StartArgs { /// Rebind to a different instance name #[arg(long = "as")] pub as_name: Option, + /// Explicitly move a stopped top-level Codex identity to this task's directory + #[arg(long)] + pub relocate: bool, /// Recover orphaned PTY process by name or PID #[arg(long)] pub orphan: Option, @@ -73,6 +78,14 @@ pub fn run(argv: &[String], flags: &GlobalFlags) -> Result { let orphan_target = start_args.orphan; let rebind_target = start_args.as_name; + let relocate = start_args.relocate; + + if relocate && rebind_target.is_none() { + bail!("--relocate requires --as "); + } + if relocate && orphan_target.is_some() { + bail!("--relocate cannot be combined with --orphan"); + } let db = HcomDb::open()?; let hcom_dir = paths::hcom_dir(); @@ -137,7 +150,7 @@ pub fn run(argv: &[String], flags: &GlobalFlags) -> Result { .as_ref() .map(|actor| actor.name.as_str()) .or(requested_name.as_deref()); - return start_rebind(&db, &rebind, &ctx, current_name); + return start_rebind_with_options(&db, &rebind, &ctx, current_name, relocate); } if let Some(subagent) = subagent_via_name { @@ -350,11 +363,22 @@ fn restore_child_links_after_root_rebind( /// Rebind session identity (`--as `), preserving last_event_id and any /// live Claude child hierarchy owned by the current root actor. +#[cfg(test)] fn start_rebind( db: &HcomDb, rebind_target: &str, ctx: &HcomContext, explicit_name: Option<&str>, +) -> Result { + start_rebind_with_options(db, rebind_target, ctx, explicit_name, false) +} + +fn start_rebind_with_options( + db: &HcomDb, + rebind_target: &str, + ctx: &HcomContext, + explicit_name: Option<&str>, + relocate: bool, ) -> Result { let hcom_dir = paths::hcom_dir(); @@ -398,6 +422,23 @@ fn start_rebind( // unbound and the identity it replaces is never cleaned up. session_id = resolve_claude_session_id(&ctx.raw_env); } + if ctx.tool == crate::tool::Tool::Codex { + // Codex Desktop is not launched through hcom, so it has no + // HCOM_PROCESS_ID/process binding. Its thread id is the stable + // session identity exposed to commands run inside the task. + if let Some(codex_session_id) = resolve_codex_session_id(ctx) { + if let Some(existing_session_id) = session_id.as_deref() + && existing_session_id != codex_session_id + { + bail!( + "Refusing Codex rebind: current task session '{}' conflicts with existing session '{}'", + codex_session_id, + existing_session_id + ); + } + session_id = Some(codex_session_id); + } + } let current_name = if !explicit_current_name.is_empty() { explicit_current_name.to_string() } else if let Some(ref sid) = session_id { @@ -407,9 +448,35 @@ fn start_rebind( }; let child_links = snapshot_child_links(db, session_id.as_deref())?; - let target_meta = load_rebind_target_metadata(db, &target_name).ok(); + let relocation = if relocate { + Some(validate_codex_relocation( + db, + &target_name, + ctx, + session_id.as_deref(), + ¤t_name, + )?) + } else { + None + }; + let target_meta = if relocation.is_some() { + None + } else { + load_rebind_target_metadata(db, &target_name).ok() + }; if let Some(ref meta) = target_meta { - ensure_rebind_compatible(&target_name, meta, ctx)?; + ensure_rebind_compatible(&target_name, meta, ctx, relocation.is_some())?; + } + + // A relocation is deliberately not a generic rebind. A generic rebind + // deletes the target before recreating it, which lets a registration that + // wins between validation and mutation be deleted and stolen. Compare the + // exact stopped proof, create the row, and bind the session under one + // BEGIN IMMEDIATE transaction. Any competing winner makes the whole + // transaction fail without changing that winner. + if let Some(ref proof) = relocation { + commit_codex_relocation(db, &target_name, ctx, proof)?; + return finish_rebind_output(db, &hcom_dir, &target_name, ctx, ¤t_name, true); } // Preserve last_event_id from target (cursor preservation) @@ -448,14 +515,17 @@ fn start_rebind( // Create fresh instance with the target name let tool = ctx.tool.as_str(); let cwd_override = ctx.cwd.to_string_lossy().to_string(); - instance_binding::initialize_instance_in_position_file( + let transcript_path = relocation + .as_ref() + .map(|proof| proof.transcript_path.as_str()); + let initialized = instance_binding::initialize_instance_in_position_file( db, &target_name, session_id.as_deref(), None, // parent_session_id None, // parent_name None, // agent_id - None, // transcript_path + transcript_path, Some(tool), false, // background None, // tag @@ -465,6 +535,11 @@ fn start_rebind( Some(&cwd_override), ); + debug_assert!(relocation.is_none()); + if !initialized { + bail!("Could not create identity '{target_name}'"); + } + if let Some(ref sid) = session_id { let old_root = if current_name.is_empty() { target_name.as_str() @@ -521,7 +596,18 @@ fn start_rebind( crate::notify::wake(db, &target_name, crate::notify::WakeKind::DELIVERY_LOOPS); } - // Print bootstrap + finish_rebind_output(db, &hcom_dir, &target_name, ctx, ¤t_name, false) +} + +fn finish_rebind_output( + db: &HcomDb, + hcom_dir: &std::path::Path, + target_name: &str, + ctx: &HcomContext, + current_name: &str, + relocated: bool, +) -> Result { + let tool = ctx.tool.as_str(); let hcom_config = HcomConfig::load(None).unwrap_or_else(|_| { let mut c = HcomConfig::default(); c.normalize(); @@ -530,8 +616,8 @@ fn start_rebind( let bootstrap_text = bootstrap::get_bootstrap( db, - &hcom_dir, - &target_name, + hcom_dir, + target_name, tool, false, false, @@ -549,12 +635,415 @@ fn start_rebind( log_info( "start", "rebind.complete", - &format!("from={} to={}", current_name, target_name), + &format!( + "from={} to={} relocated={}", + current_name, target_name, relocated + ), ); Ok(0) } +#[derive(Debug, Clone, PartialEq, Eq)] +struct CodexRelocationProof { + session_id: String, + transcript_path: String, + stopped_event_id: i64, + stopped_event_data: String, + last_event_id: i64, +} + +const MAX_CODEX_SESSION_META_BYTES: u64 = 2 * 1024 * 1024; + +fn validate_codex_relocation( + db: &HcomDb, + target_name: &str, + ctx: &HcomContext, + resolved_session_id: Option<&str>, + current_name: &str, +) -> Result { + if ctx.tool != crate::tool::Tool::Codex { + bail!("--relocate is supported only from the owning Codex Desktop task"); + } + if db.get_instance_full(target_name)?.is_some() { + bail!("Refusing to relocate '{target_name}': the identity is still live"); + } + if !current_name.is_empty() && current_name != target_name { + bail!( + "Refusing to relocate '{target_name}': this task is already bound as '{current_name}'" + ); + } + if !ctx.cwd.is_absolute() || !ctx.cwd.is_dir() { + bail!( + "Refusing to relocate '{target_name}': current directory '{}' is not an existing absolute directory", + ctx.cwd.display() + ); + } + + let thread_id = resolve_codex_session_id(ctx) + .ok_or_else(|| anyhow::anyhow!("Codex relocation requires CODEX_THREAD_ID"))?; + let session_env = ctx + .raw_env + .get("CODEX_SESSION_ID") + .map(String::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .ok_or_else(|| anyhow::anyhow!("Codex relocation requires CODEX_SESSION_ID"))?; + if thread_id != session_env { + bail!( + "Refusing Codex relocation: CODEX_THREAD_ID '{}' conflicts with CODEX_SESSION_ID '{}'", + thread_id, + session_env + ); + } + if resolved_session_id != Some(thread_id.as_str()) { + bail!("Refusing Codex relocation: current session identity is not canonical"); + } + if let Some(owner) = db.get_session_binding(&thread_id)? { + bail!( + "Refusing to relocate '{target_name}': session '{thread_id}' is already bound to '{owner}'" + ); + } + + let (stopped_event_id, stopped_event_data): (i64, String) = db + .conn() + .query_row( + "SELECT id, data FROM events + WHERE type = 'life' + AND instance = ?1 + AND json_extract(data, '$.action') = 'stopped' + ORDER BY id DESC LIMIT 1", + rusqlite::params![target_name], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .map_err(|_| anyhow::anyhow!("No stopped identity found for '{target_name}'"))?; + let data: serde_json::Value = serde_json::from_str(&stopped_event_data)?; + let snapshot = data + .get("snapshot") + .ok_or_else(|| anyhow::anyhow!("Stopped identity '{target_name}' has no snapshot"))?; + + if snapshot.get("tool").and_then(serde_json::Value::as_str) != Some("codex") { + bail!("Refusing to relocate '{target_name}': stopped identity is not Codex"); + } + if !json_identity_field_is_empty(snapshot.get("origin_device_id")) { + bail!("Refusing to relocate '{target_name}': stopped identity is remote"); + } + if !json_identity_field_is_empty(snapshot.get("parent_name")) + || !json_identity_field_is_empty(snapshot.get("parent_session_id")) + || !json_identity_field_is_empty(snapshot.get("agent_id")) + { + bail!("Refusing to relocate '{target_name}': stopped identity is a child task"); + } + if snapshot.get("background").is_some_and(|value| { + !value.is_null() && value.as_i64().is_none_or(|background| background != 0) + }) { + bail!("Refusing to relocate '{target_name}': stopped identity is not top-level"); + } + if snapshot + .get("session_id") + .and_then(serde_json::Value::as_str) + != Some(thread_id.as_str()) + { + bail!("Refusing to relocate '{target_name}': stopped session does not match this task"); + } + + let stopped_directory = snapshot + .get("directory") + .and_then(serde_json::Value::as_str) + .unwrap_or(""); + let current_directory = ctx.cwd.to_string_lossy(); + if stopped_directory.is_empty() || same_path(stopped_directory, ¤t_directory) { + bail!( + "Refusing to relocate '{target_name}': stopped and current directories are not distinct" + ); + } + + let snapshot_transcript = snapshot + .get("transcript_path") + .and_then(serde_json::Value::as_str) + .filter(|value| !value.is_empty()) + .ok_or_else(|| anyhow::anyhow!("Stopped identity '{target_name}' has no transcript"))?; + let derived_transcript = crate::hooks::codex::derive_codex_transcript_path(&thread_id) + .ok_or_else(|| anyhow::anyhow!("Could not locate the active Codex transcript"))?; + if !same_path(snapshot_transcript, &derived_transcript) { + bail!("Refusing to relocate '{target_name}': stopped and active transcript paths differ"); + } + validate_codex_session_meta(&derived_transcript, &thread_id, stopped_directory)?; + + let last_event_id = snapshot + .get("last_event_id") + .and_then(serde_json::Value::as_i64) + .filter(|value| *value >= 0) + .ok_or_else(|| { + anyhow::anyhow!("Stopped identity '{target_name}' has no valid event cursor") + })?; + + Ok(CodexRelocationProof { + session_id: thread_id, + transcript_path: derived_transcript, + stopped_event_id, + stopped_event_data, + last_event_id, + }) +} + +fn json_identity_field_is_empty(value: Option<&serde_json::Value>) -> bool { + match value { + None | Some(serde_json::Value::Null) => true, + Some(serde_json::Value::String(value)) => value.is_empty(), + Some(_) => false, + } +} + +fn commit_codex_relocation( + db: &HcomDb, + target_name: &str, + ctx: &HcomContext, + proof: &CodexRelocationProof, +) -> Result<()> { + let directory = ctx.cwd.to_string_lossy().to_string(); + let launch_context = + crate::hooks::codex::directory_override_launch_context(&directory, &proof.session_id); + let created_at = crate::shared::time::now_epoch_f64(); + let status_time = crate::shared::time::now_epoch_i64(); + let wait_timeout = HcomConfig::effective_timeout(); + + db.with_immediate_transaction(|txn| { + let latest_stop = txn + .query_row( + "SELECT id, data FROM events + WHERE type = 'life' + AND instance = ?1 + AND json_extract(data, '$.action') = 'stopped' + ORDER BY id DESC LIMIT 1", + rusqlite::params![target_name], + |row| Ok((row.get::<_, i64>(0)?, row.get::<_, String>(1)?)), + ) + .optional()?; + if latest_stop.as_ref() + != Some(&( + proof.stopped_event_id, + proof.stopped_event_data.clone(), + )) + { + bail!( + "Refusing to relocate '{target_name}': stopped identity changed during verification" + ); + } + + let target_exists: bool = txn.query_row( + "SELECT EXISTS(SELECT 1 FROM instances WHERE name = ?1)", + rusqlite::params![target_name], + |row| row.get(0), + )?; + if target_exists { + bail!( + "Refusing to relocate '{target_name}': another task registered it during verification" + ); + } + + let session_owner = txn + .query_row( + "SELECT instance_name FROM session_bindings WHERE session_id = ?1", + rusqlite::params![&proof.session_id], + |row| row.get::<_, String>(0), + ) + .optional()?; + if let Some(owner) = session_owner { + bail!( + "Refusing to relocate '{target_name}': session is already bound to '{owner}'" + ); + } + + let target_binding = txn + .query_row( + "SELECT session_id FROM session_bindings WHERE instance_name = ?1 LIMIT 1", + rusqlite::params![target_name], + |row| row.get::<_, String>(0), + ) + .optional()?; + if target_binding.is_some() { + bail!( + "Refusing to relocate '{target_name}': target binding changed during verification" + ); + } + + let instance_session_owner = txn + .query_row( + "SELECT name FROM instances WHERE session_id = ?1 LIMIT 1", + rusqlite::params![&proof.session_id], + |row| row.get::<_, String>(0), + ) + .optional()?; + if let Some(owner) = instance_session_owner { + bail!( + "Refusing to relocate '{target_name}': session is already owned by '{owner}'" + ); + } + + let process_collision: bool = txn.query_row( + "SELECT EXISTS( + SELECT 1 FROM process_bindings + WHERE instance_name = ?1 OR session_id = ?2 + )", + rusqlite::params![target_name, &proof.session_id], + |row| row.get(0), + )?; + if process_collision { + bail!( + "Refusing to relocate '{target_name}': a process binding changed during verification" + ); + } + + let initial_event_id = proof.last_event_id; + + txn.execute( + "INSERT INTO instances ( + name, session_id, last_event_id, last_stop, status, status_time, + status_context, directory, created_at, transcript_path, tool, + background, wait_timeout, name_announced, launch_context + ) VALUES ( + ?1, ?2, ?3, 0, 'inactive', ?4, + 'new', ?5, ?6, ?7, 'codex', + 0, ?8, 1, ?9 + )", + rusqlite::params![ + target_name, + &proof.session_id, + initial_event_id, + status_time, + &directory, + created_at, + &proof.transcript_path, + wait_timeout, + &launch_context, + ], + )?; + txn.execute( + "INSERT INTO session_bindings (session_id, instance_name, created_at) + VALUES (?1, ?2, ?3)", + rusqlite::params![&proof.session_id, target_name, created_at], + )?; + + let written: (Option, String, String, String, i64, String) = txn.query_row( + "SELECT session_id, tool, directory, transcript_path, last_event_id, launch_context + FROM instances WHERE name = ?1", + rusqlite::params![target_name], + |row| { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + row.get(5)?, + )) + }, + )?; + let binding: String = txn.query_row( + "SELECT instance_name FROM session_bindings WHERE session_id = ?1", + rusqlite::params![&proof.session_id], + |row| row.get(0), + )?; + if written.0.as_deref() != Some(proof.session_id.as_str()) + || written.1 != "codex" + || written.2 != directory + || written.3 != proof.transcript_path + || written.4 != initial_event_id + || written.5 != launch_context + || binding != target_name + { + bail!("Relocated Codex identity failed transactional verification"); + } + Ok(()) + })?; + + let row = db + .get_instance_full(target_name)? + .ok_or_else(|| anyhow::anyhow!("Relocated Codex identity disappeared after commit"))?; + let binding = db.get_session_binding(&proof.session_id)?; + if row.tool != "codex" + || row.session_id.as_deref() != Some(proof.session_id.as_str()) + || !same_path(&row.directory, &directory) + || !same_path(&row.transcript_path, &proof.transcript_path) + || binding.as_deref() != Some(target_name) + || crate::hooks::codex::pinned_directory_from_launch_context( + row.launch_context.as_deref(), + &proof.session_id, + ) + .is_none_or(|pinned| !same_path(&pinned, &directory)) + { + bail!("Relocated Codex identity failed final bound-state verification"); + } + + let _ = db.log_event( + "life", + target_name, + &serde_json::json!({ + "action": "created", + "by": "explicit-start-relocate-v1", + "is_hcom_launched": false, + "is_subagent": false, + "parent_name": "", + }), + ); + Ok(()) +} + +fn validate_codex_session_meta( + transcript_path: &str, + session_id: &str, + stopped_directory: &str, +) -> Result<()> { + let file = std::fs::File::open(transcript_path)?; + let mut reader = BufReader::new(file).take(MAX_CODEX_SESSION_META_BYTES + 1); + let mut first_line = String::new(); + let bytes = reader.read_line(&mut first_line)?; + if bytes == 0 || bytes as u64 > MAX_CODEX_SESSION_META_BYTES { + bail!("Codex transcript session metadata is missing or too large"); + } + let meta: serde_json::Value = serde_json::from_str(first_line.trim_end())?; + let payload = meta + .get("payload") + .ok_or_else(|| anyhow::anyhow!("Codex transcript has no session metadata payload"))?; + if meta.get("type").and_then(serde_json::Value::as_str) != Some("session_meta") + || payload.get("id").and_then(serde_json::Value::as_str) != Some(session_id) + || payload + .get("session_id") + .and_then(serde_json::Value::as_str) + != Some(session_id) + || payload + .get("originator") + .and_then(serde_json::Value::as_str) + != Some("Codex Desktop") + || payload.get("source").and_then(serde_json::Value::as_str) != Some("vscode") + || payload + .get("thread_source") + .and_then(serde_json::Value::as_str) + != Some("user") + || payload + .get("cwd") + .and_then(serde_json::Value::as_str) + .is_none_or(|cwd| !same_path(cwd, stopped_directory)) + { + bail!("Codex transcript does not describe this top-level Desktop task"); + } + for field in [ + "parent_thread_id", + "parent_session_id", + "source_thread_id", + "forked_from", + ] { + if payload + .get(field) + .is_some_and(|value| !value.is_null() && value.as_str().is_none_or(|s| !s.is_empty())) + { + bail!("Codex transcript describes a child or forked task"); + } + } + Ok(()) +} + #[derive(Debug, Clone, PartialEq, Eq)] struct RebindTargetMetadata { tool: String, @@ -566,6 +1055,7 @@ fn ensure_rebind_compatible( target_name: &str, meta: &RebindTargetMetadata, ctx: &HcomContext, + allow_directory_relocation: bool, ) -> Result<()> { let current_tool = ctx.tool.as_str(); if !meta.tool.is_empty() && meta.tool != current_tool { @@ -577,7 +1067,10 @@ fn ensure_rebind_compatible( } let current_dir = ctx.cwd.to_string_lossy(); - if !meta.directory.is_empty() && !same_path(&meta.directory, ¤t_dir) { + if !allow_directory_relocation + && !meta.directory.is_empty() + && !same_path(&meta.directory, ¤t_dir) + { bail!( "Refusing to reclaim '{target_name}': latest identity used directory '{}' but current session is '{}'", meta.directory, @@ -662,6 +1155,15 @@ fn resolve_claude_session_id(env: &HashMap) -> Option { .find_map(|key| env.get(key).filter(|value| !value.is_empty()).cloned()) } +/// Resolve the stable Codex task identity exposed to commands inside Desktop. +fn resolve_codex_session_id(ctx: &HcomContext) -> Option { + ctx.codex_thread_id + .as_deref() + .map(str::trim) + .filter(|id| !id.is_empty()) + .map(str::to_owned) +} + /// Live local Claude instances in this directory that no session id points at. /// /// These are the plausible earlier identities of a session that exposes no id @@ -744,18 +1246,22 @@ fn start_bare( } let tool = ctx.tool.as_str(); - let claude_session_id = (ctx.tool == crate::tool::Tool::Claude) - .then(|| resolve_claude_session_id(&ctx.raw_env)) - .flatten(); + let session_id = match ctx.tool { + crate::tool::Tool::Claude => resolve_claude_session_id(&ctx.raw_env), + crate::tool::Tool::Codex => resolve_codex_session_id(ctx), + _ => None, + }; if explicit_name.is_none() - && let Some(ref session_id) = claude_session_id + && let Some(ref session_id) = session_id && let Some(bound_name) = db.get_session_binding(session_id)? { // Only hcom writes session bindings, so a row keyed by this session's - // own id is trusted identity evidence. Heal bindings created by older - // versions before returning the existing row. - db.mark_claude_session_validated(session_id, &bound_name)?; + // own id is trusted identity evidence. Heal Claude bindings created by + // older versions before returning the existing row. + if ctx.tool == crate::tool::Tool::Claude { + db.mark_claude_session_validated(session_id, &bound_name)?; + } println!("hcom already started for {bound_name}"); return Ok(0); } @@ -788,7 +1294,7 @@ fn start_bare( instance_binding::initialize_instance_in_position_file( db, &name, - claude_session_id.as_deref(), + session_id.as_deref(), None, // parent_session_id None, // parent_name None, // agent_id @@ -802,9 +1308,11 @@ fn start_bare( None, // cwd_override ); - if let Some(ref session_id) = claude_session_id { + if let Some(ref session_id) = session_id { db.set_session_binding(session_id, &name)?; - db.mark_claude_session_validated(session_id, &name)?; + if ctx.tool == crate::tool::Tool::Claude { + db.mark_claude_session_validated(session_id, &name)?; + } } // Bind process if we have a process_id @@ -818,10 +1326,7 @@ fn start_bare( // recognize this session by, so a later `hcom start` here mints another // identity. Say what was just created and name the way back instead of // letting the duplicate appear silently. - if explicit_name.is_none() - && ctx.tool == crate::tool::Tool::Claude - && claude_session_id.is_none() - { + if explicit_name.is_none() && ctx.tool == crate::tool::Tool::Claude && session_id.is_none() { let candidates = unbound_claude_candidates(db, ctx, &name); eprintln!( "[hcom] warn: this Claude session exposes no session id, so it was registered \ @@ -909,6 +1414,50 @@ mod tests { HcomContext::from_env(&env, PathBuf::from(cwd)) } + fn make_codex_ctx(thread_id: Option<&str>, cwd: &str) -> HcomContext { + let mut env: HashMap = std::env::vars().collect(); + env.remove("HCOM_PROCESS_ID"); + env.remove("HCOM_LAUNCHED"); + env.remove("HCOM_PTY_MODE"); + env.remove("CODEX_SANDBOX"); + match thread_id { + Some(value) => { + env.insert("CODEX_THREAD_ID".to_string(), value.to_string()); + env.insert("CODEX_SESSION_ID".to_string(), value.to_string()); + } + None => { + env.remove("CODEX_THREAD_ID"); + env.remove("CODEX_SESSION_ID"); + } + } + HcomContext::from_env(&env, PathBuf::from(cwd)) + } + + fn write_codex_session_meta(home: &std::path::Path, session_id: &str, cwd: &str) -> String { + let codex_home = home.join(".codex"); + let sessions = codex_home + .join("sessions") + .join("2026") + .join("09") + .join("02"); + std::fs::create_dir_all(&sessions).unwrap(); + unsafe { std::env::set_var("CODEX_HOME", &codex_home) }; + let transcript = sessions.join(format!("rollout-test-{session_id}.jsonl")); + let meta = serde_json::json!({ + "type": "session_meta", + "payload": { + "id": session_id, + "session_id": session_id, + "cwd": cwd, + "originator": "Codex Desktop", + "source": "vscode", + "thread_source": "user" + } + }); + std::fs::write(&transcript, format!("{meta}\n")).unwrap(); + transcript.to_string_lossy().to_string() + } + fn log_stopped_snapshot( db: &HcomDb, name: &str, @@ -937,6 +1486,7 @@ mod tests { let args = StartArgs::try_parse_from(["start"]).unwrap(); assert!(args.orphan.is_none()); assert!(args.as_name.is_none()); + assert!(!args.relocate); } #[test] @@ -951,6 +1501,15 @@ mod tests { let args = StartArgs::try_parse_from(["start", "--as", "luna"]).unwrap(); assert!(args.orphan.is_none()); assert_eq!(args.as_name, Some("luna".to_string())); + assert!(!args.relocate); + } + + #[test] + fn test_start_args_explicit_relocation() { + let args = + StartArgs::try_parse_from(["start", "--as", "cultivation", "--relocate"]).unwrap(); + assert_eq!(args.as_name.as_deref(), Some("cultivation")); + assert!(args.relocate); } #[test] @@ -971,6 +1530,29 @@ mod tests { assert!(err.is_err()); } + #[test] + #[serial] + fn test_start_relocation_requires_as_and_rejects_orphan() { + let (_dir, _hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let flags = crate::router::GlobalFlags::default(); + let missing_as = run(&["start".into(), "--relocate".into()], &flags).unwrap_err(); + assert!(missing_as.to_string().contains("requires --as")); + + let conflict = run( + &[ + "start".into(), + "--as".into(), + "cultivation".into(), + "--relocate".into(), + "--orphan".into(), + "old".into(), + ], + &flags, + ) + .unwrap_err(); + assert!(conflict.to_string().contains("cannot be combined")); + } + #[test] #[serial] fn test_start_rejects_remote_instances() { @@ -1092,6 +1674,60 @@ mod tests { assert_eq!(resolve_claude_session_id(&env(&[])), None); } + #[test] + fn test_resolve_codex_session_id_trims_and_rejects_empty_values() { + assert_eq!( + resolve_codex_session_id(&make_codex_ctx(Some(" thread-1 "), "/tmp/project")), + Some("thread-1".to_string()) + ); + assert_eq!( + resolve_codex_session_id(&make_codex_ctx(Some(" "), "/tmp/project")), + None + ); + assert_eq!( + resolve_codex_session_id(&make_codex_ctx(None, "/tmp/project")), + None + ); + } + + #[test] + #[serial] + fn test_bare_codex_start_binds_and_reuses_thread_id() { + let (_dir, hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + assert!(crate::hooks::codex::setup_codex_hooks(false)); + let ctx = make_codex_ctx(Some("thread-bare-codex"), "/tmp/project"); + + assert_eq!(start_bare(&db, &hcom_dir, &ctx, None).unwrap(), 0); + let name = db + .get_session_binding("thread-bare-codex") + .unwrap() + .expect("the first bare start must bind the Desktop task"); + let row = db.get_instance_full(&name).unwrap().unwrap(); + assert_eq!(row.tool, "codex"); + assert_eq!(row.session_id.as_deref(), Some("thread-bare-codex")); + + assert_eq!(start_bare(&db, &hcom_dir, &ctx, None).unwrap(), 0); + assert_eq!( + db.get_session_binding("thread-bare-codex") + .unwrap() + .as_deref(), + Some(name.as_str()) + ); + let codex_rows: Vec = db + .iter_instances_full() + .unwrap() + .into_iter() + .filter(|row| row.tool == "codex") + .map(|row| row.name) + .collect(); + assert_eq!( + codex_rows, + vec![name], + "repeat start must not mint a duplicate" + ); + } + #[test] #[serial] fn test_vanilla_claude_start_reuses_claude_code_session_id() { @@ -1177,6 +1813,492 @@ mod tests { ); } + #[test] + #[serial] + fn test_codex_rebind_uses_thread_id_without_process_binding() { + let (_dir, _hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + db.conn() + .execute( + "INSERT INTO instances (name, tool, directory, status, created_at) VALUES (?1, 'codex', ?2, 'active', ?3)", + params!["desktop-old", "/tmp/project", crate::shared::time::now_epoch_f64()], + ) + .unwrap(); + let ctx = make_codex_ctx(Some("thread-desktop"), "/tmp/project"); + + assert_eq!(start_rebind(&db, "desktop-old", &ctx, None).unwrap(), 0); + let row = db.get_instance_full("desktop-old").unwrap().unwrap(); + assert_eq!(row.tool, "codex"); + assert_eq!(row.session_id.as_deref(), Some("thread-desktop")); + assert_eq!( + db.get_session_binding("thread-desktop").unwrap().as_deref(), + Some("desktop-old") + ); + } + + #[test] + #[serial] + fn test_codex_rebind_ignores_empty_thread_id() { + let (_dir, _hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + db.conn() + .execute( + "INSERT INTO instances (name, tool, directory, status, created_at) VALUES (?1, 'codex', ?2, 'active', ?3)", + params!["desktop-empty", "/tmp/project", crate::shared::time::now_epoch_f64()], + ) + .unwrap(); + let ctx = make_codex_ctx(Some(" "), "/tmp/project"); + + assert_eq!(start_rebind(&db, "desktop-empty", &ctx, None).unwrap(), 0); + let row = db.get_instance_full("desktop-empty").unwrap().unwrap(); + assert!(row.session_id.is_none()); + assert!(db.get_session_binding("").unwrap().is_none()); + } + + #[test] + #[serial] + fn test_codex_rebind_rejects_conflicting_process_session_before_mutation() { + let (_dir, _hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, directory, status, created_at) + VALUES ('current-codex', 'thread-stale', 'codex', '/tmp/project', 'active', 1)", + [], + ) + .unwrap(); + db.set_process_binding("process-current", "thread-stale", "current-codex") + .unwrap(); + log_stopped_snapshot( + &db, + "cultivation", + "codex", + "/tmp/project", + "thread-current", + 41, + ); + let mut ctx = make_codex_ctx(Some("thread-current"), "/tmp/project"); + ctx.process_id = Some("process-current".to_string()); + + let err = start_rebind(&db, "cultivation", &ctx, None).unwrap_err(); + assert!(err.to_string().contains("conflicts with existing session")); + assert!(db.get_instance_full("cultivation").unwrap().is_none()); + assert!(db.get_instance_full("current-codex").unwrap().is_some()); + assert_eq!( + db.get_process_binding_full("process-current") + .unwrap() + .unwrap() + .0 + .as_deref(), + Some("thread-stale") + ); + } + + #[test] + #[serial] + fn test_codex_rebind_rejects_conflicting_explicit_row_session_before_mutation() { + let (_dir, _hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, directory, status, created_at) + VALUES ('current-codex', 'thread-stale', 'codex', '/tmp/project', 'active', 1)", + [], + ) + .unwrap(); + log_stopped_snapshot( + &db, + "cultivation", + "codex", + "/tmp/project", + "thread-current", + 42, + ); + let ctx = make_codex_ctx(Some("thread-current"), "/tmp/project"); + + let err = start_rebind(&db, "cultivation", &ctx, Some("current-codex")).unwrap_err(); + assert!(err.to_string().contains("conflicts with existing session")); + assert!(db.get_instance_full("cultivation").unwrap().is_none()); + let current = db.get_instance_full("current-codex").unwrap().unwrap(); + assert_eq!(current.session_id.as_deref(), Some("thread-stale")); + } + + #[test] + #[serial] + fn test_codex_cli_rebind_still_rejects_cross_directory_for_same_env_session() { + let (_dir, _hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + log_stopped_snapshot( + &db, + "cultivation", + "codex", + "/tmp/old-project", + "thread-cultivation", + 43, + ); + let ctx = make_codex_ctx(Some("thread-cultivation"), "/tmp/cultivation"); + + let err = start_rebind(&db, "cultivation", &ctx, None).unwrap_err(); + assert!( + err.to_string() + .contains("Refusing to reclaim 'cultivation'") + ); + assert!(db.get_instance_full("cultivation").unwrap().is_none()); + assert!( + db.get_session_binding("thread-cultivation") + .unwrap() + .is_none() + ); + } + + #[test] + #[serial] + fn test_codex_explicit_relocation_uses_verified_transcript_and_persists_pin() { + let (tmp, _hcom_dir, home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-cultivation-relocate"; + let old_directory = tmp.path().join("old-scaffold"); + let new_directory = tmp.path().join("cultivation"); + std::fs::create_dir_all(&old_directory).unwrap(); + std::fs::create_dir_all(&new_directory).unwrap(); + let old_directory = old_directory.to_string_lossy().to_string(); + let new_directory = new_directory.to_string_lossy().to_string(); + let transcript = write_codex_session_meta(&home, session_id, &old_directory); + db.log_event( + "life", + "cultivation", + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "parent_name": null, + "parent_session_id": null, + "agent_id": null, + "origin_device_id": null, + "last_event_id": 73 + } + }), + ) + .unwrap(); + let ctx = make_codex_ctx(Some(session_id), &new_directory); + + assert_eq!( + start_rebind_with_options(&db, "cultivation", &ctx, None, true).unwrap(), + 0 + ); + let row = db.get_instance_full("cultivation").unwrap().unwrap(); + assert_eq!(row.session_id.as_deref(), Some(session_id)); + assert_eq!(row.last_event_id, 73); + assert!(same_path(&row.directory, &new_directory)); + assert!(same_path(&row.transcript_path, &transcript)); + assert_eq!( + crate::hooks::codex::pinned_directory_from_launch_context( + row.launch_context.as_deref(), + session_id, + ) + .as_deref(), + Some(new_directory.as_str()) + ); + assert_eq!( + db.get_session_binding(session_id).unwrap().as_deref(), + Some("cultivation") + ); + } + + #[test] + #[serial] + fn test_codex_relocation_binding_failure_rolls_back_instance() { + let (tmp, _hcom_dir, home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-binding-rollback"; + let old_directory = tmp.path().join("old-scaffold"); + let new_directory = tmp.path().join("cultivation"); + std::fs::create_dir_all(&old_directory).unwrap(); + std::fs::create_dir_all(&new_directory).unwrap(); + let old_directory = old_directory.to_string_lossy().to_string(); + let new_directory = new_directory.to_string_lossy().to_string(); + let transcript = write_codex_session_meta(&home, session_id, &old_directory); + db.log_event( + "life", + "cultivation", + &json!({ + "action": "stopped", + "snapshot": { + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "last_event_id": 31 + } + }), + ) + .unwrap(); + let ctx = make_codex_ctx(Some(session_id), &new_directory); + let proof = + validate_codex_relocation(&db, "cultivation", &ctx, Some(session_id), "").unwrap(); + db.conn() + .execute_batch( + "CREATE TRIGGER deny_relocation_binding + BEFORE INSERT ON session_bindings + BEGIN SELECT RAISE(ABORT, 'forced binding failure'); END;", + ) + .unwrap(); + + let error = commit_codex_relocation(&db, "cultivation", &ctx, &proof).unwrap_err(); + assert!(error.to_string().contains("forced binding failure")); + assert!(db.get_instance_full("cultivation").unwrap().is_none()); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + } + + #[test] + #[serial] + fn test_codex_relocation_preserves_target_winner_after_proof() { + let (tmp, _hcom_dir, home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-target-race"; + let old_directory = tmp.path().join("old-scaffold"); + let new_directory = tmp.path().join("cultivation"); + let winner_directory = tmp.path().join("winner"); + std::fs::create_dir_all(&old_directory).unwrap(); + std::fs::create_dir_all(&new_directory).unwrap(); + std::fs::create_dir_all(&winner_directory).unwrap(); + let old_directory = old_directory.to_string_lossy().to_string(); + let new_directory = new_directory.to_string_lossy().to_string(); + let winner_directory = winner_directory.to_string_lossy().to_string(); + let transcript = write_codex_session_meta(&home, session_id, &old_directory); + db.log_event( + "life", + "cultivation", + &json!({ + "action": "stopped", + "snapshot": { + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "last_event_id": 32 + } + }), + ) + .unwrap(); + let ctx = make_codex_ctx(Some(session_id), &new_directory); + let proof = + validate_codex_relocation(&db, "cultivation", &ctx, Some(session_id), "").unwrap(); + + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, directory, status, created_at) + VALUES ('cultivation', 'thread-winner', 'codex', ?1, 'active', 1)", + params![winner_directory], + ) + .unwrap(); + db.set_session_binding("thread-winner", "cultivation") + .unwrap(); + + let error = commit_codex_relocation(&db, "cultivation", &ctx, &proof).unwrap_err(); + assert!(error.to_string().contains("another task registered")); + let winner = db.get_instance_full("cultivation").unwrap().unwrap(); + assert_eq!(winner.session_id.as_deref(), Some("thread-winner")); + assert_eq!(winner.directory, winner_directory); + assert_eq!( + db.get_session_binding("thread-winner").unwrap().as_deref(), + Some("cultivation") + ); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + } + + #[test] + #[serial] + fn test_codex_relocation_preserves_session_winner_after_proof() { + let (tmp, _hcom_dir, home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-session-race"; + let old_directory = tmp.path().join("old-scaffold"); + let new_directory = tmp.path().join("cultivation"); + std::fs::create_dir_all(&old_directory).unwrap(); + std::fs::create_dir_all(&new_directory).unwrap(); + let old_directory = old_directory.to_string_lossy().to_string(); + let new_directory = new_directory.to_string_lossy().to_string(); + let transcript = write_codex_session_meta(&home, session_id, &old_directory); + db.log_event( + "life", + "cultivation", + &json!({ + "action": "stopped", + "snapshot": { + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "last_event_id": 33 + } + }), + ) + .unwrap(); + let ctx = make_codex_ctx(Some(session_id), &new_directory); + let proof = + validate_codex_relocation(&db, "cultivation", &ctx, Some(session_id), "").unwrap(); + + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, directory, status, created_at) + VALUES ('session-winner', ?1, 'codex', ?2, 'active', 1)", + params![session_id, new_directory], + ) + .unwrap(); + db.set_session_binding(session_id, "session-winner") + .unwrap(); + + let error = commit_codex_relocation(&db, "cultivation", &ctx, &proof).unwrap_err(); + assert!(error.to_string().contains("session is already bound")); + assert!(db.get_instance_full("cultivation").unwrap().is_none()); + assert_eq!( + db.get_session_binding(session_id).unwrap().as_deref(), + Some("session-winner") + ); + } + + #[test] + #[serial] + fn test_codex_relocation_rejects_malformed_snapshot_identity_fields() { + let (tmp, _hcom_dir, home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + let cases = [ + ("origin_device_id", json!(7)), + ("parent_name", json!({"unexpected": true})), + ("agent_id", json!(["unexpected"])), + ("background", json!(true)), + ("background", json!(1)), + ]; + + for (index, (field, bad_value)) in cases.into_iter().enumerate() { + let bad_value_display = bad_value.to_string(); + let name = format!("cultivation-{index}"); + let session_id = format!("thread-malformed-{index}"); + let old_directory = tmp.path().join(format!("old-{index}")); + let new_directory = tmp.path().join(format!("new-{index}")); + std::fs::create_dir_all(&old_directory).unwrap(); + std::fs::create_dir_all(&new_directory).unwrap(); + let old_directory = old_directory.to_string_lossy().to_string(); + let new_directory = new_directory.to_string_lossy().to_string(); + let transcript = write_codex_session_meta(&home, &session_id, &old_directory); + let mut snapshot = json!({ + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "last_event_id": 34 + }); + snapshot + .as_object_mut() + .unwrap() + .insert(field.to_string(), bad_value); + db.log_event( + "life", + &name, + &json!({"action": "stopped", "snapshot": snapshot}), + ) + .unwrap(); + let ctx = make_codex_ctx(Some(&session_id), &new_directory); + + let error = validate_codex_relocation(&db, &name, &ctx, Some(session_id.as_str()), "") + .unwrap_err(); + assert!( + error.to_string().contains("stopped identity"), + "{field}={bad_value_display} unexpectedly produced: {error}" + ); + assert!(db.get_instance_full(&name).unwrap().is_none()); + assert!(db.get_session_binding(&session_id).unwrap().is_none()); + } + } + + #[test] + #[serial] + fn test_codex_relocation_rejects_mismatched_session_env_without_mutation() { + let (tmp, _hcom_dir, home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-owned"; + let old_directory = tmp.path().join("old-scaffold"); + let new_directory = tmp.path().join("cultivation"); + std::fs::create_dir_all(&old_directory).unwrap(); + std::fs::create_dir_all(&new_directory).unwrap(); + let old_directory = old_directory.to_string_lossy().to_string(); + let new_directory = new_directory.to_string_lossy().to_string(); + let transcript = write_codex_session_meta(&home, session_id, &old_directory); + db.log_event( + "life", + "cultivation", + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "origin_device_id": null, + "last_event_id": 73 + } + }), + ) + .unwrap(); + let mut ctx = make_codex_ctx(Some(session_id), &new_directory); + ctx.raw_env + .insert("CODEX_SESSION_ID".into(), "thread-foreign".into()); + + let error = start_rebind_with_options(&db, "cultivation", &ctx, None, true).unwrap_err(); + assert!( + error + .to_string() + .contains("conflicts with CODEX_SESSION_ID") + ); + assert!(db.get_instance_full("cultivation").unwrap().is_none()); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + } + + #[test] + #[serial] + fn test_codex_relocation_rejects_remote_snapshot_without_mutation() { + let (tmp, _hcom_dir, home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-remote"; + let old_directory = tmp.path().join("old-scaffold"); + let new_directory = tmp.path().join("cultivation"); + std::fs::create_dir_all(&old_directory).unwrap(); + std::fs::create_dir_all(&new_directory).unwrap(); + let old_directory = old_directory.to_string_lossy().to_string(); + let new_directory = new_directory.to_string_lossy().to_string(); + let transcript = write_codex_session_meta(&home, session_id, &old_directory); + db.log_event( + "life", + "cultivation", + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "origin_device_id": "remote-device", + "last_event_id": 73 + } + }), + ) + .unwrap(); + let ctx = make_codex_ctx(Some(session_id), &new_directory); + + let error = start_rebind_with_options(&db, "cultivation", &ctx, None, true).unwrap_err(); + assert!(error.to_string().contains("stopped identity is remote")); + assert!(db.get_instance_full("cultivation").unwrap().is_none()); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + } + #[test] #[serial] fn test_unidentifiable_claude_start_lists_unbound_candidates() { diff --git a/src/hooks/claude.rs b/src/hooks/claude.rs index 4dd492e4..02b848fc 100644 --- a/src/hooks/claude.rs +++ b/src/hooks/claude.rs @@ -1877,7 +1877,12 @@ fn handle_poll( let result = common::poll_messages(db, instance_name, timeout as u64, ctx.is_background); - if result.timed_out { + let keep_hook_boundary_listening = db + .get_instance_full(instance_name) + .ok() + .flatten() + .is_some_and(|data| crate::instance_lifecycle::is_hook_only_claude_session(&data, db)); + if result.timed_out && !keep_hook_boundary_listening { lifecycle::set_status( db, instance_name, @@ -2811,6 +2816,12 @@ const CLAUDE_HOOK_TYPES: &[&str] = &[ "SessionEnd", ]; +/// Marker embedded in Windows hook commands that pin the native hcom binary. +/// +/// Besides making the installed command self-describing, this gives cleanup a +/// stable signature even when the executable path or filename changes. +const CLAUDE_MANAGED_HOOK_MARKER: &str = "HCOM_HOOK_MANAGED=1"; + // Static regexes for hot-path hook command detection static RE_HCOM_COMMANDS: LazyLock = LazyLock::new(|| { let pattern = CLAUDE_HOOK_COMMANDS.join("|"); @@ -2854,19 +2865,58 @@ pub fn load_claude_settings(settings_path: &Path) -> Option { serde_json::from_str(&content).ok() } +/// Quote one argument for the POSIX shell Claude uses to execute hooks. +#[cfg(any(windows, test))] +fn quote_posix_arg(arg: &str) -> String { + format!("'{}'", arg.replace('\'', "'\"'\"'")) +} + +/// Build a managed hook command from an already-resolved argv prefix. +/// +/// `set --` plus `exec "$@"` preserves every argument boundary, including a +/// native Windows executable path containing spaces or apostrophes. +#[cfg(any(windows, test))] +fn build_pinned_hook_entry_command(prefix: &[String], cmd_suffix: &str) -> String { + let prefix = if prefix.is_empty() { + vec!["hcom".to_string()] + } else { + prefix.to_vec() + }; + let argv = prefix + .iter() + .map(|arg| quote_posix_arg(arg)) + .collect::>() + .join(" "); + let suffix = quote_posix_arg(cmd_suffix); + format!( + "{CLAUDE_MANAGED_HOOK_MARKER}; set -- {argv}; command -v \"$1\" >/dev/null 2>&1 && exec \"$@\" {suffix} || exit 0" + ) +} + +/// Resolve the exact native Windows binary that installed the Claude hooks. +/// +/// Pinning this path prevents a long-running Claude Desktop process from using +/// an older `hcom.exe` inherited earlier in its PATH. Forward slashes keep the +/// path valid in Git Bash, which Claude uses for command hooks on Windows. +#[cfg(windows)] +fn windows_hook_hcom_prefix() -> Option> { + crate::runtime_env::windows_current_hcom_executable().map(|exe| vec![exe]) +} + /// Build a hook command that silently exits 0 when hcom is not installed. /// -/// Claude already executes hook commands through a shell, so this command keeps -/// all shell logic inline instead of spawning another `sh -c`. It uses the -/// ${HCOM:-hcom} env var (set in settings.json env block) so it works for both -/// direct `hcom` and `uvx hcom` invocations. When the binary is absent (e.g. -/// after `brew uninstall hcom`), the hook exits 0 instead of emitting a "command -/// not found" error inside the tool. +/// Windows pins the exact native executable that performed installation so a +/// stale inherited PATH cannot select another copy. Other platforms retain the +/// existing `${HCOM:-hcom}` behavior, including `uvx hcom` support. fn build_hook_entry_command(cmd_suffix: &str) -> String { - // Claude runs hook commands through a POSIX shell on every platform - // (Git Bash on Windows), so the same command works everywhere. The - // `${HCOM:-hcom}` default plus the `command -v` guard make it silently - // exit 0 when hcom isn't on PATH. + #[cfg(windows)] + if let Some(prefix) = windows_hook_hcom_prefix() { + return build_pinned_hook_entry_command(&prefix, cmd_suffix); + } + + // Claude runs hook commands through a POSIX shell on every platform. The + // `${HCOM:-hcom}` default plus the `command -v` guard make it silently exit + // 0 when hcom isn't on PATH. format!( "cmd=${{HCOM:-hcom}}; command -v \"${{cmd%% *}}\" >/dev/null 2>&1 && exec $cmd {} || exit 0", cmd_suffix @@ -2944,6 +2994,12 @@ fn build_all_claude_permission_patterns() -> Vec { /// Check if a hook command string matches any hcom hook pattern. fn is_hcom_hook_command(command: &str) -> bool { + // Current native Windows hook format. Match a deliberate marker rather + // than a path spelling so cleanup remains reliable after relocation. + if command.contains(CLAUDE_MANAGED_HOOK_MARKER) { + return true; + } + // Env var patterns: ${HCOM} or %HCOM% if command.contains("${HCOM}") || command.contains("$HCOM") @@ -2985,6 +3041,41 @@ fn is_hcom_hook_command(command: &str) -> bool { false } +/// Capture numeric timeout overrides from the currently installed hcom hooks. +/// +/// Reinstalling hooks is also the repair path, so malformed or missing timeout +/// values deliberately fall back to the canonical defaults. Valid numeric +/// values are user configuration and must survive a clean-slate reinstall. +fn installed_hcom_hook_timeouts(settings: &Value) -> std::collections::HashMap { + let mut timeouts = std::collections::HashMap::new(); + let Some(hooks) = settings.get("hooks").and_then(Value::as_object) else { + return timeouts; + }; + + for &(hook_type, _, cmd_suffix, _) in CLAUDE_HOOK_CONFIGS { + let Some(matchers) = hooks.get(hook_type).and_then(Value::as_array) else { + continue; + }; + 'matchers: for matcher in matchers { + let Some(entries) = matcher.get("hooks").and_then(Value::as_array) else { + continue; + }; + for entry in entries { + let command = entry.get("command").and_then(Value::as_str).unwrap_or(""); + if is_hcom_hook_command(command) + && command.contains(cmd_suffix) + && let Some(timeout) = entry.get("timeout").and_then(Value::as_u64) + { + timeouts.insert(hook_type.to_string(), timeout); + break 'matchers; + } + } + } + } + + timeouts +} + /// Remove all hcom hooks from a Claude settings dictionary (in-place). /// /// Scans all hook types and removes hooks whose command matches hcom patterns. @@ -3119,6 +3210,13 @@ pub enum VerifyFailReason { hook_type: String, cmd_suffix: String, }, + #[error( + "hcom hook command '{cmd_suffix}' under hook type '{hook_type}' is not the current pinned command" + )] + HookCommandNotPinned { + hook_type: String, + cmd_suffix: String, + }, #[error("hook type '{hook_type}' matcher mismatch: expected {expected:?}, got {actual:?}")] HookMatcherMismatch { hook_type: String, @@ -3178,6 +3276,8 @@ pub fn try_setup_claude_hooks(include_permissions: bool) -> Result<(), SetupErro settings["hooks"] = serde_json::json!({}); } + let preserved_timeouts = installed_hcom_hook_timeouts(&settings); + // Remove existing hcom hooks remove_hcom_hooks_from_settings(&mut settings); @@ -3195,6 +3295,7 @@ pub fn try_setup_claude_hooks(include_permissions: bool) -> Result<(), SetupErro "command": build_hook_entry_command(cmd_suffix), }); + let timeout = preserved_timeouts.get(hook_type).copied().or(timeout); if let Some(t) = timeout { hook_entry["timeout"] = serde_json::json!(t); } @@ -3332,9 +3433,7 @@ fn verify_claude_hooks_inner( for hook in hooks_list { let command = hook.get("command").and_then(|v| v.as_str()).unwrap_or(""); - let has_hcom = - command.contains("${HCOM}") || command.to_lowercase().contains("hcom"); - if has_hcom && command.contains(cmd_suffix) { + if is_hcom_hook_command(command) && command.contains(cmd_suffix) { if hcom_hook_found { return Err(VerifyFailReason::HookDuplicated(hook_type.to_string())); } @@ -3347,6 +3446,19 @@ fn verify_claude_hooks_inner( }); } + // On Windows the hook must invoke the exact executable + // that installed it. Accepting ${HCOM:-hcom} or a bare + // `hcom` here makes the launcher report "already + // installed" while leaving a stale PATH-selected copy + // active after an upgrade. + #[cfg(windows)] + if command != build_hook_entry_command(cmd_suffix) { + return Err(VerifyFailReason::HookCommandNotPinned { + hook_type: hook_type.to_string(), + cmd_suffix: cmd_suffix.to_string(), + }); + } + let actual_matcher = matcher_obj .get("matcher") .and_then(|v| v.as_str()) @@ -4195,13 +4307,58 @@ mod tests { #[test] fn test_build_hook_entry_command_avoids_nested_shell() { let command = build_hook_entry_command("poll"); + #[cfg(not(windows))] assert_eq!( command, "cmd=${HCOM:-hcom}; command -v \"${cmd%% *}\" >/dev/null 2>&1 && exec $cmd poll || exit 0" ); + #[cfg(windows)] + { + assert!(command.starts_with(CLAUDE_MANAGED_HOOK_MARKER)); + assert!(command.contains("command -v \"$1\"")); + assert!(command.contains("exec \"$@\" 'poll'")); + } assert!(!command.starts_with("sh -c")); } + #[test] + fn test_pinned_hook_entry_command_quotes_windows_path_and_argv() { + let command = build_pinned_hook_entry_command( + &[ + "C:/Users/O'Neil/My Tools/hcom.exe".to_string(), + "--fixed argument".to_string(), + ], + "poll", + ); + assert_eq!( + command, + concat!( + "HCOM_HOOK_MANAGED=1; set -- ", + "'C:/Users/O'\"'\"'Neil/My Tools/hcom.exe' '--fixed argument'; ", + "command -v \"$1\" >/dev/null 2>&1 && exec \"$@\" 'poll' || exit 0" + ) + ); + } + + #[test] + fn test_managed_pinned_hook_is_recognized() { + let command = build_pinned_hook_entry_command( + &["C:/Program Files/hcom/hcom.exe".to_string()], + "sessionstart", + ); + assert!(is_hcom_hook_command(&command)); + } + + #[test] + #[cfg(windows)] + fn test_windows_hook_prefix_pins_current_native_executable() { + let prefix = windows_hook_hcom_prefix().expect("current executable should resolve"); + assert_eq!(prefix.len(), 1); + assert!(prefix[0].ends_with(".exe")); + assert!(!prefix[0].contains('\\')); + assert!(std::path::Path::new(&prefix[0]).is_absolute()); + } + #[test] fn test_remove_hcom_hooks_empty() { let mut settings = serde_json::json!({}); @@ -4236,11 +4393,16 @@ mod tests { #[test] fn test_remove_hcom_hooks_preserves_non_hcom() { + let managed = build_pinned_hook_entry_command( + &["C:/Program Files/hcom/hcom.exe".to_string()], + "post", + ); let mut settings = serde_json::json!({ "hooks": { "PostToolUse": [{ "hooks": [ {"type": "command", "command": "${HCOM} post"}, + {"type": "command", "command": managed}, {"type": "command", "command": "echo custom hook"}, ] }] @@ -4370,13 +4532,20 @@ mod tests { // Can't call setup_claude_hooks directly (uses get_claude_settings_path), // but we can test the verify path with a hand-built settings file. + #[cfg(windows)] + let hook_cmd = ""; + #[cfg(not(windows))] let hook_cmd = "${HCOM}"; let mut settings = serde_json::json!({"hooks": {}, "env": {"HCOM": "hcom"}}); for &(hook_type, matcher, cmd_suffix, timeout) in CLAUDE_HOOK_CONFIGS { let mut hook_entry = serde_json::json!({ "type": "command", - "command": format!("{} {}", hook_cmd, cmd_suffix), + "command": if hook_cmd.is_empty() { + build_hook_entry_command(cmd_suffix) + } else { + format!("{} {}", hook_cmd, cmd_suffix) + }, }); if let Some(t) = timeout { hook_entry["timeout"] = serde_json::json!(t); @@ -4401,6 +4570,43 @@ mod tests { assert!(verify_claude_hooks_installed(Some(&settings_path), false,)); } + #[test] + fn test_verify_accepts_managed_pinned_hook_commands() { + crate::config::Config::init(); + let dir = tempfile::tempdir().unwrap(); + let settings_path = dir.path().join("settings.json"); + #[cfg(windows)] + let prefix = crate::runtime_env::windows_current_hcom_executable() + .map(|path| vec![path]) + .unwrap(); + #[cfg(not(windows))] + let prefix = vec!["C:/Program Files/hcom/hcom.exe".to_string()]; + let mut settings = serde_json::json!({"hooks": {}, "env": {"HCOM": "hcom"}}); + + for &(hook_type, matcher, cmd_suffix, timeout) in CLAUDE_HOOK_CONFIGS { + let mut hook_entry = serde_json::json!({ + "type": "command", + "command": build_pinned_hook_entry_command(&prefix, cmd_suffix), + }); + if let Some(t) = timeout { + hook_entry["timeout"] = serde_json::json!(t); + } + let mut hook_dict = serde_json::json!({"hooks": [hook_entry]}); + if !matcher.is_empty() { + hook_dict["matcher"] = Value::String(matcher.to_string()); + } + settings["hooks"][hook_type] = serde_json::json!([hook_dict]); + } + + std::fs::write( + &settings_path, + serde_json::to_string_pretty(&settings).unwrap(), + ) + .unwrap(); + + assert!(verify_claude_hooks_installed(Some(&settings_path), false)); + } + #[test] fn test_verify_missing_file() { crate::config::Config::init(); @@ -4434,13 +4640,20 @@ mod tests { new_timeout: Option, include_permissions: bool, ) { + #[cfg(windows)] + let hook_cmd = ""; + #[cfg(not(windows))] let hook_cmd = "${HCOM}"; let mut settings = serde_json::json!({"hooks": {}, "env": {"HCOM": "hcom"}}); for &(hook_type, matcher, cmd_suffix, timeout) in CLAUDE_HOOK_CONFIGS { let mut hook_entry = serde_json::json!({ "type": "command", - "command": format!("{} {}", hook_cmd, cmd_suffix), + "command": if hook_cmd.is_empty() { + build_hook_entry_command(cmd_suffix) + } else { + format!("{} {}", hook_cmd, cmd_suffix) + }, }); if timeout.is_some() && let Some(t) = new_timeout @@ -4831,6 +5044,150 @@ mod tests { drop(_guard); } + #[test] + #[cfg(windows)] + #[serial] + fn test_setup_claude_upgrades_legacy_commands_to_current_pinned_path() { + let (_dir, _test_home, settings_path, _guard) = claude_test_env(); + std::fs::create_dir_all(settings_path.parent().unwrap()).unwrap(); + + // This is the configuration produced by older hcom builds. Include + // an unrelated hook to prove repair does not remove foreign entries. + let mut settings = serde_json::json!({ + "env": {"HCOM": "hcom"}, + "hooks": { + "PostToolUse": [{"hooks": [{ + "type": "command", + "command": "${HCOM:-hcom} post", + "timeout": 5 + }, { + "type": "command", + "command": "echo keep-me" + }]}] + } + }); + std::fs::write( + &settings_path, + serde_json::to_string_pretty(&settings).unwrap(), + ) + .unwrap(); + + assert!(!verify_claude_hooks_installed(Some(&settings_path), false)); + assert!(setup_claude_hooks(false)); + settings = read_json(&settings_path); + + for &(hook_type, _, cmd_suffix, _) in CLAUDE_HOOK_CONFIGS { + let expected = build_hook_entry_command(cmd_suffix); + let commands: Vec<&str> = settings["hooks"][hook_type] + .as_array() + .unwrap() + .iter() + .flat_map(|matcher| matcher["hooks"].as_array().unwrap()) + .filter_map(|hook| hook["command"].as_str()) + .collect(); + assert!( + commands.iter().any(|command| *command == expected), + "{hook_type} was not pinned" + ); + assert!( + commands + .iter() + .all(|command| !command.contains("${HCOM") && !command.eq(&"hcom")) + ); + } + + let post = &settings["hooks"]["PostToolUse"]; + let post_hooks: Vec<&Value> = post + .as_array() + .unwrap() + .iter() + .flat_map(|matcher| matcher["hooks"].as_array().unwrap()) + .collect(); + assert!( + post_hooks + .iter() + .any(|hook| hook["command"] == "echo keep-me") + ); + assert_eq!( + post_hooks + .iter() + .find(|hook| hook["command"] == build_hook_entry_command("post")) + .and_then(|hook| hook["timeout"].as_u64()), + Some(5) + ); + + let first = std::fs::read_to_string(&settings_path).unwrap(); + assert!(setup_claude_hooks(false)); + assert_eq!(first, std::fs::read_to_string(&settings_path).unwrap()); + } + + #[test] + #[serial] + fn test_setup_claude_preserves_existing_numeric_hook_timeouts() { + let (_dir, _test_home, settings_path, _guard) = claude_test_env(); + std::fs::create_dir_all(settings_path.parent().unwrap()).unwrap(); + let settings = serde_json::json!({ + "env": {"MY_VAR": "preserved"}, + "hooks": { + "SessionStart": [{ + "hooks": [{ + "type": "command", + "command": "${HCOM} sessionstart", + "timeout": 7, + }] + }], + "PostToolUse": [{ + "hooks": [{ + "type": "command", + "command": "${HCOM} post", + "timeout": 5, + }] + }], + "Stop": [{ + "hooks": [{ + "type": "command", + "command": "${HCOM} poll", + "timeout": 5, + }] + }], + "SubagentStop": [{ + "hooks": [{ + "type": "command", + "command": "${HCOM} subagent-stop", + "timeout": 5, + }] + }], + } + }); + std::fs::write( + &settings_path, + serde_json::to_string_pretty(&settings).unwrap(), + ) + .unwrap(); + + assert!(setup_claude_hooks(false)); + + let updated = read_json(&settings_path); + for (hook_type, expected) in [ + ("SessionStart", 7), + ("PostToolUse", 5), + ("Stop", 5), + ("SubagentStop", 5), + ] { + let timeout = updated["hooks"][hook_type] + .as_array() + .and_then(|matchers| matchers.first()) + .and_then(|matcher| matcher["hooks"].as_array()) + .and_then(|hooks| hooks.first()) + .and_then(|hook| hook["timeout"].as_u64()); + assert_eq!(timeout, Some(expected), "{hook_type} timeout changed"); + } + assert_eq!(updated["env"]["MY_VAR"], "preserved"); + assert!(verify_claude_hooks_installed(Some(&settings_path), false)); + + drop(_guard); + } + #[test] #[serial] fn test_remove_claude_only_removes_hcom() { @@ -5064,6 +5421,34 @@ mod tests { .unwrap(); } + #[test] + #[serial] + fn hook_only_stop_poll_timeout_remains_listening() { + crate::config::Config::init(); + let (_dir, _guard, db) = make_isolated_test_db(); + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, status, status_context, status_time, + created_at, wait_timeout) + VALUES ('risa', 'sess-risa', 'claude', 'active', 'prompt', 0, 1, 0)", + [], + ) + .unwrap(); + db.set_session_binding("sess-risa", "risa").unwrap(); + let instance = db.get_instance_full("risa").unwrap().unwrap(); + + let (code, stdout, ack) = handle_poll(&db, &make_ctx(), "risa", &instance); + + assert_eq!(code, 0); + assert!(stdout.is_empty()); + assert!(ack.is_none()); + let after = db.get_instance_full("risa").unwrap().unwrap(); + assert_eq!(after.status, ST_LISTENING); + assert!(after.status_context.is_empty()); + assert!(db.has_session_binding("risa")); + } + #[test] #[serial] fn test_root_bash_pretooluse_does_not_inject_actor_capability() { diff --git a/src/hooks/codex.rs b/src/hooks/codex.rs index b02341dd..1016d951 100644 --- a/src/hooks/codex.rs +++ b/src/hooks/codex.rs @@ -32,6 +32,80 @@ use crate::shared::{ST_ACTIVE, ST_LISTENING}; use super::common::SAFE_HCOM_COMMANDS; const HCOM_TRIGGER: &str = ""; +const CODEX_DIRECTORY_PIN_KEY: &str = "codex_directory_pin_v1"; +const CODEX_DIRECTORY_PIN_SOURCE: &str = "explicit-start-relocate-v1"; + +pub(crate) fn directory_override_launch_context(directory: &str, session_id: &str) -> String { + serde_json::json!({ + CODEX_DIRECTORY_PIN_KEY: { + "directory": directory, + "session_id": session_id, + "source": CODEX_DIRECTORY_PIN_SOURCE, + } + }) + .to_string() +} + +fn pinned_directory_from_value(value: &Value, session_id: &str) -> Option { + let pin = value.as_object()?; + let directory = pin.get("directory")?.as_str()?.trim(); + let pinned_session = pin.get("session_id")?.as_str()?.trim(); + let source = pin.get("source")?.as_str()?; + if directory.is_empty() + || !Path::new(directory).is_absolute() + || session_id.is_empty() + || pinned_session != session_id + || source != CODEX_DIRECTORY_PIN_SOURCE + { + return None; + } + Some(directory.to_string()) +} + +fn pinned_directory_is_live(directory: &str) -> bool { + std::fs::symlink_metadata(directory) + .ok() + .is_some_and(|metadata| metadata.is_dir() && !metadata.file_type().is_symlink()) +} + +fn launch_context_has_directory_pin(launch_context: Option<&str>) -> bool { + launch_context + .and_then(|raw| serde_json::from_str::(raw).ok()) + .is_some_and(|context| context.get(CODEX_DIRECTORY_PIN_KEY).is_some()) +} + +pub(crate) fn pinned_directory_from_launch_context( + launch_context: Option<&str>, + session_id: &str, +) -> Option { + let context: Value = serde_json::from_str(launch_context?).ok()?; + pinned_directory_from_value(context.get(CODEX_DIRECTORY_PIN_KEY)?, session_id) +} + +fn pinned_directory_from_snapshot(snapshot: &Value, session_id: &str) -> Option { + pinned_directory_from_value(snapshot.get(CODEX_DIRECTORY_PIN_KEY)?, session_id) +} + +pub(crate) fn directory_override_snapshot_value( + launch_context: Option<&str>, + session_id: Option<&str>, +) -> Value { + let Some(session_id) = session_id.filter(|value| !value.is_empty()) else { + return Value::Null; + }; + let Some(context) = launch_context.and_then(|raw| serde_json::from_str::(raw).ok()) + else { + return Value::Null; + }; + let Some(pin) = context.get(CODEX_DIRECTORY_PIN_KEY) else { + return Value::Null; + }; + if pinned_directory_from_value(pin, session_id).is_none() { + return Value::Null; + } + pin.clone() +} + const CODEX_HOOK_COMMANDS: &[(&str, &str, Option<&str>)] = &[ ( "SessionStart", @@ -305,13 +379,43 @@ fn update_codex_position( instance_name: &str, ) { let mut updates = serde_json::Map::new(); - let cwd = payload - .raw - .get("cwd") - .and_then(|v| v.as_str()) - .map(|s| s.to_string()) - .unwrap_or_else(|| ctx.cwd.to_string_lossy().to_string()); - if !cwd.is_empty() { + let session_id = payload.session_id.as_deref().unwrap_or(""); + let launch_context = db + .get_instance_full(instance_name) + .ok() + .flatten() + .and_then(|row| row.launch_context); + let pin_present = launch_context_has_directory_pin(launch_context.as_deref()); + let pinned_directory = + pinned_directory_from_launch_context(launch_context.as_deref(), session_id); + let cwd = match pinned_directory { + Some(pinned) if pinned_directory_is_live(&pinned) => Some(pinned), + Some(_) => { + log::log_warn( + "hooks", + "codex.directory_pin_invalid", + &format!("instance={instance_name} session={session_id} directory_mutated=false"), + ); + None + } + None if pin_present => { + log::log_warn( + "hooks", + "codex.directory_pin_invalid", + &format!("instance={instance_name} session={session_id} directory_mutated=false"), + ); + None + } + None => Some( + payload + .raw + .get("cwd") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + .unwrap_or_else(|| ctx.cwd.to_string_lossy().to_string()), + ), + }; + if let Some(cwd) = cwd.filter(|cwd| !cwd.is_empty()) { updates.insert("directory".into(), Value::String(cwd)); } if let Some(session_id) = payload.session_id.as_ref().filter(|s| !s.is_empty()) { @@ -361,23 +465,247 @@ fn set_prompt_active(db: &HcomDb, instance_name: &str) { lifecycle::set_status(db, instance_name, ST_ACTIVE, "prompt", Default::default()); } -fn handle_sessionstart(db: &HcomDb, ctx: &HcomContext, payload: &HookPayload) -> HookResult { - let session_id = match payload.session_id.as_deref() { - Some(sid) if !sid.is_empty() => sid, - _ => return hook_noop(), +fn codex_hook_cwd(ctx: &HcomContext, payload: &HookPayload) -> String { + payload + .raw + .get("cwd") + .and_then(|value| value.as_str()) + .map(str::to_owned) + .unwrap_or_else(|| ctx.cwd.to_string_lossy().to_string()) +} + +/// Accept a native Codex lifecycle payload only when the task identity exposed +/// to the hook process agrees with the payload. The payload session id is the +/// single canonical identity used by SessionStart after this check; ambient +/// CLI environment alone never authorizes a registration move. +fn canonical_sessionstart_id<'a>(ctx: &HcomContext, payload: &'a HookPayload) -> Option<&'a str> { + let payload_id = payload.session_id.as_deref()?.trim(); + let context_id = ctx.codex_thread_id.as_deref()?.trim(); + let session_env = ctx + .raw_env + .get("CODEX_SESSION_ID") + .map(String::as_str) + .map(str::trim) + .unwrap_or(""); + if payload_id.is_empty() + || context_id.is_empty() + || session_env.is_empty() + || payload_id != context_id + || payload_id != session_env + { + log::log_warn( + "hooks", + "codex.sessionstart_identity_mismatch", + &format!( + "payload_session_id={} context_thread_id={} context_session_id={} mutated=false", + payload_id, context_id, session_env + ), + ); + return None; + } + Some(payload_id) +} + +fn codex_restore_paths_match(stopped: &str, current: &str) -> bool { + let stopped = std::fs::canonicalize(stopped).unwrap_or_else(|_| PathBuf::from(stopped)); + let current = std::fs::canonicalize(current).unwrap_or_else(|_| PathBuf::from(current)); + + #[cfg(windows)] + { + stopped + .to_string_lossy() + .eq_ignore_ascii_case(¤t.to_string_lossy()) + } + #[cfg(not(windows))] + { + stopped == current + } +} + +/// Recreate a Codex row that stop/exit cleanup deleted at native SessionStart. +/// +/// `bind_session_to_process` can recover the canonical name from the durable +/// stopped event without a process binding, but its generic reconstruction +/// path needs a launch placeholder. Codex Desktop has no such placeholder, so +/// SessionStart restores the row from the same stopped snapshot after checking +/// that the tool and exact stopped session still describe this task. An +/// explicit, session-scoped directory pin can preserve a deliberate logical +/// project relocation; unpinned tasks keep the same-directory guard. +fn restore_missing_codex_instance( + db: &HcomDb, + ctx: &HcomContext, + payload: &HookPayload, + session_id: &str, + instance_name: &str, +) -> bool { + if let Some(instance) = db.get_instance_full(instance_name).ok().flatten() { + let current_cwd = codex_hook_cwd(ctx, payload); + let pin_present = launch_context_has_directory_pin(instance.launch_context.as_deref()); + let pinned_directory = + pinned_directory_from_launch_context(instance.launch_context.as_deref(), session_id); + let directory_matches = match pinned_directory { + Some(ref pinned) if pinned_directory_is_live(pinned) => { + codex_restore_paths_match(&instance.directory, pinned) + } + Some(_) => false, + None if pin_present => false, + None => { + instance.directory.is_empty() + || (!current_cwd.is_empty() + && codex_restore_paths_match(&instance.directory, ¤t_cwd)) + } + }; + return instance.tool == "codex" + && !instances::is_remote_instance(&instance) + && instance.session_id.as_deref() == Some(session_id) + && directory_matches; + } + + let data: String = match db.conn().query_row( + "SELECT data FROM events + WHERE type = 'life' + AND instance = ?1 + AND json_extract(data, '$.action') = 'stopped' + ORDER BY id DESC LIMIT 1", + rusqlite::params![instance_name], + |row| row.get(0), + ) { + Ok(data) => data, + Err(_) => return false, + }; + let data: Value = match serde_json::from_str(&data) { + Ok(data) => data, + Err(_) => return false, + }; + let Some(snapshot) = data.get("snapshot") else { + return false; }; - let mut instance_name = if let Some(pid) = ctx.process_id.as_deref() { - instance_binding::bind_session_to_process(db, session_id, Some(pid)) - } else { - None + if snapshot.get("tool").and_then(Value::as_str) != Some("codex") + || snapshot.get("session_id").and_then(Value::as_str) != Some(session_id) + || snapshot + .get("origin_device_id") + .and_then(Value::as_str) + .is_some_and(|device| !device.is_empty()) + { + return false; + } + + let current_cwd = codex_hook_cwd(ctx, payload); + let stopped_cwd = snapshot + .get("directory") + .and_then(Value::as_str) + .unwrap_or(""); + let pin_present = snapshot.get(CODEX_DIRECTORY_PIN_KEY).is_some(); + let pinned_directory = pinned_directory_from_snapshot(snapshot, session_id); + let directory = match pinned_directory.as_deref() { + Some(pinned) + if pinned_directory_is_live(pinned) + && !stopped_cwd.is_empty() + && codex_restore_paths_match(stopped_cwd, pinned) => + { + pinned + } + Some(_) => return false, + None if pin_present => return false, + None if !stopped_cwd.is_empty() + && !current_cwd.is_empty() + && codex_restore_paths_match(stopped_cwd, ¤t_cwd) => + { + current_cwd.as_str() + } + None => return false, }; - if instance_name.is_none() { - instance_name = resolve_codex_instance(db, ctx, payload).map(|i| i.name); + let transcript_path = payload + .transcript_path + .as_deref() + .filter(|path| !path.is_empty()) + .or_else(|| { + snapshot + .get("transcript_path") + .and_then(Value::as_str) + .filter(|path| !path.is_empty()) + }); + if !instance_binding::initialize_instance_in_position_file( + db, + instance_name, + Some(session_id), + snapshot.get("parent_session_id").and_then(Value::as_str), + snapshot.get("parent_name").and_then(Value::as_str), + snapshot.get("agent_id").and_then(Value::as_str), + transcript_path, + Some("codex"), + snapshot + .get("background") + .and_then(Value::as_i64) + .is_some_and(|background| background != 0), + snapshot.get("tag").and_then(Value::as_str), + snapshot.get("wait_timeout").and_then(Value::as_i64), + snapshot.get("subagent_timeout").and_then(Value::as_i64), + snapshot.get("hints").and_then(Value::as_str), + Some(directory), + ) { + return false; + } + + if let Some(pinned) = pinned_directory { + let context = directory_override_launch_context(&pinned, session_id); + if db.store_launch_context(instance_name, &context).is_err() { + let _ = db.delete_instance(instance_name); + return false; + } } - let instance_name = match instance_name { + let mut updates = serde_json::Map::new(); + if let Some(last_event_id) = snapshot.get("last_event_id").and_then(Value::as_i64) { + updates.insert("last_event_id".into(), Value::from(last_event_id)); + } + if let Some(name_announced) = snapshot.get("name_announced").and_then(Value::as_i64) { + updates.insert("name_announced".into(), Value::from(name_announced)); + } + let _ = db.update_instance_fields(instance_name, &updates); + + db.get_instance_full(instance_name).ok().flatten().is_some() +} + +fn resolve_sessionstart_instance( + db: &HcomDb, + ctx: &HcomContext, + payload: &HookPayload, + session_id: &str, +) -> Option { + // Codex Desktop has no HCOM_PROCESS_ID. SessionStart is the one safe point + // to restore an identity removed by stop/exit cleanup: doing this in the + // shared resolver would undo an intentional stop on every later hook. + if let Some(process_id) = ctx.process_id.as_deref() { + return instance_binding::bind_session_to_process(db, session_id, Some(process_id)); + } + + // A live session binding is already foreign-keyed to an existing row. Check + // ownership before accepting it, but do not send Desktop through the generic + // stopped-row path: without a launch placeholder that path attempts a rebind + // before the row exists and emits the very FK error this recovery prevents. + if let Ok(Some(instance_name)) = db.get_session_binding(session_id) { + return restore_missing_codex_instance(db, ctx, payload, session_id, &instance_name) + .then_some(instance_name); + } + + if let Ok(Some(instance_name)) = db.find_stopped_instance_by_session_id(session_id) + && restore_missing_codex_instance(db, ctx, payload, session_id, &instance_name) + { + return Some(instance_name); + } + + resolve_codex_instance(db, ctx, payload).map(|instance| instance.name) +} + +fn handle_sessionstart(db: &HcomDb, ctx: &HcomContext, payload: &HookPayload) -> HookResult { + let Some(session_id) = canonical_sessionstart_id(ctx, payload) else { + return hook_noop(); + }; + + let instance_name = match resolve_sessionstart_instance(db, ctx, payload, session_id) { Some(name) => name, None => return hook_noop(), }; @@ -680,7 +1008,28 @@ fn hook_state_key_belongs_to_hcom_hooks_json(key: &str, hooks_path: &Path) -> bo paths_equivalent(Path::new(key_source), hooks_path) } +/// Build a quote-free Windows hook command. +/// +/// Codex wraps the complete command in outer quotes before passing it to +/// `cmd.exe /C`; embedding quotes around the executable makes the resulting +/// token literal `\\"...\\"` and exits 1. `windows_current_hcom_executable` +/// has already converted spaced paths to a safe DOS short path and rejected +/// cmd metacharacters. +#[cfg(windows)] +fn build_pinned_windows_codex_hook_command(executable: &str, command: &str) -> String { + debug_assert!( + executable.chars().all(|ch| !ch.is_whitespace() + && !matches!(ch, '"' | '&' | '|' | '<' | '>' | '^' | '%' | '!')) + ); + format!("{executable} {command}") +} + fn build_codex_hook_command(command: &str) -> String { + #[cfg(windows)] + if let Some(executable) = crate::runtime_env::windows_current_hcom_executable() { + return build_pinned_windows_codex_hook_command(&executable, command); + } + let mut parts = crate::runtime_env::get_hcom_prefix(); parts.push(command.to_string()); parts.join(" ") @@ -2435,6 +2784,10 @@ pub enum VerifyFailReason { #[derive(Debug, thiserror::Error)] pub enum SetupError { + #[error( + "cannot create a quote-free Codex Windows hook command for the current hcom executable (path contains spaces or cmd metacharacters and has no usable short form): {path}" + )] + HookExecutableUnavailable { path: PathBuf }, #[error("failed to enable Codex experimental hooks feature in {}: {reason}", path.display())] EnsureFeatureFailed { path: PathBuf, reason: String }, #[error("failed to read existing {}: {source}", path.display())] @@ -2470,6 +2823,13 @@ pub enum SetupError { } pub fn try_setup_codex_hooks(include_permissions: bool) -> Result<(), SetupError> { + #[cfg(windows)] + if crate::runtime_env::windows_current_hcom_executable().is_none() { + return Err(SetupError::HookExecutableUnavailable { + path: std::env::current_exe().unwrap_or_else(|_| PathBuf::from("")), + }); + } + let config_path = get_codex_config_path(); let hooks_path = get_codex_hooks_path(); let feature_key = detect_codex_hooks_feature_key(); @@ -2666,6 +3026,545 @@ mod tests { use crate::hooks::test_helpers::{EnvGuard, isolated_test_env}; use serial_test::serial; + fn codex_test_ctx(thread_id: &str, cwd: &str) -> HcomContext { + let mut env = std::env::vars().collect::>(); + env.remove("HCOM_PROCESS_ID"); + env.insert("CODEX_SANDBOX".to_string(), "1".to_string()); + env.insert("CODEX_THREAD_ID".to_string(), thread_id.to_string()); + env.insert("CODEX_SESSION_ID".to_string(), thread_id.to_string()); + HcomContext::from_env(&env, PathBuf::from(cwd)) + } + + fn codex_test_payload(event: &str, session_id: &str, cwd: &str) -> HookPayload { + HookPayload::from_codex_native( + event, + serde_json::json!({ + "session_id": session_id, + "cwd": cwd, + }), + ) + } + + fn log_codex_stopped_snapshot( + db: &HcomDb, + name: &str, + session_id: &str, + tool: &str, + cwd: &str, + last_event_id: i64, + ) { + db.log_event( + "life", + name, + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "name": name, + "session_id": session_id, + "tool": tool, + "directory": cwd, + "transcript_path": null, + "parent_name": null, + "parent_session_id": null, + "tag": "desktop", + "wait_timeout": 321, + "subagent_timeout": null, + "hints": "restored", + "background": 0, + "agent_id": null, + "name_announced": 1, + "origin_device_id": null, + "last_event_id": last_event_id + } + }), + ) + .unwrap(); + } + + #[test] + #[serial] + fn test_sessionstart_restores_stopped_desktop_without_process_binding() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-restored"; + let name = "desktop-restored"; + let cwd = "/tmp/project"; + + instance_binding::initialize_instance_in_position_file( + &db, + name, + Some(session_id), + None, + None, + None, + None, + Some("codex"), + false, + Some("desktop"), + Some(321), + None, + Some("restored"), + Some(cwd), + ); + db.set_session_binding(session_id, name).unwrap(); + log_codex_stopped_snapshot(&db, name, session_id, "codex", cwd, 27); + db.delete_instance(name).unwrap(); + assert!(db.get_instance_full(name).unwrap().is_none()); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + + let ctx = codex_test_ctx(session_id, cwd); + let payload = codex_test_payload("SessionStart", session_id, cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + let row = db + .get_instance_full(name) + .unwrap() + .expect("SessionStart must recreate the deleted Desktop row"); + assert_eq!(row.tool, "codex"); + assert_eq!(row.directory, cwd); + assert_eq!(row.session_id.as_deref(), Some(session_id)); + assert_eq!( + row.last_event_id, 27, + "delivery cursor must survive restore" + ); + assert_eq!(row.tag.as_deref(), Some("desktop")); + assert_eq!(row.hints.as_deref(), Some("restored")); + assert_eq!(row.wait_timeout, Some(321)); + assert_eq!( + db.get_session_binding(session_id).unwrap().as_deref(), + Some(name) + ); + } + + #[test] + #[serial] + fn test_sessionstart_refuses_unpinned_cross_directory_restore() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-relocated"; + let name = "cultivation"; + let old_cwd = "/tmp/old-cultivation-project"; + let new_cwd = "/tmp/cultivation"; + + log_codex_stopped_snapshot(&db, name, session_id, "codex", old_cwd, 81); + let ctx = codex_test_ctx(session_id, new_cwd); + let payload = codex_test_payload("SessionStart", session_id, new_cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + assert!(db.get_instance_full(name).unwrap().is_none()); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + } + + #[test] + #[serial] + fn test_sessionstart_restores_pinned_directory_despite_native_cwd() { + let (tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-pinned"; + let name = "cultivation"; + let native_cwd = tmp.path().join("old-scaffold"); + let pinned_cwd = tmp.path().join("cultivation"); + std::fs::create_dir_all(&native_cwd).unwrap(); + std::fs::create_dir_all(&pinned_cwd).unwrap(); + let native_cwd = native_cwd.to_string_lossy().to_string(); + let pinned_cwd = pinned_cwd.to_string_lossy().to_string(); + let pin_context = directory_override_launch_context(&pinned_cwd, session_id); + let pin_value: Value = serde_json::from_str(&pin_context).unwrap(); + + db.log_event( + "life", + name, + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "name": name, + "session_id": session_id, + "tool": "codex", + "directory": pinned_cwd, + "transcript_path": null, + "parent_name": null, + "parent_session_id": null, + "tag": "desktop", + "wait_timeout": 321, + "subagent_timeout": null, + "hints": "restored", + "background": 0, + "agent_id": null, + "name_announced": 1, + "origin_device_id": null, + "last_event_id": 81, + "codex_directory_pin_v1": pin_value[CODEX_DIRECTORY_PIN_KEY].clone() + } + }), + ) + .unwrap(); + + let ctx = codex_test_ctx(session_id, &native_cwd); + let payload = codex_test_payload("SessionStart", session_id, &native_cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + let row = db + .get_instance_full(name) + .unwrap() + .expect("a valid stopped pin must restore the logical directory"); + assert_eq!(row.directory, pinned_cwd); + assert_eq!(row.session_id.as_deref(), Some(session_id)); + assert_eq!(row.last_event_id, 81); + assert_eq!( + pinned_directory_from_launch_context(row.launch_context.as_deref(), session_id) + .as_deref(), + Some(pinned_cwd.as_str()) + ); + + let later = codex_test_payload("UserPromptSubmit", session_id, &native_cwd); + let _ = handle_userpromptsubmit(&db, &ctx, &later); + assert_eq!( + db.get_instance_full(name).unwrap().unwrap().directory, + pinned_cwd, + "ordinary hooks must not overwrite the logical directory pin" + ); + } + + #[test] + #[serial] + fn test_directory_pin_survives_real_stop_and_sessionstart_cycle() { + let (tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-pin-cycle"; + let name = "cultivation"; + let native_cwd = tmp.path().join("old-scaffold"); + let pinned_cwd = tmp.path().join("cultivation"); + std::fs::create_dir_all(&native_cwd).unwrap(); + std::fs::create_dir_all(&pinned_cwd).unwrap(); + let native_cwd = native_cwd.to_string_lossy().to_string(); + let pinned_cwd = pinned_cwd.to_string_lossy().to_string(); + + assert!(instance_binding::initialize_instance_in_position_file( + &db, + name, + Some(session_id), + None, + None, + None, + None, + Some("codex"), + false, + Some("desktop"), + Some(321), + None, + Some("relocated"), + Some(&pinned_cwd), + )); + db.store_launch_context( + name, + &directory_override_launch_context(&pinned_cwd, session_id), + ) + .unwrap(); + db.set_session_binding(session_id, name).unwrap(); + + common::stop_instance(&db, name, "test", "pin-cycle"); + assert!(db.get_instance_full(name).unwrap().is_none()); + + let ctx = codex_test_ctx(session_id, &native_cwd); + let payload = codex_test_payload("SessionStart", session_id, &native_cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + let row = db.get_instance_full(name).unwrap().unwrap(); + assert_eq!(row.directory, pinned_cwd); + assert_eq!(row.session_id.as_deref(), Some(session_id)); + assert_eq!( + pinned_directory_from_launch_context(row.launch_context.as_deref(), session_id) + .as_deref(), + Some(pinned_cwd.as_str()) + ); + } + + #[test] + #[serial] + fn test_sessionstart_refuses_older_pinned_snapshot_after_newer_owner_stop() { + let (tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let name = "cultivation"; + let old_session = "thread-old-owner"; + let new_session = "thread-new-owner"; + let native_cwd = tmp.path().join("old-scaffold"); + let pinned_cwd = tmp.path().join("cultivation"); + let newer_cwd = tmp.path().join("new-owner"); + std::fs::create_dir_all(&native_cwd).unwrap(); + std::fs::create_dir_all(&pinned_cwd).unwrap(); + std::fs::create_dir_all(&newer_cwd).unwrap(); + let native_cwd = native_cwd.to_string_lossy().to_string(); + let pinned_cwd = pinned_cwd.to_string_lossy().to_string(); + let newer_cwd = newer_cwd.to_string_lossy().to_string(); + let pin_context = directory_override_launch_context(&pinned_cwd, old_session); + let pin_value: Value = serde_json::from_str(&pin_context).unwrap(); + + db.log_event( + "life", + name, + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "session_id": old_session, + "tool": "codex", + "directory": pinned_cwd, + "origin_device_id": null, + "last_event_id": 10, + "codex_directory_pin_v1": pin_value[CODEX_DIRECTORY_PIN_KEY].clone() + } + }), + ) + .unwrap(); + db.log_event( + "life", + name, + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "session_id": new_session, + "tool": "codex", + "directory": newer_cwd, + "origin_device_id": null, + "last_event_id": 11 + } + }), + ) + .unwrap(); + + let ctx = codex_test_ctx(old_session, &native_cwd); + let payload = codex_test_payload("SessionStart", old_session, &native_cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + assert!(db.get_instance_full(name).unwrap().is_none()); + assert!(db.get_session_binding(old_session).unwrap().is_none()); + assert!(db.get_session_binding(new_session).unwrap().is_none()); + } + + #[test] + #[serial] + fn test_sessionstart_refuses_pin_whose_directory_is_missing_or_file() { + let (tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + + for (index, replacement_is_file) in [false, true].into_iter().enumerate() { + let name = format!("cultivation-{index}"); + let session_id = format!("thread-dead-pin-{index}"); + let native_cwd = tmp.path().join(format!("native-{index}")); + let pinned_cwd = tmp.path().join(format!("pin-{index}")); + std::fs::create_dir_all(&native_cwd).unwrap(); + std::fs::create_dir_all(&pinned_cwd).unwrap(); + let native_cwd = native_cwd.to_string_lossy().to_string(); + let pinned_cwd = pinned_cwd.to_string_lossy().to_string(); + let pin_context = directory_override_launch_context(&pinned_cwd, &session_id); + let pin_value: Value = serde_json::from_str(&pin_context).unwrap(); + db.log_event( + "life", + &name, + &serde_json::json!({ + "action": "stopped", + "snapshot": { + "session_id": session_id, + "tool": "codex", + "directory": pinned_cwd, + "origin_device_id": null, + "last_event_id": 12, + "codex_directory_pin_v1": pin_value[CODEX_DIRECTORY_PIN_KEY].clone() + } + }), + ) + .unwrap(); + std::fs::remove_dir(&pinned_cwd).unwrap(); + if replacement_is_file { + std::fs::write(&pinned_cwd, b"not a directory").unwrap(); + } + + let ctx = codex_test_ctx(&session_id, &native_cwd); + let payload = codex_test_payload("SessionStart", &session_id, &native_cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + assert!(db.get_instance_full(&name).unwrap().is_none()); + assert!(db.get_session_binding(&session_id).unwrap().is_none()); + } + } + + #[test] + #[serial] + fn test_sessionstart_refuses_missing_or_mismatched_context_identity() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-owned"; + let name = "cultivation"; + let cwd = "/tmp/cultivation"; + log_codex_stopped_snapshot(&db, name, session_id, "codex", "/tmp/old", 0); + + let missing_ctx = codex_test_ctx("", cwd); + let payload = codex_test_payload("SessionStart", session_id, cwd); + let _ = handle_sessionstart(&db, &missing_ctx, &payload); + assert!(db.get_instance_full(name).unwrap().is_none()); + + let mismatched_ctx = codex_test_ctx("thread-foreign", cwd); + let _ = handle_sessionstart(&db, &mismatched_ctx, &payload); + assert!(db.get_instance_full(name).unwrap().is_none()); + + let mut mismatched_session_env = codex_test_ctx(session_id, cwd); + mismatched_session_env + .raw_env + .insert("CODEX_SESSION_ID".into(), "thread-foreign".into()); + let _ = handle_sessionstart(&db, &mismatched_session_env, &payload); + assert!(db.get_instance_full(name).unwrap().is_none()); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + } + + #[test] + #[serial] + fn test_sessionstart_refuses_wrong_snapshot_session() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let name = "cultivation"; + let cwd = "/tmp/cultivation"; + log_codex_stopped_snapshot(&db, name, "thread-original", "codex", "/tmp/old", 0); + + let ctx = codex_test_ctx("thread-other", cwd); + let payload = codex_test_payload("SessionStart", "thread-other", cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + assert!(db.get_instance_full(name).unwrap().is_none()); + assert!(db.get_session_binding("thread-original").unwrap().is_none()); + assert!(db.get_session_binding("thread-other").unwrap().is_none()); + } + + #[test] + #[serial] + fn test_sessionstart_repairs_missing_binding_for_existing_codex_row() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-unbound"; + let name = "desktop-unbound"; + let cwd = "/tmp/project"; + instance_binding::initialize_instance_in_position_file( + &db, + name, + Some(session_id), + None, + None, + None, + None, + Some("codex"), + false, + None, + None, + None, + None, + Some(cwd), + ); + log_codex_stopped_snapshot(&db, name, session_id, "codex", cwd, 0); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + + let ctx = codex_test_ctx(session_id, cwd); + let payload = codex_test_payload("SessionStart", session_id, cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + assert_eq!( + db.get_session_binding(session_id).unwrap().as_deref(), + Some(name) + ); + assert!(db.get_instance_full(name).unwrap().is_some()); + } + + #[test] + #[serial] + fn test_non_sessionstart_hook_does_not_restore_intentionally_stopped_codex() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let session_id = "thread-stopped"; + let name = "desktop-stopped"; + let cwd = "/tmp/project"; + log_codex_stopped_snapshot(&db, name, session_id, "codex", cwd, 0); + + let ctx = codex_test_ctx(session_id, cwd); + let payload = codex_test_payload("UserPromptSubmit", session_id, cwd); + let _ = handle_userpromptsubmit(&db, &ctx, &payload); + + assert!(db.get_instance_full(name).unwrap().is_none()); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + } + + #[test] + #[serial] + fn test_sessionstart_refuses_incompatible_stopped_snapshot() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + let db = HcomDb::open().unwrap(); + let cwd = "/tmp/project"; + + let name = "wrong-tool"; + let session_id = "thread-wrong-tool"; + log_codex_stopped_snapshot(&db, name, session_id, "claude", cwd, 0); + let ctx = codex_test_ctx(session_id, cwd); + let payload = codex_test_payload("SessionStart", session_id, cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + + assert!( + db.get_instance_full(name).unwrap().is_none(), + "incompatible stopped identity {name} must not be recreated" + ); + assert!(db.get_session_binding(session_id).unwrap().is_none()); + + let live_name = "live-wrong-tool"; + let live_session = "thread-live-wrong-tool"; + instance_binding::initialize_instance_in_position_file( + &db, + live_name, + Some(live_session), + None, + None, + None, + None, + Some("claude"), + false, + None, + None, + None, + None, + Some(cwd), + ); + db.set_session_binding(live_session, live_name).unwrap(); + let ctx = codex_test_ctx(live_session, cwd); + let payload = codex_test_payload("SessionStart", live_session, cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + assert_eq!( + db.get_instance_full(live_name).unwrap().unwrap().tool, + "claude", + "a live non-Codex binding must not be claimed" + ); + + let live_codex_name = "live-wrong-directory"; + let live_codex_session = "thread-live-wrong-directory"; + instance_binding::initialize_instance_in_position_file( + &db, + live_codex_name, + Some(live_codex_session), + None, + None, + None, + None, + Some("codex"), + false, + None, + None, + None, + None, + Some("/tmp/other-project"), + ); + db.set_session_binding(live_codex_session, live_codex_name) + .unwrap(); + let ctx = codex_test_ctx(live_codex_session, cwd); + let payload = codex_test_payload("SessionStart", live_codex_session, cwd); + let _ = handle_sessionstart(&db, &ctx, &payload); + let row = db.get_instance_full(live_codex_name).unwrap().unwrap(); + assert_eq!(row.directory, "/tmp/other-project"); + assert_eq!(row.session_id.as_deref(), Some(live_codex_session)); + } + #[test] fn test_hook_payload_factory_uses_native_fields() { let payload = HookPayload::from_codex_native( @@ -2748,6 +3647,76 @@ mod tests { // -- settings setup/remove/verify -- + #[test] + #[cfg(windows)] + fn test_pinned_windows_codex_hook_command_is_quote_free() { + assert_eq!( + build_pinned_windows_codex_hook_command( + "C:/Users/TestUser/.hcom/bin/hcom.exe", + "codex-stop", + ), + "C:/Users/TestUser/.hcom/bin/hcom.exe codex-stop" + ); + } + + #[test] + #[cfg(windows)] + #[serial] + fn test_setup_codex_hooks_pins_current_executable_without_bare_hcom() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + unsafe { std::env::set_var("HCOM_TEST_CODEX_CLI_VERSION", "codex-cli 0.130.0") }; + + assert!(setup_codex_hooks(false)); + let json: Value = serde_json::from_str( + &std::fs::read_to_string(get_codex_hooks_path()).expect("read generated hooks"), + ) + .expect("parse generated hooks"); + let expected_executable = crate::runtime_env::windows_current_hcom_executable() + .expect("current executable should resolve"); + let expected_prefix = format!("{expected_executable} "); + + for (event, _, _) in CODEX_HOOK_COMMANDS { + let groups = json["hooks"][*event] + .as_array() + .unwrap_or_else(|| panic!("{event} groups missing")); + let commands: Vec<&str> = groups + .iter() + .flat_map(|group| { + group["hooks"] + .as_array() + .into_iter() + .flatten() + .filter_map(|hook| hook["command"].as_str()) + }) + .filter(|command| is_hcom_codex_command(command)) + .collect(); + assert_eq!(commands.len(), 1, "{event} generated hook count"); + assert!( + commands[0].starts_with(&expected_prefix), + "{event} did not pin current executable: {}", + commands[0] + ); + assert!(!commands[0].starts_with("hcom ")); + assert!(!commands[0].contains("${HCOM")); + assert!(!commands[0].contains('"')); + } + } + + #[test] + #[serial] + fn test_setup_codex_hooks_reinstall_is_idempotent() { + let (_tmp, _hcom_dir, _home, _guard) = isolated_test_env(); + unsafe { std::env::set_var("HCOM_TEST_CODEX_CLI_VERSION", "codex-cli 0.130.0") }; + + assert!(setup_codex_hooks(false)); + let first = std::fs::read_to_string(get_codex_hooks_path()).unwrap(); + assert!(setup_codex_hooks(false)); + let second = std::fs::read_to_string(get_codex_hooks_path()).unwrap(); + + assert_eq!(first, second, "Codex hook reinstall changed hooks.json"); + assert!(verify_codex_hooks_installed(false)); + } + #[test] #[serial] fn test_setup_and_remove_codex_hooks() { diff --git a/src/hooks/common.rs b/src/hooks/common.rs index 942bfeb7..f1195637 100644 --- a/src/hooks/common.rs +++ b/src/hooks/common.rs @@ -1418,6 +1418,10 @@ fn stop_instance_inner( "session_id": instance_data.session_id, "tool": instance_data.tool, "directory": instance_data.directory, + "codex_directory_pin_v1": crate::hooks::codex::directory_override_snapshot_value( + instance_data.launch_context.as_deref(), + instance_data.session_id.as_deref(), + ), "parent_name": instance_data.parent_name, "parent_session_id": instance_data.parent_session_id, "tag": instance_data.tag, @@ -1600,6 +1604,10 @@ pub fn soft_finalize_session( "session_id": instance_data.session_id, "tool": instance_data.tool, "directory": instance_data.directory, + "codex_directory_pin_v1": crate::hooks::codex::directory_override_snapshot_value( + instance_data.launch_context.as_deref(), + instance_data.session_id.as_deref(), + ), "parent_name": instance_data.parent_name, "parent_session_id": instance_data.parent_session_id, "tag": instance_data.tag, @@ -2337,6 +2345,59 @@ mod tests { assert_eq!(count, 1, "life event should be logged"); } + #[test] + fn test_stop_instance_snapshots_only_valid_codex_directory_pin() { + crate::config::Config::init(); + let (_dir, db) = make_test_db(); + let pinned_directory = std::env::current_dir() + .unwrap() + .join("cultivation") + .to_string_lossy() + .to_string(); + let launch_context = serde_json::json!({ + "terminal_id": "must-not-leak", + "env": {"SSH_CONNECTION": "must-not-leak"}, + "codex_directory_pin_v1": { + "directory": pinned_directory.clone(), + "session_id": "thread-cultivation", + "source": "explicit-start-relocate-v1" + } + }) + .to_string(); + db.conn() + .execute( + "INSERT INTO instances + (name, tool, session_id, directory, launch_context, status, + status_context, status_time, created_at) + VALUES ('cultivation', 'codex', 'thread-cultivation', + ?1, ?2, 'active', 'new', 0, 0)", + rusqlite::params![pinned_directory, launch_context], + ) + .unwrap(); + + stop_instance(&db, "cultivation", "test", "pin_snapshot"); + + let data: String = db + .conn() + .query_row( + "SELECT data FROM events + WHERE type='life' AND instance='cultivation' + ORDER BY id DESC LIMIT 1", + [], + |row| row.get(0), + ) + .unwrap(); + let data: Value = serde_json::from_str(&data).unwrap(); + let snapshot = &data["snapshot"]; + assert_eq!( + snapshot["codex_directory_pin_v1"]["directory"], + pinned_directory + ); + assert!(snapshot.get("launch_context").is_none()); + assert!(snapshot.get("terminal_id").is_none()); + assert!(snapshot.get("env").is_none()); + } + #[test] fn test_stop_instance_recursive_subagent_cleanup() { crate::config::Config::init(); diff --git a/src/instance_binding.rs b/src/instance_binding.rs index 8a9dd823..b9440b72 100644 --- a/src/instance_binding.rs +++ b/src/instance_binding.rs @@ -85,6 +85,7 @@ pub fn capture_and_store_launch_context(db: &HcomDb, instance_name: &str) { "kitty_listen_on", "process_id", "terminal_preset_effective", + "codex_directory_pin_v1", ]; let mut ctx = new_ctx; @@ -1166,8 +1167,8 @@ mod tests { #[serial] fn test_capture_and_store_launch_context_preserves_terminal_metadata() { // Preserve only the fields we can't recapture from hook env: - // pane_id, terminal_id, kitty_listen_on, process_id, and the resolved - // terminal preset name. + // pane_id, terminal_id, kitty_listen_on, process_id, the resolved + // terminal preset name, and an explicit Codex logical-directory pin. let (db, path) = setup_test_db(); db.conn() .execute( @@ -1176,7 +1177,7 @@ mod tests { "luna", "claude", 1.0f64, - r#"{"terminal_preset_effective":"herdr","pane_id":"p_7","process_id":"proc-1"}"# + r#"{"terminal_preset_effective":"herdr","pane_id":"p_7","process_id":"proc-1","codex_directory_pin_v1":{"directory":"/tmp/cultivation","session_id":"thread-1","source":"explicit-start-relocate-v1"}}"# ], ) .unwrap(); @@ -1198,6 +1199,12 @@ mod tests { ctx.get("process_id").and_then(|v| v.as_str()), Some("proc-1") ); + assert_eq!( + ctx.get("codex_directory_pin_v1") + .and_then(|v| v.get("directory")) + .and_then(|v| v.as_str()), + Some("/tmp/cultivation") + ); cleanup(path); } diff --git a/src/instance_lifecycle.rs b/src/instance_lifecycle.rs index 46b644a6..a658331c 100644 --- a/src/instance_lifecycle.rs +++ b/src/instance_lifecycle.rs @@ -62,6 +62,31 @@ pub struct ComputedStatus { pub use crate::shared::time::format_age; +/// Whether this row represents a live Claude session that can only receive +/// messages at hook boundaries (for example, Claude Desktop). +/// +/// The session binding is the durable ownership signal. Hard stops delete it, +/// and soft stops clear it, so a non-null `instances.session_id` alone is not +/// sufficient. A process binding means the PTY delivery path owns the session. +pub(crate) fn is_hook_only_claude_session(data: &InstanceRow, db: &HcomDb) -> bool { + if data.tool != "claude" + || data + .origin_device_id + .as_deref() + .is_some_and(|device_id| !device_id.is_empty()) + || db.has_process_binding_for_instance(&data.name) + { + return false; + } + + data.session_id.as_deref().is_some_and(|session_id| { + matches!( + db.get_session_binding(session_id), + Ok(Some(owner)) if owner == data.name + ) + }) +} + // Tracks wall-clock vs monotonic-clock drift to detect system sleep. // On macOS, Instant (mach_absolute_time) does not advance during sleep, // but SystemTime (gettimeofday) does. Large drift means the system just woke. @@ -686,6 +711,19 @@ pub fn cleanup_stale_instances( continue; } + // A Claude Stop-hook poll timeout is ordinary idle state, not a + // session stop. Older binaries persisted that state as + // inactive/exit:timeout; retain such rows while their exact hook + // binding remains live so queued messages survive until the next + // supported hook boundary. Explicit stop/SessionEnd removes the + // binding and therefore does not take this path. + if data.status == ST_INACTIVE + && data.status_context == "exit:timeout" + && is_hook_only_claude_session(data, db) + { + continue; + } + let context = &computed.context; let age = computed.age_seconds; @@ -1272,4 +1310,45 @@ WARNING: proceeding, even though we could not update PATH: Operation not permitt cleanup(path); } + + #[test] + fn cleanup_preserves_bound_hook_only_claude_timeout() { + crate::config::Config::init(); + let (db, path) = setup_test_db(); + let old = now_epoch_i64() - 120; + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, status, status_context, status_time, created_at) + VALUES ('risa', 'sess-risa', 'claude', 'inactive', 'exit:timeout', ?, 1)", + rusqlite::params![old], + ) + .unwrap(); + db.set_session_binding("sess-risa", "risa").unwrap(); + + assert_eq!(cleanup_stale_instances(&db, 3600, 3600), 0); + assert!(db.get_instance_full("risa").unwrap().is_some()); + + cleanup(path); + } + + #[test] + fn cleanup_removes_unbound_claude_timeout() { + crate::config::Config::init(); + let (db, path) = setup_test_db(); + let old = now_epoch_i64() - 120; + db.conn() + .execute( + "INSERT INTO instances + (name, session_id, tool, status, status_context, status_time, created_at) + VALUES ('risa', 'sess-risa', 'claude', 'inactive', 'exit:timeout', ?, 1)", + rusqlite::params![old], + ) + .unwrap(); + + assert_eq!(cleanup_stale_instances(&db, 3600, 3600), 1); + assert!(db.get_instance_full("risa").unwrap().is_none()); + + cleanup(path); + } } diff --git a/src/pty/screen.rs b/src/pty/screen.rs index 6b630204..956242c9 100644 --- a/src/pty/screen.rs +++ b/src/pty/screen.rs @@ -862,8 +862,9 @@ impl ScreenTracker { /// Extract Codex input text. /// - /// Codex uses `›` (U+203A) as prompt character. Placeholder text is rendered - /// with dim attribute, real user input is not dim. + /// Codex uses `›` (U+203A) as its normal prompt character and `»` (U+00BB) + /// for the Ultra reasoning tier. Placeholder text is rendered with dim + /// attribute, real user input is not dim. /// /// Uses vt100's cell-level dim attribute to distinguish placeholder from /// real input, avoiding race conditions where ready pattern is still visible @@ -871,28 +872,35 @@ impl ScreenTracker { fn get_codex_input_text(&self) -> Option { let lines = self.get_screen_lines(); - // Search bottom-to-top for › prompt character - // › (U+203A, SINGLE RIGHT-POINTING ANGLE QUOTATION MARK) = 3 bytes UTF-8 + 1 space = 4 bytes total + // Search bottom-to-top for either live composer prompt. Submitted prompts + // remain visible in history with `›`, even when the current Ultra composer + // uses `»`, so considering both glyphs preserves the bottommost-live-box + // invariant. for (row_idx, line) in lines.iter().enumerate().rev() { let trimmed = line.trim_start(); - if let Some(text) = trimmed.strip_prefix("› ") { - let text = trim_with_nbsp(text); + let (prompt_char, text) = if let Some(text) = trimmed.strip_prefix("› ") { + ("›", text) + } else if let Some(text) = trimmed.strip_prefix("» ") { + ("»", text) + } else { + continue; + }; + let text = trim_with_nbsp(text); - if text.is_empty() { - return Some(String::new()); - } + if text.is_empty() { + return Some(String::new()); + } - // Dim text = placeholder, not real input - match self.is_dim_after_prompt(row_idx as u16, "›") { - Some(true) => return Some(String::new()), - Some(false) => return Some(text.to_string()), - None => { - // Can't locate prompt glyph, fall back to ready-pattern logic - if self.is_ready() { - return Some(String::new()); - } - return Some(text.to_string()); + // Dim text = placeholder, not real input + match self.is_dim_after_prompt(row_idx as u16, prompt_char) { + Some(true) => return Some(String::new()), + Some(false) => return Some(text.to_string()), + None => { + // Can't locate prompt glyph, fall back to ready-pattern logic + if self.is_ready() { + return Some(String::new()); } + return Some(text.to_string()); } } } @@ -1607,6 +1615,25 @@ mod tests { ); } + #[test] + fn codex_ultra_dim_placeholder_wins_over_stale_history() { + let mut t = make_tracker(24, 80, "? for shortcuts"); + let mut data = Vec::new(); + data.extend_from_slice("› submitted prompt\r\n» ".as_bytes()); + data.extend_from_slice(b"\x1b[2mAsk Codex to do anything\x1b[0m\r\n"); + t.process(&data); + + assert_eq!(t.get_codex_input_text(), Some(String::new())); + } + + #[test] + fn codex_ultra_extracts_non_dim_input_text() { + let mut t = make_tracker(24, 80, "? for shortcuts"); + t.process("» \r\n".as_bytes()); + + assert_eq!(t.get_codex_input_text(), Some("".to_string())); + } + // ---- Cursor input extraction ---- #[test] diff --git a/src/runtime_env.rs b/src/runtime_env.rs index c815a4c5..c07a32fa 100644 --- a/src/runtime_env.rs +++ b/src/runtime_env.rs @@ -46,6 +46,64 @@ pub(crate) fn build_hcom_command() -> String { get_hcom_prefix().join(" ") } +/// Resolve the exact native hcom executable running on Windows. +/// +/// Hook installers use this instead of PATH so long-running desktop clients +/// cannot select an older `hcom.exe` from an inherited environment. The +/// forward-slash form is accepted by both Git Bash and `cmd.exe`, and avoids +/// JSON backslash escaping in generated hook declarations. +#[cfg(windows)] +pub(crate) fn windows_current_hcom_executable() -> Option { + use std::os::windows::ffi::OsStrExt; + + let exe = std::env::current_exe().ok()?; + let resolved = exe.canonicalize().unwrap_or(exe); + let resolved = crate::shared::platform::child_process_path(&resolved); + let command_path = resolved.to_string_lossy(); + let command_path = if command_path.chars().any(char::is_whitespace) { + // Codex 0.145+ wraps the complete Windows hook command in a second + // pair of quotes before passing it to cmd.exe. An inner quoted path + // therefore arrives as a literal `\\"...\\"` token and exits 1. + // Use the DOS short form to keep the command line quote-free. If 8.3 + // names are disabled, return None so hook setup fails before writing a + // declaration that Codex cannot execute. + let wide: Vec = resolved.as_os_str().encode_wide().chain(Some(0)).collect(); + let mut capacity = 260u32; + loop { + let mut buffer = vec![0u16; capacity as usize]; + let length = unsafe { + windows_sys::Win32::Storage::FileSystem::GetShortPathNameW( + wide.as_ptr(), + buffer.as_mut_ptr(), + capacity, + ) + }; + if length == 0 { + return None; + } + if length < capacity { + break String::from_utf16(&buffer[..length as usize]).ok()?; + } + capacity = length.saturating_add(1); + if capacity > 32768 { + return None; + } + } + } else { + command_path.into_owned() + }; + + // These characters have cmd.exe meaning even without quoting. Reject + // them rather than generating a hook that can execute a different command. + if command_path + .chars() + .any(|ch| ch.is_whitespace() || matches!(ch, '"' | '&' | '|' | '<' | '>' | '^' | '%' | '!')) + { + return None; + } + Some(command_path.replace('\\', "/")) +} + /// Gemini / Antigravity shared config directory (`~/.gemini` or under `GEMINI_CLI_HOME`). pub(crate) fn gemini_family_config_dir() -> std::path::PathBuf { if let Ok(dir) = std::env::var("GEMINI_CLI_HOME") diff --git a/tests/cli_smoke.rs b/tests/cli_smoke.rs index b180a2b2..fe45f83a 100644 --- a/tests/cli_smoke.rs +++ b/tests/cli_smoke.rs @@ -3,8 +3,10 @@ mod support; +use std::fs; #[cfg(unix)] use std::os::unix::process::CommandExt; +use std::path::PathBuf; use std::process::Command; use std::time::{Duration, Instant}; use support::{Hcom, parse_hcom_marker}; @@ -456,6 +458,101 @@ fn start_as_reclaims_stopped_identity() { assert!(snap_present, "stopped snapshot missing; full={full_out}"); } +#[test] +fn start_as_relocate_routes_cli_and_binds_exact_codex_session() { + let h = Hcom::new(); + let session_id = "thread-cli-relocate"; + let old_directory = h.workspace.join("old-scaffold"); + let new_directory = h.workspace.join("cultivation"); + fs::create_dir_all(&old_directory).unwrap(); + fs::create_dir_all(&new_directory).unwrap(); + let sessions = h.codex_home.join("sessions/2026/09/02"); + fs::create_dir_all(&sessions).unwrap(); + let transcript = sessions.join(format!("rollout-test-{session_id}.jsonl")); + let meta = serde_json::json!({ + "type": "session_meta", + "payload": { + "id": session_id, + "session_id": session_id, + "cwd": old_directory, + "originator": "Codex Desktop", + "source": "vscode", + "thread_source": "user" + } + }); + fs::write(&transcript, format!("{meta}\n")).unwrap(); + + let (status_code, _, status_error) = h.run(["status", "--json"]); + assert_eq!(status_code, 0, "schema init failed: {status_error}"); + let db = rusqlite::Connection::open(h.hcom_dir.join("hcom.db")).unwrap(); + let stopped = serde_json::json!({ + "action": "stopped", + "snapshot": { + "tool": "codex", + "directory": old_directory, + "session_id": session_id, + "transcript_path": transcript, + "parent_name": null, + "parent_session_id": null, + "agent_id": null, + "origin_device_id": null, + "background": 0, + "last_event_id": 47 + } + }); + db.execute( + "INSERT INTO events (timestamp, type, instance, data) + VALUES ('2026-09-02T00:00:00Z', 'life', 'cultivation', ?1)", + rusqlite::params![stopped.to_string()], + ) + .unwrap(); + drop(db); + + let mut command = h.cmd(); + command + .current_dir(&new_directory) + .env("CODEX_SANDBOX", "1") + .env("CODEX_THREAD_ID", session_id) + .env("CODEX_SESSION_ID", session_id) + .args(["start", "--as", "cultivation", "--relocate"]); + let output = command.output().expect("run relocation CLI"); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(output.status.success(), "stdout={stdout} stderr={stderr}"); + assert!(stdout.contains("[hcom:cultivation]"), "stdout={stdout}"); + + let db = rusqlite::Connection::open(h.hcom_dir.join("hcom.db")).unwrap(); + let row: (String, String, String, String, i64) = db + .query_row( + "SELECT session_id, tool, directory, transcript_path, last_event_id + FROM instances WHERE name = 'cultivation'", + [], + |row| { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + )) + }, + ) + .unwrap(); + assert_eq!(row.0, session_id); + assert_eq!(row.1, "codex"); + assert_eq!(PathBuf::from(row.2), new_directory); + assert_eq!(PathBuf::from(row.3), transcript); + assert_eq!(row.4, 47); + let binding: String = db + .query_row( + "SELECT instance_name FROM session_bindings WHERE session_id = ?1", + rusqlite::params![session_id], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(binding, "cultivation"); +} + #[test] fn bigboss_send_bypasses_identity_gate() { // Wiki contract (messaging.md §@bigboss + reference_send_bigboss_flag memory):