Skip to content

feat(background_review): make review-fork toolsets config-overridable - #36967

Closed
Wetnesday wants to merge 1 commit into
NousResearch:mainfrom
Wetnesday:feat/configurable-background-review-toolsets
Closed

feat(background_review): make review-fork toolsets config-overridable#36967
Wetnesday wants to merge 1 commit into
NousResearch:mainfrom
Wetnesday:feat/configurable-background-review-toolsets

Conversation

@Wetnesday

Copy link
Copy Markdown

What

Makes the background skill-review fork's toolsets config-overridable via background_review.enabled_toolsets (and .disabled_toolsets) in agent/background_review.py.

Default behavior is unchanged: when the config keys are absent, the fork inherits the parent agent's toolsets exactly as before (so the request tools[] stays byte-identical for Anthropic prefix-caching).

Why

The review fork inherits the parent's full toolset so tools[] is byte-identical for cache parity. But on a provider with a hard tool-count cap — e.g. xAI/Grok's 200-tool limit — once enough MCP servers are enabled, the inherited set exceeds the cap and the review request 400s (N tools provided, maximum is 200), so the fork never runs and no skill is produced.

Since the runtime whitelist already restricts review dispatch to memory/skills, a narrower request set is lossless. This adds an opt-in escape hatch for capped providers without changing anything for Anthropic.

Behavior change

None unless background_review.enabled_toolsets is set in config. The config read is wrapped in try/except and falls back to the previous getattr(agent, "enabled_toolsets", None) inheritance.

Example config

background_review:
  enabled_toolsets: [memory, skills]   # pin the fork under a provider tool cap

The background skill-review fork inherits the parent agent's full toolset
so the request tools[] is byte-identical for Anthropic prefix-caching. But
on a provider with a hard tool-count cap (e.g. xAI/Grok's 200-tool limit),
once enough MCP servers are enabled the inherited set exceeds the cap and
the review request 400s ('N tools provided, maximum is 200') — the fork
never runs. Since the runtime whitelist already restricts review dispatch
to memory/skills, a narrower request set is lossless.

Add an optional config override read from background_review.enabled_toolsets
(and .disabled_toolsets). Default (key absent) inherits the parent exactly —
no behavior change for Anthropic; opt-in escape hatch for capped providers.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jun 2, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for targeting the review-fork schema cap without changing the default cache-parity path. Current main still forwards the parent toolsets to the fork at agent/background_review.py:711-712, so the requested capability is not superseded.

Problems

  • The patch reads top-level background_review, but the established review configuration is auxiliary.background_review in hermes_cli/config.py:1723 and is consumed by agent/background_review.py:77-82. This would split review settings across two namespaces.
  • The PR changes only agent/background_review.py; it has no regression test for configured overrides or the fallback to parent toolsets.

Suggested changes

  • Add the two keys under auxiliary.background_review in DEFAULT_CONFIG, read that existing mapping, and document the setting beside the current background-review configuration.
  • Add constructor-capture coverage for override and absent-key fallback behavior.

Current main has refactored this constructor for resolved runtime metadata in 97e9c64664, and GitHub reports this branch as conflicting, so salvaging should transplant the logic into that current path.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@Wetnesday Wetnesday closed this by deleting the head repository Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants