fix(tools): block compacted content in side-effect calls

[ugoi]

Summary

Add a fail-closed dispatch guard for synthetic context-compaction truncation markers in potentially side-effecting tool arguments.

Compaction can shorten historical tool_calls[].function.arguments and insert ...[truncated]. That historical copy remains valid JSON, so a later model turn can mistake it for complete content and reuse it in a fresh write/send/publish call. There is no evidence that the deferred tool transport truncates a fresh complete argument; the unsafe boundary is reuse of synthetic compacted content.

Security boundary

• Detect current dotted marker forms and the proposed named HERMES-CONTEXT-COMPRESSION sentinel recursively in string/nested arguments.
• Block before write-capable registry dispatch with an actionable exact-content recovery + fresh-confirmation error.
• Recheck after execution middleware, so middleware cannot inject a marker into the handler payload.
• Unknown/plugin tools default to write-capable (fail closed).
• Add internal read_only registry metadata; it does not alter serialized tool schemas or prompt-cache bytes.
• Propagate MCP readOnlyHint=True; missing/malformed hints remain write-capable.
• Mark core file/web reads as read-only, so harmless reads remain usable.

Reproduction

On unmodified origin/main, the regression test generated compacted args via _truncate_tool_call_args_json(), called write_file, and reached the mocked write handler. The focused red run failed 2 tests because dispatch occurred.

Tests

• compacted historical args cannot reach a write handler
• nested and named synthetic markers are blocked
• unknown outbound tools fail closed
• complete payload passes byte-for-byte
• read-only calls and ordinary prose mentioning truncation are not blocked
• execution-middleware injection is blocked
• registry metadata does not alter prompt schemas
• role-alternating history is not mutated
• MCP read-only hints propagate to registry metadata

Canonical focused suites and full-suite results are included in the PR checks / author comment.

Existing work

This complements rather than duplicates:

• fix(context-compressor): stop priming models to abbreviate their own tool-call arguments #83843 / fix(agent): stop emitting copyable truncation markers in compacted tool-call args #83735: stop or change the marker minted at one compaction source
• fix(tools): refuse patch/write_file when content contains a truncation placeholder #83752 / fix(file-tools): abort write/patch when AI truncation placeholders detected #68512: file-tool-only placeholder guards

Those do not provide a generic boundary for messaging, mail, publishing, payments, plugins, deferred MCP tools, or already-compacted history. This PR addresses the validation-hook ask in #83435 without mutating conversation history, role order, or tool schemas.

Refs #83435

[ugoi]

Verification on commit 55d56f9fe09c9ea7d2a99c93c141b6e73b83367d:

• RED on untouched origin/main: focused regression run failed 2 tests because marker-bearing write_file arguments reached the mocked handler.
• Focused post-commit: scripts/run_tests.sh tests/test_model_tools.py tests/tools/test_registry.py tests/tools/test_mcp_trust_gating.py — 84 passed, 0 failed.
• Adjacent compaction/MCP run: 7 files — 273 passed, 0 failed.
• ruff check . — passed.
• git diff --check HEAD^ HEAD — passed.
• ty base/head diff: 16,133 diagnostics on each; no net diagnostics. The report classified one existing checkpoint_manager.py checker panic as both new/fixed only because the absolute worktree path differed.
• Full canonical suite completed with 23 failures in 13 unrelated files. Re-running those exact 13 files against detached origin/main produced the same 23 failures. No failure touched changed code; focused/adjacent suites are green.

No live tool handler or external service was invoked; all side-effect assertions use mocked dispatch and synthetic .invalid fixture data.

[alt-glitch]

This was generated by AI during triage.

Related to #87985: both prevent compacted historical content from reaching effectful calls, while this PR adds the model-tools/registry read-only metadata path and middleware recheck. They are complementary rather than duplicates.

[trevorgordon981]

Strong, well-scoped fix for a genuinely nasty failure mode: after context compression, a model turn could echo a compactor's synthetic marker back into a side-effecting tool call, persisting truncated/corrupt content as if it were the real thing. Blocking that is the right call, and the design is careful.

Detection is precise, not a blunt substring match. _SYNTHETIC_TRUNCATION_MARKER_RE requires either the compressor's exact sentinel (⟪HERMES-CONTEXT-COMPRESSION: …⟫) or an ellipsis-bracket shape (...[truncated]), and _contains_synthetic_truncation_marker walks nested dicts/lists/tuples/sets with cycle protection, so a marker buried in a nested arg is caught. The false-positive tests are the right ones: test_legitimate_truncation_prose_is_not_blocked ("…the preview was truncated by the UI" — the .../[truncated] adjacency requirement is what distinguishes it, correctly) and test_read_only_tool_is_not_blocked_by_compaction_marker (a legit path containing ... [truncated]).

Fail-closed across the whole write surface. read_only metadata is added to ToolEntry (registry.py:221, read_only = read_only is True — anything other than exactly True falls through as write-capable), and it's plumbed through read_file/search_files/web_search/web_extract plus MCP readOnlyHint/resource-tool inference. So unknown/plugin tools, MCP tools without a read hint, and any future tool fail closed by default — the safe direction. The gate runs at three dispatch boundaries (_return_bridge_result:1519, and both _dispatch closures at 1618/1629) so a marker can't slip in via execution middleware either (covered by test_execution_middleware_cannot_inject_marker_into_write).

Two small notes (non-blocking):

