From 5785577ee58dd9fc5ea8df72fb03c027c70416f6 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Sat, 17 Oct 2026 09:08:00 +0000 Subject: [PATCH] Give actionable recovery guidance for missing Base branches (#295) --- README.md | 2 +- src/launch.rs | 159 +++++++++++++++++++++++++++++++++++++++-- src/run.rs | 8 ++- tests/architect_run.rs | 28 ++++++-- tests/pickup_run.rs | 3 +- tests/preflight.rs | 10 ++- 6 files changed, 196 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 21003ecaf..0ebcc7ee6 100644 --- a/README.md +++ b/README.md @@ -653,7 +653,7 @@ A second focus, a repeated flag, `merge` with `no-merge`, `base-fix` with `no-ba An Architect run: -1. Makes the checks a Run makes that don't need an issue, before creating anything: `origin` is a GitHub repository, git has a `user.name` and `user.email`, HEAD is not detached, and the Base branch exists on `origin` with your local copy not ahead of it. With `base `, a detached HEAD is fine, since the checkout no longer picks the Base branch, and the other two checks are made on ``: `base branch does not exist on origin; push it first`, or `local is commit(s) ahead of origin/; push them first`. There is no **Origin match**, since there is no Issue URL: the repository is the one `origin` names. If another Pass on the repository is still running, it is [skipped](#one-at-a-time) here, and if not, it is skipped when an earlier [Architect plan is still open](#one-architect-plan-at-a-time), and if not, when an [Architect idea waits for triage](#an-architect-idea-waits-for-triage), and if not, when the repository has a [Ready issue](#a-ready-issue-goes-first). With `launch.pull = true` in the [User config](#user-config), it then fast-forwards your checkout of the Base branch, as a Run does, and so only when the Base branch is the branch checked out: with `base ` from a clone on another branch, your checkout is left alone. +1. Makes the checks a Run makes that don't need an issue, before creating anything: `origin` is a GitHub repository, git has a `user.name` and `user.email`, HEAD is not detached, and the Base branch exists on `origin` with your local copy not ahead of it. With `base `, a detached HEAD is fine, since the checkout no longer picks the Base branch, and the other two checks are made on ``: `base branch does not exist on origin; push it first with: git push --set-upstream origin 'refs/heads/:refs/heads/'` when that branch exists locally (the command quotes the actual refspec safely), or `local is commit(s) ahead of origin/; push them first`. If `` is absent both locally and on origin, the error instead says to create or obtain it first, or select a valid Base branch; no push command is suggested. Preflight makes no push, checkout or branch creation. There is no **Origin match**, since there is no Issue URL: the repository is the one `origin` names. If another Pass on the repository is still running, it is [skipped](#one-at-a-time) here, and if not, it is skipped when an earlier [Architect plan is still open](#one-architect-plan-at-a-time), and if not, when an [Architect idea waits for triage](#an-architect-idea-waits-for-triage), and if not, when the repository has a [Ready issue](#a-ready-issue-goes-first). With `launch.pull = true` in the [User config](#user-config), it then fast-forwards your checkout of the Base branch, as a Run does, and so only when the Base branch is the branch checked out: with `base ` from a clone on another branch, your checkout is left alone. 2. Creates a git worktree next to your clone, named `-architect`, detached at the head of the Base branch on `origin`, with no **Issue branch**. Your checkout, its uncommitted changes and its untracked files are never touched or scanned. 3. Runs the **Architecture review** there: a headless session on the Harness, with the **Factory skills** loaded, started with the [Architecture review prompt](prompts/architecture-review.md). It looks for deepening opportunities, skips any an open issue already covers, and takes the top recommendation. If it is Strong, the review publishes it as the plan, labelled `needs-triage`: a **Spec** with **Tickets**, or a single Ticket when one session is enough. It may edit files in the worktree to check an idea, but commits and pushes nothing. It ends its final message with one line naming the plan: `Architecture review plan: `. thirdshift reads only that line. With [no Strong candidate](#no-strong-candidate) the line names another issue, which thirdshift labels an **Architect idea**, and steps 5 to 7 are skipped. 4. Removes the worktree once the session ends, whatever the outcome. The temporary directory the Factory skills were written out to goes as the Architect run ends. diff --git a/src/launch.rs b/src/launch.rs index de700b947..b491d7b23 100644 --- a/src/launch.rs +++ b/src/launch.rs @@ -150,8 +150,22 @@ impl LaunchDirectory { ), } }; + let local_ref = format!("refs/heads/{base}"); if !self.git.on_origin(base)? { - bail!("base branch {base} does not exist on origin; push it first"); + if !self + .git + .succeeds(&["show-ref", "--verify", "--quiet", &local_ref])? + { + bail!( + "base branch {base} does not exist on origin or locally; \ + create or obtain that branch first, or select a valid Base branch" + ); + } + let refspec = format!("{local_ref}:{local_ref}").replace('\'', r"'\''"); + bail!( + "base branch {base} does not exist on origin; push it first with: \ + git push --set-upstream origin '{refspec}'" + ); } self.git.fetch(&[base])?; let origin_commit = self.git.run(&[ @@ -159,7 +173,6 @@ impl LaunchDirectory { "--verify", &format!("refs/remotes/origin/{base}^{{commit}}"), ])?; - let local_ref = format!("refs/heads/{base}"); if let Some(local_commit) = self.git.run_optional(&[ "rev-parse", "--verify", @@ -556,9 +569,12 @@ mod tests { } #[test] - fn a_base_branch_missing_on_origin_fails() { + fn a_local_base_branch_missing_on_origin_gets_a_working_push_command() { let (_temp, work) = launch_with_origin(); isolated(&work, &["checkout", "-q", "-b", "unpushed"]); + let refs = isolated(&work, &["show-ref"]); + let remote_refs = isolated(&work, &["ls-remote", "--heads", "origin"]); + let config = isolated(&work, &["config", "--local", "--list"]); let error = opened(&work, Opening::Run) .prepare_base_branch(None) @@ -567,7 +583,142 @@ mod tests { assert_eq!( error.to_string(), - "base branch unpushed does not exist on origin; push it first" + "base branch unpushed does not exist on origin; push it first with: \ + git push --set-upstream origin 'refs/heads/unpushed:refs/heads/unpushed'" + ); + assert_eq!(isolated(&work, &["show-ref"]), refs); + assert_eq!( + isolated(&work, &["ls-remote", "--heads", "origin"]), + remote_refs + ); + assert_eq!(isolated(&work, &["config", "--local", "--list"]), config); + assert_eq!( + isolated(&work, &["symbolic-ref", "--short", "HEAD"]), + "unpushed" + ); + run_suggested_push(&work, &error); + assert_eq!( + isolated(&work, &["rev-parse", "unpushed@{upstream}"]), + head(&work) + ); + assert_eq!( + isolated( + &work, + &["ls-remote", "--heads", "origin", "refs/heads/unpushed"] + ), + format!("{}\trefs/heads/unpushed", head(&work)) + ); + } + + fn run_suggested_push(work: &Path, error: &anyhow::Error) { + let message = error.to_string(); + let command = message.split_once("with: ").unwrap().1; + let output = Command::new("sh") + .args(["-c", command]) + .current_dir(work) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + } + + #[test] + fn a_named_local_base_gets_a_safely_quoted_push_for_that_branch() { + for base in ["release", "release/o'brien;$USER`id`", "HEAD"] { + let (_temp, work) = launch_with_origin(); + let expected = head(&work); + isolated( + &work, + &["update-ref", &format!("refs/heads/{base}"), &expected], + ); + isolated( + &work, + &["commit", "-q", "--allow-empty", "-m", "Other work"], + ); + // A same-named tag must not make the push ambiguous or publish it instead. + isolated( + &work, + &["update-ref", &format!("refs/tags/{base}"), &head(&work)], + ); + let refs = isolated(&work, &["show-ref"]); + let remote_refs = isolated(&work, &["ls-remote", "--heads", "origin"]); + let config = isolated(&work, &["config", "--local", "--list"]); + + let error = opened(&work, Opening::Pass) + .prepare_base_branch(Some(base)) + .err() + .unwrap(); + + assert!( + error + .to_string() + .starts_with(&format!("base branch {base} does not exist on origin;")) + ); + assert_eq!(isolated(&work, &["show-ref"]), refs); + assert_eq!( + isolated(&work, &["ls-remote", "--heads", "origin"]), + remote_refs + ); + assert_eq!(isolated(&work, &["config", "--local", "--list"]), config); + assert_eq!( + isolated(&work, &["symbolic-ref", "--short", "HEAD"]), + "main" + ); + run_suggested_push(&work, &error); + assert_eq!( + isolated( + &work, + &[ + "ls-remote", + "--heads", + "origin", + &format!("refs/heads/{base}") + ] + ), + format!("{expected}\trefs/heads/{base}") + ); + assert_eq!( + isolated(&work, &["config", &format!("branch.{base}.remote")]), + "origin" + ); + assert_eq!( + isolated(&work, &["config", &format!("branch.{base}.merge")]), + format!("refs/heads/{base}") + ); + assert_eq!( + isolated(&work, &["symbolic-ref", "--short", "HEAD"]), + "main" + ); + } + } + + #[test] + fn a_base_branch_absent_locally_and_on_origin_gets_no_push_command() { + let (_temp, work) = launch_with_origin(); + // A tag with that name is not a local Base branch. + isolated(&work, &["tag", "nowhere"]); + let refs = isolated(&work, &["show-ref"]); + let remote_refs = isolated(&work, &["ls-remote", "--heads", "origin"]); + + let error = opened(&work, Opening::Pass) + .prepare_base_branch(Some("nowhere")) + .err() + .unwrap(); + + assert_eq!( + error.to_string(), + "base branch nowhere does not exist on origin or locally; \ + create or obtain that branch first, or select a valid Base branch" + ); + assert_eq!(isolated(&work, &["show-ref"]), refs); + assert_eq!( + isolated(&work, &["ls-remote", "--heads", "origin"]), + remote_refs + ); + assert_eq!( + isolated(&work, &["symbolic-ref", "--short", "HEAD"]), + "main" ); } diff --git a/src/run.rs b/src/run.rs index a1b52b1a1..737595138 100644 --- a/src/run.rs +++ b/src/run.rs @@ -589,7 +589,10 @@ mod tests { pull, }); if self.script.base_fails { - bail!("base branch gone does not exist on origin; push it first"); + bail!( + "base branch gone does not exist on origin or locally; \ + create or obtain that branch first, or select a valid Base branch" + ); } Ok(named.unwrap_or(CHECKED_OUT).to_string()) } @@ -1025,7 +1028,8 @@ mod tests { assert_eq!( cause(ended), - "base branch gone does not exist on origin; push it first" + "base branch gone does not exist on origin or locally; \ + create or obtain that branch first, or select a valid Base branch" ); assert_eq!(calls.last(), Some(&base_branch(None))); } diff --git a/tests/architect_run.rs b/tests/architect_run.rs index d010a221b..f3f83176b 100644 --- a/tests/architect_run.rs +++ b/tests/architect_run.rs @@ -1229,15 +1229,33 @@ fn merge_merges_the_dispatched_runs_pull_request_into_the_named_branch() { fn a_named_branch_that_is_not_on_origin_is_rejected_before_any_work() { let scenario = scenario(); scenario.launch_git(&["branch", "local-only"]); + let refs = scenario.launch_git(&["show-ref"]); + let remote_refs = scenario.origin_git(&["show-ref"]); - for branch in ["nowhere", "local-only"] { + for (branch, guidance) in [ + ( + "nowhere", + "base branch nowhere does not exist on origin or locally; \ + create or obtain that branch first, or select a valid Base branch", + ), + ( + "local-only", + "base branch local-only does not exist on origin; push it first with: \ + git push --set-upstream origin 'refs/heads/local-only:refs/heads/local-only'", + ), + ] { let result = scenario.run(&["architect", "base", branch]); - scenario.assert_rejected_before_any_work( - &result, - &format!("base branch {branch} does not exist on origin; push it first"), - ); + scenario.assert_rejected_before_any_work(&result, guidance); assert_eq!(result.code, Some(1), "{branch}"); + assert_eq!(scenario.launch_git(&["show-ref"]), refs); + assert_eq!(scenario.origin_git(&["show-ref"]), remote_refs); + assert_eq!( + scenario + .launch_git(&["symbolic-ref", "--short", "HEAD"]) + .trim(), + "main" + ); } } diff --git a/tests/pickup_run.rs b/tests/pickup_run.rs index b24a50ae4..cb3c0cf04 100644 --- a/tests/pickup_run.rs +++ b/tests/pickup_run.rs @@ -758,7 +758,8 @@ fn a_named_branch_that_is_not_on_origin_stops_the_pass_before_any_label_changes( assert_stopped_by_preflight( &scenario, &result, - "base branch nowhere does not exist on origin; push it first", + "base branch nowhere does not exist on origin or locally; \ + create or obtain that branch first, or select a valid Base branch", ); } diff --git a/tests/preflight.rs b/tests/preflight.rs index e50e5321d..fec8f79ef 100644 --- a/tests/preflight.rs +++ b/tests/preflight.rs @@ -98,7 +98,15 @@ fn a_base_branch_missing_on_origin_is_rejected() { scenario.assert_rejected_before_any_work( &result, - "base branch local-only does not exist on origin", + "base branch local-only does not exist on origin; push it first with: \ + git push --set-upstream origin 'refs/heads/local-only:refs/heads/local-only'", + ); + assert_eq!(scenario.origin_log("local-only"), None); + assert_eq!( + scenario + .launch_git(&["symbolic-ref", "--short", "HEAD"]) + .trim(), + "local-only" ); }