Skip to content

fix(*): apply the settings that were saved and then ignored - #548

Merged
gloryfromca merged 4 commits into
refactor/ui_web_architecturefrom
fix/settings_saved_and_ignored
Sep 20, 2026
Merged

gloryfromca merged 4 commits into
refactor/ui_web_architecturefrom
fix/settings_saved_and_ignored

Conversation

@gloryfromca

Copy link
Copy Markdown
Member

Summary

#543 removed the reload-only set, which named eight keys. That set was not the
whole list. Several more settings are copied onto the objects the loop is built
with and never read again -- and those never said anything at all: the surface
answered Saved. and the process kept the value it started on. This is the rest
of the list, found by walking what build_runtime consumes and asking of each
writable key whether anything reads it again.

agents.defaults.reasoningEffort is the one with a control on the settings
page
(providers/Roles.tsx writes it). It rides every model call of a turn,
and the provider's copy is frozen when the provider is constructed -- on
purpose: the comment on ResolvingProvider explains that a credentials refresh
must not import a live agents section, or a dormant routing edit takes effect
at a key-rotation boundary. So the loop sends the effort explicitly instead,
underneath the session's pin and a hook's override, which is the path a
session's own effort already travels. The provider keeps its default for
everyone who sends nothing.

tools.deepResearch.apiKey was half done already. _maybe_promote_deep_research
runs once per turn and swaps the offer stand-in for the working tool when a key
first appears. Replacing one was not covered: the working tool holds the section
it was built with, so a rotation left every call on the old credential until a
restart. It now rebuilds when the section changed and returns when it did not --
the skip matters, because a rebuild per turn would throw away the manager a
channel's async delivery depends on.

The rest are read where they are used: agents.defaults.enablePersonalization,
memory.memoryTopK, tools.exec.timeout, and the two tools.web vendors. The
vendor pair is worth naming: the keys beside them have been read live since they
landed, so the vendor was the half that still owed a restart.

All of them answer from the turn's hold (hold_for_this_turn, added in #543), so
one turn reads each once and every repeat costs a dict lookup instead of a file
read -- measured on a 2 KB config: 14.8 us against 0.05 us. Outside a turn every
read is live, which is what a caller below run_turn and the background paths
get.

Not in this change, and why. agents.defaults.temperature rides the same
frozen GenerationSettings, but making it live means sending an explicit
temperature on every call, and some reasoning models refuse one -- a real risk
for a key no surface renders. The three llm timeouts in that object are not
writable from any surface at all, so a restart is their honest semantics rather
than a lie. The same is true of the things that would need a stack rebuild
(memory backend, sandbox backend, workspace root): none is reachable from a
settings write, so there is no "saved and nothing happened" to fix.

One property a reviewer should weigh. The hold is a ContextVar, so work
detached during a turn copies it: a sub-agent spawned inside a turn keeps that
turn's values for its whole life, past the parent turn's end. Verified by
experiment. That is the semantics the model binding already has and for the
stated reason -- a sub-agent spawned before a switch must not finish on the
model chosen after it -- so a long-running sub-agent takes an edited exec
timeout or vendor at its next spawn, not mid-run.

Type

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

Verification

  • uv run pytest tests/ -q: 23375 passed, 4 failed, 111 skipped. All four
    failures reproduce on a clean checkout of the base commit in the same
    environment (test_agent_loop_token_budget, the two test_agents_*_launcher
    tool-face cases, test_ppt_engine_image_search); the launcher pair looks
    environment-dependent (a SERPER_API_KEY in the developer environment makes
    web_search available).

  • After the cherry-pick onto the current base: the sweep over
    config-live/auth/deep-research/subagent/sandbox/shell/context-engine/memory/
    usage-sink/loop/settings/pool/console, 4324 passed, 1 failed (the pre-existing
    token-budget case).

  • npm run type-check clean; npx vitest run 186 files, 2544 tests passed.

  • uv run ruff check and ruff format --check over the full lint target set:
    clean.

  • Ten new cases, each mutation-verified on its own: reading one spelling of the
    effort, resolving the effort lazily instead of at the boundary, ignoring the
    file for personalization, keeping the built recall depth, keeping the built
    vendors, sending a call without the configured effort, ignoring a rotated
    deep-research section, rebuilding on an unchanged one, keeping the built exec
    ceiling, and dropping the hold so a turn straddles an edit.

  • The cost figures above are measured, not estimated: a micro-benchmark over a
    2 KB config, 20k reads held and 5k live.

  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • User-facing docs or screenshots are updated when needed

Risk

