From 0d9a63ad7e15be20ba55c4bda118dc7333a9afa7 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Sun, 18 Oct 2026 16:48:04 -0400 Subject: [PATCH 1/3] Refuse Ticket Stops and verify Stop across Command kinds (#774) --- README.md | 7 ++ src/logs/status/inspect.rs | 29 +++-- tests/stop.rs | 213 +++++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b29e155cd..840583771 100644 --- a/README.md +++ b/README.md @@ -592,6 +592,13 @@ the same Command names as `show`: `owner/repo#number`, an Issue URL, or a Pass's Commands are refused; Stop sends SIGTERM only to the recorded process, with no forced kill. +Stopping a Spec run ends its Tickets' Runs too, saving their unfinished work. +Only the Spec's issue receives Stop labels; its Tickets' labels stay as they +were. A Stop on a Pickup run applies to the issue it took, whether a Run or a +Spec run. A Pass still without an issue, such as an Architect run in its +Architecture review or a Security run in its audit, ends without changing +labels. Stop refuses a Ticket's issue and names the Spec run to stop instead. + By default, Stop leaves the issue for the Day shift: it adds `needs-triage` and does not put `ready-for-agent` back. It removes `in-progress` when nothing reached `origin`, and keeps it when an Issue branch or pull request remains. Add diff --git a/src/logs/status/inspect.rs b/src/logs/status/inspect.rs index 9613320d1..6c16cf0c0 100644 --- a/src/logs/status/inspect.rs +++ b/src/logs/status/inspect.rs @@ -351,12 +351,29 @@ pub fn show(input: &str, json: bool) -> Result<()> { pub(in crate::logs) fn stop_target(input: &str) -> Result { let status = super::read(true)?; match select(&status.commands, input)? { - Selected::Record(command) if command.state == "live" => Ok(command - .record - .parent_record - .as_ref() - .unwrap_or(&command.record_path) - .clone()), + Selected::Record(command) if command.record.parent_record.is_some() => { + let owner = status + .commands + .iter() + .find_map(|entry| { + let Entry::Record(root) = entry else { + return None; + }; + contains(root, command).then(|| { + root.dispatched + .iter() + .find(|dispatch| contains(dispatch, command)) + .unwrap_or(root) + }) + }) + .context("could not find the Command owning this child Run")?; + bail!( + "cannot stop {input}: child Run; stop {} {} instead", + owner.record.kind, + owner.record.name + ); + } + Selected::Record(command) if command.state == "live" => Ok(command.record_path.clone()), Selected::Record(command) => bail!( "cannot stop {input}: Command {}{}", command.state, diff --git a/tests/stop.rs b/tests/stop.rs index 64744811b..39aea72d3 100644 --- a/tests/stop.rs +++ b/tests/stop.rs @@ -3,6 +3,219 @@ mod support; use std::fs; use support::Scenario; +#[test] +fn stopping_a_pass_with_no_issue_ends_it_without_changing_labels() { + for kind in ["architect", "secure"] { + for requeue in [false, true] { + let scenario = Scenario::new(); + scenario.issue_labelled(7, &["bug"]); + let labels = scenario.gh_state()["labels"].clone(); + scenario.agent_does(&format!( + "touch '{}'\nwhile true; do sleep 0.05; done\n", + scenario.path("started").display() + )); + let mut held = scenario.run_until(&[kind], &[], "started"); + let status = scenario.run(&["status", "--json"]); + let status: serde_json::Value = serde_json::from_str(&status.stdout).unwrap(); + let command = &status["commands"][0]; + let name = command["name"].as_str().unwrap(); + let args = if requeue { + vec!["stop", name, "--requeue"] + } else { + vec!["stop", name] + }; + let stopped = scenario.run(&args); + if stopped.code != Some(0) { + held.signal("TERM"); + } + let ended = held.finish(); + assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); + assert_eq!(ended.code, Some(1), "{}", ended.stderr); + assert!(command["issue"].is_null(), "{command}"); + assert_eq!( + command["stage"]["name"], + if kind == "architect" { + "Architecture review" + } else { + "Security audit" + } + ); + assert!(stopped.stdout.contains("interrupted"), "{}", stopped.stdout); + assert!(!stopped.stdout.contains("left:"), "{}", stopped.stdout); + assert_eq!(scenario.gh_state()["labels"], labels); + let activity = fs::read_to_string( + scenario.path("home/.thirdshift/logs/acme/widgets/activity.log"), + ) + .unwrap(); + assert_eq!( + activity.matches("stopped from the CLI by").count(), + 1, + "{activity}" + ); + } + } +} + +#[test] +fn stopping_a_pickups_run_triages_or_requeues_the_issue_it_took() { + for requeue in [false, true] { + let scenario = Scenario::new(); + scenario.issue_labelled(7, &["bug", "ready-for-agent"]); + scenario.agent_does(&format!( + "echo work > work.txt\ngit add work.txt\ngit commit -qm Work\ngit push -q origin HEAD\ntouch '{}'\nwhile true; do sleep 0.05; done\n", + scenario.path("started").display() + )); + let mut held = scenario.run_until(&["pickup"], &[], "started"); + let args = if requeue { + vec!["stop", "widgets#7", "--requeue"] + } else { + vec!["stop", "widgets#7"] + }; + let stopped = scenario.run(&args); + if stopped.code != Some(0) { + held.signal("TERM"); + } + let ended = held.finish(); + assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); + assert_eq!(ended.code, Some(1), "{}", ended.stderr); + assert_eq!( + scenario.issue_labels(7), + if requeue { + vec!["bug", "ready-for-agent"] + } else { + vec!["bug", "in-progress", "needs-triage"] + } + ); + assert_eq!( + scenario.origin_file("issue-7", "work.txt"), + Some("work\n".into()) + ); + assert!( + stopped.stdout.contains("Issue branch left: issue-7"), + "{}", + stopped.stdout + ); + } +} + +#[test] +fn stopping_a_spec_ends_both_tickets_preserves_their_work_and_only_labels_the_spec() { + for (pickup, requeue) in [(false, false), (true, false), (true, true)] { + let scenario = Scenario::new(); + scenario.spec_has_tickets(20, &[(21, &[]), (22, &[])]); + scenario.issue_labelled(20, &["ready-for-agent"]); + for (ticket, sibling) in [(21, 22), (22, 21)] { + scenario.issue_labelled(ticket, &["bug", "ready-for-agent"]); + scenario.agent_does_for( + ticket, + &format!( + "trap 'touch {root}/interrupted-{ticket}; exit 143' TERM\n\ + echo 'half done {ticket}' > work.txt\n\ + touch {root}/started-{ticket}\n\ + while ! test -f {root}/started-{sibling}; do sleep 0.02; done\n\ + touch {root}/both-started\n\ + while true; do sleep 0.05; done\n", + root = scenario.path("").display() + ), + ); + } + let url = scenario.issue_url(20); + let args = if pickup { + vec!["pickup"] + } else { + vec![url.as_str()] + }; + let mut held = scenario.run_until(&args, &[], "both-started"); + let args = if requeue { + vec!["stop", "widgets#20", "--requeue"] + } else { + vec!["stop", "widgets#20"] + }; + let stopped = scenario.run(&args); + if stopped.code != Some(0) { + held.signal("TERM"); + } + let ended = held.finish(); + assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); + assert_eq!(ended.code, Some(1), "{}", ended.stderr); + assert!(stopped.stdout.contains("interrupted"), "{}", stopped.stdout); + assert_eq!( + scenario.issue_labels(20), + if requeue { + vec!["ready-for-agent"] + } else { + vec!["in-progress", "needs-triage"] + } + ); + for ticket in [21, 22] { + assert!(scenario.path(&format!("interrupted-{ticket}")).exists()); + assert_eq!(scenario.issue_labels(ticket), ["bug", "ready-for-agent"]); + assert_eq!( + scenario.origin_file(&format!("issue-{ticket}"), "work.txt"), + Some(format!("half done {ticket}\n")) + ); + let shown = scenario.run(&["show", &format!("widgets#{ticket}"), "--json"]); + assert_eq!(shown.code, Some(0), "{}", shown.stderr); + let shown: serde_json::Value = serde_json::from_str(&shown.stdout).unwrap(); + assert_eq!(shown["command"]["state"], "ended"); + assert_eq!(shown["command"]["outcome"], "interrupted"); + scenario.assert_cleaned_up(&format!("issue-{ticket}")); + } + let activity = + fs::read_to_string(scenario.path("home/.thirdshift/logs/acme/widgets/activity.log")) + .unwrap(); + assert_eq!( + activity.matches("stopped from the CLI by").count(), + 1, + "{activity}" + ); + assert!( + activity.find("stopped from").unwrap() < activity.find("ended:").unwrap(), + "{activity}" + ); + } +} + +#[test] +fn stopping_a_tickets_issue_is_refused_naming_its_spec_without_sending_a_signal() { + for pickup in [false, true] { + let scenario = Scenario::new(); + scenario.spec_has_tickets(20, &[(21, &[])]); + scenario.issue_labelled(20, &["ready-for-agent"]); + scenario.agent_does_for( + 21, + &format!( + "touch '{}'\nwhile ! test -f '{}'; do sleep 0.02; done\ntouch '{}'\nexit 3\n", + scenario.path("started").display(), + scenario.path("release").display(), + scenario.path("survived").display() + ), + ); + let url = scenario.issue_url(20); + let args = if pickup { + vec!["pickup"] + } else { + vec![url.as_str()] + }; + let held = scenario.run_until(&args, &[], "started"); + let stopped = scenario.run(&["stop", "widgets#21"]); + fs::write(scenario.path("release"), "").unwrap(); + held.finish(); + assert_ne!(stopped.code, Some(0), "{}", stopped.stdout); + assert!(stopped.stderr.contains("Spec run"), "{}", stopped.stderr); + assert!( + stopped.stderr.contains("acme/widgets#20"), + "{}", + stopped.stderr + ); + assert!(scenario.path("survived").exists(), "Ticket was signalled"); + let activity = + fs::read_to_string(scenario.path("home/.thirdshift/logs/acme/widgets/activity.log")) + .unwrap(); + assert!(!activity.contains("stopped from"), "{activity}"); + } +} + #[test] fn stopping_a_run_with_nothing_on_origin_leaves_it_for_triage_and_logs_who_before_ending() { let scenario = Scenario::new(); From 3a7eb37fa643c615d10dc4155cb510a05c8064e8 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Sun, 18 Oct 2026 16:50:05 -0400 Subject: [PATCH 2/3] Share Stop test process cleanup after Standards review --- tests/stop.rs | 43 ++++++++++++++++++------------------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/tests/stop.rs b/tests/stop.rs index 39aea72d3..3676aed5f 100644 --- a/tests/stop.rs +++ b/tests/stop.rs @@ -1,7 +1,18 @@ mod support; use std::fs; -use support::Scenario; +use support::{HeldRun, RunResult, Scenario}; + +fn stop_and_finish(scenario: &Scenario, mut held: HeldRun, args: &[&str]) -> RunResult { + let stopped = scenario.run(args); + if stopped.code != Some(0) { + held.signal("TERM"); + } + let ended = held.finish(); + assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); + assert_eq!(ended.code, Some(1), "{}", ended.stderr); + stopped +} #[test] fn stopping_a_pass_with_no_issue_ends_it_without_changing_labels() { @@ -14,7 +25,7 @@ fn stopping_a_pass_with_no_issue_ends_it_without_changing_labels() { "touch '{}'\nwhile true; do sleep 0.05; done\n", scenario.path("started").display() )); - let mut held = scenario.run_until(&[kind], &[], "started"); + let held = scenario.run_until(&[kind], &[], "started"); let status = scenario.run(&["status", "--json"]); let status: serde_json::Value = serde_json::from_str(&status.stdout).unwrap(); let command = &status["commands"][0]; @@ -24,13 +35,7 @@ fn stopping_a_pass_with_no_issue_ends_it_without_changing_labels() { } else { vec!["stop", name] }; - let stopped = scenario.run(&args); - if stopped.code != Some(0) { - held.signal("TERM"); - } - let ended = held.finish(); - assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); - assert_eq!(ended.code, Some(1), "{}", ended.stderr); + let stopped = stop_and_finish(&scenario, held, &args); assert!(command["issue"].is_null(), "{command}"); assert_eq!( command["stage"]["name"], @@ -65,19 +70,13 @@ fn stopping_a_pickups_run_triages_or_requeues_the_issue_it_took() { "echo work > work.txt\ngit add work.txt\ngit commit -qm Work\ngit push -q origin HEAD\ntouch '{}'\nwhile true; do sleep 0.05; done\n", scenario.path("started").display() )); - let mut held = scenario.run_until(&["pickup"], &[], "started"); + let held = scenario.run_until(&["pickup"], &[], "started"); let args = if requeue { vec!["stop", "widgets#7", "--requeue"] } else { vec!["stop", "widgets#7"] }; - let stopped = scenario.run(&args); - if stopped.code != Some(0) { - held.signal("TERM"); - } - let ended = held.finish(); - assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); - assert_eq!(ended.code, Some(1), "{}", ended.stderr); + let stopped = stop_and_finish(&scenario, held, &args); assert_eq!( scenario.issue_labels(7), if requeue { @@ -125,19 +124,13 @@ fn stopping_a_spec_ends_both_tickets_preserves_their_work_and_only_labels_the_sp } else { vec![url.as_str()] }; - let mut held = scenario.run_until(&args, &[], "both-started"); + let held = scenario.run_until(&args, &[], "both-started"); let args = if requeue { vec!["stop", "widgets#20", "--requeue"] } else { vec!["stop", "widgets#20"] }; - let stopped = scenario.run(&args); - if stopped.code != Some(0) { - held.signal("TERM"); - } - let ended = held.finish(); - assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); - assert_eq!(ended.code, Some(1), "{}", ended.stderr); + let stopped = stop_and_finish(&scenario, held, &args); assert!(stopped.stdout.contains("interrupted"), "{}", stopped.stdout); assert_eq!( scenario.issue_labels(20), From 31c763fdaced76852f7b5206d085f1212ff99e24 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Sun, 18 Oct 2026 17:44:24 -0400 Subject: [PATCH 3/3] Let fake Claude finish TERM handlers before exiting The Spec Stop test added on issue-774 intermittently lost its interruption markers because the fake CLI died before its script handled TERM, letting process cleanup kill the script. Reuse the stop-signal handler already used by the other session fakes so Claude waits for its script. Delay the test handler beyond a cleanup poll to make premature fake exit reproducible while retaining every assertion. Verified the delayed handler fails before the fix, then passes ten stress iterations; the full hash:4/4 shard (498 tests), all eight Stop tests, formatting and diff checks pass locally on Linux. --- tests/fakes/claude.rs | 4 ++++ tests/stop.rs | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/fakes/claude.rs b/tests/fakes/claude.rs index 90d63c633..fbcacd9ad 100644 --- a/tests/fakes/claude.rs +++ b/tests/fakes/claude.rs @@ -34,6 +34,9 @@ //! Called without --output-format, as `claude -p` with its prompt on stdin, //! it records stdin as `stdin` and prints only what the script prints, as //! print mode's plain text output does. +//! +//! Like the other session fakes, it catches stop signals and waits for its +//! script to finish handling them before exiting. use std::fs; use std::io::{Read, Write}; @@ -251,6 +254,7 @@ pub fn main(argv: Vec) { } let session = records.len(); + crate::outlast_interrupts(); let init = object([ ("type", string("system")), ("subtype", string("init")), diff --git a/tests/stop.rs b/tests/stop.rs index 3676aed5f..93b0f0fb4 100644 --- a/tests/stop.rs +++ b/tests/stop.rs @@ -103,12 +103,14 @@ fn stopping_a_spec_ends_both_tickets_preserves_their_work_and_only_labels_the_sp let scenario = Scenario::new(); scenario.spec_has_tickets(20, &[(21, &[]), (22, &[])]); scenario.issue_labelled(20, &["ready-for-agent"]); + // Let TERM handling outlast a cleanup poll, so a fake CLI exiting + // before its script has handled the signal reliably fails this test. for (ticket, sibling) in [(21, 22), (22, 21)] { scenario.issue_labelled(ticket, &["bug", "ready-for-agent"]); scenario.agent_does_for( ticket, &format!( - "trap 'touch {root}/interrupted-{ticket}; exit 143' TERM\n\ + "trap 'sleep 0.25; touch {root}/interrupted-{ticket}; exit 143' TERM\n\ echo 'half done {ticket}' > work.txt\n\ touch {root}/started-{ticket}\n\ while ! test -f {root}/started-{sibling}; do sleep 0.02; done\n\