fix(agent): stop the between-turns tool refresh from forking the cached prefix (#100336 defect b)

[JoaoMarcos44]

Summary

fixes #100336

This PR takes defect (b) of #100336 — "the tool listing embeds environment-dependent state". #100358 owns defect (a) (the nulled system_prompt_hash on a /model switch) and explicitly scopes (b) out; the two are complementary and touch disjoint code.

The reporter's second symptom is the one that fires without a model switch:

one call's input shrank 4,555 tokens versus the previous call despite 950 tokens of new history being appended — the assembly itself changed shape

The shrink is real, it is reproducible, and it has a precise source: the per-turn MCP refresh republishes agent.tools from live availability, so an availability probe that flips silently removes a tool from a request prefix the provider has already cached.

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#00f0ff', 'mainBkg': '#0a0a16', 'primaryTextColor': '#ffffff', 'primaryBorderColor': '#ff007f', 'lineColor': '#00f0ff'}}}%%
graph TD
    A[Turn N+1 Prologue] --> B[refresh_agent_mcp_tools]
    B --> C[get_tool_definitions]
    C --> D{check_fn probe<br/>TTL 30s / grace 60s}
    D -->|True| E[Tool in schema list]
    D -->|False| F[Tool omitted]
    F --> G[agent.tools = new_defs<br/>SHRUNKEN ARRAY]
    E --> H[Late MCP tool<br/>sorted into index 0]
    H --> G
    G --> I[Cached prefix forked]
    I --> J[Full re-prefill<br/>~136k tokens]

    A --> K[preserve_prefix=True]
    K --> L[Live order authoritative]
    L --> M[Flapped tool carried forward]
    L --> N[Deregistered tool dropped]
    L --> O[New tool appended at tail]
    M --> P[Prefix byte-identical<br/>cache holds]
    N --> P
    O --> P

Loading

---

Cause — line by line

agent/turn_context.py:645-659 runs a tool-snapshot rebuild in every turn prologue whenever any MCP server is registered. Its comment states the contract it believes it has:

# This is cache-safe by construction: it runs in the per-turn prologue,
# before this turn's first API call assembles tools=, so it
# only ever extends a fresh request prefix

The timing half of that is true. The content half is not, and nothing enforced it.

tools/mcp_tool.py:8226-8297 — refresh_agent_mcp_tools:

1. new_defs = get_tool_definitions(...) (line 8226) recomputes the array from scratch.
2. model_tools.get_tool_definitions memoizes on (scope, toolsets, registry._generation, config mtime, …) — check_fn results are not part of the key. Any registry generation bump (an MCP server reconnecting, a plugin load, a lazy-server adoption) invalidates the memo and forces a full recompute.
3. That recompute reaches registry.get_definitions (tools/registry.py:1061-1071), which drops every tool whose check_fn currently returns False. _check_fn_cached has a 30 s TTL and a 60 s success-grace (tools/registry.py:269-417), so a probe that stays down past the grace is honoured — the exact check_fn ... returned False lines the reporter sees 13 of.
4. agent.tools = new_defs (line 8289) publishes the shrunken array wholesale.
5. The function returns new_names - current (line 8297) — additions only. Removals are invisible to the caller, which is why the prologue's "only ever extends" comment survived.

There is a second, independent prefix fork on the same line. registry.get_definitions iterates sorted(tool_names) (line 1061), so a genuinely new tool does not extend the array — it splices into sorted position, which for aaa_*-prefixed MCP tools is index 0. Even the intended additive case moved every byte of the tool block.

Reproduction (no network, no provider)

Three registry tools, one with a flipping check_fn, driven through the real refresh helper:

turn 1 tools : ['alpha', 'bravo', 'charlie']
turn 2 tools : ['alpha', 'charlie']            added= set()
prefix forked on flip-down: True
turn 3 tools : ['alpha', 'bravo', 'charlie']   added= {'bravo'}
prefix forked on flip-up  : True
turn 4 tools : ['aaa_late', 'alpha', 'bravo', 'charlie']
late add landed at index  : 0

Two forks per availability flap, a third for the late arrival, and added reports nothing on the way down.

---

Consequence

• Cache. Every provider that renders tools ahead of the messages (vLLM and every OpenAI-compatible chat template; Anthropic's tool block sits before the messages too) re-prefills the entire conversation behind the moved byte. On the reporter's ~139k-token session that is ~136k tokens per incident — the measured 99% → 2% collapse, with no model switch involved.
• Frequency. Unlike a /model switch this needs no user action. Any probe that flaps — a headless box's browser/CDP checks, a docker daemon blip, an OAuth credential whose refresh fails, x_search's xAI resolver — forks the prefix, then forks it again on recovery.
• Silent capability loss. The model loses a tool mid-conversation with no signal to the caller or the user, because the helper only reports additions.
• Cost. Latency and spend, both proportional to session length, and worst exactly where prompt caching matters most.

---

Solution

refresh_agent_mcp_tools(..., preserve_prefix=True) — a new keyword used by the one caller that rebuilds inside a live conversation, the between-turns prologue. Under it the live array is authoritative rather than the freshly sorted one, and _merge_preserving_prefix folds the new snapshot into it:

Tool is…	Before	After
in both lists	rebuilt into sorted position	keeps its slot, takes the fresh schema
live only, still registered (probe flapped)	dropped → prefix forks	carried forward → prefix intact
live only, deregistered (server/plugin gone)	dropped	dropped
new only (late MCP connect)	spliced into sorted position	appended at the tail

The distinction that makes this safe is why a tool vanished. A check_fn returning False means the tool is still registered and only its availability probe flipped; a server shutting down means the entry left the registry and its handler is gone. The merge reads registry membership — snapshotted outside _agent_tools_lock, so registry._lock is never nested under it — and keeps only the first class.

Carrying an unavailable tool forward changes nothing about dispatch: check_fn gates exposure at snapshot time, never invocation (registry.dispatch does not consult it), every handler already owns its own unavailability error, and _check_fn_cached's 60 s success-grace already deliberately keeps flapping tools visible. This extends that same policy to the lifetime of a live session.

Explicit /reload-mcp (TUI, ACP, CLI, gateway) and the compaction boundary keep the plain rebuild — a user who just disabled a toolset expects it gone, and compaction resets the prefix anyway.

Post-fix

turn 2 tools : ['alpha', 'bravo', 'charlie']   prefix forked on flip-down: False
turn 3 tools : ['alpha', 'bravo', 'charlie']   prefix forked on flip-up  : False
turn 4 tools : ['alpha', 'bravo', 'charlie', 'aaa_late']   late add landed at index: 3

---

Files

File	Change
tools/mcp_tool.py	preserve_prefix keyword + _merge_preserving_prefix helper
agent/turn_context.py	between-turns prologue passes preserve_prefix=True; the "only ever extends" comment now describes what the code does
tests/tools/test_refresh_agent_mcp_tools.py	5 regression tests

Test plan

• python -m pytest tests/tools/test_refresh_agent_mcp_tools.py -q — 13 passed (5 new)
• python -m pytest tests/tools/test_mcp_tool.py tests/test_compaction_tool_refresh.py -q — 103 passed, 1 failed; the failure (test_windows_location_vars_passed_without_secrets, missing ProgramFiles in this Windows shell env) reproduces on unmodified main and is unrelated
• python -m pytest tests/agent/test_turn_context.py tests/agent/test_turn_context_overflow_warning.py tests/test_get_tool_definitions_cache_isolation.py -q — 34 passed
• Long gateway session with MCP servers on a box where a browser/CDP probe fails: the tool block stays byte-identical across turns, and a slow MCP server's tools appear at the end of the array

Refs #100336. Complements #100358 (defect a) — no overlapping files.

[JoaoMarcos44]

Exhaustion sweep — three passes over the remaining prefix bottlenecks

Pass 1 — concurrency and ordering holes in the merge

The merge runs inside _agent_tools_lock, and registered_names is snapshotted outside it, so the two orderings had to be proven safe rather than argued. Six threads × 400 refreshes with one probe flipping randomly, plus an interleaved deregistration:

concurrent flap violations: 0
after dereg: ['a_read', 'b_term', 'c_web']

No duplicates, no lost tools, no moved slot, and a genuine deregistration still lands exactly once. Pinned as test_preserve_prefix_holds_the_prefix_under_concurrent_flapping — the assertion is positional, not set-based, because a set-equal-but-reordered array is exactly the failure this PR exists to stop.

Pass 2 — the tool_search bridge listing

Worth checking, because assemble_tool_defs (tools/tool_search.py:951) bakes both len(deferrable) and a rendered catalog into the bridge tool's description — that is literally the issue's "~4,000-token dynamic tool listing". A flapping deferrable tool changes those bytes under a stable tool name.

It turns out that vector was already closed, and it is worth recording why: the between-turns caller is not content_aware, so a name-stable content change hits the new_names == current early return and is never republished. Verified directly against the refresh helper with a bridge-shaped schema whose catalog text shrinks:

preserve_prefix=False | listing bytes republished: False
preserve_prefix=True  | listing bytes republished: False

The name-set vector this PR fixes was the only open one on that path — and now that a flapped tool is carried forward, the merged name set matches too, so the early return does the rest.

Pass 3 — the one bottleneck this PR does not close

gateway/run.py:6083-6150 — the gateway agent cache is an LRU (_AGENT_CACHE_MAX_SIZE = 128, plus _sweep_idle_cached_agents and a cross-process message_count invalidation). When an entry is evicted, the same live session rebuilds a fresh agent, and agent_init.py:1627 re-derives agent.tools from live check_fn results with no previous snapshot to merge against. A probe that is down at that moment produces a different array than the one the provider cached, for a conversation that already has a long history.

This PR cannot reach it: the merge needs an in-memory predecessor and the agent object is gone. Closing it means persisting the session's resolved tool-name set — the "session-toolset freeze" #100358 names as separate work. Flagging it rather than half-building it here; happy to take it as a follow-up if maintainers want it in this cycle.

Regression sweep

• tests/tools/test_refresh_agent_mcp_tools.py — 14 passed (6 new)
• tests/tools/test_registry.py tests/test_model_tools.py tests/tools/test_tool_search.py — 97 passed
• tests/test_toolsets.py tests/tools/test_tool_search_multiquery.py — 65 passed with the suite above
• tests/agent/test_turn_context.py tests/agent/test_turn_context_overflow_warning.py tests/test_get_tool_definitions_cache_isolation.py — 34 passed

[kshitijk4poor]

Closing for now — not on quality. The analysis is correct (I re-verified: registry.get_definitions drops check_fn == False tools and iterates sorted(names); check_fn verdicts aren't in the get_tool_definitions memo key; registry.dispatch never consults check_fn, so carrying a flapped tool forward is dispatch-safe), the cherry-pick lands cleanly on current main, and _merge_preserving_prefix is a nice helper.

The reason to hold it is that it needs a maintainer policy decision that hasn't been made yet, and merging it now would pre-empt that decision in one direction:

• Open fix(tools): availability-gated tools appear when their credential/daemon does (salvage #92693, part 2) #95053 (maintainer salvage of fix(tool-search): parallel-execution barrier, listing truncation, source indexing, availability-cache staleness #92693) goes the opposite way — it makes availability-gated tools appear/disappear when the check_fn verdict flips, explicitly accepting a one-time prefix bust as the cost of correctness. fix(agent): respect check_fn TTL in external tool-definitions cache #84783 is in the same space. This PR pins the array instead. Both can't be the contract.
• The Prefix-cache invalidation on model switch: stored system prompt is nulled, forcing full re-prefill of entire history (2% -> 99% cache hit measured) #100336 triage's stated fix direction is a per-session tool-availability snapshot. This PR approximates that, but only on the MCP-registered prologue path (has_registered_mcp_tools() gates it), so sessions without MCP servers get nothing, and as you noted yourself the gateway agent-cache eviction path rebuilds from live probes with no predecessor to merge against. A session-level freeze would cover all three doors at once.
• Prefix-cache invalidation on model switch: stored system prompt is nulled, forcing full re-prefill of entire history (2% -> 99% cache hit measured) #100336 was withdrawn by its reporter, so the P0 framing no longer applies; defect (b) is real but P2-graded.

Once the freeze-vs-flip policy is settled, this merge helper is the obvious building block for the freeze side and I'd be glad to see it re-opened or re-picked (authorship preserved) against that design. Thanks for the thorough sweep — the concurrency proof and the tool_search bridge rule-out in your follow-up comment saved real time.

[teknium1]

Reopening — Teknium decided the freeze direction (Sep 2 2026): availability-gated tools stay fixed for a session; tools[] changes only on /reload-mcp, /new, or compaction. #95053 and #84783 (flip direction) are closed.

Your _merge_preserving_prefix in the shared refresh path is step 1. Step 2 (we'll do it, or you're welcome to): persist the resolved tool-name set on the session row (same pattern as the persisted system_prompt) and filter on rebuild so the gateway agent-cache eviction door you found also stops re-probing; plus invalidate_check_fn_cache() inside /reload-mcp as the explicit escape hatch. Please rebase onto current main when you get a chance and we'll review.

[JoaoMarcos44]

Reopening — Teknium decided the freeze direction (Sep 2 2026): availability-gated tools stay fixed for a session; tools[] changes only on /reload-mcp, /new, or compaction. #95053 and #84783 (flip direction) are closed.

Your _merge_preserving_prefix in the shared refresh path is step 1. Step 2 (we'll do it, or you're welcome to): persist the resolved tool-name set on the session row (same pattern as the persisted system_prompt) and filter on rebuild so the gateway agent-cache eviction door you found also stops re-probing; plus invalidate_check_fn_cache() inside /reload-mcp as the explicit escape hatch. Please rebase onto current main when you get a chance and we'll review.

Imma do this

[teknium1]

Merged via #101367 — your _merge_preserving_prefix commit cherry-picked onto current main with authorship preserved (tests trimmed to the two positional invariants). On top: the gateway agent-cache eviction door you flagged in #100336 is closed by persisting the session's resolved tool_names and folding the rebuilt agent's tools through the same helper, and /reload-mcp now re-probes check_fn verdicts as the consented escape hatch. Thanks for the analysis that settled the freeze-vs-flip question.