diff --git a/CONTEXT.md b/CONTEXT.md index c7c00c768..3d5a54e48 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -45,3 +45,6 @@ A **Run** that picks up an existing **Issue branch**, one with no pull request o **Repair**: A follow-up agent session a **Run** starts after the pull request exists, either to resolve a merge conflict with the **Base branch** or to fix failing CI checks. + +**Resume**: +A continuation of the same agent session, started once when that session ended its turn while waiting on background work, which was killed with it. It asks the agent to re-run that work in the foreground and finish. A Resume is not a **Repair** and does not count against the Repair cap. diff --git a/README.md b/README.md index d7f5163cd..8103d3504 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,8 @@ The user account that runs thirdshift needs: Sessions run headless in Claude Code's auto mode (`claude -p --permission-mode auto`), with the full permissions of that user account, including `sudo` if the user has it. Nobody is there to approve anything; instead, auto mode's classifier checks each action and may block ones it judges risky, such as destructive commands or actions outside the task. A blocked action the agent can't work around can end the Run as a Failed run. +`claude -p` exits as soon as the agent ends its turn, killing any background task it started. So every prompt tells the agent to run long commands, such as tests, in the foreground. If a session still ends while waiting on background work, thirdshift gives it a **Resume**: it continues that same session once (`claude -p --resume`), asking the agent to re-run the work in the foreground and finish. If the Resume ends the same way, the Run fails and says which task was killed. + ### What sessions leave behind Cleanup removes only thirdshift's own worktree, local Issue branch and temporary plugin directory. Anything else an agent does as your user stays. For example, one Run downloaded a JDK to `~/.local/jdk/` because the server had no Java, installed Playwright in `/tmp/pw`, and left a pull request body draft in `/tmp/`. This is by design, but worth knowing: @@ -120,6 +122,8 @@ Each session's full transcript, as Claude Code's `stream-json` output, is writte ~/.thirdshift/logs/--issue---repair-.jsonl ``` +A Resume is logged as its session's kind plus `-resume`, e.g. `implement-resume.jsonl`. + All sessions in a Run share the Run's UTC timestamp, so a Run's logs sort together. When a Run fails, stderr ends with the path of its most recent session log, the place to start looking. ## Continuation @@ -136,7 +140,7 @@ A Run is not idempotent: re-running builds on whatever is already on the branch, ## Failed runs -A **Failed run** is one that ends, including by Ctrl-C or a closed terminal, without an open pull request from its Issue branch that targets the Base branch, is mergeable and has passing CI. Causes include the session exiting non-zero, no pull request or one with the wrong base, running out of Repairs, and a Base branch that keeps moving while CI runs. +A **Failed run** is one that ends, including by Ctrl-C or a closed terminal, without an open pull request from its Issue branch that targets the Base branch, is mergeable and has passing CI. Causes include the session exiting non-zero, no pull request or one with the wrong base, running out of Repairs, a session that still ends while waiting on background work after its Resume, and a Base branch that keeps moving while CI runs. A Failed run: diff --git a/src/progress.rs b/src/progress.rs index 47a996742..3d87106fb 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -1,6 +1,7 @@ //! Progress lines on stderr: thirdshift's own steps, and a session's //! stream-json output condensed to one short line per notable event. +use std::collections::HashMap; use std::fmt::Display; use serde_json::Value; @@ -21,10 +22,22 @@ pub fn step(message: impl Display) { /// each resumption with another `result`. So only the first `init` gives a /// line, `result` events give none, and [`Progress::summary`] reports the last /// one once the process has exited. +/// +/// A background task still running when the session ends its turn for good is +/// killed as the process exits, so a task killed after the last `result` is +/// work the session was waiting on: see [`Progress::killed_background_work`]. #[derive(Default)] pub struct Progress { started: bool, cwd: Option, + session_id: Option, + /// Each background task's description, by task id. + tasks: HashMap, + /// Descriptions of the tasks killed since the last `result`, by task id, + /// in the order they were killed. + killed: Vec<(String, String)>, + /// The last `result` was an error, such as running out of turns. + failed: bool, /// Signed in with a claude.ai subscription rather than an API key. subscription: bool, turns_and_cost: Option<(u64, f64)>, @@ -41,6 +54,7 @@ impl Progress { Some("system") if event["subtype"] == "init" && !self.started => { self.started = true; self.cwd = event["cwd"].as_str().map(String::from); + self.session_id = event["session_id"].as_str().map(String::from); self.subscription = event["apiKeySource"] == "none"; vec!["session started".to_string()] } @@ -51,7 +65,13 @@ impl Progress { .filter(|block| block["type"] == "tool_use") .filter_map(|block| Some(self.tool_use(block["name"].as_str()?, &block["input"]))) .collect(), + Some("system") => { + self.track_task(&event); + Vec::new() + } Some("result") => { + self.killed.clear(); + self.failed = event["subtype"] != "success" || event["is_error"] == true; // Both are cumulative across a session's result events. if let (Some(turns), Some(cost)) = ( event["num_turns"].as_u64(), @@ -78,6 +98,52 @@ impl Progress { Some(format!("{turns} turns, ${cost:.2}{basis}")) } + /// The session's id, from its first `init` event. + pub fn session_id(&self) -> Option<&str> { + self.session_id.as_deref() + } + + /// Descriptions of the background tasks killed after the last `result`: + /// the work the session was still waiting on when it ended. None if that + /// `result` was an error: the session ended for another reason. + pub fn killed_background_work(&self) -> Vec<&str> { + if self.failed { + return Vec::new(); + } + self.killed + .iter() + .map(|(_, description)| description.as_str()) + .collect() + } + + /// Track a background task's description and whether it was killed. + fn track_task(&mut self, event: &Value) { + let Some(id) = event["task_id"].as_str() else { + return; + }; + let killed = match event["subtype"].as_str() { + Some("task_started") => { + if let Some(description) = event["description"].as_str() { + self.tasks.insert(id.to_string(), description.to_string()); + } + false + } + Some("task_updated") => event["patch"]["status"] == "killed", + Some("task_notification") => event["status"] == "stopped", + _ => false, + }; + if killed && !self.killed.iter().any(|(killed_id, _)| killed_id == id) { + let description = self + .tasks + .get(id) + .map(String::as_str) + .or(event["summary"].as_str()) + .unwrap_or(id) + .to_string(); + self.killed.push((id.to_string(), description)); + } + } + fn tool_use(&self, name: &str, input: &Value) -> String { match name { "Skill" => format!("skill {}", input["skill"].as_str().unwrap_or("?")), @@ -167,7 +233,7 @@ mod tests { } fn result(turns: u64, cost: f64) -> Value { - json!({ "type": "result", "num_turns": turns, "total_cost_usd": cost }) + json!({ "type": "result", "subtype": "success", "num_turns": turns, "total_cost_usd": cost }) } #[test] @@ -316,6 +382,105 @@ mod tests { assert_eq!(Progress::default().summary(), None); } + fn task_started(id: &str, description: &str) -> Value { + json!({ "type": "system", "subtype": "task_started", "task_id": id, "description": description }) + } + + fn task_updated(id: &str, status: &str) -> Value { + json!({ "type": "system", "subtype": "task_updated", "task_id": id, "patch": { "status": status } }) + } + + fn task_notification(id: &str, status: &str, summary: &str) -> Value { + json!({ "type": "system", "subtype": "task_notification", "task_id": id, "status": status, "summary": summary }) + } + + #[test] + fn a_task_killed_after_the_last_result_is_killed_background_work() { + let (progress, _) = lines(&[ + json!({ "type": "system", "subtype": "init", "session_id": "s-1" }), + task_started("b1", "./mvnw test -Dtest='GamesPageTest'"), + result(12, 0.4), + task_updated("b1", "killed"), + ]); + assert_eq!(progress.session_id(), Some("s-1")); + assert_eq!( + progress.killed_background_work(), + ["./mvnw test -Dtest='GamesPageTest'"] + ); + } + + #[test] + fn a_stopped_task_notification_is_killed_background_work_once() { + let (progress, _) = lines(&[ + task_started("b1", "cargo test"), + result(12, 0.4), + task_updated("b1", "killed"), + task_notification("b1", "stopped", "cargo test"), + task_notification("b2", "stopped", "npm run build"), + ]); + assert_eq!( + progress.killed_background_work(), + ["cargo test", "npm run build"] + ); + } + + #[test] + fn no_killed_tasks_means_no_killed_background_work() { + let (progress, _) = lines(&[ + task_started("b1", "cargo test"), + task_notification("b1", "completed", "cargo test"), + result(12, 0.4), + ]); + assert!(progress.killed_background_work().is_empty()); + } + + #[test] + fn background_sub_agents_that_resumed_the_session_are_not_killed_background_work() { + let (progress, _) = lines(&[ + task_started("a1", "Standards review"), + result(12, 0.4), + task_updated("a1", "completed"), + task_notification("a1", "completed", "Standards review"), + init("/work/widgets-issue-7"), + result(20, 0.9), + ]); + assert!(progress.killed_background_work().is_empty()); + } + + #[test] + fn a_task_killed_before_a_later_result_is_not_killed_background_work() { + let (progress, _) = lines(&[ + task_started("b1", "cargo watch"), + task_updated("b1", "killed"), + result(12, 0.4), + init("/work/widgets-issue-7"), + task_notification("b1", "stopped", "cargo watch"), + result(20, 0.9), + ]); + assert!(progress.killed_background_work().is_empty()); + } + + #[test] + fn tasks_killed_after_an_error_result_are_not_killed_background_work() { + for last in [ + json!({ "type": "result", "subtype": "error_max_turns", "is_error": true }), + json!({ "type": "result", "subtype": "success", "is_error": true }), + ] { + let (progress, _) = lines(&[ + task_started("b1", "cargo test"), + last, + task_updated("b1", "killed"), + ]); + assert!(progress.killed_background_work().is_empty()); + } + } + + #[test] + fn an_init_without_a_session_id_gives_none() { + let (progress, _) = lines(&[init("/a")]); + assert_eq!(progress.session_id(), None); + } + #[test] fn unknown_and_malformed_lines_are_skipped() { let mut progress = Progress::default(); diff --git a/src/prompt.rs b/src/prompt.rs index a34e3d663..d357260cc 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -3,6 +3,12 @@ use crate::github::Check; use crate::issue::IssueUrl; +/// Every prompt ends with this. Sessions run with `claude -p`, which exits +/// once the agent ends its turn, killing any background task still running. +const HEADLESS: &str = "You run headless: nobody is watching, and ending your turn ends the session. \ + Run tests and other long commands in the foreground, raising the Bash timeout if needed. \ + Never end your turn while a background task you depend on is still running: ending the turn kills it.\n"; + /// The fresh prompt, for a run that starts a new Issue branch. pub fn fresh(issue: &IssueUrl, base: &str, branch: &str) -> String { format!( @@ -11,7 +17,8 @@ pub fn fresh(issue: &IssueUrl, base: &str, branch: &str) -> String { Address the Standards and Spec findings you agree with.\n\ Push branch {branch} and create a pull request against {base} using /thirdshift:pr, marked ready for review.\n\ In the PR body, add an \"Unaddressed findings\" section listing each skipped finding under Standards or Spec, with at least a one-line reason.\n\ - Include \"Closes #{number}\" in the PR body.\n", + Include \"Closes #{number}\" in the PR body.\n\ + {HEADLESS}", url = issue.url, number = issue.number, ) @@ -43,7 +50,9 @@ pub fn continuation(issue: &IssueUrl, base: &str, branch: &str, pr_url: Option<& \n\ In the PR body, add an \"Unaddressed findings\" section listing each skipped finding under Standards or Spec, with at least a one-line reason.\n\ \n\ - Include \"Closes #{number}\" in the PR body.\n", + Include \"Closes #{number}\" in the PR body.\n\ + \n\ + {HEADLESS}", url = issue.url, number = issue.number, ) @@ -58,7 +67,9 @@ pub fn conflict_repair(issue: &IssueUrl, base: &str, branch: &str, pr_url: &str) A merge of origin/{base} into {branch} is in progress in this worktree and has conflicts.\n\ {branch} implements {url}; its pull request is {pr_url}.\n\ \n\ - Resolve the conflicts, finish the merge, and push {branch}. Do not rebase or force-push.\n", + Resolve the conflicts, finish the merge, and push {branch}. Do not rebase or force-push.\n\ + \n\ + {HEADLESS}", url = issue.url, ) } @@ -88,7 +99,22 @@ pub fn ci_fix_repair( Read the failure logs (e.g. `gh run view --log-failed`), find the root cause, and fix it. Do not skip, disable, or weaken tests or checks to make them pass.\n\ Run the affected checks locally, commit, and push {branch}.\n\ \n\ - If a failure is not caused by this branch (it is flaky, or also fails on {base}), do not change code for it. Instead, add it to a \"CI notes\" section of the pull request body with a one-line explanation.\n", + If a failure is not caused by this branch (it is flaky, or also fails on {base}), do not change code for it. Instead, add it to a \"CI notes\" section of the pull request body with a one-line explanation.\n\ + \n\ + {HEADLESS}", url = issue.url, ) } + +/// The resume prompt, for a session whose `killed` background work, by +/// description, was killed when it ended its turn. +pub fn resume(killed: &[&str]) -> String { + format!( + "Your background work ({killed}) was killed when your turn ended, because ending the turn ends the session.\n\ + \n\ + Re-run whatever you were waiting on in the foreground, then finish your job.\n\ + \n\ + {HEADLESS}", + killed = killed.join("; "), + ) +} diff --git a/src/run.rs b/src/run.rs index 157ac3064..0626e284d 100644 --- a/src/run.rs +++ b/src/run.rs @@ -16,7 +16,7 @@ use crate::poll; use crate::preflight; use crate::progress; use crate::prompt; -use crate::session; +use crate::session::{self, Sessions}; use crate::worktree::{Merge, Worktree}; /// Take `issue` to a ready PR and return the PR's URL. Any failure after the @@ -66,13 +66,13 @@ fn implement( ) -> Result { let branch = worktree.branch(); let plugin = Plugin::write()?; - // Every session runs in the worktree with the plugin loaded, logged as - // `kind` under the Run's timestamp. - let mut run_session = |kind: &str, prompt: &str| -> Result<()> { - *log = session::log_path(issue, timestamp, kind)?; - progress::step(format_args!("logging the session to {}", log.display())); - session::run(kind, worktree.path(), plugin.path(), prompt, log) + let sessions = Sessions { + issue, + timestamp, + worktree: worktree.path(), + plugin_dir: plugin.path(), }; + let mut run_session = |kind: &str, prompt: &str| sessions.run(kind, prompt, log); run_session("implement", prompt)?; worktree.push()?; diff --git a/src/session.rs b/src/session.rs index bc3d79839..130ca45ce 100644 --- a/src/session.rs +++ b/src/session.rs @@ -13,6 +13,7 @@ use anyhow::{Context, Result, anyhow, bail}; use crate::interrupt; use crate::issue::IssueUrl; use crate::progress::{self, Progress}; +use crate::prompt; /// Where a session's stream is logged: /// `~/.thirdshift/logs/--issue---.jsonl`. @@ -25,21 +26,92 @@ pub fn log_path(issue: &IssueUrl, timestamp: &str, kind: &str) -> Result { + pub issue: &'a IssueUrl, + pub timestamp: &'a str, + pub worktree: &'a Path, + pub plugin_dir: &'a Path, +} + +impl Sessions<'_> { + /// Run a session given `prompt`, logged and labelled as `kind`, pointing + /// `log` at each session's log as it starts. A session that ends while + /// waiting on background work, which was killed with it, gets one Resume, + /// as `-resume`; if that ends the same way, this fails and names the + /// killed work. + pub fn run(&self, kind: &str, prompt: &str, log: &mut PathBuf) -> Result<()> { + let mut ended = self.start(kind, None, prompt, log)?; + let killed = ended.killed_background_work(); + if let (false, Some(session_id)) = (killed.is_empty(), ended.session_id()) { + let (session_id, resume_prompt) = (session_id.to_string(), prompt::resume(&killed)); + progress::step(format_args!( + "{kind}: background work was killed as the session ended; resuming it once" + )); + ended = self.start( + &format!("{kind}-resume"), + Some(&session_id), + &resume_prompt, + log, + )?; + } + let killed = ended.killed_background_work(); + match killed[..] { + [] => Ok(()), + [task] => bail!( + "{kind} session ended while waiting on a background task ({task}), which was killed" + ), + _ => bail!( + "{kind} session ended while waiting on background tasks ({}), which were killed", + killed.join("; ") + ), + } + } + + /// Run one session as `kind`, logged under its own path. + fn start( + &self, + kind: &str, + resume: Option<&str>, + prompt: &str, + log: &mut PathBuf, + ) -> Result { + *log = log_path(self.issue, self.timestamp, kind)?; + progress::step(format_args!("logging the session to {}", log.display())); + run(kind, self.worktree, self.plugin_dir, resume, prompt, log) + } +} + /// Run `claude` headless in auto mode in `worktree`, with the Factory skills /// plugin at `plugin_dir` loaded, streaming its output to `log` and condensing -/// it to progress lines on stderr, each labelled `kind`. An interrupt stops the -/// session and fails with `interrupted`. -pub fn run(kind: &str, worktree: &Path, plugin_dir: &Path, prompt: &str, log: &Path) -> Result<()> { +/// it to progress lines on stderr, each labelled `kind`. With `resume`, the +/// session with that id continues, given `prompt`. Returns what the stream +/// showed once `claude` has exited cleanly. An interrupt stops the session and +/// fails with `interrupted`. +fn run( + kind: &str, + worktree: &Path, + plugin_dir: &Path, + resume: Option<&str>, + prompt: &str, + log: &Path, +) -> Result { if let Some(dir) = log.parent() { fs::create_dir_all(dir).with_context(|| format!("could not create {}", dir.display()))?; } let mut log_file = File::create(log).with_context(|| format!("could not create {}", log.display()))?; let started = Instant::now(); - let mut child = Command::new("claude") + let mut command = Command::new("claude"); + command .args(["-p", "--permission-mode", "auto", "--plugin-dir"]) .arg(plugin_dir) - .args(["--output-format", "stream-json", "--verbose"]) + .args(["--output-format", "stream-json", "--verbose"]); + if let Some(session_id) = resume { + command.args(["--resume", session_id]); + } + let mut child = command .arg(prompt) .current_dir(worktree) .stdin(Stdio::null()) @@ -101,7 +173,7 @@ pub fn run(kind: &str, worktree: &Path, plugin_dir: &Path, prompt: &str, log: &P .map_or("by signal".to_string(), |code| code.to_string()) ); } - Ok(()) + Ok(progress) } const POLL: Duration = Duration::from_millis(100); diff --git a/tests/background_work.rs b/tests/background_work.rs new file mode 100644 index 000000000..6009dc0ca --- /dev/null +++ b/tests/background_work.rs @@ -0,0 +1,145 @@ +//! Killed background work: a session that ends its turn while a background +//! task it started is still running is killed with that task. thirdshift +//! resumes such a session once, and fails with the reason if it happens again. + +mod support; + +use support::Scenario; + +/// The agent starts a background test run, then ends its turn without waiting +/// for it, so the task is killed after the session's last `result`. +const AGENT_LEAVES_TESTS_RUNNING: &str = r#" +echo '{"type": "system", "subtype": "task_started", "task_id": "b1", "description": "./mvnw test -Dtest=GamesPageTest"}' +echo '{"type": "system", "subtype": "task_updated", "task_id": "b1", "patch": {"status": "killed"}}' >> "$FAKE_CLAUDE_AFTER_RESULT" +"#; + +const AGENT_COMMITS_AND_OPENS_PR: &str = r#" +echo "feature" > feature.txt +git add feature.txt +git commit -q -m "Add feature" +gh pr create --base main --head issue-7 --title "Add feature" --body "Closes #7" +"#; + +#[test] +fn a_session_whose_background_work_was_killed_is_resumed_once_and_the_run_goes_on() { + let scenario = Scenario::new(); + scenario.agent_does_in_session( + 1, + &format!("echo wip > feature.txt\n{AGENT_LEAVES_TESTS_RUNNING}"), + ); + scenario.agent_does(AGENT_COMMITS_AND_OPENS_PR); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, "https://github.com/acme/widgets/pull/1\n"); + let calls = scenario.claude_calls(); + assert_eq!(calls.len(), 2); + let argv: Vec<&str> = calls[1]["argv"] + .as_array() + .unwrap() + .iter() + .map(|arg| arg.as_str().unwrap()) + .collect(); + assert!( + argv.windows(2) + .any(|pair| pair == ["--resume", "fake-session-1"]), + "argv: {argv:?}" + ); + assert!( + argv.windows(2) + .any(|pair| pair == ["--permission-mode", "auto"]), + "argv: {argv:?}" + ); + assert_eq!(calls[1]["cwd"], calls[0]["cwd"]); + let prompt = calls[1]["prompt"].as_str().unwrap(); + assert!( + prompt.contains("was killed when your turn ended") && prompt.contains("in the foreground"), + "prompt: {prompt}" + ); + for expected in [ + "thirdshift: implement: session ended", + "thirdshift: implement-resume: session started", + "thirdshift: implement-resume: session ended", + ] { + assert!( + result.stderr.lines().any(|line| line.starts_with(expected)), + "missing {expected:?} in stderr: {}", + result.stderr + ); + } + let logs = scenario.entries("home/.thirdshift/logs"); + assert_eq!(logs.len(), 2, "logs: {logs:?}"); + assert!(logs.iter().any(|log| log.ends_with("-implement.jsonl"))); + assert!( + logs.iter() + .any(|log| log.ends_with("-implement-resume.jsonl")) + ); + scenario.assert_cleaned_up("issue-7"); +} + +#[test] +fn a_resumed_session_whose_background_work_is_killed_again_is_a_failed_run_that_says_so() { + let scenario = Scenario::new(); + scenario.agent_does(&format!( + "echo wip >> feature.txt\n{AGENT_LEAVES_TESTS_RUNNING}" + )); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(1), "stderr: {}", result.stderr); + assert_eq!(scenario.claude_calls().len(), 2); + let reason = "implement session ended while waiting on a background task \ + (./mvnw test -Dtest=GamesPageTest), which was killed"; + assert!(result.stderr.contains(reason), "stderr: {}", result.stderr); + assert!( + !result.stderr.contains("no PR found"), + "stderr: {}", + result.stderr + ); + // The Failed run path still pushes the uncommitted work. + assert_eq!( + scenario.origin_log("issue-7").unwrap()[0], + format!("thirdshift: failed run ({reason})") + ); + assert_eq!( + scenario.origin_file("issue-7", "feature.txt"), + Some("wip\nwip\n".to_string()) + ); + assert!( + result.stderr.contains("-implement-resume.jsonl"), + "stderr: {}", + result.stderr + ); + scenario.assert_cleaned_up("issue-7"); +} + +#[test] +fn a_repair_whose_background_work_was_killed_is_resumed_too() { + let scenario = Scenario::new(); + scenario.agent_does_in_session( + 1, + &format!( + "{AGENT_COMMITS_AND_OPENS_PR}gh fake checks \"$(git rev-parse HEAD)\" \ + '[{{\"name\": \"test\", \"conclusion\": \"failure\"}}]'\n" + ), + ); + scenario.agent_does_in_session(2, AGENT_LEAVES_TESTS_RUNNING); + scenario.agent_does_in_session( + 3, + "echo fix > fix.txt\ngit add fix.txt\ngit commit -q -m Fix\n\ + gh fake checks \"$(git rev-parse HEAD)\" '[{\"name\": \"test\", \"conclusion\": \"success\"}]'\n", + ); + + let result = scenario.run(&[&scenario.issue_url(7)]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(scenario.claude_calls().len(), 3); + assert!( + result + .stderr + .contains("thirdshift: repair-1-resume: session started"), + "stderr: {}", + result.stderr + ); +} diff --git a/tests/ci.rs b/tests/ci.rs index fb1c7d20e..430f852c4 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -104,7 +104,9 @@ fn red_ci_is_handed_to_a_ci_fix_repair_whose_fix_turns_it_green() { Read the failure logs (e.g. `gh run view --log-failed`), find the root cause, and fix it. Do not skip, disable, or weaken tests or checks to make them pass.\n\ Run the affected checks locally, commit, and push issue-7.\n\ \n\ - If a failure is not caused by this branch (it is flaky, or also fails on main), do not change code for it. Instead, add it to a \"CI notes\" section of the pull request body with a one-line explanation.\n" + If a failure is not caused by this branch (it is flaky, or also fails on main), do not change code for it. Instead, add it to a \"CI notes\" section of the pull request body with a one-line explanation.\n\ + \n\ + You run headless: nobody is watching, and ending your turn ends the session. Run tests and other long commands in the foreground, raising the Bash timeout if needed. Never end your turn while a background task you depend on is still running: ending the turn kills it.\n" ); assert_eq!(calls[1]["cwd"], calls[0]["cwd"]); // thirdshift pushes the fix even though the Repair didn't. diff --git a/tests/continuation.rs b/tests/continuation.rs index 9c1e14f90..44a6ad294 100644 --- a/tests/continuation.rs +++ b/tests/continuation.rs @@ -66,7 +66,9 @@ fn without_a_pr_the_continuation_prompt_asks_for_one() { \n\ In the PR body, add an \"Unaddressed findings\" section listing each skipped finding under Standards or Spec, with at least a one-line reason.\n\ \n\ - Include \"Closes #7\" in the PR body.\n" + Include \"Closes #7\" in the PR body.\n\ + \n\ + You run headless: nobody is watching, and ending your turn ends the session. Run tests and other long commands in the foreground, raising the Bash timeout if needed. Never end your turn while a background task you depend on is still running: ending the turn kills it.\n" ); } diff --git a/tests/fakes/claude b/tests/fakes/claude index d0645a451..e0d7dd18c 100644 --- a/tests/fakes/claude +++ b/tests/fakes/claude @@ -10,6 +10,11 @@ It then runs a per-test bash script in the working directory, emits a couple of stream-json lines, and exits with the script's exit code. The script for the n-th call (1-based) is $FAKE_CLAUDE_SCRIPT with `.` appended if that file exists, else $FAKE_CLAUDE_SCRIPT itself. + +The `init` line carries session id `fake-session-` for the n-th call. Lines +the script appends to the file named by $FAKE_CLAUDE_AFTER_RESULT are emitted +after the closing `result` line, e.g. to kill a background task as the session +ends. """ import json @@ -60,12 +65,24 @@ def main(): with open(record_path, "w") as f: json.dump(records, f, indent=2) - print(json.dumps({"type": "system", "subtype": "init", "cwd": os.getcwd()}), flush=True) + init = { + "type": "system", + "subtype": "init", + "cwd": os.getcwd(), + "session_id": f"fake-session-{len(records)}", + } + print(json.dumps(init), flush=True) script = os.environ["FAKE_CLAUDE_SCRIPT"] if os.path.exists(f"{script}.{len(records)}"): script = f"{script}.{len(records)}" - code = subprocess.run(["bash", "-e", script]).returncode + # Beside the record, not in $TMPDIR, which tests expect to be left empty. + after_result = f"{record_path}.after-result.{len(records)}" + env = dict(os.environ, FAKE_CLAUDE_AFTER_RESULT=after_result) + code = subprocess.run(["bash", "-e", script], env=env).returncode print(json.dumps({"type": "result", "subtype": "success", "is_error": code != 0}), flush=True) + if os.path.exists(after_result): + with open(after_result) as f: + print(f.read(), end="", flush=True) sys.exit(code) diff --git a/tests/fresh_run.rs b/tests/fresh_run.rs index 96c5bd274..f0481d546 100644 --- a/tests/fresh_run.rs +++ b/tests/fresh_run.rs @@ -75,7 +75,8 @@ fn gives_the_agent_the_fresh_prompt() { Address the Standards and Spec findings you agree with.\n\ Push branch issue-7 and create a pull request against main using /thirdshift:pr, marked ready for review.\n\ In the PR body, add an \"Unaddressed findings\" section listing each skipped finding under Standards or Spec, with at least a one-line reason.\n\ - Include \"Closes #7\" in the PR body.\n" + Include \"Closes #7\" in the PR body.\n\ + You run headless: nobody is watching, and ending your turn ends the session. Run tests and other long commands in the foreground, raising the Bash timeout if needed. Never end your turn while a background task you depend on is still running: ending the turn kills it.\n" ); } diff --git a/tests/mergeable.rs b/tests/mergeable.rs index 4c30d770e..4769eb891 100644 --- a/tests/mergeable.rs +++ b/tests/mergeable.rs @@ -112,7 +112,9 @@ fn hands_a_conflicting_merge_to_a_conflict_repair_and_pushes_its_resolution() { A merge of origin/main into issue-7 is in progress in this worktree and has conflicts.\n\ issue-7 implements https://github.com/acme/widgets/issues/7; its pull request is https://github.com/acme/widgets/pull/1.\n\ \n\ - Resolve the conflicts, finish the merge, and push issue-7. Do not rebase or force-push.\n" + Resolve the conflicts, finish the merge, and push issue-7. Do not rebase or force-push.\n\ + \n\ + You run headless: nobody is watching, and ending your turn ends the session. Run tests and other long commands in the foreground, raising the Bash timeout if needed. Never end your turn while a background task you depend on is still running: ending the turn kills it.\n" ); assert_eq!(repair["merging"], true); assert_eq!(repair["branch"], "issue-7"); diff --git a/tests/support/mod.rs b/tests/support/mod.rs index adfa89bcd..55fb1d06b 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -16,6 +16,8 @@ //! claude-script.sh what the fake agent does this test //! claude-script.sh. what it does in the n-th session instead, if present //! claude-calls.json what the fake agent was asked to do +//! claude-calls.json.after-result. stream lines the n-th session emits +//! after its closing result //! gh-calls.json every gh command run, by thirdshift or the fake agent //! ```