fix(*): give web search its config section so maxResults takes effect

[Handsome-wzw]

Summary

Stacked on #312 -- set the base branch to feat/tool_capability_inventory_gh,
not main. Against main the diff would carry that PR's commits as well. It
closes the naming nit raised in review there, so it is a follow-up rather than
an independent change.

tools.web.search.maxResults has been declared in the schema and read by
nobody. Both registration sites built the tool from the key alone:

WebSearchTool(api_key=self.brave_api_key, proxy=self.web_proxy)

so it fell back to its own default of 5, and a deployer who set the field got no
effect and no warning that the setting was inert.

One cause, two symptoms

Web search was the one tool handed a single field instead of its config section.
Every neighbour takes a section -- media_config, exec_config,
sandbox_config, context_config -- and all three CLI entry points already
held config.tools.web.search and reached into it to pull one value out:

brave_api_key=config.tools.web.search.api_key or None,

Passing the section is what wires the field. It also retires the parameter name
in the same move, which is the second symptom: brave_api_key has named a
Serper key ever since the Brave backend went away. That was flagged in review on
#312 as pre-existing and misleading, correctly, and it is not a separate rename
-- passing one field out of a section is exactly why the parameter ended up
named after a vendor rather than after what it configures.

WebSearchTool keeps its own signature. It is still constructible from a bare
key, which its unit tests and the error-path test rely on; only the callers that
have a config section now pass one.

The sub-agent surface

Same change there, and a test of its own. This matters more than it looks: a
first mutation pass caught the main loop dropping max_results and said nothing
when the sub-agent dropped it, because nothing was watching that call site. Two
registration sites with a fix applied to one is the shape this area keeps
taking, and it is the same gap the gate in #312 had to close by hand.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

The parameter rename is a refactor riding along, not a second topic: it is the
same edit that wires the field.

Verification

uv run --all-extras pytest -q                        6188 passed, 35 skipped, 1 failed
uv run pytest tests/test_agent_loop_web_tools.py \
              tests/test_tool_capabilities.py \
              tests/test_cli_doctor_commands.py      69 passed
uv run --extra dev ruff check raven tests scripts    All checks passed
uv run --extra dev ruff format --check               817 files already formatted
npx commitlint --from main --to HEAD                 exit 0
scripts/check_commit_messages.py main..HEAD          exit 0

The one failure is test_read_file_image.py::test_an_attachment_that_cannot_be_ read_costs_a_note_not_the_turn, which fails the same way on main with these
commits absent: it makes a file unreadable with chmod 000, which does not
block root, so it fails for anyone running the suite as root and passes in CI.
#313 is the fix. This branch touches neither that file nor the code under it.

Five mutations, each caught:

main loop drops max_results again        1 failed
sub-agent drops max_results again        1 failed
main loop ignores the section's key      4 failed
sub-agent ignores the section's key      2 failed
the web_search gate is removed           3 failed

The second line is the one worth reading. Before its test existed that mutation
passed clean -- the wiring was fixed at both call sites and asserted at one.

• Relevant tests pass locally
• Relevant lint / type checks pass locally
• User-facing docs or screenshots are updated when needed

Risk

One behaviour change, and it is the point: tools.web.search.maxResults now
reaches the tool. An install that set it was getting 5 results and now gets what
it asked for. An install that never set it is unaffected -- the schema default
is 5, which is also the value the tool fell back to. The per-call count
argument still wins over both, unchanged.

brave_api_key is gone from AgentLoop and SubagentManager. Both are
constructed inside this repo only -- the three CLI entry points and the tests --
and all were updated; no deprecation shim, since nothing external names it.

Rollback is a revert. No configuration is read differently beyond the field that
was already meant to be read.

• Security impact considered
• Backward compatibility considered
• Rollback path is clear for risky changes

Related Issues

N/A

[gloryfromca]

Blocking: one caller was missed by the brave_api_key -> web_search_config rename and now raises at construction; see the inline note.

The fix itself is right and the tests are honest. WebSearchTool.execute really does read self.max_results (n = min(max(count or self.max_results, 1), 10)), so passing the section instead of one field out of it is what makes tools.web.search.maxResults reach the request -- the PR title is accurate. Both registration sites are covered, and both new tests assert the value on the registered tool object rather than on the config that was passed in, so they would catch a regression at either site.

The subagent_run refactor is a strengthening, not a loosening: the spy still asserts runs is non-empty before appending, the collector still opens before the manager is constructed, and the existing test kept its "baselines first" guard against an empty list satisfying the not in. The three call sites in raven/cli/ and the two capability tests all pass a section whose api_key was set first, so nothing silently stopped being gated.

What I checked

