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 @@ -41,6 +41,9 @@ _Avoid_: auto-merge (GitHub's own feature, which thirdshift does not use)
**Self-merge**:
The step at the end of a **Merge run** in which thirdshift itself merges the pull request into the **Base branch** with a merge commit, once it is open, ready for review, mergeable and green on the head commit it merges. No human reviews it first. It ends with the **Issue branch** deleted and the issue closed, by thirdshift if the merge did not close it.

**Policy refusal**:
A merge the **Self-merge** tried that failed, where the round of the Repair loop that followed found nothing to fix: the **Base branch** unchanged, no conflict, CI green or absent, the pull request mergeable. The cause is a repository setting or rule, such as merge commits disallowed or a review required. thirdshift never reads GitHub's error text to decide it. The **Merge run** is a **Failed run** that leaves the pull request ready for review.

**Failed run**:
A **Run** that ends, including by interruption, without an open pull request from its **Issue branch** that targets the **Base branch**, is mergeable, and has passing CI. For a **Merge run**, it is also a Run that ends without its pull request merged. Its work is still pushed so nothing is lost (or, if the push fails, its worktree and local **Issue branch** are kept), and its open pull request, if any, is converted back to a draft, unless the pull request is ready, mergeable and green and only the **Self-merge** could not happen.

Expand Down
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ From a clone of the issue's repository, `thirdshift <Issue URL>`:
4. Runs a headless Claude Code session in that worktree with the **Factory skills** loaded. The agent implements the issue, reviews its work against the Base branch, addresses the **Standards findings** and **Spec findings** it agrees with, and opens a ready-for-review pull request that lists every **Unaddressed finding** with a reason and closes the issue.
5. Takes over deterministically: pushes the Issue branch, skipping your repo's git hooks since the session runs the tests and CI gates the pull request, checks through `gh` that the pull request exists, is open and targets the Base branch, and marks it ready for review (`gh pr ready`) if the agent left it as a draft.
6. Keeps the pull request mergeable and green: merges the Base branch in and watches CI, starting a **Repair** session for a merge conflict or failing checks, at most 5 per Run. If the Base branch moves while CI runs, it merges it again and goes round, at most 5 times per Run.
7. In a **Merge run** (`thirdshift merge <Issue URL>`), does the **Self-merge**: once the pull request is open, ready for review, mergeable and green, thirdshift merges it into the Base branch with a merge commit, on exactly the head commit whose CI it watched (`gh pr merge --merge --match-head-commit <sha>`). It never uses GitHub's auto-merge ([ADR-0004](docs/adr/0004-self-merge-by-thirdshift-not-github-auto-merge.md)). A merge that fails makes the Run a Failed run.
7. In a **Merge run** (`thirdshift merge <Issue URL>`), does the **Self-merge**: once the pull request is open, ready for review, mergeable and green, thirdshift merges it into the Base branch with a merge commit, on exactly the head commit whose CI it watched (`gh pr merge --merge --match-head-commit <sha>`). It never uses GitHub's auto-merge ([ADR-0004](docs/adr/0004-self-merge-by-thirdshift-not-github-auto-merge.md)). A merge that fails goes back round step 6, within the same budgets, and is tried again on the new green head. If that round finds nothing to fix, the refusal is a **policy refusal**, such as merge commits being disallowed or a review being required.
8. Cleans up: removes the worktree, the local Issue branch and the temporary plugin directory, whether the Run succeeded or not. The one exception is a **Failed run** whose work could not be pushed: see below.

### Status
Expand Down Expand Up @@ -106,7 +106,13 @@ thirdshift merge https://github.com/acme/widgets/issues/7

`thirdshift --merge <Issue URL>` does the same. The agent gets the same prompts either way; the only difference is the Self-merge at the end. The keyword goes before the URL: anything after the URL, `--merge` included, is an argument error.

- **stdout** carries only the pull request's URL: on success, and on a Failed run that leaves a draft pull request. The exit code tells the two apart, so script it as `url=$(thirdshift "$issue") && echo "ready: $url"`.
A Merge run ends in one of three ways:

- **Merged**: exit `0`, the pull request's URL on stdout, and `PR <url> is merged` as stderr's last line.
- **Policy refusal**: GitHub refused the merge and a round of merging the Base branch and watching CI found nothing left to fix. Exit `1`, the pull request's URL on stdout, and GitHub's error on stderr. The pull request stays open and ready for review, not a draft, for you to merge by hand or to change the repository's settings. thirdshift never reads GitHub's error text to decide this.
- **Any other failure**: a [Failed run](#failed-runs), as for a Run without `merge`.

- **stdout** carries only the pull request's URL: on success, and on a Failed run that leaves an open pull request, a draft or, after a policy refusal, one ready for review. The exit code tells the two apart, so script it as `url=$(thirdshift "$issue") && echo "ready: $url"`.
- **stderr** carries everything else: errors, cleanup problems, and progress lines while sessions run. A successful Run's last line names the pull request too: `PR <url> is ready for review`, or `PR <url> is merged` after a Merge run.
- **Exit code** `0` means the Run ended with a pull request the factory stands behind, merged in a Merge run. `2` means the Issue URL is missing or isn't a GitHub Issue URL, or something follows it; the error and the help text go to stderr. Any other failure exits `1`.

Expand Down Expand Up @@ -154,7 +160,7 @@ A **Failed run** is one that ends, including by Ctrl-C or a closed terminal, wit
A Failed run:

1. Commits any uncommitted work as `thirdshift: failed run (<reason>)`, with a timestamp and the hostname, and pushes the Issue branch, so nothing is lost. If the branch has no changes against the Base branch, nothing is pushed.
2. Converts its open pull request, if any, back to a draft, so a pull request only claims to be ready when the factory stands behind it. The next successful Continuation marks it ready again.
2. Converts its open pull request, if any, back to a draft, so a pull request only claims to be ready when the factory stands behind it. The next successful Continuation marks it ready again. The exception is a Merge run's policy refusal: the pull request is ready, mergeable and green and only the Self-merge could not happen, so it stays ready for review, and no failure commit is pushed onto the head whose CI was watched.
3. Cleans up as usual, prints the reason to stderr and exits non-zero. If the push failed, the worktree and local Issue branch are kept instead, and stderr names the branch, its head commit and the worktree path, so you can recover the work or push it by hand.

Merges, never rebases or force-pushes: a branch worked on from several servers never loses history.
Expand Down
40 changes: 31 additions & 9 deletions src/failed_run.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! The Failed run path: what happens when a Run can't end with a ready PR.

use std::fmt;
use std::path::{Path, PathBuf};

use anyhow::{Result, anyhow};
Expand All @@ -20,6 +21,20 @@ pub struct FailedRun {
pub log: Option<PathBuf>,
}

/// A merge GitHub refused when a round of the Repair loop found nothing to
/// fix. The Run fails, but, as it can do no more, leaves the PR ready for review
/// rather than a draft. It is the context of the merge error.
#[derive(Debug)]
pub struct PolicyRefusal;

impl fmt::Display for PolicyRefusal {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(
"the merge was refused with nothing left to fix, so the PR stays ready for review",
)
}
}

/// A failure before the worktree exists: nothing to push or clean up.
impl From<anyhow::Error> for FailedRun {
fn from(error: anyhow::Error) -> Self {
Expand All @@ -32,9 +47,10 @@ impl From<anyhow::Error> for FailedRun {
}

/// Take the Run in `worktree` down the Failed run path: commit and push the
/// work, and send an open PR back to draft. Problems along the way are
/// reported, not raised, so `error` is what the Run fails with. The worktree
/// is cleaned up, or kept if its work may not have reached origin.
/// work, and send an open PR back to draft. A `PolicyRefusal` does neither,
/// so the PR stays ready on the head whose CI was watched. Problems along the
/// way are reported, not raised, so `error` is what the Run fails with. The
/// worktree is cleaned up, or kept if its work may not have reached origin.
pub fn fail(
issue: &IssueUrl,
worktree: Worktree,
Expand All @@ -55,13 +71,19 @@ pub fn fail(
.next()
.unwrap_or_default()
.to_string();
let pushed = commit_and_push(&worktree, base, &reason);
// Everything is already on origin: the Repair loop pushed the head.
let keep_ready = error.is::<PolicyRefusal>();
let pushed = if keep_ready {
Ok(())
} else {
commit_and_push(&worktree, base, &reason)
};
if let Err(problem) = &pushed {
progress::step(format_args!(
"could not push the failed run's work, so it may exist only locally: {problem:#}"
));
}
let pr_url = match open_pr_as_draft(issue, worktree.branch()) {
let pr_url = match open_pr_url(issue, worktree.branch(), keep_ready) {
Ok(pr_url) => pr_url,
Err(problem) => {
progress::step(format_args!(
Expand Down Expand Up @@ -112,13 +134,13 @@ fn commit_and_push(worktree: &Worktree, base: &str, reason: &str) -> Result<()>
worktree.push()
}

/// Convert the open PR for `branch`, if there is one, to a draft, and return
/// its URL.
fn open_pr_as_draft(issue: &IssueUrl, branch: &str) -> Result<Option<String>> {
/// The URL of the open PR for `branch`, if there is one, converted to a draft
/// unless `keep_ready`.
fn open_pr_url(issue: &IssueUrl, branch: &str, keep_ready: bool) -> Result<Option<String>> {
let Some(pr) = github::pull_request_for(issue, branch)?.filter(|pr| pr.is_open()) else {
return Ok(None);
};
if !pr.is_draft {
if !pr.is_draft && !keep_ready {
github::convert_to_draft(issue, branch)?;
}
Ok(Some(pr.url))
Expand Down
Loading