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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <branch>`, a detached HEAD is fine, since the checkout no longer picks the Base branch, and the other two checks are made on `<branch>`: `base branch <branch> does not exist on origin; push it first`, or `local <branch> is <n> commit(s) ahead of origin/<branch>; 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 <branch>` 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 <branch>`, a detached HEAD is fine, since the checkout no longer picks the Base branch, and the other two checks are made on `<branch>`: `base branch <branch> does not exist on origin; push it first with: git push --set-upstream origin 'refs/heads/<branch>:refs/heads/<branch>'` when that branch exists locally (the command quotes the actual refspec safely), or `local <branch> is <n> commit(s) ahead of origin/<branch>; push them first`. If `<branch>` 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 <branch>` from a clone on another branch, your checkout is left alone.
2. Creates a git worktree next to your clone, named `<repo>-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: <Issue URL>`. 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.
Expand Down
159 changes: 155 additions & 4 deletions src/launch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,29 @@ 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(&[
"rev-parse",
"--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",
Expand Down Expand Up @@ -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)
Expand All @@ -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"
);
}

Expand Down
8 changes: 6 additions & 2 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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)));
}
Expand Down
28 changes: 23 additions & 5 deletions tests/architect_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/pickup_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
}

Expand Down
10 changes: 9 additions & 1 deletion tests/preflight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}

Expand Down
Loading