Skip to content

fix(agent-core): honor abort signal in WebSearch and FetchURL tools - #1108

Open
Caldalis wants to merge 1 commit into
MoonshotAI:mainfrom
Caldalis:fix/web-tools-abort-signal
Open

fix(agent-core): honor abort signal in WebSearch and FetchURL tools#1108
Caldalis wants to merge 1 commit into
MoonshotAI:mainfrom
Caldalis:fix/web-tools-abort-signal

Conversation

@Caldalis

Copy link
Copy Markdown

Related Issue

Resolve #1106

Problem

WebSearch and FetchURL ignored the per-call AbortSignal. Cancelling a turn
(Ctrl-C / turn abort) did not cancel an in-flight network request — it kept
running in the background, holding a connection, and on a slow or hung server it
lingered until undici's default timeouts.

Every other I/O-bound builtin tool (Bash, Grep, Agent, AskUser) already threads
ctx.signal into its work; the two web tools were the only exception, with no
comment explaining the omission, so this reads as an overs
intentional choice.

What changed

  • Added an optional signal to the UrlFetcher and WebSearchProvider option objects.
  • FetchURLTool / WebSearchTool now forward ctx.signal they destructured only toolCallId`).
  • LocalFetchURLProvider, MoonshotFetchURLProvider, and MoonshotWebSearchProvider pass the signal to the underlying fetch(), which aborts both the header wait and the body read. The Moonshot → loc options object, so the signal flows through automatically.
  • Tests: updated the call-shape assertions for the two tools and added provider tests asserting the signal reaches the underlying fetch.

This matches the existing cancellation pattern used by Bash/Grep/Agent and keeps the change focused. The touched interfaces (UrlFetcher /
WebSearchProvider) are internal to agent-core and are rface.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue, or explained the problem above.
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update

@changeset-bot

changeset-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a1c5358

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@moonshot-ai/kimi-code Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@itxaiohanglover

Copy link
Copy Markdown

Clean threading of the abort signal through every layer — nice work. Tests covering both providers are solid. One small thing: MoonshotFetchURLProvider.fetch has a catch-all that swallows errors into empty content, so an abort there would surface as an 'empty content' error rather than a clean abort. Might be worth letting AbortError propagate, though it's a pre-existing pattern so no blocker.

@Caldalis

Copy link
Copy Markdown
Author

Clean threading of the abort signal through every layer — nice work. Tests covering both providers are solid. One small thing: MoonshotFetchURLProvider.fetch has a catch-all that swallows errors into empty content, so an abort there would surface as an 'empty content' error rather than a clean abort. Might be worth letting AbortError propagate, though it's a pre-existing pattern so no blocker.

Thanks addressed in a follow-up: MoonshotFetchURLProvider.fetch now rethrows when the caller's signal is aborted, so a cancelled request surfaces as a clean abort instead of kicking off a fallback fetch. Genuine Moonshot failures (network errors) still fall back to the local fetcher as before. Added a test covering the abort doesn't fall back path.

@Caldalis
Caldalis force-pushed the fix/web-tools-abort-signal branch from 120345b to a41a165 Compare July 17, 2026 16:20
@Caldalis

Copy link
Copy Markdown
Author

I've rebased this onto the latest main and refreshed the branch. #1791 ("close FetchURL SSRF bypasses and DNS-rebinding window") landed and rewrote LocalFetchURLProvider the actual fetch call moved into the per-hop redirect loop (requestWithValidatedRedirects), so I re-threaded the abort signal through that new structure instead of the old top-level fetch. WebSearch was also slimmed to query-only (#1260), so the provider options were updated to match.
The intent is unchanged:

  • Thread the turn's AbortSignal from the FetchURL / WebSearch tool context through the UrlFetcher / WebSearchProvider interfaces down to every underlying fetch (the Moonshot fetch/search providers and the local fallback's per-hop request).
  • A caller-driven abort of the Moonshot fetch is now re-thrown instead of retrying the local fallback so cancellation surfaces cleanly rather than silently re-running the request.
    Lint, typecheck, and the web-tool tests are green, and a changeset is included.

Thread the turn's AbortSignal through the FetchURL/WebSearch tools and the UrlFetcher / WebSearchProvider interfaces into every underlying fetch, so cancelling a turn aborts the in-flight request instead of leaving it running. Covers the Moonshot fetch/search providers and the local fallback fetcher's per-hop request. A caller-driven abort of the Moonshot fetch is re-thrown instead of retrying the local fallback, so cancellation surfaces cleanly.
@Caldalis
Caldalis force-pushed the fix/web-tools-abort-signal branch from a41a165 to a1c5358 Compare July 18, 2026 04:41
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.

Bug: web-tools-abort-signal

2 participants