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
15 changes: 11 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ thirdshift version # print thirdshift <version>
thirdshift help # print every form of the command, each with a one-line description
```

`version` and `help` print to stdout and exit `0`. `update`, `setup` and `email-test` follow the Run's rule: stdout stays empty, messages go to stderr. `architect` follows it too, with the plan's URL on stdout in place of a pull request's.
`version` and `help` print to stdout and exit `0`. `update`, `setup` and `email-test` follow the Run's rule: stdout stays empty, messages go to stderr. `architect` follows it too, with the URL of the issue it ended on, its plan or its idea, on stdout in place of a pull request's.

Uncommitted changes in your clone are fine: the Run works in its own worktree from `origin`, so they are simply left out. Unpushed commits on the Base branch are not: push them first, or the Run stops.

Expand Down Expand Up @@ -360,14 +360,21 @@ 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. There is no **Origin match**, since there is no Issue URL: the repository is the one `origin` names. With `launch.pull = true` in the [User config](#user-config), it then fast-forwards your checkout of the Base branch, as a Run does.
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 Claude Code session 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 publishes the top recommendation 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.
3. Runs the **Architecture review** there: a headless Claude Code session 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, and steps 4 and 5 are skipped.
4. Checks the plan: the issue is in this repository, is open, was created after the Architect run started, and carries no other label that says it is not agent work (`ready-for-human`, `needs-info` or `wontfix`).
5. Marks the plan ready, in one request: `needs-triage` is swapped for `ready-for-agent`, and its other labels are kept. A Spec's Tickets are left as the review labelled them.
6. Removes the worktree and the temporary plugin directory, whatever the outcome.

On success it exits `0` with the plan's URL alone on stdout, and `plan <url> is ready for an agent` as stderr's last line. Read or edit the plan, then run it with `thirdshift <Issue URL>`. Progress lines on stderr say when the review starts, which plan it reported, and when the labels are swapped.
Once the plan is marked ready it exits `0` with the plan's URL alone on stdout, and `plan <url> is ready for an agent` as stderr's last line. Read or edit the plan, then run it with `thirdshift <Issue URL>`. Progress lines on stderr say when the review starts, which plan it reported, and when the labels are swapped; with no Strong candidate, the last line says which issue the Architect run ended on instead.

A review session that fails or is interrupted, a final message without that line, or a plan that fails a check ends the Architect run as a failure: exit `1`, nothing on stdout, the cause on stderr and then the path of the session log. No label is changed, so a plan the review did publish stays `needs-triage` for you to finish or close. A review that reports an idea it filed, or an open issue that already covers its top recommendation, instead of a plan, is a failure too for now, naming that issue.
A review session that fails or is interrupted, a final message without one of the lines the prompt asks for, or a plan that fails a check ends the Architect run as a failure: exit `1`, nothing on stdout, the cause on stderr and then the path of the session log. No label is changed, so a plan the review did publish stays `needs-triage` for you to finish or close.

### No Strong candidate

Only a Strong top recommendation becomes a plan. When the review's top recommendation is Worth exploring or Speculative, it publishes no plan, and the Architect run ends in one of two ways, both a success: exit `0`, with one issue's URL alone on stdout. thirdshift changes no label on that issue, and there is nothing to run.

- **An idea issue.** The review files its top recommendation as one issue labelled `needs-triage`, for the **Day shift** to flesh out, and ends its final message with `Architecture review idea: <Issue URL>`. stdout carries the idea issue's URL, and stderr's last line is `no Strong candidate: the Architecture review filed the idea <url>`.
- **Already filed.** An open issue already covers that recommendation, so the review files nothing and ends its final message with `Architecture review already filed: <Issue URL>`. stdout carries that issue's URL, and stderr's last line is `no Strong candidate: <url> already covers the Architecture review's top recommendation, so it filed nothing`.

Start one Architect run per repository at a time. Nothing stops a second one, but two at once may pick the same opportunity and publish the same plan, and the second can't create its worktree while the first's is there.

Expand Down
117 changes: 78 additions & 39 deletions src/architect.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
//! One Architect run: an Architecture review of the Base branch, from the
//! Launch directory with no Issue URL, then the checks on the plan it
//! published and the label swap that marks the plan ready.
//! published and the label swap that marks the plan ready. A review that
//! found no Strong candidate published no plan, and the Architect run ends
//! on the issue it named instead: the idea issue it filed for its top
//! recommendation, or the open issue that already covers it.

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

