diff --git a/crates/stella-cli/src/daemon/boot.rs b/crates/stella-cli/src/daemon/boot.rs index 53bb8443c..a607385a9 100644 --- a/crates/stella-cli/src/daemon/boot.rs +++ b/crates/stella-cli/src/daemon/boot.rs @@ -56,8 +56,9 @@ //! the checkpoint on every terminal path, abort included, so a policy stop //! retracts its resume point on the way out. A row written by a build that //! predates #1653 — where a policy stop really did store `Error` — is -//! therefore filtered by [`SkipReason::NoResumePoint`] anyway, without this -//! module having to trust its status. +//! therefore filtered by `SkipReason::NoResumePoint` anyway (named, not +//! linked: this module is private, so rustdoc cannot resolve it), without +//! this module having to trust its status. //! - **The attempt bound still applies.** An `Error` that resumes into //! another `Error` is counted like any other continuation and retired //! after `MAX_BOOT_ATTEMPTS`. diff --git a/crates/stella-pipeline/src/management_prompt/tests.rs b/crates/stella-pipeline/src/management_prompt/tests.rs index cf99311a9..ecbf52e87 100644 --- a/crates/stella-pipeline/src/management_prompt/tests.rs +++ b/crates/stella-pipeline/src/management_prompt/tests.rs @@ -52,6 +52,7 @@ const ALL_ROLES: [ModelCallRole; 15] = [ ModelCallRole::DomainInference, ModelCallRole::Reflection, ModelCallRole::Summarization, + ModelCallRole::Research, ]; /// The system block a role dispatches through the management chokepoint @@ -84,9 +85,8 @@ fn management_system_block(role: ModelCallRole) -> Option { // the parity witness automatically. ModelCallRole::Plan | ModelCallRole::PlanRepair => None, // Never dispatched through the management chokepoint. `Research` - // (#1778) runs as an engine sub-agent turn, so its system prompt - // rides its `SubAgentSpec` rather than this family — the same - // grouping, for the same reason, that `raw_usage.rs` gives it. + // (#1778) rides the sub-agent primitive — its system prompt travels + // on the `SubAgentSpec`, not through `metered_raw_call`. ModelCallRole::Unknown | ModelCallRole::Research | ModelCallRole::WitnessAuthor @@ -95,7 +95,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 70a6c4f0c..424b4f36a 100644 --- a/crates/stella-pipeline/src/pipeline.rs +++ b/crates/stella-pipeline/src/pipeline.rs @@ -1454,7 +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. + /// 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/evidence.rs b/crates/stella-pipeline/src/pipeline/evidence.rs index 1cb03b66d..f2f5a952f 100644 --- a/crates/stella-pipeline/src/pipeline/evidence.rs +++ b/crates/stella-pipeline/src/pipeline/evidence.rs @@ -25,6 +25,32 @@ fn touched_tests_status(observed: Option) -> &'static str { } } +/// The most observations the trusted zone will render (#1787). The trace +/// grows once per verification round, and the repair gate can keep granting +/// rounds as long as a measured budget affords them — so unlike the diff, +/// which rides under a token budget, this channel had no ceiling at all. +/// Sized far above a normal run (baseline plus a handful of rounds) so the +/// bound only ever bites a pathological loop. +const MAX_ORACLE_TRACE_OBSERVATIONS: usize = 24; + +/// Render the oracle trace for the verifier prompt, bounded to the newest +/// [`MAX_ORACLE_TRACE_OBSERVATIONS`] entries. +/// +/// Newest kept, oldest dropped: the recent runs are the ones the verdict +/// weighs, and the drop is stated in-band — the verifier must read "earlier +/// observations exist" rather than a trace that silently starts mid-run. +/// The stored snapshot keeps the full trace either way; only this prompt +/// ingress is clipped (the structural-bound rule from #1932). +fn bounded_oracle_trace(trace: &[OracleObservation]) -> String { + let omitted = trace.len().saturating_sub(MAX_ORACLE_TRACE_OBSERVATIONS); + let rendered = crate::replay::render_oracle_trace(&trace[omitted..]); + if omitted == 0 { + rendered + } else { + format!("…{omitted} earlier observation(s) omitted → {rendered}") + } +} + impl<'a> Pipeline<'a> { /// Assemble the deterministic evidence summary and the witness-stripped /// diff for one model-verdict round. @@ -118,7 +144,7 @@ impl<'a> Pipeline<'a> { // order, and what each observed — instead of a diff cold. evidence_summary.push_str(&format!( "; oracle_trace=[{}]", - crate::replay::render_oracle_trace(&snapshot.oracle_trace) + bounded_oracle_trace(&snapshot.oracle_trace) )); } if let Some(symptom) = state.witness_baseline_symptom { @@ -157,7 +183,10 @@ impl<'a> Pipeline<'a> { #[cfg(test)] mod tests { - use super::touched_tests_status; + use super::{ + MAX_ORACLE_TRACE_OBSERVATIONS, OracleObservation, ProofTree, bounded_oracle_trace, + touched_tests_status, + }; #[test] fn touched_tests_render_names_the_unobserved_case() { @@ -169,4 +198,47 @@ mod tests { assert_eq!(touched_tests_status(Some(false)), "failed"); assert_eq!(touched_tests_status(None), "unobserved"); } + + fn trace_of(len: usize) -> Vec { + (0..len) + .map(|i| OracleObservation { + tree: ProofTree::Candidate, + // Alternate so a clipped render is distinguishable from a + // repeated one. + passed: i % 2 == 0, + }) + .collect() + } + + /// #1787's witness for the trusted-zone bound: a pathological run's + /// trace reaches the prompt clipped to the newest observations, with the + /// drop stated in-band rather than the trace silently starting mid-run. + #[test] + fn a_pathological_oracle_trace_is_clipped_with_the_drop_stated() { + let trace = trace_of(100); + let rendered = bounded_oracle_trace(&trace); + assert!( + rendered.starts_with("…76 earlier observation(s) omitted → "), + "{rendered}" + ); + assert_eq!( + rendered.matches("candidate:").count(), + MAX_ORACLE_TRACE_OBSERVATIONS, + "only the newest observations are rendered: {rendered}" + ); + // The newest entry survives: index 99 is odd, so it observed a fail. + assert!(rendered.ends_with("candidate:fail"), "{rendered}"); + } + + /// An ordinary run's trace is untouched — byte-identical to the + /// unbounded render, so every existing prompt (and verdict-reuse digest) + /// is unchanged where the bound does not bite. + #[test] + fn an_ordinary_oracle_trace_renders_unchanged() { + let trace = trace_of(5); + assert_eq!( + bounded_oracle_trace(&trace), + crate::replay::render_oracle_trace(&trace) + ); + } } 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 d1ad31e50..e8c387019 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,18 +1,22 @@ -//! FlipHalt arming in revise turns, on both paths (#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, and `revise_turn` passed no latch at all. 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, and even a configured-command run's revisions -//! did the same. This pins both repairs: `witness_on_demand` arms the latch -//! the moment the witness's failing baseline is credited into the oracle, and -//! a revision receives it (unfired) through the same `run_engine_turn` seam -//! the execute turn uses. +//! `--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. 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. //! -//! Both witnesses and the two doubles they need live here rather than in the -//! parent: `verification_hardening.rs` is close enough to the 1500-line -//! ceiling that it cannot hold them, and these are the only users. +//! 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::*; @@ -233,3 +237,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" + ); +}