User-visible: a reasoning-effort change on the settings page reaches the next
turn; a rotated deep-research section reaches the next turn; the four
RPC-writable settings reach their next use. A turn already running is never
moved, and neither is a sub-agent already detached.

ExecTool.timeout becomes a property over the value the tool was built with.
Nothing assigns to it; the constructor argument is unchanged and still answers
when the file has no opinion.

The deep-research rebuild is the one place that does work rather than answer a
question: it unregisters and re-registers a tool. Guarded on the section having
actually changed, so an unchanged config costs one comparison per turn.

Rollback: revert the commit. Every reader falls back to the value its object was
constructed with, which is what it used exclusively before.

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

Related Issues

N/A

The reload-only set named eight keys. It was not the whole list: several more
were copied onto the objects the loop was built with and never read again, and
those said nothing at all -- the surface answered "Saved." and the process kept
the value it started on. This is the rest of that list.

agents.defaults.reasoningEffort is the one with a control on the settings page.
It rides every model call, and the provider's copy of it is frozen when the
provider is constructed -- deliberately, so a credentials refresh cannot import
a live agents section (see ResolvingProvider). So the loop sends it explicitly
instead, under the session's pin and a hook's override, the way a session's own
effort already travels.

agents.defaults.enablePersonalization, memory.memoryTopK, tools.exec.timeout and
the two tools.web vendors are read where they are used. The web keys beside the
vendors have been read live since they landed, so the vendor was the half of the
pair that still owed a restart.

tools.deepResearch.apiKey was already picked up on the next turn when a key first
appeared. Replacing one was not: the working tool holds the section it was built
with, so a rotation left every call on the old credential. The promotion path now
rebuilds on any change to the section, and skips when nothing moved -- it runs
once per turn and a rebuild would throw away the manager a channel's async
delivery depends on.

All of them answer from the turn's hold, so a turn reads each once and the repeat
reads cost a dict lookup rather than a file read (measured: 14.8us against
0.05us). Outside a turn every read is live, as before.

Not here: agents.defaults.temperature rides the same frozen GenerationSettings,
but making it live means sending an explicit temperature on every call and some
reasoning models refuse one -- for a key no surface renders. The three llm
timeouts in that object are not writable from any surface at all, so a restart is
their honest semantics.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Blocking: live web-provider changes and deep-research credential removal must take effect in the running loop.

I found two reachable settings writes that still leave the process on the old value; both are marked inline.

Covered: the full target-to-head diff, AGENTS.md plus the Runtime/Web UI context contracts, changed callers and consumers, the four-commit history, backward compatibility, and whether tests were weakened. The new tests add coverage but omit the two opposite-direction transitions reproduced inline.

Verification:

  • Targeted runtime/RPC sweep: 513 passed.
  • uv run --all-extras pytest tests/test_rpc_settings.py -q: 87 passed.
  • Web UI: 186 files / 2,544 tests passed; TypeScript type-check passed.
  • Source-language gate and git diff --check: passed.

Comment thread raven/config/live.py
Comment thread raven/agent/loop/main.py
@gloryfromca
gloryfromca requested a review from LivXue as a code owner September 20, 2026 09:53
Review found two places the previous commit left in the state it was meant to
remove.

The web vendor was only made live for a sub-agent spawn. The main loop had
already registered WebSearchTool, ImageSearchTool and WebFetchTool with the
vendor it booted on, so a vendor changed on the settings page moved nothing --
and because the live key readers resolve against that selection, the new
vendor's key never reached anything either. The three tools now take the vendor
the way they already take the key, as a reader; an unknown name in the file
falls back to the vendor the tool was registered with rather than failing a call
mid-turn. WebFetchTool's Jina substitution moves with it: decided once at
registration it outlived the key that would have stopped it, so it is asked per
call and says so once. A spawn also hands down the keys the file has now, since
the vendor it hands down is read live and half a pair is no better than none.

Clearing the deep-research key was the other. The settings Clear action writes an
empty key, and the unconfigured check returned before anything changed, so the
working tool stayed registered on the credential that had just been removed and
kept spending against it until a restart. The real-to-offer transition is handled
now, alongside offer-to-real and real-to-real.

Both branches key off identity rather than the tool's name: a plugin that shadows
deep_research, or a test double that replaced it, owns that entry and none of
this is about it.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Blocking: saved key clearing must revoke subagent credentials, and the deep-research offer swap must retain its broker.

The two original failures are fixed on this revision, and those threads are resolved. Two transition regressions remain inline.