• AGENTS.md: commit header (fix(*): ..., 61 chars, ASCII, lowercase, (*) for multi-scope) and branch name both conform; the reworded comment in main.py stays English and stays a why; no new test files, both edits land in the existing test_agent_loop_web_tools.py, so 5.1/5.4 hold.
• Callers: grepped every AgentLoop( / SubagentManager( construction in the repo. benchmarks/pinchbench/direct/raven_executor.py, benchmarks/pinchbench/bot_runner/bot_executor.py and benchmarks/appworld/agent_cli.py pass no search kwarg at all; benchmarks/clawbench/stream.py is the one that breaks.
• Backward compat: brave_api_key and the self.brave_api_key attribute are both gone from the public signature with no shim. In-repo that costs exactly the one call site below; if anything outside the repo constructs AgentLoop directly, it is a breaking change worth a line in the PR body.
• Tests: uv run pytest tests/test_agent_loop_web_tools.py tests/test_tool_capabilities.py tests/test_agent_loop_tool_search.py -q -> 42 passed, 0 skipped.

Not blocking

max_results is clamped to 10 inside execute, so a deployer who sets maxResults: 20 still gets 10 and no warning. That is pre-existing behaviour on lines this PR does not touch, and the config field is now at least honoured up to the cap -- just noting it since "maxResults takes effect" is the headline.

[gloryfromca]

This rename misses a caller. benchmarks/clawbench/stream.py:146 still passes brave_api_key=self.config.tools.web.search.api_key or None, and AgentLoop.__init__ takes no **kwargs, so every clawbench run now dies before the first task:

$ uv run python -c "from raven.agent.loop import AgentLoop; AgentLoop(provider=None, workspace='.', brave_api_key='k')"
TypeError: AgentLoop.__init__() got an unexpected keyword argument 'brave_api_key'

Ruff will not catch it (wrong kwarg, not a lint), and no test constructs that harness, so CI stays green while the benchmark entry point is broken. The one-line fix mirrors the three CLI sites:

web_search_config=self.config.tools.web.search,

Worth grepping for brave_api_key once more before merge -- that is the only remaining hit today.

[gloryfromca]

Fixed in cc715dc, and better than I asked for. benchmarks/clawbench/stream.py:146 now passes web_search_config=self.config.tools.web.search, and you also dropped the everos_config= line next to it -- which I had missed. That one is the more interesting half: everos_config is not in AgentLoop.__init__ either, so clawbench was already dead on the target branch and my "this PR breaks it" framing was only half right.

The static guard is the right shape for this. I checked it is not vacuous -- reverting the kwarg makes both tests fail with the offending file and line named:

$ uv run pytest tests/test_benchmark_entry_points.py -q   # with brave_api_key= put back
FAILED test_the_benchmarks_still_construct_agentloop_with_parameters_it_has
FAILED test_a_retired_parameter_is_gone_from_the_benchmarks_too[brave_api_key]
2 failed, 1 passed

and on the tree as it stands, 3 passed -- it finds all four AgentLoop( call sites under benchmarks/, so pinchbench and appworld are covered too, not just clawbench. The assert seen guard is what keeps it honest if the tree moves.

One thing this does not reach: it matches ast.Name only, so a module.AgentLoop(...) call would slip past. Nothing in benchmarks/ calls it that way today, so it is a note, not a request.

[gloryfromca]

Blocking, but only on the commit message: cc715dc9's body has three em-dashes, and the repo's own lint rejects the whole message as non-ASCII -- the Commit lint workflow will be red. The code in this revision is good and my earlier finding is properly fixed.

The blocker

scripts/commit_lint.py runs _is_ascii over the entire message, not just the header (_check_message does text = message.strip() and checks that before parsing the header), and .github/workflows/commits.yml runs check_commit_messages.py "$BASE..HEAD" across the PR range. Reproduced with the repo's own script:

$ PYTHONPATH=. uv run python scripts/check_commit_messages.py \
    "github/feat/tool_capability_inventory_gh..HEAD"
Invalid commit message cc715dc93be1225caed21e603554b53b33f70a52:
  fix(benchmarks): keep clawbench constructing an AgentLoop that exists
- must be ASCII-only English

The three offenders are all in the body, on the lines starting - `brave_api_key`, - `everos_config` and keywords against the live signature. AGENTS.md 3.1.1 names this case specifically ("not just no full-width punctuation ... but also no em-dash"). 3628c146 is clean; only the new commit fails. Swapping each -- in and rewording is a message rewrite, which per 3.4 needs the maintainer's word first.

Worth noting the squash does not save you here: the squash body comes from the PR description, but commits.yml lints every commit in the range before the merge, so this fails the PR, not just the history.

The code

No blockers. The clawbench fix mirrors the three CLI sites exactly, and dropping everos_config alongside it is right -- that parameter is not on AgentLoop.__init__ either, so clawbench was already dead on the target branch and the rename was only the second of two fatal kwargs. I said the opposite last round; the commit body has it right.

The guard test earns its place and is not vacuous -- details in my reply on the main.py thread. Short version: reverting the kwarg makes both tests fail by name and line, it reaches all four AgentLoop( call sites under benchmarks/ (so pinchbench and appworld are covered too), and assert seen stops it passing over an empty sweep.

What I checked on this delta

• AGENTS.md: commit header is fine (fix(benchmarks) has precedent in history, no scope-enum rule in commitlint.config.cjs, 69 chars, lowercase, no full stop, Co-authored-by present); the body is the problem. tests/test_benchmark_entry_points.py is not a CLI test, so 5.1's test_cli_<module>_commands.py shape does not bind it, and the file itself is ASCII-clean.
• Callers and history: traced agents.defaults.everos -- raven/config/loader.py:184 drops the block as retired on migration, and no CLI site passes a replacement, so nothing is lost by deleting the line rather than porting it.
• Tests, and whether any were weakened: uv run pytest tests/test_benchmark_entry_points.py tests/test_agent_loop_web_tools.py tests/test_tool_capabilities.py tests/test_agent_loop_tool_search.py -q -> 45 passed, 0 skipped. Nothing in the delta touches an existing assertion; it only adds.
• Backward compatibility: unchanged from the last revision -- brave_api_key is gone from the public signature with no shim, and the repo now has zero references to it.

Not blocking

test_a_retired_parameter_is_gone_from_the_benchmarks_too greps for the literal "<name>=", so it would also fire on an unrelated local named everos_config. No such name exists under benchmarks/ today and the failure text would point straight at the file, so this is fine as it stands.