From 4a669ce5da352dac68cceb942c5c4d25f90efea1 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 15 Sep 2026 00:02:32 +0800 Subject: [PATCH 1/2] fix(fork): route the stopship scout through tool_search and grep_files The read-only scout brief commanded a `File` call with `action` `search_content`, but `File` is a hidden replay-only alias (`model_visible=false`): it never reaches a model-visible catalog, and the child step loop fails any call outside the child's policy-filtered catalog outright. The release-acceptance explore gate's first step therefore could not succeed; dispatch-by-alias only worked below the catalog gate (replay tests calling `registry.resolve` directly). Route the scout through the live surface instead: response 1 activates the deferred `grep_files` with one `tool_search` call, response 2 runs the same alternation search through `grep_files` (path/include/pattern/max_results/context_lines), and response 3 returns the verdict. The step and token caps are unchanged; the evidence turn grows one small activation response. Guard both halves so neither side silently drifts again: one forkguard test pins the scout surface to an active `tool_search` plus a deferred, searchable `grep_files` with no `File`, and another pins the fixture definition to catalog-visible tool names only. Signed-off-by: asto --- crates/tui/src/tools/subagent/tests.rs | 69 ++++++++++++++++++++++++++ workflows/stopship.workflow.js | 7 ++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 294f1ba33d..46cbd47318 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21016,3 +21016,72 @@ async fn agent_claim_is_withheld_from_a_role_with_no_write_authority() { .to_string(); assert!(refusal.contains("no write authority to widen"), "{refusal}"); } + +// Regression (stopship scout repair): the workflow's read-only scout +// activates `grep_files` with one `tool_search` call before searching. That +// two-step path only works while the scout surface keeps a first-turn-active +// `tool_search` and a deferred, searchable `grep_files`; the hidden `File` +// alias the brief cited before is filtered from every model-visible catalog +// (`to_api_tools`), which is what silently broke the release-acceptance +// explore gate. If a surface reshape fails this test, re-work the fixture +// brief in the same change instead of leaving it instructing calls the child +// cannot make. +#[tokio::test] +async fn forkguard_scout_surface_keeps_tool_search_grep_files_activation_path() { + let tmp = tempdir().expect("tempdir"); + let mut runtime = + stub_runtime().with_agent_tool_surface_options(enabled_agent_surface_options()); + runtime.context = ToolContext::new(tmp.path().to_path_buf()); + let todo_list = crate::tools::todo::new_shared_todo_list(); + let plan_state = crate::tools::plan::new_shared_plan_state(); + let registry = + SubAgentToolRegistry::new(runtime, FleetRole::Scout, None, todo_list, plan_state); + + let catalog = registry.deferred_catalog_for_model(&FleetRole::Scout); + let search = catalog + .iter() + .find(|tool| tool.name == "tool_search") + .expect("scout must keep an active tool_search"); + let grep = catalog + .iter() + .find(|tool| tool.name == "grep_files") + .expect("scout must keep a deferred, searchable grep_files"); + assert_eq!( + search.defer_loading, + Some(false), + "tool_search must be first-turn active on the scout" + ); + assert_eq!( + grep.defer_loading, + Some(true), + "grep_files must be deferred so the taught tool_search activation is required" + ); + assert!( + !catalog.iter().any(|tool| tool.name == "File"), + "the hidden File alias must stay out of scout catalogs" + ); +} + +// Regression (stopship scout repair, Pinvou #490 phantom-tool class): fleet +// workflow briefs are model-facing text too. The scout brief must name tools +// the child catalog can actually see (`tool_search`, `grep_files`) and never +// cite the hidden `File` alias or its retired `search_content` action — that +// wording failed the release-acceptance explore gate exactly the way the +// bundled-skills `File` citations stalled real reasoning loops. +#[test] +fn forkguard_workflow_briefs_name_catalog_visible_tools() { + const STOPSHIP: &str = include_str!("../../../../../workflows/stopship.workflow.js"); + // Guard the fixture definition, not the maintainer header comment. + let body = STOPSHIP + .split_once("export default") + .expect("workflow module must export its definition") + .1; + assert!( + body.contains("`tool_search`") && body.contains("`grep_files`"), + "the scout brief must teach the two-step activation path:\n{body}" + ); + assert!( + !body.contains("`File`") && !body.contains("search_content"), + "workflow briefs must not command calls a catalog can never return:\n{body}" + ); +} diff --git a/workflows/stopship.workflow.js b/workflows/stopship.workflow.js index 61654f7991..a949051a34 100644 --- a/workflows/stopship.workflow.js +++ b/workflows/stopship.workflow.js @@ -6,6 +6,11 @@ // tokens. Budget 24k per intended evidence turn, then add token-neutral // max_steps headroom for the required final verdict. The token ceiling // remains independent and the five role caps still total 360k. +// 2026-09-14 scout repair: the read-only catalog carries no `File` (hidden +// replay-only alias, filtered from every model-visible surface), so the scout +// now activates `grep_files` with one `tool_search` call before searching. +// The evidence turn spans three responses (activate, search, verdict) inside +// the unchanged step/token caps. export default workflow({ "id": "stopship-release-acceptance", "goal": "Verify the current Codewhale Fleet, Workflow, Lane, Runtime, and gate receipt path without changing the workspace", @@ -74,7 +79,7 @@ export default workflow({ { "agent": { "id": "explore-runtime", - "prompt": "Verify the runtime release-orchestration owners using only the five files in File scope. The host's typed run_started receipt already owns the compiled Workflow id and source path; do not re-verify the Workflow alias. You have at most six model responses and must reserve the verdict. Response 1 must make exactly one `File` call with `action` set to `search_content`, `path` set to `.`, and `include` set exactly to [`fleets/stopship.toml`, `crates/cli/src/lib.rs`, `crates/workflow/src/role_resolve.rs`, `crates/tui/src/tools/workflow.rs`, `crates/lane/src/runtime.rs`], using this high-signal alternation pattern: `name = \"stopship\"|load_named_fleet|start_lane|resolve_workflow_agent|record_task_started|WorkflowUiEventKind::GateUpdated|WorkflowUiEventKind::RunCompleted|terminal_completed_receipt|process_exit_receipt|lane_reconciled|tmux_reconcile_folds_detached_process_exit_into_lane_status|stopship_acceptance_fixture_emits_role_gate_and_terminal_receipts`. Set at most 80 results and 2 context lines. Matches outside that exact include list do not count. Do not add generic field names such as `resolved_profile` or `exit_code` to the pattern. Do not call `File` more than once and do not use any action except `search_content` or call any other tool. Response 2 must return the verdict with no tool calls; any later reserved response must do the same instead of gathering more evidence. Treat an exact match naming a call site, typed event constructor, reconciliation branch, or test assertion in a scoped file as source-owner evidence. Apply this decision rule literally: if you can populate all seven required SOURCE EVIDENCE entries from the search result, return APPROVE; never return BLOCK after citing all seven. Return BLOCK only when at least one named owner has no matching citation, and identify each missing owner as MISSING. The first non-empty line of your response must be exactly APPROVE or exactly BLOCK. Do not put any words before that verdict: no confirmation, summary, heading, or phrase such as `Here is the verdict`. After the verdict, include a `SOURCE EVIDENCE` section with concise `path: symbol` evidence for named Fleet loading, role-to-profile resolution, tmux Lane launch, typed task_started, gate_updated, terminal run_completed, and tmux process-exit reconciliation receipts. The terminal run_completed entry must carry both the `WorkflowUiEventKind::RunCompleted` constructor and the `terminal_completed_receipt` integration assertion. A bare verdict is invalid. Do not edit files, create branches, run shell commands, access GitHub, or infer success where source evidence is absent.", + "prompt": "Verify the runtime release-orchestration owners using only the five files in scope. The host's typed run_started receipt already owns the compiled Workflow id and source path; do not re-verify the Workflow alias. You have at most six model responses and must reserve the verdict. Response 1 must make exactly one `tool_search` call with `query` set to `grep_files`; the content-search tool is deferred, so this activation is required before it can be called. Response 2 must make exactly one `grep_files` call with `path` set to `.`, `include` set exactly to [`fleets/stopship.toml`, `crates/cli/src/lib.rs`, `crates/workflow/src/role_resolve.rs`, `crates/tui/src/tools/workflow.rs`, `crates/lane/src/runtime.rs`], and `pattern` set to this high-signal alternation: `name = \"stopship\"|load_named_fleet|start_lane|resolve_workflow_agent|record_task_started|WorkflowUiEventKind::GateUpdated|WorkflowUiEventKind::RunCompleted|terminal_completed_receipt|process_exit_receipt|lane_reconciled|tmux_reconcile_folds_detached_process_exit_into_lane_status|stopship_acceptance_fixture_emits_role_gate_and_terminal_receipts`. Set `max_results` to 80 and `context_lines` to 2. Matches outside that exact include list do not count. Do not add generic field names such as `resolved_profile` or `exit_code` to the pattern. Make no other tool calls: exactly one `tool_search` call, then exactly one `grep_files` call, nothing else. Response 3 must return the verdict with no tool calls; any later reserved response must do the same instead of gathering more evidence. Treat an exact match naming a call site, typed event constructor, reconciliation branch, or test assertion in a scoped file as source-owner evidence. Apply this decision rule literally: if you can populate all seven required SOURCE EVIDENCE entries from the search result, return APPROVE; never return BLOCK after citing all seven. Return BLOCK only when at least one named owner has no matching citation, and identify each missing owner as MISSING. The first non-empty line of your response must be exactly APPROVE or exactly BLOCK. Do not put any words before that verdict: no confirmation, summary, heading, or phrase such as `Here is the verdict`. After the verdict, include a `SOURCE EVIDENCE` section with concise `path: symbol` evidence for named Fleet loading, role-to-profile resolution, tmux Lane launch, typed task_started, gate_updated, terminal run_completed, and tmux process-exit reconciliation receipts. The terminal run_completed entry must carry both the `WorkflowUiEventKind::RunCompleted` constructor and the `terminal_completed_receipt` integration assertion. A bare verdict is invalid. Do not edit files, create branches, run shell commands, access GitHub, or infer success where source evidence is absent.", "agent_type": "explore", "role": "explore", "mode": "read_only", From bf15241ce9191ed8d7135723922feeefd7652c37 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 15 Sep 2026 00:54:45 +0800 Subject: [PATCH 2/2] test(fork): align stopship guards with scout repair The review of #61 caught one real break and three guard-fidelity gaps: - js_authoring.rs still pinned the retired `File`/`search_content` scout wording, so the workspace suite and CI went red on this branch. Pin the new three-response contract instead. - The scout surface guard built its registry with no explicit scope, while the fixture scout really runs under the read-only lowering `["File"]`. Build it with that scope so the alias-family intersection that keeps grep_files discoverable is guarded too. - The brief guard only denied backticked `File` and bare `search_content`; deny the hidden replay aliases as whole words so an unbackticked citation cannot slip past. - Add the behavioral counterpart: one tool_search call must make grep_files dispatchable through execute_from_surface, and a File call must keep failing the catalog gate. Signed-off-by: asto --- crates/tui/src/tools/subagent/tests.rs | 119 +++++++++++++++++++++++-- crates/workflow/src/js_authoring.rs | 19 ++-- 2 files changed, 124 insertions(+), 14 deletions(-) diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 46cbd47318..ece9d4f91e 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21023,9 +21023,12 @@ async fn agent_claim_is_withheld_from_a_role_with_no_write_authority() { // `tool_search` and a deferred, searchable `grep_files`; the hidden `File` // alias the brief cited before is filtered from every model-visible catalog // (`to_api_tools`), which is what silently broke the release-acceptance -// explore gate. If a surface reshape fails this test, re-work the fixture -// brief in the same change instead of leaving it instructing calls the child -// cannot make. +// explore gate. The registry is built with the scope the fixture scout really +// runs under (`leaf_allowed_tools` lowers a read-only leaf to `["File"]`), so +// the guard also pins the alias-family intersection that keeps `grep_files` +// discoverable under that legacy rule. If a surface reshape fails this test, +// re-work the fixture brief in the same change instead of leaving it +// instructing calls the child cannot make. #[tokio::test] async fn forkguard_scout_surface_keeps_tool_search_grep_files_activation_path() { let tmp = tempdir().expect("tempdir"); @@ -21034,8 +21037,13 @@ async fn forkguard_scout_surface_keeps_tool_search_grep_files_activation_path() runtime.context = ToolContext::new(tmp.path().to_path_buf()); let todo_list = crate::tools::todo::new_shared_todo_list(); let plan_state = crate::tools::plan::new_shared_plan_state(); - let registry = - SubAgentToolRegistry::new(runtime, FleetRole::Scout, None, todo_list, plan_state); + let registry = SubAgentToolRegistry::new( + runtime, + FleetRole::Scout, + Some(vec!["File".to_string()]), + todo_list, + plan_state, + ); let catalog = registry.deferred_catalog_for_model(&FleetRole::Scout); let search = catalog @@ -21080,8 +21088,105 @@ fn forkguard_workflow_briefs_name_catalog_visible_tools() { body.contains("`tool_search`") && body.contains("`grep_files`"), "the scout brief must teach the two-step activation path:\n{body}" ); + // Every catalog-invisible execution name (hidden replay aliases and the + // retired `search_content` action) is denied as a whole word, so an + // unbackticked or renamed citation cannot slip past the guard. `list_dir` + // and the lowercase primitives stay legal: they are model-visible. + const HIDDEN_EXEC_NAMES: [&str; 8] = [ + "File", + "Bash", + "TodoWrite", + "work_update", + "read_file", + "write_file", + "edit_file", + "search_content", + ]; + let cited: Vec<&str> = body + .split(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) + .filter(|word| HIDDEN_EXEC_NAMES.iter().any(|hidden| hidden == word)) + .collect(); assert!( - !body.contains("`File`") && !body.contains("search_content"), - "workflow briefs must not command calls a catalog can never return:\n{body}" + cited.is_empty(), + "workflow briefs must not command calls a catalog can never return: {cited:?}" ); } + +// Behavioral counterpart to the two composition guards above: on the scout's +// live surface, one `tool_search` call must make the deferred `grep_files` +// dispatchable through the same gate the child step loop uses, while the +// hidden `File` alias the old brief commanded must keep failing the catalog +// gate. Composition can drift from behavior; this cannot. +#[tokio::test] +async fn forkguard_scout_activation_makes_grep_files_dispatchable() { + let tmp = tempdir().expect("tempdir"); + let mut runtime = + stub_runtime().with_agent_tool_surface_options(enabled_agent_surface_options()); + runtime.context = ToolContext::new(tmp.path().to_path_buf()); + let todo_list = crate::tools::todo::new_shared_todo_list(); + let plan_state = crate::tools::plan::new_shared_plan_state(); + let registry = SubAgentToolRegistry::new( + runtime, + FleetRole::Scout, + Some(vec!["File".to_string()]), + todo_list, + plan_state, + ); + // Mirror the spawn loop: filtered catalog, then a cold surface. + let mut surface = + SubAgentToolSurface::new(registry.deferred_catalog_for_model(&FleetRole::Scout), &[]); + + let active_names: std::collections::HashSet = surface.active_names.clone(); + let file_error = registry + .execute_from_surface( + "agent_unknown", + "", + &mut surface, + &active_names, + "File", + serde_json::json!({"action": "search_content", "path": "."}), + ) + .await + .expect_err("the hidden File alias must fail the scout catalog gate"); + assert!( + file_error + .to_string() + .contains("not in this child's policy-filtered catalog"), + "{file_error}" + ); + + let active_names: std::collections::HashSet = surface.active_names.clone(); + registry + .execute_from_surface( + "agent_unknown", + "", + &mut surface, + &active_names, + "tool_search", + serde_json::json!({"query": "grep_files"}), + ) + .await + .expect("tool_search activation must succeed on the scout surface"); + assert!( + surface.active_names.contains("grep_files"), + "activation must admit grep_files into the same surface the dispatch gate reads" + ); + + let active_names: std::collections::HashSet = surface.active_names.clone(); + registry + .execute_from_surface( + "agent_unknown", + "", + &mut surface, + &active_names, + "grep_files", + serde_json::json!({ + "path": ".", + "pattern": "stopship", + "max_results": 5, + "context_lines": 1 + }), + ) + .await + .expect("grep_files must dispatch through the real tool after the taught activation"); +} diff --git a/crates/workflow/src/js_authoring.rs b/crates/workflow/src/js_authoring.rs index c6604961c0..94d328753a 100644 --- a/crates/workflow/src/js_authoring.rs +++ b/crates/workflow/src/js_authoring.rs @@ -673,12 +673,17 @@ workflow({ ); if expected_role == "explore" { assert!( - leaf.prompt.contains("exactly one `File` call") - && leaf.prompt.contains("Do not call `File` more than once") - && leaf - .prompt - .contains("do not use any action except `search_content`"), - "the scout must finish discovery in one bounded tool round" + leaf.prompt.contains("exactly one `tool_search` call") + && leaf.prompt.contains("exactly one `grep_files` call") + && leaf.prompt.contains( + "the content-search tool is deferred, so this activation is required before it can be called" + ), + "the scout must activate the deferred content-search tool before searching with it" + ); + assert!( + leaf.prompt.contains("Make no other tool calls") + && leaf.prompt.contains("nothing else"), + "the scout discovery must stay one bounded activation-plus-search round" ); assert_eq!( leaf.file_scope @@ -698,7 +703,7 @@ workflow({ leaf.prompt.contains( "`include` set exactly to [`fleets/stopship.toml`, `crates/cli/src/lib.rs`, `crates/workflow/src/role_resolve.rs`, `crates/tui/src/tools/workflow.rs`, `crates/lane/src/runtime.rs`]" ) && leaf.prompt.contains("Matches outside that exact include list do not count"), - "the one File search must constrain the actual tool input, not only File scope metadata" + "the grep_files search must constrain the actual tool input, not only file scope metadata" ); assert!( leaf.prompt.contains("if you can populate all seven")