Coverage: repository rules (AGENTS.md, CONTEXT-MAP.md, the runtime CONTEXT.md, and ui-web/CONTEXT.md); the full target diff and the new-commit delta; affected callers and host wiring; history and backward compatibility; test changes (additive, with no weakened tests found); and the runtime architecture constraints.

Verification: uv run --all-extras pytest tests/test_config_live.py tests/test_deep_research_tool.py tests/test_agent_loop_usage_sink.py tests/test_agent_loop_web_tools.py tests/test_agent_tools_web_providers.py tests/test_rpc_settings.py -q passed 320 tests with 17 existing pytest-marker warnings; uv run --all-extras pytest tests/test_agent_loop_run_emit.py tests/test_rpc_bootstrap.py -q passed 66 tests; git diff --check github/refactor/ui_web_architecture...HEAD passed. A focused clear-key reproduction returned {'exa': 'exa-old'} from _web_provider_keys_now() after the saved Exa key was emptied.

Comment thread raven/agent/subagent/manager.py
Comment thread raven/agent/loop/main.py Outdated
Both findings from the second read are the same mistake in two places: a
revocation read as a miss, and so answered with the value being revoked.

The plural web-key reader dropped empty values, and the spawn merged what was
left over the manager's boot set -- so clearing a vendor's key on the settings
page left the next sub-agent spending the credential that had just been removed.
The singular reader already draws this distinction and says so: a present but
empty slot is a revocation. The plural one keeps the empty now, and only a
vendor the file says nothing about falls back to the boot value.

The offer stand-in registered when a deep-research credential is cleared had no
broker. set_deep_research_broker applies the broker to whatever is registered
when the host calls it, and a stand-in built later is past that call, so
_ask_search_mode answered None and the offer degraded to regular search silently
until a restart. Both registration sites go through one helper now, which hands
over the stored broker.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No blockers; this can merge as far as I am concerned.

The two prior blockers are fixed and their threads are resolved. The cleared-key path now preserves an explicit empty value so it revokes the boot credential, and the deep-research offer replacement now inherits the host's stored broker.

Coverage: the full target diff and the 826291dfccb4..b152bec42f73 delta; repository rules (AGENTS.md, CONTEXT-MAP.md, runtime CONTEXT.md, and ui-web/CONTEXT.md); affected callers and host wiring; history and backward compatibility; architecture constraints; and test changes, with no weakened tests found.

Verification: uv run --all-extras pytest tests/test_config_live.py tests/test_deep_research_tool.py tests/test_agent_loop_usage_sink.py tests/test_agent_loop_web_tools.py tests/test_agent_tools_web_providers.py tests/test_rpc_settings.py tests/test_agent_loop_run_emit.py tests/test_rpc_bootstrap.py -q passed 387 tests with 17 existing pytest-marker warnings. git diff --check github/refactor/ui_web_architecture...HEAD also passed.

@gloryfromca
gloryfromca merged commit b63f350 into refactor/ui_web_architecture Sep 20, 2026
29 checks passed
@gloryfromca
gloryfromca deleted the fix/settings_saved_and_ignored branch September 20, 2026 11:23
gloryfromca added a commit that referenced this pull request Sep 21, 2026
## Summary

#543 removed the reload-only set, which named eight keys. That set was
not the
whole list. Several more settings are copied onto the objects the loop
is built
with and never read again -- and those never said anything at all: the
surface
answered `Saved.` and the process kept the value it started on. This is
the rest
of the list, found by walking what `build_runtime` consumes and asking
of each
writable key whether anything reads it again.

**`agents.defaults.reasoningEffort` is the one with a control on the
settings
page** (`providers/Roles.tsx` writes it). It rides every model call of a
turn,
and the provider's copy is frozen when the provider is constructed -- on
purpose: the comment on `ResolvingProvider` explains that a credentials
refresh
must not import a live `agents` section, or a dormant routing edit takes
effect
at a key-rotation boundary. So the loop sends the effort explicitly
instead,
underneath the session's pin and a hook's override, which is the path a
session's own effort already travels. The provider keeps its default for
everyone who sends nothing.

**`tools.deepResearch.apiKey` was half done already.**
`_maybe_promote_deep_research`
runs once per turn and swaps the offer stand-in for the working tool
when a key
first appears. Replacing one was not covered: the working tool holds the
section
it was built with, so a rotation left every call on the old credential
until a
restart. It now rebuilds when the section changed and returns when it
did not --
the skip matters, because a rebuild per turn would throw away the
manager a
channel's async delivery depends on.

