Run a capability's `prepare_tools` over every tool the availability gate admits

Merged
DO
2mo ago
+556-28

Availability is an input to tool resolution, not just to the execution gate, but ToolManager.for_run_step cached the resolved tool set on run_step alone. When availability changes after preparation, the gate reads live state while prepare_tools filtering stays baked into the cached set.

CombinedCapability.prepare_tools only dispatches to capabilities that are active, so a capability that becomes active mid-step never had its prepare_tools run — its tools sit in the resolved set ungoverned, and the dispatch then executes them. A capability using prepare_tools as a permission filter is silently bypassed.

Two routes reach that, both reproduced on main, both closed here.

The issue's claim that this was reverted is wrong on current main

#7305 says the fail-open was introduced by #7271 and reverted in e9ff35450, and that on main "the filter held". It does not hold. Differential repro on 68252f190 (a deferred capability whose prepare_tools filters secret_op out, plus a ProcessHistory capability injecting a complete LoadCapabilityCallPart/LoadCapabilityReturnPart pair; a FunctionModel calls secret_op on step 1):

WITHOUT the ProcessHistory injection: prepare_tools=[]              refusals=["Tool 'secret_op' is not available yet..."]
WITH the ProcessHistory injection:    prepare_tools=[['secret_op']] returns=['EXECUTED']  refusals=[]

The capability's prepare_tools runs, receives ['secret_op'], returns [] — and secret_op executes anyway.

The route in is different from the one the issue describes, which is why it survived the revert. _refresh_loaded_capability_ids is not called after processing today, so ctx.deps.loaded_capability_ids is still stale at dispatch — but _handle_tool_calls computes AnchoredEvidence.loaded_capability_ids from the post-compaction window of the response, over the history the processor has already rewritten, and RunContext.is_tool_available honours loaded_capability_ids | evidence.loaded_capability_ids. _capability_active (which gates prepare_tools dispatch) does not consult that evidence. So the gate says available and the filter never ran.

Same mechanism, other consequence: Bug 1 as written in the issue ("advertised, then refused") no longer reproduces on main — the anchored evidence already lets the injected load through.

Route 1 — availability moved because history processing rewrote it

Follows the issue's proposed direction:

  1. ToolManager records resolved_capability_ids — a snapshot of the capability-activity set its cached tools were resolved against.
  2. for_run_step returns self only when run_step and that snapshot both match; otherwise it re-resolves, so prepare_tools runs with the capability active and governs its tools.
  3. _refresh_loaded_capability_ids is called after ctx.state.message_history[:] = messages, so the run's own prospective state agrees with the history it is sending. The documented "has the runbook been loaded?" check (id in ctx.loaded_capability_ids) then answers correctly on the same step, instead of the capability being active only through dispatch-time evidence.

Per the scoping comment on the issue, discovered_tool_names is deliberately not in the key, so a response-side search that reveals a tool adds no rebuild. For every run that doesn't move capability availability mid-step, the build count is identical to today.

Both invariants that comment calls out are honoured, and each has a test that fails without it:

  • Retry carry-over. It sits after the run_step short-circuit, so a same-step rebuild must not re-run it. The rebuild carries self.ctx.retries through unchanged (the dispatch-time context arrives with none of its own, so dropping it would hand every tool a fresh budget mid-run) and recomputes only when run_step actually advanced. The per-step failed_tools/succeeded_tools accumulators are carried too, in both directions — failed_tools so the next step's carry-over still charges the failure, succeeded_tools so it still clears the count of a tool that recovered.
  • for_run_step is not idempotent on the toolset side. DynamicToolset.for_run_step exits and re-enters its inner toolset, re-running the factory. The same-step rebuild reuses the already-transitioned instance and only re-runs get_tools — which is what re-runs prepare_tools.

Also on the suspended-resume path: _prepare_resume_request runs the same history processing and dispatches the continuation's tool calls against the result. Reproduced there too, so it gets the same refresh and its own regression test.

Route 2 — availability differs because dispatch anchors its evidence to the serving provider

Surfaced by external review of the first commit, then reproduced. is_tool_available authorizes from active_capability_ids | evidence.loaded_capability_ids; _capability_active read active_capability_ids alone. A capability in the evidence and absent from the conservative window had its tool executed with its filter never dispatched — the same fail-open from the opposite direction, and one a rebuild could not correct because the new cache key ignored the evidence too.

It reproduces on two of the three boundaries _compaction_part_is_wire_boundary rejects (#7442), with a serving response that carries a provider_name:

case 1: foreign cmp (anthropic), serving=openai       : prepare_tools=[] returns=['EXECUTED']  -> FAIL-OPEN
case 3: content-less anthropic cmp, serving=anthropic : prepare_tools=[] returns=['EXECUTED']  -> FAIL-OPEN
control: no compaction, serving=openai                : prepare_tools=[['secret_op']] refusals=["Unknown tool name..."]
control: same-provider cmp with content               : prepare_tools=[] refusals=["not available yet..."]

Case 1 is the documented FallbackModel provider-switch scenario. Case 2 (openai-without-encrypted-content) does not fail open: there the gate's history-only reading is stricter than the adapter's render condition, which _compaction_part_is_wire_boundary documents as erring toward refusing.

Fix: both questions now read one set, RunContext._dispatch_active_capability_ids, and ToolManager.for_run_step keys its cache on it. The evidence is narrowed to the run's configured deferred ids — inert for both predicates, since each looks up an id that came from a registered capability, but it keeps a history naming a de-registered capability from rebuilding the tool set on every dispatch, and it reads a field that crosses the Temporal activity boundary, where the live capabilities registry raises. That last point is not hypothetical: narrowing on capabilities instead breaks two existing Temporal tests, which is how the constraint was found.

#7442's own intent is preserved and still covered by its existing tests: with no filter, such a call still executes rather than being refused.

Tests

Ten cases across tests/test_capability_deferred_loading.py, tests/test_tool_availability.py, and tests/test_toolsets.py. Seven fail on unfixed main. The two that pass there are the conservative directions, kept deliberately: the pre-existing fail-closed refusal is unchanged, and a legitimate injected load is not now over-blocked.

Mutation-tested — thirteen mutations of the fix, each caught:

mutationtests failing
cache on run_step alone (no fingerprint)7
drop the post-processing refresh (normal path)1
drop the post-processing refresh (resume path)1
same-step rebuild re-runs the retry carry-over1
same-step rebuild drops the accumulated retries1
same-step rebuild re-runs the toolset step transition1
same-step rebuild drops failed_tools only1
same-step rebuild drops succeeded_tools only1
fingerprint holds the live set, not a snapshot6
_capability_active ignores the anchored evidence3
cache key ignores the anchored evidence3
gate authorizes from a set the filter gate does not share7
narrowing reads the live registry (breaks inside a Temporal activity)1 (existing Temporal test)

Deliberately out of scope

  • #7298 (same-turn double load_capability) is not covered — verified, not assumed: two load_capability calls for one id in a single response still produce two load returns and no retry, before and after. Both calls are dispatched from one _handle_tool_calls, with no for_run_step between them, so the cache key never comes into play. Left on its own issue.
  • Capability model settings lagging by one request for an injected load: get_model_settings runs before before_model_request, so this doesn't fall out of the change. Left as noted in the issue.

Checklist

  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • No breaking changes in accordance with the version policy.
  • Any permitted compatibility impact has a label, warning, migration, release note, and exact API-check waiver.
  • PR title is fit for the release changelog.