fix(agent): narrow background-review tool schema for local providers

[JimStenstrom]

What does this PR do?

Narrows the background-review fork's advertised tool schema to memory+skills when the parent runs on a local endpoint (omlx / Ollama / llama.cpp), where there is no prefix cache to preserve. On a local endpoint, advertising the parent's full toolset buys nothing and actively harms: a weaker local model imitates the snapshot history (write_file/terminal/etc.) and burns review turns hitting the dispatch deny-wall (Background review denied non-whitelisted tool: …). Remote/cache-backed providers (Anthropic, OpenRouter) keep full-toolset inheritance so tools[] stays byte-identical for prefix-cache parity (the behavior established by #29704). The gate reuses the existing is_local_endpoint(base_url) predicate; the runtime whitelist remains as the dispatch guard in both cases.

Related Issue

Fixes #39996

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• agent/background_review.py — in _run_review_in_thread, compute _review_enabled_toolsets/_review_disabled_toolsets via is_local_endpoint(base_url): local → ["memory","skills"]/None; otherwise → the parent's enabled_toolsets/disabled_toolsets (unchanged). Comment updated to explain the cache-vs-local tradeoff.
• tests/run_agent/test_background_review_toolset_restriction.py — two tests: test_background_review_narrows_toolset_for_local_endpoint (local base_url → fork gets ["memory","skills"]/None) and test_background_review_keeps_parent_toolset_for_remote_endpoint (Anthropic base_url → fork still mirrors parent toolsets, cache parity preserved).

How to Test

1. scripts/run_tests.sh tests/run_agent/test_background_review_toolset_restriction.py — passes.
2. Local repro: point a custom provider at a local server, run a session with file edits / terminal calls so a bg-review fires. Before: the review session emits non-memory/skill tool calls that hit Background review denied non-whitelisted tool: …. After: the fork advertises only memory/skills, so those denied calls stop.
3. Remote regression: with an Anthropic/OpenRouter provider, the review request tools[] is unchanged (cache parity).

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(agent):)
• I searched for existing PRs (nearest is feat(background_review): make review-fork toolsets config-overridable #36967 — a config-override for tool-count caps; this is an automatic local-only gate, no config — low overlap)
• My PR contains only changes related to this fix
• I've run the affected tests via scripts/run_tests.sh and they pass
• I've added tests for my changes
• I've tested on my platform: macOS 15 (Darwin 25.5, Apple Silicon)

Documentation & Housekeeping

• N/A — no docs/config keys/schemas changed; runtime whitelist behavior unchanged

[teknium1]

Thanks for isolating the local-provider schema mismatch.

Problems

• Current main now resolves an auxiliary review runtime before fork construction (agent/background_review.py:673) and passes _rt.get("base_url") to the fork (:706). This patch bases its decision on the older parent runtime, so an auxiliary.background_review route can classify the wrong endpoint.
• Current main also gates the review memory tool on _memory_enabled / _user_profile_enabled (agent/background_review.py:802-808; regression test at tests/run_agent/test_background_review_toolset_restriction.py:161-199). The proposed local ["memory", "skills"] branch would bypass that invariant.

Suggested changes

• Rework the gate around the resolved review runtime (_rt) and preserve the existing conditional memory-toolset construction for local reviews.
• Add routed-endpoint and memory-disabled local regression cases; retain parent-toolset inheritance for non-local reviews so the cache-parity contract from c3a09f783 remains covered.

This is an automated hermes-sweeper review.

[JimStenstrom]

Reworked per the review — both points were right, thanks.

• The gate is now keyed on the resolved review runtime (_rt), so an auxiliary.background_review route is classified by the endpoint that actually serves the review. Added a regression case where a remote parent routed to a local aux endpoint still narrows, with the fork constructed on the routed base_url.
• The narrowed schema now mirrors the runtime whitelist's memory gate: ["skills"], plus "memory" only when _memory_enabled / _user_profile_enabled — no schema-level re-grant on memory-disabled profiles. Added a memory-disabled local regression case.
• Parent-toolset inheritance for non-local reviews is unchanged and still covered, so the cache-parity contract from c3a09f783 remains intact.

Branch rebased onto current main; 9/9 in tests/run_agent/test_background_review_toolset_restriction.py, 42/42 across the background-review suites.

[rrosson]

Draft 1 — comment on issue #39996 AND PR #39997 (toolset-narrowing for local providers)

Target: #39996
Also cross-post to: #39997 (the PR)

---

Confirming this from an independent deployment. We hit the exact same failure mode running background_review with the fork routed to a local llama.cpp backend: advertising the parent's full tool schema to the local model caused an outright request failure rather than just wasted turns — one specific tool schema (an MCP plugin's vault_patch-style tool, which used a $ref nested inside an anyOf for one parameter) couldn't be converted into a grammar by the local backend's tool-calling constraint compiler, and that failed the entire request even though the review would never have called that tool (the runtime whitelist already restricts routed reviews to memory+skills).

We independently wrote effectively the same fix locally (narrow enabled_toolsets to ["memory", "skills"] — plus "memory" conditionally — whenever the fork is routed to a different model, gated the same way #39997 does): a good sign this is the right fix, not just a workaround.

One thing worth adding to the PR description: because a local backend can build a full JSON-grammar for every schema in tools[] up front, this bug isn't limited to "weak local models thrashing the deny-wall" (the framing in #39996) — it can also produce a hard failure on backends that eagerly compile tool grammars, if the parent's toolset contains even one schema shape the backend's grammar compiler can't handle. That's a stronger argument for landing #39997 than just wasted turns.

Happy to test #39997 against our setup if that'd help move it forward — it's been open since June 5 with no update since July 14.