From a7f64b5c4830525dd91e2d3a5dad86effead97e6 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 13 Oct 2026 16:42:24 +0800 Subject: [PATCH 1/3] feat(tools): disable the deferred-toolbox size bounds in the Pinvou fork Upstream bounds the conversation toolbox to 8 names / 16 KiB with silent LRU eviction. Under provider prompt-cache pricing every eviction mutates the request tools array, which re-prefills the system + tools + entire conversation history at miss price (plus the new 1.25x-2x cache-write premiums on Anthropic, Kimi k3, and GPT-5.6+), to stop paying ~0.1x on a few hundred tokens of schema: a mid-task eviction costs ~75+ tool-free requests to break even and completes an eviction -> notify -> re-search -> re-activate cycle worth ~2.5x the whole history. The live failure was a model fabricating tool results for tools that had silently vanished mid-task. Full analysis: codewhale-hq/Codewhale#6969. This fork sets TOOL_ACTIVATION_CACHE_MAX_NAMES and TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES to usize::MAX, keeping upstream's admission bookkeeping and catalog-shrink revalidation (disconnect, deny rule, mode switch, eager promotion, session sync) exactly as-is; only the size bounds never fire. The search-result oversize reason is now derived from the constants so the prose cannot disagree with them. Tests rewritten for the disabled posture (upstream tests pinned the eviction semantics): activation above the historical bounds retains everything, oversized schemas are admitted, the recency touch reorders without displacing, and a forkguard pin locks the disabled posture. docs/TOOL_SURFACE.md states the fork divergence. Signed-off-by: asto --- crates/tui/src/core/engine/tool_catalog.rs | 16 +++- .../tui/src/core/engine/tool_catalog/tests.rs | 8 +- crates/tui/src/core/session.rs | 86 ++++++++++++------- docs/TOOL_SURFACE.md | 17 ++-- 4 files changed, 85 insertions(+), 42 deletions(-) diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index 8863cb9129..67fe41b54f 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -18,7 +18,9 @@ use crate::models::Tool; use crate::tools::spec::{ToolError, ToolResult, optional_str, optional_u64, required_str}; use crate::tui::app::AppMode; -use crate::core::session::ToolActivationCache; +use crate::core::session::{ + TOOL_ACTIVATION_CACHE_MAX_NAMES, TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES, ToolActivationCache, +}; use crate::dependencies::ExternalTool; use crate::regex_cache::compile_user_regex; @@ -1382,11 +1384,21 @@ fn execute_tool_search_inner( }) }) .collect::>(); + // The bounds statement is derived from the session constants so the prose + // can never disagree with them. With the Pinvou fork's disabled bounds + // (usize::MAX) this arm is unreachable for size reasons and only reports + // requests the activation could not admit for other causes. + let toolbox_bounds_reason = format!( + "The tool schema exceeds the bounded conversation toolbox ({} cached tools, {}KiB of \ + added serialized schemas). Narrow the search or use a smaller matching tool.", + TOOL_ACTIVATION_CACHE_MAX_NAMES, + TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES / 1024 + ); unavailable_references.extend(cache_rejected.iter().map(|name| { json!({ "type": "unavailable_tool_reference", "tool_name": name, - "reason": "The tool schema exceeds the bounded conversation toolbox (8 cached tools, 16KiB of added serialized schemas). Narrow the search or use a smaller matching tool." + "reason": toolbox_bounds_reason, }) })); diff --git a/crates/tui/src/core/engine/tool_catalog/tests.rs b/crates/tui/src/core/engine/tool_catalog/tests.rs index 9154cb7f35..4d015dee2f 100644 --- a/crates/tui/src/core/engine/tool_catalog/tests.rs +++ b/crates/tui/src/core/engine/tool_catalog/tests.rs @@ -143,7 +143,10 @@ fn cache_eviction_does_not_hide_a_tool_that_became_eager() { } #[test] -fn successful_cached_execution_updates_lru_without_granting_uncached_names() { +fn successful_cached_execution_keeps_every_activation_without_granting_uncached_names() { + // Pinvou fork: the toolbox bounds are disabled, so activating a ninth + // deferred tool no longer displaces the least-recently-used entry. The + // recency touch and the uncached-name guarantee are unchanged. let catalog = (0..=8) .map(|index| { let mut definition = tool(&format!("deferred-{index}")); @@ -168,8 +171,9 @@ fn successful_cached_execution_updates_lru_without_granting_uncached_names() { let delta = cache.activate(&catalog, &["deferred-8".to_string()]); remove_evicted_cache_activations(&catalog, &mut active, delta.evicted); active.extend(delta.admitted); + assert_eq!(cache.names().count(), 9); assert!(cache.names().any(|name| name == "deferred-0")); - assert!(!cache.names().any(|name| name == "deferred-1")); + assert!(cache.names().any(|name| name == "deferred-1")); assert!(!touch_cached_tool_after_execution( &catalog, diff --git a/crates/tui/src/core/session.rs b/crates/tui/src/core/session.rs index e654eb8b3d..d8dbd8a84f 100644 --- a/crates/tui/src/core/session.rs +++ b/crates/tui/src/core/session.rs @@ -14,16 +14,35 @@ use std::path::PathBuf; /// Maximum number of deferred schemas a conversation may keep in its active /// toolbox. The permanent `read`/`write`/`edit`/`bash`/`agent`/`tool_search` /// router surface is not counted here. -pub(crate) const TOOL_ACTIVATION_CACHE_MAX_NAMES: usize = 8; +/// +/// Pinvou fork: effectively unbounded. Upstream bounds the toolbox to 8 +/// names / 16 KiB with silent LRU eviction; under provider prompt-cache +/// pricing every eviction mutates the request tools array and re-prefills the +/// entire prefix (write premiums included), which costs orders of magnitude +/// more than the ~0.1x cached-read saving it buys (codewhale-hq/Codewhale#6969), +/// and the live incident class was a model fabricating results for tools that +/// had silently vanished mid-task. This fork keeps the admission bookkeeping +/// and the catalog-shrink revalidation exactly as upstream — a disconnected +/// server, deny rule, mode switch, or eager promotion still drops entries — +/// but the size bounds never fire, so an activated schema stays callable for +/// the conversation's lifetime. The value is the largest the engine accepts: +/// clippy's `absurd_extreme_comparisons` (deny-by-default) rejects the bounds +/// arithmetic against `usize::MAX` itself, so the bound sits one ULP below +/// arithmetic reach instead. +pub(crate) const TOOL_ACTIVATION_CACHE_MAX_NAMES: usize = usize::MAX / 2; /// Maximum serialized bytes added to requests by cached deferred schemas. -pub(crate) const TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES: usize = 16 * 1024; +/// +/// Pinvou fork: effectively unbounded; see [`TOOL_ACTIVATION_CACHE_MAX_NAMES`] +/// for why the bound is `usize::MAX / 2` rather than `usize::MAX`. +pub(crate) const TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES: usize = usize::MAX / 2; -/// Bounded, process-local conversation cache for tools activated by -/// `tool_search`. +/// Process-local conversation cache for tools activated by `tool_search`. /// /// Only names are retained. Every turn revalidates them against the currently /// filtered catalog, so a disconnected MCP server, changed allow/deny rule, or -/// mode switch cannot resurrect a tool from an older authority posture. +/// mode switch cannot resurrect a tool from an older authority posture. The +/// Pinvou fork disables the size bounds (see the constants above); upstream's +/// 8-name/16KiB LRU eviction no longer fires here. #[derive(Debug, Clone, Default)] pub(crate) struct ToolActivationCache { /// Least-recently used at the front, most-recently used at the back. @@ -458,8 +477,11 @@ mod tests { } #[test] - fn tool_activation_cache_is_lru_bounded_to_eight_names() { - let catalog = (0..10) + fn forkguard_deferred_toolbox_bounds_are_disabled() { + // The Pinvou fork disables the bounded-toolbox eviction (see the + // constant docs): activating far more names than upstream's historical + // 8-name / 16KiB bounds must retain every admission and evict nothing. + let catalog = (0..64) .map(|index| deferred_tool(&format!("tool_{index}"), 8)) .collect::>(); let requested = catalog @@ -469,20 +491,29 @@ mod tests { let mut cache = ToolActivationCache::default(); let delta = cache.activate(&catalog, &requested); - assert_eq!(cache.names().count(), TOOL_ACTIVATION_CACHE_MAX_NAMES); - assert_eq!( - cache.names().collect::>(), - vec![ - "tool_2", "tool_3", "tool_4", "tool_5", "tool_6", "tool_7", "tool_8", "tool_9" - ] - ); - assert_eq!(delta.admitted.len(), TOOL_ACTIVATION_CACHE_MAX_NAMES); - assert!(delta.evicted.contains(&"tool_0".to_string())); - assert!(delta.evicted.contains(&"tool_1".to_string())); + assert_eq!(cache.names().count(), 64); + assert_eq!(delta.admitted, requested); + assert!(delta.evicted.is_empty()); + assert!(delta.rejected.is_empty()); + } + + #[test] + fn oversized_schema_is_admitted_while_the_bounds_are_disabled() { + // 17 KiB exceeds upstream's historical 16 KiB per-tool bound; with the + // bounds disabled it is admitted like any other schema. (A literal is + // used instead of MAX + 1 because the bound is usize::MAX here.) + let catalog = vec![deferred_tool("huge", 17 * 1024)]; + let mut cache = ToolActivationCache::default(); + let delta = cache.activate(&catalog, &["huge".to_string()]); + + assert_eq!(cache.names().count(), 1); + assert_eq!(delta.admitted, vec!["huge".to_string()]); + assert!(delta.rejected.is_empty()); + assert!(delta.evicted.is_empty()); } #[test] - fn touching_a_cached_tool_makes_it_most_recent() { + fn touching_a_cached_tool_moves_it_to_the_back_and_evicts_nothing() { let catalog = (0..9) .map(|index| deferred_tool(&format!("tool_{index}"), 8)) .collect::>(); @@ -496,24 +527,15 @@ mod tests { cache.activate(&catalog, &["tool_8".to_string()]); let names = cache.names().collect::>(); + // The recency order is still maintained (most recent at the back), but + // with the bounds disabled the touched-then-re-added tool no longer + // displaces the least-recently-used entry. + assert_eq!(names.len(), 9); assert!(names.contains(&"tool_0")); - assert!(!names.contains(&"tool_1")); + assert!(names.contains(&"tool_1")); assert_eq!(names.last().copied(), Some("tool_8")); } - #[test] - fn oversized_schema_is_never_admitted() { - let catalog = vec![deferred_tool( - "huge", - TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES + 1, - )]; - let mut cache = ToolActivationCache::default(); - let delta = cache.activate(&catalog, &["huge".to_string()]); - - assert_eq!(cache.names().count(), 0); - assert_eq!(delta.rejected, vec!["huge"]); - } - #[test] fn revalidate_drops_removed_denied_or_eager_tools() { let catalog = vec![deferred_tool("kept", 8), deferred_tool("gone", 8)]; diff --git a/docs/TOOL_SURFACE.md b/docs/TOOL_SURFACE.md index 641eed374d..df8427e5b8 100644 --- a/docs/TOOL_SURFACE.md +++ b/docs/TOOL_SURFACE.md @@ -75,14 +75,19 @@ user explicitly names them in `[tools].always_load`. ### Conversation toolbox cache A successful search activation is remembered by name for the current -conversation. The cache holds at most eight deferred names and 16 KiB of -serialized schemas, evicts least-recently-used entries, and revalidates every -entry against the current catalog and policy before advertising it again. A -session sync clears it. The cache cannot resurrect a removed, denied, or -newly-eager tool. +conversation. Upstream bounds the cache to eight deferred names and 16 KiB of +serialized schemas with least-recently-used eviction; this fork disables that +bound (`usize::MAX`), so every activation stays advertised for the conversation's +lifetime — under provider prompt-cache pricing, an eviction re-prefills the +entire prefix while saving only the ~0.1x cached-read cost of the schemas it +frees (see codewhale-hq/Codewhale#6969). The cache still revalidates every entry +against the current catalog and policy before advertising it again. A session +sync clears it. The cache cannot resurrect a removed, denied, or newly-eager +tool. Each subagent gets its own policy-filtered deferred catalog, always-present -`tool_search`, and bounded activation cache. Forked messages and instructions +`tool_search`, and activation cache (unbounded in this fork, like the parent). +Forked messages and instructions remain in context, but the child cache starts empty and discovers tools locally; neither forked context nor a cache can become a discovery allowlist. A child can still search every tool its own authority permits, including Web search/fetch From 196bed7fa3fb834be0e9212d9e95ddb105c77209 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 13 Oct 2026 18:15:02 +0800 Subject: [PATCH 2/3] fix(tools): rewrite stale subagent toolbox test for disabled posture Review follow-up to this PR's first commit. - tools::subagent::tests::small_surface_caches_are_independent_bounded_ and_revalidated still pinned upstream's 8-name/16KiB eviction and ran red on CI; rewritten as forkguard_small_surface_caches_are_independent_ unbounded_and_revalidated (2 KiB schemas so nine activations exceed both historical bounds) and annotated per fork policy 3.4. - Fork-posture tests renamed under the forkguard_ convention (oversized-schema admission, cached-execution retention); the session forkguard pin's fixtures grown to 1 KiB schemas so a byte-bound-only revert fails too. - Value narrative fixed: usize::MAX / 2 is an effectively-unbounded sentinel (only comparisons against the exact usize::MAX extreme are lint-rejected), not "the largest the engine accepts"; the tool_search rejection comment now states the real reachability (per-entry size sentinel only) and the prose is built lazily; the subagent hydration error no longer claims a bounded toolbox or misadvises a narrower query; TOOL_SURFACE.md states the MAX / 2 value, the unbounded-growth tradeoff, and the parent-conversation wording. Signed-off-by: asto --- crates/tui/src/core/engine/tool_catalog.rs | 22 +++++++++------- .../tui/src/core/engine/tool_catalog/tests.rs | 2 +- crates/tui/src/core/session.rs | 22 +++++++++------- crates/tui/src/tools/subagent/mod.rs | 2 +- crates/tui/src/tools/subagent/tests.rs | 26 ++++++++++++++----- docs/TOOL_SURFACE.md | 21 ++++++++------- 6 files changed, 59 insertions(+), 36 deletions(-) diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index 67fe41b54f..41e65cebbc 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -1385,20 +1385,22 @@ fn execute_tool_search_inner( }) .collect::>(); // The bounds statement is derived from the session constants so the prose - // can never disagree with them. With the Pinvou fork's disabled bounds - // (usize::MAX) this arm is unreachable for size reasons and only reports - // requests the activation could not admit for other causes. - let toolbox_bounds_reason = format!( - "The tool schema exceeds the bounded conversation toolbox ({} cached tools, {}KiB of \ - added serialized schemas). Narrow the search or use a smaller matching tool.", - TOOL_ACTIVATION_CACHE_MAX_NAMES, - TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES / 1024 - ); + // can never disagree with them. In the Pinvou fork the bounds never fire: + // discovery pre-filters deferred membership, so the only way a name lands + // here is the per-entry size sentinel (including the serde-failure case), + // which no real schema can reach. Built lazily so a normal search pays no + // formatting cost. unavailable_references.extend(cache_rejected.iter().map(|name| { json!({ "type": "unavailable_tool_reference", "tool_name": name, - "reason": toolbox_bounds_reason, + "reason": format!( + "The tool schema exceeds the bounded conversation toolbox ({} cached tools, \ + {}KiB of added serialized schemas). Narrow the search or use a smaller \ + matching tool.", + TOOL_ACTIVATION_CACHE_MAX_NAMES, + TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES / 1024 + ), }) })); diff --git a/crates/tui/src/core/engine/tool_catalog/tests.rs b/crates/tui/src/core/engine/tool_catalog/tests.rs index 4d015dee2f..103194693c 100644 --- a/crates/tui/src/core/engine/tool_catalog/tests.rs +++ b/crates/tui/src/core/engine/tool_catalog/tests.rs @@ -143,7 +143,7 @@ fn cache_eviction_does_not_hide_a_tool_that_became_eager() { } #[test] -fn successful_cached_execution_keeps_every_activation_without_granting_uncached_names() { +fn forkguard_cached_execution_keeps_every_activation_without_granting_uncached_names() { // Pinvou fork: the toolbox bounds are disabled, so activating a ninth // deferred tool no longer displaces the least-recently-used entry. The // recency touch and the uncached-name guarantee are unchanged. diff --git a/crates/tui/src/core/session.rs b/crates/tui/src/core/session.rs index d8dbd8a84f..1d7642349b 100644 --- a/crates/tui/src/core/session.rs +++ b/crates/tui/src/core/session.rs @@ -25,10 +25,11 @@ use std::path::PathBuf; /// and the catalog-shrink revalidation exactly as upstream — a disconnected /// server, deny rule, mode switch, or eager promotion still drops entries — /// but the size bounds never fire, so an activated schema stays callable for -/// the conversation's lifetime. The value is the largest the engine accepts: -/// clippy's `absurd_extreme_comparisons` (deny-by-default) rejects the bounds -/// arithmetic against `usize::MAX` itself, so the bound sits one ULP below -/// arithmetic reach instead. +/// the conversation's lifetime. `usize::MAX` itself cannot serve as the bound: +/// comparisons against the type extreme trip clippy's deny-by-default +/// `absurd_extreme_comparisons` (and rustc's `unused_comparisons`), so the +/// bound is an effectively-unbounded sentinel at `usize::MAX / 2` — any +/// slightly smaller value compiles; only the exact extreme is lint-rejected. pub(crate) const TOOL_ACTIVATION_CACHE_MAX_NAMES: usize = usize::MAX / 2; /// Maximum serialized bytes added to requests by cached deferred schemas. /// @@ -480,9 +481,11 @@ mod tests { fn forkguard_deferred_toolbox_bounds_are_disabled() { // The Pinvou fork disables the bounded-toolbox eviction (see the // constant docs): activating far more names than upstream's historical - // 8-name / 16KiB bounds must retain every admission and evict nothing. + // 8-name bound must retain every admission and evict nothing. The + // 1 KiB schemas also push the total past upstream's historical 16 KiB + // byte budget, so a byte-bound-only revert fails here too. let catalog = (0..64) - .map(|index| deferred_tool(&format!("tool_{index}"), 8)) + .map(|index| deferred_tool(&format!("tool_{index}"), 1024)) .collect::>(); let requested = catalog .iter() @@ -498,10 +501,11 @@ mod tests { } #[test] - fn oversized_schema_is_admitted_while_the_bounds_are_disabled() { + fn forkguard_oversized_schema_is_admitted_while_the_bounds_are_disabled() { // 17 KiB exceeds upstream's historical 16 KiB per-tool bound; with the - // bounds disabled it is admitted like any other schema. (A literal is - // used instead of MAX + 1 because the bound is usize::MAX here.) + // bounds disabled it is admitted like any other schema. The literal is + // sized against the historical bound because the fork bound + // (usize::MAX / 2) has no constructible neighbor. let catalog = vec![deferred_tool("huge", 17 * 1024)]; let mut cache = ToolActivationCache::default(); let delta = cache.activate(&catalog, &["huge".to_string()]); diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index c783679cc4..e1b8d9c40a 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -14158,7 +14158,7 @@ impl SubAgentToolSurface { )); } Err(anyhow!( - "Tool {name} could not enter the bounded child toolbox; use tool_search with a narrower query" + "Tool {name} could not enter the child toolbox; it is not currently available as a deferred tool in this surface" )) } } diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index c33a7eaffb..6c8142d923 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -7595,10 +7595,16 @@ fn synthetic_deferred_tool(name: &str, description_bytes: usize) -> Tool { } } -#[test] -fn small_surface_caches_are_independent_bounded_and_revalidated() { +// Pinvou fork: the toolbox size bounds are disabled (see the constant docs in +// core/session.rs). This test formerly pinned upstream's 8-name / 16 KiB LRU +// eviction and byte-budget overflow; it now pins the disabled posture while +// keeping the surface-independence and catalog-shrink revalidation pins. +#[test] +fn forkguard_small_surface_caches_are_independent_unbounded_and_revalidated() { + // 2 KiB schemas put nine activations past upstream's historical 16 KiB + // byte budget as well as its 8-name bound, so both reverts fail here. let mut catalog = (0..9) - .map(|index| synthetic_deferred_tool(&format!("deferred_{index}"), 8)) + .map(|index| synthetic_deferred_tool(&format!("deferred_{index}"), 2 * 1024)) .collect::>(); ensure_advanced_tooling(&mut catalog, AppMode::Agent, &HashSet::new()); catalog.retain(|tool| tool.name == TOOL_SEARCH_NAME || tool.name.starts_with("deferred_")); @@ -7608,7 +7614,14 @@ fn small_surface_caches_are_independent_bounded_and_revalidated() { let mut first = SubAgentToolSurface::new(catalog.clone(), &warm); let mut second = SubAgentToolSurface::new(catalog, &[]); let first_names = model_tool_names(model_request_tools(&mut first)); - assert!(!first_names.contains("deferred_0")); + assert_eq!( + first_names + .iter() + .filter(|name| name.starts_with("deferred_")) + .count(), + 9 + ); + assert!(first_names.contains("deferred_0")); assert!(first_names.contains("deferred_8")); assert!(!model_tool_names(model_request_tools(&mut second)).contains("deferred_8")); @@ -7617,7 +7630,8 @@ fn small_surface_caches_are_independent_bounded_and_revalidated() { first .catalog .push(synthetic_deferred_tool("oversized", 17 * 1024)); - assert!(first.hydrate("oversized").is_err()); + assert!(first.hydrate("oversized").is_ok()); + assert!(model_tool_names(model_request_tools(&mut first)).contains("oversized")); let mut byte_catalog = (0..3) .map(|index| synthetic_deferred_tool(&format!("bytes_{index}"), 6 * 1024)) @@ -7629,7 +7643,7 @@ fn small_surface_caches_are_independent_bounded_and_revalidated() { .collect::>(); let mut byte_surface = SubAgentToolSurface::new(byte_catalog, &byte_warm); let byte_names = model_tool_names(model_request_tools(&mut byte_surface)); - assert!(!byte_names.contains("bytes_0")); + assert!(byte_names.contains("bytes_0")); assert!(byte_names.contains("bytes_1") && byte_names.contains("bytes_2")); } diff --git a/docs/TOOL_SURFACE.md b/docs/TOOL_SURFACE.md index df8427e5b8..d86bf1efca 100644 --- a/docs/TOOL_SURFACE.md +++ b/docs/TOOL_SURFACE.md @@ -77,17 +77,20 @@ user explicitly names them in `[tools].always_load`. A successful search activation is remembered by name for the current conversation. Upstream bounds the cache to eight deferred names and 16 KiB of serialized schemas with least-recently-used eviction; this fork disables that -bound (`usize::MAX`), so every activation stays advertised for the conversation's -lifetime — under provider prompt-cache pricing, an eviction re-prefills the -entire prefix while saving only the ~0.1x cached-read cost of the schemas it -frees (see codewhale-hq/Codewhale#6969). The cache still revalidates every entry -against the current catalog and policy before advertising it again. A session -sync clears it. The cache cannot resurrect a removed, denied, or newly-eager -tool. +bound (`usize::MAX / 2`, effectively unbounded), so every activation stays +advertised for the conversation's lifetime — under provider prompt-cache +pricing, an eviction re-prefills the entire prefix while saving only the ~0.1x +cached-read cost of the schemas it frees (see codewhale-hq/Codewhale#6969). +No size ceiling remains: the activated set is the only clamp on the request +tools array, so a session that activates very many large schemas can approach +provider request-size or tool-count limits. The cache still revalidates every +entry against the current catalog and policy before advertising it again. A +session sync clears it. The cache cannot resurrect a removed, denied, or +newly-eager tool. Each subagent gets its own policy-filtered deferred catalog, always-present -`tool_search`, and activation cache (unbounded in this fork, like the parent). -Forked messages and instructions +`tool_search`, and activation cache (unbounded in this fork, like the parent +conversation's). Forked messages and instructions remain in context, but the child cache starts empty and discovers tools locally; neither forked context nor a cache can become a discovery allowlist. A child can still search every tool its own authority permits, including Web search/fetch From fd14b6a7e0ffb8c8994389bd8c3f52855756ce44 Mon Sep 17 00:00:00 2001 From: asto Date: Tue, 13 Oct 2026 20:21:00 +0800 Subject: [PATCH 3/3] fix(tools): true up toolbox prose and pins for the disabled posture Review round 2 (comment 5502767570) follow-ups, documentation and message level only: - activate()/revalidate() doc comments now state the fork posture instead of upstream's dead oversized-rejection/LRU-eviction behavior; execute_tool_search_with_cache no longer calls the cache bounded. - the tool_search unavailable_tool_reference reason claims no bound and interpolates no numbers: in this fork the arm is dead except for the per-entry serde-failure sentinel, so the prose says the schema could not be measured for admission. The session constants import goes away with it. - the constants doc scopes rustc's unused_comparisons to the numeric-literal form (empirically it does not fire through a const item); clippy's absurd_extreme_comparisons alone carries the justification. - TOOL_SURFACE.md drops the usize::MAX / 2 implementation literal and discloses the bookkeeping growth (revalidation/admission re-serialize cached schemas against the catalog on each pass). - the recency-touch test gains the forkguard_ prefix used by its siblings; oversized-fixture comment wording fixed. Verified: cargo fmt --check clean; clippy -p codewhale-tui --lib --all-features --locked -D warnings clean; core::session + core::engine::tool_catalog 23/0; full lib suite 12091 passed / 0 failed (RUST_MIN_STACK=16777216). Signed-off-by: asto --- crates/tui/src/core/engine/tool_catalog.rs | 29 ++++++++-------------- crates/tui/src/core/session.rs | 26 +++++++++++-------- docs/TOOL_SURFACE.md | 7 ++++-- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index 41e65cebbc..faa1c32d73 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -18,9 +18,7 @@ use crate::models::Tool; use crate::tools::spec::{ToolError, ToolResult, optional_str, optional_u64, required_str}; use crate::tui::app::AppMode; -use crate::core::session::{ - TOOL_ACTIVATION_CACHE_MAX_NAMES, TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES, ToolActivationCache, -}; +use crate::core::session::ToolActivationCache; use crate::dependencies::ExternalTool; use crate::regex_cache::compile_user_regex; @@ -1296,8 +1294,8 @@ pub(super) fn execute_tool_search( execute_tool_search_inner(tool_name, input, catalog, active_tools, None, None) } -/// Execute tool search while retaining activated schemas in the bounded -/// conversation cache. Kept separate from the test-only pure search helper so existing +/// Execute tool search while retaining activated schemas in the conversation +/// cache. Kept separate from the test-only pure search helper so existing /// pure catalog tests and compatibility callers do not need an engine session. /// /// `connecting_mcp_servers` carries the boot-window status (#588): while the @@ -1384,23 +1382,18 @@ fn execute_tool_search_inner( }) }) .collect::>(); - // The bounds statement is derived from the session constants so the prose - // can never disagree with them. In the Pinvou fork the bounds never fire: - // discovery pre-filters deferred membership, so the only way a name lands - // here is the per-entry size sentinel (including the serde-failure case), - // which no real schema can reach. Built lazily so a normal search pays no - // formatting cost. + // In the Pinvou fork the toolbox size bounds never fire (see the session + // constants): discovery pre-filters deferred membership, so the only way + // a name lands here is the per-entry sentinel — a schema whose serialized + // size cannot be measured (serde failure reports usize::MAX) — which no + // real schema reaches. The reason therefore claims no bound and + // interpolates no numbers. unavailable_references.extend(cache_rejected.iter().map(|name| { json!({ "type": "unavailable_tool_reference", "tool_name": name, - "reason": format!( - "The tool schema exceeds the bounded conversation toolbox ({} cached tools, \ - {}KiB of added serialized schemas). Narrow the search or use a smaller \ - matching tool.", - TOOL_ACTIVATION_CACHE_MAX_NAMES, - TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES / 1024 - ), + "reason": + "The tool schema could not be measured for admission to the conversation toolbox.", }) })); diff --git a/crates/tui/src/core/session.rs b/crates/tui/src/core/session.rs index 1d7642349b..d047a387d7 100644 --- a/crates/tui/src/core/session.rs +++ b/crates/tui/src/core/session.rs @@ -27,9 +27,11 @@ use std::path::PathBuf; /// but the size bounds never fire, so an activated schema stays callable for /// the conversation's lifetime. `usize::MAX` itself cannot serve as the bound: /// comparisons against the type extreme trip clippy's deny-by-default -/// `absurd_extreme_comparisons` (and rustc's `unused_comparisons`), so the -/// bound is an effectively-unbounded sentinel at `usize::MAX / 2` — any -/// slightly smaller value compiles; only the exact extreme is lint-rejected. +/// `absurd_extreme_comparisons` (rustc's `unused_comparisons` joins in only +/// when the extreme is written as a numeric literal, not through a const +/// item), so the bound is an effectively-unbounded sentinel at +/// `usize::MAX / 2` — any slightly smaller value compiles; only the exact +/// extreme is lint-rejected. pub(crate) const TOOL_ACTIVATION_CACHE_MAX_NAMES: usize = usize::MAX / 2; /// Maximum serialized bytes added to requests by cached deferred schemas. /// @@ -89,7 +91,9 @@ impl ToolActivationCache { } /// Drop entries that are no longer deferred members of this turn's - /// filtered catalog and enforce both cache bounds. + /// filtered catalog. Upstream also evicts until both size bounds hold; + /// in the Pinvou fork the bounds are disabled (see the constants above) + /// and only the membership half can drop entries. pub(crate) fn revalidate(&mut self, catalog: &[crate::models::Tool]) -> Vec { let mut evicted = Vec::new(); self.names.retain(|name| { @@ -113,9 +117,11 @@ impl ToolActivationCache { evicted } - /// Touch requested deferred tools in search-result order. An oversized - /// schema is rejected; otherwise least-recently-used entries are evicted - /// until both bounds hold. + /// Touch requested deferred tools in search-result order. Upstream + /// rejects oversized schemas and evicts least-recently-used entries until + /// both size bounds hold; the Pinvou fork disables those bounds (see the + /// constants above), so those paths never fire and every request naming a + /// deferred catalog member is admitted. pub(crate) fn activate( &mut self, catalog: &[crate::models::Tool], @@ -504,8 +510,8 @@ mod tests { fn forkguard_oversized_schema_is_admitted_while_the_bounds_are_disabled() { // 17 KiB exceeds upstream's historical 16 KiB per-tool bound; with the // bounds disabled it is admitted like any other schema. The literal is - // sized against the historical bound because the fork bound - // (usize::MAX / 2) has no constructible neighbor. + // sized against the historical bound because no constructible schema + // can approach the fork bound (usize::MAX / 2). let catalog = vec![deferred_tool("huge", 17 * 1024)]; let mut cache = ToolActivationCache::default(); let delta = cache.activate(&catalog, &["huge".to_string()]); @@ -517,7 +523,7 @@ mod tests { } #[test] - fn touching_a_cached_tool_moves_it_to_the_back_and_evicts_nothing() { + fn forkguard_touching_a_cached_tool_moves_it_to_the_back_and_evicts_nothing() { let catalog = (0..9) .map(|index| deferred_tool(&format!("tool_{index}"), 8)) .collect::>(); diff --git a/docs/TOOL_SURFACE.md b/docs/TOOL_SURFACE.md index d86bf1efca..e0cec6947d 100644 --- a/docs/TOOL_SURFACE.md +++ b/docs/TOOL_SURFACE.md @@ -77,13 +77,16 @@ user explicitly names them in `[tools].always_load`. A successful search activation is remembered by name for the current conversation. Upstream bounds the cache to eight deferred names and 16 KiB of serialized schemas with least-recently-used eviction; this fork disables that -bound (`usize::MAX / 2`, effectively unbounded), so every activation stays +bound (effectively unbounded), so every activation stays advertised for the conversation's lifetime — under provider prompt-cache pricing, an eviction re-prefills the entire prefix while saving only the ~0.1x cached-read cost of the schemas it frees (see codewhale-hq/Codewhale#6969). No size ceiling remains: the activated set is the only clamp on the request tools array, so a session that activates very many large schemas can approach -provider request-size or tool-count limits. The cache still revalidates every +provider request-size or tool-count limits. Bookkeeping grows with the +activated set too: revalidation and admission re-serialize cached schemas +against the catalog on each pass, negligible at tens of tools but not free +for activation-heavy sessions. The cache still revalidates every entry against the current catalog and policy before advertising it again. A session sync clears it. The cache cannot resurrect a removed, denied, or newly-eager tool.