**The rest are read where they are used**:
`agents.defaults.enablePersonalization`,
`memory.memoryTopK`, `tools.exec.timeout`, and the two `tools.web`
vendors. The
vendor pair is worth naming: the keys beside them have been read live
since they
landed, so the vendor was the half that still owed a restart.

All of them answer from the turn's hold (`hold_for_this_turn`, added in
#543), so
one turn reads each once and every repeat costs a dict lookup instead of
a file
read -- measured on a 2 KB config: 14.8 us against 0.05 us. Outside a
turn every
read is live, which is what a caller below `run_turn` and the background
paths
get.

**Not in this change, and why.** `agents.defaults.temperature` rides the
same
frozen `GenerationSettings`, but making it live means sending an
explicit
temperature on every call, and some reasoning models refuse one -- a
real risk
for a key no surface renders. The three llm timeouts in that object are
not
writable from any surface at all, so a restart is their honest semantics
rather
than a lie. The same is true of the things that would need a stack
rebuild
(memory backend, sandbox backend, workspace root): none is reachable
from a
settings write, so there is no "saved and nothing happened" to fix.

**One property a reviewer should weigh.** The hold is a ContextVar, so
work
detached during a turn copies it: a sub-agent spawned inside a turn
keeps that
turn's values for its whole life, past the parent turn's end. Verified
by
experiment. That is the semantics the model binding already has and for
the
stated reason -- a sub-agent spawned before a switch must not finish on
the
model chosen after it -- so a long-running sub-agent takes an edited
exec
timeout or vendor at its next spawn, not mid-run.

## Type

- [x] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other

## Verification

- `uv run pytest tests/ -q`: 23375 passed, 4 failed, 111 skipped. All
four
  failures reproduce on a clean checkout of the base commit in the same
environment (`test_agent_loop_token_budget`, the two
`test_agents_*_launcher`
tool-face cases, `test_ppt_engine_image_search`); the launcher pair
looks
environment-dependent (a `SERPER_API_KEY` in the developer environment
makes
  `web_search` available).
- After the cherry-pick onto the current base: the sweep over

config-live/auth/deep-research/subagent/sandbox/shell/context-engine/memory/
usage-sink/loop/settings/pool/console, 4324 passed, 1 failed (the
pre-existing
  token-budget case).
- `npm run type-check` clean; `npx vitest run` 186 files, 2544 tests
passed.
- `uv run ruff check` and `ruff format --check` over the full lint
target set:
  clean.
- Ten new cases, each mutation-verified on its own: reading one spelling
of the
effort, resolving the effort lazily instead of at the boundary, ignoring
the
file for personalization, keeping the built recall depth, keeping the
built
vendors, sending a call without the configured effort, ignoring a
rotated
deep-research section, rebuilding on an unchanged one, keeping the built
exec
  ceiling, and dropping the hold so a turn straddles an edit.
- The cost figures above are measured, not estimated: a micro-benchmark
over a
  2 KB config, 20k reads held and 5k live.

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

## Risk

User-visible: a reasoning-effort change on the settings page reaches the
next
turn; a rotated deep-research section reaches the next turn; the four
RPC-writable settings reach their next use. A turn already running is
never
moved, and neither is a sub-agent already detached.

`ExecTool.timeout` becomes a property over the value the tool was built
with.
Nothing assigns to it; the constructor argument is unchanged and still
answers
when the file has no opinion.

The deep-research rebuild is the one place that does work rather than
answer a
question: it unregisters and re-registers a tool. Guarded on the section
having
actually changed, so an unchanged config costs one comparison per turn.

Rollback: revert the commit. Every reader falls back to the value its
object was
constructed with, which is what it used exclusively before.

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

## Related Issues

N/A

---------

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
0xKT pushed a commit that referenced this pull request Sep 22, 2026
## Summary

A 401 or 402 from a reader, or a 401, 402 or 403 from a search vendor,
is about the key or the account, not the page or the query, so every
call after it meets the same answer. The web tools kept asking: on
2026-09-20 Jina answered HTTP 402 on every `web_fetch` of a session, the
tool returned one identical error envelope per call, and the model went
on calling it.

`web_fetch`, `web_search` and `image_search` now remember a refusal per
tool instance (`_VendorRefusal` in `raven/agent/tools/web.py`) and
answer later calls without a request:

