From 5a54271f7df9eb79808260aa6e57084bb0160aa8 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 10:08:07 +0800 Subject: [PATCH 01/18] fix(cli): read flat tool-result audit receipts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rollup's tool.result arm consulted only the nested details/payload placements, but the only in-repo writer of tool.result lines — the engine's opt-in tool-audit log (CODEWHALE_TOOL_AUDIT_LOG) — emits tool_name, success, and executed at the top level of the line. Every receipt from that writer therefore bucketed as an executed success: a refused oversize hydration inflated the very success count the executed marker exists to protect. Read the top-level placement too, and count an unexecuted receipt as neither success nor failure whatever its success flag says — the tool never ran, so neither vocabulary is true of it. The pin drives the flat shape and the failed-unexecuted combination. Round-9 fresh-review MAJOR. Signed-off-by: asto --- crates/cli/src/metrics.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/cli/src/metrics.rs b/crates/cli/src/metrics.rs index 179f4f1c18..42a923b4f0 100644 --- a/crates/cli/src/metrics.rs +++ b/crates/cli/src/metrics.rs @@ -310,6 +310,7 @@ fn read_audit_log(path: &Path, since: Option>, rollup: &mut Rollup let tool_name = v .pointer("/details/tool_name") .or_else(|| v.pointer("/payload/tool_name")) + .or_else(|| v.get("tool_name")) .and_then(|t| t.as_str()) .unwrap_or("unknown"); let stats = rollup.tool_mut(tool_name); @@ -330,19 +331,29 @@ fn read_audit_log(path: &Path, since: Option>, rollup: &mut Rollup // oversize-hydration refusal) answered the call without the // tool ever running, so it counts as neither — counting it // as a success would inflate the tool's real execution rate. + // The fields are read from every placement a writer uses: + // the sensitive audit log nests them under `details`, while + // the engine's opt-in tool-audit log + // (`CODEWHALE_TOOL_AUDIT_LOG`) writes them at the top level + // of the line. let success = v .pointer("/details/success") .or_else(|| v.pointer("/payload/success")) + .or_else(|| v.get("success")) .and_then(|b| b.as_bool()) .unwrap_or(true); let executed = v .pointer("/details/executed") .or_else(|| v.pointer("/payload/executed")) + .or_else(|| v.get("executed")) .and_then(|b| b.as_bool()) .unwrap_or(true); if success && executed { stats.successes += 1; - } else if !success { + } else if executed { + // Only a receipt whose tool actually ran can be a + // failure; an unexecuted one is neither, whatever its + // success flag says. stats.failures += 1; } } @@ -1036,6 +1047,14 @@ mod tests { r#"{"event":"tool.result","ts":"2026-04-01T10:01:00+00:00","details":{"tool_name":"exec_shell","success":false}}"#, r#"{"event":"tool.result","ts":"2026-04-01T10:02:00+00:00","details":{"tool_name":"deferred_tool","success":true,"executed":false}}"#, r#"{"event":"tool.result","ts":"2026-04-01T10:03:00+00:00","details":{"tool_name":"other_tool","success":true,"executed":true}}"#, + // The engine's opt-in tool-audit log + // (`CODEWHALE_TOOL_AUDIT_LOG`) writes the same fields at the top + // level of the line instead of under `details`. + r#"{"event":"tool.result","ts":"2026-04-01T10:04:00+00:00","tool_name":"flat_tool","success":true}"#, + r#"{"event":"tool.result","ts":"2026-04-01T10:05:00+00:00","tool_name":"flat_refusal","success":true,"executed":false}"#, + // An unexecuted receipt is neither success nor failure whatever + // its success flag says — the tool never ran. + r#"{"event":"tool.result","ts":"2026-04-01T10:06:00+00:00","tool_name":"flat_failed_refusal","success":false,"executed":false}"#, ]; for line in lines { writeln!(tmp, "{line}").unwrap(); @@ -1055,6 +1074,16 @@ mod tests { assert_eq!(rollup.tools["deferred_tool"].failures, 0); assert_eq!(rollup.tools["other_tool"].calls, 1); assert_eq!(rollup.tools["other_tool"].successes, 1); + // The flat placement is read: dropping the top-level fallback + // defaults these to executed successes. + assert_eq!(rollup.tools["flat_tool"].calls, 1); + assert_eq!(rollup.tools["flat_tool"].successes, 1); + assert_eq!(rollup.tools["flat_refusal"].successes, 0); + assert_eq!(rollup.tools["flat_refusal"].failures, 0); + // Deleting the `executed` guard on the failure arm turns this pin + // red: an unexecuted receipt is never a failure. + assert_eq!(rollup.tools["flat_failed_refusal"].successes, 0); + assert_eq!(rollup.tools["flat_failed_refusal"].failures, 0); } #[test] From 508038419e5c49da2f26e88676e149e6937407f1 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 10:08:18 +0800 Subject: [PATCH 02/18] test(engine): pin the mid-turn refresh admission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only test driving the mid-turn mcp_catalog_changed arm asserted deny-side outcomes, which hold vacuously when the arm never runs: a round-9 mutation deleted the whole arm and left the suite green. Add the arm's positive side against the same pool — under a permissive policy the synthetic login tool must enter the catalog carrying the pool's own definition and join the active set, which no arm-deletion mutant survives. Signed-off-by: asto --- crates/tui/src/core/engine/tests.rs | 49 +++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index d8bfe86a8f..4b885a29c9 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -28325,6 +28325,55 @@ async fn mid_turn_refresh_honors_the_turn_policy() { !active.contains("mcp_probe_authenticate"), "a build-stripped name must not re-enter the active set on a refresh: {active:?}" ); + + // The arm's positive side, against the same pool: under a policy that + // does not strip the tool, the refresh must actually admit it — into + // the catalog with the pool's own definition and into the active set. + // The deny asserts above hold vacuously if the `mcp_catalog_changed` + // arm never runs at all; these do not. + let mut admitted_catalog = Vec::new(); + let mut admitted_active = HashSet::new(); + engine + .process_tool_results( + vec![Some(synthetic_toolbox_outcome( + "call-auth-allowed", + "unrelated", + Ok(ToolResult::success("payload").with_metadata(json!({ + "mcp_catalog_changed": true, + }))), + ))], + &mut admitted_catalog, + &mut admitted_active, + &HashMap::new(), + &permissive_turn_policy(), + ) + .await; + + assert!( + admitted_catalog + .iter() + .any(|tool| tool.name == "mcp_probe_authenticate"), + "an allowed pool tool must enter the catalog on a refresh: {admitted_catalog:?}" + ); + let expected = { + let pool = engine.mcp_pool.as_ref().expect("pool").lock().await; + pool.to_api_tools() + .into_iter() + .find(|tool| tool.name == "mcp_probe_authenticate") + .expect("premise: the needs-auth pool serves the synthetic login tool") + }; + let entered = admitted_catalog + .iter() + .find(|tool| tool.name == "mcp_probe_authenticate") + .expect("premise: the tool entered the catalog"); + assert_eq!( + entered.description, expected.description, + "the admitted entry must carry the pool's definition, not a stale one" + ); + assert!( + admitted_active.contains("mcp_probe_authenticate"), + "the eager synthetic login tool must be re-advertised onto the wire: {admitted_active:?}" + ); } #[tokio::test] From 65776ebf1e494c17ef2c7a70073b387176b64b60 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 10:08:18 +0800 Subject: [PATCH 03/18] fix(engine): declare refresh content drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mid-turn refresh declaration keyed on the model-visible name set alone, so a same-name refresh serving a changed description or schema spliced new wire bytes into place and declared nothing — undeclared drift, which the C5 prefix guard treats as a real bug (a debug panic, a release cache miss). The pre-refresh code had the same gap for count-preserving changes; the name-set keying this PR introduced fixed renames but not content. When the name set ties, compare each served tool's wire JSON — the exact bytes the C5 fingerprint hashes — against the replaced entry's, and declare on any difference. The unit phase pins both directions: a byte-identical refresh stays silent, a description change declares, and the splice still keeps the slot. Signed-off-by: asto --- crates/tui/src/core/engine/tests.rs | 31 ++++++++++++++++++++++++ crates/tui/src/core/engine/turn_loop.rs | 32 +++++++++++++++++++++++-- crates/tui/src/prefix_cache.rs | 2 +- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 4b885a29c9..6263dadb9c 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -9494,6 +9494,37 @@ fn runtime_mcp_refresh_replaces_the_pool_slice() { !active.contains("mcp_old_name") && active.contains("mcp_new_name"), "the renamed surface must be reflected in the active set: {active:?}" ); + + // A same-name refresh that changes what the tool *says* (a description + // or schema update) moves wire bytes without moving any name: the C5 + // content fingerprint sees drift the name-set identity cannot. When + // the name set ties, the declaration must fall back to the replaced + // entries' serialized bytes. + let mut stale_definition = api_tool("mcp_static_read"); + stale_definition.description = "a stale description".to_string(); + let mut catalog = vec![stale_definition, api_tool("exec_shell")]; + let mut active = HashSet::new(); + let universe: HashSet = ["mcp_static_read"] + .into_iter() + .map(str::to_string) + .collect(); + let wire_changed = replace_runtime_mcp_tools( + &mut catalog, + &mut active, + &universe, + vec![api_tool("mcp_static_read")], + &cache, + ); + assert!( + wire_changed, + "a same-name refresh that changes the served definition must be declared" + ); + let names: Vec<&str> = catalog.iter().map(|tool| tool.name.as_str()).collect(); + assert_eq!( + names, + vec!["mcp_static_read", "exec_shell"], + "the content-drift declaration still splices in place" + ); } #[test] diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 3330fcf7e9..64c7e5ec93 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -1083,8 +1083,22 @@ pub(super) fn replace_runtime_mcp_tools( .collect(); // The wire list is the eager head plus the activated deferred tail; // that identity — not the catalog's length — is what a declared re-pin - // answers for. + // answers for. Membership alone is still not the whole story: a + // same-name refresh can change what a served tool *says* (description, + // schema), which the C5 content fingerprint sees as wire bytes without + // any name moving. When the name set ties, the replaced entries' wire + // JSON — the exact bytes the fingerprint hashes — decides. let wire_before = wire_visible_tool_names(tool_catalog, active_tool_names); + let previous_wire_json: std::collections::HashMap> = tool_catalog + .iter() + .filter(|tool| universe.contains(&tool.name)) + .map(|tool| { + ( + tool.name.clone(), + crate::prefix_cache::tool_to_api_json(tool), + ) + }) + .collect(); let mut slot_by_name: std::collections::HashMap = tool_catalog .iter() .enumerate() @@ -1115,7 +1129,21 @@ pub(super) fn replace_runtime_mcp_tools( } tool_catalog.retain(|tool| !departed.contains(&tool.name)); tool_catalog.extend(arrivals); - wire_visible_tool_names(tool_catalog, active_tool_names) != wire_before + if wire_visible_tool_names(tool_catalog, active_tool_names) != wire_before { + return true; + } + // Same membership: declare only when a served tool's wire JSON changed + // under its name. A name without a predecessor is a new arrival — + // eager arrivals already moved the name set above, and a deferred one + // is invisible on the wire. + tool_catalog + .iter() + .filter(|tool| universe.contains(&tool.name)) + .any(|tool| { + previous_wire_json + .get(&tool.name) + .is_some_and(|before| *before != crate::prefix_cache::tool_to_api_json(tool)) + }) } /// Every name the next request would serialize: the eager head plus the diff --git a/crates/tui/src/prefix_cache.rs b/crates/tui/src/prefix_cache.rs index 7d23f99982..d72af7a04e 100644 --- a/crates/tui/src/prefix_cache.rs +++ b/crates/tui/src/prefix_cache.rs @@ -711,7 +711,7 @@ impl PrefixStabilityManager { /// Serialize a tool to the same JSON shape the chat API receives, /// excluding internal-only fields like `allowed_callers`, `defer_loading`, /// `input_examples`, and `cache_control` that are never sent to DeepSeek. -fn tool_to_api_json(tool: &Tool) -> Option { +pub(crate) fn tool_to_api_json(tool: &Tool) -> Option { let mut value = serde_json::json!({ "type": "function", "function": { From ed65be96d1efbda72d6db1ecdd37372e4aa820f7 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 10:08:25 +0800 Subject: [PATCH 04/18] test(kimi): pin the anyOf branch-order ship path The terminator stops at the first terminating branch, so minItems beside an empty items fails closed in an anyOf only when the offending branch is reached before a terminating one; a terminating branch ahead of it lets the tool ship the repaired, instance-set-equivalent schema. Pin the ship path so the order dependence is stated in the fixtures instead of discovered, and note the call-order invariant that keeps the terminator's minItems fallback unreachable. Signed-off-by: asto --- crates/tui/src/tools/schema_sanitize.rs | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index a92f87054f..c24ede7f1d 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -2014,6 +2014,10 @@ fn mfjs_schema_can_terminate( Ok(true) } Some("array") => { + // An absent or non-integer `minItems` reads as 0. Non-integer + // values are unreachable here only because `validate_mfjs_bounds` + // rejects them (InvalidRange) on every node before the terminator + // runs — keep that call order if the walkers are reordered. if obj.get("minItems").and_then(Value::as_u64).unwrap_or(0) == 0 { return Ok(true); } @@ -2696,6 +2700,33 @@ mod kimi_tests { "the anyOf branch's variant is InvalidSchemaNode: {error:?}" ); + // The anyOf check is branch-order dependent: the terminator stops + // at the first terminating branch, so a terminating branch ahead + // of the offending one lets the tool ship — the repaired schema is + // instance-set-equivalent (the empty `items` was a no-op + // constraint), which is the desired outcome, but it means the + // fail-closed guarantee for anyOf holds only when the offending + // branch is reached. + let mut schema = json!({ + "type": "object", + "properties": { + "a": { + "anyOf": [ + {"type": "string"}, + {"type": "array", "items": {}, "minItems": 1} + ] + } + }, + "required": ["a"] + }); + sanitize_for_kimi_parameters(&mut schema) + .expect("a terminating branch ahead of the offending one ships the tool"); + assert_eq!( + schema["properties"]["a"]["anyOf"][1], + json!({"type": "array", "minItems": 1}), + "the shipped branch is the repaired, instance-set-equivalent form" + ); + let mut schema = json!({ "type": "object", "$defs": {"a": {"type": "array", "items": {}, "minItems": 1}}, From 178adc401b2c08433a77de6f4cab0ab33fd9a935 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 10:08:25 +0800 Subject: [PATCH 05/18] fix(tools): truthful child errors and step docs The child's ride rebuilt the error with a bare display, silently dropping everything but the top cause; render the source chain with the alternate syntax. Widen the Step trigger's doc to name the child request-rebuild and warm-start parks it also covers. Signed-off-by: asto --- crates/tui/src/core/session.rs | 4 +++- crates/tui/src/tools/subagent/mod.rs | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/core/session.rs b/crates/tui/src/core/session.rs index 8ef5a86b4b..6dec377800 100644 --- a/crates/tui/src/core/session.rs +++ b/crates/tui/src/core/session.rs @@ -117,7 +117,9 @@ pub(crate) enum ToolboxEvictionTrigger { /// A `tool_search` result naming the evictions in its own payload. ToolSearch, /// A non-search activation step: a hydration batch, a retrieval - /// admission, or an executed tool's refresh. + /// admission, an executed tool's refresh, or — on the child surface — + /// a request-rebuild revalidation or a warm-start overflow, which have + /// no turn boundary of their own to attribute. Step, /// The turn-entry revalidation. TurnStart, diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index cc9a107193..80809e2b89 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -14265,7 +14265,10 @@ impl SubAgentToolSurface { }; match result.as_mut() { Ok(rich) => rich.result.content.push_str(&format!("\n\n{rendered}")), - Err(error) => *error = anyhow!("{error}\n\n{rendered}"), + // `{error:#}` renders the source chain into the message: the + // rebuild replaces the error wholesale, and a bare `{error}` + // would silently drop everything but its top cause. + Err(error) => *error = anyhow!("{error:#}\n\n{rendered}"), } } From 0333249e64a3f55af39d1d25abd09f64bff8be14 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:19:40 +0800 Subject: [PATCH 06/18] fix(tools): true up the toolbox prose for the disabled bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101, confirmed independently by four reviewers. #104's prose follow-up (fd14b6a7e) was half-lost to its rebase over #101: the comment claiming the oversize reason "claims no bound and interpolates no numbers" landed, but #101 had refactored the inline format! into the shared toolbox_oversize_reason(), so the code still interpolates the session constants — under the fork's usize::MAX / 2 sentinels any render would read "(4611686018427387903 cached tools, 4503599627370495KiB ...)". Gate the oversize reason, the eviction-note capacity label, and the search-context recovery hint on the sentinel: disabled bounds render bound-free prose (a measurement failure is not fixed by a narrower query, so the hint rides only a real budget rejection); real bounds keep the numbers for upstream parity. The child's hydrate refusal now picks its cause truthfully — catalog-absent, not-deferred, or budget — instead of blaming the per-tool budget for every rejection, and drops the "bounded child toolbox" claim #104's commit message made but never landed. The stale "bounded (8 cached tools, 16KiB)" comment at the search eviction block now states the fork posture. Signed-off-by: asto --- crates/tui/src/core/engine/tool_catalog.rs | 79 +++++++++++++++------- crates/tui/src/tools/subagent/mod.rs | 34 +++++++--- 2 files changed, 79 insertions(+), 34 deletions(-) diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index 7828a9bcff..006c0ce048 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -430,11 +430,22 @@ const TOOLBOX_CATALOG_SHRINK_REASON: &str = "Previously active in this conversat /// Why a deferred tool stays out of the bounded conversation toolbox: its /// serialized form — name, description, and schema measured together — /// exceeds the per-tool budget, so no activation can admit it while it -/// stays that large. Shared by discovery-time rejections and revalidation -/// drops so the prose cannot drift from the real bounds. Search-context -/// callers append the recovery hint via [`toolbox_oversize_search_hint`]; -/// a direct-call refusal has no query to narrow and states the bounds only. +/// stays that large. In the Pinvou fork the bounds never fire (see the +/// session constants): the only reachable trigger is the serde-failure +/// measurement sentinel, and the prose then claims no bound and +/// interpolates no numbers. Shared by discovery-time rejections and +/// revalidation drops so the prose cannot drift from the real bounds. +/// Search-context callers append the recovery hint via +/// [`toolbox_oversize_search_hint`]; a direct-call refusal has no query +/// to narrow and carries the reason alone. pub(crate) fn toolbox_oversize_reason() -> String { + if crate::core::session::TOOL_ACTIVATION_CACHE_MAX_NAMES == usize::MAX / 2 + && crate::core::session::TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES == usize::MAX / 2 + { + return "The serialized tool definition could not be measured for admission to the \ + conversation toolbox." + .to_string(); + } format!( "The serialized tool exceeds the bounded conversation toolbox ({} \ cached tools, {}KiB of serialized tool definitions).", @@ -451,25 +462,40 @@ pub(crate) fn toolbox_oversize_search_hint() -> &'static str { } /// [`toolbox_oversize_reason`] with the search-context recovery hint, for -/// the `tool_search` payload's unavailable references. +/// the `tool_search` payload's unavailable references. The hint rides only +/// a real budget rejection: under the fork's disabled sentinel the reason +/// reports a measurement failure, which no narrower query can fix. fn search_oversize_reason() -> String { - format!( - "{}{}", - toolbox_oversize_reason(), - toolbox_oversize_search_hint() - ) + let reason = toolbox_oversize_reason(); + if crate::core::session::TOOL_ACTIVATION_CACHE_MAX_NAMES == usize::MAX / 2 + && crate::core::session::TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES == usize::MAX / 2 + { + return reason; + } + format!("{}{}", reason, toolbox_oversize_search_hint()) } /// The model-facing sentence naming a bounds-driven toolbox eviction and the /// recovery contract. `trigger` names the step that surfaced it; the capacity -/// label is derived from the session constants so the prose cannot drift from -/// the real bounds. The eviction may come from the admission itself or from -/// revalidation after a cached schema grew, so the wording stays neutral -/// about the cause. +/// label renders only when the bounds are real — under the fork's disabled +/// sentinel bounds there is no meaningful capacity to quote, so the +/// parenthetical is omitted. The eviction may come from the admission itself +/// or from revalidation after a cached schema grew, so the wording stays +/// neutral about the cause. pub(crate) fn bounded_toolbox_eviction_note( trigger: ToolboxEvictionTrigger, evicted: &[String], ) -> String { + if crate::core::session::TOOL_ACTIVATION_CACHE_MAX_NAMES == usize::MAX / 2 + && crate::core::session::TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES == usize::MAX / 2 + { + return format!( + "This {} evicted tools from the bounded conversation toolbox; the evicted tools \ + are no longer callable until a later search re-activates them: {}", + trigger.label(), + evicted.join(", ") + ); + } format!( "This {} evicted tools from the bounded conversation toolbox \ ({} cached tools, {}KiB of serialized tool definitions); the evicted tools \ @@ -482,9 +508,10 @@ pub(crate) fn bounded_toolbox_eviction_note( } /// Park a bounds-driven toolbox eviction for a later result to carry, -/// combining with any note already parked — turn-start revalidation and a -/// hydration batch can both evict before a result records. Shared by the -/// parent engine and the child surface so the combine contract is stated +/// queueing beside any note already parked — turn-start revalidation and a +/// hydration batch can both evict before a result records, and names +/// repeated across notes are netted when the notes render. Shared by the +/// parent engine and the child surface so the queueing contract is stated /// and pinned once. pub(crate) fn park_toolbox_eviction( notes: &mut Vec, @@ -1601,8 +1628,9 @@ fn execute_tool_search_inner( // 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. + // real schema reaches. Under that disabled-bounds posture the reason + // claims no bound and interpolates no numbers; were the bounds real, + // toolbox_oversize_reason would quote them instead. unavailable_references.extend(cache_rejected.iter().map(|name| { json!({ "type": "unavailable_tool_reference", @@ -1654,11 +1682,14 @@ fn execute_tool_search_inner( "tool_references": references, "unavailable_tool_references": unavailable_references.clone(), }); - // The activation cache is bounded (8 cached tools, 16KiB of serialized - // tool definitions), so admitting new tools can silently revoke ones this - // conversation already used. Naming the evictions in the result keeps - // the model's picture of its callable toolbox honest — otherwise it sees - // a previously working tool disappear between turns with no explanation. + // Upstream bounds the activation cache (8 cached tools, 16KiB of + // serialized tool definitions), so admitting new tools can silently + // revoke ones this conversation already used. This fork disables the + // bounds (see the session constants), so in-fork the eviction block + // below stays empty and the reporting exists for upstream parity: + // naming the evictions in the result keeps the model's picture of its + // callable toolbox honest — otherwise it sees a previously working tool + // disappear between turns with no explanation. if !cache_evicted.is_empty() { payload["evicted_tool_references"] = json!( cache_evicted diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index 80809e2b89..54f8268d63 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -14350,16 +14350,30 @@ impl SubAgentToolSurface { } return Ok(message); } - // The only rejection here is the per-tool budget: a narrower search - // cannot help, so the recovery hint must say why the tool cannot - // enter instead of promising a narrower query would. The rejection - // runs after the activation, so revalidation may have bounds-evicted - // other tools on the way; this error is the call's only result - // payload, so the note rides it instead of being absorbed. - let mut message = format!( - "Tool {name} could not enter the bounded child toolbox: {}", - toolbox_oversize_reason() - ); + // Pick the truthful rejection cause. Only the per-tool budget keeps + // the oversize prose: a narrower search cannot help there, so the + // reason says why the tool cannot enter instead of promising a + // narrower query would. A name absent from the child's filtered + // catalog has nothing to activate, and a name that is not deferred + // is directly callable already — neither is a budget rejection. + // The rejection runs after the activation, so revalidation may have + // bounds-evicted other tools on the way; this error is the call's + // only result payload, so the note rides it instead of being + // absorbed. + let mut message = match self.catalog.iter().find(|tool| tool.name == name) { + None => format!( + "Tool {name} could not enter the child toolbox: no such deferred tool in the \ + child catalog." + ), + Some(tool) if !tool.defer_loading.unwrap_or(false) => format!( + "Tool {name} could not enter the child toolbox: it is not deferred, so it is \ + directly callable and hydration has nothing to load." + ), + Some(_) => format!( + "Tool {name} could not enter the child toolbox: {}", + toolbox_oversize_reason() + ), + }; if !activation.evicted.is_empty() { message.push_str(&format!( "\n\n{}", From 99475f7262fa3556284fe250dc78adee6fbbe627 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:19:55 +0800 Subject: [PATCH 07/18] perf(tools): skip the disabled bounds pop loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101, confirmed independently by three reviewers. pop_collective_overflow's loop condition is permanently false under the fork's usize::MAX / 2 sentinel bounds, but evaluating it re-serialized every cached tool via serde_json::to_vec — once per revalidate and once per requested name inside activate, i.e. O(cache x requested) full-cache serializations per search or hydration batch, over a cache the disabled bounds let grow for the conversation's lifetime. Early-return when both constants are the disabled sentinel. Behavior is unchanged in every reachable state: the only non-real trigger, the serde-failure measurement sentinel, is classified stale by the revalidation that runs before the pop loop. Also true up the session-side docs for the fork posture: the constants doc measures serialized tool definitions (not just schemas), the struct doc drops the "Bounded" lead-in, and revalidate's doc states the bounds-enforcement half never fires in-fork. Signed-off-by: asto --- crates/tui/src/core/session.rs | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/crates/tui/src/core/session.rs b/crates/tui/src/core/session.rs index 6dec377800..8ce317d5fb 100644 --- a/crates/tui/src/core/session.rs +++ b/crates/tui/src/core/session.rs @@ -33,21 +33,22 @@ use std::path::PathBuf; /// `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. +/// Maximum serialized bytes added to requests by cached deferred tool +/// definitions. /// /// 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 deferred tools activated by +/// Process-local conversation cache for deferred tools activated by /// `tool_search` or loaded by a direct call (hydration, execution refresh, -/// retrieval admission). +/// retrieval admission). Bounded upstream; the Pinvou fork disables the size +/// bounds (see the constants above), so in-fork the cache is unbounded and +/// upstream's 8-name/16KiB LRU eviction never fires. /// /// 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. The -/// Pinvou fork disables the size bounds (see the constants above); upstream's -/// 8-name/16KiB LRU eviction no longer fires here. +/// mode switch cannot resurrect a tool from an older authority posture. #[derive(Debug, Clone, Default)] pub(crate) struct ToolActivationCache { /// Least-recently used at the front, most-recently used at the back. @@ -218,6 +219,17 @@ impl ToolActivationCache { /// Shared by revalidation and admission so the bounds condition and the /// LRU-front-first policy live in exactly one place. fn pop_collective_overflow(&mut self, catalog: &[crate::models::Tool]) -> Vec { + // Under the fork's disabled bounds the loop condition can never hold + // for real schemas — the serde-failure sentinel is removed as stale + // by revalidation before this runs — so skip the full-cache + // re-serialization each condition evaluation would cost. Without the + // early return every `activate` pays it once per requested name and + // every `revalidate` pays it once. + if TOOL_ACTIVATION_CACHE_MAX_NAMES == usize::MAX / 2 + && TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES == usize::MAX / 2 + { + return Vec::new(); + } let mut popped = Vec::new(); while self.names.len() > TOOL_ACTIVATION_CACHE_MAX_NAMES || self.total_serialized_bytes(catalog) > TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES @@ -234,7 +246,9 @@ impl ToolActivationCache { /// filtered catalog and enforce both cache bounds, classifying every /// removal by recoverability. A tool that became eager is forgotten /// from the cache without being reported: it is directly callable, and - /// the eager catalog entry must stay active. + /// the eager catalog entry must stay active. In the Pinvou fork the + /// bounds-enforcement half never fires (see the constants above); only + /// the membership/eager/oversize-sentinel classification is live. pub(crate) fn revalidate(&mut self, catalog: &[crate::models::Tool]) -> ToolActivationRemovals { let mut removals = ToolActivationRemovals::default(); self.names.retain(|name| { From 8be52020ed4ae424e215e7a81a8d8de03d6c301c Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:19:55 +0800 Subject: [PATCH 08/18] test(engine): pin search premises against the active-set fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101. initial_active_tools' empty-scan fallback inserts the first catalog entry into the active set before any search runs, so two tool-search tests' post-search active.contains(...) asserts were vacuous — a mutant whose search never activates into the cache stayed green. Both tests now assert the cache population directly. The render test's cross-note dedup pinned the mention count but not which note survives; it now pins first-note-wins attribution. The new forkguard_toolbox_prose_claims_no_bound_under_the_disabled_sentinel pins the disabled-sentinel prose verbatim — red against the pre-fix code, which interpolated the sentinel numbers. Signed-off-by: asto --- .../tui/src/core/engine/tool_catalog/tests.rs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/crates/tui/src/core/engine/tool_catalog/tests.rs b/crates/tui/src/core/engine/tool_catalog/tests.rs index bd2fcd7076..c720a82d27 100644 --- a/crates/tui/src/core/engine/tool_catalog/tests.rs +++ b/crates/tui/src/core/engine/tool_catalog/tests.rs @@ -378,6 +378,14 @@ fn search_reports_catalog_stale_evictions_as_unavailable_not_evicted() { None, ) .expect("first search should succeed"); + assert_eq!( + cache.names().count(), + 3, + "premise: the first search must admit all three deferred tools into the cache — the \ + initial_active_tools empty-scan fallback already put deferred-0 into the active set \ + before any search ran, so the active-set assert below is vacuous and only the cache \ + proves admission" + ); assert!(active.contains("deferred-0")); let catalog_v2: Vec = catalog_v1 @@ -441,6 +449,13 @@ fn search_does_not_report_an_eager_promotion_as_unavailable() { None, ) .expect("first search should succeed"); + assert_eq!( + cache.names().count(), + 1, + "premise: the first search must activate the tool into the cache — the active set \ + already contains it via the initial_active_tools fallback, so only the cache proves \ + admission" + ); assert!(active.contains("promoted")); catalog[0].defer_loading = Some(false); @@ -574,6 +589,14 @@ fn toolbox_note_rendering_nets_re_admitted_names_at_ride_time() { 1, "a name already rendered by an earlier note must not be repeated: {rendered}" ); + assert!( + rendered.contains("This turn's start evicted"), + "first-note-wins: the surviving mention keeps the earlier note's attribution: {rendered}" + ); + assert!( + !rendered.contains("This step evicted"), + "first-note-wins: the deduped later note contributes no attribution: {rendered}" + ); assert!(notes.is_empty()); // A name that left the catalog between its eviction and this ride can @@ -622,3 +645,30 @@ fn toolbox_note_rendering_nets_re_admitted_names_at_ride_time() { "a fully unrecoverable note must drop the recovery promise: {rendered}" ); } + +/// Pinvou fork: the toolbox bounds are sentinel-disabled +/// (`TOOL_ACTIVATION_CACHE_MAX_*` are `usize::MAX / 2`), so no activation can +/// ever hit them and the oversize and eviction prose must claim no numeric +/// bound — a bound that never fires is a false statement about the runtime. +#[test] +fn forkguard_toolbox_prose_claims_no_bound_under_the_disabled_sentinel() { + let reason = super::toolbox_oversize_reason(); + assert_eq!( + reason, + "The serialized tool definition could not be measured for admission \ + to the conversation toolbox." + ); + assert!( + !reason.chars().any(|ch| ch.is_ascii_digit()), + "the disabled-sentinel prose must name no numeric bound: {reason}" + ); + + use crate::core::session::ToolboxEvictionTrigger; + let note = + super::bounded_toolbox_eviction_note(ToolboxEvictionTrigger::Step, &["x".to_string()]); + assert_eq!( + note, + "This step evicted tools from the bounded conversation toolbox; the evicted tools are \ + no longer callable until a later search re-activates them: x" + ); +} From fa9cd357b057519211ebedc299cd4254b5d0c924 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:20:11 +0800 Subject: [PATCH 09/18] test(subagent): restore the bounds-independent ride-channel pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101. #104 deleted all ten child-surface eviction tests as bounds-triggered, but two of them parked notes directly and never touched the bounds — while the file's comment claims exactly that class "is kept". The parent engine kept its direct-park pins (hydration_eviction_note_rides_the_batch_first_result and its error-path twin); the child lost its only glue-level coverage of the round-2 defect shape (the child surface discarding every report its helpers produced). Restore child_parked_note_rides_success_and_error_results_and_nets_re_admissions and parked_note_rides_through_the_execute_from_surface_glue. Both pass unchanged against the current tree; the former also covers the round-9 {error:#} error-ride fix, which had shipped without a test. Signed-off-by: asto --- crates/tui/src/tools/subagent/tests.rs | 97 ++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 4d58ebaa3f..a1beea19c2 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -7669,6 +7669,103 @@ async fn small_surface_successful_cached_use_touches_lru() { assert!(model_tool_names(model_request_tools(&mut surface)).contains("get_goal")); } +/// The child's ride channel mirrors the parent's: parked notes ride success +/// and error results alike (a note held back on `Err` would strand until a +/// later `Ok` — or forever if the child ends first), and names the ride +/// re-admits are netted out against the child's live cache. +#[test] +fn child_parked_note_rides_success_and_error_results_and_nets_re_admissions() { + // A parked eviction note names cache members, and cache members are + // catalog members — the premise the ride-time catalog check relies on. + // The eager lead defeats `initial_active_tools`' single-tool fallback, + // which would otherwise force-activate `big-a` and the ride would net + // it out of the note. + let mut lead = synthetic_deferred_tool("lead", 8); + lead.defer_loading = Some(false); + let catalog = vec![ + lead, + synthetic_deferred_tool("big-a", 8), + synthetic_deferred_tool("big-b", 8), + synthetic_deferred_tool("big-c", 8), + ]; + let mut surface = SubAgentToolSurface::new(catalog, &[]); + surface.park_eviction_note( + ToolboxEvictionTrigger::Step, + vec!["big-a".to_string(), "big-b".to_string()].as_slice(), + ); + + let mut ok = Ok(RichToolResult::plain(ToolResult::success("payload"))); + surface.ride_pending_eviction_notes(&mut ok); + let ok = ok.expect("stays ok"); + assert!( + ok.result.content.contains("payload") + && ok.result.content.contains("no longer callable") + && ok.result.content.contains("big-a") + && ok.result.content.contains("big-b"), + "the note rides a success result: {}", + ok.result.content + ); + + surface.park_eviction_note(ToolboxEvictionTrigger::Step, &["big-c".to_string()]); + let mut failure: Result = Err(anyhow::anyhow!("boom")); + surface.ride_pending_eviction_notes(&mut failure); + let message = failure.expect_err("stays an error").to_string(); + assert!( + message.contains("boom") + && message.contains("no longer callable") + && message.contains("big-c"), + "the note must ride an error result too: {message}" + ); + assert!(surface.pending_eviction_notes.is_empty()); + + // A name the ride hands back is netted out; a fully netted note does + // not ride at all. + let deferred = synthetic_deferred_tool("big-a", 8); + surface.cache.activate(&[deferred], &["big-a".to_string()]); + surface.park_eviction_note(ToolboxEvictionTrigger::Step, &["big-a".to_string()]); + let mut netted = Ok(RichToolResult::plain(ToolResult::success("clean"))); + surface.ride_pending_eviction_notes(&mut netted); + let netted = netted.expect("stays ok"); + assert!( + netted.result.content == "clean", + "a note naming only re-admitted tools must be absorbed: {}", + netted.result.content + ); +} + +/// The park side is covered above; this pin is the consumption side — +/// `execute_from_surface` must hand parked notes to the child's recorded +/// result, the production edge that once discarded every report. Driven +/// through the same wrapper a real child tool call takes. +#[tokio::test] +async fn parked_note_rides_through_the_execute_from_surface_glue() { + let registry = small_surface_registry(FleetRole::Builder); + let catalog = registry.deferred_catalog_for_model(&FleetRole::Builder); + let mut surface = SubAgentToolSurface::new(catalog, &["get_goal".to_string()]); + // A parked eviction note names a cache member, and a cache member is a + // catalog member — the premise the ride-time catalog check relies on. + surface + .catalog + .push(synthetic_deferred_tool("ghost-tool", 8)); + surface.park_eviction_note( + ToolboxEvictionTrigger::TurnStart, + &["ghost-tool".to_string()], + ); + + let content = execute_surface_tool(®istry, &mut surface, "get_goal", json!({})) + .await + .expect("cached read tool executes"); + assert!( + content.contains("no longer callable") && content.contains("ghost-tool"), + "the production ride glue must attach the parked note to the recorded result: {content}" + ); + assert!( + surface.pending_eviction_notes.is_empty(), + "the ride consumes the parked notes: {:?}", + surface.pending_eviction_notes + ); +} + #[test] fn small_surface_depth_cap_removes_only_agent() { let mut runtime = From b84cec6bea2d470d72555739a63e75aea8b55cff Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:20:11 +0800 Subject: [PATCH 10/18] test(engine): pin the hydration audit marker on the arm that fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101. refusal_audit_payload_carries_the_executed_marker sized its fixture (heavy_deferred_tool("notify", 20_000)) against the 16KiB per-tool budget #104 removed, so it silently took the DeferredHydration::Loaded arm while its name and docstring said Refused — passing either way, because both arms carry the executed:false marker. Renamed to hydration_receipt_audit_payload_carries_the_executed_marker and re-documented for the Loaded arm it actually drives; the docstring now states the oversize Refused arm is unreachable in this fork because the per-tool budget is the disabled usize::MAX / 2 sentinel. Signed-off-by: asto --- crates/tui/src/core/engine/tests.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 6263dadb9c..b512d774d5 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -28551,13 +28551,14 @@ async fn turn_start_note_does_not_name_a_tool_the_result_re_admits() { "the receipt must not declare a tool uncallable that the same result re-admits: {receipt}" ); } -/// The refusal's `executed: false` marker must reach the `tool.result` audit -/// payload, not just the receipt metadata: the audit is the record that a -/// refused, never-executed call is not a successful execution. Driven through -/// the real refusal receipt this commit's marker reads, the audit line must -/// carry the marker verbatim. +/// The hydration receipt — the `DeferredHydration::Loaded` arm; the oversize +/// `Refused` arm is unreachable in this fork because the per-tool budget is +/// the disabled `usize::MAX / 2` sentinel — answers the call without +/// executing the tool. Its `executed: false` marker must reach the +/// `tool.result` audit payload verbatim, not just the receipt metadata, so +/// the audit cannot record a never-executed call as a successful execution. #[tokio::test] -async fn refusal_audit_payload_carries_the_executed_marker() { +async fn hydration_receipt_audit_payload_carries_the_executed_marker() { use crate::llm_client::mock::{MockLlmClient, canned}; let _guard = lock_test_env(); @@ -28582,7 +28583,7 @@ async fn refusal_audit_payload_carries_the_executed_marker() { let surface = test_tool_surface(&engine, registry, Some(catalog.clone()), AppMode::Agent); let tool_registry = Some(&surface.registry); let mut tool_uses = vec![super::streaming::ToolUseState { - id: "refuse-audit".to_string(), + id: "hydrate-audit".to_string(), name: "notify".to_string(), input: json!({}), caller: None, @@ -28610,19 +28611,19 @@ async fn refusal_audit_payload_carries_the_executed_marker() { let plan = planned .plans .iter() - .find(|plan| plan.id == "refuse-audit") - .expect("the refused call must be planned"); + .find(|plan| plan.id == "hydrate-audit") + .expect("the hydrated call must be planned"); let receipt = plan .guard_result .as_ref() - .expect("the refusal must answer the call as a guard result") + .expect("the hydration receipt must answer the call as a guard result") .clone(); // The results phase consumes the receipt exactly as production does. engine .process_tool_results( vec![Some(synthetic_toolbox_outcome( - "refuse-audit", + "hydrate-audit", "notify", Ok(receipt), ))], @@ -28638,14 +28639,14 @@ async fn refusal_audit_payload_carries_the_executed_marker() { .lines() .find_map(|line| { let value: serde_json::Value = serde_json::from_str(line).ok()?; - (value.get("event")? == "tool.result" && value.get("tool_id")? == "refuse-audit") + (value.get("event")? == "tool.result" && value.get("tool_id")? == "hydrate-audit") .then_some(value) }) - .expect("the refusal's tool.result audit line must exist"); + .expect("the hydration receipt's tool.result audit line must exist"); assert_eq!( line.get("executed").and_then(serde_json::Value::as_bool), Some(false), - "the audit must record the refused call as never executed: {line}" + "the audit must record the hydration-answered call as never executed: {line}" ); assert_eq!( line.get("success").and_then(serde_json::Value::as_bool), From 07341563927ad3189b375b6d6c44b41564facd2a Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:20:28 +0800 Subject: [PATCH 11/18] fix(cli): read the opt-in tool audit log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101, MAJOR. The metrics rollup read only ~/.deepseek/audit.log, but the only in-repo tool.result writer is the engine's opt-in tool-audit log at $CODEWHALE_TOOL_AUDIT_LOG / $DEEPSEEK_TOOL_AUDIT_LOG, which has no default path — so #101's executed-marker consult and the round-9 flat-placement fallback were inert unless a user happened to point the variable at the very file metrics reads. codewhale metrics now also reads the opt-in path when set (deduplicated against the default so a co-located file is not counted twice; the writer's variable precedence is mirrored). The reader comment claiming "the sensitive audit log nests them under details" was false — audit.rs has no tool.result call sites; the nested placements are tolerated for foreign or hand-written lines and the comment now says so. The restricted-turn audit projection no longer strips success and executed: both are non-private booleans, and the reader defaults a missing flag to true, so stripping them bucketed every restricted receipt — failures included — as an executed success. The forkguard redaction test now pins that the flags survive while inputs, outputs, and paths stay omitted. Signed-off-by: asto --- crates/cli/src/metrics.rs | 78 ++++++++++++++++++-- crates/tui/src/core/engine/tool_execution.rs | 28 +++++-- 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/crates/cli/src/metrics.rs b/crates/cli/src/metrics.rs index 42a923b4f0..1ac67ca724 100644 --- a/crates/cli/src/metrics.rs +++ b/crates/cli/src/metrics.rs @@ -3,6 +3,7 @@ //! //! Data sources: //! - `~/.deepseek/audit.log` — one JSON line per event (approvals, credentials) +//! - `$CODEWHALE_TOOL_AUDIT_LOG` (opt-in) — engine tool receipts (`tool.result`) //! - `~/.deepseek/sessions/` — saved session JSON files (tool call history) //! - `~/.deepseek/tasks/runtime/events/` — runtime thread JSONL event streams @@ -31,7 +32,15 @@ pub fn run(args: MetricsArgs) -> Result<()> { // Collect data from every source; treat missing files as empty. let mut rollup = Rollup::default(); - read_audit_log(&base.join("audit.log"), args.since, &mut rollup); + // The engine's only in-repo `tool.result` writer is the opt-in + // tool-audit log; it has no default path, so it is read only when the + // variable is set. Precedence mirrors the writer's: CODEWHALE first, + // then the legacy DEEPSEEK spelling. + let tool_audit_log = std::env::var_os("CODEWHALE_TOOL_AUDIT_LOG") + .or_else(|| std::env::var_os("DEEPSEEK_TOOL_AUDIT_LOG")); + for path in audit_log_paths(&base, tool_audit_log.as_deref()) { + read_audit_log(&path, args.since, &mut rollup); + } read_session_files(&base.join("sessions"), args.since, &mut rollup); read_runtime_events( &base.join("tasks").join("runtime").join("events"), @@ -240,6 +249,23 @@ impl Rollup { // Source readers // ────────────────────────────────────────────────────────────────────────────── +/// Audit-log files the rollup reads: the default `base/audit.log` first, +/// then the engine's opt-in tool-audit log when one is configured. That +/// writer (`CODEWHALE_TOOL_AUDIT_LOG`, falling back to +/// `DEEPSEEK_TOOL_AUDIT_LOG`) is the only in-repo source of `tool.result` +/// receipts. An opt-in path equal to the default collapses to one entry — +/// reading the same file twice would double-count every line. +fn audit_log_paths(base: &Path, opt_in: Option<&std::ffi::OsStr>) -> Vec { + let default = base.join("audit.log"); + let mut paths = vec![default.clone()]; + if let Some(opt_in) = opt_in.map(PathBuf::from) + && opt_in != default + { + paths.push(opt_in); + } + paths +} + /// Read one-JSON-line-per-event audit log. fn read_audit_log(path: &Path, since: Option>, rollup: &mut Rollup) { let content = match std::fs::read_to_string(path) { @@ -331,11 +357,12 @@ fn read_audit_log(path: &Path, since: Option>, rollup: &mut Rollup // oversize-hydration refusal) answered the call without the // tool ever running, so it counts as neither — counting it // as a success would inflate the tool's real execution rate. - // The fields are read from every placement a writer uses: - // the sensitive audit log nests them under `details`, while - // the engine's opt-in tool-audit log - // (`CODEWHALE_TOOL_AUDIT_LOG`) writes them at the top level - // of the line. + // The engine's opt-in tool-audit log + // (`CODEWHALE_TOOL_AUDIT_LOG`) — the only in-repo + // `tool.result` writer — writes these fields at the top + // level of the line. The nested `details`/`payload` + // placements are tolerated for foreign or hand-written + // lines; no in-repo writer nests them. let success = v .pointer("/details/success") .or_else(|| v.pointer("/payload/success")) @@ -946,6 +973,45 @@ mod tests { assert_eq!(fmt_num(1_000_000), "1,000,000"); } + // ── audit_log_paths ── + + #[test] + fn audit_log_paths_unset_reads_default_only() { + let paths = audit_log_paths(Path::new("/home/user/.codewhale"), None); + assert_eq!( + paths, + vec![PathBuf::from("/home/user/.codewhale/audit.log")] + ); + } + + #[test] + fn audit_log_paths_opt_in_appends_after_default() { + let paths = audit_log_paths( + Path::new("/home/user/.codewhale"), + Some(std::ffi::OsStr::new("/tmp/tool-audit.log")), + ); + assert_eq!( + paths, + vec![ + PathBuf::from("/home/user/.codewhale/audit.log"), + PathBuf::from("/tmp/tool-audit.log"), + ] + ); + } + + #[test] + fn audit_log_paths_opt_in_equal_to_default_is_read_once() { + // Reading the same file twice would double-count every line. + let paths = audit_log_paths( + Path::new("/home/user/.codewhale"), + Some(std::ffi::OsStr::new("/home/user/.codewhale/audit.log")), + ); + assert_eq!( + paths, + vec![PathBuf::from("/home/user/.codewhale/audit.log")] + ); + } + // ── Rollup from audit log ── fn make_audit_line(event: &str, tool: &str, ts: &str) -> String { diff --git a/crates/tui/src/core/engine/tool_execution.rs b/crates/tui/src/core/engine/tool_execution.rs index 19709a145a..6517b3f2cf 100644 --- a/crates/tui/src/core/engine/tool_execution.rs +++ b/crates/tui/src/core/engine/tool_execution.rs @@ -199,9 +199,13 @@ pub(crate) fn emit_tool_audit(event: serde_json::Value) { emit_tool_audit_to_path(&PathBuf::from(path), event); } -/// Preserve only non-private identity fields for restricted embedded turns. -/// Tool inputs, outputs, paths, ids, policy details, and future payload keys -/// are omitted by construction instead of relying on every caller to redact. +/// Preserve only non-private fields for restricted embedded turns. Tool +/// inputs, outputs, paths, ids, policy details, and future payload keys are +/// omitted by construction instead of relying on every caller to redact. +/// `success` and `executed` are non-private booleans and survive: the +/// `codewhale metrics` rollup defaults a missing flag to `true`, so +/// stripping them would bucket every restricted receipt — failures +/// included — as an executed success. pub(crate) fn tool_audit_event_for_policy( restricted: bool, event: serde_json::Value, @@ -216,6 +220,11 @@ pub(crate) fn tool_audit_event_for_policy( if let Some(tool_name) = event.get("tool_name").and_then(serde_json::Value::as_str) { projected["tool_name"] = serde_json::json!(tool_name); } + for flag in ["success", "executed"] { + if let Some(value) = event.get(flag).and_then(serde_json::Value::as_bool) { + projected[flag] = serde_json::json!(value); + } + } projected } @@ -1039,18 +1048,27 @@ mod tests { let event = tool_audit_event_for_policy( true, json!({ - "event": "tool.error", + "event": "tool.result", "tool_name": "File", "tool_input": private, "error": format!("failed at C:/private/{private}"), + "path": format!("/private/{private}"), + "success": false, + "executed": false, }), ); let encoded = serde_json::to_string(&event).expect("audit json"); assert!(!encoded.contains(private)); - assert_eq!(event["event"], "tool.error"); + assert_eq!(event["event"], "tool.result"); assert_eq!(event["tool_name"], "File"); assert_eq!(event["restricted"], true); + // Non-private booleans survive: the metrics rollup defaults a + // missing flag to `true`, so dropping them would misbucket this + // restricted failure as an executed success. + assert_eq!(event["success"], false); + assert_eq!(event["executed"], false); assert!(event.get("tool_input").is_none()); assert!(event.get("error").is_none()); + assert!(event.get("path").is_none()); } } From 228b9b2f181161f5d137119e8e839276ac0add7c Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:20:28 +0800 Subject: [PATCH 12/18] test(kimi): pin the root-additionalProperties and ref-sibling repairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101. Two empty-items repair positions were exercised only by construction: an empty items under a root-level object-form additionalProperties (existing coverage nested it inside a property), and an empty items beside a $ref (pre-#101 the sibling failed closed with InvalidReference; the repair drops the keyword and the clean $ref node ships). Fixture both so a future walker change cannot silently alter them. The walker doc also named the schema-value positions it visits as "exactly" the set — true only for the MFJS-legal subset. It now notes that schema values under keywords MFJS rejects (contains, prefixItems, patternProperties, not, if/then/else, tuple-form items) need no visit: the keyword itself fails closed first. Signed-off-by: asto --- crates/tui/src/tools/schema_sanitize.rs | 36 +++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index c24ede7f1d..ee83e847cb 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -1304,6 +1304,11 @@ fn sanitize_kimi_parameters_candidate( /// and only its values are visited. `oneOf` needs no visit: this runs after /// `normalize_kimi_compatibility` has converted every non-root `oneOf` to /// `anyOf`, and root composition was already flattened before that. +/// Schema values under keywords MFJS rejects (`contains`, `prefixItems`, +/// `patternProperties`, `not`, `if`/`then`/`else`, tuple-form `items`) need +/// no visit: the keyword itself fails closed — `UnsupportedKeyword`, or +/// `InvalidSchemaNode` for tuple-form `items` — before a stranded empty +/// `items` node inside one could matter. /// /// Children are visited before the node's own `items` check so a nested /// repair can empty the parent (`"items": {"items": {}}` is vacuously true @@ -2627,6 +2632,37 @@ mod kimi_tests { nested_positions["properties"]["choice"]["anyOf"][1], json!({"type": "array"}) ); + + // The root's own object-form `additionalProperties` is a schema-value + // position too: an empty `items` there repairs to the keyword being + // absent and the tool ships. + let mut root_additional = json!({ + "type": "object", + "properties": {"name": {"type": "string"}}, + "additionalProperties": {"type": "array", "items": {}} + }); + sanitize_for_kimi_parameters(&mut root_additional) + .expect("root additionalProperties empty items repair"); + assert_eq!( + root_additional["additionalProperties"], + json!({"type": "array"}) + ); + + // An empty `items` beside a `$ref` drops the same way, leaving the + // clean `$ref` node — MFJS admits no sibling beyond + // title/description next to a reference, so without the repair the + // whole tool would fail closed with InvalidReference. + let mut ref_with_items = json!({ + "type": "object", + "properties": {"a": {"$ref": "#/$defs/a", "items": {}}}, + "$defs": {"a": {"type": "string"}} + }); + sanitize_for_kimi_parameters(&mut ref_with_items) + .expect("$ref with an empty items sibling repairs"); + assert_eq!( + ref_with_items["properties"]["a"], + json!({"$ref": "#/$defs/a"}) + ); } #[test] From 620553c335997e89a47a17b944a1124e894e81a9 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 13:20:39 +0800 Subject: [PATCH 13/18] docs(tool-search): round-10 truthfulness pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-10 fresh review of #101, prose only: - replace_runtime_mcp_tools' docstring tail claimed a same-count refresh never moves wire bytes; the round-9 content compare makes a same-name content change declare, and the tail now says so. - The arrivals comment justified eager arrival only by the 401-auth case; it now states the bulk post-login arrival truth (any newly served name arrives eager by the same rule, deliberately). - park_toolbox_eviction_note's doc said "combining" where the implementation queues; repeated names are netted at render. - The SyncSession note clear now states it is redundant with the turn-entry sweep and kept so no future non-turn reader inherits cross-conversation state. - TOOL_SURFACE.md: the cache paragraph measures serialized tool definitions, not just schemas; the bounds-eviction reporting paragraph is marked inert in this fork (upstream-parity machinery); and the catalog-shrink absorption claim is corrected — a search's revalidation reports a cached-but-departed name under unavailable_tool_references regardless of the query, and only a shrink first observed at a turn boundary is absorbed silently. Signed-off-by: asto --- crates/tui/src/core/engine.rs | 4 +++- crates/tui/src/core/engine/turn_loop.rs | 29 ++++++++++++++++--------- docs/TOOL_SURFACE.md | 27 ++++++++++++++--------- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 68d4e3bf79..d7cb6cfcd4 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -3462,7 +3462,9 @@ impl Engine { // generated-ID new-session path), so never carry the // previous conversation's toolbox across this edge — // including a parked eviction note whose batch never - // recorded a result. + // recorded a result. The note clear is redundant with + // the turn-entry sweep and is kept so no future + // non-turn reader inherits cross-conversation state. self.session.tool_activation_cache.clear(); self.session.pending_toolbox_eviction_notes.clear(); let plugin_workspace_changed = diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 64c7e5ec93..7d95667e4c 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -1060,15 +1060,22 @@ fn registered_tool_requires_non_bypassable_approval(tool_name: &str) -> bool { /// marking across the refresh: adopting the pool's flag would flip deferred /// tools back onto the head of the wire list and bust the cached prefix. /// A name the conversation has never seen (the synthetic login tool a -/// mid-turn 401 just started offering, or a server-side rename) arrives -/// with the pool's eager flag and joins the active set: the 401 result's -/// auth hint points straight at that tool, so deferring it would strand -/// the recovery path behind a search. Replaced entries are spliced back -/// into their own catalog slot, so a refresh that serves the same names -/// reorders nothing and the caller's declaration (this return value: did -/// the model-visible wire list change?) stays false — a same-count -/// refresh moves no wire bytes and must not trip the C5 undeclared-drift -/// guard as a false positive. +/// mid-turn 401 just started offering, a server-side rename, or a server's +/// full real tool set after a completed login) arrives with the pool's +/// eager flag and joins the active set. The auth-hint case needs it: the +/// 401 result's auth hint points straight at that tool, so deferring it +/// would strand the recovery path behind a search. Bulk arrivals after a +/// login follow the same rule deliberately: the graceful direct-call +/// hydration path bounds what deferring would cost, but a fresh login's +/// tools are advertised at once. Replaced entries are spliced back into +/// their own catalog slot, so a refresh that serves the same names reorders +/// nothing. The caller's declaration (this return value: did the +/// model-visible wire list change?) then rests on content, not on the name +/// count: a same-name refresh with byte-identical served definitions moves +/// no wire bytes and stays false; one whose served definition changed under +/// the same name splices new wire bytes and declares — the wire-JSON +/// compare below exists so that drift is declared rather than tripping the +/// C5 undeclared-drift guard. pub(super) fn replace_runtime_mcp_tools( tool_catalog: &mut Vec, active_tool_names: &mut std::collections::HashSet, @@ -5172,8 +5179,10 @@ impl Engine { } /// Park a bounded-toolbox eviction note for the next recorded tool - /// result, combining with any note already parked — turn-start + /// result, queueing beside any note already parked — turn-start /// revalidation and a hydration batch can both evict in the same turn. + /// Names repeated across notes are netted when the notes render onto a + /// result. pub(super) fn park_toolbox_eviction_note( &mut self, trigger: crate::core::session::ToolboxEvictionTrigger, diff --git a/docs/TOOL_SURFACE.md b/docs/TOOL_SURFACE.md index 0d571b638c..0eac2ea90f 100644 --- a/docs/TOOL_SURFACE.md +++ b/docs/TOOL_SURFACE.md @@ -75,9 +75,9 @@ user explicitly names them in `[tools].always_load`. ### Conversation toolbox cache 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 (effectively unbounded), so every activation stays +conversation. Upstream bounds the cache to eight deferred names and 16 KiB +of serialized tool definitions with least-recently-used eviction; this fork +disables that 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). @@ -98,7 +98,9 @@ error), a retrieval activation or a refresh of an executed tool rides the triggering result, and a turn's start revalidation parks its note on the session so the turn's first recorded result carries it (a refresh only evicts when a cached tool's serialized definition grew since its last -activation enough that the collective budget overflows). A note that never reaches a recorded result is dropped rather +activation enough that the collective budget overflows). With the size bounds +disabled in this fork no bounds eviction ever fires, so this channel stays +silent — it is upstream-parity machinery. A note that never reaches a recorded result is dropped rather than riding an unrelated later result: a session sync clears it at the conversation edge, and a turn that ends without a recorded result has it swept at the next turn's entry. Child agents report the same way on their @@ -125,12 +127,17 @@ the model cannot get back by searching are reported differently: whether the tool left the catalog (a disconnect, a changed allow/deny rule, a mode switch — any catalog-shrink cause) or outgrew the per-tool serialized-size budget. A cached tool that became -eager is not reported at all — it stays directly callable. Unavailable drops -observed by non-search paths are absorbed silently: an oversize drop is -re-reported with its reason the next time a search returns the tool (a -match ranked past the result cap re-reports nothing), while a -catalog-shrink drop left the catalog entirely and no later search can name -it — the model learns of those only when a search for the capability misses. +eager is not reported at all — it stays directly callable. A catalog-shrink +drop is reported when a search observes it first: every search revalidates +the cache before admitting its matches, so a cached-but-departed name +surfaces in that result's unavailable references with the catalog-shrink +reason, whether or not the query would have matched it. Only a shrink +first observed at a turn boundary is absorbed silently — the name leaves +the cache there, so no later search can name it and the model learns of it +only when a search for the capability misses. An oversize drop observed by +a non-search path is likewise silent in the moment but re-reported with +its reason the next time a search returns the tool (a match ranked past +the result cap re-reports nothing). Each subagent gets its own policy-filtered deferred catalog, always-present `tool_search`, and activation cache (unbounded in this fork, like the parent From 664a7154d66dc18781d1be98a89809b9cdf63a18 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 16:43:18 +0800 Subject: [PATCH 14/18] fix(engine): scope content drift to the wire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content compare declared drift for any served universe tool, including a deferred tool the conversation never activated — a name that never ships, so its description change moves no wire bytes and the declaration was phantom (absorbed harmlessly by the fingerprint match, but a false "tool_surface" attribution all the same). Filter the content compare to the wire-visible set: when the visible sets tie, every visible-after name was visible before, so the compare stays exact. Also state the strict-mode caveat at the compare. Pinned both ways: wire-invisible drift now stays silent (red against the unfiltered compare) while visible drift still declares. Signed-off-by: asto --- crates/tui/src/core/engine/tests.rs | 34 +++++++++++++++++++++++++ crates/tui/src/core/engine/turn_loop.rs | 27 ++++++++++++++------ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index b512d774d5..e8bf6e8584 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -9525,6 +9525,40 @@ fn runtime_mcp_refresh_replaces_the_pool_slice() { vec!["mcp_static_read", "exec_shell"], "the content-drift declaration still splices in place" ); + + // The same content change on a tool the conversation never ships must + // NOT declare: a deferred tool that was never activated is invisible on + // the wire, so its description drift moves no wire bytes. The deferral + // flag survives the splice (the refresh restores the prior flag), and + // the empty activation cache keeps it out of the active set. + let mut deferred_stale = api_tool("mcp_static_read"); + deferred_stale.description = "another stale description".to_string(); + deferred_stale.defer_loading = Some(true); + let mut catalog = vec![deferred_stale, api_tool("exec_shell")]; + let mut active = HashSet::new(); + let wire_changed = replace_runtime_mcp_tools( + &mut catalog, + &mut active, + &universe, + vec![api_tool("mcp_static_read")], + &cache, + ); + assert!( + !wire_changed, + "content drift on a wire-invisible tool moves no wire bytes and must stay silent" + ); + assert!( + !active.contains("mcp_static_read"), + "the deferred entry must stay off the wire: {active:?}" + ); + assert_eq!( + catalog + .iter() + .find(|tool| tool.name == "mcp_static_read") + .and_then(|tool| tool.defer_loading), + Some(true), + "the splice restores the pre-existing deferral flag" + ); } #[test] diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 7d95667e4c..999896168c 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -1093,8 +1093,10 @@ pub(super) fn replace_runtime_mcp_tools( // answers for. Membership alone is still not the whole story: a // same-name refresh can change what a served tool *says* (description, // schema), which the C5 content fingerprint sees as wire bytes without - // any name moving. When the name set ties, the replaced entries' wire - // JSON — the exact bytes the fingerprint hashes — decides. + // any name moving. When the visible name set ties, the wire-visible + // replaced entries' JSON — the exact bytes the fingerprint hashes — + // decides; a tool the conversation never ships is skipped so its drift + // cannot declare what no request will see. let wire_before = wire_visible_tool_names(tool_catalog, active_tool_names); let previous_wire_json: std::collections::HashMap> = tool_catalog .iter() @@ -1136,16 +1138,25 @@ pub(super) fn replace_runtime_mcp_tools( } tool_catalog.retain(|tool| !departed.contains(&tool.name)); tool_catalog.extend(arrivals); - if wire_visible_tool_names(tool_catalog, active_tool_names) != wire_before { + let wire_after = wire_visible_tool_names(tool_catalog, active_tool_names); + if wire_after != wire_before { return true; } - // Same membership: declare only when a served tool's wire JSON changed - // under its name. A name without a predecessor is a new arrival — - // eager arrivals already moved the name set above, and a deferred one - // is invisible on the wire. + // Same visible membership: declare only when a wire-visible tool's + // served JSON changed under its name. Because the visible sets tie, + // every visible-after name was visible before and so has a snapshot + // entry; a name outside the visible set never ships, so its content + // drift moves no wire bytes and must not declare. A name without a + // predecessor is a new arrival — an eager arrival already moved the + // name set above, and a deferred one is invisible on the wire. + // `tool_to_api_json` is the exact serializer the fingerprint hashes + // for the non-strict request shape; under strict mode the fingerprint + // hashes sanitized copies, and raw-identical definitions stay + // sanitized-identical, so this compare can only over-declare there, + // never miss. tool_catalog .iter() - .filter(|tool| universe.contains(&tool.name)) + .filter(|tool| universe.contains(&tool.name) && wire_after.contains(&tool.name)) .any(|tool| { previous_wire_json .get(&tool.name) From b1373d8ba44488448ad518c84cdf32004bb68a38 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 16:43:18 +0800 Subject: [PATCH 15/18] test(subagent): pin the error-chain ride rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The {error:#} rebuild shipped in this PR without a test that could fail: the restored ride test used a chainless anyhow error, whose bare and alternate displays render identically. Chain the fixture (context over "boom") and assert both levels appear in the ridden message — reverting to a bare display drops the "boom" half and turns the pin red. Signed-off-by: asto --- crates/tui/src/tools/subagent/tests.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index a1beea19c2..b9821fc494 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -7707,14 +7707,19 @@ fn child_parked_note_rides_success_and_error_results_and_nets_re_admissions() { ); surface.park_eviction_note(ToolboxEvictionTrigger::Step, &["big-c".to_string()]); - let mut failure: Result = Err(anyhow::anyhow!("boom")); + // The error rebuild renders the full source chain (`{error:#}`): the + // context is the top level and "boom" its source, so a bare-display + // rebuild would keep "outer" and silently drop "boom". + use anyhow::Context as _; + let mut failure: Result = Err(anyhow::anyhow!("boom")).context("outer"); surface.ride_pending_eviction_notes(&mut failure); let message = failure.expect_err("stays an error").to_string(); assert!( - message.contains("boom") + message.contains("outer") + && message.contains("boom") && message.contains("no longer callable") && message.contains("big-c"), - "the note must ride an error result too: {message}" + "the note must ride an error result with its full source chain: {message}" ); assert!(surface.pending_eviction_notes.is_empty()); From b66de8bc3d519897b6d5738e758270aad79b0ceb Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 16:43:19 +0800 Subject: [PATCH 16/18] fix(tools): drop the sentinel note's bound claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sentinel branch still called the toolbox "bounded" in the very state where the fork disabled the bound — the exact never-firing claim the forkguard test's own doc condemns. Drop the word from the sentinel branch (real bounds keep it), update the pinned literal, add an explicit no-"bounded" assert, and update the netting count assert in the rendering test that matches the phrase. Signed-off-by: asto --- crates/tui/src/core/engine/tool_catalog.rs | 2 +- crates/tui/src/core/engine/tool_catalog/tests.rs | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/core/engine/tool_catalog.rs b/crates/tui/src/core/engine/tool_catalog.rs index 006c0ce048..0de5af83f5 100644 --- a/crates/tui/src/core/engine/tool_catalog.rs +++ b/crates/tui/src/core/engine/tool_catalog.rs @@ -490,7 +490,7 @@ pub(crate) fn bounded_toolbox_eviction_note( && crate::core::session::TOOL_ACTIVATION_CACHE_MAX_SCHEMA_BYTES == usize::MAX / 2 { return format!( - "This {} evicted tools from the bounded conversation toolbox; the evicted tools \ + "This {} evicted tools from the conversation toolbox; the evicted tools \ are no longer callable until a later search re-activates them: {}", trigger.label(), evicted.join(", ") diff --git a/crates/tui/src/core/engine/tool_catalog/tests.rs b/crates/tui/src/core/engine/tool_catalog/tests.rs index c720a82d27..e919ef0ea1 100644 --- a/crates/tui/src/core/engine/tool_catalog/tests.rs +++ b/crates/tui/src/core/engine/tool_catalog/tests.rs @@ -529,7 +529,7 @@ fn toolbox_note_rendering_nets_re_admitted_names_at_ride_time() { ); assert_eq!( rendered - .matches("evicted tools from the bounded conversation toolbox") + .matches("evicted tools from the conversation toolbox") .count(), 1, "a companion note the netting empties must contribute no prose: {rendered}" @@ -648,8 +648,9 @@ fn toolbox_note_rendering_nets_re_admitted_names_at_ride_time() { /// Pinvou fork: the toolbox bounds are sentinel-disabled /// (`TOOL_ACTIVATION_CACHE_MAX_*` are `usize::MAX / 2`), so no activation can -/// ever hit them and the oversize and eviction prose must claim no numeric -/// bound — a bound that never fires is a false statement about the runtime. +/// ever hit them and the oversize and eviction prose must claim no bound, +/// numeric or otherwise — a bound that never fires is a false statement +/// about the runtime. #[test] fn forkguard_toolbox_prose_claims_no_bound_under_the_disabled_sentinel() { let reason = super::toolbox_oversize_reason(); @@ -668,7 +669,11 @@ fn forkguard_toolbox_prose_claims_no_bound_under_the_disabled_sentinel() { super::bounded_toolbox_eviction_note(ToolboxEvictionTrigger::Step, &["x".to_string()]); assert_eq!( note, - "This step evicted tools from the bounded conversation toolbox; the evicted tools are \ + "This step evicted tools from the conversation toolbox; the evicted tools are \ no longer callable until a later search re-activates them: x" ); + assert!( + !note.contains("bounded"), + "the disabled-sentinel note must not claim a bound the fork disabled: {note}" + ); } From f0f5d1cf2432e3afd6b947b112446ba4bba6ee3d Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 16:43:19 +0800 Subject: [PATCH 17/18] fix(cli): audit-source docs and undated pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flat tool.result fixtures carried a `ts` the engine writer never emits, pinning a fictional shape. Drop it, and pin the real consequence instead: an undatable line is never excluded by --since (the since-filter test now counts one). The module header also claimed the default audit.log holds approvals/credentials without qualifying that the writer resolves to CODEWHALE_HOME/~/.codewhale while the reader base falls back to ~/.deepseek — state the home resolution and the divergence truthfully. Signed-off-by: asto --- crates/cli/src/metrics.rs | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/crates/cli/src/metrics.rs b/crates/cli/src/metrics.rs index 1ac67ca724..694dba560c 100644 --- a/crates/cli/src/metrics.rs +++ b/crates/cli/src/metrics.rs @@ -1,11 +1,17 @@ //! `codewhale metrics` — reads the audit log and session/task stores and prints //! a human-readable usage rollup. //! -//! Data sources: -//! - `~/.deepseek/audit.log` — one JSON line per event (approvals, credentials) -//! - `$CODEWHALE_TOOL_AUDIT_LOG` (opt-in) — engine tool receipts (`tool.result`) -//! - `~/.deepseek/sessions/` — saved session JSON files (tool call history) -//! - `~/.deepseek/tasks/runtime/events/` — runtime thread JSONL event streams +//! Data sources (bases resolve `CODEWHALE_HOME` → `DEEPSEEK_HOME` → +//! `~/.deepseek`): +//! - `audit.log` under the base — one JSON line per event (approvals, +//! credentials). The sensitive-audit writer resolves to +//! `CODEWHALE_HOME` → `~/.codewhale/audit.log` and never consults +//! `DEEPSEEK_HOME`, so without `CODEWHALE_HOME` set its events land in a +//! file this rollup does not read. +//! - `$CODEWHALE_TOOL_AUDIT_LOG` (opt-in) — engine tool receipts +//! (`tool.result`), written top-level with no `ts` stamp +//! - `sessions/` under the base — saved session JSON files (tool call history) +//! - `tasks/runtime/events/` under the base — runtime thread JSONL event streams use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -1090,15 +1096,25 @@ mod tests { ); writeln!(tmp, "{line_old}").unwrap(); writeln!(tmp, "{line_new}").unwrap(); + // The engine's opt-in tool-audit log writes `tool.result` lines with + // no `ts`; a line that cannot be dated is never excluded by + // `--since` — it counts whatever the cutoff. + writeln!( + tmp, + r#"{{"event":"tool.result","tool_name":"flat_undated","success":true}}"# + ) + .unwrap(); let cutoff: DateTime = "2026-01-01T00:00:00Z".parse().unwrap(); let mut rollup = Rollup::default(); read_audit_log(tmp.path(), Some(cutoff), &mut rollup); - // Only the newer line should be counted. - assert_eq!(rollup.parsed_lines, 1); + // Only the newer dated line and the undatable flat line are counted. + assert_eq!(rollup.parsed_lines, 2); assert!(!rollup.tools.contains_key("exec_shell")); assert_eq!(rollup.tools["read_file"].calls, 1); + assert_eq!(rollup.tools["flat_undated"].calls, 1); + assert_eq!(rollup.tools["flat_undated"].successes, 1); } #[test] @@ -1115,12 +1131,13 @@ mod tests { r#"{"event":"tool.result","ts":"2026-04-01T10:03:00+00:00","details":{"tool_name":"other_tool","success":true,"executed":true}}"#, // The engine's opt-in tool-audit log // (`CODEWHALE_TOOL_AUDIT_LOG`) writes the same fields at the top - // level of the line instead of under `details`. - r#"{"event":"tool.result","ts":"2026-04-01T10:04:00+00:00","tool_name":"flat_tool","success":true}"#, - r#"{"event":"tool.result","ts":"2026-04-01T10:05:00+00:00","tool_name":"flat_refusal","success":true,"executed":false}"#, + // level of the line instead of under `details`, and stamps no + // `ts` — the fixtures mirror that writer shape. + r#"{"event":"tool.result","tool_name":"flat_tool","success":true}"#, + r#"{"event":"tool.result","tool_name":"flat_refusal","success":true,"executed":false}"#, // An unexecuted receipt is neither success nor failure whatever // its success flag says — the tool never ran. - r#"{"event":"tool.result","ts":"2026-04-01T10:06:00+00:00","tool_name":"flat_failed_refusal","success":false,"executed":false}"#, + r#"{"event":"tool.result","tool_name":"flat_failed_refusal","success":false,"executed":false}"#, ]; for line in lines { writeln!(tmp, "{line}").unwrap(); From bfb22657e77d1b9a79a14aaa0f2413c153e77475 Mon Sep 17 00:00:00 2001 From: asto Date: Wed, 14 Oct 2026 16:43:19 +0800 Subject: [PATCH 18/18] docs(tool-search): round-11 truthfulness fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-11 review found the absorption rewrite had narrowed truth: a shrink is silent under EVERY non-search first observer (turn boundary, hydration batch, executed-tool refresh, retrieval admission, child request rebuild), not only at turn boundaries — the hydration path's own comment says so. Say that. Also: the leftover "cached schemas" wording becomes "cached tool definitions"; the fail-closed keyword list gains propertyNames, unevaluated* and nested allOf, and notes dependentSchemas is stripped rather than rejected; activate's doc now names the serde sentinel as the sole remaining admission rejection. Signed-off-by: asto --- crates/tui/src/core/session.rs | 5 ++++- crates/tui/src/tools/schema_sanitize.rs | 10 +++++++--- docs/TOOL_SURFACE.md | 26 +++++++++++++------------ 3 files changed, 25 insertions(+), 16 deletions(-) diff --git a/crates/tui/src/core/session.rs b/crates/tui/src/core/session.rs index 8ce317d5fb..5079b3c9c8 100644 --- a/crates/tui/src/core/session.rs +++ b/crates/tui/src/core/session.rs @@ -285,7 +285,10 @@ impl ToolActivationCache { /// 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. + /// deferred catalog member is admitted. The one admission rejection left + /// is the serde-failure measurement sentinel (see + /// `toolbox_oversize_reason`'s doc) — unreachable for the concrete + /// `Tool` type, whose derived serialization cannot fail. pub(crate) fn activate( &mut self, catalog: &[crate::models::Tool], diff --git a/crates/tui/src/tools/schema_sanitize.rs b/crates/tui/src/tools/schema_sanitize.rs index ee83e847cb..bdc0bda33b 100644 --- a/crates/tui/src/tools/schema_sanitize.rs +++ b/crates/tui/src/tools/schema_sanitize.rs @@ -1305,10 +1305,14 @@ fn sanitize_kimi_parameters_candidate( /// `normalize_kimi_compatibility` has converted every non-root `oneOf` to /// `anyOf`, and root composition was already flattened before that. /// Schema values under keywords MFJS rejects (`contains`, `prefixItems`, -/// `patternProperties`, `not`, `if`/`then`/`else`, tuple-form `items`) need -/// no visit: the keyword itself fails closed — `UnsupportedKeyword`, or +/// `patternProperties`, `not`, `if`/`then`/`else`, `propertyNames`, +/// `unevaluatedProperties`, `unevaluatedItems`, nested `allOf`, tuple-form +/// `items`) need no visit: the keyword itself fails closed — +/// `UnsupportedKeyword`, `UnsupportedNestedAllOf` for a nested `allOf`, or /// `InvalidSchemaNode` for tuple-form `items` — before a stranded empty -/// `items` node inside one could matter. +/// `items` node inside one could matter. `dependentSchemas` is the one +/// exception in shape: `strip_dependent_keywords` removes it (with its +/// subtree) rather than failing closed, so it cannot strand one either. /// /// Children are visited before the node's own `items` check so a nested /// repair can empty the parent (`"items": {"items": {}}` is vacuously true diff --git a/docs/TOOL_SURFACE.md b/docs/TOOL_SURFACE.md index 0eac2ea90f..6d9fa62362 100644 --- a/docs/TOOL_SURFACE.md +++ b/docs/TOOL_SURFACE.md @@ -84,8 +84,8 @@ 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. 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 +activated set too: revalidation and admission re-serialize cached tool +definitions 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 @@ -128,16 +128,18 @@ whether the tool left the catalog (a disconnect, a changed allow/deny rule, a mode switch — any catalog-shrink cause) or outgrew the per-tool serialized-size budget. A cached tool that became eager is not reported at all — it stays directly callable. A catalog-shrink -drop is reported when a search observes it first: every search revalidates -the cache before admitting its matches, so a cached-but-departed name -surfaces in that result's unavailable references with the catalog-shrink -reason, whether or not the query would have matched it. Only a shrink -first observed at a turn boundary is absorbed silently — the name leaves -the cache there, so no later search can name it and the model learns of it -only when a search for the capability misses. An oversize drop observed by -a non-search path is likewise silent in the moment but re-reported with -its reason the next time a search returns the tool (a match ranked past -the result cap re-reports nothing). +drop is reported only when a search observes it first: every search +revalidates the cache before admitting its matches, so a cached-but-departed +name surfaces in that result's unavailable references with the +catalog-shrink reason, whether or not the query would have matched it. Any +other first observer — a turn boundary's revalidation, a hydration batch, +an executed tool's refresh, a retrieval admission, or the child surface's +request rebuild — absorbs the drop silently: the name leaves the cache +there, so no later search can name it and the model learns of it only when +a search for the capability misses. An oversize drop observed by a +non-search path is likewise silent in the moment but re-reported with its +reason the next time a search returns the tool (a match ranked past the +result cap re-reports nothing). Each subagent gets its own policy-filtered deferred catalog, always-present `tool_search`, and activation cache (unbounded in this fork, like the parent