diff --git a/crates/stella-cli/src/agent/goal.rs b/crates/stella-cli/src/agent/goal.rs index 805c0e1f7..1061f574a 100644 --- a/crates/stella-cli/src/agent/goal.rs +++ b/crates/stella-cli/src/agent/goal.rs @@ -694,7 +694,8 @@ async fn run_goal_pipeline_turn( // the read-only tool view, the verifier tuning, and the session // calibration (keyed per model, so a cross-family verifier learns its // own drift) into each assessment. - let read_only = stella_core::ports::ReadOnlyTools::new(&tools); + let scoped = VerifierScopedTools::new(&tools); + let read_only = stella_core::ports::ReadOnlyTools::new(&scoped); let verifier_engine = Engine::with_sleeper( verifier, &read_only, @@ -876,3 +877,137 @@ async fn run_goal_pipeline_turn( tui::files_touched_panel(&files); goal_result } + +/// The six tools the goal verifier's system prompt promises +/// (`stella_core::goal::VERIFIER_SYSTEM_PROMPT`), and the only ones it is +/// offered. Pinned against the prompt by a test below so the two cannot +/// drift. +const VERIFIER_TOOL_ALLOWLIST: &[&str] = &[ + "read_file", + "grep", + "glob", + "explorations", + "ci_status", + "search_issues", +]; + +/// The goal verifier's tool surface (#1783): the session stack narrowed to +/// [`VERIFIER_TOOL_ALLOWLIST`] BEFORE the read-only view applies. The bare +/// `ReadOnlyTools` wrap admitted every schema claiming `read_only: true` — +/// ~25 tools including `web_fetch`/`web_search` (outbound HTTP from a role +/// that reads worker-influenced content: a prompt-injection egress channel) +/// and any MCP/custom tool that self-declares read-only. The pipeline's +/// verdict call carries zero tools; this is the goal loop's equivalent +/// posture: exactly what the prompt names, enforced at execution rather +/// than by prompt. +struct VerifierScopedTools<'a> { + inner: &'a dyn stella_core::ToolExecutor, +} + +impl<'a> VerifierScopedTools<'a> { + fn new(inner: &'a dyn stella_core::ToolExecutor) -> Self { + Self { inner } + } +} + +#[async_trait::async_trait] +impl stella_core::ToolExecutor for VerifierScopedTools<'_> { + fn schemas(&self) -> Vec { + self.inner + .schemas() + .into_iter() + .filter(|schema| VERIFIER_TOOL_ALLOWLIST.contains(&schema.name.as_str())) + .collect() + } + + async fn execute(&self, name: &str, input: &serde_json::Value) -> stella_protocol::ToolOutput { + if !VERIFIER_TOOL_ALLOWLIST.contains(&name) { + return stella_protocol::ToolOutput::Error { + message: format!( + "`{name}` is not available to the goal verifier: only the tools its \ + instructions name ({}) may be called", + VERIFIER_TOOL_ALLOWLIST.join(", ") + ), + }; + } + self.inner.execute(name, input).await + } + + // Forwarded, not zeroed (the trait doc's decorator rule): nothing on the + // allowlist dispatches sub-agents today, but a wrapper that drops spend + // is wrong the day that changes. + fn drain_sub_agent_spend_usd(&self) -> f64 { + self.inner.drain_sub_agent_spend_usd() + } +} + +#[cfg(test)] +mod verifier_tools_tests { + use super::*; + use stella_protocol::{ToolOutput, ToolSchema}; + + struct FakeStack; + #[async_trait::async_trait] + impl stella_core::ToolExecutor for FakeStack { + fn schemas(&self) -> Vec { + ["read_file", "grep", "web_fetch", "mcp__srv__read", "bash"] + .into_iter() + .map(|name| ToolSchema { + name: name.into(), + description: String::new(), + input_schema: serde_json::json!({}), + // Everything claims read-only — the exact shape a + // self-declaring MCP tool or the web group presents. + read_only: true, + speculation_safe: false, + }) + .collect() + } + async fn execute(&self, name: &str, _input: &serde_json::Value) -> ToolOutput { + ToolOutput::Ok { + content: format!("ran {name}"), + } + } + } + + /// #1783's witness: the goal verifier is offered exactly what its + /// prompt names — a read-only claim alone (web egress, a self-declared + /// MCP read) no longer admits a tool, and execution is enforced, not + /// prompted. + #[tokio::test] + async fn the_goal_verifier_gets_only_the_tools_its_prompt_names() { + let stack = FakeStack; + let scoped = VerifierScopedTools::new(&stack); + let names: Vec<_> = scoped + .schemas() + .into_iter() + .map(|schema| schema.name) + .collect(); + assert_eq!(names, vec!["read_file", "grep"]); + for denied in ["web_fetch", "mcp__srv__read", "bash", "web_search"] { + assert!( + scoped + .execute(denied, &serde_json::json!({})) + .await + .is_error(), + "{denied} must be refused at execution" + ); + } + assert!(matches!( + scoped.execute("read_file", &serde_json::json!({})).await, + ToolOutput::Ok { .. } + )); + } + + /// The allowlist and the prompt must name the same six tools — the + /// drift this pins is exactly how the surface grew to ~25 unnoticed. + #[test] + fn the_allowlist_matches_what_the_prompt_promises() { + for name in VERIFIER_TOOL_ALLOWLIST { + assert!( + stella_core::goal::VERIFIER_SYSTEM_PROMPT.contains(name), + "allowlisted `{name}` is not named by the verifier prompt" + ); + } + } +} diff --git a/crates/stella-cli/src/candidate_ws.rs b/crates/stella-cli/src/candidate_ws.rs index 3424d0ba8..2d241acb8 100644 --- a/crates/stella-cli/src/candidate_ws.rs +++ b/crates/stella-cli/src/candidate_ws.rs @@ -385,7 +385,16 @@ impl GitCandidateWorkspaces { // (the workspace outlives every borrow). Custom tools re-root // to `ws_root`, so their subprocesses run in the shadow. let registry: Arc = Arc::new(registry); - let witness_tools = WitnessToolExecutor::new(ws_root.clone(), registry.clone()); + // The witness author's reads honor the operator's tool policy + // exactly like the worker's do (#1784): before this wrap, a + // `"tools": {"read_file": "off"}` switch reached every worker + // surface while the witness author kept reading through the + // raw registry — an unstated exemption from a setting that + // claims to govern the session's whole tool stack. + let witness_reads: Arc = Arc::new( + crate::agent::PolicyToolSet::new_owned(registry.clone(), self.policy.clone()), + ); + let witness_tools = WitnessToolExecutor::new(ws_root.clone(), witness_reads); let native = CustomToolSet::new_owned(registry, self.custom_tools.clone(), ws_root.clone()); // MCP: layer the candidate_safe-filtered session view on top diff --git a/crates/stella-cli/src/candidate_ws/witness_tools.rs b/crates/stella-cli/src/candidate_ws/witness_tools.rs index a391e315a..993b121ee 100644 --- a/crates/stella-cli/src/candidate_ws/witness_tools.rs +++ b/crates/stella-cli/src/candidate_ws/witness_tools.rs @@ -155,7 +155,7 @@ impl ToolExecutor for WitnessToolExecutor { .reads .schemas() .into_iter() - .filter(|schema| schema.name == "read_file") + .filter(|schema| schema.name == "read_file" || schema.name == "glob") .collect(); schemas.push(ToolSchema { name: "create_witness_test".into(), @@ -188,6 +188,30 @@ impl ToolExecutor for WitnessToolExecutor { } self.reads.execute(name, input).await } + // Discovery for the blind author (#1792): the repo listing in the + // prompt is truncated to its first 200 sorted paths, and an + // author that cannot see any `tests/` directory there had no + // legal move — `read_file` needs a path it can already name, and + // `create_witness_test` requires the parent to exist. `glob` is + // names-only, root-confined by the tool itself, and its results + // pass the same credential exclusion the read path enforces. + "glob" => { + if let Some(raw_path) = input.get("path").and_then(serde_json::Value::as_str) + && normalized_candidate_path(raw_path).is_none() + { + return Self::denied(name, "the path must stay within the candidate root"); + } + match self.reads.execute(name, input).await { + ToolOutput::Ok { content } => ToolOutput::Ok { + content: content + .lines() + .filter(|line| !is_credential_path(line.trim())) + .collect::>() + .join("\n"), + }, + error => error, + } + } "create_witness_test" => self.create_test(input), _ => Self::denied( name, @@ -385,7 +409,7 @@ mod tests { .into_iter() .map(|schema| schema.name) .collect(); - assert_eq!(names, vec!["read_file", "create_witness_test"]); + assert_eq!(names, vec!["glob", "read_file", "create_witness_test"]); for denied in [ "write_file", "edit_file", @@ -543,6 +567,83 @@ mod tests { assert!(root.path().join("tests/real_witness.rs").exists()); } + /// #1792's witness: the blind author can discover the tree by name — + /// `glob` is offered and executes root-confined, and its results pass + /// the same credential exclusion as the read path, so discovery never + /// becomes a map of the workspace's secrets. + #[tokio::test] + async fn glob_lets_the_author_discover_tests_without_leaking_credentials() { + let root = tempfile::tempdir().unwrap(); + std::fs::create_dir(root.path().join("tests")).unwrap(); + std::fs::write(root.path().join("tests/existing.rs"), "#[test] fn t() {}").unwrap(); + std::fs::write(root.path().join(".env.local"), "secret").unwrap(); + let tools = witness_executor(root.path()).await; + + let output = tools + .execute("glob", &serde_json::json!({"pattern": "**/*"})) + .await; + let ToolOutput::Ok { content } = output else { + panic!("glob must be available to the witness author: {output:?}"); + }; + assert!( + content.contains("tests/existing.rs"), + "discovery must surface the test directory: {content}" + ); + assert!( + !content.contains(".env.local"), + "credential paths must not appear in discovery output: {content}" + ); + + let escaped = tools + .execute( + "glob", + &serde_json::json!({"pattern": "*", "path": "../.."}), + ) + .await; + assert!( + escaped.is_error(), + "an escaping search path must be refused" + ); + } + + /// #1784's witness: the operator's tool policy governs the witness + /// author's reads exactly as it governs the worker's. The executor takes + /// whatever read surface it is handed — the candidate workspace hands it + /// the policy-wrapped one — so a `read_file: off` switch removes both + /// the schema and the dispatch here, not just on the worker path. + #[tokio::test] + async fn the_tool_policy_reaches_the_witness_authors_reads() { + let root = tempfile::tempdir().unwrap(); + std::fs::create_dir(root.path().join("tests")).unwrap(); + std::fs::write(root.path().join("src.rs"), "source").unwrap(); + let registry: Arc = Arc::new( + ToolRegistry::new_detected(root.path().to_path_buf(), RegistryOptions::default()).await, + ); + let policied: Arc = Arc::new(crate::agent::PolicyToolSet::new_owned( + registry, + stella_tools::policy::ToolPolicy::from_switches([("read_file".to_string(), false)]), + )); + let tools = WitnessToolExecutor::new(root.path().to_path_buf(), policied); + + let names: Vec<_> = tools + .schemas() + .into_iter() + .map(|schema| schema.name) + .collect(); + assert_eq!( + names, + vec!["glob", "create_witness_test"], + "a switched-off read_file must not be offered to the author (glob stays)" + ); + assert!( + tools + .execute("read_file", &serde_json::json!({"path": "src.rs"})) + .await + .is_error(), + "and must not execute either" + ); + } + #[cfg(unix)] #[tokio::test] async fn refuses_a_terminal_symlink() { diff --git a/crates/stella-core/src/goal.rs b/crates/stella-core/src/goal.rs index f8c60e2d6..dcde43084 100644 --- a/crates/stella-core/src/goal.rs +++ b/crates/stella-core/src/goal.rs @@ -155,7 +155,11 @@ pub fn goal_round_turn_offset(round: usize) -> u32 { u32::try_from(round.saturating_sub(1).saturating_mul(2)).unwrap_or(u32::MAX) } -const VERIFIER_SYSTEM_PROMPT: &str = "You are an impartial verifier assessing whether a coding agent \ +/// Public so the CLI's goal loop can pin its tool allowlist against the six +/// tools this prompt names (#1783): the prompt and the offered surface must +/// not drift apart, and the test that enforces that lives beside the +/// executor it guards. +pub const VERIFIER_SYSTEM_PROMPT: &str = "You are an impartial verifier assessing whether a coding agent \ has fully met a stated goal. Judge from EVIDENCE, never from claims: use your read-only \ tools (read_file, grep, glob, explorations, ci_status, search_issues) to verify the work \ directly whenever the transcript alone is not conclusive — read the changed files, check \ diff --git a/crates/stella-pipeline/src/pipeline.rs b/crates/stella-pipeline/src/pipeline.rs index 1d8ec7471..053d92878 100644 --- a/crates/stella-pipeline/src/pipeline.rs +++ b/crates/stella-pipeline/src/pipeline.rs @@ -161,10 +161,6 @@ const CONVERSATIONAL_SYSTEM_PROMPT: &str = "You are Stella, a careful software e no test. Do not invent a task. If it fits, add one short line inviting \ them to describe a change, bug, or question about their codebase."; -/// Small fixed system prompt for the independent witness author. -const WITNESS_SYSTEM_PROMPT: &str = "You are a precise test author. You write minimal failing tests that pin down intended \ - behavior. You never modify production code and never fix the problem yourself."; - /// Per-role request overrides for the pipeline's raw completion calls /// (triage / verifier / guidance), resolved by the caller from /// `agent_engine_config`. Every field is optional and falls through to the @@ -183,10 +179,13 @@ pub struct RoleCallOverrides { pub params: Option, } -/// The pipeline's per-role override set. Worker (and plan/witness, which -/// ride the worker's tier) is configured through -/// [`PipelineConfig::engine`] directly; only the two roles with their own -/// models get their own request shaping. +/// The pipeline's per-role override set. Worker (and plan, which rides the +/// worker's tier) is configured through [`PipelineConfig::engine`] directly; +/// only the two roles with their own models get their own request shaping. +/// The witness author/repair engines ride the verifier's model, so they take +/// the `verifier` row's shaping too (#1785) — everything except `prompt`, +/// which stays scoped to the raw verdict/guidance calls +/// (`Pipeline::witness_engine_config` says why). #[derive(Debug, Clone, Default)] pub struct PipelineRoleOverrides { pub triage: RoleCallOverrides, @@ -2871,6 +2870,7 @@ impl<'a> Pipeline<'a> { // than being forwarded (§4.3). let feedback = self .airlock_forward(&verdict.reasoning, "verifier_reasoning", &sealed) + .map(|text| crate::verify::bound_forwarded_reasoning(&text)) .unwrap_or_else(|| redact(&sealed, DisclosureGrain::Symptom).message()); if let Err(abort) = self .revise_candidate(engine, surface, budget, &feedback, total, &mut state) diff --git a/crates/stella-pipeline/src/pipeline/witness_stage.rs b/crates/stella-pipeline/src/pipeline/witness_stage.rs index c0815c4aa..f1b7a8b80 100644 --- a/crates/stella-pipeline/src/pipeline/witness_stage.rs +++ b/crates/stella-pipeline/src/pipeline/witness_stage.rs @@ -7,7 +7,7 @@ use super::*; -use crate::witness::{RUNNER_VOCABULARY, runner_probe}; +use crate::witness::{RUNNER_VOCABULARY, WITNESS_SYSTEM_PROMPT, runner_probe}; /// Which of the closed vocabulary's runners this workspace can actually /// spawn (#1539) — the fact the capability-minimal author cannot discover @@ -33,6 +33,30 @@ async fn runner_availability(tests: &dyn TestRunner) -> Vec { .collect() } +/// Overlay one role's request shaping onto an engine config — the pure half +/// of [`Pipeline::witness_engine_config`], so the field-by-field flow is +/// testable without a pipeline. Only the knobs `EngineConfig` itself carries +/// participate; `RoleCallOverrides::prompt` is a raw-call concern and is +/// deliberately absent (see the caller's doc for why). +fn apply_role_shaping(mut config: EngineConfig, overrides: &RoleCallOverrides) -> EngineConfig { + if let Some(effort) = overrides.effort { + config.effort = Some(effort); + } + if let Some(reasoning) = overrides.reasoning { + config.reasoning = Some(reasoning); + } + if let Some(temperature) = overrides.temperature { + config.temperature = Some(temperature); + } + if let Some(max_output_tokens) = overrides.max_output_tokens { + config.max_output_tokens = Some(max_output_tokens); + } + if let Some(params) = &overrides.params { + config.params = Some(params.clone()); + } + config +} + /// Candidate-bound hook execution: both the hook process and the engine's /// payload use the isolated root, never the session root. pub(super) struct BoundHookRunner<'a> { @@ -136,6 +160,27 @@ impl<'a> Pipeline<'a> { }); } + /// The witness author/repair engine tuning (#1785): the worker's engine + /// config rebased into the authoring snapshot, with the VERIFIER's + /// request shaping applied on top. The witness author rides the + /// verifier's model (`Role::Verifier`, independence-filtered), so an + /// operator tuning `agents.verifier.effort` or `.temperature` reasonably + /// expects the witness author to honor it — before this, those knobs + /// reached the verdict and guidance calls while the author silently ran + /// on the worker's tuning. + /// + /// `RoleCallOverrides::prompt` is deliberately NOT applied here: it is + /// operator prose written against the reviewer instructions, and + /// injecting it into a test-authoring engine would steer the wrong role. + /// It stays scoped to the raw verdict/guidance calls + /// (`metered_raw_call`). + pub(super) fn witness_engine_config(&self, surface: CandidateSurface<'_>) -> EngineConfig { + apply_role_shaping( + self.engine_config_for(surface), + &self.config.role_overrides.verifier, + ) + } + pub(super) fn engine_config_for(&self, surface: CandidateSurface<'_>) -> EngineConfig { let mut config = self.config.engine.clone(); if let Some(cwd) = surface.cwd { @@ -227,7 +272,7 @@ impl<'a> Pipeline<'a> { let mut engine = Engine::with_sleeper( author.provider, witness_tools, - self.engine_config_for(baseline), + self.witness_engine_config(baseline), self.sleeper, ) .with_call_role(stella_protocol::ModelCallRole::WitnessAuthor); @@ -335,7 +380,7 @@ impl<'a> Pipeline<'a> { let mut repair_engine = Engine::with_sleeper( author.provider, witness_tools, - self.engine_config_for(baseline), + self.witness_engine_config(baseline), self.sleeper, ) .with_call_role(stella_protocol::ModelCallRole::WitnessRepair); @@ -589,3 +634,40 @@ impl<'a> Pipeline<'a> { Ok(Some(witness)) } } + +#[cfg(test)] +mod tests { + use super::*; + + /// #1785's witness: the verifier's request shaping reaches the witness + /// engines field by field, `None` overrides leave the worker's values + /// standing, and `prompt` has no channel here at all (it is not an + /// `EngineConfig` knob — the type system keeps it raw-call-scoped). + #[test] + fn verifier_shaping_overlays_the_worker_engine_config() { + let mut worker = EngineConfig::default(); + worker.temperature = Some(0.7); + worker.max_output_tokens = Some(1000); + worker.effort = None; + + let shaped = apply_role_shaping( + worker.clone(), + &RoleCallOverrides { + temperature: Some(0.1), + effort: Some(stella_protocol::ReasoningEffort::High), + ..RoleCallOverrides::default() + }, + ); + assert_eq!(shaped.temperature, Some(0.1)); + assert_eq!(shaped.effort, Some(stella_protocol::ReasoningEffort::High)); + assert_eq!( + shaped.max_output_tokens, + Some(1000), + "an absent override must leave the worker's value standing" + ); + + let untouched = apply_role_shaping(worker.clone(), &RoleCallOverrides::default()); + assert_eq!(untouched.temperature, worker.temperature); + assert_eq!(untouched.max_output_tokens, worker.max_output_tokens); + } +} diff --git a/crates/stella-pipeline/src/verify.rs b/crates/stella-pipeline/src/verify.rs index bd5367e2e..d970a6175 100644 --- a/crates/stella-pipeline/src/verify.rs +++ b/crates/stella-pipeline/src/verify.rs @@ -879,6 +879,31 @@ pub fn parse_verifier_response(text: &str) -> Option { None } +/// Ceiling on verifier prose forwarded into a worker's revision prompt. +/// +/// `Verdict::reasoning` is the model's whole reply and has no length +/// contract; on a FAIL it becomes the revision reason, and an unbounded +/// reply would ride into every subsequent turn of the conversation. The +/// trusted evidence summary and the diff are budgeted — the one +/// model-authored blob crossing to the worker should not be the exception. +/// Head-kept: the verdict protocol puts the verdict and its core reason +/// first, so the head is the load-bearing part. +pub const FORWARDED_REASONING_MAX_CHARS: usize = 4_000; + +/// Bound one piece of verifier prose for forwarding to the worker. A +/// char-boundary-safe head truncation with an explicit marker, so the worker +/// reads "there was more" rather than a sentence that stops mid-claim. +pub fn bound_forwarded_reasoning(text: &str) -> String { + if text.len() <= FORWARDED_REASONING_MAX_CHARS { + return text.to_string(); + } + let mut end = FORWARDED_REASONING_MAX_CHARS; + while !text.is_char_boundary(end) { + end -= 1; + } + format!("{}\n[verifier reasoning truncated]", &text[..end]) +} + /// The conservative heuristic verdict used when the *verifier model call itself* /// fails or its response is unparseable (L-E11: "a heuristic fallback verdict /// if the verifier call itself fails"). It never fabricates confidence: it diff --git a/crates/stella-pipeline/src/verify/tests.rs b/crates/stella-pipeline/src/verify/tests.rs index 1b98bbff7..218b2a9b8 100644 --- a/crates/stella-pipeline/src/verify/tests.rs +++ b/crates/stella-pipeline/src/verify/tests.rs @@ -759,6 +759,30 @@ fn a_negated_pass_is_the_fail_it_states() { ); } +/// #1787 (the bounded half): verifier prose forwarded into a revision +/// prompt has a ceiling. An unbounded reply rode into every subsequent +/// turn of the conversation; the head is kept because the verdict protocol +/// puts the verdict and its core reason first. +#[test] +fn forwarded_reasoning_is_bounded_with_an_explicit_marker() { + let short = "FAIL — the parser drops the last field"; + assert_eq!(bound_forwarded_reasoning(short), short); + + let long = "x".repeat(FORWARDED_REASONING_MAX_CHARS + 500); + let bounded = bound_forwarded_reasoning(&long); + assert!(bounded.len() < long.len()); + assert!( + bounded.ends_with("[verifier reasoning truncated]"), + "{bounded}" + ); + + // Truncation lands on a char boundary even when the ceiling splits a + // multi-byte character. + let multibyte = "é".repeat(FORWARDED_REASONING_MAX_CHARS); + let bounded = bound_forwarded_reasoning(&multibyte); + assert!(bounded.ends_with("[verifier reasoning truncated]")); +} + #[test] fn unparseable_verifier_response_is_none() { assert_eq!(parse_verifier_response("hmm, hard to say"), None); diff --git a/crates/stella-pipeline/src/witness.rs b/crates/stella-pipeline/src/witness.rs index 7df9e3bee..3125f8029 100644 --- a/crates/stella-pipeline/src/witness.rs +++ b/crates/stella-pipeline/src/witness.rs @@ -627,11 +627,57 @@ pub fn runner_probe(program: &str) -> Option { }) } -/// The witness author's task prompt: split context exactly like the planner -/// (goal + recall + repo structure, never the worker transcript — L-E6). The -/// hard requirements — new file only, must fail now, no production edits, -/// marker line — are the parts [`parse_witness_command`] and the pipeline's -/// fail-check enforce mechanically; the prose is guidance. +/// The witness author's fixed system prompt (#1786): the role and every hard +/// requirement, byte-identical for the life of the process — the same +/// contract as `VERIFIER_INSTRUCTIONS` (#1434). This block used to open the +/// *user* message, ahead of the volatile sections, which re-billed the whole +/// instruction set uncached on every author call, every repair call, and +/// every tool round-trip inside them — the one verification role that runs a +/// multi-step tool loop was the one paying full freight. As the system +/// message it rides the cache-markable stable prefix (#1474); everything +/// per-call stays in [`witness_prompt`]. +pub const WITNESS_SYSTEM_PROMPT: &str = "You are the WITNESS AUTHOR for a coding agent: a precise test author who writes a \ + minimal test that FAILS on the current code and will PASS once the goal you are \ + given is correctly accomplished. The fail→pass flip of your test is what verifies \ + the work. You never modify production code and never fix the problem yourself.\n\n\ + Hard requirements:\n\ + - Create ONE NEW test file. Never modify existing files, and never touch \ + production code — the implementation is someone else's job.\n\ + - CHOOSE A RUNNER THIS REPOSITORY ALREADY USES. You cannot execute anything in \ + this role, so you cannot discover a missing toolchain — and a command whose \ + runner is not installed does not fail the test, it produces NO observation at \ + all, which discards your witness and leaves the work unverified. Pick the \ + ecosystem the repository listing evidences (a manifest such as \ + `Cargo.toml`, `package.json`, `pyproject.toml`/`setup.py`, `go.mod`, or a \ + `*.csproj`, plus existing tests written for it) and match the conventions of \ + the tests already there. If the listing evidences no test runner at all, say \ + so in prose and emit no TEST_COMMAND line rather than guessing one.\n\ + - Put it where that runner collects it. Rust integration tests MUST \ + live in `tests/` (cargo cannot run a test file under `src/`); Python, Vitest, Go \ + and .NET may use their filename conventions.\n\ + - The test must fail NOW for the RIGHT reason (it exercises the missing/broken \ + behavior), not because of a typo, a missing import, or a harness error.\n\ + - ASSERT on a value the goal decides. A test with no assertions, one comparing \ + constants (`assert_eq!(2, 2)`), one comparing a value to itself, or a bare \ + `#[should_panic]` / `raises(Exception)` is REFUSED at creation — each of those \ + flips green without constraining the change. Name the expected panic if a panic \ + is what you mean to prove.\n\ + - Explore with `read_file` and `glob` (names only) to find the test directories \ + and conventions; create with `create_witness_test`. No general write, edit, \ + process, network, or external action is available in this role.\n\ + - The command must directly name this artifact and an exact test: for Rust use \ + `cargo test --test -- --exact`; for Python/Vitest name the \ + file path; for Go/.NET include an exact test filter. Never run a whole suite.\n\ + - End your reply with exactly one line:\n\ + TEST_COMMAND: "; + +/// The witness author's per-call user prompt: split context exactly like the +/// planner (goal + recall + repo structure, never the worker transcript — +/// L-E6). The fixed role and hard requirements ride separately as +/// [`WITNESS_SYSTEM_PROMPT`]; the parts the prompt promises — +/// new file only, must fail now, marker line — are what +/// [`parse_witness_command`] and the pipeline's fail-check enforce +/// mechanically, so the prose is guidance over an enforced contract. /// /// `available_runners` is the probed availability set (#1539): the runners /// the pipeline confirmed this workspace can actually spawn. The author's @@ -644,40 +690,7 @@ pub fn witness_prompt( repo_structure: &str, available_runners: &[String], ) -> String { - let mut s = String::from( - "You are the WITNESS AUTHOR for a coding agent. Write a witness test: a minimal \ - test that FAILS on the current code and will PASS once the goal below is correctly \ - accomplished. The fail→pass flip of your test is what verifies the work.\n\n\ - Hard requirements:\n\ - - Create ONE NEW test file. Never modify existing files, and never touch \ - production code — the implementation is someone else's job.\n\ - - CHOOSE A RUNNER THIS REPOSITORY ALREADY USES. You cannot execute anything in \ - this role, so you cannot discover a missing toolchain — and a command whose \ - runner is not installed does not fail the test, it produces NO observation at \ - all, which discards your witness and leaves the work unverified. Pick the \ - ecosystem the repository listing below evidences (a manifest such as \ - `Cargo.toml`, `package.json`, `pyproject.toml`/`setup.py`, `go.mod`, or a \ - `*.csproj`, plus existing tests written for it) and match the conventions of \ - the tests already there. If the listing evidences no test runner at all, say \ - so in prose and emit no TEST_COMMAND line rather than guessing one.\n\ - - Put it where that runner collects it. Rust integration tests MUST \ - live in `tests/` (cargo cannot run a test file under `src/`); Python, Vitest, Go \ - and .NET may use their filename conventions.\n\ - - The test must fail NOW for the RIGHT reason (it exercises the missing/broken \ - behavior), not because of a typo, a missing import, or a harness error.\n\ - - ASSERT on a value the goal decides. A test with no assertions, one comparing \ - constants (`assert_eq!(2, 2)`), one comparing a value to itself, or a bare \ - `#[should_panic]` / `raises(Exception)` is REFUSED at creation — each of those \ - flips green without constraining the change. Name the expected panic if a panic \ - is what you mean to prove.\n\ - - Use `create_witness_test`; no general write, edit, process, network, or external \ - action is available in this role.\n\ - - The command must directly name this artifact and an exact test: for Rust use \ - `cargo test --test -- --exact`; for Python/Vitest name the \ - file path; for Go/.NET include an exact test filter. Never run a whole suite.\n\ - - End your reply with exactly one line:\n\ - TEST_COMMAND: \n", - ); + let mut s = String::new(); if !available_runners.is_empty() { // Probed fact, not inference (#1539): the pipeline spawned each // vocabulary runner's version probe in this very workspace. The @@ -713,7 +726,9 @@ pub fn witness_prompt( } s.push_str("\n## Goal\n"); s.push_str(goal.trim()); - s + // The first section opens with its own separating newline; with the + // fixed block gone to the system message there is nothing above it. + s.trim_start().to_string() } /// The one bounded repair retry (the L-V2 pattern): the authored test passed @@ -1280,16 +1295,36 @@ mod tests { content_digest: None, }]; let p = witness_prompt("fix the retry bug", &recall, "src/\n lib.rs", &[]); - assert!(p.contains("TEST_COMMAND:")); assert!(p.contains("fix the retry bug")); assert!(p.contains("src/")); assert!(p.contains("memory: retries")); - assert!(p.contains("ONE NEW test file")); - // The density screen refuses at creation (#863), so the prompt has to - // state the rule — a refusal the author could not have anticipated - // costs the same round trip the screen exists to save. - assert!(p.contains("assert_eq!(2, 2)"), "{p}"); - assert!(p.contains("REFUSED"), "{p}"); + // The fixed half rides the system message (#1786), never the + // volatile user prompt — repeating it here would re-bill it. + assert!(!p.contains("Hard requirements"), "{p}"); + assert!( + p.trim_start().len() == p.len(), + "no dangling blank opening: {p:?}" + ); + } + + /// The fixed system block carries the whole enforced contract: the + /// marker line, the one-new-file rule, and the density-screen rule + /// (#863) — a refusal the author could not have anticipated costs the + /// same round trip the screen exists to save. + #[test] + fn the_system_prompt_carries_the_hard_requirements() { + for required in [ + "TEST_COMMAND:", + "ONE NEW test file", + "assert_eq!(2, 2)", + "REFUSED", + "never modify production code", + ] { + assert!( + WITNESS_SYSTEM_PROMPT.contains(required), + "missing {required:?}" + ); + } } /// The author has `read_file` and one create — no execution at all — so it @@ -1299,22 +1334,21 @@ mod tests { /// that choosing a runner is a decision made from evidence, not a default. #[test] fn the_author_is_told_it_cannot_probe_and_must_pick_an_evidenced_runner() { - let p = witness_prompt("add a parser", &[], "go.mod\nmain.go", &[]); assert!( - p.contains("CHOOSE A RUNNER THIS REPOSITORY ALREADY USES"), - "{p}" + WITNESS_SYSTEM_PROMPT.contains("CHOOSE A RUNNER THIS REPOSITORY ALREADY USES"), + "{WITNESS_SYSTEM_PROMPT}" ); assert!( - p.contains("cannot execute anything in this role"), - "the author must know its own blindness, not just the rule: {p}" + WITNESS_SYSTEM_PROMPT.contains("cannot execute anything in this role"), + "the author must know its own blindness, not just the rule" ); assert!( - p.contains("NO observation at all"), - "the cost has to be named — a missing runner is not a failing test: {p}" + WITNESS_SYSTEM_PROMPT.contains("NO observation at all"), + "the cost has to be named — a missing runner is not a failing test" ); assert!( - p.contains("emit no TEST_COMMAND line rather than guessing"), - "with no evidenced runner, abstaining beats a fabricated command: {p}" + WITNESS_SYSTEM_PROMPT.contains("emit no TEST_COMMAND line rather than guessing"), + "with no evidenced runner, abstaining beats a fabricated command" ); } diff --git a/crates/stella-protocol/src/event.rs b/crates/stella-protocol/src/event.rs index e053c846a..1dc72aad6 100644 --- a/crates/stella-protocol/src/event.rs +++ b/crates/stella-protocol/src/event.rs @@ -113,13 +113,15 @@ pub enum StageKind { Plan, /// The interactive approval gate a large plan passes through (L-E5). ScopeReview, - /// Witness authoring: before the worker executes, an independent model - /// (the verifier's resolution, never the worker's transcript) writes the - /// witness test — a test that FAILS on the current code and will pass - /// once the goal is met — arming the deterministic flip oracle (L-E11). - /// The witness is visible to the worker (iterating against a failing - /// test is where convergence comes from); integrity comes from tamper - /// exclusion at verify time, not from hiding the test. + /// Witness authoring: after the worker executes — once the warrant has + /// read the diff and found something worth proving — an independent + /// model (the verifier's resolution, never the worker's transcript) + /// writes the witness test in a pristine snapshot of the pre-execution + /// tree: a test that FAILS there and will pass once the goal is met, + /// arming the deterministic flip oracle (L-E11). The witness is visible + /// to the worker's revise turns (iterating against a failing test is + /// where convergence comes from); integrity comes from tamper exclusion + /// at verify time, not from hiding the test. Witness, /// The worker's own tool-calling loop — the steps that actually change /// the workspace. diff --git a/docs/wire/agentevent.schema.json b/docs/wire/agentevent.schema.json index 9b9ab46a6..675c29b7a 100644 --- a/docs/wire/agentevent.schema.json +++ b/docs/wire/agentevent.schema.json @@ -1281,7 +1281,7 @@ }, { "const": "witness", - "description": "Witness authoring: before the worker executes, an independent model\n(the verifier's resolution, never the worker's transcript) writes the\nwitness test — a test that FAILS on the current code and will pass\nonce the goal is met — arming the deterministic flip oracle (L-E11).\nThe witness is visible to the worker (iterating against a failing\ntest is where convergence comes from); integrity comes from tamper\nexclusion at verify time, not from hiding the test.", + "description": "Witness authoring: after the worker executes — once the warrant has\nread the diff and found something worth proving — an independent\nmodel (the verifier's resolution, never the worker's transcript)\nwrites the witness test in a pristine snapshot of the pre-execution\ntree: a test that FAILS there and will pass once the goal is met,\narming the deterministic flip oracle (L-E11). The witness is visible\nto the worker's revise turns (iterating against a failing test is\nwhere convergence comes from); integrity comes from tamper exclusion\nat verify time, not from hiding the test.", "type": "string" }, { diff --git a/docs/wire/serveframe.schema.json b/docs/wire/serveframe.schema.json index fcbc44a34..6b72cd05a 100644 --- a/docs/wire/serveframe.schema.json +++ b/docs/wire/serveframe.schema.json @@ -2882,7 +2882,7 @@ }, { "const": "witness", - "description": "Witness authoring: before the worker executes, an independent model\n(the verifier's resolution, never the worker's transcript) writes the\nwitness test — a test that FAILS on the current code and will pass\nonce the goal is met — arming the deterministic flip oracle (L-E11).\nThe witness is visible to the worker (iterating against a failing\ntest is where convergence comes from); integrity comes from tamper\nexclusion at verify time, not from hiding the test.", + "description": "Witness authoring: after the worker executes — once the warrant has\nread the diff and found something worth proving — an independent\nmodel (the verifier's resolution, never the worker's transcript)\nwrites the witness test in a pristine snapshot of the pre-execution\ntree: a test that FAILS there and will pass once the goal is met,\narming the deterministic flip oracle (L-E11). The witness is visible\nto the worker's revise turns (iterating against a failing test is\nwhere convergence comes from); integrity comes from tamper exclusion\nat verify time, not from hiding the test.", "type": "string" }, { diff --git a/website/content/docs/inference-pipeline.mdx b/website/content/docs/inference-pipeline.mdx index f43656616..003171b56 100644 --- a/website/content/docs/inference-pipeline.mdx +++ b/website/content/docs/inference-pipeline.mdx @@ -37,7 +37,8 @@ in stages: In full: triage → recall → plan → scope review → execute → witness → verify -→ verifier, with a bounded revise loop. Recall runs concurrently with triage +→ verdict, with a bounded revise loop, then reflection and context +write-back after the verdict settles. Recall runs concurrently with triage but emits after it; plan, scope review, and witness are conditional — a simple lookup skips them, scope review fires only above thresholds, and the witness is authored only without `--test-command`, on demand *after* @@ -122,6 +123,36 @@ that merely restates the patch; its test must fail there first, and its command then arms the flip oracle. Witness files are watched for tampering: a worker that edits the witness test doesn't get credit for "passing" it. +**What the author can actually do.** The witness author is the most +capability-restricted role in the pipeline, and the restriction is enforced +by its executor, not by prompt. It gets exactly three tools: `read_file` +and `glob` (both confined to the candidate root, with credential and +private-state paths excluded from reads and from discovery results), and +`create_witness_test` — an atomic, exclusive, symlink-refusing create that +holds a single-artifact claim: at most one witness file ever exists, and a +repeat create replaces the author's own previous artifact (which is what +lets the one bounded repair turn genuinely rewrite a test that passed too +early). Every other tool name — the shell, writes, network, MCP — returns +an error. The accepted artifact's full filesystem identity is pinned where +it was written, grafted into the executed candidate as a byte copy, and +re-pinned there; that second pin is what tamper exclusion checks for the +rest of the run. + +**Why a warranted witness can still be missing.** Authoring can decline +without failing the run — the change is already done by the time this +stage runs, so a witness that cannot be *produced* degrades to an honest +`Unverified` with a stated reason, while a witness that cannot be +*trusted* (a tracked-file edit, a wrong or multiple-file artifact, an +identity mismatch) still stops the candidate. The degradable reasons you +will see in a transcript: no supported test runner answered its version +probe (the pipeline probes the runner vocabulary before spending an +author call, and enforces the probed set against the authored command); +the author produced no `TEST_COMMAND` line or an unusable one; the +authored test was refused at creation for asserting nothing the goal +decides (the assertion-density screen — no assertions, constant +comparisons, and bare catch-all panic checks never reach disk); or the +test still passed on the unmodified code after the one repair. + The witness is scaffolding for one run, not a test you inherit. It is written to *fail* — it encodes a moment ("this code doesn't do X yet"), not an invariant, which is the opposite of what a durable regression test encodes. So @@ -203,6 +234,12 @@ the diff, and the evidence summary, and answers with a leading `PASS` or changed. If the verifier call fails, stella falls back to a conservative heuristic verdict rather than hanging or guessing. +One wire-level note for stream consumers: the distress-guidance call — +the course-correction a stuck worker gets after repeated deterministic +failures — is served by the verifier role and emits under the same +`verdict` stage name, though it is advice rather than a verdict. Filter on +the call's role (`distress_guidance`) if the distinction matters to you. + ### 8. Revise A failed verification sends the worker back with the failure evidence — up to