Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -120,6 +122,8 @@ Each session's full transcript, as Claude Code's `stream-json` output, is writte
~/.thirdshift/logs/<owner>-<repo>-issue-<n>-<timestamp>-repair-<i>.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
Expand All @@ -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:

Expand Down
167 changes: 166 additions & 1 deletion src/progress.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<String>,
session_id: Option<String>,
/// Each background task's description, by task id.
tasks: HashMap<String, String>,
/// 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)>,
Expand All @@ -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()]
}
Expand All @@ -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(),
Expand All @@ -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("?")),
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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();
Expand Down
34 changes: 30 additions & 4 deletions src/prompt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
}
Expand Down Expand Up @@ -88,7 +99,22 @@ pub fn ci_fix_repair(
Read the failure logs (e.g. `gh run view <run-id> --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("; "),
)
}
14 changes: 7 additions & 7 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -66,13 +66,13 @@ fn implement(
) -> Result<String> {
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()?;
Expand Down
Loading