Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions raven/agent/loop/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@
RuntimeConfig,
SkillForgeRouterConfig,
)
from raven.config.schema import ChannelsConfig, DeepResearchToolConfig, ExecToolConfig
from raven.config.schema import (
ChannelsConfig,
DeepResearchToolConfig,
ExecToolConfig,
WebSearchConfig,
)
from raven.context_engine import ContextEngine
from raven.memory_engine.backend import MemoryBackend
from raven.proactive_engine.schedulers.cron.service import CronService
Expand Down Expand Up @@ -292,7 +297,7 @@ def __init__(
model: str | None = None,
max_iterations: int = 40,
context_window_tokens: int | None = None,
brave_api_key: str | None = None,
web_search_config: "WebSearchConfig | None" = None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

web_proxy: str | None = None,
exec_config: ExecToolConfig | None = None,
cron_service: CronService | None = None,
Expand Down Expand Up @@ -402,11 +407,11 @@ def __init__(
self.context_window_tokens = context_window_tokens or effective_context_window(
self.model, None, allow_fetch=False
)
self.brave_api_key = brave_api_key
self.jina_api_key = jina_api_key
self.web_proxy = web_proxy
from raven.config.schema import DeepResearchToolConfig, MediaGenConfig
from raven.config.schema import DeepResearchToolConfig, MediaGenConfig, WebSearchConfig

self.web_search_config = web_search_config or WebSearchConfig()
self.media_config = media_config or MediaGenConfig()
self.deep_research_config = deep_research_config or DeepResearchToolConfig()
self.exec_config = exec_config or ExecToolConfig()
Expand Down Expand Up @@ -534,7 +539,7 @@ def __init__(
provider=provider,
workspace=workspace,
model=self.model,
brave_api_key=brave_api_key,
web_search_config=web_search_config,
jina_api_key=jina_api_key,
web_proxy=web_proxy,
exec_config=self.exec_config,
Expand Down Expand Up @@ -785,9 +790,13 @@ def _register_default_tools(self) -> None:
# call fails, and the error text -- naming a config file and an env var --
# gets relayed to whoever is on the other end of the channel. Ask the tool
# rather than the config, because it resolves the key at call time from
# either source; gating on `brave_api_key` alone would withdraw the tool
# from a deploy that only exports SERPER_API_KEY.
web_search = WebSearchTool(api_key=self.brave_api_key, proxy=self.web_proxy)
# either source; gating on the configured key alone would withdraw the
# tool from a deploy that only exports SERPER_API_KEY.
web_search = WebSearchTool(
api_key=self.web_search_config.api_key or None,
max_results=self.web_search_config.max_results,
proxy=self.web_proxy,
)
if web_search.api_key:
self.tools.register(web_search)
# web_fetch is unconditional by contrast: it works without a key, and the
Expand Down
12 changes: 8 additions & 4 deletions raven/agent/subagent/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from raven.agent.tools.registry import ToolRegistry
from raven.agent.tools.shell import ExecTool
from raven.agent.tools.web import WebFetchTool, WebSearchTool
from raven.config.schema import ExecToolConfig
from raven.config.schema import ExecToolConfig, WebSearchConfig
from raven.providers.base import LLMProvider
from raven.sandbox import SandboxConfig, build_executor
from raven.security.trust import wrap_untrusted
Expand All @@ -38,7 +38,7 @@ def __init__(
provider: LLMProvider,
workspace: Path,
model: str | None = None,
brave_api_key: str | None = None,
web_search_config: "WebSearchConfig | None" = None,
web_proxy: str | None = None,
exec_config: "ExecToolConfig | None" = None,
restrict_to_workspace: bool = False,
Expand All @@ -59,7 +59,7 @@ def __init__(
# SUBAGENT-origin turn.
self._submit = None
self.model = model or provider.get_default_model()
self.brave_api_key = brave_api_key
self.web_search_config = web_search_config or WebSearchConfig()
self.jina_api_key = jina_api_key
self.web_proxy = web_proxy
self.exec_config = exec_config or ExecToolConfig()
Expand Down Expand Up @@ -199,7 +199,11 @@ async def _run_subagent_inner(
# Withheld without a key, same as the main loop: a sub-agent that
# reaches for a search it cannot run reports the failure to its
# caller, and that text ends up in the parent turn.
web_search = WebSearchTool(api_key=self.brave_api_key, proxy=self.web_proxy)
web_search = WebSearchTool(
api_key=self.web_search_config.api_key or None,
max_results=self.web_search_config.max_results,
proxy=self.web_proxy,
)
if web_search.api_key:
tools.register(web_search)
tools.register(WebFetchTool(api_key=self.jina_api_key, proxy=self.web_proxy))
Expand Down
2 changes: 1 addition & 1 deletion raven/cli/agent_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ def agent(
context_window_tokens=config.agents.defaults.context_window_tokens,
max_concurrent_subagents=config.agents.defaults.max_concurrent_subagents,
max_subagent_spawns_per_hour=config.agents.defaults.max_subagent_spawns_per_hour,
brave_api_key=config.tools.web.search.api_key or None,
web_search_config=config.tools.web.search,
jina_api_key=config.tools.web.jina_api_key or None,
web_proxy=config.tools.web.proxy or None,
media_config=config.effective_media_config(),
Expand Down
2 changes: 1 addition & 1 deletion raven/cli/gateway_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def gateway(
context_window_tokens=config.agents.defaults.context_window_tokens,
max_concurrent_subagents=config.agents.defaults.max_concurrent_subagents,
max_subagent_spawns_per_hour=config.agents.defaults.max_subagent_spawns_per_hour,
brave_api_key=config.tools.web.search.api_key or None,
web_search_config=config.tools.web.search,
jina_api_key=config.tools.web.jina_api_key or None,
web_proxy=config.tools.web.proxy or None,
media_config=config.effective_media_config(),
Expand Down
2 changes: 1 addition & 1 deletion raven/cli/tui_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ def _build_tui_agent_loop():
context_window_tokens=config.agents.defaults.context_window_tokens,
max_concurrent_subagents=config.agents.defaults.max_concurrent_subagents,
max_subagent_spawns_per_hour=config.agents.defaults.max_subagent_spawns_per_hour,
brave_api_key=config.tools.web.search.api_key or None,
web_search_config=config.tools.web.search,
web_proxy=config.tools.web.proxy or None,
media_config=config.effective_media_config(),
deep_research_config=config.tools.deep_research,
Expand Down
4 changes: 2 additions & 2 deletions tests/test_agent_loop_tool_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import pytest

from raven.agent.loop import AgentLoop
from raven.config.schema import ToolSearchConfig
from raven.config.schema import ToolSearchConfig, WebSearchConfig
from raven.providers.base import LLMProvider, LLMResponse
from raven.token_wise.base import TokenStrategy
from raven.token_wise.registry import StrategyRegistry
Expand Down Expand Up @@ -64,7 +64,7 @@ def _make_loop(workspace: Path, cfg, strategies=None) -> AgentLoop:
# web_search is the cataloged domain tool these tests fold away, and the
# loop only registers it when a search key resolves. Supplying one keeps
# the subject of the test present for the right reason.
brave_api_key="test-serper-key",
web_search_config=WebSearchConfig(api_key="test-serper-key"),
)


Expand Down
76 changes: 53 additions & 23 deletions tests/test_agent_loop_web_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from raven.agent.subagent.manager import SubagentManager
from raven.agent.tools.registry import ToolRegistry
from raven.agent.tools.web import WebSearchTool
from raven.config.schema import WebSearchConfig
from raven.providers.base import LLMProvider, LLMResponse


Expand Down Expand Up @@ -75,11 +76,24 @@ def test_web_search_is_withheld_without_a_key(workspace) -> None:


def test_a_configured_key_registers_web_search(workspace) -> None:
loop = _loop(workspace, brave_api_key="sk-serper")
loop = _loop(workspace, web_search_config=WebSearchConfig(api_key="sk-serper"))

assert loop.tools.has("web_search")


def test_the_configured_result_count_reaches_the_tool(workspace) -> None:
"""``tools.web.search.maxResults`` was declared and never wired.

Both registration sites passed the key alone, so the tool fell back to its
own default and a deployer who set the field got no effect and no warning.
Passing the section rather than one field out of it is what closes that, and
this is the assertion that keeps it closed.
"""
loop = _loop(workspace, web_search_config=WebSearchConfig(api_key="sk-serper", max_results=9))

assert loop.tools.get("web_search").max_results == 9


def test_the_env_var_alone_registers_web_search(workspace, monkeypatch: pytest.MonkeyPatch) -> None:
# The tool resolves its key at call time from the config value *or*
# SERPER_API_KEY, so a gate that reads only the config would withdraw the
Expand All @@ -91,39 +105,46 @@ def test_the_env_var_alone_registers_web_search(workspace, monkeypatch: pytest.M
assert loop.tools.has("web_search")


def test_the_subagent_surface_applies_the_same_rule(workspace, monkeypatch: pytest.MonkeyPatch) -> None:
# A sub-agent reaching for a search it cannot run reports the failure to its
# caller, and that text lands in the parent turn.
#
# The manager builds its registry inside the run and keeps no reference, so
# the names are observed as they are registered. The collector opens before
# the manager is constructed and drops nothing on the floor: were a
# registration ever to happen outside a window, it would land in the previous
# run's list and be caught, rather than vanishing and leaving an assertion
# that passes over an empty list.
registered: list[list[str]] = []
@pytest.fixture
def subagent_run(workspace, monkeypatch: pytest.MonkeyPatch):
"""Run a sub-agent and hand back the tools it registered.

The manager builds its registry inside the run and keeps no reference, so
the tools are observed as they are registered. The collector opens before
the manager is constructed and drops nothing on the floor: were a
registration ever to happen outside a window, it would land in the previous
run's list and be caught, rather than vanishing and leaving an assertion
that passes over an empty list.
"""
import asyncio

runs: list[list] = []
real = ToolRegistry.register

def _spy(self, tool): # noqa: ANN001, ANN202
real(self, tool)
assert registered, f"{tool.name} was registered outside a collection window"
registered[-1].append(tool.name)
assert runs, f"{tool.name} was registered outside a collection window"
runs[-1].append(tool)

monkeypatch.setattr(ToolRegistry, "register", _spy)
# The run announces its result through the spine, which is not wired here and
# is not what this is about.
# The run announces its result through the spine, which is not wired here
# and is not what these are about.
monkeypatch.setattr(SubagentManager, "_announce_result", _noop)

async def _names(**kw) -> list[str]:
registered.append([])
def run(**kw):
runs.append([])
manager = SubagentManager(provider=_StubProvider(), workspace=workspace, model="stub", **kw)
await manager._run_subagent_inner("t1", "task", "label", {}, None, manager.provider, manager.model)
return registered[-1]
asyncio.run(manager._run_subagent_inner("t1", "task", "label", {}, None, manager.provider, manager.model))
return runs[-1]

import asyncio
return run

without = asyncio.run(_names())
with_key = asyncio.run(_names(brave_api_key="sk-serper"))

def test_the_subagent_surface_applies_the_same_rule(subagent_run) -> None:
# A sub-agent reaching for a search it cannot run reports the failure to its
# caller, and that text lands in the parent turn.
without = [t.name for t in subagent_run()]
with_key = [t.name for t in subagent_run(web_search_config=WebSearchConfig(api_key="sk-serper"))]

# Baselines first: an empty list would satisfy the "not in" assertion below
# without proving anything about the gate.
Expand All @@ -133,6 +154,15 @@ async def _names(**kw) -> list[str]:
assert "web_search" in with_key


def test_the_subagent_surface_gets_the_configured_result_count_too(subagent_run) -> None:
"""The second registration site had the same unwired field, and a fix
applied to one of two call sites is the shape this whole area keeps taking."""
tools = subagent_run(web_search_config=WebSearchConfig(api_key="sk-serper", max_results=7))

web_search = next(t for t in tools if t.name == "web_search")
assert web_search.max_results == 7


@pytest.mark.asyncio
async def test_the_unconfigured_error_names_the_config_actually_in_force(tmp_path: Path) -> None:
"""Reachable only if the key disappears after registration, but the message
Expand Down
4 changes: 2 additions & 2 deletions tests/test_tool_capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def test_the_table_names_exactly_the_credential_gated_tools(workspace, tmp_path:
for attr in _media_attrs():
getattr(full.tools.media, attr).model = "some/model"
full.providers.openrouter.api_key = "sk-or-test"
loop_full = _loop(workspace, full, brave_api_key="sk-serper")
loop_full = _loop(workspace, full, web_search_config=full.tools.web.search)

gated = set(loop_full.tools.tool_names) - set(loop_bare.tools.tool_names)
declared = {c.tool for c in CAPABILITIES if c.need is not Need.NOTHING}
Expand All @@ -136,7 +136,7 @@ def test_the_table_agrees_with_the_loop_when_unconfigured(cap, workspace, tmp_pa
def test_a_configured_search_key_agrees_on_both_sides(workspace, tmp_path: Path) -> None:
config = _config(tmp_path)
config.tools.web.search.api_key = "sk-serper"
loop = _loop(workspace, config, brave_api_key="sk-serper")
loop = _loop(workspace, config, web_search_config=config.tools.web.search)

cap = next(c for c in CAPABILITIES if c.tool == "web_search")
assert is_configured(cap, config) and loop.tools.has("web_search")
Expand Down
Loading