diff --git a/README.md b/README.md index 98301a16e..547e84f78 100644 --- a/README.md +++ b/README.md @@ -167,7 +167,7 @@ thirdshift version # print thirdshift 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: stdout carries the pull request's URL of the run it dispatches, or, in its place, the URL of the issue it ended on: its plan with `--plan-only`, or its idea when the review published no plan. A skipped Architect run prints nothing there when [another is still running](#one-at-a-time), the URL of each Architect plan that is [still open](#one-architect-plan-at-a-time), and the URL of each [Architect idea waiting for triage](#an-architect-idea-waits-for-triage). `pickup` follows it as well: stdout carries the pull request's URL of the run it dispatches, and nothing when the [Pickup run](#pickup-runs) is skipped. +`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: stdout carries the pull request's URL of the run it dispatches, or, in its place, the URL of the issue it ended on: its plan with `--plan-only`, or its idea when the review published no plan. A skipped Architect run prints nothing there when [another is still running](#one-at-a-time), the URL of each Architect plan that is [still open](#one-architect-plan-at-a-time), the URL of each [Architect idea waiting for triage](#an-architect-idea-waits-for-triage), and the URL of the [Ready issue that goes first](#a-ready-issue-goes-first). `pickup` follows it as well: stdout carries the pull request's URL of the run it dispatches, and nothing when the [Pickup run](#pickup-runs) is skipped. 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. @@ -489,7 +489,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 Architect run on the repository, or a [Pickup run](#pickup-runs), 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). 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`, 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 Architect run on the repository, or a [Pickup run](#pickup-runs), 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 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: `. 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 and the temporary plugin directory once the session ends, whatever the outcome. @@ -507,7 +507,7 @@ A review session that fails or is interrupted, a final message without one of th ### One Run notification -An Architect run asked for a [Run notification](#run-notifications), by `email` or by `email.always = true` in the [User config](#user-config) without `no-email`, sends exactly one, whatever its outcome, with or without `--plan-only`, unless it is skipped. A skipped Architect run, [however](#one-at-a-time) [it is](#one-architect-plan-at-a-time) [skipped](#an-architect-idea-waits-for-triage), sends none, even when asked, so a schedule can start one every few minutes without flooding your inbox: its line is in the log its scheduler keeps. It makes a Run's checks before any other work all the same, an address and a Resend API key, and stops with exit `1` if either is missing, so a run that would have been skipped fails on them too. The email goes after the outcome is final and printed, and a failed send is only a `warning:` line on stderr: it changes neither the exit code nor stdout. +An Architect run asked for a [Run notification](#run-notifications), by `email` or by `email.always = true` in the [User config](#user-config) without `no-email`, sends exactly one, whatever its outcome, with or without `--plan-only`, unless it is skipped. A skipped Architect run, [however](#one-at-a-time) [it](#one-architect-plan-at-a-time) [is](#an-architect-idea-waits-for-triage) [skipped](#a-ready-issue-goes-first), sends none, even when asked, so a schedule can start one every few minutes without flooding your inbox: its line is in the log its scheduler keeps. It makes a Run's checks before any other work all the same, an address and a Resend API key, and stops with exit `1` if either is missing, so a run that would have been skipped fails on them too. The email goes after the outcome is final and printed, and a failed send is only a `warning:` line on stderr: it changes neither the exit code nor stdout. - **Subject**: `[thirdshift] / Architect run: `. With a dispatched run, the outcome is that run's: `ready for review`, `merged`, `failed` or `interrupted`. Without one, it is the review's: `plan published` (with `--plan-only`), `idea filed`, `idea already filed`, `review failed` (also for a plan that fails a check) or `interrupted`. The repository is left out if `origin` doesn't name one on GitHub. - **Body**, plain text: a `Review:` line saying how the Architecture review ended, with the URL of the plan or idea issue it named (`plan published: `, `idea filed: `, `idea already filed: `, `failed` or `interrupted`); when the plan was dispatched, a `Dispatched:` line with that run's outcome; then what a Run's notification holds, for the dispatched run or else the failed review: the pull request URL (if any), the failure cause (if failed), with the [lines after it](#an-inherited-failure-links-the-base-branchs-checks) of a dispatched run that failed on an Inherited failure, a `Base fix:` line for a dispatched run that started or waited on a [Base fix](#base-fix), the session log path (if any), the `Command log:` path (if any), the hostname and how long the Architect run took. After a dispatched Spec run, it ends with a line per Ticket, as a Spec run's notification does. @@ -529,7 +529,19 @@ An Architect idea means the factory has run out of Strong ideas, so while any Ar - **stderr** carries one progress line naming each of them as waiting for triage, several joined by `; `: `Architect idea # "" is waiting for triage: <Issue URL>`. - **Exit code** `0`: skipped is not a failure. No agent session starts, and nothing else is done. -Any triage decision releases the pause: take `needs-triage` off, whatever replaces it (`ready-for-agent`, `ready-for-human`, `needs-info`, `wontfix` or nothing), or close the issue. The issue keeps `architect-idea`, which only counts alongside `needs-triage`. +Any triage decision releases the pause: take `needs-triage` off, whatever replaces it (`ready-for-agent`, `ready-for-human`, `needs-info`, `wontfix` or nothing), or close the issue. The issue keeps `architect-idea`, which only counts alongside `needs-triage`. An idea triaged `ready-for-agent` is then a [Ready issue](#a-ready-issue-goes-first), once it has settled, so the next Architect run is skipped for it in turn, until a Pickup run, or you, takes it. + +### A Ready issue goes first + +Work a human shaped goes ahead of the factory's own: while the repository has a **Ready issue**, by the very rule a [Pickup run](#pickup-runs) takes one by, every Architect run on it is skipped, before any review. The check comes last, after the [lock](#one-at-a-time), the [open Architect plan](#one-architect-plan-at-a-time) check and the [waiting Architect idea](#an-architect-idea-waits-for-triage) check, so with an open plan or a waiting idea as well, the skip reports that instead. It is only the search: an Architect run makes no [Sweep](#the-sweep) and holds no [Claim limit](#the-claim-limit), which are the Pickup run's. + +- **stdout** carries the URL of the lowest-numbered Ready issue, the one a Pickup run would take. +- **stderr** carries, as a Pickup run's does, [a line on each issue labelled `ready-for-agent`](#why-an-issue-was-passed-over) passed over before it, then one progress line naming it: `Ready issue #<n> "<title>" goes first: <Issue URL>`. +- **Exit code** `0`: skipped is not a failure. No agent session starts, and nothing else is done. + +An issue labelled `ready-for-agent` that is not a Ready issue never skips an Architect run: one that carries a Claim, was labelled or shaped less than ten minutes ago, has an open blocker, is a Ticket inside a Spec that is not itself a Ready issue, or is a Base fix's issue, among the rest of the rule. Those get their lines on stderr all the same, and the Architect run goes on. + +The rule doesn't ask whether anything will take the Ready issue. A `ready-for-agent` issue that you mean to run by hand, on a repository with no Pickup line in its crontab, keeps every Architect run on it from starting until you run it, which makes the Claim, or take the label off. ### One at a time @@ -581,7 +593,7 @@ The command's flags and the User config decide what a scheduled Architect run do The first leaves the pull request for you to review, and the second stops at the Architect plan, for you to read and run with `thirdshift <Issue URL>`. -A scheduled Architect run is skipped when another on the repository, or a Pickup run, is [still running](#one-at-a-time), the Spec run or Run it dispatched included, when an [Architect plan is still open](#one-architect-plan-at-a-time), or when an [Architect idea waits for triage](#an-architect-idea-waits-for-triage): no new refactor is planned until last night's Architect plan is closed or loses its `architect-plan` label. Merging its pull request closes it; if its run failed, pick it up with `thirdshift <plan URL>` first. Closing the pull request alone leaves the Architect plan open. A skip exits `0`, so the scheduler sees no failure, and the log file has the line that says why. With Run notifications on, by `email.always = true` in the User config or `email` on the line, an Architect run that did work sends one email, and a skipped one sends none, so a crontab line can fire every few minutes without flooding your inbox. A night without an email means it was skipped, or failed before the notification's checks passed: look in the log file. +A scheduled Architect run is skipped when another on the repository, or a Pickup run, is [still running](#one-at-a-time), the Spec run or Run it dispatched included, when an [Architect plan is still open](#one-architect-plan-at-a-time), when an [Architect idea waits for triage](#an-architect-idea-waits-for-triage), or when the repository has a [Ready issue](#a-ready-issue-goes-first), so a `ready-for-agent` issue with no Pickup line to take it holds off every night until you run it: no new refactor is planned until last night's Architect plan is closed or loses its `architect-plan` label. Merging its pull request closes it; if its run failed, pick it up with `thirdshift <plan URL>` first. Closing the pull request alone leaves the Architect plan open. A skip exits `0`, so the scheduler sees no failure, and the log file has the line that says why. With Run notifications on, by `email.always = true` in the User config or `email` on the line, an Architect run that did work sends one email, and a skipped one sends none, so a crontab line can fire every few minutes without flooding your inbox. A night without an email means it was skipped, or failed before the notification's checks passed: look in the log file. Any scheduler that runs the command works; a systemd user timer needs lingering on (`loginctl enable-linger`) to fire while you are logged out. diff --git a/src/architect.rs b/src/architect.rs index 201276a75..4c445d5ca 100644 --- a/src/architect.rs +++ b/src/architect.rs @@ -12,7 +12,9 @@ //! one that finds an Architect plan still open on the repository: it never //! retries or dispatches an Architect plan that is already there. And so is //! one that finds an Architect idea there waiting for triage: the factory has -//! run out of Strong ideas until the Day shift decides on it. +//! run out of Strong ideas until the Day shift decides on it. And so, last, +//! is one that finds a Ready issue there, by the Pickup run's own search: +//! work a human shaped goes first. use std::fmt; use std::path::{Path, PathBuf}; @@ -30,6 +32,7 @@ use crate::launch::{self, AlreadyRunning, Launch, Start}; use crate::plugin::Plugin; use crate::progress; use crate::prompt; +use crate::ready::{self, ReadyIssue}; use crate::run; use crate::session::{Logs, Sessions}; use crate::worktree::ReviewWorktree; @@ -66,14 +69,15 @@ impl Outcome { /// The URLs of the issues the Architect run ended on, as its stdout /// carries them when nothing was dispatched: the issue its Architecture /// review ended on, or each open Architect plan, or each Architect idea - /// waiting for triage, it was skipped for. None when it was skipped as - /// another, or a Pickup run, is still running. + /// waiting for triage, or the Ready issue, it was skipped for. None when + /// it was skipped as another, or a Pickup run, is still running. pub fn urls(&self) -> Vec<&str> { match self { Self::Skipped(Skipped::AlreadyRunning(_)) => Vec::new(), Self::Skipped(Skipped::OpenPlans(issues) | Skipped::IdeasWaiting(issues)) => { issues.iter().map(|(issue, _)| issue.url.as_str()).collect() } + Self::Skipped(Skipped::ReadyIssue(issue, _)) => vec![issue.url.as_str()], Self::Reviewed(reviewed) => vec![reviewed.url()], } } @@ -101,6 +105,9 @@ pub enum Skipped { /// These Architect ideas, each with its title, are open on this /// repository and still labelled `needs-triage`: at least one. IdeasWaiting(Vec<(IssueUrl, String)>), + /// This Ready issue, with its title, the lowest-numbered on this + /// repository, goes first. + ReadyIssue(IssueUrl, String), } impl fmt::Display for Skipped { @@ -119,6 +126,11 @@ impl fmt::Display for Skipped { idea.number, idea.url ) }), + Self::ReadyIssue(issue, title) => write!( + f, + "Ready issue #{} \"{title}\" goes first: {}", + issue.number, issue.url + ), } } } @@ -208,8 +220,11 @@ impl fmt::Display for Reviewed { /// then skipped, likewise, if the repository has an open Architect plan: only /// then, so that the plan of an Architect run still running is never taken /// for an unfinished one. It is then skipped if the repository has an open -/// Architect idea still labelled `needs-triage`. Past all three, it keeps its -/// Command log under `logs_dir`, where its Session logs go too. +/// Architect idea still labelled `needs-triage`. It is then skipped if the +/// repository has a Ready issue, by the very search a Pickup run makes, with +/// its lines on the issues passed over, but with no Sweep and no Claim limit, +/// which are the Pickup run's. Past all four, it keeps its Command log under +/// `logs_dir`, where its Session logs go too. pub fn run( focus: Option<&str>, base: Option<&str>, @@ -243,6 +258,10 @@ pub fn run( if !waiting.is_empty() { return Ok(Outcome::Skipped(Skipped::IdeasWaiting(waiting))); } + if let Some(ReadyIssue { listed, .. }) = ready::first(&launch, &repo)? { + let skipped = Skipped::ReadyIssue(listed.issue, listed.title); + return Ok(Outcome::Skipped(skipped)); + } command_log::keep(command_log::of_architect_run(logs_dir, &repo)); if launch_pull { run::pull_base_branch(&launch, checked_out.as_deref(), &base); diff --git a/src/main.rs b/src/main.rs index adc0b14ac..64046efc6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -182,6 +182,11 @@ is skipped as well: the factory has run out of Strong ideas. It prints each such stdout, names it on stderr as waiting for triage, and exits 0. Any triage decision lets the next Architect run go ahead: take needs-triage off, whatever replaces it, or close the issue. +Last, an Architect run is skipped while the repository has a Ready issue, by the search a +Pickup run makes, so that work a human shaped goes first. It prints that issue's URL on stdout, +names it on stderr as going first, and exits 0. A ready-for-agent issue that is not a Ready +issue doesn't skip it, but one with no Pickup run to take it keeps Architect runs from starting. + To run an Architect run on a schedule, have the operating system's scheduler, such as cron, run thirdshift architect base main from the clone: the README's \"On a schedule\" has a crontab entry. @@ -366,12 +371,14 @@ fn main() -> ExitCode { /// plan fails puts the cause and the session log on stderr. One that is /// skipped says why on stderr, and is no failure: as another on its /// repository, or a Pickup run, is still running, it puts nothing on stdout, -/// and as Architect plans are still open there, the URL of each. If asked, by -/// the command or the User config, it sends one Run notification, however it -/// ended, short of being skipped; the run it dispatched sends none of its -/// own. The notification's checks are made before any other work all the -/// same, so a run that would be skipped fails on them too. Its -/// Command log, kept once it is past its skip checks, covers that run too. +/// and as Architect plans are still open there, or Architect ideas wait for +/// triage there, the URL of each, or as it has a Ready issue, that issue's +/// URL. If asked, by the command or the User config, it sends one Run +/// notification, however it ended, short of being skipped; the run it +/// dispatched sends none of its own. The notification's checks are made +/// before any other work all the same, so a run that would be skipped fails +/// on them too. Its Command log, kept once it is past its skip checks, covers +/// that run too. fn architect(args: ArchitectArgs) -> ExitCode { command_log::begin("Architect run starting"); let config = match user_config() { diff --git a/tests/architect_run.rs b/tests/architect_run.rs index fd98c83dd..a0719cfcf 100644 --- a/tests/architect_run.rs +++ b/tests/architect_run.rs @@ -16,15 +16,16 @@ //! thirdshift labels the issue such a review ends on an Architect idea, with //! `architect-idea` and `needs-triage`. //! An Architect run started while another on the repository, or a Pickup run, -//! is still running, while an Architect plan is still open there, or while an -//! Architect idea there waits for triage, is skipped. +//! is still running, while an Architect plan is still open there, while an +//! Architect idea there waits for triage, or while it has a Ready issue, is +//! skipped. mod support; use std::fs; use support::resend::ResendStandIn; -use support::{REPO, RunResult, Scenario, before_command_log, leaves_running}; +use support::{REPO, RunResult, Scenario, TimelineEvent, before_command_log, leaves_running}; /// The first issue the fake agent creates: the scenario starts with issue #7. const PLAN_URL: &str = "https://github.com/acme/widgets/issues/8"; @@ -2121,7 +2122,9 @@ fn base_skips_an_architect_run_from_a_clone_on_another_branch_or_a_detached_head fn an_architect_plan_that_is_closed_or_no_longer_labelled_lets_the_architect_run_go_ahead() { for lift_the_rule in [ (|scenario| scenario.issue_is(7, "CLOSED")) as fn(&Scenario), - |scenario| scenario.issue_labelled(7, &["ready-for-agent"]), + // Still `ready-for-agent`, it would be a Ready issue, but for its + // Claim. + |scenario| scenario.issue_labelled(7, &["ready-for-agent", IN_PROGRESS]), ] { let scenario = scenario(); open_architect_plan(&scenario, 7, "Deepen the session module"); @@ -2311,10 +2314,10 @@ fn an_architect_idea_labelled_in_another_case_skips_the_architect_run_too() { #[test] fn an_architect_idea_triaged_whatever_the_decision_or_closed_lets_the_architect_run_go_ahead() { + // Triaged `ready-for-agent`, it is a Ready issue, which goes first. for triage in [ - (|scenario| scenario.issue_labelled(7, &["ready-for-agent", ARCHITECT_IDEA])) + (|scenario| scenario.issue_labelled(7, &["ready-for-human", ARCHITECT_IDEA])) as fn(&Scenario), - |scenario| scenario.issue_labelled(7, &["ready-for-human", ARCHITECT_IDEA]), |scenario| scenario.issue_labelled(7, &["Needs-Info", "Architect-Idea"]), |scenario| scenario.issue_labelled(7, &["wontfix", ARCHITECT_IDEA]), |scenario| scenario.issue_labelled(7, &["architecture", ARCHITECT_IDEA]), @@ -2420,3 +2423,193 @@ fn an_idea_that_cannot_be_labelled_sends_one_notification_with_the_cause() { "{text}" ); } + +/// Make issue `number` an open issue titled `title`, labelled +/// `ready-for-agent` with `more` labels after it: a Ready issue unless one +/// of them, or something else about it, says otherwise. +fn ready_issue(scenario: &Scenario, number: u32, title: &str, more: &[&str]) { + scenario.issue_is(number, "OPEN"); + scenario.issue_titled(number, title); + scenario.issue_labelled(number, &[&["ready-for-agent"], more].concat()); +} + +/// What a skipped Architect run says of the Ready issue `number`, titled +/// `title`, on the scenario's repository. +fn goes_first(scenario: &Scenario, number: u32, title: &str) -> String { + format!( + "Ready issue #{number} \"{title}\" goes first: {}", + scenario.issue_url(number) + ) +} + +#[test] +fn a_ready_issue_skips_the_architect_run_before_any_review() { + for args in [ + vec!["architect"], + vec!["architect", "--plan-only"], + vec!["architect", "the Spec run", "merge", "base", "main"], + ] { + let scenario = scenario(); + ready_issue(&scenario, 7, "Fix the login page", &["bug"]); + scenario.agent_does(AGENT_PUBLISHES_A_TICKET); + let github = scenario.gh_state(); + + let result = scenario.run(&args); + + assert_eq!(result.code, Some(0), "{args:?}: stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{}\n", scenario.issue_url(7))); + assert_eq!( + after_start(&result.stderr), + format!( + "thirdshift: {}\n", + goes_first(&scenario, 7, "Fix the login page") + ) + ); + assert!(scenario.claude_calls().is_empty(), "claude was started"); + assert_eq!(scenario.gh_state(), github); + assert_nothing_left_behind(&scenario); + } +} + +#[test] +fn the_skip_names_the_lowest_numbered_ready_issue_after_the_lines_on_those_passed_over() { + let scenario = scenario(); + ready_issue(&scenario, 5, "Fix the logout page", &[IN_PROGRESS]); + ready_issue(&scenario, 6, "Fix the login page", &[]); + ready_issue(&scenario, 9, "Fix the signup page", &[]); + + let result = scenario.run(&["architect"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{}\n", scenario.issue_url(6))); + assert_eq!( + after_start(&result.stderr), + format!( + "thirdshift: #5 labelled {IN_PROGRESS}\nthirdshift: {}\n", + goes_first(&scenario, 6, "Fix the login page") + ) + ); + assert!(scenario.claude_calls().is_empty(), "claude was started"); +} + +#[test] +fn a_ready_for_agent_issue_that_is_claimed_unsettled_blocked_or_a_base_fixs_does_not_skip_the_architect_run() + { + for (not_ready, passed_over) in [ + ( + (|scenario| scenario.issue_labelled(7, &["ready-for-agent", IN_PROGRESS])) + as fn(&Scenario), + format!("#7 labelled {IN_PROGRESS}"), + ), + ( + |scenario| { + let labelled = (TimelineEvent::Labelled("ready-for-agent"), 9); + scenario.issue_timeline(7, &[labelled]); + }, + "#7 not settled: labelled ready-for-agent less than 10 minutes ago".to_string(), + ), + ( + |scenario| scenario.issue_blocked_by(7, &[5]), + "#7 blocked by #5".to_string(), + ), + ( + |scenario| scenario.issue_labelled(7, &["ready-for-agent", "base-fix"]), + "#7 labelled base-fix".to_string(), + ), + ] { + let scenario = scenario(); + ready_issue(&scenario, 7, "Fix the login page", &[]); + not_ready(&scenario); + scenario.agent_does(AGENT_PUBLISHES_A_TICKET); + + let result = scenario.run(&["architect", "--plan-only"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{PLAN_URL}\n")); + assert!( + after_start(&result.stderr).starts_with(&format!("thirdshift: {passed_over}\n")), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.claude_calls().len(), 1); + } +} + +#[test] +fn a_ready_for_agent_ticket_inside_a_spec_does_not_skip_the_architect_run() { + let scenario = scenario(); + // #20 is a Spec that is not ready, so its Ticket #21 is not reached. + scenario.spec_has_tickets(20, &[(21, &[])]); + scenario.issue_labelled(20, &["needs-triage"]); + ready_issue(&scenario, 21, "Deepen the session module", &[]); + scenario.agent_does(AGENT_PUBLISHES_A_TICKET); + + let result = scenario.run(&["architect", "--plan-only"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert!( + after_start(&result.stderr) + .starts_with("thirdshift: #21 is a Ticket of #20, which is not ready\n"), + "stderr: {}", + result.stderr + ); + assert_eq!(scenario.claude_calls().len(), 1); +} + +#[test] +fn with_an_architect_idea_waiting_and_a_ready_issue_the_skip_reports_the_idea() { + let scenario = scenario(); + waiting_architect_idea(&scenario, 7, "Deepen the session module"); + ready_issue(&scenario, 5, "Fix the login page", &[]); + + let result = scenario.run(&["architect"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{}\n", scenario.issue_url(7))); + assert_eq!( + after_start(&result.stderr), + format!( + "thirdshift: {}\n", + waiting_for_triage(&scenario, 7, "Deepen the session module") + ) + ); +} + +#[test] +fn with_an_open_architect_plan_and_a_ready_issue_the_skip_reports_the_plan() { + let scenario = scenario(); + open_architect_plan(&scenario, 7, "Deepen the session module"); + ready_issue(&scenario, 5, "Fix the login page", &[]); + + let result = scenario.run(&["architect"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{}\n", scenario.issue_url(7))); + assert_eq!( + after_start(&result.stderr), + format!( + "thirdshift: {}\n", + still_open(&scenario, 7, "Deepen the session module") + ) + ); +} + +#[test] +fn an_architect_idea_triaged_ready_for_agent_goes_first_as_a_ready_issue() { + let scenario = scenario(); + ready_issue(&scenario, 7, "Deepen the session module", &[ARCHITECT_IDEA]); + scenario.agent_does(AGENT_PUBLISHES_A_TICKET); + + let result = scenario.run(&["architect", "--plan-only"]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + assert_eq!(result.stdout, format!("{}\n", scenario.issue_url(7))); + assert_eq!( + after_start(&result.stderr), + format!( + "thirdshift: {}\n", + goes_first(&scenario, 7, "Deepen the session module") + ) + ); + assert!(scenario.claude_calls().is_empty(), "claude was started"); +} diff --git a/tests/command_surface.rs b/tests/command_surface.rs index 3e3b500d6..8b0e02eb5 100644 --- a/tests/command_surface.rs +++ b/tests/command_surface.rs @@ -97,6 +97,8 @@ fn help_documents_architect_its_focus_the_dispatch_plan_only_and_when_an_archite "Either way thirdshift labels that issue an Architect idea: architect-idea and needs-triage", "an Architect run that finds an Architect idea open and still labelled needs-triage is skipped as well", "Any triage decision lets the next Architect run go ahead", + "Last, an Architect run is skipped while the repository has a Ready issue, by the search a Pickup run makes", + "one with no Pickup run to take it keeps Architect runs from starting", "Only one Architect run or Pickup run per repository runs at a time on a machine", "is skipped: it prints an Architect run or a Pickup run is already running on <owner>/<repo>, does nothing else and exits 0", "A skipped run sends none, even when asked", diff --git a/tests/pickup_run.rs b/tests/pickup_run.rs index 78b0a2100..ffa695ca3 100644 --- a/tests/pickup_run.rs +++ b/tests/pickup_run.rs @@ -939,9 +939,11 @@ fn release(scenario: &Scenario) { #[test] fn a_pickup_run_is_skipped_while_an_architect_run_on_the_repository_is_still_running() { let scenario = Scenario::new(); - ready_issue(&scenario, 7, &[]); scenario.agent_does(AGENT_WAITS_FOR_RELEASE); let architect = scenario.run_until(&["architect", "--plan-only"], &[], "started"); + // Labelled only now, as a Ready issue would have skipped the Architect + // run. + ready_issue(&scenario, 7, &[]); let (github, gh_calls) = (scenario.gh_state(), scenario.gh_calls()); let result = scenario.run(&["pickup"]); @@ -1566,10 +1568,12 @@ fn the_sweep_runs_on_a_pass_that_is_then_skipped_for_having_no_ready_issue() { #[test] fn the_sweep_does_not_run_on_a_pass_skipped_for_the_lock() { let scenario = Scenario::new(); - ready_issue(&scenario, 7, &[]); closed_issue_in_progress(&scenario, 3, &["bug"]); scenario.agent_does(AGENT_WAITS_FOR_RELEASE); let architect = scenario.run_until(&["architect", "--plan-only"], &[], "started"); + // Labelled only now, as a Ready issue would have skipped the Architect + // run. + ready_issue(&scenario, 7, &[]); let result = scenario.run(&["pickup"]);