- the `error` string is the same for the refusing call and every paused
call (`<Vendor> refused the key (HTTP 402)`), so `failure_class` reads
them as one cause and the loop's stop-repeating nudge fires;
`is_hard_tool_failure` reads the envelope as deterministic (none of the
transient markers appear in the text);
- the `detail` says what the status means (401 rejected key, 402 out of
credit, 403 not allowed), that this call was not sent, and what the user
has to do: the vendor's `tools.web.providers.<vendor>.apiKey` slot, its
env var, its sign-up page, or another vendor under
`tools.web.<search|fetch>.provider`. The `Tell the user:` framing is
deliberate: a model cannot fix a key or a bill, and the incident's model
retried instead of saying so;
- the pause is per tool rather than per session because the key is
shared by every session of the process, so one session's refusal is
every session's; per-session marking would send each session into the
same 402 once;
- all three tools read their key live, in both loops: `WebFetchTool`
takes a callable key source the way `WebSearchTool` and
`ImageSearchTool` do (main gave it the same source in #548 while this PR
was open; the rebase keeps main's version), the main loop hands each of
them one through `_live_vendor_key`, and the sub-agent lane through one
`live_key` helper over the same resolver on its module `LiveConfig` (it
used to hand `web_search` and `image_search` the key it was constructed
with, while their envelope told the user the slot is read on the next
call). The vendor is live too since #543: the main loop hands the tools
a callable `provider`, and the lane runs on the vendor main reads at
spawn. That resolver, `live_vendor_key` in `raven/config/live.py`, is
one function the main loop's two live readers now delegate to, and it
follows `WebToolsConfig.vendor_key` in full: the slot, then the
pre-vendor leaf that is Serper's (`tools.web.search.apiKey`) or Jina's
(`tools.web.jinaApiKey`, through a new `web_jina_key` reader with the
same last-good memory as Serper's), then a revocation only where the
file names the vendor with an empty key, then the boot value where the
file says nothing about the vendor. Review measured the earlier shape
losing a paid Jina key the moment any other vendor's key was set (the
schema-backed slot reader answers an empty key for a vendor the subtree
does not name, and that empty was read as a revocation before the leaf
or the boot value), and a harness-supplied key the same way; both rows
now answer what the canonical resolver answers. A key set at
`tools.web.providers.<vendor>.apiKey` therefore reaches the next call
and lifts the pause without a restart, which is what the envelope tells
the user to do; otherwise one real request goes through after
`VENDOR_REFUSAL_PAUSE_S` (10 minutes) and re-arms the pause if refused
again. The envelope's advice names what each route needs: the config
slot and the vendor selection are read from the config file without a
restart (per call in the main loop, at spawn for a sub-agent), the env
var needs a restart;
- each call resolves its (vendor, key) pair once, at its start, through
a `_resolve()` on each tool -- and for `web_fetch` that one read also
decides the Jina substitution, so a keyed reader whose key is cleared in
the file (the edit the refusal text points at) reads through Jina,
keyless, from the next call instead of sending an empty credential that
could arm no pause
(`test_a_keyed_reader_whose_key_is_cleared_falls_back_to_jina_per_call`,
and the same through the main loop's wiring) -- and that pair is what
the request carries, what the envelope names and what a refusal is
recorded against; the request builders and normalisers take the vendor
and the key as parameters and read the live properties no more. Read
again on the way, a key the user replaced while a request was in flight
was recorded as refused without ever having been sent, and the next call
was answered from the pause (in the main loop the tool serves every
session of the process, so all of them lost it for ten minutes); and
with the vendor live, an envelope could name a backend the request never
reached, since `WebFetchTool.provider` decides the Jina substitution per
access. The pause is keyed on the pair, so another vendor given the same
key string is a new request;
- which statuses count differs by tool, because what a status can mean
differs by endpoint. A search vendor's API is the endpoint, so 401, 402
and 403 all speak of the account: Serper answers a rejected key with 403
(measured against the live endpoint). A reader's 403 speaks of the URL:
Jina answers an anonymous request for a domain it has blocked with 403
for every URL under it (measured:
`r.jina.ai/https://httpbin.org/status/200` answers 403 with Jina's own
`AbuseAlleviationError` envelope, "Anonymous access to domain
httpbin.org blocked ... due to previous abuse"), and Firecrawl answers
403 for a site its policy does not scrape ("This website is no longer
supported"). So `SEARCH_REFUSAL_STATUSES` is `{401, 402, 403}` and
`FETCH_REFUSAL_STATUSES` is `{401, 402}`; a reader's 403 is reported per
URL exactly as before this PR;
- a request that carried no key is never paused, whatever the status,
because there was no key to refuse. That is the default configuration,
Jina without a key (`needs_key=False`), where review found that one
blocked domain paused `web_fetch` for every URL for ten minutes and told
the user to replace a key that does not exist;
- statuses outside a tool's set (500, 429, a reader's 403, ...) are
reported exactly as before and pause nothing.

Not changed: which vendor is configured. The incident's Jina account is
out of credit; whether to top it up or select Tavily/Exa under
`tools.web.fetch.provider` is a configuration decision this PR only
makes visible. Tavily reports an exhausted plan with its own 432 and 433
rather than 402; those stay outside the set and are reported per call.
The research sub-agent's web tools are the fork's own classes
(`agents/raven-research/.../research_flow/tools/web.py`, which imports
only `raven.contracts.tool` and `raven.security.network`): they already
classify these statuses as `quota_err`, carry no `_VendorRefusal`, and
are untouched, so the fork has no pause to lift and gets none of this
PR's behaviour. An earlier revision of this description said its pause
lifts on the plugin's rebuild; there is no such pause.

## Type

- [ ] Fix
- [x] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [ ] Other

## Verification

At head 40893f7, eight commits on origin/main at a40e911 (rebased once
more for the changelog anchor; main's commits since d50a13c touch no
file this PR touches). Main merged #543 and #548 while this PR was open
(a callable `provider` on the three tools, the Jina substitution decided
per call, a callable key source on `WebFetchTool`, live vendors and keys
handed by the gateway loop); `git range-diff` against 3822234: the four
earlier commits replayed with main's version taken where the two had
written the same thing (the `WebFetchTool` key source, the gateway's
`web_fetch` registration) and no other hunk changed, the fifth commit
adapts the pause to the live vendor, the sixth makes the live key reader
follow the canonical order (the Jina leaf, and the boot value for a
vendor the file says nothing about), the seventh pins a leaf rotated in
the file, and the eighth is `ruff format` on one test file:

```
uv run --frozen --python 3.12 --all-extras pytest -q -n 0 -p no:randomly tests/test_agent_tools_web_providers.py tests/test_agent_loop_web_tools.py tests/test_config_live.py tests/test_security_web_ssrf.py
  235 passed (17 pre-existing PytestWarnings: asyncio marks on sync tests in that file; the four files, test_config_live.py included since main's live-vendor tests live there)
COLORTERM=truecolor uv run --frozen --python 3.12 --all-extras pytest -q      (full suite, in its own worktree at this head)
  24717 passed, 110 skipped in 260.48s (0:04:20)
make lint-python lint-imports lint-deps lint-types                            # all pass
PYTHONPATH=. uv run --frozen --python 3.12 --extra dev python scripts/check_source_language.py origin/main...HEAD   # exit 0
make check-commits; PR_TITLE=... make check-pr-title; COMMIT_RANGE=origin/main...HEAD make check-large-files       # exit 0
git diff --check origin/main...HEAD; git merge-tree --write-tree HEAD origin/main                                   # clean
```

The reviewer's probe, run through the real `WebFetchTool` (Jina, no key;
a transport answering 403 for one host and a page for the others), at
the previous head and at this one:

```
previous head 067f623 (web.py swapped in):
  PAUSED https://blocked.example/a   Jina Reader refused the key (HTTP 403)
  PAUSED https://example.org/b       Jina Reader refused the key (HTTP 403)
  PAUSED https://example.net/c       Jina Reader refused the key (HTTP 403)
  requests sent: 1
this head:
  error  https://blocked.example/a   Jina Reader answered HTTP 403
  ok     https://example.org/b       len=4
  ok     https://example.net/c       len=4
  requests sent: 3
```

Revert-to-red: `raven/agent/tools/web.py` reverted to `origin/main`, or
to the previous head, with the tests kept fails at collection
(`AttributeError: module 'raven.agent.tools.web' has no attribute
'FETCH_REFUSAL_STATUSES'`, exit 2).

Mutation, third round, each mutant applied alone with the file restored
between runs (the two web test files, 152 tests at this head, `-n 0`; a
mutant that changes a status set also changes how many cases the
parametrised tests collect, so the rows give the failures):

```
note() records a refusal from a request that carried no key    2 failed  (keyless reader, 401 and 402; its 403 case is held by the fetch set alone)
FETCH_REFUSAL_STATUSES keeps 403                                5 failed  (a reader's 403, all five readers)
FETCH_REFUSAL_STATUSES drops 402                                3 failed  (the 402 pause tests, including the loop-level key rotation)
SEARCH_REFUSAL_STATUSES drops 403                               1 failed  (image_search on Serper's 403)
web_fetch built on the search set                               5 failed
control (nothing mutated)                                     152 passed
```

Fourth round, one key read per call and the sub-agent lane's two tools
(the two web test files, 155 tests, `-n 0`):

```
baseline                                                   155 passed
MUTANT  web_fetch's note() re-reads the key                1 failed   (the in-flight case for web_fetch)
MUTANT  web_search's note() re-reads the key               1 failed   (for web_search)
MUTANT  image_search's note() re-reads the key             1 failed   (for image_search)
MUTANT  web_fetch's request re-reads the key               1 failed   (the request carries a key the pause was not recorded against)
MUTANT  sub-agent lane: web_search on a boot key           1 failed   (the lane test)
MUTANT  sub-agent lane: image_search on a boot key         1 failed   (the lane test)
MUTANT  vendor switch loses its restart qualifier          34 failed  (every pause test, both tools)
REVERT  web.py + raven_loop.py to reviewed head 428cbe6   38 failed  (the three in-flight cases, the lane test, the 34 wording cases)
CONTROL restored                                           155 passed
```

Fifth round, the live vendor (the two web test files plus
`test_config_live.py`, 219 tests, `-n 0`):

```
baseline                                                                   219 passed
MUTANT  the pause ignores the vendor                                       1 failed
MUTANT  web_fetch resolves the pair twice (key read again for the request) 1 failed
MUTANT  web_fetch's envelope names the vendor read again                   1 failed
MUTANT  the vendor switch loses its live qualifier                         34 failed
REVERT  web.py to the rebased head 9c788e0 (before the adaptation)        36 failed
CONTROL restored                                                           219 passed
```

Sixth round, the live key reader (the two web test files plus
`test_config_live.py`, 232 tests, `-n 0`):

```
baseline                                                                     232 passed
MUTANT  the jina leaf is not consulted (serper's alone, as reviewed)         2 failed
MUTANT  an empty slot is a revocation whether or not the file names the vendor 1 failed
MUTANT  the leaf reader forgets its last admitted key on a rejected edit     1 failed
MUTANT  web_fetch runs on the selected vendor even when its key is gone      3 failed
REVERT  live.py + schema.py to the reviewed head dbc8ffe                    5 failed
CONTROL restored                                                             232 passed
```

The reviewer's measurement of the key reader, `boot` from
`WebToolsConfig.vendor_key` and `live` from `live_vendor_key` through
the real readers, at the reviewed head and at this one:

```
### reviewed head dbc8ffe
  tools.web.jinaApiKey only                    boot='sk-jina-paid'       live='sk-jina-paid'
  same key + providers.serper                  boot='sk-jina-paid'       live=''
  serper's legacy leaf + providers.firecrawl   boot='sk-serper-legacy'   live='sk-serper-legacy'
  a cleared slot beside the serper leaf        boot='sk-serper-legacy'   live='sk-serper-legacy'
  harness key, providers names other vendors   boot='tv-harness'         live=''
### this head 40893f7
  tools.web.jinaApiKey only                    boot='sk-jina-paid'       live='sk-jina-paid'
  same key + providers.serper                  boot='sk-jina-paid'       live='sk-jina-paid'
  serper's legacy leaf + providers.firecrawl   boot='sk-serper-legacy'   live='sk-serper-legacy'
  a cleared slot beside the serper leaf        boot='sk-serper-legacy'   live='sk-serper-legacy'
  harness key, providers names other vendors   boot='tv-harness'         live='tv-harness'
```

The reviewer's in-flight probe (the real `WebFetchTool` on Jina, a
transport answering 402 and rotating the key source while the request is
in flight), at the fourth round's previous head and at its head
(unchanged by the fifth round: the pair is still read once and the old
key is still the one sent and paused):

```
### reviewed head 428cbe6
  key-source reads during one execute(): 4  ['KEY-OLD', 'KEY-OLD', 'KEY-OLD', 'KEY-NEW']
  Authorization actually sent          : ['KEY-OLD']
  next call                            : answered from the pause, 0 request sent, carrying []  (api_key resolves to KEY-NEW)
### this head 3822234
  key-source reads during one execute(): 1  ['KEY-OLD']
  Authorization actually sent          : ['KEY-OLD']
  next call                            : served, 1 request sent, carrying ['KEY-NEW']  (api_key resolves to KEY-NEW)
```

Earlier rounds, kept for the record; each was measured at the head of
its round, and the tests they name are unchanged since except for the
parametrisations noted below. First round (131 tests in the two files,
at f87ec32):

```
refusals never remembered                     25 failed
a new key does not lift the pause              1 failed
the pause never ends                           1 failed
a 500 pauses too                              19 failed
paused calls change the error string          23 failed
web_search never short-circuits                8 failed
image_search never short-circuits              1 failed
web_fetch never short-circuits                16 failed
control (nothing mutated)                     131 passed
```

Second round, the live key (133 tests, at 145344f):

```
the four production files reverted to the first-round head   3 failed
WebFetchTool ignores its key source                          4 failed
main loop hands the reader a one-time key                    1 failed
sub-agent lane hands the reader a one-time key               1 failed
live_vendor_key never reads the file                         6 failed
control                                                     133 passed
```

Tests changed across the rounds:
`test_a_new_key_lifts_the_pause_at_once` rotates the key through the
callable source the loops hand the tool instead of a private field; the
"vendor and status only, never the key" rule for search (was 401) and
fetch (was 402) now uses a 500, since a refusal status renders
differently on purpose; `test_a_reader_refusing_the_key_pauses_the_tool`
is parametrised over the reader set (401, 402) and
`test_a_search_vendor_refusing_the_key_pauses_the_tool` over the search
set (401, 402, 403). New in the third round:
`test_a_keyless_reader_is_never_paused_by_a_status[401|402|403]` (the
probe above) and `test_a_readers_403_is_about_the_page_not_the_key` for
every reader. Fourth round:
`test_the_subagent_lanes_reader_reads_its_key_live_too` became
`test_the_subagent_lanes_web_tools_read_their_keys_live_too` and covers
all three tools; new:
`test_a_key_rotated_while_a_request_is_in_flight_is_tried_before_it_is_paused[web_search|image_search|web_fetch]`;
the two pause tests assert the remedy wording. Fifth round: new
`test_a_refusal_by_one_vendor_pauses_no_other`; the two pause tests
assert the remedy's new wording (the config routes are read without a
restart, the env var is not); main's own `test_config_live.py` tests for
the callable provider pass unchanged. Sixth round:
`TestLiveVendorKeyFollowsTheCanonicalOrder` (the reviewer's three rows
and three more against `WebToolsConfig.vendor_key` as the oracle),
`test_a_vendor_the_file_says_nothing_about_keeps_its_boot_key`,
`test_a_revoked_leaf_is_an_answer_not_a_miss`,
`test_a_leaf_the_schema_rejects_dispenses_no_new_answer`,
`test_a_keyed_reader_whose_key_is_cleared_falls_back_to_jina_per_call`
and `test_a_reader_whose_key_is_cleared_in_the_file_reads_through_jina`.

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [x] User-facing docs or screenshots are updated when needed (CHANGELOG
`Unreleased / Fixed`)