• The check happens after some argument normalization but the block path correctly returns before any handler runs, and _emit_post_tool_call_hook is called with status="blocked" + error_type="compacted_tool_arguments" — good observability. The test_guard_does_not_mutate_existing_role_alternation test also pins that dropping the call doesn't corrupt the message history.
• _SYNTHETIC_TRUNCATION_MARKER_RE is now the second encoder-aware copy of the compressor's marker format (the compressor emits it, this scans for it). If the marker shape ever changes, both must change in lockstep — worth a cross-referencing comment or a shared constant. Minor.

Tests pass at head (test_model_tools.py, test_registry.py) and the MCP-trust/registry updates are consistent. Merge-ready from my side.

[teamster22]

Production hit of this exact boundary on v0.21.0, with a write target that isn't a file — posting it because the failure mode is silent rather than loud.

A cronjob_manage update carried a 9,383-char prompt. Compaction rewrote the session's history 3 minutes earlier; the next call replayed the compacted copy and the job store persisted 214 chars — 200 + ...[truncated]. success: true, no warning.

Timeline from state.db, one session:

22:15:59  action=update   prompt_len=9383   original
22:18:54  compaction rewrote 239 messages in this session
22:18:54  action=create   prompt_len=214    compacted copy
22:22:49  action=update   prompt_len=214    replayed -> written to jobs.json

Control: a fresh 803-char prompt in a single call stored intact (803, no marker). Consistent with this PR's framing that the transport is fine and reuse of compacted history is the unsafe boundary. The cron handler stores verbatim and has no length cap, so it faithfully persisted the damaged value.

Why this target class is worse than the file cases in #83714: ...[truncated] in Python source raises SyntaxError on next run. A truncated cron prompt is still a valid prompt. It runs on schedule, produces plausible output, and errors nowhere — detection requires re-reading jobs.json and comparing lengths. No linter or test ever reads that file.

The fail-closed default for unknown tools already covers cronjob_manage, so this needs no change to the PR. Recording it as evidence that the guard's value isn't limited to filesystem writers.

[kshitijk4poor]

PR Review — #96592

Verdict: Request changes / do not merge as-is. The bug is real and reproduced on main, the guard tests are meaningful (mutation-verified), but the design turns a targeted replay guard into a global content filter with a high false-positive rate and no override. A maintainer choice between this and #87985 (same bug, agent layer) is needed first.

Premise (verified on origin/main): agent/context_compressor.py:1265-1283 _truncate_tool_call_args_json still shrinks string leaves of historical tool_calls[].function.arguments to 200 chars + ...[truncated] (via _truncate_tool_call_args_at :2618-2630 for args > 500 chars), leaving valid JSON a later turn can replay into write_file/patch. No guard exists on main; tools/registry.py has no read_only field.

Conflicts: model_tools.py, tools/file_tools.py, tests/tools/test_mcp_trust_gating.py — mechanical (main moved the dispatch pre-checks into _pre_dispatch_guards); port applied for testing, 88 passed. Mutation (revert model_tools.py guard to main): 5 red — the guard tests bite. 4 of the 9 tests (byte-for-byte, read_only passthrough, prose, role-alternation) are non-discriminating by design.

Critical

• model_tools.py:798-804 (head) — _SYNTHETIC_TRUNCATION_MARKER_RE matched 5 of 7 realistic legit payloads in a probe: pasted pip output ... [truncated] 40 more lines, a markdown note quoting `...[truncated]`, a grep hit of context_compressor.py source, assert out.endswith('...[truncated]') in a test file, (…[truncated]…). Any write_file/patch/terminal carrying those is blocked with no override — it will hit developers editing this repo, log readers, and anyone quoting a tool error.
• Fail-closed default: with read_only unset on 91 of 95 tools, the guard is effectively global. If a dispatch guard is kept, the default must ship with a broad read_only marking pass (skill_view, skills_list, session_search, read_terminal, browser snapshot/console, kanban list/show, …).
• The PR pre-guards a not-yet-existing unique sentinel, which concedes the right fix is at the producer.

Warnings

• Middleware re-check duplicates the dispatch-side check; one seam is enough.
• 9 tests vs the ≤2-invariant bar.

Preferred path: fix at the single producer — have _truncate_tool_call_args_json emit an unambiguous structured placeholder (that's #87985's layer, and AGENTS.md's "narrow waist" rule points there). Then, if a dispatch guard is still wanted, salvage this PR's read_only registry/MCP readOnlyHint propagation (clean, matches _annotation_read_only_hint in tools/mcp_tool_registration.py:43) with credit, rebase into _pre_dispatch_guards, narrow the regex to the exact producer form, mark the obvious read tools, drop the middleware recheck and the speculative sentinel, cut tests to 2.

[kshitijk4poor]

Closing in favour of #87985 (@LevSky22, 2026-08-16, earlier submitter) as the carrier for the compacted-content-in-effectful-calls class; the verdict from my 09-07 review stands (5/7 legitimate payloads blocked by the regex; 91/95 tools fail closed with no override), and the branch has not moved since 08-27.

What is worth keeping from here, as a co-authored follow-up once #87985 lands: the read_only registry field and MCP readOnlyHint propagation — that is orthogonal to the detection mechanism and useful on its own. Thanks also to @teamster22 for the production hit; the true fix belongs at the producer (context_compressor emitting [truncated] into historical tool_call args), which #87985 addresses at the agent layer.