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 GLOSSARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,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**.

**Security review**:
When a flag or the **User config** asks for one, the agent session a **Run** starts after its opening session, or a **Spec run** starts on its **Spec PR** after the **Spec review**: it reviews the branch's change against the **Base branch** for vulnerabilities, and fixes each **Security finding** it can show with a failing test. A **Ticket**'s Run gets none. A Security review that a model refused, or that ended early, holds a **Self-merge** as an unaddressed Security finding does.
When a flag or the **User config** asks for one, the agent session a **Run** starts after its opening session, or a **Spec run** starts on its **Spec PR** after the **Spec review**: it reviews the branch's change against the **Base branch** for vulnerabilities, and fixes each **Security finding** it can show with a failing test. A **Ticket**'s Run gets none. A **Base fix** gets one when its Command asked for one, whichever Run started it, a Ticket's Run included. A Security review that a model refused, or that ended early, holds a **Self-merge** as an unaddressed Security finding does.
_Avoid_: security axis, security audit (a **Security audit** covers the whole Base branch)

**Failed spec run**:
Expand Down
2 changes: 1 addition & 1 deletion docs/code-navigation.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Start with the issue's terms in `GLOSSARY.md`, then use this index to find their
| Stop | `src/logs/reading.rs` — `retained_work` reload by captured record; `src/logs/reading/selection.rs` — `Selection::stop_target`; `src/logs/stop.rs` — request, signaling and waiting; `src/claim.rs` — Stop endings | `src/logs/reading/tests.rs`, `tests/stop.rs`; inline Claim and Stop tests |
| User config, Setup | `src/config.rs`; `src/setup.rs` | `tests/user_config.rs`, `tests/setup.rs` |
| Architecture review, Architect run | `src/architect.rs` | `tests/architect_run.rs` |
| Security audit, Security review, Security run | `src/security.rs` and `src/security/` | `tests/security_review.rs`, `tests/security_run.rs` |
| Security audit, Security review, Security run | `src/security.rs` and `src/security/`; which Runs run a Security review, and what every child Run is handed: `src/child_run/asks.rs` — `ChildAsks` | Inline tests; `tests/security_review.rs`, `tests/security_run.rs` |

For example, `rg -n 'Claim|TicketBoard|RunNotification' src/claim.rs src/spec_run/ticket_board.rs src/notification.rs` locates the owners of Claim, Ticket board and Run notification without reading the whole glossary or README. Ticket board is an implementation term; the glossary defines its Spec run and Ticket vocabulary.

Expand Down
12 changes: 7 additions & 5 deletions src/asks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@ impl Asks {
tickets_at_once: config.spec_parallel,
parallel_asked: false,
base_fix: given.base_fix.clone(),
security_fix: given.security.fix,
security_review: given.security.review,
security_fix: given.asks.security_fix,
security_review: given.asks.security_review,
launch_pull: false,
harness: given.harness.clone(),
harness: given.asks.harness.clone(),
}
}