use anyhow::{Context, Result, anyhow, bail};
Expand Down Expand Up @@ -29,14 +33,57 @@ const REVIEW: &str = "architecture-review";
/// plan passes its checks.
const READY_FOR_AGENT: &str = "ready-for-agent";

/// How an Architect run ended, short of a failure, with the issue it ended
/// on. Its `Display` is the line that says how it ended, naming that issue.
#[derive(Debug)]
pub enum Outcome {
/// The plan the review published is marked ready.
PlanReady(IssueUrl),
/// The review found no Strong candidate, and filed its top
/// recommendation as this issue.
IdeaFiled(IssueUrl),
/// The review found no Strong candidate, and filed nothing: this open
/// issue already covers its top recommendation.
AlreadyFiled(IssueUrl),
}

impl Outcome {
/// The URL of the issue the Architect run ended on.
pub fn url(&self) -> &str {
match self {
Self::PlanReady(issue) | Self::IdeaFiled(issue) | Self::AlreadyFiled(issue) => {
&issue.url
}
}
}
}

impl fmt::Display for Outcome {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let url = self.url();
match self {
Self::PlanReady(_) => write!(f, "plan {url} is ready for an agent"),
Self::IdeaFiled(_) => write!(
f,
"no Strong candidate: the Architecture review filed the idea {url}"
),
Self::AlreadyFiled(_) => write!(
f,
"no Strong candidate: {url} already covers the Architecture review's top recommendation, so it filed nothing"
),
}
}
}

/// Run an Architecture review of the branch checked out in the Launch
/// directory, the Base branch, pointed at `focus` if given, and mark the plan
/// it publishes ready. Returns the plan's URL. With `launch_pull`, the Launch
/// directory's checkout of the Base branch is first brought up to date with
/// origin. The review's worktree and the plugin directory are gone when this
/// returns. A failure after the plan is published leaves its labels as the
/// review left them.
pub fn run(focus: Option<&str>, logs_dir: &Path, launch_pull: bool) -> Result<String, FailedRun> {
/// it publishes ready. A review that reports an idea issue it filed, or the
/// open issue that already covers its top recommendation, instead of a plan,
/// changes no label. With `launch_pull`, the Launch directory's checkout of
/// the Base branch is first brought up to date with origin. The review's
/// worktree and the plugin directory are gone when this returns. A failure
/// after the plan is published leaves its labels as the review left them.
pub fn run(focus: Option<&str>, logs_dir: &Path, launch_pull: bool) -> Result<Outcome, FailedRun> {
let started = Utc::now();
let timestamp = started.format("%Y%m%dT%H%M%SZ").to_string();
let launch = Git::new(std::env::current_dir().context("no current directory")?);
Expand All @@ -61,7 +108,7 @@ pub fn run(focus: Option<&str>, logs_dir: &Path, launch_pull: bool) -> Result<St
let logs = Logs::of_architect_run(&repo, logs_dir, &timestamp);
let mut log = logs.path(REVIEW);
review(worktree, &base, focus, &logs, &mut log)
.and_then(|final_message| mark_plan_ready(final_message.as_deref(), &origin, started))
.and_then(|final_message| conclude(final_message.as_deref(), &origin, started))
.map_err(|error| FailedRun {
log: log.exists().then_some(log),
..FailedRun::from(error)
Expand Down Expand Up @@ -124,21 +171,32 @@ impl Report {
}
}

/// Mark ready the plan that the review's `final_message` names, and return
/// its URL: check it, then swap its `needs-triage` for `ready-for-agent`, in
/// one request. Fails, changing no label, without a final line naming a
/// plan, or if the plan fails its checks.
fn mark_plan_ready(
final_message: Option<&str>,
origin: &str,
started: DateTime<Utc>,
) -> Result<String> {
let plan = reported_plan(final_message)?;
/// End the Architect run on the issue the last line of the review's
/// `final_message` names: a plan is marked ready, and an idea issue, or the
/// issue that already covers the top recommendation, is left as it is. Fails if the session had no
/// final message, if its last line is not one the prompt asks for, or if the
/// plan can't be marked ready.
fn conclude(final_message: Option<&str>, origin: &str, started: DateTime<Utc>) -> Result<Outcome> {
match final_message.and_then(Report::read) {
Some(Report::Plan(plan)) => {
mark_plan_ready(&plan, origin, started)?;
Ok(Outcome::PlanReady(plan))
}
Some(Report::Idea(idea)) => Ok(Outcome::IdeaFiled(idea)),
Some(Report::AlreadyFiled(issue)) => Ok(Outcome::AlreadyFiled(issue)),
None => bail!("the Architecture review ended without the final line its prompt asks for"),
}
}

/// Mark `plan` ready: check it, then swap its `needs-triage` for
/// `ready-for-agent`, in one request. Fails, changing no label, if the plan
/// fails its checks.
fn mark_plan_ready(plan: &IssueUrl, origin: &str, started: DateTime<Utc>) -> Result<()> {
progress::step(format_args!(
"the Architecture review published the plan {}",
plan.url
));
let kept = labels_to_keep(&plan, origin, started)?;
let kept = labels_to_keep(plan, origin, started)?;
if interrupt::requested() {
bail!("interrupted");
}
Expand All @@ -152,26 +210,7 @@ fn mark_plan_ready(
.filter(|&label| label != READY_FOR_AGENT)
.collect();
labels.push(READY_FOR_AGENT);
github::set_labels(&plan, &labels)?;
Ok(plan.url)
}

/// The plan the last line of the review's `final_message` names. Fails if
/// the session had no final message, if its last line is not one the prompt
/// asks for, or if that line names an issue that is not a plan.
fn reported_plan(final_message: Option<&str>) -> Result<IssueUrl> {
match final_message.and_then(Report::read) {
Some(Report::Plan(plan)) => Ok(plan),
Some(Report::Idea(idea)) => bail!(
"the Architecture review published no plan: it filed the idea {}",
idea.url
),
Some(Report::AlreadyFiled(issue)) => bail!(
"the Architecture review published no plan: {} already covers its top recommendation",
issue.url
),
None => bail!("the Architecture review ended without a final line naming its plan"),
}
github::set_labels(plan, &labels)
}

/// The labels `plan` keeps once it is marked ready: all it has but
Expand Down
28 changes: 18 additions & 10 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,14 @@ that points the review at an area:

thirdshift architect \"the Spec run\" --plan-only

A review that fails, is interrupted, or ends without naming a plan fails the Architect run and
leaves any plan it published labelled needs-triage. Start one Architect run per repository at a
time: two at once may publish the same plan.
A review that finds no Strong candidate publishes no plan. It files its top recommendation as
one idea issue labelled needs-triage, or names the open issue that already covers it, and
thirdshift prints that issue's URL instead, changing no label. Its last line says which: the
review filed the idea, or it filed nothing.

A review that fails, is interrupted, or ends without naming one of these issues fails the
Architect run and leaves any plan it published labelled needs-triage. Start one Architect run
per repository at a time: two at once may publish the same plan.

The User config, ~/.thirdshift/config.toml, sets defaults for every Run on this machine;
thirdshift setup asks for your defaults and writes one listing every setting, to edit.
Expand Down Expand Up @@ -213,8 +218,10 @@ fn main() -> ExitCode {
code
}

/// An Architect run that stops at the plan: the plan's URL on stdout once it
/// is marked ready, or the cause and the session log on stderr.
/// An Architect run that stops at the plan: the URL of the issue it ended on
/// on stdout, the plan once it is marked ready or, with no Strong candidate,
/// the idea issue the review filed or the issue that already covers its top
/// recommendation, or the cause and the session log on stderr.
fn architect(args: &ArchitectArgs) -> ExitCode {
let config = match user_config() {
Ok(config) => config,
Expand All @@ -224,10 +231,10 @@ fn architect(args: &ArchitectArgs) -> ExitCode {
return failure(&error);
}
match architect::run(args.focus.as_deref(), &config.logs_dir, config.launch_pull) {
Ok(plan_url) => {
Ok(outcome) => {
// Also on stderr, so the outcome shows even when stdout is captured.
progress::step(format_args!("plan {plan_url} is ready for an agent"));
print_url(&plan_url);
progress::step(format_args!("{outcome}"));
print_url(outcome.url());
ExitCode::SUCCESS
}
Err(failed) => report(&failed),
Expand Down Expand Up @@ -262,8 +269,9 @@ fn report(failed: &FailedRun) -> ExitCode {
ExitCode::FAILURE
}

/// A pull request's or a plan's URL on stdout. A failed write, as once the
/// terminal has closed, is ignored, so the Run notification still goes.
/// A pull request's URL, or the URL of the issue an Architect run ended on,
/// on stdout. A failed write, as once the terminal has closed, is ignored, so
/// the Run notification still goes.
fn print_url(url: &str) {
let _ = writeln!(std::io::stdout(), "{url}");
}
Expand Down
Loading
Loading