## Risk

User-visible behaviour changes:

- After a 401 or 402 from a reader, or a 401, 402 or 403 from a search
vendor, the tool answers for up to ten minutes (or until the key
changes) without contacting the vendor, with an envelope/text that names
the cause and the fix. A key set at the vendor's config slot is read on
the next call and lifts the pause; a user who instead tops up the same
account gets one retry at the window's end.
- A request that carried no key never pauses, and a reader's 403 never
pauses: on the default configuration (Jina without a key) `web_fetch`
behaves exactly as before this PR.
- All three tools in both loops read their key from the live config on
every call (the main loop's `web_search` and `image_search` already did;
`web_fetch` and the sub-agent lane's two now do), and resolve their
vendor and key once per call, so the pair a request carried is what its
refusal is recorded against and a refusal by one vendor pauses no other;
a key removed from the file (an empty slot) is a revocation for them
too.
- The refusal text says the config slot and the vendor selection under
`tools.web.<kind>.provider` are read from the config file without a
restart, and the env var on restart.
- `WebFetchTool.provider` and `api_key` now both come from one
`_resolve()`; their values are what main's properties returned, read
once per access rather than the key source twice.
- A Jina key held at the pre-vendor leaf `tools.web.jinaApiKey` keeps
being sent when a `providers` subtree exists beside it, as it did before
this PR and as `WebToolsConfig.vendor_key` resolves it; a key the
process booted with is kept for a vendor the file's `providers` subtree
does not name, and revoked where the subtree names it with an empty key.
- The refusing call's own error text changes from `<Vendor> answered
HTTP 402` to `<Vendor> refused the key (HTTP 402)` plus a detail, for
the statuses in the tool's set. Anything matching the old text for those
statuses would need updating; nothing in the repo does.

Rollback: revert the squash commit. The pause is process-local state; no
config or stored data changes shape.

- [x] Security impact considered (the key is never echoed; the envelope
names the config path and env var, not the value)
- [x] Backward compatibility considered (tool schemas unchanged;
non-refusal statuses render exactly as before)
- [x] Rollback path is clear for risky changes

## Related Issues

N/A

---------

Co-authored-by: Claude (claude-fable-5-1) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants