From c34c3a4309c03ca6308b422848e95e9000b1d7c9 Mon Sep 17 00:00:00 2001 From: asto18089 <44870036+asto18089@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:57:23 +0800 Subject: [PATCH 1/8] fix(fork): stop naming absent tools in skills text The rendered skills index told the model to call load_skill with name="list", but load_skill is a deferred tool that is not in the first-turn catalog, and the subagent first-turn active set excludes load_skill by design. Name the activation path instead: fall back to tool_search when load_skill is missing (main sessions), and teach subagents to discover skills through tool_search. Extend the same treatment to the remaining phantom-naming sites found in review: the bundled mcp-discovery skill now gates its registry_sync command on tool availability and stops citing the retired exec_shell alias (bash is the catalog name); the subagent ## Skills header stays honest for tool-free children that also lack tool_search; the omitted-skills tail carries the same tool_search fallback; and a new engine regression pins MCP_REGISTRY_FIRST_INSTRUCTION to the registered ToolSpec names so instruction/registration renames cannot drift apart silently (the pinvou3-app allowlist coupling is documented in the test). Regression coverage: forkguard_skill_index_usage_names_tool_search_activation, forkguard_subagent_skill_catalog_uses_tool_search_discovery, forkguard_omitted_skills_line_carries_tool_search_fallback, forkguard_mcp_discovery_skill_conditions_registry_commands, forkguard_registry_first_instruction_names_registered_tool_specs. Also repair pre-existing baseline gate failures that this branch's first CI run surfaced (they block the required gates but were not caused by this fix): the clippy redundant closure in engine/tests.rs (new stable lint), the crates/tui/CHANGELOG.md slice resync via scripts/sync-changelog.sh, and the v0.9.11 -> v0.9.12 refresh of web/data/latest-published-release.json plus docs/public-surface-facts.json via web/scripts/sync-latest-release.mjs. Release-note receipts for already-merged #51/#48/#43/#37 remain advisory (check-versions exits 0) and must land before the next release. Signed-off-by: asto18089 <44870036+asto18089@users.noreply.github.com> --- crates/tui/CHANGELOG.md | 12 ++++ .../tui/assets/skills/mcp-discovery/SKILL.md | 9 ++- crates/tui/src/core/engine/tests.rs | 36 ++++++++++- crates/tui/src/skills/mod.rs | 7 ++- crates/tui/src/skills/tests.rs | 59 +++++++++++++++++++ crates/tui/src/tools/subagent/mod.rs | 5 +- crates/tui/src/tools/subagent/tests.rs | 39 ++++++++++++ docs/public-surface-facts.json | 8 +-- web/data/latest-published-release.json | 8 +-- web/lib/changelog.generated.ts | 10 +++- web/lib/facts.generated.ts | 10 ++-- 11 files changed, 182 insertions(+), 21 deletions(-) diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 3be9d7ed7c..a607a79cda 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- API-backed `[search]` providers now visibly degrade directly to the + keyless Bing tail instead of routing through DuckDuckGo when + unavailable: DuckDuckGo is unreachable from mainland-China networks + (DNS poisoning plus SNI reset), while Bing serves its global and China + endpoints without a key. The all-backends-down guidance names every + keyed provider (tavily, bocha, metaso, baidu, volcengine, sofya) and + the keyless routes (firecrawl, bing), and the `web_search` tool + description no longer claims a DuckDuckGo hop for configured API + backends. + ## [0.9.12] - 2026-09-04 Codewhale v0.9.12 puts computer use in the binary, opens two new routes — diff --git a/crates/tui/assets/skills/mcp-discovery/SKILL.md b/crates/tui/assets/skills/mcp-discovery/SKILL.md index 77f9c86b37..fcc959dcd4 100644 --- a/crates/tui/assets/skills/mcp-discovery/SKILL.md +++ b/crates/tui/assets/skills/mcp-discovery/SKILL.md @@ -27,7 +27,10 @@ surface whenever MCP support is enabled. ## Workflow -1. **Check the registry.** Call `registry_sync {}`. It returns the complete +1. **Check the registry.** If `registry_sync` is in your tool list, call + `registry_sync {}`; if it is not, MCP Registry access is unavailable in + this session — say so and solve the task with local tools instead of + following the rest of this workflow. The call returns the complete catalog of eligible local stdio packages, including each server's name, description, and required launch arguments. Packages declaring any environment variable (including API keys/tokens) are excluded and never @@ -36,7 +39,7 @@ surface whenever MCP support is enabled. task against every server name and description. A candidate is a match when it plausibly covers the task's core specialized capability; wording does not need to be exact. When such a candidate exists, you **must start it and inspect - its tools before** using `exec_shell`, local programs, custom code, or a manual + its tools before** using `bash`, local programs, custom code, or a manual implementation. The availability or familiarity of a local alternative is not a reason to skip the candidate. Skip Registry use only when every entry is clearly irrelevant, or when a matching server fails to start after the retry @@ -44,7 +47,7 @@ surface whenever MCP support is enabled. 3. **Install + run transactionally.** Call `start_registry_mcp_server {registry_name: "", arguments: {...}}`. Supply only values listed in `required_args`; omit `arguments` when none - are required. Never install or launch the package through `exec_shell`. + are required. Never install or launch the package through `bash`. 4. **Solve the task with the new tools.** Their complete schemas are added to the current turn immediately after a successful connection; call the exact names returned by the start result. diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 91cc6c7354..60d9b27d06 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -417,6 +417,40 @@ fn registry_first_instruction_only_names_published_tools() { assert!(MCP_REGISTRY_FIRST_INSTRUCTION.contains("`Web`")); } +/// The registry-first instruction commands tools that the host must keep +/// callable, so the names in the text and the registered `ToolSpec` +/// implementations must stay a paired set: renaming either side alone +/// resurrects the phantom-tool incident (Pinvou #490) where the instruction +/// cites a tool absent from the catalog and allowlist. If this test fails +/// after a rename, update the instruction, the registration in +/// `tool_setup`, and the pinvou3-app allowlist +/// (`features/assistant/tool_policy.rs`) in the same change. +#[test] +fn forkguard_registry_first_instruction_names_registered_tool_specs() { + use crate::mcp::{McpConfig, McpPool}; + use crate::tools::mcp_registry::{McpSyncRegistry, StartRegistryMcpServer}; + use crate::tools::spec::ToolSpec; + use std::sync::Arc; + use tokio::sync::Mutex as AsyncMutex; + + let sync = McpSyncRegistry::new(); + let registry_sync = ToolSpec::name(&sync); + assert_eq!(registry_sync, "registry_sync"); + assert!( + MCP_REGISTRY_FIRST_INSTRUCTION.contains("`registry_sync`"), + "instruction must keep naming the registered `{registry_sync}`" + ); + + let pool = Arc::new(AsyncMutex::new(McpPool::new(McpConfig::default()))); + let start_tool = StartRegistryMcpServer::new(pool); + let start = ToolSpec::name(&start_tool); + assert_eq!(start, "start_registry_mcp_server"); + assert!( + MCP_REGISTRY_FIRST_INSTRUCTION.contains("`start_registry_mcp_server`"), + "instruction must keep naming the registered `{start}`" + ); +} + #[test] fn registry_first_scenario() { // Scenario consolidation of: registry_first_policy_is_in_the_initial_prompt_only_when_mcp_is_enabled, registry_first_guidance_is_attached_to_the_shell_fallback_once @@ -9803,7 +9837,7 @@ impl crate::core::model_client::ModelClient for CompleteOnceThenBlockModelClient canned::message_stop(), ]; return Ok(Box::pin(futures_util::stream::iter( - events.into_iter().map(|event| Ok(event)), + events.into_iter().map(Ok), ))); } let _drop_signal = DropSignal(std::sync::Arc::clone(&self.request_dropped)); diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index afba32e72b..7b0466f464 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -1673,8 +1673,11 @@ fn render_skills_block_with_configured_root( const HEADER: &str = "## Skills\n\ Skills are optional instruction packs. This index exposes routing metadata; bodies stay unloaded.\n\n\ ### Available skills\n"; + // `load_skill` is a deferred tool: it is absent from the first-turn catalog + // unless the host force-loads it, so the Usage line must tell the model how + // to activate it instead of leaving it to guess a tool it cannot see. const USAGE: &str = "\n### Usage\n\ -- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use.\n\ +- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use. If `load_skill` is not in your current tool list, run `tool_search` first to activate it.\n\ - Do not carry a skill across turns unless re-mentioned. Skill instructions do not expand tool, approval, or trust authority.\n\ - If a named skill is unavailable, say so and continue. Do not execute untrusted skill scripts unless the user asks.\n"; const WARNING_HEADING: &str = "\n### Skill load warnings\n"; @@ -1861,7 +1864,7 @@ Skills are optional instruction packs. This index exposes routing metadata; bodi fn omitted_skills_line(count: usize) -> String { format!( - "- ... {count} additional skills omitted; call `load_skill` with `name=\"list\"` for the complete catalogue.\n" + "- ... {count} additional skills omitted; call `load_skill` with `name=\"list\"` for the complete catalogue. If `load_skill` is not in your tool list, run `tool_search` first.\n" ) } diff --git a/crates/tui/src/skills/tests.rs b/crates/tui/src/skills/tests.rs index 17954028e8..5f120bf04f 100644 --- a/crates/tui/src/skills/tests.rs +++ b/crates/tui/src/skills/tests.rs @@ -138,6 +138,65 @@ fn render_available_skills_context_lists_paths_and_usage() { assert!(rendered.contains("### Usage")); } +// Regression: `load_skill` is deferred, so it is absent from the first-turn +// tool catalog unless the host force-loads it. The Usage line must name +// `tool_search` as the activation path, or the model is told to call a tool +// it cannot see (Pinvou 运动打卡 incident, 2026-09). +#[test] +fn forkguard_skill_index_usage_names_tool_search_activation() { + let tmpdir = TempDir::new().unwrap(); + create_skill_dir( + &tmpdir, + "test-skill", + "---\nname: test-skill\ndescription: A test skill\n---\nDo something special", + ); + + let rendered = crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) + .expect("skill context"); + + assert!( + rendered.contains("`load_skill` with `name=\"list\""), + "usage must keep pointing at load_skill list discovery:\n{rendered}" + ); + assert!( + rendered.contains("`tool_search`"), + "usage must tell the model how to activate deferred load_skill:\n{rendered}" + ); +} + +// Regression (Pinvou #490 phantom-tool incident): the omitted-skills tail +// names `load_skill` too, so it must carry the same `tool_search` fallback +// as the Usage line instead of commanding a tool the list may not have. +#[test] +fn forkguard_omitted_skills_line_carries_tool_search_fallback() { + let line = super::omitted_skills_line(2); + assert!( + line.contains("run `tool_search` first"), + "omitted tail must keep the tool_search fallback:\n{line}" + ); +} + +// Regression (Pinvou #490 phantom-tool incident): the bundled mcp-discovery +// skill must condition its registry commands on tool availability instead of +// commanding `registry_sync` unconditionally, and it must not cite the +// retired `exec_shell` alias as if it were callable. +#[test] +fn forkguard_mcp_discovery_skill_conditions_registry_commands() { + const SKILL: &str = include_str!("../../assets/skills/mcp-discovery/SKILL.md"); + assert!( + SKILL.contains("If `registry_sync` is in your tool list"), + "workflow step 1 must gate the registry_sync command on availability:\n{SKILL}" + ); + assert!( + SKILL.contains("`start_registry_mcp_server`"), + "the structured start tool must stay documented:\n{SKILL}" + ); + assert!( + !SKILL.contains("`exec_shell`"), + "exec_shell is a retired alias; cite `bash` instead:\n{SKILL}" + ); +} + #[test] fn workspace_prompt_omits_disabled_skills_without_configured_directory() { let _env_lock = crate::test_support::lock_test_env(); diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index be0e13edcd..3b4b6da115 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -9934,8 +9934,11 @@ fn subagent_skill_catalog(context: &ToolContext) -> String { if registry.list().is_empty() { return String::new(); } + // `load_skill` never sits in a child's first-turn active set (skills are + // discovered through `tool_search`), and tool-free children also lack + // `tool_search`, so the header below must stay honest in both states. let mut output = String::from( - "## Skills\n\nUse `load_skill` with an exact name before applying a Skill. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", + "## Skills\n\nIf `tool_search` is in your tool list, use it to activate `load_skill`, then call it with an exact name before applying a Skill; if it is not, you cannot load Skills in this session and must not attempt to. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", ); for skill in registry.list() { let source = match &skill.source { diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 294f1ba33d..24f2fe4728 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21016,3 +21016,42 @@ 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: `load_skill` never sits in a child's first-turn active set — +// skills are discovered through `tool_search`, and tool-free children lack +// `tool_search` as well. The ## Skills block rendered into a child prompt +// must therefore stay honest in both states: name tool_search as the +// discovery path when it exists, and do not command a tool the child cannot +// see (Pinvou 运动打卡 incident, 2026-09). +#[test] +fn forkguard_subagent_skill_catalog_uses_tool_search_discovery() { + let tmp = tempdir().expect("tempdir"); + let skill_dir = tmp + .path() + .join(".codewhale") + .join("skills") + .join("demo-skill"); + std::fs::create_dir_all(&skill_dir).unwrap(); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: demo-skill\ndescription: A demo skill\n---\nDo the demo thing", + ) + .unwrap(); + + let context = ToolContext::new(tmp.path()); + let catalog = subagent_skill_catalog(&context); + + assert!(catalog.contains("## Skills"), "catalog missing:\n{catalog}"); + assert!( + catalog.contains("demo-skill"), + "catalog missing skill entry:\n{catalog}" + ); + assert!( + catalog.contains("`tool_search`"), + "child has no load_skill in its wire catalog; discovery must go through tool_search:\n{catalog}" + ); + assert!( + catalog.contains("If `tool_search` is in your tool list"), + "header must stay honest for tool-free children that also lack tool_search:\n{catalog}" + ); +} diff --git a/docs/public-surface-facts.json b/docs/public-surface-facts.json index a3e9dbd955..4ee68884da 100644 --- a/docs/public-surface-facts.json +++ b/docs/public-surface-facts.json @@ -36,10 +36,10 @@ "providerCountDefinition": "Website-derived ApiProvider labels in this source snapshot, excluding Custom, DeepseekCN and retired Antigravity; includes protocol/plan variants. Not a count of distinct vendors, ProviderKind identities, live catalogs, or released v0.9.11 providers." }, "latestPublishedRelease": { - "tag": "v0.9.11", - "version": "0.9.11", - "publishedAt": "2026-08-23T17:39:46Z", - "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.11", + "tag": "v0.9.12", + "version": "0.9.12", + "publishedAt": "2026-09-05T09:59:53Z", + "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.12", "sources": [ "web/data/latest-published-release.json" ] diff --git a/web/data/latest-published-release.json b/web/data/latest-published-release.json index 2f876e3c54..ca65042725 100644 --- a/web/data/latest-published-release.json +++ b/web/data/latest-published-release.json @@ -1,6 +1,6 @@ { - "tag": "v0.9.11", - "version": "0.9.11", - "publishedAt": "2026-08-23T17:39:46Z", - "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.11" + "tag": "v0.9.12", + "version": "0.9.12", + "publishedAt": "2026-09-05T09:59:53Z", + "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.12" } diff --git a/web/lib/changelog.generated.ts b/web/lib/changelog.generated.ts index 6624c4cd4e..b3c07b470c 100644 --- a/web/lib/changelog.generated.ts +++ b/web/lib/changelog.generated.ts @@ -26,7 +26,15 @@ export const CHANGELOG: ChangelogRelease[] = [ "date": null, "unreleased": true, "compareUrl": "https://github.com/Hmbown/CodeWhale/compare/v0.9.12...HEAD", - "sections": [] + "sections": [ + { + "heading": "Fixed", + "items": [ + "API-backed [search] providers now visibly degrade directly to the keyless Bing tail instead of routing through DuckDuckGo when unavailable: DuckDuckGo is unreachable from mainland-China networks (DNS poisoning plus SNI reset), while Bing serves its global and China endpoints without a key. The all-backends-down guidance names every keyed provider (tavily, bocha, metaso, baidu, volcengine, sofya) and the keyless routes (firecrawl, bing), and the web_search tool description no…" + ], + "itemCount": 1 + } + ] }, { "version": "0.9.12", diff --git a/web/lib/facts.generated.ts b/web/lib/facts.generated.ts index e32d91f745..8a9ea2f017 100644 --- a/web/lib/facts.generated.ts +++ b/web/lib/facts.generated.ts @@ -27,7 +27,7 @@ export interface RepoFacts { } export const FACTS: RepoFacts = { - "generatedAt": "2026-09-03T18:51:05.927Z", + "generatedAt": "2026-09-14T03:09:31.167Z", "sourceRevision": null, "sourceCommittedAt": null, "version": "0.9.12", @@ -295,9 +295,9 @@ export const FACTS: RepoFacts = { "toolCount": 75, "license": "MIT", "latestPublishedRelease": { - "tag": "v0.9.11", - "version": "0.9.11", - "publishedAt": "2026-08-23T17:39:46Z", - "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.11" + "tag": "v0.9.12", + "version": "0.9.12", + "publishedAt": "2026-09-05T09:59:53Z", + "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.12" } }; From f81528358d684bc311a2859b4e928adfe14b0e6d Mon Sep 17 00:00:00 2001 From: asto18089 <44870036+asto18089@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:12:40 +0800 Subject: [PATCH 2/8] fix(fork): finish the phantom-tool sweep across bundled skills text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the skills-index fix in c34c3a430 after an adversarial review pass. Same defect class, same honest-availability treatment, applied to the surfaces the first pass missed or got wrong: - mcp-discovery: `registry_sync` is deferred but `tool_search`- searchable on stock hosts, so step 1's "not in your tool list => Registry unavailable" surrendered a reachable capability and contradicted the load_skill treatment in the same change. Step 1 now activates via `tool_search` first and declares unavailability only if that fails, teaches a `query`-bearing call (the schema rejects empty input), states the eight-scored-matches contract instead of a "complete catalog" dump, and the preamble no longer claims an always-active tool surface. Step 3 gates `start_registry_mcp_server`, which can be absent while `registry_sync` is registered (pool init failure, tool-security mode). - skills index: plugin-snapshot rows and the omitted-skills tail carry the `load_skill` -> `tool_search` fallback, and the Usage/omitted lines stay honest for surfaces where `tool_search` is absent too (ACP, allowlist-restricted exec). - subagent ## Skills header: covers the third child class — explicit allowlists that keep `tool_search` but filter `load_skill` — with "absent or does not surface" instead of a binary premise. - bundled pdf/help skills cited the `File` tool (`action: "read"`), a hidden compatibility alias no model-visible catalog or `tool_search` result can ever return; they now cite `read`. best-of-n gates `create_goal` on availability (child registries remove the tool entirely). New regressions: forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names sweeps every bundled body against hidden/retired names; forkguard_best_of_n_goal_tool_is_availability_gated pins the gating. The mcp-discovery and subagent pins were extended to the new wording. Fingerprints, the guard CANDIDATE_HEAD, and the parent gitlink re-pin ride in Pinvou/pinvou-agent#490. Signed-off-by: asto18089 <44870036+asto18089@users.noreply.github.com> --- crates/tui/assets/skills/best-of-n/SKILL.md | 5 +- crates/tui/assets/skills/help/SKILL.md | 2 +- .../tui/assets/skills/mcp-discovery/SKILL.md | 32 +++++---- crates/tui/assets/skills/pdf/SKILL.md | 2 +- crates/tui/src/skills/mod.rs | 7 +- crates/tui/src/skills/system/tests.rs | 67 ++++++++++++++++++- crates/tui/src/skills/tests.rs | 43 +++++++++++- crates/tui/src/tools/subagent/mod.rs | 9 ++- crates/tui/src/tools/subagent/tests.rs | 12 +++- 9 files changed, 148 insertions(+), 31 deletions(-) diff --git a/crates/tui/assets/skills/best-of-n/SKILL.md b/crates/tui/assets/skills/best-of-n/SKILL.md index 18021b3768..7afc78ba7e 100644 --- a/crates/tui/assets/skills/best-of-n/SKILL.md +++ b/crates/tui/assets/skills/best-of-n/SKILL.md @@ -25,8 +25,9 @@ the user has already chosen the approach. 3. Give every candidate the same task and rubric. Add only a candidate number; do not steer candidates toward different conclusions unless diversity is an explicit part of the request. -4. Prefer a session goal (`create_goal` or active `/goal`) when the tournament - spans more than one parent turn. +4. When the tournament spans more than one parent turn, prefer a session goal + if `create_goal` is in your tool list (it is absent in subagent sessions); + otherwise track progress in your own notes. ## Generate Independently diff --git a/crates/tui/assets/skills/help/SKILL.md b/crates/tui/assets/skills/help/SKILL.md index d01a2bcc22..4fdb696f4a 100644 --- a/crates/tui/assets/skills/help/SKILL.md +++ b/crates/tui/assets/skills/help/SKILL.md @@ -40,7 +40,7 @@ Answer from the surface that owns the fact, in this order: ## Working in a Codewhale checkout When the workspace *is* a Codewhale checkout, `docs/` is present on disk and -`File` with `action: "read"` is the right tool. Read the single most relevant file and quote +`read` is the right tool. Read the single most relevant file and quote the specific lines. Outside a checkout, `docs/` is usually absent — in that case rely on `/help`, `/config`, and `doctor`, and say plainly that the reference docs are not installed locally. diff --git a/crates/tui/assets/skills/mcp-discovery/SKILL.md b/crates/tui/assets/skills/mcp-discovery/SKILL.md index fcc959dcd4..6463bb8ab8 100644 --- a/crates/tui/assets/skills/mcp-discovery/SKILL.md +++ b/crates/tui/assets/skills/mcp-discovery/SKILL.md @@ -11,8 +11,9 @@ whether an MCP server already does it. The public MCP Registry ships hundreds of ready-made servers (filesystems, databases, browsers, media processing, developer utilities, cloud APIs, SaaS integrations, …). -The discovery and structured start tools are available in the active tool -surface whenever MCP support is enabled. +The discovery and structured start tools are registered whenever MCP support +is enabled, but hosts may defer them out of your first-turn tool list or +restrict them entirely; check your tool list and follow step 1 either way. ## When to use @@ -27,16 +28,21 @@ surface whenever MCP support is enabled. ## Workflow -1. **Check the registry.** If `registry_sync` is in your tool list, call - `registry_sync {}`; if it is not, MCP Registry access is unavailable in - this session — say so and solve the task with local tools instead of - following the rest of this workflow. The call returns the complete - catalog of eligible local stdio packages, including each server's name, - description, and required launch arguments. Packages declaring any - environment variable (including API keys/tokens) are excluded and never - written to the cache. +1. **Check the registry.** Call `registry_sync` with a `query` describing the + specialized capability the task needs, for example + `registry_sync {query: "convert PDF to markdown"}`. It returns at most + eight scored matches from the eligible local stdio package catalog, each + with the server's name, description, and required launch arguments. + Packages declaring any environment variable (including API keys/tokens) + are excluded and never written to the cache. + If `registry_sync` is not in your tool list, + run `tool_search` first to activate it; if `tool_search` cannot + surface it either, MCP Registry access is unavailable in this + session — say so and solve the task with local tools instead of + following the rest of this workflow. 2. **Match from context with a Registry-first bias.** Compare the user's full - task against every server name and description. A candidate is a match when + task against every returned candidate's name and description. A candidate + is a match when it plausibly covers the task's core specialized capability; wording does not need to be exact. When such a candidate exists, you **must start it and inspect its tools before** using `bash`, local programs, custom code, or a manual @@ -44,7 +50,9 @@ surface whenever MCP support is enabled. not a reason to skip the candidate. Skip Registry use only when every entry is clearly irrelevant, or when a matching server fails to start after the retry described below. -3. **Install + run transactionally.** Call +3. **Install + run transactionally.** If `start_registry_mcp_server` is not in + your tool list after `registry_sync` succeeded, registry starts are + unavailable in this session — fall back to local tools. Otherwise call `start_registry_mcp_server {registry_name: "", arguments: {...}}`. Supply only values listed in `required_args`; omit `arguments` when none are required. Never install or launch the package through `bash`. diff --git a/crates/tui/assets/skills/pdf/SKILL.md b/crates/tui/assets/skills/pdf/SKILL.md index f2b7bf4430..eb3187a05a 100644 --- a/crates/tui/assets/skills/pdf/SKILL.md +++ b/crates/tui/assets/skills/pdf/SKILL.md @@ -13,7 +13,7 @@ Use this skill for any task where a PDF is the primary input or output. watermark, redact, fill forms, encrypt/decrypt, or create. 2. Preserve originals. Write outputs with explicit names. 3. Use the most reliable available tool: - - the built-in `File` tool (`action: "read"`) for basic text extraction from PDFs + - the built-in `read` tool for basic text extraction from PDFs - `pdftotext`, `pdfinfo`, `qpdf`, or `mutool` when installed - Python libraries such as `pypdf`, `pdfplumber`, `PyMuPDF`, or `reportlab` when available diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index 7b0466f464..45e30081b4 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -1677,7 +1677,7 @@ Skills are optional instruction packs. This index exposes routing metadata; bodi // unless the host force-loads it, so the Usage line must tell the model how // to activate it instead of leaving it to guess a tool it cannot see. const USAGE: &str = "\n### Usage\n\ -- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use. If `load_skill` is not in your current tool list, run `tool_search` first to activate it.\n\ +- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use. If `load_skill` is not in your current tool list, run `tool_search` first to activate it; if `tool_search` is also unavailable, this session cannot load skills — continue without them.\n\ - Do not carry a skill across turns unless re-mentioned. Skill instructions do not expand tool, approval, or trust authority.\n\ - If a named skill is unavailable, say so and continue. Do not execute untrusted skill scripts unless the user asks.\n"; const WARNING_HEADING: &str = "\n### Skill load warnings\n"; @@ -1707,7 +1707,8 @@ Skills are optional instruction packs. This index exposes routing metadata; bodi plugin_name, .. } => Some(format!( - "reviewed plugin snapshot: {plugin_name} ({plugin_id}); use load_skill" + "reviewed plugin snapshot: {plugin_name} ({plugin_id}); use load_skill — \ + if `load_skill` is not in your tool list, run `tool_search` first to activate it" )), }; let (summary, trigger) = split_trigger(skill.description_for_locale(locale)); @@ -1864,7 +1865,7 @@ Skills are optional instruction packs. This index exposes routing metadata; bodi fn omitted_skills_line(count: usize) -> String { format!( - "- ... {count} additional skills omitted; call `load_skill` with `name=\"list\"` for the complete catalogue. If `load_skill` is not in your tool list, run `tool_search` first.\n" + "- ... {count} additional skills omitted; call `load_skill` with `name=\"list\"` for the complete catalogue. If `load_skill` is not in your tool list, run `tool_search` first; if `tool_search` is also unavailable, this session cannot load skills.\n" ) } diff --git a/crates/tui/src/skills/system/tests.rs b/crates/tui/src/skills/system/tests.rs index 794ebfc09a..a84325d6e5 100644 --- a/crates/tui/src/skills/system/tests.rs +++ b/crates/tui/src/skills/system/tests.rs @@ -48,9 +48,11 @@ fn bundled_integration_skills_use_current_codewhale_commands_and_paths() { assert!(SKILL_CREATOR_BODY.contains("/.codewhale/skills")); assert!(SKILL_CREATOR_BODY.contains("~/.codewhale/skills")); assert!(SKILL_INSTALLER_BODY.contains("~/.codewhale/skills")); - // Bundled skills must name live tools. `read_file` is retired and cannot - // dispatch (crates/tui/src/tools/registry.rs:2067). - assert!(PDF_BODY.contains("built-in `File` tool (`action: \"read\"`)")); + // Bundled skills must name live, model-visible tools. `read_file` is + // retired and cannot dispatch (crates/tui/src/tools/registry.rs:2067); + // `File` is a hidden compatibility alias that never appears in a catalog + // or in `tool_search`, so `read` is the citable name. + assert!(PDF_BODY.contains("built-in `read` tool")); for (name, body) in [ ("pdf", PDF_BODY), ("help", HELP_BODY), @@ -64,6 +66,65 @@ fn bundled_integration_skills_use_current_codewhale_commands_and_paths() { } } +// Regression (Pinvou #490 phantom-tool class): bundled skill bodies must +// never cite hidden compatibility aliases or retired dispatch-only names. +// Those names never appear in a model-visible catalog or in a `tool_search` +// result, so hosts whose allowlists derive from the wire catalog reject the +// call outright — the backticked `File` citations in pdf/help stalled real +// reasoning loops exactly that way. The list is backtick-anchored so plain +// prose (e.g. "Files or modules") never false-positives. +#[test] +fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { + const PHANTOM_NAMES: &[&str] = &[ + "`File`", + "`Bash`", + "`Read`", + "`Write`", + "`Edit`", + "`exec_shell`", + "`read_file`", + "`write_file`", + "`edit_file`", + "`list_dir`", + "`fetch_url`", + "`work_update`", + "`TodoWrite`", + ]; + for skill in BUNDLED_SKILLS { + for phantom in PHANTOM_NAMES { + assert!( + !skill.body.contains(phantom), + "bundled skill `{}` cites {phantom}, which no model-visible \ + catalog or `tool_search` result can ever return:\n{}", + skill.name, + skill.body + ); + } + } +} + +// Regression: `create_goal` is deferred in main sessions and removed from +// subagent registries entirely (tools/subagent/mod.rs drops it when building +// the child surface), so the best-of-n body must gate the command on +// availability instead of commanding it unconditionally. +#[test] +fn forkguard_best_of_n_goal_tool_is_availability_gated() { + let skill = BUNDLED_SKILLS + .iter() + .find(|skill| skill.name == "best-of-n") + .expect("best-of-n must be bundled"); + assert!( + skill.body.contains("`create_goal` is in your tool list"), + "best-of-n must gate create_goal on availability:\n{}", + skill.body + ); + assert!( + !skill.body.contains("`create_goal` or active `/goal`"), + "best-of-n must not command create_goal unconditionally:\n{}", + skill.body + ); +} + /// #4227 (requested by @JayBeest): the contributor sync/gate/digest skill /// ships in generation 8, and its two load-bearing refusals — never move a /// contributor's HEAD, never touch a dirty tree — must survive any later diff --git a/crates/tui/src/skills/tests.rs b/crates/tui/src/skills/tests.rs index 5f120bf04f..f20f3534e1 100644 --- a/crates/tui/src/skills/tests.rs +++ b/crates/tui/src/skills/tests.rs @@ -177,16 +177,48 @@ fn forkguard_omitted_skills_line_carries_tool_search_fallback() { } // Regression (Pinvou #490 phantom-tool incident): the bundled mcp-discovery -// skill must condition its registry commands on tool availability instead of -// commanding `registry_sync` unconditionally, and it must not cite the +// skill must keep `registry_sync` reachable on stock hosts — where it is +// deferred but `tool_search`-activatable — instead of declaring the Registry +// unavailable, must teach a `query`-bearing call (the schema rejects `{}`), +// must describe the scored-matches contract honestly, and must not cite the // retired `exec_shell` alias as if it were callable. #[test] fn forkguard_mcp_discovery_skill_conditions_registry_commands() { const SKILL: &str = include_str!("../../assets/skills/mcp-discovery/SKILL.md"); assert!( - SKILL.contains("If `registry_sync` is in your tool list"), + SKILL.contains("If `registry_sync` is not in your tool list"), "workflow step 1 must gate the registry_sync command on availability:\n{SKILL}" ); + assert!( + SKILL.contains("run `tool_search` first to activate it"), + "registry_sync is deferred on stock hosts; step 1 must name the \ + activation path instead of surrendering a reachable capability:\n{SKILL}" + ); + assert!( + SKILL.contains("`registry_sync {query:"), + "registry_sync requires a non-empty `query`; the skill must teach a \ + call that can succeed:\n{SKILL}" + ); + assert!( + SKILL.contains("eight scored matches"), + "the skill must describe the scored-matches contract, not a complete \ + catalog dump:\n{SKILL}" + ); + assert!( + !SKILL.contains("`registry_sync {}`"), + "registry_sync rejects empty input; never teach a call that cannot \ + succeed:\n{SKILL}" + ); + assert!( + !SKILL.contains("available in the active tool surface"), + "both tools are deferred by default; the preamble must not claim an \ + always-active surface:\n{SKILL}" + ); + assert!( + SKILL.contains("If `start_registry_mcp_server` is not in"), + "step 3 must gate the start tool, which can be absent while \ + registry_sync is registered (pool init failure, tool-security mode):\n{SKILL}" + ); assert!( SKILL.contains("`start_registry_mcp_server`"), "the structured start tool must stay documented:\n{SKILL}" @@ -1745,6 +1777,11 @@ fn plugin_skills_are_qualified_and_denied_until_trusted_and_enabled() { let rendered = super::render_skills_block(®istry, "en", tmp.path()).unwrap(); assert!(rendered.contains("reviewed plugin snapshot: demo")); assert!(rendered.contains("use load_skill")); + assert!( + rendered.contains("use load_skill — if `load_skill` is not in your tool list"), + "plugin rows must carry the same deferred-load_skill fallback as the \ + Usage line:\n{rendered}" + ); assert!( rendered.contains("hello"), "plugin skill descriptions must reach the model catalogue like native skills: {rendered}" diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 3b4b6da115..10d08e1487 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -9935,10 +9935,13 @@ fn subagent_skill_catalog(context: &ToolContext) -> String { return String::new(); } // `load_skill` never sits in a child's first-turn active set (skills are - // discovered through `tool_search`), and tool-free children also lack - // `tool_search`, so the header below must stay honest in both states. + // discovered through `tool_search`), but three child classes exist: role + // children carry both tools, explicit allowlists can keep `tool_search` + // while filtering `load_skill` out of the catalog entirely, and tool-free + // children lack `tool_search` too. The header below must stay honest in + // all three states. let mut output = String::from( - "## Skills\n\nIf `tool_search` is in your tool list, use it to activate `load_skill`, then call it with an exact name before applying a Skill; if it is not, you cannot load Skills in this session and must not attempt to. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", + "## Skills\n\nIf `tool_search` is in your tool list, use it to activate `load_skill`, then call it with an exact name before applying a Skill; if `tool_search` is absent or does not surface `load_skill`, you cannot load Skills in this session and must not attempt to. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", ); for skill in registry.list() { let source = match &skill.source { diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 24f2fe4728..4ac5507a6a 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21020,9 +21020,10 @@ async fn agent_claim_is_withheld_from_a_role_with_no_write_authority() { // Regression: `load_skill` never sits in a child's first-turn active set — // skills are discovered through `tool_search`, and tool-free children lack // `tool_search` as well. The ## Skills block rendered into a child prompt -// must therefore stay honest in both states: name tool_search as the -// discovery path when it exists, and do not command a tool the child cannot -// see (Pinvou 运动打卡 incident, 2026-09). +// must therefore stay honest in all three child states: name tool_search as +// the discovery path when it exists, stay truthful for allowlist children +// that carry tool_search but no load_skill, and do not command a tool the +// child cannot see (Pinvou 运动打卡 incident, 2026-09). #[test] fn forkguard_subagent_skill_catalog_uses_tool_search_discovery() { let tmp = tempdir().expect("tempdir"); @@ -21054,4 +21055,9 @@ fn forkguard_subagent_skill_catalog_uses_tool_search_discovery() { catalog.contains("If `tool_search` is in your tool list"), "header must stay honest for tool-free children that also lack tool_search:\n{catalog}" ); + assert!( + catalog.contains("does not surface `load_skill`"), + "header must stay honest for allowlist children that carry tool_search \ + but no load_skill in their catalog:\n{catalog}" + ); } From a2e21fcf9dd79cf57ba5b245faf425f4569007a9 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Mon, 14 Sep 2026 21:16:18 +0800 Subject: [PATCH 3/8] fix(fork): close the two-stage activation gaps left in model-facing text A fresh adversarial pass over the branch found the sweep's own text surrendering deferred-but-tool_search-searchable tools on visibility gates alone, plus one phantom citation the sweep had missed: - mcp-discovery step 3 now mirrors step 1: run tool_search first to activate start_registry_mcp_server (its activation does not follow from registry_sync's), and declare registry starts unavailable only when tool_search cannot surface it either. The preamble no longer overclaims registration (the start tool additionally needs the host's MCP pool initialized), and step 4 teaches re-activation for connected tools that re-defer on later turns. - best-of-n now names the tool_search activation path for create_goal (deferred on every stock host, so the visibility gate alone always failed) and stays honest for subagent sessions where the tool is removed entirely. - The bundled-skills denylist no longer lists list_dir, which is a live, model-visible, searchable tool; it now covers the registry's remaining hidden aliases and canonical retired names instead. - The parent-context sub-agent hint cites `read` or `bash` (first-turn active) instead of the hidden `File` alias with a nonexistent `list` action; the pin is renamed into the forkguard set and flipped. - MAX_REGISTRY_MATCHES is pinned to the quoted "eight" wording at compile time so the cap and the text cannot drift apart silently. Disclosed, deliberately not changed here: workflows/stopship.workflow.js briefs an explore child with the hidden `File` `search_content` alias. The call dispatches by alias on every surface this fleet runs on, and a rename to `grep_files` is behavior-changing (deferred hydration would burn the scout's one-round response budget), so it needs its own protocol rework rather than a drive-by edit. Fingerprints and the parent gitlink re-pin ride in Pinvou/pinvou-agent#490. Signed-off-by: asto18089 --- crates/tui/assets/skills/best-of-n/SKILL.md | 5 +- .../tui/assets/skills/mcp-discovery/SKILL.md | 14 +++-- crates/tui/src/core/engine/context.rs | 2 +- crates/tui/src/core/engine/tests.rs | 22 +++++--- crates/tui/src/skills/system/tests.rs | 52 +++++++++++++++---- crates/tui/src/skills/tests.rs | 19 ++++++- crates/tui/src/tools/mcp_registry.rs | 8 +++ crates/tui/src/tools/subagent/tests.rs | 2 +- 8 files changed, 98 insertions(+), 26 deletions(-) diff --git a/crates/tui/assets/skills/best-of-n/SKILL.md b/crates/tui/assets/skills/best-of-n/SKILL.md index 7afc78ba7e..1dcadcd51e 100644 --- a/crates/tui/assets/skills/best-of-n/SKILL.md +++ b/crates/tui/assets/skills/best-of-n/SKILL.md @@ -26,8 +26,9 @@ the user has already chosen the approach. do not steer candidates toward different conclusions unless diversity is an explicit part of the request. 4. When the tournament spans more than one parent turn, prefer a session goal - if `create_goal` is in your tool list (it is absent in subagent sessions); - otherwise track progress in your own notes. + if `create_goal` is in your tool list; if it is not, run `tool_search` first + to activate it. In subagent sessions `create_goal` does not exist and + `tool_search` cannot surface it — track progress in your own notes instead. ## Generate Independently diff --git a/crates/tui/assets/skills/mcp-discovery/SKILL.md b/crates/tui/assets/skills/mcp-discovery/SKILL.md index 6463bb8ab8..929c272052 100644 --- a/crates/tui/assets/skills/mcp-discovery/SKILL.md +++ b/crates/tui/assets/skills/mcp-discovery/SKILL.md @@ -12,8 +12,9 @@ of ready-made servers (filesystems, databases, browsers, media processing, developer utilities, cloud APIs, SaaS integrations, …). The discovery and structured start tools are registered whenever MCP support -is enabled, but hosts may defer them out of your first-turn tool list or -restrict them entirely; check your tool list and follow step 1 either way. +is enabled and the host's MCP pool initialized, but hosts may defer them out +of your first-turn tool list or restrict them entirely; check your tool list +and follow step 1 either way. ## When to use @@ -51,14 +52,17 @@ restrict them entirely; check your tool list and follow step 1 either way. clearly irrelevant, or when a matching server fails to start after the retry described below. 3. **Install + run transactionally.** If `start_registry_mcp_server` is not in - your tool list after `registry_sync` succeeded, registry starts are - unavailable in this session — fall back to local tools. Otherwise call + your tool list after `registry_sync` succeeded, run `tool_search` first to + activate it; if `tool_search` cannot surface it either, registry starts + are unavailable in this session — fall back to local tools. Otherwise call `start_registry_mcp_server {registry_name: "", arguments: {...}}`. Supply only values listed in `required_args`; omit `arguments` when none are required. Never install or launch the package through `bash`. 4. **Solve the task with the new tools.** Their complete schemas are added to the current turn immediately after a successful connection; call the - exact names returned by the start result. + exact names returned by the start result. On a later turn a connected + tool may drop out of your tool list again; run `tool_search` first + before calling it. ## If a server fails to start diff --git a/crates/tui/src/core/engine/context.rs b/crates/tui/src/core/engine/context.rs index 9a74129376..fc68f693d1 100644 --- a/crates/tui/src/core/engine/context.rs +++ b/crates/tui/src/core/engine/context.rs @@ -214,7 +214,7 @@ fn compact_subagent_tool_result_for_context(tool_name: &str, raw: &str) -> Optio let mut out = String::from("[sub-agent result summarized for parent context]\n"); out.push_str( - "Child results are self-reports; verify side effects with `File` actions like `read` or `list` before claiming success.\n", + "Child results are self-reports; verify side effects with `read` or `bash` before claiming success.\n", ); out.push_str("Use `handle_read` on `transcript_handle` for bounded transcript slices when the returned summary is not enough.\n"); for (idx, snapshot) in snapshots.iter().enumerate() { diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 60d9b27d06..13be1ea006 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -422,9 +422,9 @@ fn registry_first_instruction_only_names_published_tools() { /// implementations must stay a paired set: renaming either side alone /// resurrects the phantom-tool incident (Pinvou #490) where the instruction /// cites a tool absent from the catalog and allowlist. If this test fails -/// after a rename, update the instruction, the registration in -/// `tool_setup`, and the pinvou3-app allowlist -/// (`features/assistant/tool_policy.rs`) in the same change. +/// after a rename, update the instruction and the registration in +/// `tool_setup` in the same change; downstream hosts that gate these tool +/// names by allowlist must follow in the same commit. #[test] fn forkguard_registry_first_instruction_names_registered_tool_specs() { use crate::mcp::{McpConfig, McpPool}; @@ -16838,8 +16838,13 @@ fn codex_tool_retention_uses_oauth_route_window_not_asmall_contract_model_window assert!(context.len() < content.len()); } +// Regression (Pinvou #490 phantom-tool class): the parent-context hint must +// name tools that are first-turn active on every stock host (`read`, `bash`). +// `File` is a hidden compatibility alias no catalog or `tool_search` result +// can return, and `list` is not a tool name at all — the earlier wording +// commanded calls that allowlist hosts reject outright. #[test] -fn subagent_results_are_summarized_before_parent_context_insertion() { +fn forkguard_subagent_context_hint_names_active_tools() { let long_result = "verified detail\n".repeat(1_000); let output = ToolResult::success( json!({ @@ -16865,9 +16870,12 @@ fn subagent_results_are_summarized_before_parent_context_insertion() { assert!(context.contains("steps=12")); assert!(context.len() < output.content.len()); assert!(context.contains("self-report")); - assert!(context.contains("verify side effects")); - assert!(context.contains("`File` actions like `read` or `list`")); - assert!(!context.contains("read_file") && !context.contains("list_dir")); + assert!(context.contains("verify side effects with `read` or `bash`")); + assert!( + !context.contains("`File`") + && !context.contains("read_file") + && !context.contains("list_dir") + ); assert!(context.contains("handle_read")); } diff --git a/crates/tui/src/skills/system/tests.rs b/crates/tui/src/skills/system/tests.rs index a84325d6e5..cdd1bc6a68 100644 --- a/crates/tui/src/skills/system/tests.rs +++ b/crates/tui/src/skills/system/tests.rs @@ -49,9 +49,9 @@ fn bundled_integration_skills_use_current_codewhale_commands_and_paths() { assert!(SKILL_CREATOR_BODY.contains("~/.codewhale/skills")); assert!(SKILL_INSTALLER_BODY.contains("~/.codewhale/skills")); // Bundled skills must name live, model-visible tools. `read_file` is - // retired and cannot dispatch (crates/tui/src/tools/registry.rs:2067); - // `File` is a hidden compatibility alias that never appears in a catalog - // or in `tool_search`, so `read` is the citable name. + // retired and cannot dispatch; `File` is a hidden compatibility alias + // that never appears in a catalog or in `tool_search`, so `read` is the + // citable name. assert!(PDF_BODY.contains("built-in `read` tool")); for (name, body) in [ ("pdf", PDF_BODY), @@ -72,7 +72,10 @@ fn bundled_integration_skills_use_current_codewhale_commands_and_paths() { // result, so hosts whose allowlists derive from the wire catalog reject the // call outright — the backticked `File` citations in pdf/help stalled real // reasoning loops exactly that way. The list is backtick-anchored so plain -// prose (e.g. "Files or modules") never false-positives. +// prose (e.g. "Files or modules") never false-positives. Entries track the +// registry's hidden compatibility aliases and the canonical retired-name +// list; `list_dir` is deliberately absent because it is a live, searchable +// tool (tools/file.rs ListDirTool), not a phantom. #[test] fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { const PHANTOM_NAMES: &[&str] = &[ @@ -82,13 +85,31 @@ fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { "`Write`", "`Edit`", "`exec_shell`", + "`exec_shell_wait`", + "`exec_shell_interact`", + "`exec_shell_cancel`", "`read_file`", "`write_file`", "`edit_file`", - "`list_dir`", "`fetch_url`", + "`web_fetch`", + "`web_search`", "`work_update`", "`TodoWrite`", + "`todo`", + "`checklist_write`", + "`checklist_update`", + "`update_plan`", + "`rlm`", + "`run_tests`", + "`run_verifiers`", + "`git_status`", + "`wait_for_dev_server`", + "`agents/list`", + "`agents/message`", + "`agents/followup`", + "`agents/interrupt`", + "`agents/wait`", ]; for skill in BUNDLED_SKILLS { for phantom in PHANTOM_NAMES { @@ -103,10 +124,11 @@ fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { } } -// Regression: `create_goal` is deferred in main sessions and removed from -// subagent registries entirely (tools/subagent/mod.rs drops it when building -// the child surface), so the best-of-n body must gate the command on -// availability instead of commanding it unconditionally. +// Regression: `create_goal` is deferred-but-`tool_search`-searchable in main +// sessions and removed from subagent registries entirely (tools/subagent/ +// mod.rs drops it before catalog filtering and search), so the best-of-n body +// must gate the command on availability, name the activation path instead of +// surrendering a reachable capability, and stay honest for child sessions. #[test] fn forkguard_best_of_n_goal_tool_is_availability_gated() { let skill = BUNDLED_SKILLS @@ -118,6 +140,18 @@ fn forkguard_best_of_n_goal_tool_is_availability_gated() { "best-of-n must gate create_goal on availability:\n{}", skill.body ); + assert!( + skill.body.contains("run `tool_search` first"), + "create_goal is deferred on every stock host, so the visibility gate \ + alone would always fail; the skill must name the activation path:\n{}", + skill.body + ); + assert!( + skill.body.contains("does not exist and"), + "best-of-n must stay honest for subagent sessions where create_goal \ + is removed entirely:\n{}", + skill.body + ); assert!( !skill.body.contains("`create_goal` or active `/goal`"), "best-of-n must not command create_goal unconditionally:\n{}", diff --git a/crates/tui/src/skills/tests.rs b/crates/tui/src/skills/tests.rs index f20f3534e1..c364327584 100644 --- a/crates/tui/src/skills/tests.rs +++ b/crates/tui/src/skills/tests.rs @@ -141,7 +141,7 @@ fn render_available_skills_context_lists_paths_and_usage() { // Regression: `load_skill` is deferred, so it is absent from the first-turn // tool catalog unless the host force-loads it. The Usage line must name // `tool_search` as the activation path, or the model is told to call a tool -// it cannot see (Pinvou 运动打卡 incident, 2026-09). +// it cannot see (Pinvou #490 phantom-tool incident). #[test] fn forkguard_skill_index_usage_names_tool_search_activation() { let tmpdir = TempDir::new().unwrap(); @@ -219,6 +219,23 @@ fn forkguard_mcp_discovery_skill_conditions_registry_commands() { "step 3 must gate the start tool, which can be absent while \ registry_sync is registered (pool init failure, tool-security mode):\n{SKILL}" ); + assert!( + SKILL.contains("activate it; if `tool_search` cannot surface it either, registry starts"), + "the start tool is deferred-but-searchable and its activation does not \ + follow from registry_sync's, so step 3 must offer the same \ + tool_search path as step 1 instead of surrendering on visibility \ + alone:\n{SKILL}" + ); + assert!( + SKILL.contains("and the host's MCP pool initialized"), + "the start tool registers only after the pool initializes; the \ + preamble must not overclaim registration:\n{SKILL}" + ); + assert!( + SKILL.contains("drop out of your tool list again"), + "connected tools re-defer on later turns; step 4 must teach \ + re-activation instead of a blind call:\n{SKILL}" + ); assert!( SKILL.contains("`start_registry_mcp_server`"), "the structured start tool must stay documented:\n{SKILL}" diff --git a/crates/tui/src/tools/mcp_registry.rs b/crates/tui/src/tools/mcp_registry.rs index af3c1428e5..e1a118f4bf 100644 --- a/crates/tui/src/tools/mcp_registry.rs +++ b/crates/tui/src/tools/mcp_registry.rs @@ -584,6 +584,14 @@ const REGISTRY_FIRST_PROMPT: &str = concat!( /// Host-side cap on model-visible Registry matches. The complete catalog /// stays on disk; only this many matched entries ever reach the model. const MAX_REGISTRY_MATCHES: usize = 8; +// The cap is quoted as a literal number in model-facing text; changing one +// side alone turns that text into a phantom fact (Pinvou #490 class). +const _: () = assert!( + MAX_REGISTRY_MATCHES == 8, + "update the \"eight scored matches\" wording in \ + assets/skills/mcp-discovery/SKILL.md, the registry_sync schema \ + description here, and MCP_REGISTRY_FIRST_INSTRUCTION in core/engine.rs", +); #[derive(Serialize)] struct RegistryCatalogResult { diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 4ac5507a6a..d1a193e964 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21023,7 +21023,7 @@ async fn agent_claim_is_withheld_from_a_role_with_no_write_authority() { // must therefore stay honest in all three child states: name tool_search as // the discovery path when it exists, stay truthful for allowlist children // that carry tool_search but no load_skill, and do not command a tool the -// child cannot see (Pinvou 运动打卡 incident, 2026-09). +// child cannot see (Pinvou #490 phantom-tool incident). #[test] fn forkguard_subagent_skill_catalog_uses_tool_search_discovery() { let tmp = tempdir().expect("tempdir"); From 18f7c7b15469f9244f9087bac54d96f092c8dc2a Mon Sep 17 00:00:00 2001 From: asto18089 Date: Mon, 14 Sep 2026 21:18:20 +0800 Subject: [PATCH 4/8] chore(fork): split the baseline gate repairs out of this PR CodeWhale CONTRIBUTING.md writes changelog entries on main at merge time and asks PRs carrying changelog hunks to strip them, and the facts/web generated refreshes have no gate on the pinvou3-clean lane (the web freshness checks run on master/main only). The CHANGELOG slice resync, changelog.generated.ts, and the surface-facts trio move to a dedicated chore PR; the clippy map(Ok) test-target cleanup moves with them (fork-ci does not lint test targets). The net diff of this branch is now the phantom-tool text fix only. Signed-off-by: asto18089 --- crates/tui/CHANGELOG.md | 12 ------------ crates/tui/src/core/engine/tests.rs | 2 +- docs/public-surface-facts.json | 8 ++++---- web/data/latest-published-release.json | 8 ++++---- web/lib/changelog.generated.ts | 10 +--------- web/lib/facts.generated.ts | 10 +++++----- 6 files changed, 15 insertions(+), 35 deletions(-) diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index a607a79cda..3be9d7ed7c 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -7,18 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Fixed - -- API-backed `[search]` providers now visibly degrade directly to the - keyless Bing tail instead of routing through DuckDuckGo when - unavailable: DuckDuckGo is unreachable from mainland-China networks - (DNS poisoning plus SNI reset), while Bing serves its global and China - endpoints without a key. The all-backends-down guidance names every - keyed provider (tavily, bocha, metaso, baidu, volcengine, sofya) and - the keyless routes (firecrawl, bing), and the `web_search` tool - description no longer claims a DuckDuckGo hop for configured API - backends. - ## [0.9.12] - 2026-09-04 Codewhale v0.9.12 puts computer use in the binary, opens two new routes — diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 13be1ea006..c4fbde7895 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -9837,7 +9837,7 @@ impl crate::core::model_client::ModelClient for CompleteOnceThenBlockModelClient canned::message_stop(), ]; return Ok(Box::pin(futures_util::stream::iter( - events.into_iter().map(Ok), + events.into_iter().map(|event| Ok(event)), ))); } let _drop_signal = DropSignal(std::sync::Arc::clone(&self.request_dropped)); diff --git a/docs/public-surface-facts.json b/docs/public-surface-facts.json index 4ee68884da..a3e9dbd955 100644 --- a/docs/public-surface-facts.json +++ b/docs/public-surface-facts.json @@ -36,10 +36,10 @@ "providerCountDefinition": "Website-derived ApiProvider labels in this source snapshot, excluding Custom, DeepseekCN and retired Antigravity; includes protocol/plan variants. Not a count of distinct vendors, ProviderKind identities, live catalogs, or released v0.9.11 providers." }, "latestPublishedRelease": { - "tag": "v0.9.12", - "version": "0.9.12", - "publishedAt": "2026-09-05T09:59:53Z", - "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.12", + "tag": "v0.9.11", + "version": "0.9.11", + "publishedAt": "2026-08-23T17:39:46Z", + "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.11", "sources": [ "web/data/latest-published-release.json" ] diff --git a/web/data/latest-published-release.json b/web/data/latest-published-release.json index ca65042725..2f876e3c54 100644 --- a/web/data/latest-published-release.json +++ b/web/data/latest-published-release.json @@ -1,6 +1,6 @@ { - "tag": "v0.9.12", - "version": "0.9.12", - "publishedAt": "2026-09-05T09:59:53Z", - "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.12" + "tag": "v0.9.11", + "version": "0.9.11", + "publishedAt": "2026-08-23T17:39:46Z", + "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.11" } diff --git a/web/lib/changelog.generated.ts b/web/lib/changelog.generated.ts index b3c07b470c..6624c4cd4e 100644 --- a/web/lib/changelog.generated.ts +++ b/web/lib/changelog.generated.ts @@ -26,15 +26,7 @@ export const CHANGELOG: ChangelogRelease[] = [ "date": null, "unreleased": true, "compareUrl": "https://github.com/Hmbown/CodeWhale/compare/v0.9.12...HEAD", - "sections": [ - { - "heading": "Fixed", - "items": [ - "API-backed [search] providers now visibly degrade directly to the keyless Bing tail instead of routing through DuckDuckGo when unavailable: DuckDuckGo is unreachable from mainland-China networks (DNS poisoning plus SNI reset), while Bing serves its global and China endpoints without a key. The all-backends-down guidance names every keyed provider (tavily, bocha, metaso, baidu, volcengine, sofya) and the keyless routes (firecrawl, bing), and the web_search tool description no…" - ], - "itemCount": 1 - } - ] + "sections": [] }, { "version": "0.9.12", diff --git a/web/lib/facts.generated.ts b/web/lib/facts.generated.ts index 8a9ea2f017..e32d91f745 100644 --- a/web/lib/facts.generated.ts +++ b/web/lib/facts.generated.ts @@ -27,7 +27,7 @@ export interface RepoFacts { } export const FACTS: RepoFacts = { - "generatedAt": "2026-09-14T03:09:31.167Z", + "generatedAt": "2026-09-03T18:51:05.927Z", "sourceRevision": null, "sourceCommittedAt": null, "version": "0.9.12", @@ -295,9 +295,9 @@ export const FACTS: RepoFacts = { "toolCount": 75, "license": "MIT", "latestPublishedRelease": { - "tag": "v0.9.12", - "version": "0.9.12", - "publishedAt": "2026-09-05T09:59:53Z", - "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.12" + "tag": "v0.9.11", + "version": "0.9.11", + "publishedAt": "2026-08-23T17:39:46Z", + "url": "https://github.com/Hmbown/CodeWhale/releases/tag/v0.9.11" } }; From 9f46ac5f01b6579e450eae46af49cbc63f70e6f8 Mon Sep 17 00:00:00 2001 From: asto Date: Mon, 14 Sep 2026 23:37:23 +0800 Subject: [PATCH 5/8] fix(fork): dedupe deferred-activation text, fix review nits Collapse the duplicated tool_search fallback teaching: the skills-index Usage line and mcp-discovery step 1 stay the single teaching points; the plugin rows, the omitted-skills tail, and mcp-discovery step 3 now rely on them by reference instead of repeating the full fallback, and the subagent ## Skills header shrinks to the same one-sentence pattern. Make the mcp-discovery preamble precise: only the start tool needs the host's MCP pool initialized; registry_sync registers with MCP support alone. Harden the bundled-skills phantom sweep: drop the speculative `Read`/`Write`/`Edit` entries that no registry table ever carried, add the missed `git_diff`/`git_log`/`git_show`/`git_blame` and `agents/coordinate` retired names, correct the tracking comment, and disclose the BUNDLED_SKILLS scope (v4-best-practices, feishu uncovered). Pin the remaining "eight" wording sides (first-turn instruction and the registry_sync schema description) to MAX_REGISTRY_MATCHES, and fix the parent-context test comment that overclaimed first-turn activity on "every stock host". Fork-guard fingerprints for the changed text are re-synced in Pinvou/pinvou-agent#490. Signed-off-by: asto --- .../tui/assets/skills/mcp-discovery/SKILL.md | 14 +++---- crates/tui/src/core/engine/tests.rs | 25 ++++++++++-- crates/tui/src/skills/mod.rs | 12 +++--- crates/tui/src/skills/system/tests.rs | 20 ++++++---- crates/tui/src/skills/tests.rs | 39 +++++++------------ crates/tui/src/tools/subagent/mod.rs | 2 +- crates/tui/src/tools/subagent/tests.rs | 11 +++--- 7 files changed, 69 insertions(+), 54 deletions(-) diff --git a/crates/tui/assets/skills/mcp-discovery/SKILL.md b/crates/tui/assets/skills/mcp-discovery/SKILL.md index 929c272052..a67ed337ce 100644 --- a/crates/tui/assets/skills/mcp-discovery/SKILL.md +++ b/crates/tui/assets/skills/mcp-discovery/SKILL.md @@ -11,10 +11,10 @@ whether an MCP server already does it. The public MCP Registry ships hundreds of ready-made servers (filesystems, databases, browsers, media processing, developer utilities, cloud APIs, SaaS integrations, …). -The discovery and structured start tools are registered whenever MCP support -is enabled and the host's MCP pool initialized, but hosts may defer them out -of your first-turn tool list or restrict them entirely; check your tool list -and follow step 1 either way. +The discovery and structured start tools are registered when MCP support is +enabled — the start tool once the host's MCP pool is initialized as well — +but hosts may defer them out of your first-turn tool list or restrict them +entirely; check your tool list and follow step 1 either way. ## When to use @@ -52,9 +52,9 @@ and follow step 1 either way. clearly irrelevant, or when a matching server fails to start after the retry described below. 3. **Install + run transactionally.** If `start_registry_mcp_server` is not in - your tool list after `registry_sync` succeeded, run `tool_search` first to - activate it; if `tool_search` cannot surface it either, registry starts - are unavailable in this session — fall back to local tools. Otherwise call + your tool list after `registry_sync` succeeded, activate it via + `tool_search` as in step 1; if that fails, registry starts are + unavailable in this session — fall back to local tools. Otherwise call `start_registry_mcp_server {registry_name: "", arguments: {...}}`. Supply only values listed in `required_args`; omit `arguments` when none are required. Never install or launch the package through `bash`. diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index c4fbde7895..2fb59b08ca 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -449,6 +449,21 @@ fn forkguard_registry_first_instruction_names_registered_tool_specs() { MCP_REGISTRY_FIRST_INSTRUCTION.contains("`start_registry_mcp_server`"), "instruction must keep naming the registered `{start}`" ); + // The match cap is quoted as a literal word in the instruction, the + // `registry_sync` schema description, and the bundled mcp-discovery + // skill; `MAX_REGISTRY_MATCHES` pins the constant to it at compile time, + // and these two assertions pin the remaining text sides. + assert!( + MCP_REGISTRY_FIRST_INSTRUCTION.contains("eight"), + "instruction must keep the match-cap wording in sync with \ + MAX_REGISTRY_MATCHES" + ); + let schema = ToolSpec::input_schema(&sync).to_string(); + assert!( + schema.contains("eight"), + "registry_sync schema must keep the match-cap wording in sync with \ + MAX_REGISTRY_MATCHES: {schema}" + ); } #[test] @@ -16839,10 +16854,12 @@ fn codex_tool_retention_uses_oauth_route_window_not_asmall_contract_model_window } // Regression (Pinvou #490 phantom-tool class): the parent-context hint must -// name tools that are first-turn active on every stock host (`read`, `bash`). -// `File` is a hidden compatibility alias no catalog or `tool_search` result -// can return, and `list` is not a tool name at all — the earlier wording -// commanded calls that allowlist hosts reject outright. +// name tools that are first-turn active wherever the default native toolset +// is registered (`read`, `bash`). `File` is a hidden compatibility alias no +// catalog or `tool_search` result can return, and `list` is not a tool name +// at all — the earlier wording commanded calls that allowlist hosts reject +// outright. Shell-restricted sessions that carry neither tool surface the +// same names through the catalog's core-action fallback explanations. #[test] fn forkguard_subagent_context_hint_names_active_tools() { let long_result = "verified detail\n".repeat(1_000); diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index 45e30081b4..243456069c 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -1674,10 +1674,11 @@ fn render_skills_block_with_configured_root( Skills are optional instruction packs. This index exposes routing metadata; bodies stay unloaded.\n\n\ ### Available skills\n"; // `load_skill` is a deferred tool: it is absent from the first-turn catalog - // unless the host force-loads it, so the Usage line must tell the model how - // to activate it instead of leaving it to guess a tool it cannot see. + // unless the host force-loads it. This Usage line is the single place the + // index teaches the activation fallback; per-skill rows and the omitted + // tail stay short and rely on it instead of repeating it. const USAGE: &str = "\n### Usage\n\ -- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use. If `load_skill` is not in your current tool list, run `tool_search` first to activate it; if `tool_search` is also unavailable, this session cannot load skills — continue without them.\n\ +- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use. If `load_skill` is not in your tool list, activate it via `tool_search`; if that fails, this session cannot load skills — continue without them.\n\ - Do not carry a skill across turns unless re-mentioned. Skill instructions do not expand tool, approval, or trust authority.\n\ - If a named skill is unavailable, say so and continue. Do not execute untrusted skill scripts unless the user asks.\n"; const WARNING_HEADING: &str = "\n### Skill load warnings\n"; @@ -1707,8 +1708,7 @@ Skills are optional instruction packs. This index exposes routing metadata; bodi plugin_name, .. } => Some(format!( - "reviewed plugin snapshot: {plugin_name} ({plugin_id}); use load_skill — \ - if `load_skill` is not in your tool list, run `tool_search` first to activate it" + "reviewed plugin snapshot: {plugin_name} ({plugin_id}); use load_skill" )), }; let (summary, trigger) = split_trigger(skill.description_for_locale(locale)); @@ -1865,7 +1865,7 @@ Skills are optional instruction packs. This index exposes routing metadata; bodi fn omitted_skills_line(count: usize) -> String { format!( - "- ... {count} additional skills omitted; call `load_skill` with `name=\"list\"` for the complete catalogue. If `load_skill` is not in your tool list, run `tool_search` first; if `tool_search` is also unavailable, this session cannot load skills.\n" + "- ... {count} additional skills omitted; call `load_skill` with `name=\"list\"` for the complete catalogue.\n" ) } diff --git a/crates/tui/src/skills/system/tests.rs b/crates/tui/src/skills/system/tests.rs index cdd1bc6a68..cfe4d918c3 100644 --- a/crates/tui/src/skills/system/tests.rs +++ b/crates/tui/src/skills/system/tests.rs @@ -72,18 +72,19 @@ fn bundled_integration_skills_use_current_codewhale_commands_and_paths() { // result, so hosts whose allowlists derive from the wire catalog reject the // call outright — the backticked `File` citations in pdf/help stalled real // reasoning loops exactly that way. The list is backtick-anchored so plain -// prose (e.g. "Files or modules") never false-positives. Entries track the -// registry's hidden compatibility aliases and the canonical retired-name -// list; `list_dir` is deliberately absent because it is a live, searchable -// tool (tools/file.rs ListDirTool), not a phantom. +// prose (e.g. "Files or modules") never false-positives. Entries are the +// registry's hidden compatibility aliases (`File`, `Bash`, the todo family, +// `update_plan`, `rlm`) plus the canonical retired-name lists +// (`RETIRED_TOOL_NAMES`, the `agents/*` family); `list_dir` is deliberately +// absent because it is a live, searchable tool (tools/file.rs ListDirTool), +// not a phantom. Scope note: `v4-best-practices` and `feishu` are not in +// BUNDLED_SKILLS (legacy/optional, never auto-installed), so this sweep does +// not cover them. #[test] fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { const PHANTOM_NAMES: &[&str] = &[ "`File`", "`Bash`", - "`Read`", - "`Write`", - "`Edit`", "`exec_shell`", "`exec_shell_wait`", "`exec_shell_interact`", @@ -104,9 +105,14 @@ fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { "`run_tests`", "`run_verifiers`", "`git_status`", + "`git_diff`", + "`git_log`", + "`git_show`", + "`git_blame`", "`wait_for_dev_server`", "`agents/list`", "`agents/message`", + "`agents/coordinate`", "`agents/followup`", "`agents/interrupt`", "`agents/wait`", diff --git a/crates/tui/src/skills/tests.rs b/crates/tui/src/skills/tests.rs index c364327584..de8c40930d 100644 --- a/crates/tui/src/skills/tests.rs +++ b/crates/tui/src/skills/tests.rs @@ -141,7 +141,9 @@ fn render_available_skills_context_lists_paths_and_usage() { // Regression: `load_skill` is deferred, so it is absent from the first-turn // tool catalog unless the host force-loads it. The Usage line must name // `tool_search` as the activation path, or the model is told to call a tool -// it cannot see (Pinvou #490 phantom-tool incident). +// it cannot see (Pinvou #490 phantom-tool incident). It is also the single +// fallback teaching point in the index: per-skill rows and the omitted tail +// deliberately do not repeat it. #[test] fn forkguard_skill_index_usage_names_tool_search_activation() { let tmpdir = TempDir::new().unwrap(); @@ -164,24 +166,13 @@ fn forkguard_skill_index_usage_names_tool_search_activation() { ); } -// Regression (Pinvou #490 phantom-tool incident): the omitted-skills tail -// names `load_skill` too, so it must carry the same `tool_search` fallback -// as the Usage line instead of commanding a tool the list may not have. -#[test] -fn forkguard_omitted_skills_line_carries_tool_search_fallback() { - let line = super::omitted_skills_line(2); - assert!( - line.contains("run `tool_search` first"), - "omitted tail must keep the tool_search fallback:\n{line}" - ); -} - // Regression (Pinvou #490 phantom-tool incident): the bundled mcp-discovery // skill must keep `registry_sync` reachable on stock hosts — where it is // deferred but `tool_search`-activatable — instead of declaring the Registry // unavailable, must teach a `query`-bearing call (the schema rejects `{}`), // must describe the scored-matches contract honestly, and must not cite the -// retired `exec_shell` alias as if it were callable. +// retired `exec_shell` alias as if it were callable. Step 3 reuses step 1's +// activation teaching by reference instead of repeating the full fallback. #[test] fn forkguard_mcp_discovery_skill_conditions_registry_commands() { const SKILL: &str = include_str!("../../assets/skills/mcp-discovery/SKILL.md"); @@ -220,16 +211,16 @@ fn forkguard_mcp_discovery_skill_conditions_registry_commands() { registry_sync is registered (pool init failure, tool-security mode):\n{SKILL}" ); assert!( - SKILL.contains("activate it; if `tool_search` cannot surface it either, registry starts"), + SKILL.contains("`tool_search` as in step 1"), "the start tool is deferred-but-searchable and its activation does not \ - follow from registry_sync's, so step 3 must offer the same \ - tool_search path as step 1 instead of surrendering on visibility \ - alone:\n{SKILL}" + follow from registry_sync's, so step 3 must point at step 1's \ + tool_search path instead of surrendering on visibility alone:\n{SKILL}" ); assert!( - SKILL.contains("and the host's MCP pool initialized"), - "the start tool registers only after the pool initializes; the \ - preamble must not overclaim registration:\n{SKILL}" + SKILL.contains("the start tool once the host's MCP pool is initialized"), + "the start tool registers only after the pool initializes (the \ + discovery tool needs only MCP support); the preamble must not \ + overclaim registration for either side:\n{SKILL}" ); assert!( SKILL.contains("drop out of your tool list again"), @@ -1795,9 +1786,9 @@ fn plugin_skills_are_qualified_and_denied_until_trusted_and_enabled() { assert!(rendered.contains("reviewed plugin snapshot: demo")); assert!(rendered.contains("use load_skill")); assert!( - rendered.contains("use load_skill — if `load_skill` is not in your tool list"), - "plugin rows must carry the same deferred-load_skill fallback as the \ - Usage line:\n{rendered}" + !rendered.contains("use load_skill —"), + "plugin rows stay short; the deferred-load_skill fallback is taught \ + once in the Usage line:\n{rendered}" ); assert!( rendered.contains("hello"), diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 10d08e1487..3b8936f40a 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -9941,7 +9941,7 @@ fn subagent_skill_catalog(context: &ToolContext) -> String { // children lack `tool_search` too. The header below must stay honest in // all three states. let mut output = String::from( - "## Skills\n\nIf `tool_search` is in your tool list, use it to activate `load_skill`, then call it with an exact name before applying a Skill; if `tool_search` is absent or does not surface `load_skill`, you cannot load Skills in this session and must not attempt to. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", + "## Skills\n\nLoad a Skill with `load_skill`, activating it via `tool_search` first if it is not in your tool list; if `tool_search` is absent or does not surface `load_skill`, you cannot load Skills and must not attempt to. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", ); for skill in registry.list() { let source = match &skill.source { diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index d1a193e964..c90cf9f5e2 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21052,12 +21052,13 @@ fn forkguard_subagent_skill_catalog_uses_tool_search_discovery() { "child has no load_skill in its wire catalog; discovery must go through tool_search:\n{catalog}" ); assert!( - catalog.contains("If `tool_search` is in your tool list"), - "header must stay honest for tool-free children that also lack tool_search:\n{catalog}" + catalog.contains("activating it via `tool_search` first if it is not in your tool list"), + "child has no load_skill in its first-turn active set; the header must \ + teach the tool_search activation path:\n{catalog}" ); assert!( - catalog.contains("does not surface `load_skill`"), - "header must stay honest for allowlist children that carry tool_search \ - but no load_skill in their catalog:\n{catalog}" + catalog.contains("if `tool_search` is absent or does not surface `load_skill`"), + "header must stay honest for tool-free and allowlist children that \ + lack tool_search or load_skill:\n{catalog}" ); } From be2b2fe92d34a9f651822077e8c14458fc1d4293 Mon Sep 17 00:00:00 2001 From: asto18089 Date: Tue, 15 Sep 2026 19:09:09 +0800 Subject: [PATCH 6/8] fix(fork): close the review-round phantom-text gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review of this branch found the same phantom-tool defect class at four more model-facing sites, plus two over-claims in the new fallback wording: - GOAL_CONTINUATION_PROMPT commanded `update_goal`, which is deferred on stock hosts (a goal created by a host-side /goal never activates it); the prompt now names the tool_search activation path. - The parent-context sub-agent hint cited `handle_read` unconditionally; it now names the activation path too. - The skills-index Usage line and the subagent ## Skills header declared skills unloadable when tool_search cannot surface load_skill, but a registered deferred tool still hydrates on demand when called directly; both now teach the direct-call fallback before declaring skills unavailable. - Test-only fixes: the mcp-discovery pin's "rejects empty input" attribution (the schema requires the `query` field; the host rejects an empty value) and the MAX_REGISTRY_MATCHES assert message (SKILL.md path prefix, and the instruction side says "eight matches", not "eight scored matches"). - Under-pin repairs: pin the new direct-call fallback clauses in the Usage and subagent-header tests, add an omitted-tail guard so the commit-5 dedup cannot silently regress, and pin the goal-continuation activation teaching. New regressions: forkguard_goal_continuation_names_tool_search_activation, forkguard_omitted_skills_line_stays_short. Verified: cargo fmt --check clean; cargo test -p codewhale-tui --lib (RUST_MIN_STACK=8388608, matching CI) passes apart from three remote_control timing tests that also fail intermittently on unmodified trees and pass in isolation. Fork-guard fingerprints for the changed text ride in Pinvou/pinvou-agent#490 — that register also gains the two new test names above. Signed-off-by: asto18089 --- crates/tui/src/core/engine/context.rs | 2 +- crates/tui/src/core/engine/tests.rs | 25 +++++++++++++++++++++++++ crates/tui/src/prompts/text.rs | 6 ++++-- crates/tui/src/skills/mod.rs | 2 +- crates/tui/src/skills/tests.rs | 25 +++++++++++++++++++++++-- crates/tui/src/tools/mcp_registry.rs | 7 ++++--- crates/tui/src/tools/subagent/mod.rs | 2 +- crates/tui/src/tools/subagent/tests.rs | 7 +++++++ 8 files changed, 66 insertions(+), 10 deletions(-) diff --git a/crates/tui/src/core/engine/context.rs b/crates/tui/src/core/engine/context.rs index fc68f693d1..7568936579 100644 --- a/crates/tui/src/core/engine/context.rs +++ b/crates/tui/src/core/engine/context.rs @@ -216,7 +216,7 @@ fn compact_subagent_tool_result_for_context(tool_name: &str, raw: &str) -> Optio out.push_str( "Child results are self-reports; verify side effects with `read` or `bash` before claiming success.\n", ); - out.push_str("Use `handle_read` on `transcript_handle` for bounded transcript slices when the returned summary is not enough.\n"); + out.push_str("Use `handle_read` on `transcript_handle` for bounded transcript slices when the returned summary is not enough — if `handle_read` is not in your tool list, activate it via `tool_search` first.\n"); for (idx, snapshot) in snapshots.iter().enumerate() { if idx >= 8 { out.push_str(&format!( diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 2fb59b08ca..66a5f90b83 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -16894,6 +16894,31 @@ fn forkguard_subagent_context_hint_names_active_tools() { && !context.contains("list_dir") ); assert!(context.contains("handle_read")); + assert!( + context.contains("activate it via `tool_search` first"), + "handle_read is deferred on stock hosts; the hint must name the \ + activation path instead of commanding a tool the model cannot see:\n\ + {context}" + ); +} + +// Regression (Pinvou #490 phantom-tool class): GOAL_CONTINUATION_PROMPT +// commands `update_goal`, which is deferred on stock hosts — it must name the +// `tool_search` activation path instead of telling the model to call a tool +// that is not in its first-turn tool list. +#[test] +fn forkguard_goal_continuation_names_tool_search_activation() { + let prompt = crate::prompts::GOAL_CONTINUATION_PROMPT; + assert!( + prompt.contains("`update_goal`"), + "the continuation prompt must keep commanding the goal-close tool:\n\ + {prompt}" + ); + assert!( + prompt.contains("activate it via `tool_search` first"), + "update_goal is deferred on stock hosts; the prompt must teach \ + activation instead of commanding an absent tool:\n{prompt}" + ); } #[test] diff --git a/crates/tui/src/prompts/text.rs b/crates/tui/src/prompts/text.rs index 8720022242..c6578af90f 100644 --- a/crates/tui/src/prompts/text.rs +++ b/crates/tui/src/prompts/text.rs @@ -222,8 +222,10 @@ current state before relying on it. Before deciding the goal is achieved, verify it against the actual current state — files, command output, tests, runtime behavior, issue or PR state, or other authoritative evidence — then call `update_goal` with -`status: "complete"` and concise evidence. If something genuinely prevents -progress, call `update_goal` with `status: "blocked"` and explain it. +`status: "complete"` and concise evidence. `update_goal` may sit outside your +first-turn tool list; if it does, activate it via `tool_search` first. If +something genuinely prevents progress, call `update_goal` with +`status: "blocked"` and explain it. "#; /// Memory hygiene guidance — appended to the system prompt only when the /// session has a non-empty user-memory block. Steers the model toward diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index 243456069c..7a36be19d7 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -1678,7 +1678,7 @@ Skills are optional instruction packs. This index exposes routing metadata; bodi // index teaches the activation fallback; per-skill rows and the omitted // tail stay short and rely on it instead of repeating it. const USAGE: &str = "\n### Usage\n\ -- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use. If `load_skill` is not in your tool list, activate it via `tool_search`; if that fails, this session cannot load skills — continue without them.\n\ +- When the user names a skill or one may help, call `load_skill` with `name=\"list\"`; load the exact skill before use. If `load_skill` is not in your tool list, activate it via `tool_search`; if it is still missing, call `load_skill` anyway — registered tools hydrate on demand — and continue without skills only if that call also fails.\n\ - Do not carry a skill across turns unless re-mentioned. Skill instructions do not expand tool, approval, or trust authority.\n\ - If a named skill is unavailable, say so and continue. Do not execute untrusted skill scripts unless the user asks.\n"; const WARNING_HEADING: &str = "\n### Skill load warnings\n"; diff --git a/crates/tui/src/skills/tests.rs b/crates/tui/src/skills/tests.rs index de8c40930d..1b1afbd9ed 100644 --- a/crates/tui/src/skills/tests.rs +++ b/crates/tui/src/skills/tests.rs @@ -164,6 +164,27 @@ fn forkguard_skill_index_usage_names_tool_search_activation() { rendered.contains("`tool_search`"), "usage must tell the model how to activate deferred load_skill:\n{rendered}" ); + assert!( + rendered.contains("call `load_skill` anyway"), + "tool_search being unable to surface load_skill does not make it \ + unreachable — a registered deferred tool hydrates on demand when \ + called directly; the usage line must not surrender that path:\n\ + {rendered}" + ); +} + +// Regression guard paired with the Usage-line dedup (commit 9f46ac5f0): the +// omitted tail stays a short pointer and must not re-grow the full fallback +// teaching — the Usage block appended below it is the single teaching point. +#[test] +fn forkguard_omitted_skills_line_stays_short() { + let line = super::omitted_skills_line(2); + assert!( + !line.contains("tool_search"), + "omitted tail must not repeat the Usage block's tool_search fallback; \ + re-adding it reintroduces the per-row duplication the dedup removed:\n\ + {line}" + ); } // Regression (Pinvou #490 phantom-tool incident): the bundled mcp-discovery @@ -197,8 +218,8 @@ fn forkguard_mcp_discovery_skill_conditions_registry_commands() { ); assert!( !SKILL.contains("`registry_sync {}`"), - "registry_sync rejects empty input; never teach a call that cannot \ - succeed:\n{SKILL}" + "registry_sync requires a `query` field and the host rejects an empty \ + one; never teach a call that cannot succeed:\n{SKILL}" ); assert!( !SKILL.contains("available in the active tool surface"), diff --git a/crates/tui/src/tools/mcp_registry.rs b/crates/tui/src/tools/mcp_registry.rs index e1a118f4bf..7d8bf326c0 100644 --- a/crates/tui/src/tools/mcp_registry.rs +++ b/crates/tui/src/tools/mcp_registry.rs @@ -588,9 +588,10 @@ const MAX_REGISTRY_MATCHES: usize = 8; // side alone turns that text into a phantom fact (Pinvou #490 class). const _: () = assert!( MAX_REGISTRY_MATCHES == 8, - "update the \"eight scored matches\" wording in \ - assets/skills/mcp-discovery/SKILL.md, the registry_sync schema \ - description here, and MCP_REGISTRY_FIRST_INSTRUCTION in core/engine.rs", + "update the \"eight\" wording in \ + crates/tui/assets/skills/mcp-discovery/SKILL.md, the registry_sync \ + schema description here, and MCP_REGISTRY_FIRST_INSTRUCTION in \ + core/engine.rs", ); #[derive(Serialize)] diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 3b8936f40a..2442eeef22 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -9941,7 +9941,7 @@ fn subagent_skill_catalog(context: &ToolContext) -> String { // children lack `tool_search` too. The header below must stay honest in // all three states. let mut output = String::from( - "## Skills\n\nLoad a Skill with `load_skill`, activating it via `tool_search` first if it is not in your tool list; if `tool_search` is absent or does not surface `load_skill`, you cannot load Skills and must not attempt to. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", + "## Skills\n\nLoad a Skill with `load_skill`, activating it via `tool_search` first if it is not in your tool list; if `tool_search` is absent or does not surface `load_skill`, try `load_skill` directly anyway — registered tools hydrate on demand — and treat Skills as unavailable only if that call fails too. Catalog entries are workspace-scoped snapshots; plugin entries are revalidated at use.\n", ); for skill in registry.list() { let source = match &skill.source { diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index c90cf9f5e2..3b571f4fe8 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21061,4 +21061,11 @@ fn forkguard_subagent_skill_catalog_uses_tool_search_discovery() { "header must stay honest for tool-free and allowlist children that \ lack tool_search or load_skill:\n{catalog}" ); + assert!( + catalog.contains("try `load_skill` directly anyway"), + "tool_search being absent does not make load_skill unreachable — a \ + registered deferred tool hydrates on demand when called directly; \ + the header must teach the direct-call fallback instead of declaring \ + Skills unloadable:\n{catalog}" + ); } From 4eb487cd3e6bcfc9b8315672a908979afd0dfd9b Mon Sep 17 00:00:00 2001 From: asto18089 Date: Wed, 16 Sep 2026 16:59:54 +0800 Subject: [PATCH 7/8] fix(fork): teach activation for the remaining deferred call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fresh adversarial sweep found the same phantom-tool defect class at five more model-facing sites, plus one evidence-contract gap: - MCP_REGISTRY_FIRST_INSTRUCTION commanded `registry_sync` and `start_registry_mcp_server`, both deferred on stock hosts, with no activation path and no availability gate; it now teaches the `tool_search` activation for both, gates on reachability, and discloses that activating `registry_sync` does not activate the start tool. - REGISTRY_FIRST_PROMPT (attached to every `registry_sync` result) commanded the start tool with no path; it now names the activation. - The worker-record prose pairing `handle_read` with a transcript handle (takeover targets, session projection, the transcript artifact description, and every handle_read-recommending status reason) now carries one shared activation hint instead of commanding a tool absent from the first-turn catalog. - The /agent dispatch brief and the bare /goal brief named `handle_read`/`create_goal` with no path; both now teach the activation. - GOAL_CONTINUATION_PROMPT and the parent-context hint offered only the `tool_search` path, which allowed_tools-filtered sessions strip; both now keep the direct-call fallback (registered deferred tools hydrate when called by name). - Web/fetch overflow metadata now sets `evidence_available: true` so the engine auto-activates `retrieve_tool_result` — the recovery tool the overflow footer already names — matching the shell-truncation spillover contract. New pins: forkguard_registry_first_prompt_teaches_start_tool_activation, forkguard_worker_record_hints_teach_handle_read_activation, forkguard_slash_agent_dispatch_teaches_handle_read_activation; the registry-first, goal-continuation, context-hint, bare-/goal, and web overflow tests gain the new assertions. Verified: cargo fmt --check clean; cargo clippy -p codewhale-tui --lib --tests --locked clean apart from the pre-existing engine/tests.rs CompleteOnceThenBlockModelClient closure that 18f7c7b15 restored from base (fork-ci lints only non-test targets); RUST_MIN_STACK=8388608 cargo test -p codewhale-tui --lib passes 11768/0. Fork-guard fingerprints for the reworded registry-first instruction, continuation prompt, context hint, and worker-record text ride in Pinvou/pinvou-agent#490. Signed-off-by: asto18089 --- crates/tui/src/commands/groups/core/agent.rs | 20 +++++- .../tui/src/commands/groups/project/goal.rs | 8 ++- crates/tui/src/core/engine.rs | 2 +- crates/tui/src/core/engine/context.rs | 2 +- crates/tui/src/core/engine/tests.rs | 24 +++++++ crates/tui/src/prompts/text.rs | 4 +- crates/tui/src/tools/fetch_url.rs | 16 ++++- crates/tui/src/tools/mcp_registry.rs | 29 ++++++-- crates/tui/src/tools/subagent/mod.rs | 43 ++++++++---- crates/tui/src/tools/subagent/tests.rs | 70 +++++++++++++++++++ crates/tui/src/tools/web_run.rs | 10 +++ 11 files changed, 203 insertions(+), 25 deletions(-) diff --git a/crates/tui/src/commands/groups/core/agent.rs b/crates/tui/src/commands/groups/core/agent.rs index 2d42586609..9b18aea36b 100644 --- a/crates/tui/src/commands/groups/core/agent.rs +++ b/crates/tui/src/commands/groups/core/agent.rs @@ -59,7 +59,7 @@ pub fn agent(_app: &mut App, arg: Option<&str>) -> CommandResult { } }; let message = format!( - "Launch one sub-agent for this task by calling `agent` with name `slash_agent`, `prompt: {task:?}`, and `max_depth: {max_depth}`. Use `handle_read` on the returned transcript_handle if you need more detail. Verify any claimed side effects before reporting success." + "Launch one sub-agent for this task by calling `agent` with name `slash_agent`, `prompt: {task:?}`, and `max_depth: {max_depth}`. Use `handle_read` on the returned transcript_handle if you need more detail; if it is not in your tool list, activate it via `tool_search` first. Verify any claimed side effects before reporting success." ); CommandResult::with_message_and_action( format!("Opening persistent sub-agent at depth {max_depth}..."), @@ -124,4 +124,22 @@ mod tests { }; assert_eq!(agent_id, "agent_123"); } + + #[test] + fn forkguard_slash_agent_dispatch_teaches_handle_read_activation() { + // `handle_read` is deferred on stock hosts, so the dispatch brief + // must teach the `tool_search` activation path instead of pointing + // the model at a tool absent from its first-turn catalog (Pinvou + // #490 phantom-tool class). + let mut app = test_app(); + let result = agent(&mut app, Some("inspect the failing test")); + let Some(AppAction::SendMessage(message)) = result.action else { + panic!("expected SendMessage action"); + }; + assert!(message.contains("`handle_read`")); + assert!( + message.contains("activate it via `tool_search` first"), + "the dispatch brief must teach the handle_read activation path:\n{message}" + ); + } } diff --git a/crates/tui/src/commands/groups/project/goal.rs b/crates/tui/src/commands/groups/project/goal.rs index ff0ae84f6b..8c720e6ffb 100644 --- a/crates/tui/src/commands/groups/project/goal.rs +++ b/crates/tui/src/commands/groups/project/goal.rs @@ -92,7 +92,8 @@ fn goal_command( CURRENT work. Synthesize the objective from the conversation context (the \ task in flight, recent findings, open items) and set it by calling \ `create_goal` with the full objective (and a token_budget only if one was \ - discussed). Then continue working toward it. Only if the conversation \ + discussed); if `create_goal` is not in your tool list, activate it via \ + `tool_search` first. Then continue working toward it. Only if the conversation \ genuinely contains no work yet, ask the user what the goal should be." .to_string(); CommandResult::with_message_and_action( @@ -458,6 +459,11 @@ mod tests { }; assert!(message.contains("Synthesize the objective from the conversation")); assert!(message.contains("`create_goal`")); + assert!( + message.contains("activate it via `tool_search` first"), + "create_goal is deferred on stock hosts; the bare /goal brief must \ + teach the activation path instead of commanding an absent tool:\n{message}" + ); } #[test] diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 2b5deb2389..86992bbe00 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -146,7 +146,7 @@ fn agent_list_event(manager: &SubAgentManager, active_session_id: &str) -> Event } const MCP_REGISTRY_FIRST_INSTRUCTION_SOURCE: &str = "runtime:mcp-registry-first"; -const MCP_REGISTRY_FIRST_INSTRUCTION: &str = "## MCP Registry-first policy\n\nFor any task centered on a specialized capability, including media or document conversion, data transformation, browser automation, database or service access, or a developer utility, you must call `registry_sync` with a `query` describing that capability before `bash`, the `Web` tool, code execution, local programs, custom code, or a manual implementation. It scores the local Registry snapshot host-side and returns at most eight matches; the full catalog never enters the conversation. Treat a returned server as a match when it plausibly covers the core capability; wording need not be exact. If any plausible match exists, you must call `start_registry_mcp_server` with its exact name and inspect its tools before considering a local alternative. If nothing matches, refine the query once; a still-empty refined result means every Registry entry is clearly irrelevant. An installed or familiar shell command is not a reason to skip Registry discovery. Use local tools directly only for ordinary repo-native work and simple file operations, or after the matching server fails to start."; +const MCP_REGISTRY_FIRST_INSTRUCTION: &str = "## MCP Registry-first policy\n\nFor any task centered on a specialized capability, including media or document conversion, data transformation, browser automation, database or service access, or a developer utility, you must call `registry_sync` with a `query` describing that capability before `bash`, the `Web` tool, code execution, local programs, custom code, or a manual implementation. If `registry_sync` is not in your tool list, run `tool_search` first to activate it; if it cannot be surfaced or called at all, Registry access is unavailable in this session — use local tools instead. It scores the local Registry snapshot host-side and returns at most eight matches; the full catalog never enters the conversation. Treat a returned server as a match when it plausibly covers the core capability; wording need not be exact. If any plausible match exists, you must call `start_registry_mcp_server` with its exact name and inspect its tools before considering a local alternative; activate it via `tool_search` as well, since activating `registry_sync` does not activate it. If nothing matches, refine the query once; a still-empty refined result means every Registry entry is clearly irrelevant. An installed or familiar shell command is not a reason to skip Registry discovery. Use local tools directly only for ordinary repo-native work and simple file operations, or after the matching server fails to start."; const ISOLATED_CHAT_ENGINE_PROMPT: &str = "You are Codewhale Chat. Answer the user's request directly and conversationally. This isolated chat-only session has no local workspace, project, memory, skill, account, credential, path, runtime context, or tools."; fn sanitize_isolated_chat_attachments(mut text: String) -> String { diff --git a/crates/tui/src/core/engine/context.rs b/crates/tui/src/core/engine/context.rs index 7568936579..11256a0164 100644 --- a/crates/tui/src/core/engine/context.rs +++ b/crates/tui/src/core/engine/context.rs @@ -216,7 +216,7 @@ fn compact_subagent_tool_result_for_context(tool_name: &str, raw: &str) -> Optio out.push_str( "Child results are self-reports; verify side effects with `read` or `bash` before claiming success.\n", ); - out.push_str("Use `handle_read` on `transcript_handle` for bounded transcript slices when the returned summary is not enough — if `handle_read` is not in your tool list, activate it via `tool_search` first.\n"); + out.push_str("Use `handle_read` on `transcript_handle` for bounded transcript slices when the returned summary is not enough — if `handle_read` is not in your tool list, activate it via `tool_search` first; if `tool_search` cannot surface it, call `handle_read` directly anyway, since registered deferred tools hydrate when called by name.\n"); for (idx, snapshot) in snapshots.iter().enumerate() { if idx >= 8 { out.push_str(&format!( diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 66a5f90b83..d3b5e092e4 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -464,6 +464,19 @@ fn forkguard_registry_first_instruction_names_registered_tool_specs() { "registry_sync schema must keep the match-cap wording in sync with \ MAX_REGISTRY_MATCHES: {schema}" ); + // Both registry commands are deferred on stock hosts, so the instruction + // must teach the `tool_search` activation path instead of only + // commanding names the model cannot see yet. + assert!( + MCP_REGISTRY_FIRST_INSTRUCTION.contains("run `tool_search` first to activate it"), + "the instruction must teach the `registry_sync` activation path:\n\ + {MCP_REGISTRY_FIRST_INSTRUCTION}" + ); + assert!( + MCP_REGISTRY_FIRST_INSTRUCTION.contains("activate it via `tool_search` as well"), + "activating `registry_sync` must not be taught as also activating \ + `start_registry_mcp_server`:\n{MCP_REGISTRY_FIRST_INSTRUCTION}" + ); } #[test] @@ -16900,6 +16913,11 @@ fn forkguard_subagent_context_hint_names_active_tools() { activation path instead of commanding a tool the model cannot see:\n\ {context}" ); + assert!( + context.contains("call `handle_read` directly anyway"), + "allowed_tools-filtered sessions can strip tool_search too; the hint \ + must keep the direct-call fallback instead of dead-ending:\n{context}" + ); } // Regression (Pinvou #490 phantom-tool class): GOAL_CONTINUATION_PROMPT @@ -16919,6 +16937,12 @@ fn forkguard_goal_continuation_names_tool_search_activation() { "update_goal is deferred on stock hosts; the prompt must teach \ activation instead of commanding an absent tool:\n{prompt}" ); + assert!( + prompt.contains("call `update_goal` directly anyway"), + "allowed_tools-filtered sessions can strip tool_search too; the \ + prompt must keep the direct-call fallback instead of dead-ending:\n\ + {prompt}" + ); } #[test] diff --git a/crates/tui/src/prompts/text.rs b/crates/tui/src/prompts/text.rs index c6578af90f..060ebe3bbd 100644 --- a/crates/tui/src/prompts/text.rs +++ b/crates/tui/src/prompts/text.rs @@ -223,7 +223,9 @@ Before deciding the goal is achieved, verify it against the actual current state — files, command output, tests, runtime behavior, issue or PR state, or other authoritative evidence — then call `update_goal` with `status: "complete"` and concise evidence. `update_goal` may sit outside your -first-turn tool list; if it does, activate it via `tool_search` first. If +first-turn tool list; if it does, activate it via `tool_search` first, and if +`tool_search` is unavailable too, call `update_goal` directly anyway — +registered deferred tools hydrate when called by name. If something genuinely prevents progress, call `update_goal` with `status: "blocked"` and explain it. "#; diff --git a/crates/tui/src/tools/fetch_url.rs b/crates/tui/src/tools/fetch_url.rs index fd1e480c63..f2d9170f90 100644 --- a/crates/tui/src/tools/fetch_url.rs +++ b/crates/tui/src/tools/fetch_url.rs @@ -453,6 +453,11 @@ fn artifact_metadata(write: ArtifactWrite) -> Value { "artifact_relative_path": crate::artifacts::format_artifact_relative_path(&write.relative_path), "artifact_byte_size": write.byte_size, "artifact_preview": write.preview, + // The overflow footer tells the model to recover via + // `retrieve_tool_result`; this flag is what makes the engine + // auto-activate that tool on the next turn (same contract as the + // shell-truncation spillover), so the named tool is actually present. + "evidence_available": true, }) } @@ -601,9 +606,18 @@ mod tests { assert!(inline.contains("retrieve_tool_result")); assert!(inline.chars().count() <= inline_char_budget(&context)); assert_eq!( - std::fs::read_to_string(artifact.absolute_path).unwrap(), + std::fs::read_to_string(&artifact.absolute_path).unwrap(), full ); + // The footer names `retrieve_tool_result` as the recovery path; the + // evidence flag is what makes the engine auto-activate that tool on + // the next turn, so the named tool is actually present. + let metadata = artifact_metadata(artifact); + assert_eq!( + metadata.get("evidence_available"), + Some(&json!(true)), + "overflow metadata must flag retrievable evidence:\n{metadata}" + ); } #[test] diff --git a/crates/tui/src/tools/mcp_registry.rs b/crates/tui/src/tools/mcp_registry.rs index 7d8bf326c0..0bc557f20e 100644 --- a/crates/tui/src/tools/mcp_registry.rs +++ b/crates/tui/src/tools/mcp_registry.rs @@ -568,6 +568,9 @@ fn server_to_entry(server: RegistryServer) -> Option { } /// Prompt attached to every `registry_sync` result (Registry-first policy). +/// A successful `registry_sync` call proves its own availability, but +/// `start_registry_mcp_server` is deferred independently, so the prompt must +/// teach its activation path instead of commanding an absent tool name. const REGISTRY_FIRST_PROMPT: &str = concat!( "REGISTRY-FIRST POLICY: These are the top scored matches for your ", "query from the local Registry snapshot; the full catalog stays on the ", @@ -575,10 +578,11 @@ const REGISTRY_FIRST_PROMPT: &str = concat!( "core specialized capability; wording need not be exact. If a returned ", "match is plausible, you must call start_registry_mcp_server with its ", "exact name and inspect its tools before using shell commands, local ", - "programs, custom code, or a manual implementation. When no returned ", - "match plausibly covers the capability, refine the query once; if the ", - "refined query still returns nothing plausible, fall back to local ", - "tools.", + "programs, custom code, or a manual implementation; if ", + "start_registry_mcp_server is not in your tool list, run tool_search ", + "first to activate it. When no returned match plausibly covers the ", + "capability, refine the query once; if the refined query still returns ", + "nothing plausible, fall back to local tools.", ); /// Host-side cap on model-visible Registry matches. The complete catalog @@ -1889,4 +1893,21 @@ mod tests { eprintln!("{}", result.content); eprintln!("=== end ===\n"); } + + // REGISTRY_FIRST_PROMPT rides on every `registry_sync` result. A + // successful call proves `registry_sync`'s own availability, but + // `start_registry_mcp_server` is deferred independently, so the prompt + // must teach its activation path instead of commanding an absent name + // (Pinvou #490 phantom-tool class). + #[test] + fn forkguard_registry_first_prompt_teaches_start_tool_activation() { + assert!( + REGISTRY_FIRST_PROMPT.contains("must call start_registry_mcp_server"), + "prompt must keep commanding the start tool:\n{REGISTRY_FIRST_PROMPT}" + ); + assert!( + REGISTRY_FIRST_PROMPT.contains("run tool_search first to activate it"), + "start_registry_mcp_server is deferred independently of registry_sync; the prompt must name the activation path:\n{REGISTRY_FIRST_PROMPT}" + ); + } } diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 2442eeef22..a9cb00629a 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -897,6 +897,21 @@ fn default_agent_inspect_tool() -> String { "handle_read".to_string() } +/// `handle_read` is deferred on stock hosts, so model-facing text that pairs +/// it with a transcript handle must teach the activation path instead of +/// commanding a tool absent from the first-turn catalog (Pinvou #490 class). +const HANDLE_READ_ACTIVATION_HINT: &str = + "if `handle_read` is not in your tool list, activate it via `tool_search` first"; + +/// Shared inspect brief for worker records and takeover targets; both name +/// `handle_read`, so both must carry the activation hint. +fn agent_transcript_inspect_instructions(agent_ref: &str) -> String { + format!( + "Inspect agent '{agent_ref}' through the returned transcript_handle with handle_read; \ + {HANDLE_READ_ACTIVATION_HINT}; open a replacement with agent if the lane no longer fits." + ) +} + fn default_subagent_takeover_kind() -> String { "local_subagent_session".to_string() } @@ -1154,8 +1169,10 @@ fn default_agent_run_recommended_action() -> AgentRunRecommendedAction { AgentRunRecommendedAction { action: "inspect_transcript".to_string(), tool: Some(default_agent_inspect_tool()), - reason: "Inspect the returned transcript handle if the child result needs audit detail." - .to_string(), + reason: format!( + "Inspect the returned transcript handle if the child result needs audit detail; \ + {HANDLE_READ_ACTIVATION_HINT}." + ), } } @@ -1190,21 +1207,21 @@ fn recommended_action_for_worker_status( action: "inspect_or_replace".to_string(), tool: Some(default_agent_inspect_tool()), reason: format!( - "Worker {agent_ref} needs parent action; inspect the transcript handle and open a replacement with agent if the task still matters." + "Worker {agent_ref} needs parent action; inspect the transcript handle and open a replacement with agent if the task still matters; {HANDLE_READ_ACTIVATION_HINT}." ), }, AgentWorkerStatus::Completed => AgentRunRecommendedAction { action: "verify_self_report".to_string(), tool: Some("handle_read".to_string()), reason: format!( - "Worker {agent_ref} completed; verify its self-report before treating side effects as fact." + "Worker {agent_ref} completed; verify its self-report before treating side effects as fact; {HANDLE_READ_ACTIVATION_HINT}." ), }, AgentWorkerStatus::Failed => AgentRunRecommendedAction { action: "inspect_failure".to_string(), tool: Some(default_agent_inspect_tool()), reason: format!( - "Worker {agent_ref} failed; inspect the transcript handle and decide whether to open a replacement." + "Worker {agent_ref} failed; inspect the transcript handle and decide whether to open a replacement; {HANDLE_READ_ACTIVATION_HINT}." ), }, AgentWorkerStatus::Cancelled => AgentRunRecommendedAction { @@ -1218,7 +1235,7 @@ fn recommended_action_for_worker_status( action: "inspect_or_replace".to_string(), tool: Some(default_agent_inspect_tool()), reason: format!( - "Worker {agent_ref} was interrupted; inspect the transcript handle before deciding whether to re-dispatch." + "Worker {agent_ref} was interrupted; inspect the transcript handle before deciding whether to re-dispatch; {HANDLE_READ_ACTIVATION_HINT}." ), }, } @@ -1253,9 +1270,7 @@ fn takeover_target_for_spec(spec: &AgentWorkerSpec) -> AgentRunTakeoverTarget { supported: true, agent_id: spec.worker_id.clone(), session_name: spec.session_name.clone(), - instructions: format!( - "Inspect agent '{agent_ref}' through the returned transcript_handle with handle_read; open a replacement with agent if the lane no longer fits." - ), + instructions: agent_transcript_inspect_instructions(agent_ref), unsupported_reason: None, } } @@ -1273,8 +1288,9 @@ fn default_subagent_artifacts(run_id: &str) -> Vec { kind: "transcript".to_string(), name: "transcript_handle".to_string(), target: format!("agent:{run_id}"), - description: "Open loads the complete private chat artifact, including the child's agent-owned todo_write working notes; use the bounded transcript_handle with handle_read for slices and artifact metadata." - .to_string(), + description: format!( + "Open loads the complete private chat artifact, including the child's agent-owned todo_write working notes; use the bounded transcript_handle with handle_read for slices and artifact metadata; {HANDLE_READ_ACTIVATION_HINT}." + ), }, AgentRunArtifactRef { kind: "receipt".to_string(), @@ -7597,10 +7613,7 @@ async fn subagent_session_projection( supported: true, agent_id: snapshot.agent_id.clone(), session_name: Some(snapshot.name.clone()), - instructions: format!( - "Inspect agent '{}' through the returned transcript_handle with handle_read; open a replacement with agent if the lane no longer fits.", - snapshot.agent_id - ), + instructions: agent_transcript_inspect_instructions(&snapshot.agent_id), unsupported_reason: None, }); let artifacts = worker_record diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 3b571f4fe8..006f9c7d2f 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -21069,3 +21069,73 @@ fn forkguard_subagent_skill_catalog_uses_tool_search_discovery() { Skills unloadable:\n{catalog}" ); } + +// Worker records, takeover targets, and transcript artifact descriptions all +// point the parent at `handle_read`, which is deferred on stock hosts; every +// prose site that names it must carry the `tool_search` activation hint so +// the parent is never commanded to call a tool it cannot see (Pinvou #490 +// phantom-tool class). Statuses whose recommended tool is `agent` (first-turn +// active wherever children exist) or none are exempt. +#[test] +fn forkguard_worker_record_hints_teach_handle_read_activation() { + assert!( + HANDLE_READ_ACTIVATION_HINT.contains("`tool_search`"), + "shared hint must name the activation tool:\n{HANDLE_READ_ACTIVATION_HINT}" + ); + + let instructions = agent_transcript_inspect_instructions("worker_1"); + assert!( + instructions.contains("with handle_read") + && instructions.contains(HANDLE_READ_ACTIVATION_HINT), + "takeover/projection inspect briefs must pair handle_read with the \ + activation hint:\n{instructions}" + ); + + let transcript = default_subagent_artifacts("run_1") + .iter() + .find(|artifact| artifact.name == "transcript_handle") + .expect("transcript artifact must be listed") + .clone(); + assert!( + transcript.description.contains("with handle_read") + && transcript.description.contains(HANDLE_READ_ACTIVATION_HINT), + "transcript artifact description must carry the activation hint:\n{}", + transcript.description + ); + + let default_action = default_agent_run_recommended_action(); + assert!( + default_action.reason.contains(HANDLE_READ_ACTIVATION_HINT), + "default recommended action must carry the activation hint:\n{}", + default_action.reason + ); + + let spec = make_worker_spec("worker_1", PathBuf::from(".")); + for status in [ + AgentWorkerStatus::WaitingForUser, + AgentWorkerStatus::Completed, + AgentWorkerStatus::Failed, + AgentWorkerStatus::Interrupted, + ] { + let action = recommended_action_for_worker_status(status, &spec); + assert_eq!(action.tool.as_deref(), Some("handle_read")); + assert!( + action.reason.contains(HANDLE_READ_ACTIVATION_HINT), + "{status:?} recommends handle_read without the activation hint:\n{}", + action.reason + ); + } + + for status in [ + AgentWorkerStatus::Queued, + AgentWorkerStatus::Starting, + AgentWorkerStatus::Running, + AgentWorkerStatus::ModelWait, + AgentWorkerStatus::RunningTool, + ] { + let action = recommended_action_for_worker_status(status, &spec); + assert_eq!(action.tool, None, "{status:?} must not recommend a tool"); + } + let cancelled = recommended_action_for_worker_status(AgentWorkerStatus::Cancelled, &spec); + assert_eq!(cancelled.tool.as_deref(), Some("agent")); +} diff --git a/crates/tui/src/tools/web_run.rs b/crates/tui/src/tools/web_run.rs index 5b5c252762..1fd2a15a4b 100644 --- a/crates/tui/src/tools/web_run.rs +++ b/crates/tui/src/tools/web_run.rs @@ -1212,6 +1212,10 @@ fn bounded_web_run_result( "artifact_relative_path": crate::artifacts::format_artifact_relative_path(&artifact.relative_path), "artifact_byte_size": artifact.byte_size, "artifact_preview": artifact.preview, + // The overflow footer points at `retrieve_tool_result`; flag the + // evidence so the engine auto-activates that tool next turn (same + // contract as the shell-truncation spillover). + "evidence_available": true, }) }); @@ -1653,6 +1657,12 @@ mod tests { assert!(result.content.contains("retrieve_tool_result")); assert!(result.content.chars().count() <= inline_char_budget(&context)); + assert_eq!( + metadata["evidence_available"], + json!(true), + "overflow footer points at retrieve_tool_result; the metadata must \ + flag the evidence so the engine auto-activates that tool:\n{metadata}" + ); assert_eq!( serde_json::from_str::(&full).unwrap()["warnings"][0], output.warnings[0] From dc1f391d6e8876f9627284a868ea7d3cf279326a Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 16 Sep 2026 22:20:47 +0800 Subject: [PATCH 8/8] fix(fork): close the round-three review gaps A third independent review pass found no functional defects and no remaining phantom-tool sites on stock hosts; it did surface small consistency and honesty gaps, fixed here: - The /agent dispatch brief and the bare /goal brief taught only the two-step `tool_search` activation. Explicit allowed_tools can strip `tool_search` itself - the exact corner the goal-continuation and parent-context pins already require the direct-call fallback for - so both briefs now carry the third tier and their pins assert it. - The engine's parent-context hint re-typed the first two tiers of HANDLE_READ_ACTIVATION_HINT verbatim; the constant is pub(crate) now and the hint formats it in (rendered text byte-identical). - fetch_url flags every artifact it writes as retrievable evidence, not just text overflows: binary PDF/media saves set evidence_available so the saved-artifact pointer line is actionable instead of a dead end. The behavior is kept; the comment and the assertion message claimed overflow-only and now describe the actual contract. - The bundled-skills phantom denylist is a hand-maintained superset of the canonical retired and hidden-alias lists; it is now hoisted to module scope and anchored by a subset assertion against RETIRED_TOOL_NAMES/HIDDEN_COMPAT_TOOL_NAMES so a newly registered alias cannot silently skip the sweep. - A test assertion message carried a fourteen-space line-continuation accident; whitespace only. New pin: forkguard_phantom_denylist_covers_canonical_lists; the slash-agent and bare-/goal pins gain the direct-call assertions. Verified: cargo fmt --check clean; cargo clippy --workspace --all-features --locked (fork-ci invocation) clean; RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib passes 11769/0 (one run reported two timing flakes that pass on immediate rerun on the same tree); no Cargo.lock drift. Fork-guard fingerprints for the new brief tiers and the denylist anchor ride in Pinvou/pinvou-agent#490. Signed-off-by: asto18089 --- crates/tui/src/commands/groups/core/agent.rs | 9 +- .../tui/src/commands/groups/project/goal.rs | 8 +- crates/tui/src/core/engine/context.rs | 5 +- crates/tui/src/skills/system/tests.rs | 92 ++++++++++++------- crates/tui/src/tools/canonical_action.rs | 68 +++++++------- crates/tui/src/tools/fetch_url.rs | 13 ++- crates/tui/src/tools/mcp_registry.rs | 2 +- crates/tui/src/tools/subagent/mod.rs | 4 +- 8 files changed, 123 insertions(+), 78 deletions(-) diff --git a/crates/tui/src/commands/groups/core/agent.rs b/crates/tui/src/commands/groups/core/agent.rs index 9b18aea36b..3363f06d1f 100644 --- a/crates/tui/src/commands/groups/core/agent.rs +++ b/crates/tui/src/commands/groups/core/agent.rs @@ -59,7 +59,8 @@ pub fn agent(_app: &mut App, arg: Option<&str>) -> CommandResult { } }; let message = format!( - "Launch one sub-agent for this task by calling `agent` with name `slash_agent`, `prompt: {task:?}`, and `max_depth: {max_depth}`. Use `handle_read` on the returned transcript_handle if you need more detail; if it is not in your tool list, activate it via `tool_search` first. Verify any claimed side effects before reporting success." + "Launch one sub-agent for this task by calling `agent` with name `slash_agent`, `prompt: {task:?}`, and `max_depth: {max_depth}`. Use `handle_read` on the returned transcript_handle if you need more detail; {handle_read_hint}; if `tool_search` cannot surface it, call `handle_read` directly anyway, since registered deferred tools hydrate when called by name. Verify any claimed side effects before reporting success.", + handle_read_hint = crate::tools::subagent::HANDLE_READ_ACTIVATION_HINT ); CommandResult::with_message_and_action( format!("Opening persistent sub-agent at depth {max_depth}..."), @@ -141,5 +142,11 @@ mod tests { message.contains("activate it via `tool_search` first"), "the dispatch brief must teach the handle_read activation path:\n{message}" ); + assert!( + message.contains("call `handle_read` directly anyway"), + "allowed_tools-filtered sessions can strip tool_search too; the \ + dispatch brief must keep the direct-call fallback instead of \ + dead-ending:\n{message}" + ); } } diff --git a/crates/tui/src/commands/groups/project/goal.rs b/crates/tui/src/commands/groups/project/goal.rs index 8c720e6ffb..e41c1a769f 100644 --- a/crates/tui/src/commands/groups/project/goal.rs +++ b/crates/tui/src/commands/groups/project/goal.rs @@ -93,7 +93,7 @@ fn goal_command( task in flight, recent findings, open items) and set it by calling \ `create_goal` with the full objective (and a token_budget only if one was \ discussed); if `create_goal` is not in your tool list, activate it via \ - `tool_search` first. Then continue working toward it. Only if the conversation \ + `tool_search` first; if `tool_search` cannot surface it, call `create_goal` directly anyway, since registered deferred tools hydrate when called by name. Then continue working toward it. Only if the conversation \ genuinely contains no work yet, ask the user what the goal should be." .to_string(); CommandResult::with_message_and_action( @@ -464,6 +464,12 @@ mod tests { "create_goal is deferred on stock hosts; the bare /goal brief must \ teach the activation path instead of commanding an absent tool:\n{message}" ); + assert!( + message.contains("call `create_goal` directly anyway"), + "allowed_tools-filtered sessions can strip tool_search too; the \ + bare /goal brief must keep the direct-call fallback instead of \ + dead-ending:\n{message}" + ); } #[test] diff --git a/crates/tui/src/core/engine/context.rs b/crates/tui/src/core/engine/context.rs index 11256a0164..8585c8aaf0 100644 --- a/crates/tui/src/core/engine/context.rs +++ b/crates/tui/src/core/engine/context.rs @@ -216,7 +216,10 @@ fn compact_subagent_tool_result_for_context(tool_name: &str, raw: &str) -> Optio out.push_str( "Child results are self-reports; verify side effects with `read` or `bash` before claiming success.\n", ); - out.push_str("Use `handle_read` on `transcript_handle` for bounded transcript slices when the returned summary is not enough — if `handle_read` is not in your tool list, activate it via `tool_search` first; if `tool_search` cannot surface it, call `handle_read` directly anyway, since registered deferred tools hydrate when called by name.\n"); + out.push_str(&format!( + "Use `handle_read` on `transcript_handle` for bounded transcript slices when the returned summary is not enough — {handle_read_hint}; if `tool_search` cannot surface it, call `handle_read` directly anyway, since registered deferred tools hydrate when called by name.\n", + handle_read_hint = crate::tools::subagent::HANDLE_READ_ACTIVATION_HINT + )); for (idx, snapshot) in snapshots.iter().enumerate() { if idx >= 8 { out.push_str(&format!( diff --git a/crates/tui/src/skills/system/tests.rs b/crates/tui/src/skills/system/tests.rs index cfe4d918c3..3a552fc038 100644 --- a/crates/tui/src/skills/system/tests.rs +++ b/crates/tui/src/skills/system/tests.rs @@ -80,43 +80,44 @@ fn bundled_integration_skills_use_current_codewhale_commands_and_paths() { // not a phantom. Scope note: `v4-best-practices` and `feishu` are not in // BUNDLED_SKILLS (legacy/optional, never auto-installed), so this sweep does // not cover them. +const PHANTOM_NAMES: &[&str] = &[ + "`File`", + "`Bash`", + "`exec_shell`", + "`exec_shell_wait`", + "`exec_shell_interact`", + "`exec_shell_cancel`", + "`read_file`", + "`write_file`", + "`edit_file`", + "`fetch_url`", + "`web_fetch`", + "`web_search`", + "`work_update`", + "`TodoWrite`", + "`todo`", + "`checklist_write`", + "`checklist_update`", + "`update_plan`", + "`rlm`", + "`run_tests`", + "`run_verifiers`", + "`git_status`", + "`git_diff`", + "`git_log`", + "`git_show`", + "`git_blame`", + "`wait_for_dev_server`", + "`agents/list`", + "`agents/message`", + "`agents/coordinate`", + "`agents/followup`", + "`agents/interrupt`", + "`agents/wait`", +]; + #[test] fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { - const PHANTOM_NAMES: &[&str] = &[ - "`File`", - "`Bash`", - "`exec_shell`", - "`exec_shell_wait`", - "`exec_shell_interact`", - "`exec_shell_cancel`", - "`read_file`", - "`write_file`", - "`edit_file`", - "`fetch_url`", - "`web_fetch`", - "`web_search`", - "`work_update`", - "`TodoWrite`", - "`todo`", - "`checklist_write`", - "`checklist_update`", - "`update_plan`", - "`rlm`", - "`run_tests`", - "`run_verifiers`", - "`git_status`", - "`git_diff`", - "`git_log`", - "`git_show`", - "`git_blame`", - "`wait_for_dev_server`", - "`agents/list`", - "`agents/message`", - "`agents/coordinate`", - "`agents/followup`", - "`agents/interrupt`", - "`agents/wait`", - ]; for skill in BUNDLED_SKILLS { for phantom in PHANTOM_NAMES { assert!( @@ -130,6 +131,27 @@ fn forkguard_bundled_skills_cite_no_hidden_or_retired_tool_names() { } } +// The denylist above is a hand-maintained superset of the canonical lists in +// `tools/canonical_action.rs` (`RETIRED_TOOL_NAMES`, `HIDDEN_COMPAT_TOOL_NAMES`) +// plus extra fork-era names (the todo family, `rlm`, the `agents/*` set). A +// newly registered hidden alias would otherwise skip this sweep silently, so +// anchor the hand list to the canonical one: every canonical entry must stay +// covered here in its backticked citation form. +#[test] +fn forkguard_phantom_denylist_covers_canonical_lists() { + let canonical = crate::tools::canonical_action::RETIRED_TOOL_NAMES + .iter() + .chain(crate::tools::canonical_action::HIDDEN_COMPAT_TOOL_NAMES); + for name in canonical { + let cited = format!("`{name}`"); + assert!( + PHANTOM_NAMES.contains(&cited.as_str()), + "PHANTOM_NAMES must cover canonical entry {cited} or retired/hidden \ + names can re-enter bundled skill bodies unguarded" + ); + } +} + // Regression: `create_goal` is deferred-but-`tool_search`-searchable in main // sessions and removed from subagent registries entirely (tools/subagent/ // mod.rs drops it before catalog filtering and search), so the best-of-n body diff --git a/crates/tui/src/tools/canonical_action.rs b/crates/tui/src/tools/canonical_action.rs index 5f09580ea5..51bd46ac27 100644 --- a/crates/tui/src/tools/canonical_action.rs +++ b/crates/tui/src/tools/canonical_action.rs @@ -201,44 +201,46 @@ pub(crate) fn canonical_action_alias<'a>(tool_name: &'a str, input: &Value) -> & .unwrap_or(tool_name) } +/// Names the v0.9.3 consolidation retired from the advertised catalog. +/// Fully removed spellings cannot dispatch at all — `ToolRegistry::resolve` +/// has no fuzzy step — and the rest survive only as `model_visible=false` +/// replay aliases, so any one of them inside a model-visible description or +/// schema teaches a name the model is never offered. +// list_dir, file_search and grep_files are published standalone tools again. +#[cfg(test)] +pub(crate) const RETIRED_TOOL_NAMES: &[&str] = &[ + "read_file", + "write_file", + "edit_file", + "git_status", + "git_diff", + "git_log", + "git_show", + "git_blame", + "run_tests", + "run_verifiers", + "web_search", + "fetch_url", + "wait_for_dev_server", + "exec_shell", + "exec_shell_wait", + "exec_shell_interact", + "exec_shell_cancel", +]; + +/// Uppercase action-family aliases that still dispatch for saved v0.9.x +/// transcript replay but are `model_visible=false`: new sessions publish +/// `bash` and the independent `read`/`write`/`edit` primitives instead +/// (`canonical_runtime_tools_hide_compatibility_aliases`). Matching is +/// whole-token so prose like "BashHistory" or lowercase `bash` never trips. +#[cfg(test)] +pub(crate) const HIDDEN_COMPAT_TOOL_NAMES: &[&str] = &["Bash", "File"]; + #[cfg(test)] mod tests { use super::*; use serde_json::json; - /// Names the v0.9.3 consolidation retired from the advertised catalog. - /// Fully removed spellings cannot dispatch at all — `ToolRegistry::resolve` - /// has no fuzzy step — and the rest survive only as `model_visible=false` - /// replay aliases, so any one of them inside a model-visible description or - /// schema teaches a name the model is never offered. - // list_dir, file_search and grep_files are published standalone tools again. - const RETIRED_TOOL_NAMES: &[&str] = &[ - "read_file", - "write_file", - "edit_file", - "git_status", - "git_diff", - "git_log", - "git_show", - "git_blame", - "run_tests", - "run_verifiers", - "web_search", - "fetch_url", - "wait_for_dev_server", - "exec_shell", - "exec_shell_wait", - "exec_shell_interact", - "exec_shell_cancel", - ]; - - /// Uppercase action-family aliases that still dispatch for saved v0.9.x - /// transcript replay but are `model_visible=false`: new sessions publish - /// `bash` and the independent `read`/`write`/`edit` primitives instead - /// (`canonical_runtime_tools_hide_compatibility_aliases`). Matching is - /// whole-token so prose like "BashHistory" or lowercase `bash` never trips. - const HIDDEN_COMPAT_TOOL_NAMES: &[&str] = &["Bash", "File"]; - /// The catalog is re-sent on every request, so a retired name in it is a /// per-turn lie to every model. `verifier.rs` already guarded one such /// description by hand; this covers the whole advertised surface at once. diff --git a/crates/tui/src/tools/fetch_url.rs b/crates/tui/src/tools/fetch_url.rs index f2d9170f90..141039d4ab 100644 --- a/crates/tui/src/tools/fetch_url.rs +++ b/crates/tui/src/tools/fetch_url.rs @@ -453,10 +453,13 @@ fn artifact_metadata(write: ArtifactWrite) -> Value { "artifact_relative_path": crate::artifacts::format_artifact_relative_path(&write.relative_path), "artifact_byte_size": write.byte_size, "artifact_preview": write.preview, - // The overflow footer tells the model to recover via - // `retrieve_tool_result`; this flag is what makes the engine - // auto-activate that tool on the next turn (same contract as the - // shell-truncation spillover), so the named tool is actually present. + // Every artifact this tool writes is retrievable evidence, so flag it + // for the engine's `activate_result_dependencies` (same contract as + // the shell-truncation spillover): the next turn auto-activates + // `retrieve_tool_result`. Text overflows name that tool in their + // footer; binary PDF/media saves name only the saved-artifact path in + // their inline pointer, so this flag is what makes that pointer + // actionable rather than a dead end. "evidence_available": true, }) } @@ -616,7 +619,7 @@ mod tests { assert_eq!( metadata.get("evidence_available"), Some(&json!(true)), - "overflow metadata must flag retrievable evidence:\n{metadata}" + "artifact metadata must flag retrievable evidence:\n{metadata}" ); } diff --git a/crates/tui/src/tools/mcp_registry.rs b/crates/tui/src/tools/mcp_registry.rs index 0bc557f20e..ef9017bc42 100644 --- a/crates/tui/src/tools/mcp_registry.rs +++ b/crates/tui/src/tools/mcp_registry.rs @@ -1907,7 +1907,7 @@ mod tests { ); assert!( REGISTRY_FIRST_PROMPT.contains("run tool_search first to activate it"), - "start_registry_mcp_server is deferred independently of registry_sync; the prompt must name the activation path:\n{REGISTRY_FIRST_PROMPT}" + "start_registry_mcp_server is deferred independently of registry_sync; the prompt must name the activation path:\n{REGISTRY_FIRST_PROMPT}" ); } } diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index a9cb00629a..2bbd303ea2 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -900,7 +900,9 @@ fn default_agent_inspect_tool() -> String { /// `handle_read` is deferred on stock hosts, so model-facing text that pairs /// it with a transcript handle must teach the activation path instead of /// commanding a tool absent from the first-turn catalog (Pinvou #490 class). -const HANDLE_READ_ACTIVATION_HINT: &str = +/// `pub(crate)` so the engine's parent-context hint reuses the exact wording +/// instead of re-typing a drifting copy. +pub(crate) const HANDLE_READ_ACTIVATION_HINT: &str = "if `handle_read` is not in your tool list, activate it via `tool_search` first"; /// Shared inspect brief for worker records and takeover targets; both name