From d451be8cd065bfcf2cc38ec0f0fc3b123b92f412 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sat, 5 Sep 2026 10:06:24 -0700 Subject: [PATCH 1/5] feat: refine shutdown logic and documentation for task termination and grace periods --- docs/README.md | 2 +- docs/commands.md | 4 +- src/core.rs | 11 +++-- src/supervisor.rs | 38 ++++----------- src/supervisor_tests.rs | 100 ++++++++++++++++++++++------------------ src/task.rs | 58 +---------------------- src/task_tests.rs | 68 ++++++--------------------- 7 files changed, 90 insertions(+), 191 deletions(-) diff --git a/docs/README.md b/docs/README.md index b1e30a6..7e693f0 100644 --- a/docs/README.md +++ b/docs/README.md @@ -211,7 +211,7 @@ A second `fleetcom` prints a waiting notice, then attaches when the active clien ### Shutdown is graceful-first -`X`, `Q`, `--kill`, and daemon shutdown signals send `SIGTERM` to each task's *process group*, then escalate to `SIGKILL` after two seconds. Removing one task (`X`) keeps an exited leader unreaped through escalation, reserving the process-group ID so background children remain signalable. During full shutdown (`Q`/`--kill`), checking whether a group is empty reaps its exited leader. A `TERM`-ignoring member that outlives the leader can then become unsafe to signal by group ID and survive daemon shutdown. A child created by `cmd &` in a non-interactive shell normally remains in its parent's group. A process that calls `setsid` or otherwise leaves the group is outside the sweep and must be terminated separately. +`X`, `Q`, `--kill`, and daemon shutdown signals send `SIGTERM` to each task's *process group*, then escalate to `SIGKILL` after two seconds. Exited leaders remain unreaped through escalation, reserving the process-group IDs so background children remain signalable. Full shutdown (`Q`/`--kill`) waits one shared grace period even when all listed tasks have finished or exit on `TERM`; with no tasks left to clean up, shutdown returns immediately. Tasks already terminating keep their original escalation timers. A child created by `cmd &` in a non-interactive shell normally remains in its parent's group. A process that calls `setsid` or otherwise leaves the group is outside the sweep and must be terminated separately. ### `--foreground` is ephemeral diff --git a/docs/commands.md b/docs/commands.md index 35864a9..69faa03 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -112,9 +112,7 @@ Rerunning preserves the task's ID, `◆` tag, group, name, and spawn order; its Input meaning depends on the active surface. From the dashboard, `q` or `Ctrl-C` disconnects the client. A daemon and its tasks continue running, so the next `fleetcom` invocation reconnects. Under `--foreground`, the in-process core exits with the client and its tasks die. While attached, `Ctrl-C` belongs to the child. In prompts and pickers, `Esc` cancels without disconnecting. -Uppercase `Q` stops the daemon and terminates each task's process group. Shutdown sends `TERM` first, then `KILL` after a two-second grace period. Processes that have moved into another group are outside this sweep. - -A `TERM`-ignoring member can also survive when its leader exits during shutdown. The group-emptiness check then releases the process-group ID reservation before escalation; [Shutdown is graceful-first](README.md#shutdown-is-graceful-first) explains the tradeoff. +Uppercase `Q` stops the daemon and terminates each task's process group. Shutdown sends `TERM` first, then `KILL` after one shared two-second grace period. Exited leaders remain unreaped until escalation so `TERM`-ignoring descendants in their groups still receive `KILL`. A nonempty fleet waits the grace even when its listed tasks have finished or respect `TERM`; with no tasks left to clean up, shutdown returns immediately. Tasks already terminating keep their original escalation timers. Processes that have moved into another group or session are outside this sweep. ### Task organization diff --git a/src/core.rs b/src/core.rs index 81f989a..394d1cb 100644 --- a/src/core.rs +++ b/src/core.rs @@ -89,9 +89,9 @@ fn ready_to_tick(dirty: bool, since_last_tick: Duration) -> bool { /// signal that ends the loop with `ClientGone`. /// /// `stop` is an external stop request (the daemon's signal flag): checked once -/// per wake/timeout, so a raised flag ends the loop within one `FALLBACK` even -/// when nothing else is happening. It shuts down exactly like a `Shutdown` -/// command: tasks killed, `LoopExit::Shutdown` returned. +/// per wake/timeout, so a raised flag begins shutdown within one `FALLBACK` +/// even when nothing else is happening. It shuts down exactly like a +/// `Shutdown` command: TERM grace, tasks killed, `LoopExit::Shutdown` returned. pub fn run_loop( sup: &mut Supervisor, wake_rx: &Receiver, @@ -268,6 +268,7 @@ mod tests { fn stop_flag_ends_loop_with_shutdown() { let cwd = std::env::current_dir().unwrap(); let mut sup = Supervisor::new(24, 80, 2000); + sup.set_kill_grace(Duration::ZERO); sup.set_launch_context(crate::protocol::LaunchContext::here()); let (wake_tx, wake_rx) = std::sync::mpsc::channel::(); sup.set_waker(wake_tx); @@ -284,8 +285,8 @@ mod tests { let started = Instant::now(); let exit = run_loop(&mut sup, &wake_rx, &stop, |_| true); assert!(matches!(exit, LoopExit::Shutdown)); - // The flag is checked before blocking, so the return is immediate: well - // under the FALLBACK a wake-starved loop would otherwise sleep. + // With no TERM grace, only stop observation contributes to the bound: + // the flag must be checked before a wake-starved loop sleeps. assert!(started.elapsed() < FALLBACK); // Shutdown cleared the task set: a tick emits an empty snapshot. sup.tick(); diff --git a/src/supervisor.rs b/src/supervisor.rs index 1c6b222..a88b992 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -90,8 +90,7 @@ fn effective_scrollback(flag: Option, env: Option<&str>) -> usize { .map_or(DEFAULT_SCROLLBACK, |lines| lines.min(MAX_SCROLLBACK)) } -/// Grace period between SIGTERM and SIGKILL, bounding shutdown delay for tasks -/// that do not exit after SIGTERM. +/// Grace period between SIGTERM and SIGKILL, shared by all tasks at shutdown. const KILL_GRACE: Duration = Duration::from_secs(2); /// Quiet period used to coalesce recipe changes into one recovery write. @@ -249,11 +248,8 @@ pub struct Supervisor { /// leader's zombie is collected. Invisible to `tick` snapshots, so the row /// disappears instantly while the sweep runs behind it. /// - /// Entries remain through `kill_grace` because observing group emptiness - /// would cost the escalation: the probe (`Task::group_gone`) must reap - /// the leader to see past its zombie, and a reaped group can no longer - /// be KILLed. Entries therefore keep their zombie until `kill_sent`, and - /// `shutdown_all` counts the graveyard instead of probing it. + /// Entries keep their leader's zombie until SIGKILL has been sent: + /// collecting it earlier would release the process-group ID. graveyard: Vec, next_id: u64, /// PTY content size (rows already minus the client's status bar). Every task @@ -469,24 +465,17 @@ impl Supervisor { self.graveyard.retain_mut(|t| !t.try_collect()); } - /// Kill every task for the quit path: TERM all groups at once, wait out - /// one shared grace, then SIGKILL the stragglers. The wait exits early - /// once `swept` proves there is nothing left to wait for; a task's - /// `finished` alone cannot gate it, because leader exit says nothing - /// about the rest of the group (`cmd & exit 0` leaves members behind), - /// and a leader-only predicate KILLed those members the instant the last - /// leader happened to be done, skipping the TERM grace entirely. - /// Blocking is bounded by the grace. Anything the final KILLs don't - /// collect (a leader in uninterruptible sleep) reparents to init when - /// the daemon exits moments later, as do TERM-refusing members of a - /// group whose leader the emptiness probe reaped (see - /// `Task::group_gone`); blocking on either could wedge shutdown forever. + /// TERM every owned group, wait one shared grace, then SIGKILL before + /// collecting leaders. Exited leaders retain their process-group IDs: + /// their descendants may still need escalation. Existing TERM timers + /// continue through `reap`; a nonempty fleet waits even if leaders exit. + /// Collection and PTY teardown never block on live processes or workers. fn shutdown_all(&mut self) { for t in &mut self.tasks { t.terminate(); } let deadline = Instant::now() + self.kill_grace; - while !self.swept() && Instant::now() < deadline { + while (!self.tasks.is_empty() || !self.graveyard.is_empty()) && Instant::now() < deadline { std::thread::sleep(Duration::from_millis(25)); self.reap(); } @@ -494,15 +483,6 @@ impl Supervisor { self.graveyard.clear(); } - /// Shutdown's exit test: every live task's process group probes gone and - /// the graveyard has drained. Graveyard entries are counted, not probed: - /// probing reaps the leader, and a reaped group forfeits the KILL its - /// pending escalation still owes (`Task::try_collect`'s `kill_sent` gate - /// exists for the same reason); they leave through `reap` as always. - fn swept(&mut self) -> bool { - self.graveyard.is_empty() && self.tasks.iter_mut().all(Task::group_gone) - } - /// One step of the core's own loop: reap exits, then emit a fresh task /// snapshot (plus the watched task's screen). In process the client calls /// this each UI tick; in the daemon it runs on the core's thread and the diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index ce308d5..40db886 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -622,17 +622,17 @@ fn term_ignoring_task_escalates_to_kill() { wait_for_lifecycle(&mut s, id, |l| l == Lifecycle::Failed); } -/// `Shutdown` exits as soon as TERM-respecting tasks die: well inside the -/// grace, not after it. +/// Leader exit does not shorten shutdown's grace, even for ordinary tasks. #[test] -fn shutdown_returns_early_when_tasks_respect_term() { +fn shutdown_waits_the_grace_when_tasks_respect_term() { let mut s = sup(24, 80); + s.set_kill_grace(Duration::from_millis(200)); spawn(&mut s, "sleep 300", here()); let t0 = Instant::now(); s.apply(Command::Shutdown); assert!( - t0.elapsed() < Duration::from_secs(1), - "shutdown waited the full grace for a TERM-respecting task" + t0.elapsed() >= Duration::from_millis(200), + "shutdown skipped the grace for a TERM-respecting task" ); s.tick(); assert!( @@ -705,29 +705,27 @@ fn overfull_writer_queue_refuses_message_with_notice() { ); } -/// `Shutdown` with a TERM-ignoring task is bounded by the grace, then -/// SIGKILLs it: quit can be slowed, never wedged. +/// TERM-ignoring tasks share one grace; shutdown never waits per task. #[test] fn shutdown_is_bounded_by_grace() { let dir = scratch("shutdown_bound"); - let ready = dir.join("ready"); let mut s = sup(24, 80); s.set_kill_grace(Duration::from_millis(200)); - spawn_ready( - &mut s, - format!( - "trap '' TERM; echo r > {r}; while :; do sleep 0.1; done", - r = ready.display() - ), - dir.to_path_buf(), - &ready, - ); + for i in 0..8 { + let ready = dir.join(format!("ready_{i}")); + spawn_ready( + &mut s, + format!("trap '' TERM; echo r > {}; exec sleep 300", ready.display()), + dir.to_path_buf(), + &ready, + ); + } let t0 = Instant::now(); s.apply(Command::Shutdown); let elapsed = t0.elapsed(); assert!( - elapsed < Duration::from_secs(2), - "shutdown took {elapsed:?}: not bounded by the 200 ms grace" + elapsed >= Duration::from_millis(200) && elapsed < Duration::from_secs(1), + "shutdown took {elapsed:?}: eight tasks must share the 200 ms grace" ); s.tick(); assert!( @@ -1406,7 +1404,7 @@ fn kill_escalation_reaches_term_ignoring_straggler_after_leader_exit() { /// Shutdown after removal preserves the removed task's TERM grace. #[test] fn shutdown_waits_for_graveyard_grace() { - use nix::sys::signal::kill; + use nix::sys::signal::{Signal, kill}; let dir = scratch("shutdown_graveyard"); let (spid, ready) = (dir.join("spid"), dir.join("ready")); let mut s = sup(24, 80); @@ -1416,7 +1414,7 @@ fn shutdown_waits_for_graveyard_grace() { let id = spawn_ready( &mut s, format!( - "trap '' HUP; (trap '' TERM; exec sleep 300) & echo $! > {sp}; echo r > {r}", + "trap '' HUP TERM; sleep 300 & echo $! > {sp}; echo r > {r}", sp = spid.display(), r = ready.display() ), @@ -1435,23 +1433,21 @@ fn shutdown_waits_for_graveyard_grace() { kill(straggler, None).is_ok() }); s.apply(Command::Shutdown); + let alive_mid_grace = alive_mid_grace.join(); + let dead = wait_until(Duration::from_secs(5), || kill(straggler, None).is_err()); + // A failed escalation must not leak the fixture after shutdown clears ownership. + if !dead { + let _ = kill(straggler, Signal::SIGKILL); + } assert!( - alive_mid_grace.join().unwrap(), + alive_mid_grace.unwrap(), "straggler was KILLed before its grace elapsed" ); - assert!( - reap_until(&mut s, Duration::from_secs(5), |_| kill(straggler, None) - .is_err()), - "straggler survived shutdown" - ); + assert!(dead, "straggler survived shutdown"); } -/// The defect the group probe fixes: every leader exits at birth after -/// backgrounding a TERM-refusing child, so the old leader-only predicate -/// saw nothing to wait for and Drop KILLed the child instantly. Shutdown -/// must instead hold the full grace while the group probes non-empty; -/// the child, unreachable by KILL once the probe reaped its leader, -/// survives to reparent. +/// Leader exit must neither skip the descendant's grace nor release the +/// process-group ID before escalation. #[test] fn shutdown_holds_the_grace_for_members_of_an_exited_leader() { use nix::sys::signal::{Signal, kill}; @@ -1463,7 +1459,7 @@ fn shutdown_holds_the_grace_for_members_of_an_exited_leader() { let id = spawn_ready( &mut s, format!( - "trap '' HUP; (trap '' TERM; exec sleep 300) & echo $! > {sp}; echo r > {r}", + "trap '' HUP TERM; sleep 300 & echo $! > {sp}; echo r > {r}", sp = spid.display(), r = ready.display() ), @@ -1476,12 +1472,19 @@ fn shutdown_holds_the_grace_for_members_of_an_exited_leader() { })); assert!(kill(straggler, None).is_ok(), "straggler should be alive"); + let alive_mid_grace = std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(200)); + kill(straggler, None).is_ok() + }); let t0 = Instant::now(); s.apply(Command::Shutdown); let elapsed = t0.elapsed(); - let survived = kill(straggler, None).is_ok(); - // Clean up the reparented survivor before asserting. - let _ = kill(straggler, Signal::SIGKILL); + let alive_mid_grace = alive_mid_grace.join(); + let dead = wait_until(Duration::from_secs(5), || kill(straggler, None).is_err()); + // A failed escalation must not leak the fixture after shutdown clears ownership. + if !dead { + let _ = kill(straggler, Signal::SIGKILL); + } assert!( elapsed >= Duration::from_millis(400), "shutdown returned in {elapsed:?} with a non-empty group: the grace was skipped" @@ -1491,16 +1494,17 @@ fn shutdown_holds_the_grace_for_members_of_an_exited_leader() { "shutdown took {elapsed:?}: not bounded by the 400 ms grace" ); assert!( - survived, - "the straggler was KILLed instead of receiving the TERM grace" + alive_mid_grace.unwrap(), + "straggler was KILLed before its grace elapsed" ); + assert!(dead, "straggler survived shutdown after its leader exited"); } -/// Prompt exit, pinned: leaders exited long ago and left empty groups, -/// so shutdown returns in a few probe passes, nowhere near the grace. +/// Completed rows retain group ownership and receive the same shutdown grace. #[test] -fn shutdown_is_prompt_when_every_group_is_already_empty() { +fn shutdown_waits_the_grace_when_every_task_has_finished() { let mut s = sup(24, 80); + s.set_kill_grace(Duration::from_millis(200)); for _ in 0..2 { spawn(&mut s, "true", here()); } @@ -1510,12 +1514,20 @@ fn shutdown_is_prompt_when_every_group_is_already_empty() { let t0 = Instant::now(); s.apply(Command::Shutdown); assert!( - t0.elapsed() < Duration::from_millis(500), - "shutdown of already-empty groups took {:?}: the early exit is gone", + t0.elapsed() >= Duration::from_millis(200), + "shutdown of completed tasks skipped the grace: {:?}", t0.elapsed() ); } +#[test] +fn shutdown_is_prompt_when_the_fleet_is_empty() { + let mut s = sup(24, 80); + let t0 = Instant::now(); + s.apply(Command::Shutdown); + assert!(t0.elapsed() < Duration::from_millis(500)); +} + /// Session paths follow the connection's launch context: a hello env /// carrying `FLEETCOM_CONFIG_DIR` decides where save, list, and load look. /// The context env holds *only* the override, so anything this process's diff --git a/src/task.rs b/src/task.rs index 9ffdb07..5edc425 100644 --- a/src/task.rs +++ b/src/task.rs @@ -134,11 +134,7 @@ pub struct Task { kill_sent: bool, /// Whether the leader has been reaped; its process group must not be /// signalled afterward because the ID may have been reused (`terminate` - /// and `force_kill` gate on this). The signal-0 existence probe - /// (`group_gone`) is the one carve-out: it delivers nothing, so a - /// recycled ID cannot be harmed, and its errors are one-sided; ESRCH is - /// conclusive while a stale "exists" only extends a wait that stays - /// bounded by the shutdown grace. + /// and `force_kill` gate on this). reaped: bool, } @@ -376,8 +372,7 @@ impl Task { /// reaping it. `WNOWAIT` leaves the zombie in place, which is what keeps /// the pid (and therefore the pgid) reserved so the group stays signalable /// for the task's whole life; see the `reaped` field. The zombie is - /// collected exactly once: at teardown (`collect`), or by the shutdown - /// emptiness probe (`group_gone`). + /// collected at teardown, after SIGKILL. pub fn poll_exit(&mut self) -> io::Result<()> { if self.finished.is_some() || self.reaped { return Ok(()); @@ -720,55 +715,6 @@ impl Task { // clone drops when it exits, closing the queue. self.input_tx.take(); } - - /// Whether this task's process group is observably gone: leader reaped - /// and a signal-0 group probe answering ESRCH. The shutdown wait's exit - /// test; nothing else may call it, because it spends the zombie. - /// - /// The order inside one call is load-bearing. An unreaped zombie leader - /// keeps the group answering kill-style probes regardless of member - /// count (Linux reports it Ok, macOS EPERM, never ESRCH), so emptiness - /// is unobservable until the leader is reaped: reap first, probe second, - /// in the same pass, before the freed pid could plausibly recycle. Later - /// calls re-probe a long-reaped ID, which is safe only because the - /// probe's errors are one-sided: surviving members keep the pgid - /// reserved (a pid still serving as a live group's ID is not reissued), - /// so "exists" stays truthful while anyone remains; a recycled ID - /// misreads only as "exists", a bounded wait, never a stray signal; and - /// ESRCH cannot be wrong, since an ID with no group behind it cannot be - /// this group with members. Real signals get no such carve-out (see - /// `reaped`). - /// - /// The reap spends the pgid reservation `force_kill` relies on: a group - /// that still has members afterward can no longer be KILL-escalated, so - /// TERM-refusing members outlive shutdown and reparent to init. That is - /// the price of observing emptiness at all; the graveyard declines to - /// pay it and keeps its zombies until `kill_sent` (see - /// `Supervisor::reap`). - pub fn group_gone(&mut self) -> bool { - let Some(pid) = self.pid else { - // No pid was ever known: nothing waitable or signalable exists. - return true; - }; - if self.finished.is_none() { - // A live leader is a live group; the zombie-spending reap below - // must never run before the leader has exited. - return false; - } - if !self.reaped { - self.collect(); - if !self.reaped { - // Transient waitid failure: hold shutdown and retry next pass. - return false; - } - } - // Only ESRCH reads as gone. Ok is a live signalable member; EPERM is - // a member that exists but is beyond our signals. Both hold the wait. - matches!( - killpg(Pid::from_raw(pid as i32), None::), - Err(Errno::ESRCH) - ) - } } impl Drop for Task { diff --git a/src/task_tests.rs b/src/task_tests.rs index d021b4c..bf2a7f4 100644 --- a/src/task_tests.rs +++ b/src/task_tests.rs @@ -179,63 +179,25 @@ fn killed_leader_latches_137_via_collect() { assert_eq!(t.exit_code, Some(137)); } -/// The shutdown probe reaps the exited leader, then probes the group in -/// the same pass: a zombie-only group turns gone in that one call. The -/// pre-reap assertions pin why the reap must come first: the zombie -/// alone keeps the group id resolvable for kill-style probes. +/// Collection must retain the group reservation until escalation, including +/// when a repeated TERM request arrives after the leader has exited. #[test] -fn group_gone_reaps_then_probes_past_the_zombie() { - use nix::errno::Errno; +fn exited_leader_is_collectible_only_after_kill() { let mut t = spawn(30, "exit 0"); wait_finished(&mut t); - let pgid = Pid::from_raw(t.pid.expect("spawn always yields a pid") as i32); - // Zombie in place: the probe answer is Ok on Linux, EPERM on macOS, - // never ESRCH, so emptiness is invisible before the reap. - assert_ne!( - killpg(pgid, None::), - Err(Errno::ESRCH), - "an unreaped zombie must keep the group id resolvable" - ); - assert!( - t.group_gone(), - "a zombie-only group must probe gone in one reap+probe pass" - ); - // The probe spent the zombie: the group id no longer resolves. - assert_eq!(killpg(pgid, None::), Err(Errno::ESRCH)); -} - -/// A member that survives the leader holds the probe after the reap, -/// and the probe turns gone once that member dies. -#[test] -fn group_gone_holds_while_a_member_survives() { - use nix::sys::signal::kill; - let dir = temp("task_gone"); - let spid = dir.join("spid"); - // `trap '' HUP` first so the background child survives its session - // leader's exit and remains available for the group probe. - let cmd = format!("trap '' HUP; sleep 300 & echo $! > {}", spid.display()); - let mut t = Task::spawn(31, &cmd, &cmd, &here(), 24, 80, 2000, &sh_env(), no_waker()).unwrap(); - wait_finished(&mut t); - let straggler = read_pid(&spid); - - assert!(!t.group_gone(), "a surviving member must hold the probe"); - assert!(t.reaped, "the probe reaps the exited leader to see past it"); - - let _ = kill(straggler, Signal::SIGKILL); - assert!( - wait_until(Duration::from_secs(5), || t.group_gone()), - "the group must probe gone once its last member dies" - ); -} - -/// `finished` gates the zombie-spending reap: a leader that has not -/// exited is never reaped (or waited on) by the probe. -#[test] -fn group_gone_never_reaps_a_live_leader() { - let mut t = spawn(32, "sleep 300"); - assert!(!t.group_gone(), "a live leader is a live group"); - assert!(!t.reaped, "the probe must not reap a running leader"); + assert!(!t.try_collect()); t.terminate(); + let term_sent = t.term_sent.unwrap(); + t.terminate(); + assert_eq!( + t.term_sent, + Some(term_sent), + "repeated TERM reset the grace" + ); + assert!(!t.try_collect()); + assert!(kill(Pid::from_raw(t.pid.unwrap() as i32), None).is_ok()); + t.force_kill(); + assert!(t.try_collect()); } /// Scrollback clamps at both ends and input returns to live output. From 8726955ec83df8def6d4369aa41a4d3689997f05 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sat, 5 Sep 2026 16:17:11 -0700 Subject: [PATCH 2/5] Refactor harness configuration handling and improve task command preservation --- README.md | 6 +- docs/agent-resume.md | 37 +-- docs/sessions.md | 2 +- src/harness/claude.rs | 115 +------ src/harness/codex.rs | 271 +---------------- src/harness/grok.rs | 428 +------------------------- src/harness/mod.rs | 174 ++--------- src/harness/omp.rs | 523 +------------------------------- src/supervisor.rs | 30 +- src/supervisor_capture_tests.rs | 324 ++++++-------------- src/supervisor_tests.rs | 2 +- src/task.rs | 2 +- src/testutil.rs | 53 +--- 13 files changed, 194 insertions(+), 1773 deletions(-) diff --git a/README.md b/README.md index db38370..2b5974f 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ Organize related tasks into named groups, even when they run in different direct ### Resume agent sessions -Start `claude`, `codex`, `grok`, or `omp` normally. When you rerun the task or reload a saved session, `fleetcom` resumes the same conversation automatically. +Start `claude`, `codex`, `grok`, or `omp` normally. When you rerun the task or reload a saved session, `fleetcom` resumes the conversation when its ID is known; otherwise, it runs the authored command. ## Operational model @@ -40,7 +40,7 @@ Running several long-lived commands is pesky once they span terminal panes or ne - Delegates tasks to a daemon, so a disconnecting client stops nothing. - Saves and reloads task recipes: directories, commands, group assignments, and display names. - Reruns a completed task in place, keeping its identity, group, and name. -- Preserves `claude`, `codex`, `grok`, and `omp` conversations, so saved or rerun tasks resume instead of starting fresh. +- Captures `claude`, `codex`, `grok`, and `omp` conversation IDs for saved and rerun tasks. - Automatically snapshots the current task set for recovery. ## Documentation @@ -102,7 +102,7 @@ Every task runs in its own pseudo-terminal, emulated with `alacritty_terminal`. - Several long-lived commands need one place for observation, tagging, and attachment. - Jobs must survive a terminal closing and remain available for reattachment. - The same command set is launched often enough to justify a saved session. -- Agent sessions (`claude`, `codex`, `grok`, `omp`) must resume their conversations on rerun rather than start new ones. +- Captured agent conversations (`claude`, `codex`, `grok`, `omp`) should resume on rerun. ### When to avoid `fleetcom` diff --git a/docs/agent-resume.md b/docs/agent-resume.md index 438ee10..3682769 100644 --- a/docs/agent-resume.md +++ b/docs/agent-resume.md @@ -8,10 +8,10 @@ Start a supported agent without flags: 1. Press `n` and run `claude`, `codex`, `grok`, or `omp`. The task appears in the dashboard under the command you typed. Instrumentation changes only the string executed through `$SHELL -c`, so a direct spawn still displays the requested command. 2. Work in it. `Enter` attaches; `Ctrl-\` returns to the dashboard. Depending on the agent, `fleetcom` pins an ID at launch and may update it from a hook, notifier, or extension while the task runs or from terminal output after it exits. -3. Press `w`, enter a session name, and press `Enter`. If the earlier sources produced no ID, the save also checks the agent's on-disk session store. A captured bare command becomes its canonical resume form, such as `claude --resume ''`. +3. Press `w`, enter a session name, and press `Enter`. A captured bare command becomes its canonical resume form, such as `claude --resume ''`. Without a known ID, the save preserves the authored command. 4. Run `fleetcom `, or press `o` in the dashboard, to start new processes from the saved commands. A stored resume command reopens its captured conversation. -On a finished agent task, `r` uses the captured launch, hook, notifier, registry, or exit ID without performing save-time filesystem correlation. A registry record remains eligible after exit if it is still present. The replacement keeps the task's ID, tag, group, and name. After a successful rewrite, the row shows the resume command because it has become the task's launch recipe; a [saved session](sessions.md) records the same string. +On a finished agent task, `r` uses the captured launch, hook, notifier, extension, registry, or exit ID. A registry record remains eligible after exit if it is still present. The replacement keeps the task's ID, tag, group, and name. After a successful rewrite, the row shows the resume command because it has become the task's launch recipe; a [saved session](sessions.md) records the same string. Capture is best-effort and narrow by design. A command carrying a prompt, extra flags, or shell syntax stays opaque and saves verbatim. An accepted command with no available ID also saves unchanged. In both cases, loading the recipe reruns the original command. @@ -58,7 +58,7 @@ Claude also publishes one `/sessions/.json` record per session A record counts only when its `kind` is `interactive` and its `pid`, `cwd`, and `startedAt` match the task. The PID must match the filename, the working directories must be identical or resolve to the same path, and the process start must fall within 30 seconds of the task spawn. Missing, malformed, or mismatched records contribute no evidence. The dashboard also maps a matching record's `waiting` status to the top tier of its [preview cascade](commands.md#peek); other statuses do not affect the preview. -After the process exits and the PTY reader reaches EOF, the harness scans the retained terminal text for the last `claude --resume ` hint. Save-time filesystem correlation checks `/projects//.jsonl`, where the slug replaces `/` and `.` in the absolute working directory with `-`. +After the process exits and the PTY reader reaches EOF, the harness scans the retained terminal text for the last `claude --resume ` hint. ### `codex` @@ -72,13 +72,13 @@ After each turn, the notifier writes the `agent-turn-complete` JSON argument to Replacing a configured notifier would change user behavior. When the effective Codex configuration contains a one-line `notify` array of non-empty basic strings, the capture script executes that notifier after writing the capture file. Its argv is carried in `FLEETCOM_NOTIFY_CHAIN`, joined by newlines, and the notification payload is appended. An empty, multiline, ambiguous, or unsupported `notify` value disables the injected override so the configured route remains unchanged. The line-based configuration reader checks `config.toml` and the profile selected by its first `profile = ...` assignment; the profile's notify assignment takes precedence. -The exit scraper accepts `codex resume ` and `codex resume, then select ()`, using only the UUID. Save-time filesystem correlation checks dated rollout directories under `/sessions/YYYY/MM/DD/`. A rollout matches when its v7 UUID timestamp is within 30 seconds of task spawn and the first record names the task's working directory. The search covers the spawn's UTC date plus or minus two days because the directory date is local time. +The exit scraper accepts `codex resume ` and `codex resume, then select ()`, using only the UUID. ### `grok` Grok accepts a launch-time ID but exposes no injectable live-capture channel. A bare command therefore receives `--session-id ''`, while a canonical resume command needs no instrumentation. -After exit, the harness scans retained terminal text for the last `grok -r ` or `grok --resume ` hint. Save-time filesystem correlation checks `/sessions///`, percent-encoding the canonical working directory, falling back to a group whose `.cwd` file names that path when the encoded name is too long, and ignoring `session_kind: subagent` directories. +After exit, the harness scans retained terminal text for the last `grok -r ` or `grok --resume ` hint. ### `omp` @@ -92,12 +92,6 @@ omp cannot pin an ID at launch: it has no `--session-id`, and `--resume` require After exit, the harness scans retained terminal text for the last trusted `omp --resume ` hint. It accepts ordinary exit hints and `Main:` entries in `[Recovery]` blocks. Other labels identify subagent sessions that `omp --resume` cannot open, so they contribute no exit evidence. The aliases `-r`, `--session`, and `-c` remain opaque because `fleetcom` rewrites only the canonical form it detects exactly. -Save-time filesystem correlation reads `//_.jsonl`, where the sessions root comes from omp's own variable chain rather than one home override; the [environment-variable table](#environment-variables) lists it. `PI_CODING_AGENT_SESSION_DIR` is the exception: omp passes that path straight through as the session file's parent and never computes a bucket, so the store is flat under it. The scan covers the root and one level below without inferring which layout is in play. - -Correlation enumerates buckets instead of deriving their names, then verifies each session header's `cwd` against the task path and its canonical target. A candidate must be the sole UUIDv7 session created within 30 seconds of task spawn. UUIDs found in multiple buckets count once. - -An empty sessions root or bucket contributes no candidate. - ## ID precedence Several channels can identify different conversations during one task. To make the result deterministic, `fleetcom` chooses the first available ID in this order: @@ -106,7 +100,8 @@ Several channels can identify different conversations during one task. To make t 2. The current capture-file payload. 3. The live session registry, implemented by `claude`. 4. The ID pinned or targeted at spawn. -5. Save-time filesystem correlation, when exactly one store entry matches the task and the 30-second spawn window. + +Named saves, recovery snapshots, and reruns use this same precedence. `fleetcom` does not scan session stores to infer conversation ownership: a nearby transcript or rollout cannot identify which task owns it. The registry outranks the spawn pin because it can contain a session ID selected after launch, including one created by `/clear`. The capture file outranks the registry. @@ -123,7 +118,7 @@ The program word is preserved as typed. If no valid ID is available, the origina ## Validation boundary -Every captured value eventually enters a shell command, which makes validation the security boundary. Accepted IDs contain exactly lowercase hexadecimal characters in the `8-4-4-4-12` UUID shape. Capture payloads, terminal hints, registry records, store names, and the final command builder all apply the same check. Malformed values are ignored rather than interpolated. +Every captured value eventually enters a shell command, which makes validation the security boundary. Accepted IDs contain exactly lowercase hexadecimal characters in the `8-4-4-4-12` UUID shape. Capture payloads, terminal hints, registry records, and the final command builder all apply the same check. Malformed values are ignored rather than interpolated. ## Extending capture @@ -135,10 +130,9 @@ Each tool implements the `Harness` trait in [`src/harness/mod.rs`](../src/harnes - `scrape_exit` reads an ID from retained terminal text. - `live_session_id` reads the ID a live session publishes on disk. It defaults to `None` for tools that publish no registry. - `live_blocked_status` reads that same registry for one display fact: whether the tool says it is blocked on the user. It returns preview text, never an ID, and defaults to `None`. -- `correlate_fs` finds one matching on-disk session. -- `resolve_home` turns the task's launch environment into the tool's store root. +- `resolve_home` resolves configuration needed by instrumentation or the live registry. It defaults to `None`. -The supervisor supplies the launch environment and delegates the decision to `resolve_home`. Its default is the two-step rule three of the four tools follow: the tool-specific variable first, then `$HOME` plus the tool's dot directory. omp overrides it, because its store root comes from a chain of variables and a filesystem-conditional XDG branch that no single override can express. Either way, the resolved path remains attached to the task for later filesystem correlation. +The supervisor supplies the launch environment to `resolve_home`. Claude and Codex read their explicit override first, then `$HOME` plus their dot directory. When neither is supplied, configuration reads fall back to the supervisor's platform home. The resolved path stays attached to the task so Claude registry reads continue using its launch-time home after a reconnect. Grok and omp need no home resolution; their environment passes through to the child unchanged. ## Environment variables @@ -147,12 +141,5 @@ The supervisor supplies the launch environment and delegates the decision to `re | `FLEETCOM_RUNTIME_DIR` | Explicit capture-asset root as well as the daemon runtime override. | | `FLEETCOM_CAPTURE_FILE` | Per-run capture file used by the injected hook, notifier, or extension module. | | `FLEETCOM_NOTIFY_CHAIN` | Newline-joined argv for the configured Codex notifier; empty when none is active. | -| `CLAUDE_CONFIG_DIR` | Claude home holding the `sessions/.json` registry and the transcripts used for correlation; defaults to `$HOME/.claude`. | -| `CODEX_HOME` | Codex home used for notify routing and rollout correlation; defaults to `$HOME/.codex`. | -| `GROK_HOME` | Grok home used for session-directory correlation; defaults to `$HOME/.grok`. | -| `PI_CODING_AGENT_SESSION_DIR` | omp sessions root, used verbatim for correlation. The rest of omp's chain builds that path instead of naming it. | -| `PI_CODING_AGENT_DIR` | omp agent directory, whose `sessions` subdirectory is the store. A selected profile ignores it. | -| `PI_CONFIG_DIR` | omp config directory under `$HOME`; defaults to `.omp`. Under `fleetcom`, an absolute value replaces `$HOME`. | -| `OMP_PROFILE` | omp profile, read by presence: it selects a profile when non-empty and suppresses `PI_PROFILE` when empty. | -| `PI_PROFILE` | omp profile used only when `OMP_PROFILE` is absent. A profile inserts `profiles/` under the config directory. | -| `XDG_DATA_HOME` | Redirects the still-default omp agent directory to `/omp`, flattening the `agent/` level, when that target already exists. | +| `CLAUDE_CONFIG_DIR` | Claude home holding the `sessions/.json` registry; defaults to `$HOME/.claude`. | +| `CODEX_HOME` | Codex home used for notify routing; defaults to `$HOME/.codex`. | diff --git a/docs/sessions.md b/docs/sessions.md index 0e4e4b9..75390ec 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -51,7 +51,7 @@ The file is plain JSON and practical to edit by hand. Editing the `name` field c Commands with neither a group nor a name use the string form. String and object entries can appear in the same directory array. -A bare agent command does not identify its conversation, so saving it verbatim would start another one on load. When `fleetcom` captures an ID for `claude`, `codex`, `grok`, or `omp`, it stores the resume form instead. The result remains an ordinary command string that can run directly in a shell: +A bare agent command does not identify its conversation, so saving it verbatim would start another one on load. When `fleetcom` captures an ID for `claude`, `codex`, `grok`, or `omp`, it stores the resume form instead. Without a known ID, named saves and recovery snapshots preserve the authored command. The result remains an ordinary command string that can run directly in a shell: ```json { diff --git a/src/harness/claude.rs b/src/harness/claude.rs index c51a8cb..48a966e 100644 --- a/src/harness/claude.rs +++ b/src/harness/claude.rs @@ -1,8 +1,7 @@ //! Claude session capture uses a launch-time `--session-id`, a `SessionStart` //! hook, the live session registry, and the exit-time resume hint. Bare launches //! pin a v4 UUID; accepted launches install the hook through `--settings`. -//! Live lookup reads `/sessions/.json`; fallback correlation -//! reads project transcripts. +//! Live lookup reads `/sessions/.json`. use std::{ fs, @@ -12,19 +11,15 @@ use std::{ use super::summary::AWAITING_APPROVAL; use super::{ - CAPTURE_ENV, CapturePaths, Harness, Invocation, SpawnPlan, capture_id, is_uuid, last_hint, - pin_plan, same_cwd, shell_quote, sole_id, unix_millis, within_window, within_window_ms, + CAPTURE_ENV, CapturePaths, Harness, Invocation, SpawnPlan, capture_id, home_root, is_uuid, + last_hint, pin_plan, resolve_home, shell_quote, }; pub struct Claude; impl Harness for Claude { - fn home_env_var(&self) -> &'static str { - "CLAUDE_CONFIG_DIR" - } - - fn home_dot_dir(&self) -> &'static str { - ".claude" + fn resolve_home(&self, env: &dyn Fn(&str) -> Option) -> Option { + resolve_home(env, "CLAUDE_CONFIG_DIR", ".claude") } fn shape(&self) -> (&'static str, &'static str) { @@ -80,11 +75,6 @@ impl Harness for Claude { rec.waiting .then(|| waiting_preview(rec.waiting_for.as_deref())) } - - fn correlate_fs(&self, cwd: &Path, spawned: SystemTime, home: Option<&Path>) -> Option { - let dir = self.home_root(home)?.join("projects").join(slug(cwd)?); - unique_in_window(dir, spawned) - } } /// Validated fields used to correlate a registry record with a task and render @@ -135,7 +125,8 @@ fn parse_record(text: &str) -> Option { /// Read `sessions/.json` and require its PID, working directory, and /// process start to match the task. The unreaped task leader reserves its PID; /// the directory and start-time checks reject stale records already present at -/// that path. A mismatch returns `None` because the ID may enter a shell command. +/// that path. The start time may differ by at most 30 seconds. A mismatch +/// returns `None` because the ID may enter a shell command. fn record_for_pid( pid: u32, cwd: &Path, @@ -143,50 +134,17 @@ fn record_for_pid( home: Option<&Path>, ) -> Option { let pid = i32::try_from(pid).ok()?; - let dir = Claude.home_root(home)?.join("sessions"); + let dir = home_root(home, ".claude")?.join("sessions"); let text = fs::read_to_string(dir.join(format!("{pid}.json"))).ok()?; let rec = parse_record(&text)?; + let spawn_ms = spawned + .duration_since(SystemTime::UNIX_EPOCH) + .ok()? + .as_millis(); (rec.pid == pid - && same_cwd(&rec.cwd, cwd, cwd.canonicalize().ok().as_deref()) - && within_window_ms(rec.started_at, unix_millis(spawned)?)) - .then_some(rec) -} - -/// Return the UUID stem of the sole `.jsonl` transcript created within -/// [`super::CORRELATE_WINDOW`] of `spawned`. Unreadable entries and creation -/// times are ignored; directory errors, zero or multiple candidates, and an -/// invalid sole stem return `None`. -fn unique_in_window(dir: PathBuf, spawned: SystemTime) -> Option { - let mut candidates: Vec = Vec::new(); - for entry in fs::read_dir(dir).ok()?.flatten() { - let path = entry.path(); - if path.extension().and_then(|e| e.to_str()) != Some("jsonl") { - continue; - } - let Some(name) = path.file_stem().and_then(|s| s.to_str()) else { - continue; - }; - let Ok(created) = entry.metadata().and_then(|m| m.created()) else { - continue; - }; - if !within_window(created, spawned) { - continue; - } - candidates.push(name.to_string()); - } - sole_id(candidates) -} - -/// Convert an absolute working directory to Claude's project slug by replacing -/// `/` and `.` with `-` (`/a/b.c` becomes `-a-b-c`). Non-UTF-8 paths have no -/// representable slug. -fn slug(cwd: &Path) -> Option { - Some( - cwd.to_str()? - .chars() - .map(|c| if c == '/' || c == '.' { '-' } else { c }) - .collect(), - ) + && (rec.cwd == cwd || cwd.canonicalize().is_ok_and(|canon| rec.cwd == canon)) + && rec.started_at.abs_diff(spawn_ms) <= 30_000) + .then_some(rec) } #[cfg(test)] @@ -320,47 +278,6 @@ mod tests { assert_eq!(Claude.scrape_exit(&format!("claude --resume {ID}ff")), None); } - #[test] - fn correlate_fs_requires_a_unique_in_window_transcript() { - let home = temp("claude_correlate"); - // Slug: `/` and `.` both become `-`. - let cwd = Path::new("/a/b.c"); - let dir = home.join("projects").join("-a-b-c"); - fs::create_dir_all(&dir).unwrap(); - fs::write(dir.join(format!("{ID}.jsonl")), "{}").unwrap(); - let now = SystemTime::now(); - - assert_eq!( - Claude.correlate_fs(cwd, now, Some(&home)).as_deref(), - Some(ID) - ); - // Outside the window: the transcript predates the spawn by minutes. - let late = now + std::time::Duration::from_secs(120); - assert_eq!(Claude.correlate_fs(cwd, late, Some(&home)), None); - // Wrong project directory. - assert_eq!( - Claude.correlate_fs(Path::new("/other"), now, Some(&home)), - None - ); - - // A second in-window transcript makes the match ambiguous. - fs::write(dir.join(format!("{OTHER}.jsonl")), "{}").unwrap(); - assert_eq!(Claude.correlate_fs(cwd, now, Some(&home)), None); - } - - #[test] - fn correlate_fs_rejects_a_unique_non_uuid_stem() { - let home = temp("claude_nonuuid"); - let cwd = Path::new("/w"); - let dir = home.join("projects").join("-w"); - fs::create_dir_all(&dir).unwrap(); - fs::write(dir.join("agent-notes.jsonl"), "{}").unwrap(); - assert_eq!( - Claude.correlate_fs(cwd, SystemTime::now(), Some(&home)), - None - ); - } - /// A complete matching record exposes its validated session ID. #[test] fn record_for_pid_reads_a_live_record() { @@ -454,7 +371,7 @@ mod tests { &record(4242, ID, "/w", LIVE_STARTED, "interactive", ""), ); - // The correlation window includes both endpoints. + // The start-time tolerance includes both endpoints. assert!(record_for_pid(4242, cwd, at_ms(LIVE_STARTED + 30_000), Some(&home)).is_some()); assert!(record_for_pid(4242, cwd, at_ms(LIVE_STARTED - 30_000), Some(&home)).is_some()); // One millisecond outside the window is stale. diff --git a/src/harness/codex.rs b/src/harness/codex.rs index 385911a..ee0de0f 100644 --- a/src/harness/codex.rs +++ b/src/harness/codex.rs @@ -1,26 +1,23 @@ //! Codex does not let the caller select an ID at launch. This harness instead //! injects a `notify` override, chains compatible configured notifiers, and -//! scans supported exit lines for an ID. When neither channel yields one, it -//! correlates rollout files under `/sessions/YYYY/MM/DD/`. -//! Missing, empty, and malformed rollouts do not produce a candidate. +//! scans supported exit lines for an ID. -use std::{fmt::Write as _, fs, path::Path, time::SystemTime}; +use std::{ + fmt::Write as _, + fs, + path::{Path, PathBuf}, +}; use super::{ CAPTURE_ENV, CapturePaths, Harness, Invocation, NOTIFY_CHAIN_ENV, SpawnPlan, capture_id, - is_uuid, jsonl_head, last_hint, leading_uuid, push_unique, same_cwd, shell_quote, sole_id, - unix_millis, v7_millis, within_window_ms, + home_root, last_hint, leading_uuid, resolve_home, shell_quote, }; pub struct Codex; impl Harness for Codex { - fn home_env_var(&self) -> &'static str { - "CODEX_HOME" - } - - fn home_dot_dir(&self) -> &'static str { - ".codex" + fn resolve_home(&self, env: &dyn Fn(&str) -> Option) -> Option { + resolve_home(env, "CODEX_HOME", ".codex") } fn shape(&self) -> (&'static str, &'static str) { @@ -99,60 +96,6 @@ impl Harness for Codex { } last } - - fn correlate_fs(&self, cwd: &Path, spawned: SystemTime, home: Option<&Path>) -> Option { - let root = self.home_root(home)?; - let spawn_ms = unix_millis(spawned)?; - // Day directories are named by LOCAL date, which std cannot compute - // without a timezone database. The UTC date differs from it by at - // most one day, so probing the UTC date ±2 covers local ±1. - let spawn_days = (spawn_ms / 86_400_000) as i64; - let mut survivors: Vec = Vec::new(); - for day in (spawn_days - 2)..=(spawn_days + 2) { - let (y, m, d) = crate::format::civil_from_days(day); - let dir = root - .join("sessions") - .join(format!("{y:04}")) - .join(format!("{m:02}")) - .join(format!("{d:02}")); - let Ok(entries) = fs::read_dir(&dir) else { - continue; - }; - for entry in entries.flatten() { - let name = entry.file_name(); - let Some(stem) = name - .to_str() - .and_then(|n| n.strip_prefix("rollout-")) - .and_then(|n| n.strip_suffix(".jsonl")) - else { - continue; - }; - // The 20-byte timestamp prefix precedes the thread ID. A - // suffix may carry a second UUID, so reading the final UUID - // can select a rollout ID instead of the conversation. - let Some(ids) = stem.get(20..) else { - continue; - }; - let id = ids.split_once('_').map_or(ids, |(thread, _)| thread); - if !is_uuid(id) { - continue; - } - // Correlate with the v7 ID's embedded UTC instant; the - // filename timestamp is local wall-clock time. - let Some(ms) = v7_millis(id) else { continue }; - if !within_window_ms(u128::from(ms), spawn_ms) { - continue; - } - if !line1_admits(&entry.path(), cwd) { - continue; - } - // Multiple rollouts may name the same thread. Correlation - // counts that thread once. - push_unique(&mut survivors, id.to_string()); - } - } - sole_id(survivors) - } } /// Whether Codex notification capture can preserve the configured route. @@ -171,7 +114,7 @@ enum NotifyRoute { /// deliberately line-based: duplicate assignments are ambiguous and produce /// [`NotifyRoute::Opaque`]. fn config_notify_route(home: Option<&Path>) -> NotifyRoute { - let Some(root) = Codex.home_root(home) else { + let Some(root) = home_root(home, ".codex") else { return NotifyRoute::Vacant; }; let text = fs::read_to_string(root.join("config.toml")).unwrap_or_default(); @@ -280,29 +223,6 @@ fn toml_escape(s: &str) -> String { out } -/// Check the rollout's first record for a matching `cwd` and no explicit -/// spawned-thread provenance. Missing and unrecognized `thread_source` values -/// remain eligible; `"subagent"` or any `parent_thread_id` rejects the record. -fn line1_admits(path: &Path, cwd: &Path) -> bool { - let Some(records) = jsonl_head(path, 1) else { - return false; - }; - let Some(meta) = records[0].as_ref() else { - return false; - }; - let payload = &meta["payload"]; - if payload["thread_source"].as_str() == Some("subagent") - || !payload["parent_thread_id"].is_null() - { - return false; - } - // Rollouts can contain the physical cwd while the task retains a symlinked - // path. Canonicalize the task path before rejecting the match. - payload["cwd"] - .as_str() - .is_some_and(|c| same_cwd(Path::new(c), cwd, cwd.canonicalize().ok().as_deref())) -} - #[cfg(test)] mod tests { use std::path::PathBuf; @@ -310,7 +230,7 @@ mod tests { use super::*; use crate::{ harness::fixtures::{OTHER, assert_all_opaque, assert_corpus_scrape, paths}, - testutil::{CORPUS_COLS, Scratch, temp, v7_at, write_rollout, write_rollout_named}, + testutil::{CORPUS_COLS, Scratch, temp}, }; /// Codex's own launch and resume commands carry v7 IDs; the shared v4 @@ -662,175 +582,6 @@ mod tests { assert_eq!(config_notify_route(Some(&home)), NotifyRoute::Opaque); } - #[test] - fn correlate_fs_requires_a_unique_cwd_matched_rollout() { - let home = temp("codex_correlate"); - let spawn_ms: u64 = 1_785_000_000_000; // 2026-07-25T02:40Z - let spawned = SystemTime::UNIX_EPOCH + std::time::Duration::from_millis(spawn_ms); - - let id = write_rollout(&home, spawn_ms + 4_000, 1, Path::new("/work/proj")); - assert_eq!( - Codex - .correlate_fs(Path::new("/work/proj"), spawned, Some(&home)) - .as_deref(), - Some(id.as_str()) - ); - // A different task directory does not match this rollout. - assert_eq!( - Codex.correlate_fs(Path::new("/elsewhere"), spawned, Some(&home)), - None - ); - - // Outside the ±30 s window: excluded. - write_rollout(&home, spawn_ms + 90_000, 2, Path::new("/late/proj")); - assert_eq!( - Codex.correlate_fs(Path::new("/late/proj"), spawned, Some(&home)), - None - ); - - // Two in-window rollouts from the same directory are ambiguous. - write_rollout(&home, spawn_ms + 8_000, 3, Path::new("/work/proj")); - assert_eq!( - Codex.correlate_fs(Path::new("/work/proj"), spawned, Some(&home)), - None - ); - } - - /// Either spawned-thread provenance field disqualifies a rollout. - #[test] - fn correlate_fs_excludes_spawned_threads() { - let home = temp("codex_subagent"); - let spawn_ms: u64 = 1_785_000_000_000; - let spawned = SystemTime::UNIX_EPOCH + std::time::Duration::from_millis(spawn_ms); - let cwd = Path::new("/work/proj"); - let parent = write_rollout(&home, spawn_ms + 1_000, 1, cwd); - let resolves = |home: &Path| Codex.correlate_fs(cwd, spawned, Some(home)); - - // `thread_source` alone disqualifies the rollout. - write_rollout_named( - &home, - spawn_ms + 3_000, - 2, - cwd, - "", - r#","source":{"subagent":{"other":"guardian"}},"thread_source":"subagent""#, - ); - assert_eq!(resolves(&home).as_deref(), Some(parent.as_str())); - - // `parent_thread_id` alone: any value at all names a spawning thread. - write_rollout_named( - &home, - spawn_ms + 5_000, - 3, - cwd, - "", - &format!(r#","parent_thread_id":"{parent}""#), - ); - assert_eq!(resolves(&home).as_deref(), Some(parent.as_str())); - - // A second eligible thread makes correlation ambiguous. - write_rollout(&home, spawn_ms + 7_000, 4, cwd); - assert_eq!(resolves(&home), None); - } - - /// Unknown thread sources remain eligible unless another field marks the - /// rollout as spawned. - #[test] - fn correlate_fs_admits_thread_sources_it_does_not_know() { - let spawn_ms: u64 = 1_785_000_000_000; - let spawned = SystemTime::UNIX_EPOCH + std::time::Duration::from_millis(spawn_ms); - let cwd = Path::new("/work/proj"); - for source in ["user", "some_future_kind"] { - let home = temp("codex_thread_source"); - let id = write_rollout_named( - &home, - spawn_ms + 1_000, - 1, - cwd, - "", - &format!(r#","thread_source":"{source}""#), - ); - assert_eq!( - Codex.correlate_fs(cwd, spawned, Some(&home)).as_deref(), - Some(id.as_str()), - "{source:?}" - ); - } - } - - /// A suffixed rollout filename carries the thread ID before the rollout ID. - #[test] - fn correlate_fs_reads_the_thread_id_not_the_rollout_id() { - let spawn_ms: u64 = 1_785_000_000_000; - let spawned = SystemTime::UNIX_EPOCH + std::time::Duration::from_millis(spawn_ms); - let cwd = Path::new("/work/proj"); - let rollout_id = v7_at(spawn_ms + 1_000, 9); - // Both names coexist and resolve to one deduplicated thread. - let home = temp("codex_revert_name"); - for suffix in [String::new(), format!("_{rollout_id}")] { - let thread = write_rollout_named(&home, spawn_ms + 1_000, 1, cwd, &suffix, ""); - assert_ne!(thread, rollout_id); - assert_eq!( - Codex.correlate_fs(cwd, spawned, Some(&home)).as_deref(), - Some(thread.as_str()), - "{suffix:?}" - ); - } - } - - /// Correlation matches a physical rollout cwd to a symlinked task cwd. - #[test] - fn correlate_fs_matches_a_symlinked_spawn_path() { - let home = temp("codex_symlink_cwd"); - let spawn_ms: u64 = 1_785_000_000_000; - let spawned = SystemTime::UNIX_EPOCH + std::time::Duration::from_millis(spawn_ms); - - let real = home.join("real"); - fs::create_dir_all(&real).unwrap(); - let link = home.join("link"); - std::os::unix::fs::symlink(&real, &link).unwrap(); - - // The rollout names the resolved path; the task carries the link. - let id = write_rollout(&home, spawn_ms + 1_000, 1, &real.canonicalize().unwrap()); - assert_eq!( - Codex.correlate_fs(&link, spawned, Some(&home)).as_deref(), - Some(id.as_str()) - ); - // An unrelated directory still fails, resolved or not. - assert_eq!(Codex.correlate_fs(&home, spawned, Some(&home)), None); - } - - /// The ±2-day probe includes a rollout in the adjacent day directory. - #[test] - fn correlate_fs_spans_adjacent_day_directories() { - let home = temp("codex_dayspan"); - let spawn_ms: u64 = 1_785_000_000_000; - let spawned = SystemTime::UNIX_EPOCH + std::time::Duration::from_millis(spawn_ms); - - let id = v7_at(spawn_ms + 2_000, 7); - let (y, m, d) = crate::format::civil_from_days((spawn_ms / 86_400_000) as i64 - 1); - let dir = home - .join("sessions") - .join(format!("{y:04}")) - .join(format!("{m:02}")) - .join(format!("{d:02}")); - fs::create_dir_all(&dir).unwrap(); - fs::write( - dir.join(format!("rollout-2026-07-24T19-40-02-{id}.jsonl")), - format!( - r#"{{"timestamp":"x","type":"session_meta","payload":{{"id":"{id}","cwd":"/w"}}}}"# - ), - ) - .unwrap(); - - assert_eq!( - Codex - .correlate_fs(Path::new("/w"), spawned, Some(&home)) - .as_deref(), - Some(id.as_str()) - ); - } - /// A preceding full-width row does not merge with the session-ID row after /// terminal emulation. #[test] diff --git a/src/harness/grok.rs b/src/harness/grok.rs index 9b58a9b..2a8ab4a 100644 --- a/src/harness/grok.rs +++ b/src/harness/grok.rs @@ -1,32 +1,14 @@ //! Grok has no injectable live-capture channel. Bare launches instead pin a v4 //! UUID, and completed tasks expose either `grok -r ` or -//! `grok --resume ` in terminal output. The filesystem fallback -//! correlates `/sessions///` directories: the group is -//! a percent-encoding of the working directory, or a long-name slug whose -//! `.cwd` file names that path. +//! `grok --resume ` in terminal output. -use std::{ - fs, - path::{Path, PathBuf}, - time::{Duration, SystemTime}, -}; +use std::path::Path; -use super::{ - CapturePaths, Harness, Invocation, SpawnPlan, last_hint, pin_plan, push_unique, sole_id, - within_window, -}; +use super::{CapturePaths, Harness, Invocation, SpawnPlan, last_hint, pin_plan}; pub struct Grok; impl Harness for Grok { - fn home_env_var(&self) -> &'static str { - "GROK_HOME" - } - - fn home_dot_dir(&self) -> &'static str { - ".grok" - } - fn shape(&self) -> (&'static str, &'static str) { ("grok", "--resume") } @@ -45,224 +27,14 @@ impl Harness for Grok { // The last valid short or long resume hint names the conversation. last_hint(text, &["grok -r ", "grok --resume "]) } - - fn correlate_fs(&self, cwd: &Path, spawned: SystemTime, home: Option<&Path>) -> Option { - let groups = matching_groups(&self.home_root(home)?.join("sessions"), cwd); - unique_session(&groups, spawned) - } -} - -/// Byte-wise URL-encode of a working directory as Grok's session group name. -/// RFC 3986 unreserved bytes stay literal; every other byte becomes uppercase -/// `%XX`. Non-UTF-8 paths have no key. The path is encoded as given. -pub(crate) fn encode_cwd(cwd: &Path) -> Option { - let s = cwd.to_str()?; - let mut out = String::with_capacity(s.len() * 3); - for &b in s.as_bytes() { - if b.is_ascii_alphanumeric() || matches!(b, b'-' | b'.' | b'_' | b'~') { - out.push(b as char); - } else { - const HEX: &[u8; 16] = b"0123456789ABCDEF"; - out.push('%'); - out.push(HEX[(b >> 4) as usize] as char); - out.push(HEX[(b & 0x0f) as usize] as char); - } - } - Some(out) -} - -/// Session-store groups that name `cwd`: the encoded given path, the encoded -/// canonical path when it differs, and any directory whose `.cwd` record is -/// that path. Same path through two aliases is one group. Distinct folders -/// stay in the list; uniqueness is decided on in-window sessions, not here. -fn matching_groups(sessions: &Path, cwd: &Path) -> Vec { - let mut found: Vec = Vec::new(); - - if let Some(p) = encoded_dir(sessions, cwd) { - push_unique(&mut found, p); - } - let canon = cwd.canonicalize().ok(); - if let Some(ref canon) = canon - && canon.as_path() != cwd - && let Some(p) = encoded_dir(sessions, canon) - { - push_unique(&mut found, p); - } - - let given = cwd.to_str(); - let canon_s = canon.as_ref().and_then(|p| p.to_str()); - if let Ok(entries) = fs::read_dir(sessions) { - for entry in entries.flatten() { - if !entry.file_type().is_ok_and(|t| t.is_dir()) { - continue; - } - let Ok(text) = fs::read_to_string(entry.path().join(".cwd")) else { - continue; - }; - let record = cwd_record(&text); - if given == Some(record) || canon_s == Some(record) { - push_unique(&mut found, entry.path()); - } - } - } - found -} - -/// The path grok stored in `.cwd`: the file bytes minus one trailing `\n`, -/// and a `\r` immediately before that `\n` if present. Interior and leading -/// spaces stay; they are part of the directory name. -fn cwd_record(text: &str) -> &str { - text.strip_suffix('\n') - .map(|s| s.strip_suffix('\r').unwrap_or(s)) - .unwrap_or(text) -} - -fn encoded_dir(sessions: &Path, cwd: &Path) -> Option { - let p = sessions.join(encode_cwd(cwd)?); - p.is_dir().then_some(p) -} - -/// The one in-window top-level session across `groups`. Subagent siblings -/// do not count. The same uuid in two groups is one candidate. A unique -/// non-uuid name still yields `None`. -fn unique_session(groups: &[PathBuf], spawned: SystemTime) -> Option { - let mut candidates: Vec = Vec::new(); - for group in groups { - let Ok(entries) = fs::read_dir(group) else { - continue; - }; - for entry in entries.flatten() { - // One directory per session, named by its uuid. Files such as - // the `prompt_history.jsonl` sibling are not sessions. - if !entry.file_type().is_ok_and(|t| t.is_dir()) { - continue; - } - let summary = fs::read_to_string(entry.path().join("summary.json")) - .ok() - .and_then(|text| jzon::parse(&text).ok()); - if summary - .as_ref() - .is_some_and(|v| v["session_kind"].as_str() == Some("subagent")) - { - continue; - } - let ts = summary - .as_ref() - .and_then(|v| v["created_at"].as_str()) - .and_then(parse_created_at) - .or_else(|| entry.metadata().ok()?.created().ok()); - let Some(ts) = ts else { - continue; - }; - if !within_window(ts, spawned) { - continue; - } - let name = entry.file_name(); - let Some(name) = name.to_str() else { - continue; - }; - push_unique(&mut candidates, name.to_string()); - } - } - sole_id(candidates) -} - -/// `YYYY-MM-DDTHH:MM:SS[.frac]Z` as grok writes `created_at`. Any other shape -/// fails so the caller can fall back to directory birth time. -fn parse_created_at(s: &str) -> Option { - let s = s.strip_suffix('Z')?; - let (head, frac) = match s.split_once('.') { - Some((h, f)) => (h, Some(f)), - None => (s, None), - }; - let b = head.as_bytes(); - if b.len() != 19 - || b[4] != b'-' - || b[7] != b'-' - || b[10] != b'T' - || b[13] != b':' - || b[16] != b':' - { - return None; - } - let year = parse_digits(&head[..4])?; - let month = u32::try_from(parse_digits(&head[5..7])?).ok()?; - let day = u32::try_from(parse_digits(&head[8..10])?).ok()?; - let hour = u32::try_from(parse_digits(&head[11..13])?).ok()?; - let minute = u32::try_from(parse_digits(&head[14..16])?).ok()?; - let second = u32::try_from(parse_digits(&head[17..19])?).ok()?; - let nanos = match frac { - None => 0, - Some(f) if !f.is_empty() && f.bytes().all(|c| c.is_ascii_digit()) => frac_nanos(f)?, - _ => return None, - }; - if !valid_ymd(year, month, day) || hour > 23 || minute > 59 || second > 59 { - return None; - } - let days = days_from_civil(year, month, day); - let day_secs = i64::from(hour) * 3600 + i64::from(minute) * 60 + i64::from(second); - let secs = u64::try_from(days.checked_mul(86_400)?.checked_add(day_secs)?).ok()?; - SystemTime::UNIX_EPOCH.checked_add(Duration::new(secs, nanos)) -} - -fn parse_digits(s: &str) -> Option { - if s.is_empty() || !s.bytes().all(|b| b.is_ascii_digit()) { - return None; - } - s.parse().ok() -} - -fn frac_nanos(frac: &str) -> Option { - let take = frac.len().min(9); - let mut n: u32 = frac[..take].parse().ok()?; - for _ in take..9 { - n = n.checked_mul(10)?; - } - Some(n) -} - -fn valid_ymd(year: i64, month: u32, day: u32) -> bool { - let mdays = match month { - 1 | 3 | 5 | 7 | 8 | 10 | 12 => 31, - 4 | 6 | 9 | 11 => 30, - 2 => { - if year.rem_euclid(4) == 0 && (year.rem_euclid(100) != 0 || year.rem_euclid(400) == 0) { - 29 - } else { - 28 - } - } - _ => return false, - }; - (1..=mdays).contains(&day) -} - -/// Inverse of [`crate::format::civil_from_days`]: days since 1970-01-01. -fn days_from_civil(year: i64, month: u32, day: u32) -> i64 { - let y = if month <= 2 { year - 1 } else { year }; - let era = if y >= 0 { y } else { y - 399 } / 400; - let yoe = y - era * 400; - let mp = if month > 2 { - i64::from(month) - 3 - } else { - i64::from(month) + 9 - }; - let doy = (153 * mp + 2) / 5 + i64::from(day) - 1; - let doe = yoe * 365 + yoe / 4 - yoe / 100 + doy; - era * 146_097 + doe - 719_468 } #[cfg(test)] mod tests { - use std::fs; - use super::*; - use crate::{ - harness::{ - fixtures::{ID, OTHER, assert_all_opaque, assert_corpus_scrape, paths}, - is_uuid, - }, - testutil::temp, + use crate::harness::{ + fixtures::{ID, OTHER, assert_all_opaque, assert_corpus_scrape, paths}, + is_uuid, }; /// Grok-specific opaque shapes: flags, the `-r`/`-s`/`=` spellings the @@ -336,194 +108,6 @@ mod tests { assert_eq!(Grok.scrape_exit(&format!("grok -r {ID}ff")), None); } - /// Store keys percent-encode every non-unreserved byte, uppercase hex. - #[test] - fn encode_cwd_matches_the_observed_store_names() { - assert_eq!( - encode_cwd(Path::new("/Users/chris/Documents/Code/Apple/turret")).as_deref(), - Some("%2FUsers%2Fchris%2FDocuments%2FCode%2FApple%2Fturret") - ); - // Dots stay literal. - assert_eq!( - encode_cwd(Path::new("/Users/chris/.claude/jobs/ac6e4777/tmp/groktest")).as_deref(), - Some("%2FUsers%2Fchris%2F.claude%2Fjobs%2Fac6e4777%2Ftmp%2Fgroktest") - ); - // A literal `%` must encode for the mapping to stay injective. - assert_eq!(encode_cwd(Path::new("/a%b")).as_deref(), Some("%2Fa%25b")); - assert_eq!( - encode_cwd(Path::new("/has space")).as_deref(), - Some("%2Fhas%20space") - ); - assert_eq!(encode_cwd(Path::new("/a+b")).as_deref(), Some("%2Fa%2Bb")); - assert_eq!(encode_cwd(Path::new("/ü")).as_deref(), Some("%2F%C3%BC")); - } - - #[test] - fn correlate_fs_requires_a_unique_in_window_session_dir() { - let home = temp("grok_correlate"); - let cwd = Path::new("/work/proj.rs"); - let dir = home.join("sessions").join("%2Fwork%2Fproj.rs"); - fs::create_dir_all(dir.join(ID)).unwrap(); - // The prompt-history sibling is a file, not a session. - fs::write(dir.join("prompt_history.jsonl"), "{}").unwrap(); - let now = SystemTime::now(); - - assert_eq!( - Grok.correlate_fs(cwd, now, Some(&home)).as_deref(), - Some(ID) - ); - // Outside the window: the session predates the spawn by minutes. - let late = now + std::time::Duration::from_secs(120); - assert_eq!(Grok.correlate_fs(cwd, late, Some(&home)), None); - // Wrong working directory. - assert_eq!( - Grok.correlate_fs(Path::new("/other"), now, Some(&home)), - None - ); - - // A second in-window session makes the match ambiguous. - fs::create_dir_all(dir.join(OTHER)).unwrap(); - assert_eq!(Grok.correlate_fs(cwd, now, Some(&home)), None); - } - - #[test] - fn correlate_fs_rejects_a_unique_non_uuid_dir() { - let home = temp("grok_nonuuid"); - let cwd = Path::new("/w"); - let dir = home.join("sessions").join("%2Fw"); - fs::create_dir_all(dir.join("not-a-session")).unwrap(); - assert_eq!(Grok.correlate_fs(cwd, SystemTime::now(), Some(&home)), None); - } - - /// `created_at` as grok writes it: 2026-07-15 is day 20_649 since epoch. - const GROK_CREATED_AT: &str = "2026-07-15T00:34:19.339081Z"; - - fn spec_spawned() -> SystemTime { - SystemTime::UNIX_EPOCH + Duration::new(20_649 * 86_400 + 34 * 60 + 19, 339_081_000) - } - - fn write_summary(dir: &Path, id: &str, cwd: &str, subagent: bool) { - let kind = if subagent { - r#","session_kind":"subagent""# - } else { - "" - }; - fs::write( - dir.join("summary.json"), - format!( - r#"{{"info":{{"id":"{id}","cwd":"{cwd}"}},"created_at":"{GROK_CREATED_AT}"{kind}}}"# - ), - ) - .unwrap(); - } - - #[test] - fn correlate_fs_reads_a_long_name_group_via_dot_cwd() { - let home = temp("grok_longpath"); - let cwd = Path::new("/work/very-long-path-name-that-would-encode-past-the-limit"); - let group = home - .join("sessions") - .join("would-encode-past-the-limit-0123456789abcdef"); - fs::create_dir_all(group.join(ID)).unwrap(); - fs::write(group.join(".cwd"), format!("{}\n", cwd.display())).unwrap(); - write_summary(&group.join(ID), ID, cwd.to_str().unwrap(), false); - assert_eq!( - Grok.correlate_fs(cwd, spec_spawned(), Some(&home)) - .as_deref(), - Some(ID) - ); - } - - #[test] - fn correlate_fs_follows_a_symlink_cwd_to_the_canonical_group() { - let tmp = temp("grok_canon"); - let real = tmp.join("real"); - let link = tmp.join("link"); - let home = tmp.join("home"); - fs::create_dir_all(&real).unwrap(); - fs::create_dir_all(&home).unwrap(); - std::os::unix::fs::symlink(&real, &link).unwrap(); - let canonical = real.canonicalize().unwrap(); - let group = home - .join("sessions") - .join(encode_cwd(&canonical).expect("canonical path is UTF-8")); - fs::create_dir_all(group.join(ID)).unwrap(); - write_summary(&group.join(ID), ID, canonical.to_str().unwrap(), false); - assert_eq!( - Grok.correlate_fs(&link, spec_spawned(), Some(&home)) - .as_deref(), - Some(ID) - ); - } - - #[test] - fn correlate_fs_ignores_an_in_window_subagent_sibling() { - let home = temp("grok_subagent"); - let cwd = Path::new("/work/proj.rs"); - let dir = home.join("sessions").join("%2Fwork%2Fproj.rs"); - fs::create_dir_all(dir.join(ID)).unwrap(); - fs::create_dir_all(dir.join(OTHER)).unwrap(); - write_summary(&dir.join(ID), ID, "/work/proj.rs", false); - write_summary(&dir.join(OTHER), OTHER, "/work/proj.rs", true); - assert_eq!( - Grok.correlate_fs(cwd, spec_spawned(), Some(&home)) - .as_deref(), - Some(ID) - ); - } - - /// A leftover group that also names `cwd` does not hide the one - /// in-window session in the other group. - #[test] - fn correlate_fs_accepts_one_in_window_session_across_two_groups() { - let home = temp("grok_stale_group"); - let cwd = Path::new("/work/proj.rs"); - let live = home.join("sessions").join("%2Fwork%2Fproj.rs"); - let stale = home.join("sessions").join("stale-alias-0123456789abcdef"); - fs::create_dir_all(live.join(ID)).unwrap(); - fs::create_dir_all(&stale).unwrap(); - fs::write(stale.join(".cwd"), "/work/proj.rs\n").unwrap(); - write_summary(&live.join(ID), ID, "/work/proj.rs", false); - assert_eq!( - Grok.correlate_fs(cwd, spec_spawned(), Some(&home)) - .as_deref(), - Some(ID) - ); - - fs::create_dir_all(stale.join(OTHER)).unwrap(); - write_summary(&stale.join(OTHER), OTHER, "/work/proj.rs", false); - assert_eq!(Grok.correlate_fs(cwd, spec_spawned(), Some(&home)), None); - } - - #[test] - fn cwd_record_strips_only_one_newline_terminator() { - assert_eq!(cwd_record("/work/proj.rs"), "/work/proj.rs"); - assert_eq!(cwd_record("/work/proj.rs\n"), "/work/proj.rs"); - assert_eq!(cwd_record("/work/proj.rs\r\n"), "/work/proj.rs"); - assert_eq!(cwd_record("/work/project \n"), "/work/project "); - assert_eq!(cwd_record(" /work/lead"), " /work/lead"); - } - - /// A `.cwd` path with trailing spaces is a different directory. - #[test] - fn correlate_fs_does_not_trim_cwd_record_spaces() { - let home = temp("grok_cwd_spaces"); - let spaced = Path::new("/work/project "); - let group = home.join("sessions").join("spaced-cwd-0123456789abcdef"); - fs::create_dir_all(group.join(ID)).unwrap(); - fs::write(group.join(".cwd"), "/work/project \n").unwrap(); - write_summary(&group.join(ID), ID, "/work/project ", false); - assert_eq!( - Grok.correlate_fs(spaced, spec_spawned(), Some(&home)) - .as_deref(), - Some(ID) - ); - assert_eq!( - Grok.correlate_fs(Path::new("/work/project"), spec_spawned(), Some(&home)), - None - ); - } - /// The scraper recovers the exit-hint ID from the corpus terminal bytes. #[test] fn corpus_scrape_recovers_the_exit_hint_id() { diff --git a/src/harness/mod.rs b/src/harness/mod.rs index 9c67167..b63a0c3 100644 --- a/src/harness/mod.rs +++ b/src/harness/mod.rs @@ -9,8 +9,8 @@ //! //! # Security invariant //! -//! Every ID returned by `parse_capture`, `scrape_exit`, `live_session_id`, or -//! `correlate_fs` eventually enters a shell command. These methods return only +//! Every ID returned by `parse_capture`, `scrape_exit`, or `live_session_id` +//! eventually enters a shell command. These methods return only //! strings accepted by [`is_uuid`]; free text, paths, and malformed IDs yield //! `None`. Summary adapters and `live_blocked_status` are display-only. @@ -24,16 +24,14 @@ pub mod summary; use std::{ ffi::OsString, fs::File, - io::{BufRead, BufReader, Read}, + io::Read, path::{Path, PathBuf}, - time::{Duration, SystemTime}, + time::SystemTime, }; pub use claude::Claude; pub use codex::Codex; pub use grok::Grok; -#[cfg(test)] -pub(crate) use grok::encode_cwd; pub use omp::Omp; /// Environment variable naming the capture file used by injected assets. @@ -44,33 +42,12 @@ pub const CAPTURE_ENV: &str = "FLEETCOM_CAPTURE_FILE"; /// configured so inherited values cannot reach the capture script. pub const NOTIFY_CHAIN_ENV: &str = "FLEETCOM_NOTIFY_CHAIN"; -/// Maximum difference between a task spawn and a correlated session timestamp. -const CORRELATE_WINDOW: Duration = Duration::from_secs(30); - -/// Detection, capture, correlation, and resume behavior for one agent CLI. +/// Detection, capture, and resume behavior for one agent CLI. pub trait Harness: Sync { - /// Environment variable overriding the tool's home root. The supervisor - /// resolves it from the launch context used for instrumentation or save. - fn home_env_var(&self) -> &'static str; - - /// Default store path relative to the launched process's `$HOME`. - fn home_dot_dir(&self) -> &'static str; - - /// Resolve the store root from the launch environment. The default uses - /// the tool-specific override, then `$HOME` plus [`Self::home_dot_dir`]. - /// Returning `None` delegates to [`Self::home_root`]'s platform fallback. - fn resolve_home(&self, env: &dyn Fn(&str) -> Option) -> Option { - env(self.home_env_var()).or_else(|| Some(env("HOME")?.join(self.home_dot_dir()))) - } - - /// Resolve the tool's home root. `home` follows the `instrument` contract: - /// falling back to this process's home happens only when the launch - /// environment supplied neither the tool-specific override nor `HOME`. - fn home_root(&self, home: Option<&Path>) -> Option { - match home { - Some(p) => Some(p.to_path_buf()), - None => Some(dirs::home_dir()?.join(self.home_dot_dir())), - } + /// Resolve configuration needed by instrumentation or the live registry + /// from the launch environment. Tools that need neither return `None`. + fn resolve_home(&self, _env: &dyn Fn(&str) -> Option) -> Option { + None } /// Program word and canonical resume selector. The default detection and @@ -129,10 +106,6 @@ pub trait Harness: Sync { None } - /// Find one session ID in the tool's on-disk store. Missing or ambiguous - /// matches return `None`. `home` follows the `instrument` contract. - fn correlate_fs(&self, cwd: &Path, spawned: SystemTime, home: Option<&Path>) -> Option; - /// Rewrite an accepted `cmd` into the canonical command that resumes /// `id`. fn resume_command(&self, cmd: &str, id: &str) -> String { @@ -141,6 +114,23 @@ pub trait Harness: Sync { } } +/// Resolve a tool-specific override before the launch environment's home. +/// Without either, the consumer applies [`home_root`]'s platform fallback. +fn resolve_home( + env: &dyn Fn(&str) -> Option, + override_var: &str, + dot_dir: &str, +) -> Option { + env(override_var).or_else(|| Some(env("HOME")?.join(dot_dir))) +} + +/// Use the launch-time configuration root, falling back to this process's +/// platform home only when the launch supplied no root. +fn home_root(home: Option<&Path>, dot_dir: &str) -> Option { + home.map(Path::to_path_buf) + .or_else(|| Some(dirs::home_dir()?.join(dot_dir))) +} + /// One registered agent CLI: capture harness and display adapter. struct Agent { harness: &'static dyn Harness, @@ -349,78 +339,6 @@ fn pin_plan(inv: &Invocation) -> SpawnPlan { plan } -/// Whether `a` and `b` differ by at most [`CORRELATE_WINDOW`]. -fn within_window(a: SystemTime, b: SystemTime) -> bool { - match a.duration_since(b) { - Ok(d) => d <= CORRELATE_WINDOW, - Err(e) => e.duration() <= CORRELATE_WINDOW, - } -} - -/// Epoch-millisecond form of [`within_window`]. -fn within_window_ms(a: u128, b: u128) -> bool { - a.abs_diff(b) <= CORRELATE_WINDOW.as_millis() -} - -/// Milliseconds embedded in the first 48 bits of a UUIDv7: the session's -/// creation instant. `None` when `id` is not v7. `id` must already satisfy -/// [`is_uuid`], which fixes its length and alphabet. -fn v7_millis(id: &str) -> Option { - if id.as_bytes()[14] != b'7' { - return None; - } - u64::from_str_radix(&format!("{}{}", &id[..8], &id[9..13]), 16).ok() -} - -/// Epoch milliseconds of `t`; `None` before the epoch. -fn unix_millis(t: SystemTime) -> Option { - Some(t.duration_since(SystemTime::UNIX_EPOCH).ok()?.as_millis()) -} - -/// Whether a store-recorded path names the task's working directory: literal -/// equality first, so identical nonexistent paths stay eligible, then the -/// canonical task path `canon` for symlinked invocations. -fn same_cwd(recorded: &Path, cwd: &Path, canon: Option<&Path>) -> bool { - recorded == cwd || canon.is_some_and(|c| recorded == c) -} - -/// The one candidate when exactly one strict UUID survives; any other count -/// or shape yields `None`. -fn sole_id(candidates: Vec) -> Option { - match candidates.as_slice() { - [only] if is_uuid(only) => Some(only.clone()), - _ => None, - } -} - -/// Append `x` unless an equal entry is present. -fn push_unique(v: &mut Vec, x: T) { - if !v.contains(&x) { - v.push(x); - } -} - -/// Parse the first `n` JSONL records of `path`, reading at most 64 KiB so -/// later transcript content cannot affect correlation. Each entry is `None` -/// when its line does not parse, so callers decide whether a malformed record -/// rejects or is skipped. `None` when the file cannot be opened, a read -/// fails, or the file ends before `n` records. -fn jsonl_head(path: &Path, n: usize) -> Option>> { - let file = File::open(path).ok()?; - let mut reader = BufReader::new(file.take(64 * 1024)); - let mut records = Vec::with_capacity(n); - let mut line = String::new(); - for _ in 0..n { - line.clear(); - match reader.read_line(&mut line) { - Ok(len) if len > 0 => {} - _ => return None, - } - records.push(jzon::parse(&line).ok()); - } - Some(records) -} - /// Single-quote `s` for `$SHELL -c`, encoding embedded `'` as `'\''`. fn shell_quote(s: &str) -> String { format!("'{}'", s.replace('\'', "'\\''")) @@ -685,54 +603,22 @@ mod tests { assert_eq!(String::from_utf8(out.stdout).unwrap(), path); } - #[test] - fn within_window_is_symmetric_and_bounded() { - let t = SystemTime::UNIX_EPOCH + Duration::from_secs(1_000_000); - assert!(within_window(t, t + Duration::from_secs(30))); - assert!(within_window(t + Duration::from_secs(30), t)); - assert!(!within_window(t, t + Duration::from_secs(31))); - assert!(within_window_ms(5_000, 35_000)); - assert!(!within_window_ms(5_000, 35_001)); - } - - #[test] - fn home_env_vars_name_each_tools_override() { - // Expected environment override and default directory for each - // `AGENTS` entry, in the same order. - const OVERRIDES: [(&str, &str); 4] = [ - ("CLAUDE_CONFIG_DIR", ".claude"), - ("CODEX_HOME", ".codex"), - ("GROK_HOME", ".grok"), - ("PI_CODING_AGENT_SESSION_DIR", ".omp/agent/sessions"), - ]; - assert_eq!( - AGENTS.len(), - OVERRIDES.len(), - "a new harness needs its (env var, dot dir) row added here" - ); - for (a, (env_var, dot_dir)) in AGENTS.iter().zip(OVERRIDES) { - let program = a.harness.shape().0; - assert_eq!(a.harness.home_env_var(), env_var, "{program}"); - assert_eq!(a.harness.home_dot_dir(), dot_dir, "{program}"); - } - } - #[test] fn registry_detect_routes_to_the_matching_harness() { // The literal count keeps this hand-written routing coverage aligned // with `AGENTS`. assert_eq!(AGENTS.len(), 4, "route the new harness's command here"); let (h, inv) = detect("claude").unwrap(); - assert_eq!(h.home_dot_dir(), ".claude"); + assert_eq!(h.shape().0, "claude"); assert_eq!(inv, Invocation::Bare); let (h, inv) = detect(&format!("codex resume {ID}")).unwrap(); - assert_eq!(h.home_dot_dir(), ".codex"); + assert_eq!(h.shape().0, "codex"); assert_eq!(inv, Invocation::Resume(ID.into())); let (h, inv) = detect("grok").unwrap(); - assert_eq!(h.home_dot_dir(), ".grok"); + assert_eq!(h.shape().0, "grok"); assert_eq!(inv, Invocation::Bare); let (h, inv) = detect("omp").unwrap(); - assert_eq!(h.home_dot_dir(), ".omp/agent/sessions"); + assert_eq!(h.shape().0, "omp"); assert_eq!(inv, Invocation::Bare); assert!(detect("vim").is_none()); assert!(detect("").is_none()); diff --git a/src/harness/omp.rs b/src/harness/omp.rs index d70f813..98bbbf8 100644 --- a/src/harness/omp.rs +++ b/src/harness/omp.rs @@ -9,30 +9,12 @@ //! //! `-r`, `--session`, and `-c` resume as well, but detection stays on the //! canonical pair: a command fleetcom cannot rewrite exactly is left verbatim. -//! -//! # The store -//! -//! Sessions live at `//_.jsonl`. -//! The harness home *is* the sessions root: `PI_CODING_AGENT_SESSION_DIR` -//! names a sessions directory outright, so no agent-dir value can express it. -//! That override also flattens the store: it is passed straight through as the -//! session file's parent and the bucket level is never computed, so -//! correlation scans the root and one level below it. -//! -//! Correlation does not derive bucket names. It enumerates the root and its -//! immediate subdirectories, then verifies the working directory from each -//! session header. -use std::{ - fs, - path::{Path, PathBuf}, - time::SystemTime, -}; +use std::path::Path; use super::{ - CAPTURE_ENV, CapturePaths, Harness, Invocation, SpawnPlan, capture_id, is_uuid, jsonl_head, - leading_uuid, push_unique, same_cwd, shell_quote, sole_id, unix_millis, v7_millis, - within_window_ms, + CAPTURE_ENV, CapturePaths, Harness, Invocation, SpawnPlan, capture_id, leading_uuid, + shell_quote, }; /// Command fragment shared by ordinary exit and recovery hints. @@ -44,70 +26,6 @@ const MAIN_LABEL: &str = "Main"; pub struct Omp; impl Harness for Omp { - /// The only variable that names the sessions root directly. - fn home_env_var(&self) -> &'static str { - "PI_CODING_AGENT_SESSION_DIR" - } - - /// Default sessions path relative to `$HOME`. - fn home_dot_dir(&self) -> &'static str { - ".omp/agent/sessions" - } - - /// Resolve omp's sessions root, not its agent directory. - /// `PI_CODING_AGENT_SESSION_DIR` names the root directly; other inputs name - /// or construct its parent directories. - /// - /// Precedence: `PI_CODING_AGENT_SESSION_DIR`; an unprofiled - /// `PI_CODING_AGENT_DIR`; an existing XDG store; then the config path under - /// `$HOME`. `OMP_PROFILE` is selected by presence, so an empty value still - /// suppresses `PI_PROFILE`. Empty directory overrides are treated as unset. - fn resolve_home(&self, env: &dyn Fn(&str) -> Option) -> Option { - let set = |key: &str| env(key).filter(|p| !p.as_os_str().is_empty()); - if let Some(sessions) = set(self.home_env_var()) { - return Some(sessions); - } - - // Presence of `OMP_PROFILE` decides; an empty value selects no profile - // and still shadows `PI_PROFILE`. - let profile = match env("OMP_PROFILE") { - Some(p) => p, - None => env("PI_PROFILE").unwrap_or_default(), - }; - // Trim profile names; empty and `default` select the unprofiled store. - let profile = profile - .to_str() - .map(str::trim) - .filter(|p| !p.is_empty() && *p != "default") - .map(PathBuf::from); - - // Named profiles ignore `PI_CODING_AGENT_DIR`. - if let (None, Some(agent)) = (&profile, set("PI_CODING_AGENT_DIR")) { - return Some(agent.join("sessions")); - } - - // XDG redirects only when the target path already exists. - if let Some(xdg) = set("XDG_DATA_HOME") { - let data = match &profile { - Some(p) => xdg.join("omp").join("profiles").join(p), - None => xdg.join("omp"), - }; - if data.exists() { - return Some(data.join("sessions")); - } - } - - // Only the config-path branch requires `$HOME`; earlier overrides are - // complete paths. An absolute `PI_CONFIG_DIR` replaces `$HOME` under - // `Path::join`. - let config = env("HOME")?.join(set("PI_CONFIG_DIR").unwrap_or_else(|| ".omp".into())); - let root = match &profile { - Some(p) => config.join("profiles").join(p), - None => config, - }; - Some(root.join("agent").join("sessions")) - } - fn shape(&self) -> (&'static str, &'static str) { ("omp", "--resume") } @@ -159,144 +77,18 @@ impl Harness for Omp { } last } - - /// Return the sole in-window session whose header names `cwd`. Scan the - /// sessions root and each immediate subdirectory to cover flat and bucketed - /// stores. The ID follows the last `_` in the filename. - fn correlate_fs(&self, cwd: &Path, spawned: SystemTime, home: Option<&Path>) -> Option { - let sessions = self.home_root(home)?; - let spawn_ms = unix_millis(spawned)?; - // Session headers may record either the supplied or canonical path. - let canon = cwd.canonicalize().ok(); - - // The default store is bucketed; `PI_CODING_AGENT_SESSION_DIR` is flat. - let mut dirs = vec![sessions.clone()]; - dirs.extend( - fs::read_dir(&sessions) - .ok()? - .flatten() - .filter(|e| e.file_type().is_ok_and(|t| t.is_dir())) - .map(|e| e.path()), - ); - - let mut survivors: Vec = Vec::new(); - for dir in dirs { - let Ok(entries) = fs::read_dir(dir) else { - continue; - }; - for entry in entries.flatten() { - let name = entry.file_name(); - let Some(id) = name - .to_str() - .and_then(|n| n.strip_suffix(".jsonl")) - .and_then(|stem| stem.rsplit_once('_')) - .map(|(_, id)| id) - .filter(|id| is_uuid(id)) - else { - continue; - }; - // Only UUIDv7 provides the creation instant used for matching. - let Some(ms) = v7_millis(id) else { continue }; - if !within_window_ms(u128::from(ms), spawn_ms) { - continue; - } - if !header_cwd_matches(&entry.path(), cwd, canon.as_deref()) { - continue; - } - // The same session may appear in multiple buckets; count its - // UUID once. - push_unique(&mut survivors, id.to_string()); - } - } - sole_id(survivors) - } -} - -/// Whether either of the first two records is a session header naming `cwd`, -/// its canonical form, or a path with the same canonical target. The optional -/// first record is a fixed-width title slot. Nothing later can affect -/// correlation, so transcripts are not read beyond the header. -fn header_cwd_matches(path: &Path, cwd: &Path, canon: Option<&Path>) -> bool { - let Some(records) = jsonl_head(path, 2) else { - return false; - }; - for record in records.into_iter().flatten() { - if record["type"].as_str() != Some("session") { - continue; - } - return record["cwd"].as_str().is_some_and(|c| { - let header = Path::new(c); - same_cwd(header, cwd, canon) - || canon.is_some_and(|canon| header.canonicalize().is_ok_and(|h| h == canon)) - }); - } - false } #[cfg(test)] mod tests { - use std::time::Duration; + use std::path::PathBuf; use super::*; - use crate::{ - harness::fixtures::{ID, OTHER, assert_all_opaque, assert_corpus_scrape, paths}, - testutil::{temp, v7_at}, - }; + use crate::harness::fixtures::{ID, OTHER, assert_all_opaque, assert_corpus_scrape, paths}; - /// Spawn instant shared by the correlation tests: 2026-08-15T22:13:20Z. - const SPAWN_MS: u64 = 1_786_000_000_000; - /// Working directory recorded in the generated headers. - const CWD: &str = "/work/proj"; /// Valid UUIDv7 used in capture payloads. const CAPTURED: &str = "01a0077c-e18e-7000-ae0b-016f4834b6e9"; - fn spawned() -> SystemTime { - SystemTime::UNIX_EPOCH + Duration::from_millis(SPAWN_MS) - } - - /// Fixed-width title record preceding a session header. - fn title_slot() -> String { - let head = concat!( - r#"{"type":"title","v":1,"title":"Run ls -la","source":"auto","#, - r#""updatedAt":"2026-08-15T22:19:51.048Z","pad":""# - ); - let tail = r#""}"#; - format!("{head}{}{tail}", " ".repeat(256 - head.len() - tail.len())) - } - - /// Write a session file under `//`, optionally - /// behind the title slot. - fn write_named(sessions: &Path, bucket: &str, file: &str, id: &str, cwd: &str, slot: bool) { - let dir = sessions.join(bucket); - fs::create_dir_all(&dir).unwrap(); - let mut body = String::new(); - if slot { - body.push_str(&title_slot()); - body.push('\n'); - } - body.push_str(&format!( - r#"{{"type":"session","version":3,"id":"{id}","timestamp":"2026-08-15T22:19:51.048Z","cwd":"{cwd}","title":"Run ls -la"}}"# - )); - // Transcript content after the header does not participate. - body.push_str("\n{\"type\":\"message\",\"role\":\"assistant\"}\n"); - fs::write(dir.join(file), body).unwrap(); - } - - /// Write a session under `_.jsonl`. - fn write_session(sessions: &Path, bucket: &str, id: &str, cwd: &str, slot: bool) { - let file = format!("2026-08-15T22-19-51-048Z_{id}.jsonl"); - write_named(sessions, bucket, &file, id, cwd, slot); - } - - /// Resolve omp's sessions root against a synthetic launch environment. - fn home(env: &[(&str, &str)]) -> Option { - Omp.resolve_home(&|key| { - env.iter() - .find(|(name, _)| *name == key) - .map(|(_, value)| PathBuf::from(value)) - }) - } - /// omp-specific aliases, shortcuts, prompts, and malformed resume forms /// remain opaque. #[test] @@ -400,311 +192,6 @@ mod tests { assert_eq!(Omp.scrape_exit(&format!("omp --resume {ID}ff")), None); } - /// One in-window session whose header names the task's cwd correlates; - /// another directory, another window, and a second candidate do not. - #[test] - fn correlate_fs_requires_a_unique_in_window_session_for_the_cwd() { - let sessions = temp("omp_correlate"); - let id = v7_at(SPAWN_MS + 4_000, 1); - write_session(&sessions, "bucket", &id, CWD, true); - assert_eq!( - Omp.correlate_fs(Path::new(CWD), spawned(), Some(&sessions)) - .as_deref(), - Some(id.as_str()) - ); - - // The header decides the directory, so another cwd matches nothing. - assert_eq!( - Omp.correlate_fs(Path::new("/elsewhere"), spawned(), Some(&sessions)), - None - ); - - // A session minted 90 s later falls outside the window. - let late = v7_at(SPAWN_MS + 90_000, 2); - write_session(&sessions, "late", &late, "/late/proj", true); - assert_eq!( - Omp.correlate_fs(Path::new("/late/proj"), spawned(), Some(&sessions)), - None - ); - - // Two in-window sessions for one directory cannot be told apart. - write_session(&sessions, "bucket", &v7_at(SPAWN_MS + 8_000, 3), CWD, true); - assert_eq!( - Omp.correlate_fs(Path::new(CWD), spawned(), Some(&sessions)), - None - ); - } - - /// `PI_CODING_AGENT_SESSION_DIR` becomes the session file's parent - /// directly, so its store carries no bucket level. One scan covers both - /// layouts: a file in the root and a file one level down, in the same - /// root, each correlating for its own header cwd. - #[test] - fn correlate_fs_reads_a_flat_store_and_a_bucketed_one() { - let sessions = temp("omp_layouts"); - // An empty bucket name writes into the root itself. - let flat = v7_at(SPAWN_MS + 3_000, 8); - write_session(&sessions, "", &flat, CWD, true); - let nested = v7_at(SPAWN_MS + 5_000, 9); - write_session(&sessions, "bucket", &nested, "/work/other", true); - - assert_eq!( - Omp.correlate_fs(Path::new(CWD), spawned(), Some(&sessions)) - .as_deref(), - Some(flat.as_str()) - ); - assert_eq!( - Omp.correlate_fs(Path::new("/work/other"), spawned(), Some(&sessions)) - .as_deref(), - Some(nested.as_str()) - ); - // The header still decides the directory in either layout. - assert_eq!( - Omp.correlate_fs(Path::new("/elsewhere"), spawned(), Some(&sessions)), - None - ); - } - - /// The same session ID in two buckets remains one candidate. - #[test] - fn correlate_fs_collapses_one_session_seen_in_two_buckets() { - let sessions = temp("omp_dupe"); - let id = v7_at(SPAWN_MS + 6_000, 10); - write_session(&sessions, "encoded", &id, CWD, true); - write_session(&sessions, "--legacy--", &id, CWD, true); - assert_eq!( - Omp.correlate_fs(Path::new(CWD), spawned(), Some(&sessions)) - .as_deref(), - Some(id.as_str()) - ); - } - - /// A header path alias matches the task's canonical working directory. - #[test] - fn correlate_fs_matches_a_header_holding_an_alias_of_the_cwd() { - let tmp = temp("omp_alias"); - let (real, alias) = (tmp.join("real"), tmp.join("alias")); - fs::create_dir_all(&real).unwrap(); - std::os::unix::fs::symlink(&real, &alias).unwrap(); - let physical = real.canonicalize().unwrap(); - let sessions = tmp.join("sessions"); - let id = v7_at(SPAWN_MS + 7_000, 11); - write_session(&sessions, "bucket", &id, alias.to_str().unwrap(), true); - assert_eq!( - Omp.correlate_fs(&physical, spawned(), Some(&sessions)) - .as_deref(), - Some(id.as_str()) - ); - } - - /// A session header may occupy the first line when no title record exists. - #[test] - fn correlate_fs_reads_a_legacy_file_whose_header_is_line_one() { - let sessions = temp("omp_legacy"); - let id = v7_at(SPAWN_MS + 1_000, 5); - write_session(&sessions, "bucket", &id, CWD, false); - assert_eq!( - Omp.correlate_fs(Path::new(CWD), spawned(), Some(&sessions)) - .as_deref(), - Some(id.as_str()) - ); - } - - /// Malformed filenames, non-v7 IDs, and empty buckets contribute nothing. - #[test] - fn correlate_fs_refuses_names_and_buckets_it_cannot_read() { - let sessions = temp("omp_names"); - // UUIDv4 embeds no creation instant. - let v4 = format!("2026-08-15T22-19-51-048Z_{ID}.jsonl"); - write_named(&sessions, "v4", &v4, ID, CWD, true); - // Without a `_` nothing marks where the id starts. - let good = v7_at(SPAWN_MS + 1_000, 6); - write_named( - &sessions, - "nosep", - &format!("{good}.jsonl"), - &good, - CWD, - true, - ); - // An empty bucket contributes no candidate. - fs::create_dir_all(sessions.join("empty")).unwrap(); - assert_eq!( - Omp.correlate_fs(Path::new(CWD), spawned(), Some(&sessions)), - None - ); - - // A valid filename remains the sole candidate. - write_session(&sessions, "good", &good, CWD, true); - assert_eq!( - Omp.correlate_fs(Path::new(CWD), spawned(), Some(&sessions)) - .as_deref(), - Some(good.as_str()) - ); - } - - /// Canonical cwd comparison lets a task launched through a symlink match. - #[test] - fn correlate_fs_matches_a_symlinked_cwd_through_its_canonical_form() { - let tmp = temp("omp_canon"); - let (real, link) = (tmp.join("real"), tmp.join("link")); - fs::create_dir_all(&real).unwrap(); - std::os::unix::fs::symlink(&real, &link).unwrap(); - let canonical = real.canonicalize().unwrap(); - let sessions = tmp.join("sessions"); - let id = v7_at(SPAWN_MS + 2_000, 4); - write_session(&sessions, "bucket", &id, canonical.to_str().unwrap(), true); - assert_eq!( - Omp.correlate_fs(&link, spawned(), Some(&sessions)) - .as_deref(), - Some(id.as_str()) - ); - } - - /// Every branch of the chain resolves to a sessions root. - #[test] - fn resolve_home_walks_the_store_root_chain() { - assert_eq!( - home(&[("HOME", "/h")]), - Some("/h/.omp/agent/sessions".into()) - ); - - // The session dir names the store outright and bypasses the rest. - assert_eq!( - home(&[ - ("HOME", "/h"), - ("PI_CONFIG_DIR", ".alt"), - ("PI_CODING_AGENT_DIR", "/a"), - ("PI_CODING_AGENT_SESSION_DIR", "/s"), - ]), - Some("/s".into()) - ); - // Set but empty is unset. - assert_eq!( - home(&[("HOME", "/h"), ("PI_CODING_AGENT_SESSION_DIR", "")]), - Some("/h/.omp/agent/sessions".into()) - ); - - // The agent dir replaces `/agent` whole. - assert_eq!( - home(&[("HOME", "/h"), ("PI_CODING_AGENT_DIR", "/a")]), - Some("/a/sessions".into()) - ); - // A selected profile ignores it. - assert_eq!( - home(&[ - ("HOME", "/h"), - ("PI_CODING_AGENT_DIR", "/a"), - ("PI_PROFILE", "work"), - ]), - Some("/h/.omp/profiles/work/agent/sessions".into()) - ); - // Trimmed `default` selects the unprofiled store. - assert_eq!( - home(&[("HOME", "/h"), ("OMP_PROFILE", "default")]), - Some("/h/.omp/agent/sessions".into()) - ); - assert_eq!( - home(&[ - ("HOME", "/h"), - ("PI_PROFILE", " default "), - ("PI_CODING_AGENT_DIR", "/a"), - ]), - Some("/a/sessions".into()) - ); - assert_eq!( - home(&[("HOME", "/h"), ("OMP_PROFILE", " work ")]), - Some("/h/.omp/profiles/work/agent/sessions".into()) - ); - // Whitespace alone trims to nothing, which is no profile. - assert_eq!( - home(&[("HOME", "/h"), ("OMP_PROFILE", " ")]), - Some("/h/.omp/agent/sessions".into()) - ); - - // `OMP_PROFILE` wins by presence: it selects when non-empty and - // suppresses `PI_PROFILE` when empty. - assert_eq!( - home(&[("HOME", "/h"), ("OMP_PROFILE", "a"), ("PI_PROFILE", "b")]), - Some("/h/.omp/profiles/a/agent/sessions".into()) - ); - assert_eq!( - home(&[ - ("HOME", "/h"), - ("OMP_PROFILE", ""), - ("PI_PROFILE", "b"), - ("PI_CODING_AGENT_DIR", "/a"), - ]), - Some("/a/sessions".into()) - ); - - // `PI_CONFIG_DIR` replaces the default config-directory name. - assert_eq!( - home(&[("HOME", "/h"), ("PI_CONFIG_DIR", ".alt")]), - Some("/h/.alt/agent/sessions".into()) - ); - // An absolute value replaces `$HOME` under `Path::join`. - assert_eq!( - home(&[("HOME", "/h"), ("PI_CONFIG_DIR", "/abs")]), - Some("/abs/agent/sessions".into()) - ); - - // A complete agent-directory override does not require `HOME`. - assert_eq!( - home(&[("PI_CODING_AGENT_DIR", "/a")]), - Some("/a/sessions".into()) - ); - - // `home_root` applies the platform fallback when no path resolves. - assert_eq!(home(&[]), None); - } - - /// The XDG redirect requires an existing target and drops `agent/`. - #[test] - fn resolve_home_redirects_to_xdg_only_when_that_directory_exists() { - let dir = temp("omp_xdg"); - let xdg = dir.to_str().unwrap(); - - assert_eq!( - home(&[("HOME", "/h"), ("XDG_DATA_HOME", xdg)]), - Some("/h/.omp/agent/sessions".into()) - ); - fs::create_dir_all(dir.join("omp")).unwrap(); - assert_eq!( - home(&[("HOME", "/h"), ("XDG_DATA_HOME", xdg)]), - Some(dir.join("omp/sessions")) - ); - - // With a profile, the profile target must exist. - let profile = [ - ("HOME", "/h"), - ("XDG_DATA_HOME", xdg), - ("OMP_PROFILE", "work"), - ]; - assert_eq!( - home(&profile), - Some("/h/.omp/profiles/work/agent/sessions".into()) - ); - fs::create_dir_all(dir.join("omp/profiles/work")).unwrap(); - assert_eq!(home(&profile), Some(dir.join("omp/profiles/work/sessions"))); - - // An agent dir named outright is never redirected. - assert_eq!( - home(&[ - ("HOME", "/h"), - ("XDG_DATA_HOME", xdg), - ("PI_CODING_AGENT_DIR", "/a"), - ]), - Some("/a/sessions".into()) - ); - - // The redirect target is absolute, so it too answers without `HOME`. - assert_eq!( - home(&[("XDG_DATA_HOME", xdg)]), - Some(dir.join("omp/sessions")) - ); - } - /// The scraper recovers the exit-hint ID from the corpus terminal bytes. #[test] fn corpus_scrape_recovers_the_exit_hint_id() { diff --git a/src/supervisor.rs b/src/supervisor.rs index a88b992..e901cd0 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -167,7 +167,7 @@ fn scrape_now(t: &mut Task) { t.scrape_exit_hint(); } -/// Resolve the harness store root from the task's launch environment. +/// Resolve harness configuration from the task's launch environment. fn harness_home(env: &[(OsString, OsString)], h: &dyn harness::Harness) -> Option { h.resolve_home(&|key| env_get(env, key).map(PathBuf::from)) } @@ -789,7 +789,7 @@ impl Supervisor { )?; if let Some((h, home, capture_file, resume_id)) = meta { task.harness = Some(h); - // Preserve the launch-time store for later correlation. + // Registry reads must keep using the launch-time configuration. task.harness_home = home; task.capture_file = Some(capture_file); task.resume_id = resume_id; @@ -864,10 +864,7 @@ impl Supervisor { // targeted conversation. let (command, cwd) = { let old = &self.tasks[i]; - let command = match (old.harness, current_resume_id(old)) { - (Some(h), Some(rid)) => h.resume_command(&old.command, &rid), - _ => old.command.clone(), - }; + let command = Self::recipe_command(old); (command, old.cwd.clone()) }; // Preserve the finished task if its replacement cannot start. The run @@ -913,7 +910,7 @@ impl Supervisor { cfg.entry(path::abbreviate(&t.cwd)) .or_default() .push(SessionEntry { - cmd: self.recipe_command(t), + cmd: Self::recipe_command(t), group: t.group.clone(), name: t.name.clone(), }); @@ -921,19 +918,12 @@ impl Supervisor { cfg } - /// Build the command stored for one task. Agent commands use the best live - /// ID, then filesystem correlation; without either, the requested command - /// remains unchanged. - fn recipe_command(&self, t: &Task) -> String { - let Some(h) = t.harness else { - return t.command.clone(); - }; - // Correlate against the store selected when this task launched. - let id = current_resume_id(t) - .or_else(|| h.correlate_fs(&t.cwd, t.spawned_at, t.harness_home.as_deref())); - match id { - Some(id) => h.resume_command(&t.command, &id), - None => t.command.clone(), + /// Build the saved or rerun command from the task's best-known ID. + /// Without an ID, preserve the requested command exactly. + fn recipe_command(t: &Task) -> String { + match (t.harness, current_resume_id(t)) { + (Some(h), Some(id)) => h.resume_command(&t.command, &id), + _ => t.command.clone(), } } diff --git a/src/supervisor_capture_tests.rs b/src/supervisor_capture_tests.rs index 0161cfa..811afe5 100644 --- a/src/supervisor_capture_tests.rs +++ b/src/supervisor_capture_tests.rs @@ -492,15 +492,11 @@ fn spawn_grok_pins_an_id_and_injects_nothing_else() { let dir = scratch("cap_grok"); let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); install_stub(&bin, "grok", &dir); - // Keep save-time correlation inside the scratch tree. let mut s = sup_ctx(agent_ctx_plus( &bin, &runtime, dir.to_path_buf(), - &[ - ("FLEETCOM_CONFIG_DIR", &config), - ("GROK_HOME", &dir.join("grok_home")), - ], + &[("FLEETCOM_CONFIG_DIR", &config)], )); spawn(&mut s, "grok", dir.to_path_buf()); @@ -593,10 +589,7 @@ fn grok_exit_hint_is_scraped_and_saved_as_a_resume() { &bin, &runtime, dir.to_path_buf(), - &[ - ("FLEETCOM_CONFIG_DIR", &config), - ("GROK_HOME", &dir.join("grok_home")), - ], + &[("FLEETCOM_CONFIG_DIR", &config)], )); spawn(&mut s, "grok", dir.to_path_buf()); assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] @@ -631,10 +624,7 @@ fn save_scrapes_a_finished_grok_task_without_reap() { &bin, &runtime, dir.to_path_buf(), - &[ - ("FLEETCOM_CONFIG_DIR", &config), - ("GROK_HOME", &dir.join("grok_home")), - ], + &[("FLEETCOM_CONFIG_DIR", &config)], )); spawn(&mut s, "grok", dir.to_path_buf()); @@ -667,10 +657,7 @@ fn rerun_scrapes_a_finished_grok_task_without_reap() { &bin, &runtime, dir.to_path_buf(), - &[ - ("FLEETCOM_CONFIG_DIR", &config), - ("GROK_HOME", &dir.join("grok_home")), - ], + &[("FLEETCOM_CONFIG_DIR", &config)], )); spawn(&mut s, "grok", dir.to_path_buf()); let id = s.tasks[0].id; @@ -689,136 +676,6 @@ fn rerun_scrapes_a_finished_grok_task_without_reap() { ); } -/// A silent grok task falls back to one in-window top-level session dir -/// under `GROK_HOME` when live channels produce no ID. -#[test] -fn save_falls_back_to_fs_correlation_for_a_silent_grok() { - let dir = scratch("grok_correlate_save"); - let (bin, runtime, config, grok_home) = ( - dir.join("bin"), - dir.join("run"), - dir.join("config"), - dir.join("grok_home"), - ); - install_stub(&bin, "grok", &dir); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config), ("GROK_HOME", &grok_home)], - )); - spawn(&mut s, "grok", dir.to_path_buf()); - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .finished - .is_some())); - assert!(s.tasks[0].scraped_id.is_none(), "a silent exit has no hint"); - // Bare grok always pins; recipe_command would take that ID and never - // reach correlate_fs unless the pin is absent. - s.tasks[0].resume_id = None; - assert!( - current_resume_id(&s.tasks[0]).is_none(), - "clearing the pin is what exposes filesystem correlation" - ); - - let group = grok_home - .join("sessions") - .join(crate::harness::encode_cwd(&s.tasks[0].cwd).expect("task cwd is UTF-8")); - std::fs::create_dir_all(group.join(CAP_ID)).unwrap(); - - let text = save_and_read(&mut s, &config, "corr"); - assert!( - text.contains(&format!("grok --resume '{CAP_ID}'")), - "save must fall back to filesystem correlation; got {text}" - ); -} - -/// An in-window `session_kind: subagent` sibling does not steal uniqueness -/// from the top-level session directory. -#[test] -fn save_ignores_an_in_window_grok_subagent_sibling() { - let dir = scratch("grok_correlate_subagent"); - let (bin, runtime, config, grok_home) = ( - dir.join("bin"), - dir.join("run"), - dir.join("config"), - dir.join("grok_home"), - ); - install_stub(&bin, "grok", &dir); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config), ("GROK_HOME", &grok_home)], - )); - spawn(&mut s, "grok", dir.to_path_buf()); - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .finished - .is_some())); - assert!(s.tasks[0].scraped_id.is_none(), "a silent exit has no hint"); - s.tasks[0].resume_id = None; - - let group = grok_home - .join("sessions") - .join(crate::harness::encode_cwd(&s.tasks[0].cwd).expect("task cwd is UTF-8")); - std::fs::create_dir_all(group.join(CAP_ID)).unwrap(); - // No created_at: birthtime is in-window, so the skip is session_kind. - let other = group.join(CAP_OTHER); - std::fs::create_dir_all(&other).unwrap(); - std::fs::write(other.join("summary.json"), r#"{"session_kind":"subagent"}"#).unwrap(); - - let text = save_and_read(&mut s, &config, "subagent"); - assert!( - text.contains(&format!("grok --resume '{CAP_ID}'")), - "the recipe must resume the top-level session; got {text}" - ); - assert!( - !text.contains(CAP_OTHER), - "an in-window subagent sibling must not correlate; got {text}" - ); -} - -/// A spawn through a symlink cwd correlates against the canonical group's -/// encoded name. -#[test] -fn save_follows_a_symlink_cwd_to_the_canonical_grok_group() { - let dir = scratch("grok_correlate_symlink"); - let (bin, runtime, config, grok_home, real, link) = ( - dir.join("bin"), - dir.join("run"), - dir.join("config"), - dir.join("grok_home"), - dir.join("real"), - dir.join("link"), - ); - std::fs::create_dir_all(&real).unwrap(); - std::os::unix::fs::symlink(&real, &link).unwrap(); - install_stub(&bin, "grok", &dir); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config), ("GROK_HOME", &grok_home)], - )); - spawn(&mut s, "grok", link); - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .finished - .is_some())); - assert!(s.tasks[0].scraped_id.is_none(), "a silent exit has no hint"); - s.tasks[0].resume_id = None; - - let canonical = real.canonicalize().unwrap(); - let group = grok_home - .join("sessions") - .join(crate::harness::encode_cwd(&canonical).expect("canonical path is UTF-8")); - std::fs::create_dir_all(group.join(CAP_ID)).unwrap(); - - let text = save_and_read(&mut s, &config, "symlink"); - assert!( - text.contains(&format!("grok --resume '{CAP_ID}'")), - "save must correlate through the canonical group; got {text}" - ); -} - /// A `claude` exit hint becomes the session ID used by the saved recipe. #[test] fn exit_hint_is_scraped_and_saved_as_a_resume() { @@ -1036,17 +893,34 @@ fn resume_id_precedence_registry_over_spawn_under_capture() { std::fs::write(&done, b"").unwrap(); } -/// A silent Codex task falls back to one matching rollout under -/// `CODEX_HOME` when live channels produce no ID. +/// Two tasks sharing a directory do not own a nearby rollout. Named saves +/// and recovery must preserve both authored commands when capture is silent. #[test] -fn save_falls_back_to_fs_correlation_for_a_silent_codex() { - let dir = scratch("correlate_save"); +fn silent_codex_tasks_keep_authored_commands_in_saves_and_recovery() { + let dir = scratch("uncaptured_codex"); let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); let codex_home = dir.join("codex_home"); - install_stub(&bin, "codex", &dir); - // Create a rollout with a current v7 instant and the task's cwd. - let now_ms = now_ms(); - let id = write_rollout(&codex_home, now_ms, 1, &dir); + install_script(&bin, "codex", "exit 0"); + + // This sole rollout matches the directory and the former 30-second + // window for both tasks, but predates both launches. + let ms = now_ms(); + let id = format!( + "{:08x}-{:04x}-7000-8000-000000000001", + ms >> 16, + ms & 0xffff + ); + let (y, m, d) = crate::format::civil_from_days((ms / 86_400_000) as i64); + let rollouts = codex_home.join(format!("sessions/{y:04}/{m:02}/{d:02}")); + std::fs::create_dir_all(&rollouts).unwrap(); + std::fs::write( + rollouts.join(format!("rollout-2026-07-13T09-00-00-{id}.jsonl")), + format!( + r#"{{"type":"session_meta","payload":{{"id":"{id}","cwd":"{}"}}}}"#, + dir.display() + ), + ) + .unwrap(); let mut s = sup_ctx(agent_ctx_plus( &bin, @@ -1057,62 +931,62 @@ fn save_falls_back_to_fs_correlation_for_a_silent_codex() { ("CODEX_HOME", &codex_home), ], )); - spawn(&mut s, "codex", dir.to_path_buf()); - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .finished - .is_some())); - assert!(s.tasks[0].scraped_id.is_none(), "a silent exit has no hint"); - assert!( - current_resume_id(&s.tasks[0]).is_none(), - "no capture channel fired" + s.set_recovery_timing(Duration::from_millis(20), Duration::from_millis(100)); + let commands = ["codex".to_string(), format!(" {}/codex\t", bin.display())]; + for command in &commands { + spawn(&mut s, command, dir.to_path_buf()); + } + assert_eq!(s.tasks.len(), 2); + assert!(reap_until(&mut s, Duration::from_secs(5), |s| s + .tasks + .iter() + .all(|t| t.finished.is_some()))); + for task in &s.tasks { + assert!( + current_resume_id(task).is_none(), + "no capture channel fired" + ); + let spawn_ms = task + .spawned_at + .duration_since(std::time::SystemTime::UNIX_EPOCH) + .unwrap() + .as_millis(); + assert!( + spawn_ms.abs_diff(u128::from(ms)) <= 30_000, + "the decoy must remain plausible" + ); + } + let expected = SessionConfig::from([( + path::abbreviate(&dir), + commands + .iter() + .map(|cmd| SessionEntry { + cmd: cmd.clone(), + group: None, + name: None, + }) + .collect(), + )]); + save_and_read(&mut s, &config, "silent"); + assert_eq!( + session::load_in(&config.join("sessions"), "silent").unwrap(), + expected ); - let text = save_and_read(&mut s, &config, "corr"); assert!( - text.contains(&format!("codex resume '{id}'")), - "save must fall back to filesystem correlation; got {text}" - ); -} - -/// Save-time correlation uses the task's spawn-time `CODEX_HOME`, even -/// after a reconnect supplies another home containing a matching rollout. -#[test] -fn save_correlates_against_the_spawn_time_home() { - let dir = scratch("correlate_home"); - let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - let (home_a, home_b) = (dir.join("codex_a"), dir.join("codex_b")); - install_stub(&bin, "codex", &dir); - let now_ms = now_ms(); - // One unique in-window rollout per store, both naming the task cwd. - let id_a = write_rollout(&home_a, now_ms, 1, &dir); - let id_b = write_rollout(&home_b, now_ms, 2, &dir); - - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config), ("CODEX_HOME", &home_a)], - )); - spawn(&mut s, "codex", dir.to_path_buf()); - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .finished - .is_some())); - - // Reconnect under home B, then save. - s.set_launch_context(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config), ("CODEX_HOME", &home_b)], - )); - let text = save_and_read(&mut s, &config, "homepin"); - assert!( - text.contains(&format!("codex resume '{id_a}'")), - "the recipe must resolve from the launch-time store; got {text}" + wait_until(Duration::from_secs(5), || { + s.tick(); + !recovery_files(&config).is_empty() + }), + "the recovery snapshot never landed" ); - assert!( - !text.contains(&id_b), - "the reconnect store's decoy must not correlate; got {text}" + let files = recovery_files(&config); + assert_eq!(files.len(), 1); + let stem = files[0].strip_suffix(".json").unwrap(); + let recovery = config.join("sessions").join("recovery"); + assert_eq!( + session::load_recovery_in(&recovery, stem).unwrap(), + expected ); } @@ -1120,7 +994,7 @@ fn save_correlates_against_the_spawn_time_home() { /// joined with the tool's dot directory, then nothing. #[test] fn harness_home_prefers_the_tool_var_then_home() { - use crate::harness::{Claude, Codex, Grok}; + use crate::harness::{Claude, Codex, Grok, Omp}; let env: Vec<(OsString, OsString)> = vec![ ("HOME".into(), "/h".into()), ("CODEX_HOME".into(), "/x".into()), @@ -1135,15 +1009,13 @@ fn harness_home_prefers_the_tool_var_then_home() { harness_home(&env, &Claude).as_deref(), Some(Path::new("/h/.claude")) ); - assert_eq!( - harness_home(&env, &Grok).as_deref(), - Some(Path::new("/h/.grok")) - ); + assert_eq!(harness_home(&env, &Grok), None); + assert_eq!(harness_home(&env, &Omp), None); assert_eq!(harness_home(&[], &Codex), None); } -/// With only `HOME` in the launch environment, both notify routing and -/// save-time correlation resolve through `/.codex`. +/// With only `HOME` in the launch environment, notify routing reads +/// `/.codex/config.toml`. #[test] fn home_only_launch_env_targets_the_clients_dot_codex() { let dir = scratch("home_resolve"); @@ -1163,11 +1035,6 @@ fn home_only_launch_env_targets_the_clients_dot_codex() { ) .unwrap(); install_stub(&bin, "codex", &dir); - // A unique in-window rollout in the same tree for save-time - // correlation: with injection suppressed, no capture channel fires. - let now_ms = now_ms(); - let id = write_rollout(&codex_home, now_ms, 1, &dir); - let mut s = sup_ctx(agent_ctx_plus( &bin, &runtime, @@ -1189,10 +1056,15 @@ fn home_only_launch_env_targets_the_clients_dot_codex() { .finished .is_some())); - let text = save_and_read(&mut s, &config, "homeonly"); - assert!( - text.contains(&format!("codex resume '{id}'")), - "correlation must read /.codex; got {text}" + save_and_read(&mut s, &config, "homeonly"); + let cfg = session::load_in(&config.join("sessions"), "homeonly").unwrap(); + assert_eq!( + cfg[&path::abbreviate(&dir)], + vec![SessionEntry { + cmd: "codex".into(), + group: None, + name: None, + }] ); } @@ -1245,14 +1117,12 @@ fn stale_inherited_notify_chain_is_never_executed() { ); } -/// Without a live or filesystem ID, an agent recipe retains the original -/// command. +/// Without a known ID, an agent recipe retains the original command. #[test] fn agent_save_without_any_id_keeps_the_plain_command() { let dir = scratch("no_id"); let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - // CODEX_HOME names a store that never exists: correlation has - // nothing to find, and the notify routing nothing to read. + // No config.toml exists, so notifier routing has nothing to read. let codex_home = dir.join("codex_home"); install_stub(&bin, "codex", &dir); let mut s = sup_ctx(agent_ctx_plus( diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index 40db886..95d5fbe 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -5,7 +5,7 @@ use crate::{ protocol::{ClipboardKind, Key, Mods}, testutil::{ Scratch, here, install_fake_notifier, now_ms, read_pid, sh_env, wait_until, - write_executable, write_rollout, + write_executable, }, }; diff --git a/src/task.rs b/src/task.rs index 5edc425..9067742 100644 --- a/src/task.rs +++ b/src/task.rs @@ -122,7 +122,7 @@ pub struct Task { blocked: Option<(String, &'static str)>, /// Last registry probe time; `None` before the first probe. blocked_probed: Option, - /// Wall-clock spawn time used for registry and transcript correlation. + /// Wall-clock spawn time used to validate the Claude PID registry record. pub spawned_at: SystemTime, exit_code: Option, pub started: Instant, diff --git a/src/testutil.rs b/src/testutil.rs index eb7426f..9b255ef 100644 --- a/src/testutil.rs +++ b/src/testutil.rs @@ -1,5 +1,5 @@ //! Test scaffolds shared by the in-src test modules: scratch directories, -//! deadline polling, and the corpus/rollout fixtures. Test-only (`#[cfg(test)]` +//! deadline polling, and corpus fixtures. Test-only (`#[cfg(test)]` //! at the declaration in `main.rs`), so nothing here ships. use std::{ @@ -17,7 +17,6 @@ use std::{ use crate::{ emulator::Emulator, - format::civil_from_days, task::{pid_is_dead, positive_pid}, }; @@ -207,53 +206,3 @@ pub(crate) const CORPUS_COLS: usize = 120; pub(crate) fn corpus_emulator() -> Emulator { Emulator::new(CORPUS_LINES as u16, CORPUS_COLS as u16, 2000) } - -/// A v7-shaped ID whose embedded instant is `ms`, with a fixed tail. -pub(crate) fn v7_at(ms: u64, tail: u32) -> String { - format!( - "{:08x}-{:04x}-7000-8000-0000000{:05x}", - ms >> 16, - ms & 0xffff, - tail - ) -} - -/// Write a rollout under the UTC day dir for `ms` with `cwd` in its -/// `session_meta` line; returns the ID. The filename timestamp is inert: -/// correlation reads the v7 ID's embedded instant, never the name. -pub(crate) fn write_rollout(home: &Path, ms: u64, tail: u32, cwd: &Path) -> String { - write_rollout_named(home, ms, tail, cwd, "", "") -} - -/// [`write_rollout`] with an optional filename suffix and additional -/// `session_meta` payload members. `stem_suffix` follows the thread ID; -/// `meta_extra` is inserted verbatim and must include each leading comma. -pub(crate) fn write_rollout_named( - home: &Path, - ms: u64, - tail: u32, - cwd: &Path, - stem_suffix: &str, - meta_extra: &str, -) -> String { - let id = v7_at(ms, tail); - let (y, m, d) = civil_from_days((ms / 86_400_000) as i64); - let dir = home - .join("sessions") - .join(format!("{y:04}")) - .join(format!("{m:02}")) - .join(format!("{d:02}")); - fs::create_dir_all(&dir).unwrap(); - let meta = format!( - r#"{{"timestamp":"x","type":"session_meta","payload":{{"id":"{id}","cwd":"{}"{meta_extra}}}}}"#, - cwd.display() - ); - fs::write( - dir.join(format!( - "rollout-2026-07-13T09-00-00-{id}{stem_suffix}.jsonl" - )), - format!("{meta}\n{{}}\n"), - ) - .unwrap(); - id -} From a2ca4723580213648253a3d0f385facb2c3747ed Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sat, 5 Sep 2026 16:36:48 -0700 Subject: [PATCH 3/5] Refactor session ID handling and exit scraping in harness and supervisor --- docs/README.md | 2 +- docs/agent-resume.md | 24 +- src/harness/claude.rs | 38 +-- src/harness/codex.rs | 107 +-------- src/harness/grok.rs | 42 +--- src/harness/mod.rs | 58 +---- src/harness/omp.rs | 88 +------ src/supervisor.rs | 38 +-- src/supervisor_capture_tests.rs | 400 ++++++++++++-------------------- src/task.rs | 35 +-- src/task_tests.rs | 49 ++-- src/terminal/emulator.rs | 106 +-------- tests/corpus/README.md | 22 +- tests/corpus/omp_resume.bin | 372 ----------------------------- 14 files changed, 227 insertions(+), 1154 deletions(-) delete mode 100644 tests/corpus/omp_resume.bin diff --git a/docs/README.md b/docs/README.md index 7e693f0..bae8e88 100644 --- a/docs/README.md +++ b/docs/README.md @@ -169,7 +169,7 @@ Recipes persist full command lines, which can embed secrets. A token passed as a ### Captured IDs cross a shell boundary -Agent resume writes a captured conversation ID into a command run through `$SHELL -c`, so validation is a security boundary. Accepted IDs contain only lowercase hexadecimal in the `8-4-4-4-12` UUID shape. Hook payloads, terminal scrapes, live session records, filesystem correlation, and the command builder all apply that check. Instrumentation applies only to a bare program word or its canonical resume form, never arbitrary shell text. [Agent session resume](agent-resume.md#validation-boundary) documents both boundaries. +Agent resume writes a captured conversation ID into a command run through `$SHELL -c`, so validation is a security boundary. Accepted IDs contain only lowercase hexadecimal in the `8-4-4-4-12` UUID shape. Capture payloads, live session records, and the command builder all apply that check. Terminal output supplies no session IDs. Instrumentation applies only to a bare program word or its canonical resume form, never arbitrary shell text. [Agent session resume](agent-resume.md#validation-boundary) documents both boundaries. ### Copied text leaves through the terminal diff --git a/docs/agent-resume.md b/docs/agent-resume.md index 3682769..17dcccb 100644 --- a/docs/agent-resume.md +++ b/docs/agent-resume.md @@ -7,11 +7,11 @@ Session files preserve launch commands, not process state. Relaunching a bare `c Start a supported agent without flags: 1. Press `n` and run `claude`, `codex`, `grok`, or `omp`. The task appears in the dashboard under the command you typed. Instrumentation changes only the string executed through `$SHELL -c`, so a direct spawn still displays the requested command. -2. Work in it. `Enter` attaches; `Ctrl-\` returns to the dashboard. Depending on the agent, `fleetcom` pins an ID at launch and may update it from a hook, notifier, or extension while the task runs or from terminal output after it exits. +2. Work in it. `Enter` attaches; `Ctrl-\` returns to the dashboard. Depending on the agent, `fleetcom` pins an ID at launch and may update it from a hook, notifier, extension, or matching live registry record. 3. Press `w`, enter a session name, and press `Enter`. A captured bare command becomes its canonical resume form, such as `claude --resume ''`. Without a known ID, the save preserves the authored command. 4. Run `fleetcom `, or press `o` in the dashboard, to start new processes from the saved commands. A stored resume command reopens its captured conversation. -On a finished agent task, `r` uses the captured launch, hook, notifier, extension, registry, or exit ID. A registry record remains eligible after exit if it is still present. The replacement keeps the task's ID, tag, group, and name. After a successful rewrite, the row shows the resume command because it has become the task's launch recipe; a [saved session](sessions.md) records the same string. +On a finished agent task, `r` uses the captured launch, hook, notifier, extension, or registry ID. A registry record remains eligible after exit if it is still present. The replacement keeps the task's ID, tag, group, and name. After a successful rewrite, the row shows the resume command because it has become the task's launch recipe; a [saved session](sessions.md) records the same string. Capture is best-effort and narrow by design. A command carrying a prompt, extra flags, or shell syntax stays opaque and saves verbatim. An accepted command with no available ID also saves unchanged. In both cases, loading the recipe reruns the original command. @@ -58,8 +58,6 @@ Claude also publishes one `/sessions/.json` record per session A record counts only when its `kind` is `interactive` and its `pid`, `cwd`, and `startedAt` match the task. The PID must match the filename, the working directories must be identical or resolve to the same path, and the process start must fall within 30 seconds of the task spawn. Missing, malformed, or mismatched records contribute no evidence. The dashboard also maps a matching record's `waiting` status to the top tier of its [preview cascade](commands.md#peek); other statuses do not affect the preview. -After the process exits and the PTY reader reaches EOF, the harness scans the retained terminal text for the last `claude --resume ` hint. - ### `codex` Codex does not let the caller choose an ID at launch. Both accepted forms instead receive a notify override: @@ -72,14 +70,10 @@ After each turn, the notifier writes the `agent-turn-complete` JSON argument to Replacing a configured notifier would change user behavior. When the effective Codex configuration contains a one-line `notify` array of non-empty basic strings, the capture script executes that notifier after writing the capture file. Its argv is carried in `FLEETCOM_NOTIFY_CHAIN`, joined by newlines, and the notification payload is appended. An empty, multiline, ambiguous, or unsupported `notify` value disables the injected override so the configured route remains unchanged. The line-based configuration reader checks `config.toml` and the profile selected by its first `profile = ...` assignment; the profile's notify assignment takes precedence. -The exit scraper accepts `codex resume ` and `codex resume, then select ()`, using only the UUID. - ### `grok` Grok accepts a launch-time ID but exposes no injectable live-capture channel. A bare command therefore receives `--session-id ''`, while a canonical resume command needs no instrumentation. -After exit, the harness scans retained terminal text for the last `grok -r ` or `grok --resume ` hint. - ### `omp` omp cannot pin an ID at launch: it has no `--session-id`, and `--resume` requires an existing session. Both accepted forms therefore receive the same injection and no pinned ID: @@ -90,19 +84,20 @@ omp cannot pin an ID at launch: it has no `--session-id`, and `--resume` require `-e` loads the JavaScript module into the agent process and appends it to the user's extensions. Its `session_start` and `session_switch` handlers write `sessionId` as JSON to `FLEETCOM_CAPTURE_FILE`. The second handler follows in-TUI `/resume` changes. Capture writes are best-effort: the module returns when the capture path is empty and ignores write errors. -After exit, the harness scans retained terminal text for the last trusted `omp --resume ` hint. It accepts ordinary exit hints and `Main:` entries in `[Recovery]` blocks. Other labels identify subagent sessions that `omp --resume` cannot open, so they contribute no exit evidence. The aliases `-r`, `--session`, and `-c` remain opaque because `fleetcom` rewrites only the canonical form it detects exactly. +The aliases `-r`, `--session`, and `-c` remain opaque because `fleetcom` rewrites only the canonical form it detects exactly. ## ID precedence Several channels can identify different conversations during one task. To make the result deterministic, `fleetcom` chooses the first available ID in this order: -1. The exit hint scraped after process exit and PTY-reader EOF. -2. The current capture-file payload. -3. The live session registry, implemented by `claude`. -4. The ID pinned or targeted at spawn. +1. The current capture-file payload. +2. The live session registry, implemented by `claude`. +3. The ID pinned or targeted at spawn. Named saves, recovery snapshots, and reruns use this same precedence. `fleetcom` does not scan session stores to infer conversation ownership: a nearby transcript or rollout cannot identify which task owns it. +Terminal output never supplies a session ID: examples, quoted commands, and tool output can contain another conversation's valid UUID. An ID available only in an exit hint is not recovered. Without a capture, registry, or launch ID, the authored command remains unchanged. + The registry outranks the spawn pin because it can contain a session ID selected after launch, including one created by `/clear`. The capture file outranks the registry. Saving and rerunning rewrite accepted commands to one of these forms: @@ -118,7 +113,7 @@ The program word is preserved as typed. If no valid ID is available, the origina ## Validation boundary -Every captured value eventually enters a shell command, which makes validation the security boundary. Accepted IDs contain exactly lowercase hexadecimal characters in the `8-4-4-4-12` UUID shape. Capture payloads, terminal hints, registry records, and the final command builder all apply the same check. Malformed values are ignored rather than interpolated. +Every captured value eventually enters a shell command, which makes validation the security boundary. Accepted IDs contain exactly lowercase hexadecimal characters in the `8-4-4-4-12` UUID shape. Capture payloads, registry records, and the final command builder all apply the same check. Malformed values are ignored rather than interpolated. A valid UUID alone does not establish conversation ownership. ## Extending capture @@ -127,7 +122,6 @@ Each tool implements the `Harness` trait in [`src/harness/mod.rs`](../src/harnes - `shape` supplies the program word and resume selector. The default `detect` and `resume_command` methods derive the accepted and canonical forms from that pair. - `instrument` returns spawn-time arguments, environment entries, and an optional pinned ID. - `parse_capture` reads an ID from hook, notify, or extension JSON. -- `scrape_exit` reads an ID from retained terminal text. - `live_session_id` reads the ID a live session publishes on disk. It defaults to `None` for tools that publish no registry. - `live_blocked_status` reads that same registry for one display fact: whether the tool says it is blocked on the user. It returns preview text, never an ID, and defaults to `None`. - `resolve_home` resolves configuration needed by instrumentation or the live registry. It defaults to `None`. diff --git a/src/harness/claude.rs b/src/harness/claude.rs index 48a966e..b37c425 100644 --- a/src/harness/claude.rs +++ b/src/harness/claude.rs @@ -1,6 +1,6 @@ //! Claude session capture uses a launch-time `--session-id`, a `SessionStart` -//! hook, the live session registry, and the exit-time resume hint. Bare launches -//! pin a v4 UUID; accepted launches install the hook through `--settings`. +//! hook, and the live session registry. Bare launches pin a v4 UUID; accepted +//! launches install the hook through `--settings`. //! Live lookup reads `/sessions/.json`. use std::{ @@ -12,7 +12,7 @@ use std::{ use super::summary::AWAITING_APPROVAL; use super::{ CAPTURE_ENV, CapturePaths, Harness, Invocation, SpawnPlan, capture_id, home_root, is_uuid, - last_hint, pin_plan, resolve_home, shell_quote, + pin_plan, resolve_home, shell_quote, }; pub struct Claude; @@ -48,11 +48,6 @@ impl Harness for Claude { capture_id(&jzon::parse(payload).ok()?, "session_id") } - fn scrape_exit(&self, text: &str) -> Option { - // The last valid hint names the conversation at exit. - last_hint(text, &["claude --resume "]) - } - fn live_session_id( &self, pid: u32, @@ -153,7 +148,7 @@ mod tests { use super::*; use crate::{ - harness::fixtures::{ID, OTHER, assert_all_opaque, assert_corpus_scrape, paths}, + harness::fixtures::{ID, OTHER, assert_all_opaque, paths}, testutil::temp, }; @@ -263,21 +258,6 @@ mod tests { assert_eq!(Claude.parse_capture("{}"), None); } - #[test] - fn scrape_exit_takes_the_last_hint() { - let text = format!( - "Resume this session with:\nclaude --resume {OTHER}\n...\n\ - Resume this session with:\nclaude --resume {ID}\n" - ); - assert_eq!(Claude.scrape_exit(&text).as_deref(), Some(ID)); - - assert_eq!(Claude.scrape_exit("no hint here"), None); - // A hint whose ID fails validation returns nothing. - assert_eq!(Claude.scrape_exit("claude --resume NOT-A-UUID"), None); - // A longer hexadecimal run is not an ID. - assert_eq!(Claude.scrape_exit(&format!("claude --resume {ID}ff")), None); - } - /// A complete matching record exposes its validated session ID. #[test] fn record_for_pid_reads_a_live_record() { @@ -526,14 +506,4 @@ mod tests { None ); } - - /// The scraper recovers the exit-hint ID from the corpus terminal bytes. - #[test] - fn corpus_scrape_recovers_the_exit_hint_id() { - assert_corpus_scrape( - &Claude, - include_bytes!("../../tests/corpus/claude_resume.bin"), - "c8c4a5cc-0b32-4ba0-a6b4-6ed08c218e0d", - ); - } } diff --git a/src/harness/codex.rs b/src/harness/codex.rs index ee0de0f..03e3942 100644 --- a/src/harness/codex.rs +++ b/src/harness/codex.rs @@ -1,6 +1,5 @@ //! Codex does not let the caller select an ID at launch. This harness instead -//! injects a `notify` override, chains compatible configured notifiers, and -//! scans supported exit lines for an ID. +//! injects a `notify` override and chains compatible configured notifiers. use std::{ fmt::Write as _, @@ -10,7 +9,7 @@ use std::{ use super::{ CAPTURE_ENV, CapturePaths, Harness, Invocation, NOTIFY_CHAIN_ENV, SpawnPlan, capture_id, - home_root, last_hint, leading_uuid, resolve_home, shell_quote, + home_root, resolve_home, shell_quote, }; pub struct Codex; @@ -66,36 +65,6 @@ impl Harness for Codex { } capture_id(&v, "thread-id") } - - fn scrape_exit(&self, text: &str) -> Option { - let mut last = None; - for line in text.lines() { - // `Session ID:` has no program marker and can appear in captured - // conversation text. Accept it only at the start of a row. - if let Some(rest) = line.strip_prefix("Session ID: ") - && let Some(id) = leading_uuid(rest) - { - last = Some(id.to_string()); - } - // Plain hint: `... run codex resume `. - if let Some(id) = last_hint(line, &["codex resume "]) { - last = Some(id); - } - // Named-thread hint: `codex resume, then select ()`. - // Only the parenthesized ID is trusted, never the name. - if line.contains("codex resume") && line.contains("then select") { - for (i, _) in line.match_indices('(') { - let inner = &line[i + 1..]; - if let Some(id) = leading_uuid(inner) - && inner.as_bytes().get(36) == Some(&b')') - { - last = Some(id.to_string()); - } - } - } - } - last - } } /// Whether Codex notification capture can preserve the configured route. @@ -229,8 +198,8 @@ mod tests { use super::*; use crate::{ - harness::fixtures::{OTHER, assert_all_opaque, assert_corpus_scrape, paths}, - testutil::{CORPUS_COLS, Scratch, temp}, + harness::fixtures::{assert_all_opaque, paths}, + testutil::{Scratch, temp}, }; /// Codex's own launch and resume commands carry v7 IDs; the shared v4 @@ -511,55 +480,6 @@ mod tests { assert_eq!(Codex.parse_capture("not json"), None); } - #[test] - fn scrape_exit_reads_both_hint_shapes_and_never_names() { - let plain = format!("To continue this session, run codex resume {ID}"); - assert_eq!(Codex.scrape_exit(&plain).as_deref(), Some(ID)); - - let named = format!("To continue this session, run codex resume, then select docs ({ID})"); - assert_eq!(Codex.scrape_exit(&named).as_deref(), Some(ID)); - - // A named form without an ID yields nothing. - assert_eq!( - Codex.scrape_exit("run codex resume, then select my-thread"), - None - ); - assert_eq!(Codex.scrape_exit("codex resume my-thread"), None); - - // The last hint wins. - let both = format!("run codex resume {OTHER}\n...\nrun codex resume, then select x ({ID})"); - assert_eq!(Codex.scrape_exit(&both).as_deref(), Some(ID)); - } - - /// A fatal exit can name the session without printing a resume hint. - #[test] - fn scrape_exit_reads_the_fatal_session_id_line() { - assert_eq!( - Codex.scrape_exit(&format!("Session ID: {ID}")).as_deref(), - Some(ID) - ); - - // The label alone, a name, and a token-extending ID yield nothing. - assert_eq!(Codex.scrape_exit("Session ID:"), None); - assert_eq!(Codex.scrape_exit("Session ID: my session"), None); - assert_eq!(Codex.scrape_exit(&format!("Session ID: {ID}ff")), None); - - // An indented or embedded label can be conversation text. - for quoted in [ - format!("the log said Session ID: {ID}"), - format!("• Session ID: {ID}"), - format!(" Session ID: {ID}"), - ] { - assert_eq!(Codex.scrape_exit("ed), None, "{quoted:?}"); - } - - // Across lines, the last valid ID wins. - let hint_last = format!("Session ID: {OTHER}\nrun codex resume {ID}"); - assert_eq!(Codex.scrape_exit(&hint_last).as_deref(), Some(ID)); - let id_last = format!("run codex resume {OTHER}\nSession ID: {ID}"); - assert_eq!(Codex.scrape_exit(&id_last).as_deref(), Some(ID)); - } - /// Notification chaining reads `config.toml` and ignores sibling files. #[test] fn config_notify_route_reads_config_toml_alone() { @@ -581,23 +501,4 @@ mod tests { fs::write(&cfg, "notify = [\"/a\"]\nnotify = [\"/b\"]\n").unwrap(); assert_eq!(config_notify_route(Some(&home)), NotifyRoute::Opaque); } - - /// A preceding full-width row does not merge with the session-ID row after - /// terminal emulation. - #[test] - fn fatal_session_id_holds_offset_zero_after_a_full_width_row() { - let bytes = format!("{}\r\nSession ID: {ID}\r\n", "x".repeat(CORPUS_COLS)); - assert_corpus_scrape(&Codex, bytes.as_bytes(), ID); - } - - /// The scraper recovers an SGR-split exit hint from the corpus bytes after - /// terminal emulation removes the styling. - #[test] - fn corpus_scrape_recovers_the_exit_hint_id() { - assert_corpus_scrape( - &Codex, - include_bytes!("../../tests/corpus/codex_resume.bin"), - "019f5453-de22-7240-b2e5-0d32692aa6d9", - ); - } } diff --git a/src/harness/grok.rs b/src/harness/grok.rs index 2a8ab4a..4357d72 100644 --- a/src/harness/grok.rs +++ b/src/harness/grok.rs @@ -1,10 +1,9 @@ //! Grok has no injectable live-capture channel. Bare launches instead pin a v4 -//! UUID, and completed tasks expose either `grok -r ` or -//! `grok --resume ` in terminal output. +//! UUID; canonical resume commands retain their explicit ID. use std::path::Path; -use super::{CapturePaths, Harness, Invocation, SpawnPlan, last_hint, pin_plan}; +use super::{CapturePaths, Harness, Invocation, SpawnPlan, pin_plan}; pub struct Grok; @@ -22,18 +21,13 @@ impl Harness for Grok { ) -> SpawnPlan { pin_plan(inv) } - - fn scrape_exit(&self, text: &str) -> Option { - // The last valid short or long resume hint names the conversation. - last_hint(text, &["grok -r ", "grok --resume "]) - } } #[cfg(test)] mod tests { use super::*; use crate::harness::{ - fixtures::{ID, OTHER, assert_all_opaque, assert_corpus_scrape, paths}, + fixtures::{ID, assert_all_opaque, paths}, is_uuid, }; @@ -87,34 +81,4 @@ mod tests { assert_eq!(Grok.parse_capture(&payload), None); assert_eq!(Grok.parse_capture(""), None); } - - #[test] - fn scrape_exit_reads_both_spellings_and_takes_the_last() { - let short = format!("Resume with: grok -r {ID}"); - assert_eq!(Grok.scrape_exit(&short).as_deref(), Some(ID)); - let long = format!("Resume with: grok --resume {ID}"); - assert_eq!(Grok.scrape_exit(&long).as_deref(), Some(ID)); - - // The last hint by position wins across spellings, either order. - let both = format!("grok -r {OTHER}\n...\ngrok --resume {ID}\n"); - assert_eq!(Grok.scrape_exit(&both).as_deref(), Some(ID)); - let both = format!("grok --resume {OTHER}\n...\ngrok -r {ID}\n"); - assert_eq!(Grok.scrape_exit(&both).as_deref(), Some(ID)); - - assert_eq!(Grok.scrape_exit("no hint here"), None); - // A hint whose ID fails validation returns nothing. - assert_eq!(Grok.scrape_exit("grok -r NOT-A-UUID"), None); - // A longer hexadecimal run is not an ID. - assert_eq!(Grok.scrape_exit(&format!("grok -r {ID}ff")), None); - } - - /// The scraper recovers the exit-hint ID from the corpus terminal bytes. - #[test] - fn corpus_scrape_recovers_the_exit_hint_id() { - assert_corpus_scrape( - &Grok, - include_bytes!("../../tests/corpus/grok_resume.bin"), - "17ac97af-8cfc-46a7-9599-8cea45a687a6", - ); - } } diff --git a/src/harness/mod.rs b/src/harness/mod.rs index b63a0c3..4bd02cb 100644 --- a/src/harness/mod.rs +++ b/src/harness/mod.rs @@ -9,7 +9,7 @@ //! //! # Security invariant //! -//! Every ID returned by `parse_capture`, `scrape_exit`, or `live_session_id` +//! Every ID returned by `parse_capture` or `live_session_id` //! eventually enters a shell command. These methods return only //! strings accepted by [`is_uuid`]; free text, paths, and malformed IDs yield //! `None`. Summary adapters and `live_blocked_status` are display-only. @@ -77,9 +77,6 @@ pub trait Harness: Sync { None } - /// Extract a session ID from final terminal text, including scrollback. - fn scrape_exit(&self, text: &str) -> Option; - /// Read the current session ID from the tool's on-disk registry. `pid`, /// `cwd`, and `spawned` identify the task; implementations must reject a /// record that does not match all three. Defaults to `None`. @@ -277,33 +274,6 @@ fn capture_id(v: &jzon::JsonValue, key: &str) -> Option { is_uuid(id).then(|| id.to_string()) } -/// Return the strict UUID at the start of `s`. The next byte must end the token; -/// an alphanumeric character, `-`, or `_` extends the token and rejects it. -fn leading_uuid(s: &str) -> Option<&str> { - let head = s.get(..36).filter(|h| is_uuid(h))?; - match s.as_bytes().get(36) { - Some(&c) if c.is_ascii_alphanumeric() || c == b'-' || c == b'_' => None, - _ => Some(head), - } -} - -/// Extract the ID after the last valid resume hint in `text`. Every -/// occurrence of every `hints` prefix competes when a strict UUID follows it, -/// and the largest byte offset wins across prefixes. -fn last_hint(text: &str, hints: &[&str]) -> Option { - let mut last: Option<(usize, String)> = None; - for hint in hints { - for (i, _) in text.match_indices(hint) { - if let Some(id) = leading_uuid(&text[i + hint.len()..]) - && last.as_ref().is_none_or(|(j, _)| i > *j) - { - last = Some((i, id.to_string())); - } - } - } - last.map(|(_, id)| id) -} - /// Generate a v4 UUID from `/dev/urandom`. A read failure returns `None`, which /// lets the caller launch without pinning an ID. fn uuid_v4() -> Option { @@ -344,17 +314,16 @@ fn shell_quote(s: &str) -> String { format!("'{}'", s.replace('\'', "'\\''")) } -/// Fixtures and assertions for harness detection and exit scraping. +/// Fixtures and assertions for harness detection and capture. #[cfg(test)] pub(crate) mod fixtures { use std::path::PathBuf; use super::{CapturePaths, Harness}; - use crate::testutil::corpus_emulator; /// Strict v4 UUID used wherever a valid session ID is needed. pub(crate) const ID: &str = "c8c4a5cc-0b32-4ba0-a6b4-6ed08c218e0d"; - /// A second distinct ID for last-hint, requote, and ambiguity cases. + /// A second distinct ID for requote and precedence cases. pub(crate) const OTHER: &str = "11111111-2222-4333-8444-555555555555"; /// Capture-path fixture. The spaced asset paths keep the shell- and @@ -377,14 +346,6 @@ pub(crate) mod fixtures { assert_eq!(resumed, *cmd, "an opaque command must never be rewritten"); } } - - /// Replay `bytes` at corpus geometry and assert the scraped exit ID. - pub(super) fn assert_corpus_scrape(h: &dyn Harness, bytes: &[u8], expected: &str) { - let mut emu = corpus_emulator(); - emu.process(bytes); - let text = emu.text_with_history(); - assert_eq!(h.scrape_exit(&text).as_deref(), Some(expected)); - } } #[cfg(test)] @@ -504,19 +465,6 @@ mod tests { assert!(!is_uuid("")); } - #[test] - fn leading_uuid_requires_a_token_boundary() { - assert_eq!(leading_uuid(ID), Some(ID)); - assert_eq!(leading_uuid(&format!("{ID} tail")), Some(ID)); - assert_eq!(leading_uuid(&format!("{ID})")), Some(ID)); - - // A continuing token is not an id. - assert_eq!(leading_uuid(&format!("{ID}f")), None); - assert_eq!(leading_uuid(&format!("{ID}-x")), None); - assert_eq!(leading_uuid(&format!("{ID}_x")), None); - assert_eq!(leading_uuid("short"), None); - } - #[test] fn uuid_v4_is_strict_versioned_and_random() { let a = uuid_v4().expect("/dev/urandom must be readable"); diff --git a/src/harness/omp.rs b/src/harness/omp.rs index 98bbbf8..cb70ceb 100644 --- a/src/harness/omp.rs +++ b/src/harness/omp.rs @@ -1,8 +1,7 @@ //! omp cannot pin a session ID at launch: it has no `--session-id` flag, and //! `--resume` requires an existing session. Live capture therefore loads an //! extension whose `session_start` and `session_switch` handlers write the -//! current ID. Exit capture reads `omp --resume ` hints from ordinary -//! exit output and `[Recovery]` blocks. +//! current ID. //! //! omp's IDs are UUIDv7. [`is_uuid`](super::is_uuid) validates the 8-4-4-4-12 //! lowercase-hex shape and not the version field, so they pass unchanged. @@ -12,16 +11,7 @@ use std::path::Path; -use super::{ - CAPTURE_ENV, CapturePaths, Harness, Invocation, SpawnPlan, capture_id, leading_uuid, - shell_quote, -}; - -/// Command fragment shared by ordinary exit and recovery hints. -const RESUME_HINT: &str = "omp --resume "; - -/// Label identifying the resumable session in a recovery block. -const MAIN_LABEL: &str = "Main"; +use super::{CAPTURE_ENV, CapturePaths, Harness, Invocation, SpawnPlan, capture_id, shell_quote}; pub struct Omp; @@ -54,29 +44,6 @@ impl Harness for Omp { fn parse_capture(&self, payload: &str) -> Option { capture_id(&jzon::parse(payload).ok()?, "sessionId") } - - /// Return the last trusted exit hint. Unlabelled hints are ordinary exit - /// lines. Labelled recovery hints count only when their label is `Main`; - /// other labels identify subagent sessions that `omp --resume` cannot open. - fn scrape_exit(&self, text: &str) -> Option { - let mut last: Option = None; - for (i, _) in text.match_indices(RESUME_HINT) { - let Some(id) = leading_uuid(&text[i + RESUME_HINT.len()..]) else { - continue; - }; - let head = &text[..i]; - let head = &head[head.rfind(['\n', '\r']).map_or(0, |n| n + 1)..]; - // A trailing `": "` marks a labelled recovery entry. Reject - // unknown labels because exit evidence outranks the capture file. - if let Some(label) = head.strip_suffix(": ") - && label.trim() != MAIN_LABEL - { - continue; - } - last = Some(id.to_string()); - } - last - } } #[cfg(test)] @@ -84,7 +51,7 @@ mod tests { use std::path::PathBuf; use super::*; - use crate::harness::fixtures::{ID, OTHER, assert_all_opaque, assert_corpus_scrape, paths}; + use crate::harness::fixtures::{ID, assert_all_opaque, paths}; /// Valid UUIDv7 used in capture payloads. const CAPTURED: &str = "01a0077c-e18e-7000-ae0b-016f4834b6e9"; @@ -152,53 +119,4 @@ mod tests { ); assert_eq!(Omp.parse_capture(""), None); } - - #[test] - fn scrape_exit_reads_the_hint_and_takes_the_last() { - let hint = format!("Resume this session with omp --resume {ID}"); - assert_eq!(Omp.scrape_exit(&hint).as_deref(), Some(ID)); - - // The last hint by position wins. - let both = format!( - "Resume this session with omp --resume {OTHER}\n...\n\ - Resume this session with omp --resume {ID}\n" - ); - assert_eq!(Omp.scrape_exit(&both).as_deref(), Some(ID)); - - // A labelled recovery hint names the main session. - let crash = format!("[Recovery]\n Main: omp --resume {ID}\n"); - assert_eq!(Omp.scrape_exit(&crash).as_deref(), Some(ID)); - - // Subagent labels do not displace the main session. - let sub_a = "22222222-3333-4444-8555-666666666666"; - let sub_b = "33333333-4444-4555-8666-777777777777"; - let subagents = - format!(" agent-1: omp --resume {sub_a}\n agent-2: omp --resume {sub_b}\n"); - let swarm = format!("[Recovery]\n Main: omp --resume {ID}\n{subagents}"); - assert_eq!(Omp.scrape_exit(&swarm).as_deref(), Some(ID)); - - // A recovery block containing only subagents yields no exit evidence. - let orphans = format!("[Recovery]\n{subagents}"); - assert_eq!(Omp.scrape_exit(&orphans), None); - - // A later unlabelled exit hint remains eligible. - let recovered = format!("{orphans}...\nResume this session with omp --resume {ID}\n"); - assert_eq!(Omp.scrape_exit(&recovered).as_deref(), Some(ID)); - - assert_eq!(Omp.scrape_exit("no hint here"), None); - // A hint whose ID fails validation returns nothing. - assert_eq!(Omp.scrape_exit("omp --resume NOT-A-UUID"), None); - // A longer hexadecimal run is not an ID. - assert_eq!(Omp.scrape_exit(&format!("omp --resume {ID}ff")), None); - } - - /// The scraper recovers the exit-hint ID from the corpus terminal bytes. - #[test] - fn corpus_scrape_recovers_the_exit_hint_id() { - assert_corpus_scrape( - &Omp, - include_bytes!("../../tests/corpus/omp_resume.bin"), - "01a0078a-7714-7000-9927-f167df9b6476", - ); - } } diff --git a/src/supervisor.rs b/src/supervisor.rs index e901cd0..0b43ebd 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -133,13 +133,10 @@ fn fnv1a_hex(bytes: &[u8]) -> String { format!("{h:016x}") } -/// Resolve the session ID in precedence order: exit scrape, capture file, live -/// registry, then spawn-time ID. The first three can reflect a session selected +/// Resolve the session ID in precedence order: capture file, live registry, +/// then spawn-time ID. The first two can reflect a session selected /// after launch and therefore outrank the spawn-time value. fn current_resume_id(task: &Task) -> Option { - if let Some(id) = &task.scraped_id { - return Some(id.clone()); - } if let (Some(h), Some(path)) = (task.harness, &task.capture_file) && let Ok(payload) = std::fs::read_to_string(path) && let Some(id) = h.parse_capture(&payload) @@ -159,14 +156,6 @@ fn current_resume_id(task: &Task) -> Option { task.resume_id.clone() } -/// Poll for exit before save or rerun reads the session ID. Scraping remains -/// deferred until the PTY reader reaches EOF and the terminal contains every -/// child byte. -fn scrape_now(t: &mut Task) { - let _ = t.poll_exit(); - t.scrape_exit_hint(); -} - /// Resolve harness configuration from the task's launch environment. fn harness_home(env: &[(OsString, OsString)], h: &dyn harness::Harness) -> Option { h.resolve_home(&|key| env_get(env, key).map(PathBuf::from)) @@ -446,16 +435,13 @@ impl Supervisor { // latched this pass and is retried next. waitid failing is rare and // must not take down the loop. let _ = t.poll_exit(); - // Scrape after process exit and reader EOF, when every child byte - // is present in the grid (see `Task::scrape_exit_hint`). - t.scrape_exit_hint(); // Freeze the preview from the complete output and final screen. t.finalize_preview(); if t.overdue(now, self.kill_grace) { t.force_kill(); } } - // Removed tasks need exit handling and escalation, not hint scraping. + // Removed tasks still need exit handling and escalation. for t in &mut self.graveyard { let _ = t.poll_exit(); if t.overdue(now, self.kill_grace) { @@ -605,8 +591,7 @@ impl Supervisor { self.recovery.dirty = false; return; }; - // Refresh finished tasks' resume IDs before serialization. - let cfg = self.refreshed_config(); + let cfg = self.session_config(); // Exclude the timestamped label from content comparison. let hash = fnv1a_hex(session::fingerprint_json(&cfg).as_bytes()); // Deduplication is scoped to the current root and requires the snapshot @@ -850,9 +835,8 @@ impl Supervisor { self.status(format!("rerun failed: no task {id}")); return; }; - // Latch a recent exit and scrape its drained terminal before choosing - // the rerun command. - scrape_now(&mut self.tasks[i]); + // A task can exit between the last reap tick and this request. + let _ = self.tasks[i].poll_exit(); if self.tasks[i].finished.is_none() { self.status("rerun failed: task is still running"); return; @@ -888,14 +872,6 @@ impl Supervisor { } } - /// Refresh finished tasks' resume IDs before building the session recipe. - fn refreshed_config(&mut self) -> SessionConfig { - for t in &mut self.tasks { - scrape_now(t); - } - self.session_config() - } - /// Build `{dir: [entries]}` in spawn order. Groups and names remain intact; /// agent entries use the command returned by `recipe_command`. fn session_config(&self) -> SessionConfig { @@ -940,7 +916,7 @@ impl Supervisor { } fn save_session(&mut self, name: &str) { - let cfg = self.refreshed_config(); + let cfg = self.session_config(); let count: usize = cfg.values().map(Vec::len).sum(); let status = match self .sessions_root() diff --git a/src/supervisor_capture_tests.rs b/src/supervisor_capture_tests.rs index 811afe5..c8e9f4d 100644 --- a/src/supervisor_capture_tests.rs +++ b/src/supervisor_capture_tests.rs @@ -283,12 +283,7 @@ fn rerun_resumes_the_captured_conversation() { fn rerun_cannot_read_the_old_runs_stale_capture() { let dir = scratch("cap_stale_run"); let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - // The session drifts to CAP_ID mid-run and the exit hint reports it. - install_script( - &bin, - "claude", - &format!("printf 'Resume this session with:\\nclaude --resume {CAP_ID}\\n'"), - ); + install_stub(&bin, "claude", &dir); let mut s = sup_ctx(agent_ctx_plus( &bin, &runtime, @@ -297,16 +292,15 @@ fn rerun_cannot_read_the_old_runs_stale_capture() { )); spawn(&mut s, "claude", dir.to_path_buf()); let id = s.tasks[0].id; - // The capture file still holds the pre-drift session. + // The old run's final capture becomes the new run's launch ID. let stale = format!( r#"{{"session_id":"{CAP_OTHER}","hook_event_name":"SessionStart","source":"startup"}}"# ); let old_cap = s.tasks[0].capture_file.clone().expect("capture file set"); - std::fs::write(&old_cap, &stale).unwrap(); + std::fs::write(&old_cap, format!(r#"{{"session_id":"{CAP_ID}"}}"#)).unwrap(); assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .scraped_id + .finished .is_some())); - assert_eq!(s.tasks[0].scraped_id.as_deref(), Some(CAP_ID)); s.apply(Command::Restart { id }); let new_cap = s.tasks[0].capture_file.clone().expect("capture file set"); @@ -574,262 +568,170 @@ fn spawn_omp_loads_the_capture_extension() { assert!(t.resume_id.is_none(), "omp cannot pin an id at launch"); } -/// A `grok` exit hint becomes the session ID used by the saved recipe. -/// The spawn pin stays; scrape must outrank it. +/// Printed hints are display content even after exit and reader EOF. Named +/// saves, recovery snapshots, and reruns retain the capture or launch ID; an +/// uncaptured task retains its exact authored command. #[test] -fn grok_exit_hint_is_scraped_and_saved_as_a_resume() { - let dir = scratch("grok_scrape_exit"); - let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - install_script( - &bin, - "grok", - &format!("printf 'Resume this session with:\\ngrok --resume {CAP_ID}\\n'"), - ); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config)], - )); - spawn(&mut s, "grok", dir.to_path_buf()); - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .scraped_id - .is_some())); - assert_eq!(s.tasks[0].scraped_id.as_deref(), Some(CAP_ID)); - assert!( - s.tasks[0].resume_id.is_some(), - "the spawn pin stays; scrape must outrank it" - ); - assert_ne!(s.tasks[0].resume_id.as_deref(), Some(CAP_ID)); - - let text = save_and_read(&mut s, &config, "hint"); - assert!( - text.contains(&format!("grok --resume '{CAP_ID}'")), - "the recipe must resume the scraped session; got {text}" - ); +fn printed_resume_hints_do_not_change_saved_recovery_or_rerun_commands() { + for (tool, hints) in [ + ( + "claude", + format!("Resume this session with:\nclaude --resume {CAP_ID}\n"), + ), + ( + "codex", + format!( + "To continue this session, run codex resume {CAP_ID}\n\ + To continue this session, run codex resume, then select docs ({CAP_ID})\n\ + Session ID: {CAP_ID}\n" + ), + ), + ( + "grok", + format!("grok -r {CAP_ID}\ngrok --resume {CAP_ID}\n"), + ), + ( + "omp", + format!( + "Resume this session with omp --resume {CAP_ID}\n\ + [Recovery]\n Main: omp --resume {CAP_ID}\n" + ), + ), + ] { + let dir = scratch(tool); + let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); + install_script(&bin, tool, &format!("printf '%s' '{hints}'")); + let mut s = sup_ctx(agent_ctx_plus( + &bin, + &runtime, + dir.to_path_buf(), + &[ + ("FLEETCOM_CONFIG_DIR", &config), + ("CLAUDE_CONFIG_DIR", &dir.join("claude-home")), + ("CODEX_HOME", &dir.join("codex-home")), + ], + )); + s.set_recovery_timing(Duration::from_millis(20), Duration::from_millis(100)); + let authored = format!(" {}/{}\t", bin.display(), tool); + spawn(&mut s, &authored, dir.to_path_buf()); + s.tasks[0].group = Some("agents".into()); + s.tasks[0].name = Some(tool.into()); + let expected_id = match tool { + "claude" => { + std::fs::write( + s.tasks[0].capture_file.as_ref().unwrap(), + format!(r#"{{"session_id":"{CAP_OTHER}"}}"#), + ) + .unwrap(); + Some(CAP_OTHER.to_string()) + } + "grok" => s.tasks[0].resume_id.clone(), + _ => None, + }; + assert_ne!(expected_id.as_deref(), Some(CAP_ID)); + let command = match expected_id.as_deref() { + Some(id) => format!("{}/{} --resume '{id}'", bin.display(), tool), + None => authored, + }; + assert!( + reap_until(&mut s, Duration::from_secs(5), |s| { + s.tasks[0].finished.is_some() && s.tasks[0].reader_done() + }), + "{tool}: output never completed" + ); + // EOF can become visible after this pass's per-task work. + s.reap(); + assert!( + s.tasks[0] + .screen_lines() + .iter() + .any(|line| line.contains(CAP_ID)), + "{tool}: the printed UUID must reach the terminal" + ); + let expected = SessionConfig::from([( + path::abbreviate(&dir), + vec![SessionEntry { + cmd: command.clone(), + group: Some("agents".into()), + name: Some(tool.into()), + }], + )]); + save_and_read(&mut s, &config, "hints"); + assert_eq!( + session::load_in(&config.join("sessions"), "hints").unwrap(), + expected, + "{tool}: named save" + ); + assert!( + wait_until(Duration::from_secs(5), || { + s.tick(); + !recovery_files(&config).is_empty() + }), + "{tool}: recovery snapshot never landed" + ); + let files = recovery_files(&config); + assert_eq!(files.len(), 1); + let stem = files[0].strip_suffix(".json").unwrap(); + assert_eq!( + session::load_recovery_in(&config.join("sessions/recovery"), stem).unwrap(), + expected, + "{tool}: recovery" + ); + let id = s.tasks[0].id; + s.apply(Command::Restart { id }); + assert_eq!(s.tasks[0].run, 1, "{tool}: rerun must replace the task"); + assert_eq!(s.tasks[0].command, command, "{tool}: rerun command"); + assert_eq!(s.tasks[0].resume_id, expected_id, "{tool}: rerun ID"); + } } -/// Saving between process exit and the next reap tick still captures the -/// grok exit hint because `save_session` performs its own ready scrape. +/// Rerun latches a recent exit before checking eligibility and retains the +/// explicit launch ID when terminal output names another conversation. #[test] -fn save_scrapes_a_finished_grok_task_without_reap() { - let dir = scratch("grok_save_sync_scrape"); - let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); +fn rerun_latches_exit_without_reap_and_preserves_the_launch_id() { + use rustix::process::{Pid, WaitId, WaitIdOptions, waitid}; + let dir = scratch("rerun_without_reap"); + let (bin, runtime) = (dir.join("bin"), dir.join("run")); install_script( &bin, "grok", - &format!("printf 'Resume this session with:\\ngrok --resume {CAP_ID}\\n'"), + &format!( + "printf '%s\\n' \"$@\" > '{}/argv'\n\ + printf 'grok --resume {CAP_ID}\\n'", + dir.display() + ), ); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, + let mut s = sup_ctx(agent_ctx(&bin, &runtime, dir.to_path_buf())); + spawn( + &mut s, + format!("grok --resume {CAP_OTHER}"), dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config)], - )); - spawn(&mut s, "grok", dir.to_path_buf()); - - assert!( - wait_until(Duration::from_secs(5), || s.tasks[0].reader_done()), - "the stub never reached EOF" - ); - assert!(s.tasks[0].finished.is_none(), "no reap may have run yet"); - - let text = save_and_read(&mut s, &config, "syncsave"); - assert!( - text.contains(&format!("grok --resume '{CAP_ID}'")), - "save must scrape the finished task itself; got {text}" ); - assert_eq!(s.tasks[0].scraped_id.as_deref(), Some(CAP_ID)); -} - -/// Rerunning between process exit and the next reap tick latches the exit, -/// scrapes the grok hint, and resumes that session. -#[test] -fn rerun_scrapes_a_finished_grok_task_without_reap() { - let dir = scratch("grok_rerun_sync_scrape"); - let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - install_script( - &bin, - "grok", - &format!("printf 'Resume this session with:\\ngrok --resume {CAP_ID}\\n'"), - ); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config)], - )); - spawn(&mut s, "grok", dir.to_path_buf()); let id = s.tasks[0].id; - - assert!( - wait_until(Duration::from_secs(5), || s.tasks[0].reader_done()), - "the stub never reached EOF" - ); - assert!(s.tasks[0].finished.is_none(), "no reap may have run yet"); - - s.apply(Command::Restart { id }); - assert_eq!( - s.tasks[0].command, - format!("grok --resume '{CAP_ID}'"), - "rerun must compute its resume command from the exit scrape" - ); -} - -/// A `claude` exit hint becomes the session ID used by the saved recipe. -#[test] -fn exit_hint_is_scraped_and_saved_as_a_resume() { - let dir = scratch("scrape_exit"); - let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - install_script( - &bin, - "claude", - &format!("printf 'Resume this session with:\\nclaude --resume {CAP_ID}\\n'"), - ); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config)], - )); - spawn(&mut s, "claude", dir.to_path_buf()); - // No pre-exit synchronization: the scrape's reader-EOF gate means - // reap can run against the exiting stub at any point and the hint - // still lands. - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .scraped_id - .is_some())); - assert_eq!(s.tasks[0].scraped_id.as_deref(), Some(CAP_ID)); - - let text = save_and_read(&mut s, &config, "hint"); + let pid = Pid::from_raw(s.tasks[0].pid().unwrap() as i32).unwrap(); assert!( - text.contains(&format!("claude --resume '{CAP_ID}'")), - "the recipe must resume the scraped session; got {text}" - ); -} - -/// Saving between process exit and the next reap tick still captures the -/// exit hint because `save_session` performs its own ready scrape. -#[test] -fn save_scrapes_a_finished_task_without_reap() { - let dir = scratch("save_sync_scrape"); - let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - install_script( - &bin, - "claude", - &format!("printf 'Resume this session with:\\nclaude --resume {CAP_ID}\\n'"), - ); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config)], - )); - spawn(&mut s, "claude", dir.to_path_buf()); - - // Wait out only the residual reader-drain race: after EOF the sole - // remaining gate is the exit latch, which save's own pass must flip. - assert!( - wait_until(Duration::from_secs(5), || s.tasks[0].reader_done()), - "the stub never reached EOF" - ); - assert!(s.tasks[0].finished.is_none(), "no reap may have run yet"); - - let text = save_and_read(&mut s, &config, "syncsave"); - assert!( - text.contains(&format!("claude --resume '{CAP_ID}'")), - "save must scrape the finished task itself; got {text}" - ); - assert_eq!(s.tasks[0].scraped_id.as_deref(), Some(CAP_ID)); -} - -/// Rerunning between process exit and the next reap tick latches the exit, -/// scrapes the hint, and resumes that session. -#[test] -fn rerun_scrapes_a_finished_task_without_reap() { - let dir = scratch("rerun_sync_scrape"); - let (bin, runtime) = (dir.join("bin"), dir.join("run")); - install_script( - &bin, - "claude", - &format!("printf 'Resume this session with:\\nclaude --resume {CAP_ID}\\n'"), + wait_until(Duration::from_secs(5), || { + let exited = waitid( + WaitId::Pid(pid), + WaitIdOptions::EXITED | WaitIdOptions::NOWAIT | WaitIdOptions::NOHANG, + ) + .unwrap() + .is_some(); + exited && s.tasks[0].reader_done() + }), + "the stub never exited and drained its output" ); - let mut s = sup_ctx(agent_ctx(&bin, &runtime, dir.to_path_buf())); - spawn(&mut s, "claude", dir.to_path_buf()); - let id = s.tasks[0].id; - assert!( - wait_until(Duration::from_secs(5), || s.tasks[0].reader_done()), - "the stub never reached EOF" + s.tasks[0].finished.is_none(), + "no exit latch may have run yet" ); - assert!(s.tasks[0].finished.is_none(), "no reap may have run yet"); - + std::fs::remove_file(dir.join("argv")).unwrap(); s.apply(Command::Restart { id }); + assert_eq!(s.tasks[0].run, 1); + assert_eq!(s.tasks[0].command, format!("grok --resume '{CAP_OTHER}'")); assert_eq!( - s.tasks[0].command, - format!("claude --resume '{CAP_ID}'"), - "rerun must compute its resume command from the exit scrape" - ); -} - -/// Session-ID precedence is exit scrape, capture file, then spawn-time ID. -#[test] -fn resume_id_precedence_scrape_over_capture_over_spawn() { - let dir = scratch("precedence"); - let (bin, runtime, config) = (dir.join("bin"), dir.join("run"), dir.join("config")); - let (hinted, done) = (dir.join("hinted"), dir.join("done")); - install_script( - &bin, - "claude", - &format!( - "until [ -e '{h}' ]; do sleep 0.05; done\n\ - printf 'Resume this session with:\\nclaude --resume {CAP_ID}\\n'\n\ - until [ -e '{d}' ]; do sleep 0.05; done", - h = hinted.display(), - d = done.display() - ), - ); - let mut s = sup_ctx(agent_ctx_plus( - &bin, - &runtime, - dir.to_path_buf(), - &[("FLEETCOM_CONFIG_DIR", &config)], - )); - spawn(&mut s, "claude", dir.to_path_buf()); - let injected = s.tasks[0] - .resume_id - .clone() - .expect("a fresh claude launch pins an id"); - assert_ne!(injected.as_str(), CAP_OTHER); - - // The hook moved the session mid-run: pre-exit, the capture file - // must beat the injected id. - let cap = s.tasks[0].capture_file.clone().expect("capture file set"); - std::fs::write( - &cap, - format!( - r#"{{"session_id":"{CAP_OTHER}","hook_event_name":"SessionStart","source":"clear"}}"# - ), - ) - .unwrap(); - let text = save_and_read(&mut s, &config, "mid"); - assert!( - text.contains(&format!("claude --resume '{CAP_OTHER}'")), - "pre-exit the capture file must beat the injected id; got {text}" - ); - - // Print the hint and let the task exit: post-exit, the scrape must - // beat the capture file. - std::fs::write(&hinted, b"").unwrap(); - std::fs::write(&done, b"").unwrap(); - assert!(reap_until(&mut s, Duration::from_secs(5), |s| s.tasks[0] - .scraped_id - .is_some())); - assert_eq!(s.tasks[0].scraped_id.as_deref(), Some(CAP_ID)); - let text = save_and_read(&mut s, &config, "post"); - assert!( - text.contains(&format!("claude --resume '{CAP_ID}'")), - "post-exit the scraped hint must beat the capture file; got {text}" + wait_argv(&mut s, &dir.join("argv")), + ["--resume", CAP_OTHER] ); } diff --git a/src/task.rs b/src/task.rs index 9067742..ba71e99 100644 --- a/src/task.rs +++ b/src/task.rs @@ -105,16 +105,11 @@ pub struct Task { pub summary_adapter: Option<&'static dyn crate::preview::SummaryAdapter>, /// Run number used to give each rerun a distinct capture path. pub run: u32, - /// Session ID injected or recognized at spawn. Capture data, a live - /// registry record, or an exit hint can supersede it. + /// Session ID injected or recognized at spawn. Capture data or a live + /// registry record can supersede it. pub resume_id: Option, /// Capture path allocated for this task run. pub capture_file: Option, - /// Session ID scraped once from final terminal text after exit and reader - /// EOF. - pub scraped_id: Option, - /// Whether the one-shot full-history exit scrape has run. - scraped: bool, /// Dashboard-preview resolution state; resets with the task on rerun /// because a rerun replaces the whole `Task`. preview: PreviewState, @@ -348,8 +343,6 @@ impl Task { run: 0, resume_id: None, capture_file: None, - scraped_id: None, - scraped: false, preview: PreviewState::new(), blocked: None, blocked_probed: None, @@ -398,29 +391,7 @@ impl Task { self.finished.is_some() && self.handle.as_ref().is_none_or(JoinHandle::is_finished) } - /// Scrape at most one exit hint after the process exits and the PTY reader - /// reaches EOF (see [`Task::output_complete`]). - pub fn scrape_exit_hint(&mut self) { - let Some(h) = self.harness else { return }; - if self.scraped || !self.output_complete() { - return; - } - self.scraped = true; - let text = { - let mut emu = grid(&self.parser); - // Land any open synchronized frame before scraping. The reader is - // stopped, so no closing ESU can arrive; all slave fds are closed, - // so generated probe replies have no recipient. - let _ = emu.finish_output(); - emu.text_with_history() - }; - if let Some(id) = h.scrape_exit(&text) { - self.scraped_id = Some(id); - } - } - - /// Report whether the reader reached EOF. Tests use this second scrape gate - /// without driving the reap loop. + /// Report whether the reader reached EOF without driving the reap loop. #[cfg(test)] pub(crate) fn reader_done(&self) -> bool { self.handle.as_ref().is_none_or(|h| h.is_finished()) diff --git a/src/task_tests.rs b/src/task_tests.rs index bf2a7f4..310a338 100644 --- a/src/task_tests.rs +++ b/src/task_tests.rs @@ -332,15 +332,12 @@ fn input_hints_track_child_modes() { t.terminate(); } -/// The scrape waits on two criteria: the child must have exited and the PTY -/// reader must have stopped; a live reader may still hold bytes that -/// have not reached the grid. +/// A live reader may still hold bytes that have not reached the grid, so +/// finalization waits for reader EOF even after the child exits. #[test] -fn scrape_exit_hint_waits_for_reader_eof() { - const ID: &str = "c8c4a5cc-0b32-4ba0-a6b4-6ed08c218e0d"; - let cmd = format!("printf 'Resume this session with:\\nclaude --resume {ID}\\n'"); - let mut t = Task::spawn(20, &cmd, &cmd, &here(), 24, 80, 2000, &sh_env(), no_waker()).unwrap(); - t.harness = Some(&crate::harness::Claude); +fn finalize_preview_waits_for_reader_eof() { + let cmd = "printf 'test result: ok\\n'"; + let mut t = Task::spawn(20, cmd, cmd, &here(), 24, 80, 2000, &sh_env(), no_waker()).unwrap(); assert!( wait_until(Duration::from_secs(60), || { t.poll_exit().unwrap(); @@ -355,8 +352,12 @@ fn scrape_exit_hint_waits_for_reader_eof() { t.handle = Some(thread::spawn(move || { let _ = parked.recv(); })); - t.scrape_exit_hint(); - assert_eq!(t.scraped_id, None, "the scrape must wait for reader EOF"); + t.finalize_preview(); + assert!( + !t.resolve_preview(Instant::now()).frozen, + "finalization must wait for reader EOF" + ); + grid(&t.parser).process(b"late output\r\n"); // Dropping the sender ends the stand-in: the reader reached EOF. drop(release); @@ -364,19 +365,17 @@ fn scrape_exit_hint_waits_for_reader_eof() { wait_until(Duration::from_secs(60), || t.reader_done()), "the stand-in reader never stopped" ); - t.scrape_exit_hint(); - assert_eq!(t.scraped_id.as_deref(), Some(ID)); + t.finalize_preview(); + let p = t.resolve_preview(Instant::now()); + assert_eq!((p.text.as_str(), p.frozen), ("late output", true)); } -/// A child that dies with a `?2026` frame still open leaves its hint -/// buffered in the parser, and no ESU can ever arrive to release it: the -/// scrape must land the frame instead of reading pre-frame text. +/// A child that dies with a `?2026` frame still open leaves output buffered: +/// no ESU can arrive, so finalization must land the frame before resolving. #[test] -fn scrape_exit_hint_lands_an_open_sync_frame() { - const ID: &str = "7f3b9c1e-5a2d-4e8f-9b6a-0c4d2e8f1a3b"; - let cmd = format!("printf '\\033[?2026hResume this session with:\\nclaude --resume {ID}\\n'"); - let mut t = Task::spawn(21, &cmd, &cmd, &here(), 24, 80, 2000, &sh_env(), no_waker()).unwrap(); - t.harness = Some(&crate::harness::Claude); +fn finalize_preview_lands_an_open_sync_frame() { + let cmd = "printf '\\033[?2026htest result: ok\\n'"; + let mut t = Task::spawn(21, cmd, cmd, &here(), 24, 80, 2000, &sh_env(), no_waker()).unwrap(); assert!( wait_until(Duration::from_secs(60), || { t.poll_exit().unwrap(); @@ -385,11 +384,13 @@ fn scrape_exit_hint_lands_an_open_sync_frame() { "child never exited" ); assert!( - !grid(&t.parser).text_with_history().contains(ID), - "premise: the unclosed frame still buffers the hint at scrape time" + !grid(&t.parser).contents().contains("test result: ok"), + "premise: the unclosed frame still buffers the final output" ); - t.scrape_exit_hint(); - assert_eq!(t.scraped_id.as_deref(), Some(ID)); + t.finalize_preview(); + assert!(grid(&t.parser).contents().contains("test result: ok")); + let p = t.resolve_preview(Instant::now()); + assert_eq!((p.text.as_str(), p.frozen), ("test result: ok", true)); } /// Primary-screen finalization re-resolves: a final line that lands diff --git a/src/terminal/emulator.rs b/src/terminal/emulator.rs index 316b783..ace2587 100644 --- a/src/terminal/emulator.rs +++ b/src/terminal/emulator.rs @@ -10,7 +10,7 @@ use alacritty_terminal::{ Term, event::{Event, EventListener}, grid::{Dimensions, Row, Scroll}, - index::{Column, Line}, + index::Line, term::{ Config, TermMode, cell::{Cell, Flags}, @@ -372,33 +372,6 @@ impl Emulator { crate::ansi::contents(&self.term) } - /// Reconstruct retained terminal text from the oldest history row through - /// the live viewport. Soft wraps join into logical lines, hard lines lose - /// trailing padding, and the current scroll offset does not affect output. - pub fn text_with_history(&self) -> String { - let grid = self.term.grid(); - let top = -(grid.history_size() as i32); - let bottom = grid.screen_lines() as i32 - 1; - let last_col = grid.columns() - 1; - let mut out = String::new(); - for row in top..=bottom { - let row_start = out.len(); - let line = &grid[Line(row)]; - push_row_glyphs(&mut out, line); - // A soft wrap continues on the next grid row. - if line[Column(last_col)].flags.contains(Flags::WRAPLINE) { - continue; - } - while out.len() > row_start && out.ends_with(' ') { - out.pop(); - } - if row < bottom { - out.push('\n'); - } - } - out - } - /// Which mouse events the child asked for; the most recent DECSET wins /// (the backend keeps the modes mutually exclusive). DECSET 9 (X10) is /// not modeled, so an X10-only child gets no mouse reports. @@ -1136,7 +1109,7 @@ mod tests { assert!(emu.contents().contains("and on")); } - /// The end-of-life landing `finish_output` exists for: BSU, a hint, no + /// The end-of-life landing `finish_output` exists for: BSU, output, no /// ESU ever. The frame must land without waiting out the sync timeout, /// and a clean emulator must pass through untouched. #[test] @@ -1149,12 +1122,12 @@ mod tests { emu.process(b"before\x1b[?2026hafter"); assert!( - !emu.text_with_history().contains("after"), + !emu.contents().contains("after"), "premise: the unclosed frame buffers the text" ); emu.finish_output(); assert!( - emu.text_with_history().contains("after"), + emu.contents().contains("after"), "finish_output must land the frame with the timeout still pending" ); @@ -1242,77 +1215,6 @@ mod tests { assert_eq!(emu.scrollback(), 0); } - /// Retained text includes scrollback in chronological order and does not - /// change when the viewport scroll offset changes. - #[test] - fn text_with_history_includes_scrolled_off_rows() { - let mut emu = Emulator::new(4, 10, 100); - for i in 0..12 { - emu.process(format!("l{i}\r\n").as_bytes()); - } - // 12 newlines on a 4-row screen: the first rows are history now. - assert!(!emu.contents().contains("l0")); - let full = emu.text_with_history(); - assert!(full.starts_with("l0"), "oldest history row leads"); - assert!(full.contains("l11"), "the live screen is included"); - // 9 history rows plus the 4-row viewport, one line per row. - assert_eq!(full.split('\n').count(), 13); - // The view offset must not change what is reported. - emu.set_scrollback(usize::MAX); - assert_eq!(emu.text_with_history(), full); - } - - /// Soft wraps reconstruct one logical line without erasing explicit line - /// breaks. - #[test] - fn text_with_history_joins_soft_wrapped_rows() { - let mut emu = Emulator::new(6, 20, 100); - let hint = "claude --resume 123e4567-e89b-42d3-a456-426614174000"; - emu.process(format!("before\r\n{hint}\r\nafter").as_bytes()); - let full = emu.text_with_history(); - assert!( - full.contains(hint), - "52 chars over 3 rows at 20 columns must come back unbroken: {full:?}" - ); - // Explicit newlines still bound logical lines on both sides. - assert!(full.contains(&format!("before\n{hint}\nafter"))); - } - - /// A wrapped codex named-thread hint remains one logical line. - #[test] - fn text_with_history_joins_codex_hint_across_rows() { - let mut emu = Emulator::new(8, 40, 100); - let hint = "To continue this session, run codex resume, then select \ - mythic-otter (123e4567-e89b-42d3-a456-426614174000)"; - emu.process(hint.as_bytes()); - assert!( - emu.text_with_history().contains(hint), - "the hint spans 3 rows at 40 columns and must join unbroken" - ); - } - - /// Wrap markers travel with rows into scrollback: a wrapped line pushed - /// off the live screen still joins, including across the history to - /// viewport boundary. - #[test] - fn text_with_history_joins_wrapped_rows_in_scrollback() { - let mut emu = Emulator::new(4, 20, 100); - let hint = "claude --resume 123e4567-e89b-42d3-a456-426614174000"; - emu.process(format!("{hint}\r\n").as_bytes()); - for i in 0..6 { - emu.process(format!("pad {i}\r\n").as_bytes()); - } - let full = emu.text_with_history(); - assert!( - !emu.contents().contains("claude"), - "premise: the hint scrolled fully into history" - ); - assert!( - full.contains(hint), - "history rows keep their wrap markers: {full:?}" - ); - } - /// Top-anchored region scrollback remains reachable after shrinking and /// regrowing the grid, while new output continues to accumulate. #[test] diff --git a/tests/corpus/README.md b/tests/corpus/README.md index a4ca1df..f283969 100644 --- a/tests/corpus/README.md +++ b/tests/corpus/README.md @@ -3,7 +3,7 @@ Synthetic escape sequences isolate parser rules, but they do not reproduce the state transitions emitted by real terminal programs. This corpus keeps their raw PTY output so the emulator tests can replay those transitions byte for -byte. The fixtures provide the evidence for display, parser, and resume-hint +byte. The fixtures provide the evidence for display and parser assertions that would otherwise depend on synthetic approximations. ## Capture method @@ -15,10 +15,9 @@ feed the bytes to the emulator verbatim. | Fixture | Scenario | Coverage | | --- | --- | --- | -| `claude_resume.bin` | `claude --session-id`: one prompt, reply, `/exit` | alternate-screen exit followed by the primary-screen resume hint (`claude --resume `); the scrape target for harness exit capture | -| `codex_resume.bin` | `codex resume` | top-anchored DECSTBM scroll regions (`CSI 1;N r`), reverse index, inline-TUI history insertion, and an SGR-split resume hint for harness exit capture | -| `grok_resume.bin` | `grok --session-id`: one prompt, reply, `/exit` | primary-screen exit followed by the resume hint (`grok --resume `); the scrape target for harness exit capture | -| `omp_resume.bin` | `omp`: one launch, `/exit` | primary-screen exit followed by the resume hint (`omp --resume `); the scrape target for harness exit capture | +| `claude_resume.bin` | `claude --session-id`: one prompt, reply, `/exit` | alternate-screen teardown and final primary-screen display | +| `codex_resume.bin` | `codex resume` | top-anchored DECSTBM scroll regions (`CSI 1;N r`), reverse index, inline-TUI history insertion, and SGR-styled exit output | +| `grok_resume.bin` | `grok --session-id`: one prompt, reply, `/exit` | final primary-screen display | | `tmux_split.bin` | `tmux` session with two splits and one command per pane | scroll regions, pane borders, full redraws | | `vim_session.bin` | `vim -u NONE`: insert, navigate, `:set number`, `:q!` | alternate screen, cursor addressing, line editing | | `less_altscreen.bin` | `less` over `/usr/share/dict/words`: page, `G`, `g`, `q` | alternate-screen entry and exit, full-screen paging | @@ -96,7 +95,7 @@ status rows carry a streamed intent phrase rather than omp's default ## What the fixtures prove -The fixtures provide evidence for three distinct boundaries: +The fixtures provide evidence for display and parser behavior: - `tmux_split`, `vim_session`, `less_altscreen`, `top_live`, `shell_colors`, and `build_log` pin displayed state: every plain-text row, the cursor, and @@ -104,10 +103,9 @@ The fixtures provide evidence for three distinct boundaries: - `codex_resume`, `wide_emoji`, `dec_scrollregion`, and `topregion_scroll` pin parser semantics: scrollback retention, intensity stacking, charset translation, and VS16 width. -- `claude_resume`, `codex_resume`, `grok_resume`, and `omp_resume` verify that - retained terminal text preserves the exit hints consumed by their harnesses. +- `claude_resume`, `codex_resume`, and `grok_resume` compare the emulator's + final display, cursor, and alternate-screen state with the terminal backend. -`src/golden.rs` contains the absolute display and parser expectations. -`src/harness/claude.rs`, `src/harness/codex.rs`, `src/harness/grok.rs`, and -`src/harness/omp.rs` contain the agent-resume scrape expectations. -`src/harness/summary.rs` contains the preview-fixture expectations. +`src/terminal/golden.rs` contains the absolute display and parser expectations +and the terminal-backend comparisons. `src/harness/summary_tests.rs` contains +the preview-fixture expectations. diff --git a/tests/corpus/omp_resume.bin b/tests/corpus/omp_resume.bin deleted file mode 100644 index a376c1f..0000000 --- a/tests/corpus/omp_resume.bin +++ /dev/null @@ -1,372 +0,0 @@ -[?2004h[?1l>[?u]11;?[?2031h[?2026$p[?2048$p[?2031$p[?1010$p[?1011$p[?5522h[?25l]0;π >]0;π > work3[?25l[?2026h[?7l -╭─── omp v17.3.4 ──────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ Welcome back! │ -│ │ -│ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │ -│ │ -│ /Users/chris/Documents/Models/Qwen/Qwen3.8-27B-UD-Q8_K_XL.gguf │ -│ llama.cpp │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ - Tip: `/copy code` grabs the last code block to your clipboard — `/copy cmd` grabs the last -  shell/python command - - Connecting to MCP servers: node_repl…  - -╭── π  > ⬢ /Users/chris/Documents/Models/Qwen/Qwen3.8-27B-UD-Q8_K_XL.gguf · ◒ high > 🗑 ]8;id=8327217c;file:///tmp/claude-501/-Users-chris-Documents-Code-Rust-fleetcom/195801f2-9949-4130-addc-4289c1cde424/scratchpad/work3\…hpad/work3]8;;\ > ◫ 11.8%/131K ⟲ ▶──╮]8;; -╰─   ─╯ - - - - - - - - - - - - - - - - - - -[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[>4;2m[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │ -│ │ -│ /Users/chris/Documents/Models/Qwen/Qwen3.8-27B-UD-Q8_K_XL.gguf │ -│ llama.cpp │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ - Tip: `/copy code` grabs the last code block to your clipboard — `/copy cmd` grabs the last -  shell/python command - - xdev: xd://: mounted mcp__node_repl_js, mcp__node_repl_js_add_node_module_dir, mcp__node_repl_js_reset  - -╭── π  > ⬢ /Users/chris/Documents/Models/Qwen/Qwen3.8-27B-UD-Q8_K_XL.gguf · ◒ high > 🗑 ]8;id=8327217c;file:///tmp/claude-501/-Users-chris-Documents-Code-Rust-fleetcom/195801f2-9949-4130-addc-4289c1cde424/scratchpad/work3\…hpad/work3]8;;\ > ◫ 12.2%/131K ⟲ ▶──╮]8;;[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[?25l[?2026h[?7l │ ▀██████████▀ │ -│ ╘██ ██ │ -│ ██ ██ │ -│ ██ ██ │ -│ ▄██▄ ▄██▄ │[?25h[?7h[?2026l[>4;0m[?25l[?2026h[?7l  Closing session… [?25h[?7h[?2026l [?25h[?2026l[?7h[?1l>[?2004l[?5522l[?1006l[?1003l[?1000l[?2031l -Resume this session with omp --resume 01a0078a-7714-7000-9927-f167df9b6476 -[?2026l[?7h[?1l>[?2004l[?2031l[?2048l[?5522l[?1006l[?1003l[?1000l[?25h \ No newline at end of file From d2b80bf0004d6e8edbcfbe0b4d1be316c5259ac0 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sat, 5 Sep 2026 23:42:28 -0700 Subject: [PATCH 4/5] Removed the `parked` field from `TaskView` and related structures and update protocol version --- docs/how-it-works.md | 2 +- src/app.rs | 17 +++-- src/app_readme_tests.rs | 26 -------- src/app_tests.rs | 130 ++++++++++++++++++++------------------ src/protocol.rs | 36 ++--------- src/protocol_tests.rs | 129 +++++++++++++++++++++---------------- src/supervisor.rs | 1 - src/task.rs | 5 -- src/task_tests.rs | 27 ++++---- src/ui.rs | 58 ++++++++++------- tests/common/mod.rs | 2 +- tests/daemon_handshake.rs | 14 ++-- 12 files changed, 215 insertions(+), 232 deletions(-) diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 2a59a52..6510fc5 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -10,4 +10,4 @@ Attached input follows the terminal modes reported by the child. Modified Enter ## Grouping follows one activity window -The dashboard groups tasks by state (In use / Running / Idle / Completed), working directory, or names assigned with `g`. One 10-second window drives both idle signals: after 10 s without output, the row glyph changes from `✻` to `∙` and, under state grouping, the task moves to the Idle section in the same refresh. Idle state does not affect row order within directory or custom sections. A tool such as `top`, which prints every 1–2 s, never crosses the window, so it stays `✻` under Running. Preview text is independent: a status from a weaker source must persist for 600 ms before it replaces a stronger one, which absorbs repaint flicker without affecting grouping. Claude's on-disk `waiting` status and screen-derived agent statuses both produce the top-tier `anchor` source. The on-disk status wins when both are present; returning to the screen-derived status does not cross a tier and therefore renders immediately. +The dashboard groups tasks by state (In use / Running / Idle / Completed), working directory, or names assigned with `g`. The lifecycle becomes Idle after more than 10 s without output: the row glyph changes from `✻` to `∙` and, under state grouping, the task moves to the Idle section in the same refresh. Idle state does not affect row order within directory or custom sections. A tool such as `top`, which prints every 1–2 s, never crosses the window, so it stays `✻` under Running. Preview text is independent: a status from a weaker source must persist for 600 ms before it replaces a stronger one, which absorbs repaint flicker without affecting grouping. Claude's on-disk `waiting` status and screen-derived agent statuses both produce the top-tier `anchor` source. The on-disk status wins when both are present; returning to the screen-derived status does not cross a tier and therefore renders immediately. diff --git a/src/app.rs b/src/app.rs index f593fc6..90db75f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -314,23 +314,22 @@ fn step_down(sel: usize, len: usize) -> usize { } /// State-section order: In use, Running, Idle, then Completed. -/// Tags take precedence, completion follows `lifecycle`, and live tasks use -/// `parked` to distinguish Running from Idle. +/// Tags take precedence over lifecycle. fn section_rank(v: &TaskView) -> u8 { if v.tagged { 0 - } else if matches!(v.lifecycle, Lifecycle::Ok | Lifecycle::Failed) { - 3 - } else if v.parked { - 2 } else { - 1 + match v.lifecycle { + Lifecycle::Active => 1, + Lifecycle::Idle => 2, + Lifecycle::Ok | Lifecycle::Failed => 3, + } } } /// Within-section row order: tagged, live, then finished. -/// `parked` does not affect row order, so transitions between active and idle -/// preserve a task's position outside state grouping. +/// Transitions between active and idle preserve a task's position outside +/// state grouping. fn row_rank(v: &TaskView) -> u8 { if v.tagged { 0 diff --git a/src/app_readme_tests.rs b/src/app_readme_tests.rs index 300933b..be4fd22 100644 --- a/src/app_readme_tests.rs +++ b/src/app_readme_tests.rs @@ -127,7 +127,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("dashboard".to_string()), name: Some("Dashboard Refine".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: anchor( "Scope small fixes for dashboard and CLI", "claude:action-row", @@ -144,7 +143,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("dashboard".to_string()), name: Some("Summary Refine".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: anchor("Inferring… · thinking with high effort", "claude:spinner"), started_ago: mins(5), quiet_ago: Some(secs(8)), @@ -158,7 +156,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("dashboard".to_string()), name: Some("Grok Language".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: anchor("Grok 4.5 (xhigh) · Responding…", "grok:spinner"), started_ago: mins(12), quiet_ago: Some(secs(4)), @@ -172,7 +169,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("dashboard".to_string()), name: Some("Codex Language".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: anchor(CODEX_LANGUAGE, "codex:working"), started_ago: mins(18), quiet_ago: Some(secs(2)), @@ -186,7 +182,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("dashboard".to_string()), name: Some("Codex Review".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: anchor(CODEX_REVIEW, "codex:working"), started_ago: mins(24), quiet_ago: Some(secs(6)), @@ -200,7 +195,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("tests".to_string()), name: None, lifecycle: Lifecycle::Ok, - parked: false, preview: frozen(FLEETCOM_TESTS), started_ago: mins(2), quiet_ago: None, @@ -214,7 +208,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("tests".to_string()), name: None, lifecycle: Lifecycle::Failed, - parked: false, preview: frozen(FLEETCOM_CLIPPY), started_ago: mins(5), quiet_ago: None, @@ -228,7 +221,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("desktop".to_string()), name: Some("claude agents".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: title("2 awaiting input · claude agents"), started_ago: mins(63), quiet_ago: Some(secs(9)), @@ -242,7 +234,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("desktop".to_string()), name: Some("Zellij".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: title("Desktop ¦ Utility"), started_ago: mins(126), quiet_ago: Some(secs(4)), @@ -256,7 +247,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: None, name: None, lifecycle: Lifecycle::Idle, - parked: true, preview: floor(">>>"), started_ago: mins(48), quiet_ago: Some(mins(41)), @@ -270,7 +260,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("desktop".to_string()), name: None, lifecycle: Lifecycle::Ok, - parked: false, preview: frozen("Already up-to-date."), started_ago: mins(14), quiet_ago: None, @@ -284,7 +273,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("turret".to_string()), name: Some("Game Infra Review".to_string()), lifecycle: Lifecycle::Active, - parked: false, preview: title("Turret Game Codebase Organization and Ex… - grok"), started_ago: mins(8), quiet_ago: Some(secs(5)), @@ -298,7 +286,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("turret".to_string()), name: Some("Missile Nerf".to_string()), lifecycle: Lifecycle::Active, - parked: false, // omp's spinner phrase carries no model-label prefix: the // adapter's model_label is None. preview: anchor("Tuning missile damage falloff", "omp:spinner"), @@ -314,7 +301,6 @@ fn live_fleet(dirs: &Dirs) -> Vec { group: Some("turret".to_string()), name: Some("EMP Nerf".to_string()), lifecycle: Lifecycle::Active, - parked: false, // At its prompt: the primary-screen title tier renders the // conversation label omp announces as `π >