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
10 changes: 5 additions & 5 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ _Avoid_: task, sub-task
One invocation of the factory on a single issue that is not a **Spec**, from launch to cleanup, ending in one pull request. Started directly on an **Issue URL**, by a **Spec run** for one of its **Tickets**, by an **Architect run** for the Ticket its **Architecture review** published, or by a **Pickup run** for the **Ready issue** it took.

**Claim**:
The mark that the factory has taken an issue: thirdshift labels the issue `in-progress`, in place of `ready-for-agent` if it has that, when a **Run** or a **Spec run** starts on it, whether started on its **Issue URL** or by an **Architect run** or a **Pickup run**. A **Ticket**'s Run in a Spec run and a **Base fix** make none. A Claim is released, `ready-for-agent` put back, only when the Run ends with nothing on `origin` to take over: no **Issue branch** and no pull request. Otherwise it stays while the issue is open, whether its pull request is ready for review or the Run failed, until the **Day shift** relabels it, and thirdshift takes the label off once the issue is closed.
The mark that the factory has taken an issue: thirdshift labels the issue `in-progress`, in place of `ready-for-agent` if it has that, when a **Run** or a **Spec run** starts on it, whether started on its **Issue URL** or by an **Architect run** or a **Pickup run**. A **Ticket**'s Run in a Spec run and a **Base fix** make none. Ending a Claim finishes even after Command interruption. A Claim is released, `ready-for-agent` put back, only when the Run ends with nothing on `origin` to take over: no **Issue branch** and no pull request. Otherwise it stays while the issue is open, whether its pull request is ready for review or the Run failed, until the **Day shift** relabels it, and thirdshift takes the label off once the issue is closed.
_Avoid_: lock (a lock is per machine and ends with its process), assignment

**Spec run**:
Expand Down Expand Up @@ -126,7 +126,7 @@ An open **Ticket** labelled with a triage role that says it is not agent work: `
The agent session a **Spec run** starts once every **Ticket** is done: it reviews the whole **Spec branch** against the **Base branch** and the **Spec**, fixes what it agrees with, and writes the **Spec PR**'s description, listing the rest as **Unaddressed findings**.

**Failed spec run**:
A **Spec run** that ends, including by interruption, with any of its **Tickets** not done, or with its **Spec PR** not ready, mergeable and green (or, when asked to merge, not merged). It still takes every Ticket it can reach, and a failed Ticket stops only the Tickets it blocks.
A **Spec run** that ends, including by interruption, with any of its **Tickets** not done, or with its **Spec PR** not ready, mergeable and green (or, when asked to merge, not merged). It still takes every Ticket it can reach, and a failed Ticket stops only the Tickets it blocks. After Command interruption it starts no new Ticket or Spec review, drains its active Tickets, and finishes publishing their final Tickets checklist and outcome lines.

**Merge run**:
A **Run** asked to end with its pull request merged rather than left for review, by the command it was started with or by the **User config**. It does everything a **Run** does, then a **Self-merge**. A **Ticket**'s Run in a **Spec run** is always a Merge run into the **Spec branch**; the command's ask applies to the **Spec PR**.
Expand All @@ -148,7 +148,7 @@ Writing the **User config** by answering a few questions, one per setting that m
_Avoid_: init, onboarding, configure

**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, the issue closed by thirdshift, unless it is already closed, and the issue's **Claim** removed.
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. Once the merge is confirmed at the requested head, completion continues even after Command interruption. It ends with the **Issue branch** deleted, the issue closed by thirdshift, unless it is already closed, and the issue's **Claim** removed.

**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.
Expand All @@ -169,7 +169,7 @@ A **Merge run** into a **Run**'s **Base branch** that thirdshift starts when tha
_Avoid_: hotfix, base repair (a **Repair** works on the Issue branch)

**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.
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. Even after Command interruption, any unfinished merge is aborted and 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 through the same finishing path, unless the pull request is ready, mergeable and green and only the **Self-merge** could not happen.

**Issue branch**:
A branch a **Run** works on for one issue: `issue-<n>` (the first), then `issue-<n>-branch-<k>` for k ≥ 2. A number counts as used once its pull request is merged or closed, even if the branch itself was deleted.
Expand Down Expand Up @@ -197,7 +197,7 @@ The full transcript of one agent session, one per session, a **Resume** and each
_Avoid_: transcript, session file

**Command**:
One invocation of thirdshift that does factory work: a **Run** or **Spec run** started on an **Issue URL**, an **Architect run**, or a **Pickup run**. A child Run, a **Ticket**'s Run or a **Base fix**, is part of the command that started it, not one of its own.
One invocation of thirdshift that does factory work: a **Run** or **Spec run** started on an **Issue URL**, an **Architect run**, or a **Pickup run**. A child Run, a **Ticket**'s Run or a **Base fix**, is part of the command that started it, not one of its own. Interruption stops its ordinary CLI work and prevents more from starting, while Failed run salvage, confirmed Self-merge completion, Claim ending and Spec accounting still finish; finishing never clears the recorded interruption.
_Avoid_: invocation, job

**Command log**:
Expand Down
8 changes: 4 additions & 4 deletions src/base_fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use anyhow::{Result, bail};
use crate::child_run::{self, Ended, Kind};
use crate::ci::{self, FailedChecks};
use crate::git::Git;
use crate::github::{self, Check, ListedIssue};
use crate::github::{Check, GitHub, ListedIssue};
use crate::harness::Choice;
use crate::issue::IssueUrl;
use crate::labels::{Label, Labels, READY_FOR_AGENT};
Expand Down Expand Up @@ -484,7 +484,7 @@ impl Outside for LaunchAndGitHub<'_> {

/// One `gh issue list`.
fn open_fixes(&mut self) -> Result<Vec<ListedIssue>> {
github::open_issues_labelled(&self.issue.repo_slug(), BASE_FIX)
GitHub::new().open_issues_labelled(&self.issue.repo_slug(), BASE_FIX)
}

/// Left if the mark file is there and its lock free when first opened.
Expand All @@ -508,11 +508,11 @@ impl Outside for LaunchAndGitHub<'_> {
}

fn issue_is_open(&mut self, issue: &IssueUrl) -> Result<bool> {
github::issue_is_open(issue)
GitHub::new().issue_is_open(issue)
}

fn create_issue(&mut self, title: &str, body: &str, labels: &[Label]) -> Result<IssueUrl> {
github::create_issue(self.issue, title, body, labels)
GitHub::new().create_issue(self.issue, title, body, labels)
}

fn mark_running(&mut self, number: u64) -> Result<Running> {
Expand Down
14 changes: 8 additions & 6 deletions src/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use anyhow::{Result, bail};

use crate::git::Git;
use crate::github::{self, PrState, PullRequest};
use crate::github::{GitHub, PrState, PullRequest};
use crate::issue::IssueUrl;

pub enum Selection {
Expand Down Expand Up @@ -66,8 +66,8 @@ impl Selection {
/// their PRs in any state: the highest number seen on either decides. Fails
/// if a local copy of the chosen branch in the Launch directory differs from
/// origin's: the Run replaces it and deletes it at cleanup.
pub fn select(launch: &Git, issue: &IssueUrl) -> Result<Selection> {
select_through(&GitHubAndOrigin { launch }, issue)
pub fn select(launch: &Git, github: &GitHub, issue: &IssueUrl) -> Result<Selection> {
select_through(&GitHubAndOrigin { launch, github }, issue)
}

/// What shows an issue was started.
Expand All @@ -82,8 +82,8 @@ pub enum Started {
/// What shows `issue` was ever started, if it was: the first Issue branch for
/// it on origin, or else the newest pull request from one, open, merged or
/// closed.
pub fn started(launch: &Git, issue: &IssueUrl) -> Result<Option<Started>> {
started_through(&GitHubAndOrigin { launch }, issue)
pub fn started(launch: &Git, github: &GitHub, issue: &IssueUrl) -> Result<Option<Started>> {
started_through(&GitHubAndOrigin { launch, github }, issue)
}

/// What selection reads of origin, GitHub and the Launch directory. Each
Expand All @@ -109,6 +109,7 @@ struct OnOrigin {
/// The reads of GitHub, of the Launch directory `launch` and of its origin.
struct GitHubAndOrigin<'a> {
launch: &'a Git,
github: &'a GitHub,
}

impl Reads for GitHubAndOrigin<'_> {
Expand All @@ -135,7 +136,8 @@ impl Reads for GitHubAndOrigin<'_> {

/// One `gh pr list`.
fn pull_requests(&self, issue: &IssueUrl, first_branch: &str) -> Result<Vec<PullRequest>> {
github::pull_requests_with_head_prefix(issue, first_branch)
self.github
.pull_requests_with_head_prefix(issue, first_branch)
}

fn local_head(&self, branch: &str) -> Result<Option<String>> {
Expand Down
6 changes: 3 additions & 3 deletions src/ci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::time::{Duration, Instant};

use anyhow::Result;

use crate::github::{self, ActionsJob, Check, CheckState};
use crate::github::{ActionsJob, Check, CheckState, GitHub};
use crate::issue::IssueUrl;
use crate::poll;
use crate::progress;
Expand Down Expand Up @@ -109,11 +109,11 @@ impl Outside for OnGitHub<'_> {
type Deadline = Instant;

fn checks_on(&mut self, sha: &str) -> Result<Vec<Check>> {
github::checks_on(self.issue, sha)
GitHub::new().checks_on(self.issue, sha)
}

fn rerun_failed_jobs(&mut self, workflow_run: u64) -> Result<()> {
github::rerun_failed_jobs(self.issue, workflow_run)
GitHub::new().rerun_failed_jobs(self.issue, workflow_run)
}

fn pause(&mut self) -> Result<()> {
Expand Down
27 changes: 20 additions & 7 deletions src/claim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use anyhow::{Context, Result};

use crate::branch;
use crate::git::Git;
use crate::github;
use crate::github::GitHub;
use crate::interrupt;
use crate::issue::IssueUrl;
use crate::labels::{self, Edit, Label, Labels, READY_FOR_AGENT};
Expand Down Expand Up @@ -45,7 +45,13 @@ pub struct Claim<'a> {
/// `ready-for-agent`, is left as it is, with no request made. A failure names
/// the Claim as what could not be made.
pub fn make<'a>(issue: &'a IssueUrl, launch: &'a Git) -> Result<Claim<'a>> {
let claimed = Claimed::make(&mut OnGitHub { launch }, issue)?;
let claimed = Claimed::make(
&mut OnGitHub {
launch,
github: GitHub::new(),
},
issue,
)?;
Ok(Claim { launch, claimed })
}

Expand All @@ -63,7 +69,13 @@ impl Claim<'_> {
/// warning naming what to run by hand, and an interrupt doesn't stop it.
pub fn end(self, reached: Option<Goal>) {
let launch = self.launch.completion();
self.claimed.end(&mut OnGitHub { launch: &launch }, reached);
self.claimed.end(
&mut OnGitHub {
launch: &launch,
github: GitHub::new().completion(),
},
reached,
);
}
}

Expand Down Expand Up @@ -98,27 +110,28 @@ struct State {
/// The issue on GitHub, and origin as the Launch directory `launch` sees it.
struct OnGitHub<'a> {
launch: &'a Git,
github: GitHub,
}

impl Outside for OnGitHub<'_> {
fn labels(&mut self, issue: &IssueUrl) -> Result<Labels> {
github::issue_labels(issue)
self.github.issue_labels(issue)
}

fn state(&mut self, issue: &IssueUrl) -> Result<State> {
let issue = github::issue(issue)?;
let issue = self.github.issue(issue)?;
Ok(State {
is_open: issue.is_open,
labels: issue.labels,
})
}

fn started(&mut self, issue: &IssueUrl) -> Result<bool> {
Ok(branch::started(self.launch, issue)?.is_some())
Ok(branch::started(self.launch, &self.github, issue)?.is_some())
}

fn apply(&mut self, edit: &Edit) -> Result<()> {
edit.apply()
edit.apply(&self.github)
}

fn interrupted(&mut self) -> bool {
Expand Down
29 changes: 17 additions & 12 deletions src/delivery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ mod repair_loop;
use crate::base_fix::BaseFix;
use crate::ci::{self, Ci, FailedChecks};
use crate::failed_run::{FailedRun, PolicyRefusal, interrupted_or};
use crate::github::{self, Mergeable, PullRequest};
use crate::github::{GitHub, Mergeable, PullRequest};
use crate::harness::Choice;
use crate::host;
use crate::interrupt;
Expand Down Expand Up @@ -433,19 +433,19 @@ impl Outside for InWorktree<'_> {
}

fn pull_request(&mut self) -> Result<Option<PullRequest>> {
github::pull_request_for(self.issue, self.worktree.branch())
GitHub::new().pull_request_for(self.issue, self.worktree.branch())
}

fn pr_body(&mut self, number: u64) -> Result<String> {
github::pr_body(self.issue, number)
GitHub::new().pr_body(self.issue, number)
}

fn set_pr_body(&mut self, number: u64, body: &str) -> Result<()> {
github::set_pr_body(self.issue, number, body)
GitHub::new().set_pr_body(self.issue, number, body)
}

fn mark_ready(&mut self) -> Result<()> {
github::mark_ready(self.issue, self.worktree.branch())
GitHub::new().mark_ready(self.issue, self.worktree.branch())
}

fn take_to_goal(&mut self, pr_url: &str, goal: Goal) -> Result<()> {
Expand All @@ -465,11 +465,11 @@ impl Outside for InWorktree<'_> {
}

fn issue_is_open(&mut self) -> Result<bool> {
github::issue_is_open(self.issue)
GitHub::new().completion().issue_is_open(self.issue)
}

fn close_issue(&mut self, comment: &str) -> Result<()> {
github::close_issue(self.issue, comment)
GitHub::new().completion().close_issue(self.issue, comment)
}

fn interrupted(&mut self) -> bool {
Expand Down Expand Up @@ -529,11 +529,15 @@ impl FailedOutside for OnFailure<'_> {
}

fn pull_request(&mut self) -> Result<Option<PullRequest>> {
github::pull_request_for(self.issue, self.worktree.branch())
GitHub::new()
.completion()
.pull_request_for(self.issue, self.worktree.branch())
}

fn convert_to_draft(&mut self) -> Result<()> {
github::convert_to_draft(self.issue, self.worktree.branch())
GitHub::new()
.completion()
.convert_to_draft(self.issue, self.worktree.branch())
}

fn keep(&mut self) {
Expand Down Expand Up @@ -648,7 +652,7 @@ impl repair_loop::Outside for RunOutside<'_> {
}

fn merge(&mut self, head: &str) -> Result<()> {
github::merge(self.issue, self.worktree.branch(), head)
GitHub::new().merge(self.issue, self.worktree.branch(), head)
}

fn interrupt_requested(&mut self) -> bool {
Expand All @@ -672,12 +676,13 @@ fn pending(merge: Merge) -> Option<PendingMerge> {
/// mergeable, waiting up to the grace period for GitHub to work out the last.
fn ensure_pr_ready_and_mergeable(issue: &IssueUrl, branch: &str) -> Result<()> {
progress::step("checking the PR is open, ready and mergeable");
let pr = open(github::pull_request_for(issue, branch)?)?;
let github = GitHub::new();
let pr = open(github.pull_request_for(issue, branch)?)?;
if pr.is_draft {
bail!("PR {} is a draft", pr.url);
}
let mergeable = poll::within(poll::grace_period(), || {
Ok(Some(github::mergeable(issue, branch)?).filter(|m| *m != Mergeable::Unknown))
Ok(Some(github.mergeable(issue, branch)?).filter(|m| *m != Mergeable::Unknown))
})?;
match mergeable {
Some(Mergeable::Yes) => Ok(()),
Expand Down
Loading
Loading