Skip to content

fix(web-search): name a credential for a provider, fill Brave's open date bound, and thread abort - #886

Merged
sroussey merged 1 commit into
claude/websearch-task-providers-8aik77from
claude/eloquent-gauss-zd517e-websearch-hardening
Sep 2, 2026
Merged

fix(web-search): name a credential for a provider, fill Brave's open date bound, and thread abort#886
sroussey merged 1 commit into
claude/websearch-task-providers-8aik77from
claude/eloquent-gauss-zd517e-websearch-hardening

Conversation

@sroussey

@sroussey sroussey commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Targets claude/websearch-task-providers-8aik77, the head branch of #850, so these fixes fold into that PR rather than landing on main separately.

Three findings from a review of #850.

Rebased 2026-09-02. #850 has been rebased onto origin/main (linear, 20 commits), so this branch no longer carries a merge commit and its diff is now just the one fix commit — 18 files, down from 473. A fourth finding (the new package's build-types calling tsgo, removed by main's TypeScript 7 migration) moved into #850's rebase, where it belongs: the four vendor package.jsons resolved to tsc as merge conflicts, and packages/web-search/package.json is a new file that could not conflict, so it was fixed there for consistency rather than here.

1. provider: "auto" sent the caller's key to whichever vendor routing picked

WebSearchTask.execute() copied credential_key onto baseRequest.credentialKey before a provider was chosen. route() selects purely on unhonorableOptions(...) and returns the first registered provider in insertion order, and the built-ins register brave → tavily → searxng — so { query, provider: "auto", credential_key: "tavily-api-key" } routed to Brave, which put the resolved Tavily secret on its X-Subscription-Token header to api.search.brave.com. Nothing warned, and the caller could not know which vendor received the secret.

Routing is now credential-aware rather than just refusing the combination:

  • A new credential_keys input maps provider name → credential-store key. The key handed to a provider is the one named for that provider, so a key issued for one vendor cannot reach another.
  • route() takes the set of named providers and considers them first — naming a key states which vendors the caller holds a key for, so "auto" no longer picks a keyless provider while a usable one sits behind it. The preference yields to the capability check, never overrides it.
  • A bare credential_key combined with "auto" is refused with a TaskConfigurationError naming credential_keys. It remains valid with a pinned provider, where the vendor is settled before the run.
  • Routing lands on a provider no key was named for → the request goes out unauthenticated. An unauthenticated search that fails is recoverable; a secret arriving at the wrong vendor is not.

One supporting change in @workglow/task-graph: scanGraphForCredentials / scanGraphForFormat now also read a credential format annotated on a map's additionalProperties value schema. Without it the new port is invisible to the scan that unlocks the credential store, and every keyed "auto" run would go out unauthenticated — the same silent failure in a different place.

Tests: WebSearchCredential.test.ts drives a real FetchUrlTask and asserts a Tavily-named credential never reaches Brave (and never appears on any Brave call), that a routed provider with no named key gets none, and that auto + credential_key throws before any fetch. WebSearchTask.test.ts covers per-provider targeting, the routing preference and its subordination to capabilities. GraphFormatScanner.test.ts covers the map-valued port, including empty maps and empty values.

2. Brave declared dateFilter: true but silently dropped an end-only range

freshnessParam returned undefined when only range.end was set, so no freshness parameter was sent and the request ran unfiltered — while dateFilter: true meant unhonorableOptions found no gap, a pinned brave request passed, and "auto" would route end-only date requests here. The caller got undated results reported as a successful date-bounded search, which is exactly what the capability record exists to prevent.

The open end is now filled, mirroring what this branch already does for Gemini's timeRangeFilter (OPEN_INTERVAL_START): freshness takes a closed interval and accepts an arbitrary start date, so an end-only range becomes 1970-01-01to<end>. An entirely empty range still sends nothing. The test that locked in the silent drop now asserts the filled range.

3. No abort signal reached any grounded provider's SDK call

All four grounded providers called context.signal.throwIfAborted() and then issued the SDK request with no signal. A Ctrl-C or graph abort mid-turn cancelled nothing: the request stayed in flight and the run was billed for tokens nobody would read — worst for Anthropic, which runs up to MAX_PAUSE_RESUMES + 1 requests at max_tokens: 16000. Every other Anthropic run-fn in the repo threads it, so this was a deviation rather than a choice.