Expand Down Expand Up @@ -424,11 +424,13 @@ mod tests {
fn given(kind: Kind, base_fix: BaseFixAsk) -> Given {
Given {
command: crate::logs::CommandKind::Issue,
security: crate::security::Options::default(),
kind,
stamp: "20261003T120000-0400".to_string(),
base_fix,
harness: sonnet(None),
asks: crate::child_run::ChildAsks {
harness: sonnet(None),
..crate::child_run::ChildAsks::default()
},
}
}

Expand Down
81 changes: 26 additions & 55 deletions src/base_fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,14 @@ use std::path::PathBuf;

use anyhow::{Result, bail};

use crate::child_run::{self, Ended, Kind};
use crate::child_run::{self, ChildAsks, Ended, Kind};
use crate::ci::{self, FailedChecks};
use crate::git::Git;
use crate::github::{Check, GitHub, ListedIssue};
use crate::harness::Choice;
use crate::issue::IssueUrl;
use crate::labels::{Label, Labels, READY_FOR_AGENT};
use crate::poll;
use crate::progress;
use crate::security::Options;

/// The label that marks a Base fix issue, which a Run finds an open one by.
pub const BASE_FIX: Label = Label::new("base-fix", "A Base fix: CI is red on a Base branch");
Expand Down Expand Up @@ -113,10 +111,6 @@ pub struct BaseFix {
/// What the Run says after its cause, if Inherited failures failed it
/// with no Base fix taken.
advice: Vec<Advice>,
/// The Harness, Model and Effort a Base fix it starts runs its sessions
/// on: the Run's.
harness: Choice,
security: Options,
}

/// The Base fix a Run took as its one: one it started, or one it found
Expand All @@ -140,10 +134,9 @@ impl Taken {

impl BaseFix {
/// The Base fix of a Run that is the child Run `child`, if it is one,
/// that asked `ask` about a Base fix, and whose sessions run on
/// `harness`, as a Base fix it starts does. A Base fix starts none of
/// its own, whatever it asked.
pub fn new(child: Option<&Kind>, ask: BaseFixAsk, harness: Choice, security: Options) -> Self {
/// that asked `ask` about a Base fix. A Base fix starts none of its own,
/// whatever it asked.
pub fn new(child: Option<&Kind>, ask: BaseFixAsk) -> Self {
let (on_inherited_failures, retry) = match (child, ask) {
(Some(Kind::BaseFix { .. }), _) => (OnInheritedFailures::IsBaseFix, None),
(_, BaseFixAsk::Allow) => (OnInheritedFailures::StartBaseFix, None),
Expand All @@ -155,17 +148,9 @@ impl BaseFix {
retry,
taken: None,
advice: Vec::new(),
harness,
security,
}
}

/// Have any Base fix it starts run its sessions on `harness`: the Run's,
/// once the Run has checked it.
pub fn runs_on(&mut self, harness: Choice) {
self.harness = harness;
}

/// Whether the Run compares its red checks with the Base branch's, so
/// that some may be Inherited failures.
pub fn sees_inherited_failures(&self) -> bool {
Expand Down Expand Up @@ -193,14 +178,15 @@ impl BaseFix {
}

/// Fix the Inherited failures in `failed`, the only red checks on the PR
/// `pr_url` of the Run on `issue`, started from `launch`, which also fail
/// on `base` at `base_commit`, where `failed` holds that commit's checks
/// of their names, with a Base fix: write its issue, start it as a child
/// `thirdshift` and wait for it to merge, after which the Run is to merge
/// `base` in and watch CI again. If an open Base fix issue other than
/// `issue` already names `base` and those checks, no issue is written:
/// `pr_url` of the Run on `outside.issue`, started from `outside.launch`,
/// which also fail on `base` at `base_commit`, where `failed` holds that
/// commit's checks of their names, with a Base fix: write its issue,
/// start it as a child `thirdshift` handed `outside.asks`, and wait for
/// it to merge, after which the Run is to merge `base` in and watch CI
/// again. If an open Base fix issue other than the Run's issue already
/// names `base` and those checks, no issue is written:
/// the Run waits for that one to close instead, as its one Base fix (see
/// [`BaseFix::wait_on`]), or, if a Run from `launch` started that Base fix
/// [`BaseFix::wait_on`]), or, if a Run from its Launch directory started that Base fix
/// and it ended with the issue still open, starts it again on the same
/// issue. Fails, with the Run's cause, if the Run was not asked to start
/// one, if the Base fix fails, in which case the cause names its issue,
Expand All @@ -209,20 +195,13 @@ impl BaseFix {
/// [`BaseFix::into_advice`] to give after its cause.
pub fn fix(
&mut self,
launch: &Git,
issue: &IssueUrl,
mut outside: LaunchAndGitHub,
pr_url: &str,
base: &str,
base_commit: &str,
failed: &FailedChecks,
) -> Result<()> {
let harness = self.harness.clone();
let mut outside = LaunchAndGitHub {
launch,
issue,
harness: &harness,
security: self.security,
};
let issue = outside.issue;
self.fix_through(&mut outside, issue, pr_url, base, base_commit, failed)
}

Expand Down Expand Up @@ -485,14 +464,14 @@ impl<W> Mark<W> {
}
}

/// The outside world of a Run on `issue` started from `launch`: its git
/// directory's locks and marks, GitHub, the child Run, its sessions on
/// `harness`, and the poll interval.
struct LaunchAndGitHub<'a> {
launch: &'a Git,
issue: &'a IssueUrl,
harness: &'a Choice,
security: Options,
/// The outside world of a Run on `issue` started from `launch`, as it
/// starts a Base fix: its git directory's locks and marks, GitHub, the child
/// Run, handed `asks`, as the Run hands every child Run, and the poll
/// interval.
pub struct LaunchAndGitHub<'a> {
pub launch: &'a Git,
pub issue: &'a IssueUrl,
pub asks: &'a ChildAsks,
}

impl Outside for LaunchAndGitHub<'_> {
Expand Down Expand Up @@ -550,7 +529,7 @@ impl Outside for LaunchAndGitHub<'_> {
let kind = Kind::BaseFix {
base: base.to_string(),
};
child_run::start(fix, kind, BaseFixAsk::Forbid, self.security, self.harness)?.wait()
child_run::start(fix, kind, BaseFixAsk::Forbid, self.asks)?.wait()
}

fn pause(&mut self) -> Result<()> {
Expand Down Expand Up @@ -898,7 +877,7 @@ mod tests {

/// A Run's Base fix as asked `ask`, not itself a Base fix.
fn asked(ask: BaseFixAsk) -> BaseFix {
BaseFix::new(None, ask, Choice::default(), Options::default())
BaseFix::new(None, ask)
}

fn undecided() -> BaseFixAsk {
Expand Down Expand Up @@ -1349,22 +1328,14 @@ mod tests {
base: "main".to_string(),
};
for ask in [BaseFixAsk::Allow, BaseFixAsk::Forbid, undecided()] {
let base_fix = BaseFix::new(Some(&child), ask, Choice::default(), Options::default());
let base_fix = BaseFix::new(Some(&child), ask);
assert!(!base_fix.sees_inherited_failures());
assert_eq!(base_fix.ask_of_tickets(), BaseFixAsk::Forbid);
}
let ticket = Kind::Ticket {
spec_branch: "spec-3".to_string(),
};
assert!(
BaseFix::new(
Some(&ticket),
BaseFixAsk::Allow,
Choice::default(),
Options::default()
)
.sees_inherited_failures()
);
assert!(BaseFix::new(Some(&ticket), BaseFixAsk::Allow).sees_inherited_failures());
assert!(asked(BaseFixAsk::Forbid).sees_inherited_failures());
}
}
Loading
Loading