From a037a8f4978edb0a36a463f26aed72598ec27739 Mon Sep 17 00:00:00 2001 From: Mac Anderson Date: Thu, 6 Aug 2026 18:43:23 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(stella-pipeline):=20unbreak=20main=20?= =?UTF-8?q?=E2=80=94=20two=20merges=20left=20four=20breaks=20behind?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cargo clippy -p stella-pipeline --all-targets -- -D warnings` fails on b5ab7f84. A compile error in the lib masks the test target entirely, so this reads as one failure and is really four, from two different merges. From #1953 (the #1778 research stage): 1. `management_prompt/tests.rs` — `ModelCallRole::Research` is a new variant and `management_system_block`'s match is exhaustive on purpose (E0004). Research rides the sub-agent primitive, so its system prompt travels on the `SubAgentSpec`, never through `metered_raw_call`: it joins the never-dispatched arm, and `ALL_ROLES` grows to 15. 2. `pipeline.rs` — the new `research` parameter pushed `plan_stage` to 8 arguments, one over clippy's cap. Bundled `budget`/`total` into the `Spend` struct every stage downstream of the fan-out already takes, rather than `#[allow]`-ing the lint. From #1951, which rewrote `tests/verification_hardening.rs` wholesale and dropped three items #1945 had added to it hours earlier — a same-seam clobber, in a file #1951's own subject (per-candidate verifier degradation) never needed to touch: 3. `PassingShell` and `shell_call_result` went with it, leaving the child module `flip_halt_arming.rs` referencing two helpers that exist nowhere in the tree (E0425 ×2). Restored to their original home, which the child reaches through `use super::*`. 4. `a_revision_halts_at_the_step_where_the_tracked_test_flips` went too — the configured-command **witness for #1793**. Deleting it did not fail any gate, because the crate stopped compiling for reason 3 first: #1793 has been shipping with half its witness silently gone. Restored verbatim. Both #1793 witnesses now run and pass. Neither is vacuous: each asserts a scripted-prompt count, so a `PassingShell` that omitted the `[exit code: 0]` marker `flip_halt::exit_status` parses would leave the halt unarmed, the revision would consume the steps scripted beyond the flip, and the count would be wrong. `cargo test -p stella-pipeline`: 605 passed, 0 failed. `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean. --- .../src/management_prompt/tests.rs | 10 +- crates/stella-pipeline/src/pipeline.rs | 15 ++- .../src/pipeline/scope_stage.rs | 4 +- .../pipeline/tests/management_accounting.rs | 6 +- .../pipeline/tests/verification_hardening.rs | 123 ++++++++++++++++++ 5 files changed, 144 insertions(+), 14 deletions(-) diff --git a/crates/stella-pipeline/src/management_prompt/tests.rs b/crates/stella-pipeline/src/management_prompt/tests.rs index 43c80ac28..b0cb37c6d 100644 --- a/crates/stella-pipeline/src/management_prompt/tests.rs +++ b/crates/stella-pipeline/src/management_prompt/tests.rs @@ -36,7 +36,7 @@ const SHARED_MANAGEMENT_PREAMBLE: &str = ""; /// family. Completeness is not compiler-checked here — that job belongs to /// the exhaustive match in [`management_system_block`], which forces a new /// variant to declare its prefix posture before this array matters. -const ALL_ROLES: [ModelCallRole; 14] = [ +const ALL_ROLES: [ModelCallRole; 15] = [ ModelCallRole::Unknown, ModelCallRole::Triage, ModelCallRole::Plan, @@ -51,6 +51,7 @@ const ALL_ROLES: [ModelCallRole; 14] = [ ModelCallRole::DomainInference, ModelCallRole::Reflection, ModelCallRole::Summarization, + ModelCallRole::Research, ]; /// The system block a role dispatches through the management chokepoint @@ -82,7 +83,9 @@ fn management_system_block(role: ModelCallRole) -> Option { // adopt the split these arms move to `Some(...)` and the roles join // the parity witness automatically. ModelCallRole::Plan | ModelCallRole::PlanRepair => None, - // Never dispatched through the management chokepoint. + // Never dispatched through the management chokepoint. `Research` + // (#1778) rides the sub-agent primitive — its system prompt travels + // on the `SubAgentSpec`, not through `metered_raw_call`. ModelCallRole::Unknown | ModelCallRole::WitnessAuthor | ModelCallRole::WitnessRepair @@ -90,7 +93,8 @@ fn management_system_block(role: ModelCallRole) -> Option { | ModelCallRole::SkillAuthor | ModelCallRole::DomainInference | ModelCallRole::Reflection - | ModelCallRole::Summarization => None, + | ModelCallRole::Summarization + | ModelCallRole::Research => None, } } diff --git a/crates/stella-pipeline/src/pipeline.rs b/crates/stella-pipeline/src/pipeline.rs index e4b6c598d..54644bd6b 100644 --- a/crates/stella-pipeline/src/pipeline.rs +++ b/crates/stella-pipeline/src/pipeline.rs @@ -1454,7 +1454,9 @@ impl<'a> Pipeline<'a> { // Stage: plan /// `revision` is the reviewer's note from a rejected scope card, or `None` - /// for a turn's first plan. + /// for a turn's first plan. `spend` bundles the turn's budget guard and + /// running total, as everywhere downstream of the fan-out — the #1778 + /// research param pushed the unbundled pair over clippy's argument cap. async fn plan_stage( &self, goal: &str, @@ -1462,8 +1464,7 @@ impl<'a> Pipeline<'a> { research: &[ResearchFinding], repo_structure: &str, revision: Option<&str>, - budget: &mut BudgetGuard, - total: &mut f64, + spend: &mut Spend<'_>, ) -> Result, PipelineBudgetAbort> { self.emit(AgentEvent::Stage { name: StageKind::Plan, @@ -1491,8 +1492,8 @@ impl<'a> Pipeline<'a> { overrides: &worker_overrides, timeout: self.config.engine.model_timeout, }, - budget, - total, + spend.budget, + spend.total, ) .await { @@ -1516,8 +1517,8 @@ impl<'a> Pipeline<'a> { overrides: &worker_overrides, timeout: self.config.engine.model_timeout, }, - budget, - total, + spend.budget, + spend.total, ) .await { diff --git a/crates/stella-pipeline/src/pipeline/scope_stage.rs b/crates/stella-pipeline/src/pipeline/scope_stage.rs index 77d4e273e..3ec11aa67 100644 --- a/crates/stella-pipeline/src/pipeline/scope_stage.rs +++ b/crates/stella-pipeline/src/pipeline/scope_stage.rs @@ -31,6 +31,7 @@ impl Pipeline<'_> { let repo_structure = self.repo.structure_summary().await; let mut revision: Option = None; let mut spent_revisions = 0usize; + let mut spend = Spend { budget, total }; loop { let plan = match self @@ -40,8 +41,7 @@ impl Pipeline<'_> { research, &repo_structure, revision.as_deref(), - budget, - total, + &mut spend, ) .await { diff --git a/crates/stella-pipeline/src/pipeline/tests/management_accounting.rs b/crates/stella-pipeline/src/pipeline/tests/management_accounting.rs index 7e03e3fd8..629902302 100644 --- a/crates/stella-pipeline/src/pipeline/tests/management_accounting.rs +++ b/crates/stella-pipeline/src/pipeline/tests/management_accounting.rs @@ -453,8 +453,10 @@ async fn a_late_plan_is_abandoned_and_falls_back_to_the_single_step_plan() { &[], "", None, - &mut budget, - &mut total, + &mut Spend { + budget: &mut budget, + total: &mut total, + }, ) .await .expect("a wedged planner is never a run-ending failure"); diff --git a/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs b/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs index 6816d75ac..0f133dc4a 100644 --- a/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs +++ b/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs @@ -1432,3 +1432,126 @@ async fn a_candidate_degrading_on_every_round_records_one_fact() { "two degraded rounds, one candidate, one fact" ); } + +/// A shell double whose every command "passes": the output carries the +/// trailing exit-0 marker [`crate::flip_halt::exit_status`] parses. What +/// [`EmptyTools`] can never express — a worker *observing* the tracked test +/// succeed through a tool result. +struct PassingShell; +#[async_trait] +impl ToolExecutor for PassingShell { + fn schemas(&self) -> Vec { + vec![ToolSchema { + name: "bash".into(), + description: "run a shell command".into(), + input_schema: serde_json::json!({ "type": "object" }), + read_only: false, + speculation_safe: false, + }] + } + async fn execute(&self, _name: &str, _input: &Value) -> ToolOutput { + ToolOutput::Ok { + content: "1 passed\n[exit code: 0]".into(), + } + } +} + +/// A completion that runs `command` through the shell — the observation the +/// flip halt correlates by `call_id` and scores against the tracked test. +fn shell_call_result(command: &str) -> CompletionResult { + CompletionResult { + tool_calls: vec![ToolCall { + call_id: format!("call-shell-{command}"), + name: "bash".into(), + input: serde_json::json!({ "command": command }), + }], + ..text_result("") + } +} + +/// #1793 witness (configured-command side): a revision that observes the +/// tracked test go fail→pass halts at that step boundary instead of running +/// on. The provider is scripted with steps BEYOND the flip; consuming them +/// is exactly the waste `flip_halt` exists to stop, so the call count is the +/// assertion. +#[tokio::test] +async fn a_revision_halts_at_the_step_where_the_tracked_test_flips() { + let provider = ScriptedProvider::new(vec![ + text_result("single"), + // Execute turn: acts, but the suite still fails afterwards. + text_result("done"), + // Revision, step 1: re-run the tracked test — it now passes. + shell_call_result("cargo test -p x"), + // Steps the revision would burn WITHOUT the halt. They must never be + // consumed: the goal was met at the step above. + shell_call_result("cargo test -p x"), + text_result("revision done"), + ]); + let resolver = OneProvider(&provider); + // Baseline fails (arms the halt), post-execute fails (forces the + // revision), post-revise passes (the flip), confirmation passes (#859). + let runner = ScriptedRunner::scripted( + vec![ + TestScript::Fail, + TestScript::Fail, + TestScript::Pass, + TestScript::Pass, + ], + "@@ -1 +1 @@\n-old\n+new", + ); + let tools = PassingShell; + let recall = NoContextRecall; + let repo = NoRepoStructure; + let repo_status = NoRepoStatus; + let approvals = AutoApproveGate; + let sleeper = NoopSleeper; + let router = router(); + let (tx, _rx) = mpsc::unbounded_channel(); + + let config = PipelineConfig { + test_command: Some("cargo test -p x".into()), + diff_diagnostic: Some(DiagnosticInvocation::GitDiff), + ..PipelineConfig::default() + }; + let pipeline = Pipeline::new( + PipelinePorts { + router: &router, + providers: &resolver, + tools: &tools, + recall: &recall, + repo: &repo, + repo_status: &repo_status, + touches: &NoFileTouches, + diagnostics: &runner, + tests: &runner, + lint: None, + mutation: None, + coverage: None, + approvals: &approvals, + sleeper: &sleeper, + hooks: None, + candidate_workspaces: None, + mcp_prefetch: None, + steering: None, + }, + tx, + config, + ); + + let mut messages = vec![CompletionMessage::system("sys")]; + let mut budget = BudgetGuard::new(BudgetMode::Off, None, None); + let outcome = pipeline + .run("Fix the failing test", &mut messages, &mut budget) + .await + .expect("run succeeds"); + + let verdict = outcome.verdict.expect("a verdict was produced"); + assert!(verdict.passed, "the flip was confirmed: {verdict:?}"); + assert_eq!( + provider.prompts().len(), + 3, + "triage, execute, one revision step — the revision must halt at the \ + boundary where the tracked test flipped, not spend the scripted \ + steps beyond it" + ); +} From 2aa25860f9f0150d82d7ea93c5e7dbbf5d77ba84 Mon Sep 17 00:00:00 2001 From: Mac Anderson Date: Thu, 6 Aug 2026 19:16:07 -0700 Subject: [PATCH 2/2] test(stella-pipeline): keep both #1793 flip-halt witnesses in one file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restoring `a_revision_halts_at_the_step_where_the_tracked_test_flips` in the parent commit took `tests/verification_hardening.rs` to 1557 lines, which `file-size` rejects outright — the baseline takes no new entries. Split rather than exempted, and the split is the one the content was asking for: both #1793 witnesses and the two doubles they share (`PassingShell`, `shell_call_result`) now live in `verification_hardening/flip_halt_arming.rs`, the module already named for the concern. The parent drops to 1434. That the two witnesses were ever in separate files is what let #1951's clobber happen quietly: it rewrote the parent wholesale, taking the configured-command witness and both doubles with it, and nothing failed that named the missing test — the crate had already stopped compiling for the missing doubles. With the cluster in one file the same rewrite is a merge conflict instead of a silent deletion, so the module doc says so. `cargo test -p stella-pipeline`: 605 passed, 0 failed — both witnesses among them, at their new path. --- crates/stella-pipeline/src/pipeline.rs | 5 +- .../pipeline/tests/verification_hardening.rs | 123 --------------- .../flip_halt_arming.rs | 143 +++++++++++++++++- 3 files changed, 139 insertions(+), 132 deletions(-) diff --git a/crates/stella-pipeline/src/pipeline.rs b/crates/stella-pipeline/src/pipeline.rs index 54644bd6b..424b4f36a 100644 --- a/crates/stella-pipeline/src/pipeline.rs +++ b/crates/stella-pipeline/src/pipeline.rs @@ -1454,9 +1454,8 @@ impl<'a> Pipeline<'a> { // Stage: plan /// `revision` is the reviewer's note from a rejected scope card, or `None` - /// for a turn's first plan. `spend` bundles the turn's budget guard and - /// running total, as everywhere downstream of the fan-out — the #1778 - /// research param pushed the unbundled pair over clippy's argument cap. + /// for a turn's first plan. `spend` bundles budget + total as downstream + /// does: #1778's `research` param took the pair one over clippy's cap. async fn plan_stage( &self, goal: &str, diff --git a/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs b/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs index 0f133dc4a..6816d75ac 100644 --- a/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs +++ b/crates/stella-pipeline/src/pipeline/tests/verification_hardening.rs @@ -1432,126 +1432,3 @@ async fn a_candidate_degrading_on_every_round_records_one_fact() { "two degraded rounds, one candidate, one fact" ); } - -/// A shell double whose every command "passes": the output carries the -/// trailing exit-0 marker [`crate::flip_halt::exit_status`] parses. What -/// [`EmptyTools`] can never express — a worker *observing* the tracked test -/// succeed through a tool result. -struct PassingShell; -#[async_trait] -impl ToolExecutor for PassingShell { - fn schemas(&self) -> Vec { - vec![ToolSchema { - name: "bash".into(), - description: "run a shell command".into(), - input_schema: serde_json::json!({ "type": "object" }), - read_only: false, - speculation_safe: false, - }] - } - async fn execute(&self, _name: &str, _input: &Value) -> ToolOutput { - ToolOutput::Ok { - content: "1 passed\n[exit code: 0]".into(), - } - } -} - -/// A completion that runs `command` through the shell — the observation the -/// flip halt correlates by `call_id` and scores against the tracked test. -fn shell_call_result(command: &str) -> CompletionResult { - CompletionResult { - tool_calls: vec![ToolCall { - call_id: format!("call-shell-{command}"), - name: "bash".into(), - input: serde_json::json!({ "command": command }), - }], - ..text_result("") - } -} - -/// #1793 witness (configured-command side): a revision that observes the -/// tracked test go fail→pass halts at that step boundary instead of running -/// on. The provider is scripted with steps BEYOND the flip; consuming them -/// is exactly the waste `flip_halt` exists to stop, so the call count is the -/// assertion. -#[tokio::test] -async fn a_revision_halts_at_the_step_where_the_tracked_test_flips() { - let provider = ScriptedProvider::new(vec![ - text_result("single"), - // Execute turn: acts, but the suite still fails afterwards. - text_result("done"), - // Revision, step 1: re-run the tracked test — it now passes. - shell_call_result("cargo test -p x"), - // Steps the revision would burn WITHOUT the halt. They must never be - // consumed: the goal was met at the step above. - shell_call_result("cargo test -p x"), - text_result("revision done"), - ]); - let resolver = OneProvider(&provider); - // Baseline fails (arms the halt), post-execute fails (forces the - // revision), post-revise passes (the flip), confirmation passes (#859). - let runner = ScriptedRunner::scripted( - vec![ - TestScript::Fail, - TestScript::Fail, - TestScript::Pass, - TestScript::Pass, - ], - "@@ -1 +1 @@\n-old\n+new", - ); - let tools = PassingShell; - let recall = NoContextRecall; - let repo = NoRepoStructure; - let repo_status = NoRepoStatus; - let approvals = AutoApproveGate; - let sleeper = NoopSleeper; - let router = router(); - let (tx, _rx) = mpsc::unbounded_channel(); - - let config = PipelineConfig { - test_command: Some("cargo test -p x".into()), - diff_diagnostic: Some(DiagnosticInvocation::GitDiff), - ..PipelineConfig::default() - }; - let pipeline = Pipeline::new( - PipelinePorts { - router: &router, - providers: &resolver, - tools: &tools, - recall: &recall, - repo: &repo, - repo_status: &repo_status, - touches: &NoFileTouches, - diagnostics: &runner, - tests: &runner, - lint: None, - mutation: None, - coverage: None, - approvals: &approvals, - sleeper: &sleeper, - hooks: None, - candidate_workspaces: None, - mcp_prefetch: None, - steering: None, - }, - tx, - config, - ); - - let mut messages = vec![CompletionMessage::system("sys")]; - let mut budget = BudgetGuard::new(BudgetMode::Off, None, None); - let outcome = pipeline - .run("Fix the failing test", &mut messages, &mut budget) - .await - .expect("run succeeds"); - - let verdict = outcome.verdict.expect("a verdict was produced"); - assert!(verdict.passed, "the flip was confirmed: {verdict:?}"); - assert_eq!( - provider.prompts().len(), - 3, - "triage, execute, one revision step — the revision must halt at the \ - boundary where the tracked test flipped, not spend the scripted \ - steps beyond it" - ); -} diff --git a/crates/stella-pipeline/src/pipeline/tests/verification_hardening/flip_halt_arming.rs b/crates/stella-pipeline/src/pipeline/tests/verification_hardening/flip_halt_arming.rs index d4f9895bc..5d5c21cb1 100644 --- a/crates/stella-pipeline/src/pipeline/tests/verification_hardening/flip_halt_arming.rs +++ b/crates/stella-pipeline/src/pipeline/tests/verification_hardening/flip_halt_arming.rs @@ -1,13 +1,22 @@ -//! FlipHalt arming on the authored-witness path (#1793). +//! FlipHalt arming (#1793) — **both** witnesses and the doubles they share. //! //! The mid-turn early stop used to be armed only from a configured //! `--test-command` baseline, so on the authored-witness path — the default //! for every run without a configured command — a revision kept running to -//! its step and loop caps after the witness had already flipped. This pins -//! the repair: `witness_on_demand` arms the latch the moment the witness's -//! failing baseline is credited into the oracle, and the revision receives -//! it (unfired) through the same `run_engine_turn` seam the execute turn -//! uses. +//! its step and loop caps after the witness had already flipped. The repair: +//! `witness_on_demand` arms the latch the moment the witness's failing +//! baseline is credited into the oracle, and the revision receives it +//! (unfired) through the same `run_engine_turn` seam the execute turn uses. +//! +//! The two paths are pinned by two witnesses that differ only in where the +//! tracked command comes from, so they live together with `PassingShell` and +//! `shell_call_result` — the doubles both need — rather than reaching for +//! them across a module boundary. They were split apart once already, and the +//! parent's next wholesale rewrite deleted the configured-command witness and +//! both doubles without failing a gate: the crate had stopped compiling for +//! the missing doubles first, so nothing was left to notice the missing test. +//! Keeping the cluster in one file is what makes that clobber a merge +//! conflict instead of a silent deletion. use super::*; @@ -105,3 +114,125 @@ async fn an_authored_witness_arms_the_revision_flip_halt() { flipped, not spend the scripted steps beyond it" ); } +/// A shell double whose every command "passes": the output carries the +/// trailing exit-0 marker [`crate::flip_halt::exit_status`] parses. What +/// [`EmptyTools`] can never express — a worker *observing* the tracked test +/// succeed through a tool result. +struct PassingShell; +#[async_trait] +impl ToolExecutor for PassingShell { + fn schemas(&self) -> Vec { + vec![ToolSchema { + name: "bash".into(), + description: "run a shell command".into(), + input_schema: serde_json::json!({ "type": "object" }), + read_only: false, + speculation_safe: false, + }] + } + async fn execute(&self, _name: &str, _input: &Value) -> ToolOutput { + ToolOutput::Ok { + content: "1 passed\n[exit code: 0]".into(), + } + } +} + +/// A completion that runs `command` through the shell — the observation the +/// flip halt correlates by `call_id` and scores against the tracked test. +fn shell_call_result(command: &str) -> CompletionResult { + CompletionResult { + tool_calls: vec![ToolCall { + call_id: format!("call-shell-{command}"), + name: "bash".into(), + input: serde_json::json!({ "command": command }), + }], + ..text_result("") + } +} + +/// #1793 witness (configured-command side): a revision that observes the +/// tracked test go fail→pass halts at that step boundary instead of running +/// on. The provider is scripted with steps BEYOND the flip; consuming them +/// is exactly the waste `flip_halt` exists to stop, so the call count is the +/// assertion. +#[tokio::test] +async fn a_revision_halts_at_the_step_where_the_tracked_test_flips() { + let provider = ScriptedProvider::new(vec![ + text_result("single"), + // Execute turn: acts, but the suite still fails afterwards. + text_result("done"), + // Revision, step 1: re-run the tracked test — it now passes. + shell_call_result("cargo test -p x"), + // Steps the revision would burn WITHOUT the halt. They must never be + // consumed: the goal was met at the step above. + shell_call_result("cargo test -p x"), + text_result("revision done"), + ]); + let resolver = OneProvider(&provider); + // Baseline fails (arms the halt), post-execute fails (forces the + // revision), post-revise passes (the flip), confirmation passes (#859). + let runner = ScriptedRunner::scripted( + vec![ + TestScript::Fail, + TestScript::Fail, + TestScript::Pass, + TestScript::Pass, + ], + "@@ -1 +1 @@\n-old\n+new", + ); + let tools = PassingShell; + let recall = NoContextRecall; + let repo = NoRepoStructure; + let repo_status = NoRepoStatus; + let approvals = AutoApproveGate; + let sleeper = NoopSleeper; + let router = router(); + let (tx, _rx) = mpsc::unbounded_channel(); + + let config = PipelineConfig { + test_command: Some("cargo test -p x".into()), + diff_diagnostic: Some(DiagnosticInvocation::GitDiff), + ..PipelineConfig::default() + }; + let pipeline = Pipeline::new( + PipelinePorts { + router: &router, + providers: &resolver, + tools: &tools, + recall: &recall, + repo: &repo, + repo_status: &repo_status, + touches: &NoFileTouches, + diagnostics: &runner, + tests: &runner, + lint: None, + mutation: None, + coverage: None, + approvals: &approvals, + sleeper: &sleeper, + hooks: None, + candidate_workspaces: None, + mcp_prefetch: None, + steering: None, + }, + tx, + config, + ); + + let mut messages = vec![CompletionMessage::system("sys")]; + let mut budget = BudgetGuard::new(BudgetMode::Off, None, None); + let outcome = pipeline + .run("Fix the failing test", &mut messages, &mut budget) + .await + .expect("run succeeds"); + + let verdict = outcome.verdict.expect("a verdict was produced"); + assert!(verdict.passed, "the flip was confirmed: {verdict:?}"); + assert_eq!( + provider.prompts().len(), + 3, + "triage, execute, one revision step — the revision must halt at the \ + boundary where the tracked test flipped, not spend the scripted \ + steps beyond it" + ); +}