Option names verified against the installed SDKs:

Provider SDK Option
Anthropic @anthropic-ai/sdk 0.123.0 messages.create(params, { signal })
OpenAI openai 7.8.0 responses.create(params, { signal })
OpenRouter openai 7.8.0 chat.completions.create(params, { signal })
Gemini @google/genai 2.20.0 config.abortSignal

Each provider gains a test asserting the signal reaches the SDK; the Anthropic one checks every request of a resumed paused turn, not just the first.

Verification

Re-run after the rebase, on ad6b411e6:

$ bun run format-check
All matched files use the correct format.
Finished in 258ms on 2392 files using 4 threads.

$ bun run build:types
 Tasks:    43 successful, 43 total

$ oxlint --type-aware --deny-warnings packages providers
(exit 0, no findings)

$ vitest run --project web-search --project anthropic --project openai \
    --project openrouter --project google-gemini --project task-graph
 Test Files  121 passed (121)
      Tests  1153 passed (1153)

The new tests were also run against the pre-fix sources to confirm they are not vacuous: 9 of them fail there (7 credential/routing, 1 abort signal, 1 Brave date range).

🤖 Generated with Claude Code

https://claude.ai/code/session_01DdZebeeuQjgQTUszAjJDLb


Generated by Claude Code

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 55.35% 40102 / 72440
🔵 Statements 54.85% 42090 / 76732
🔵 Functions 56.23% 7797 / 13866
🔵 Branches 44.38% 20652 / 46533
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/web-search/src/WebSearchProviderRegistry.ts 96.15% 90.9% 90% 95.65% 95
packages/web-search/src/WebSearchTask.ts 100% 92.85% 100% 100%
packages/web-search/src/providers/BraveWebSearchProvider.ts 100% 88% 100% 100%
providers/anthropic/src/web-search/AnthropicWebSearchProvider.ts 92.3% 79.16% 100% 97.82% 93, 148-150, 170, 179
providers/google-gemini/src/web-search/GeminiWebSearchProvider.ts 97.14% 80% 100% 100% 51
providers/openai/src/web-search/OpenAiWebSearchProvider.ts 97.29% 82.75% 100% 100% 117
providers/openrouter/src/web-search/OpenRouterWebSearchProvider.ts 97.14% 89.28% 100% 100% 120
Generated in workflow #3345 for commit ad6b411 by the Vitest Coverage Report Action

…date bound, and thread abort

Four things a review found on the branch.

A search credential was resolved onto the request BEFORE routing chose a
provider, so `{ provider: "auto", credential_key: "tavily-api-key" }` sent the
Tavily secret to whichever vendor `route()` returned first — Brave, in
registration order, on its `X-Subscription-Token` header. `credential_keys` now
maps provider name to store key: the key sent is the one named for the provider
that runs, routing prefers a provider a key is named for, and the ambiguous
combination of a bare `credential_key` with `"auto"` is refused. The graph
credential scan learns to read a format annotated on a map's `additionalProperties`
value schema, so a run using the map still unlocks the store.

Brave declared `dateFilter: true` but sent no `freshness` for an end-only range,
running the search unfiltered while reporting the bound as honored. The open end
is filled, the way this branch already fills Gemini's `timeRangeFilter`.

The four grounded providers checked `context.signal` and then issued the SDK
request without it, so an abort cancelled nothing and the run was still billed
for the turn in flight. Each now passes the signal through the option its SDK
takes.

`packages/web-search` is a new package and shipped `build-types: tsgo`, which no
longer exists, plus a dead `lint` script.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdZebeeuQjgQTUszAjJDLb
@sroussey
sroussey force-pushed the claude/websearch-task-providers-8aik77 branch from a9bef17 to 1c07c86 Compare September 2, 2026 15:29
@sroussey
sroussey force-pushed the claude/eloquent-gauss-zd517e-websearch-hardening branch from d071aa4 to ad6b411 Compare September 2, 2026 15:29
@sroussey
sroussey merged commit d075b1a into claude/websearch-task-providers-8aik77 Sep 2, 2026
12 checks passed
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