fix(agent): inherit reasoning_config in background review fork (#18871)

[izzzzzi]

Summary

Fix for issue #18871 (open). _run_review_in_thread (agent/background_review.py) now clones the parent's reasoning_config, service_tier, and request_overrides into the forked AIAgent.

Without this, a parent session configured for xhigh effort (or none on thinking models) could still spawn a review that costs tokens the user explicitly opted out of — because a fresh AIAgent with reasoning_config=None falls back to a transport-local medium default on Codex Responses and Ollama routes.

Changes

• agent/background_review.py: Added 3 kwargs to the review fork's AIAgent constructor: reasoning_config, service_tier, request_overrides. Each uses getattr(agent, ...) for backward compatibility with parents that don't carry these fields.
• tests/run_agent/test_background_review_reasoning_inheritance.py: 7 new tests locking the contract:
  • reasoning_config inherited verbatim (xhigh, none, None)
  • service_tier inherited
  • request_overrides inherited
  • All three inherited together
  • None forwarded explicitly (not absent — prevents regression)
  • enabled_toolsets still inherited (sanity)

Testing

python -m pytest tests/run_agent/test_background_review_reasoning_inheritance.py -v  # 7/7 passed
python -m pytest tests/tools/ tests/run_agent/ --tb=short -q  # 163/163 passed

Related

• Issue: Background review agents ignore reasoning_config and fall back to medium #18871
• Builds on existing review fixes: [Bug]: Background self-improvement review's fresh AIAgent generates a system prompt that bytes-differs from the parent, busting prompt cache + preprocessing tree-dedup #25322 (system prompt cache), Background skill-review agent can perform non-skill side effects after creating a skill #15204 (toolset restriction), background_review fork sends wider tools[] than parent, fragments Anthropic prefix cache (~50% wasted cache-write on long sessions) #29567 (tools[] cache parity)

[alt-glitch]

Duplicate of #27510, which applies the same fix in the same post-refactor location (agent/background_review.py) — forwarding reasoning_config (plus service_tier/request_overrides) into the forked review AIAgent. Both target issue #18871. Earlier attempts #18973 and #20674 fixed the old run_agent.py location. Consolidating on one of these would help maintainers pick a single fix.

[teknium1]

Thanks for the focused regression report. The underlying bug remains on current main: the live review-fork construction at agent/background_review.py:699-715 omits reasoning_config, while the Codex transport defaults an absent config to medium at agent/transports/codex.py:156-164.

Problems

• The submitted hunk is based on the older constructor shape. Current main now resolves review runtime metadata before constructing the fork (agent/background_review.py:46-110,699-715), so this needs a targeted salvage rather than a clean cherry-pick.
• request_overrides is already correctly inherited through _rt (agent/background_review.py:65-70,709) and can be route-specific for an auxiliary review model (agent/background_review.py:95-106). A direct parent-attribute replacement should not be carried forward.
• The explicit-None test is not behaviorally meaningful because the constructor already defaults reasoning_config to None (run_agent.py:463; agent/agent_init.py:624).

Suggested changes

• Port only parent reasoning_config propagation into the current constructor and cover enabled/high-effort plus disabled-reasoning parent configurations.
• Retain the current resolved request_overrides path.

Automated hermes-sweeper review.

[kshitijk4poor]

Thanks for the fix. The core of this PR — inheriting reasoning_config in the background review fork — has now landed on main via #64379, a salvage of #30532 (which paired the identical fix with Anthropic cache-parity framing, wire-level cache measurements, and a regression test in the existing cache-parity suite). Current main additionally gates the inheritance on the non-routed path (if not _routed:), so a review routed to a different aux model via auxiliary.background_review.{provider,model} uses provider defaults rather than the parent's effort vocabulary.

On the other two fields this PR forwards:

• request_overrides — already inherited on current main: the fork site passes request_overrides=_rt.get("request_overrides") or {}, and _resolve_review_runtime() seeds that from the parent's request_overrides on the non-routed path.
• service_tier — this one is a genuine residual gap on main (the fork still defaults to None even when the parent session sets e.g. flex tier). It's a billing-tier nuance rather than a cache-parity issue, and it deserves the same routed/non-routed gating discussion as reasoning_config, so it's best handled as its own small follow-up rather than bundled here.

Closing as superseded for the merged portion. Thanks for the contribution — the service_tier observation is a good catch.