Commit Graph

8 Commits

Author SHA1 Message Date
Masato Hoshino bdcc5d54a5 fix(agents): honor run abort signal in image and pdf tools (#112644)
* fix(agents): honor run abort signal in image and pdf tools

The image and pdf agent tools declared `execute: async (_toolCallId, args)`
and dropped the run abort signal that `wrapToolWithAbortSignal` supplies as
the third execute argument. The wrapper only races the execute promise, so an
aborted run kept sequentially downloading images/PDFs (up to the per-tool cap,
each up to the byte cap) and still issued a paid vision/PDF-model call for a
dead run.

Thread the signal into the existing `requestInit: { signal }` seam (which the
media fetch layer already merges into the download fetch) and add
`signal.throwIfAborted()` between sequential loop items and before the paid
model call. No new media-options signal field; non-abort behavior is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(agents): keep the pdf test suite under the lint ceiling; guard model dispatch

Follow-up on the abort-signal change, resolving the check-lint failure and a
review finding on the same seam.

max-lines
---------
Adding the two pdf abort tests pushed src/agents/tools/pdf-tool.test.ts to 1018
effective lines against a ceiling of 1000, failing check-lint. Resolved without
touching config/max-lines-baseline.txt — a suppression there would also have
tripped the max-lines ratchet.

- The new tests declared their own `describe` with `beforeEach`/`afterEach`
  hooks identical to the existing `describe("createPdfTool")`. They exercise
  that same tool, so they now live in it and the duplicated scaffolding is gone.
- `stubPdfToolInfra`, `createPdfModelRegistry` and `FAKE_PDF_MEDIA` moved to
  pdf-tool.test-support.ts, which already exists for exactly this. They go
  through `createPdfToolInfraStub(completeMock)` rather than being exported
  directly, because the stub wires the suite's own `complete` mock into the
  model registry and vi.mock handles are file-scoped. All 21 existing call
  sites are unchanged.

Abort propagation into model dispatch
-------------------------------------
The previous revision stopped cancellation at download boundaries and before
the first model call, but `runImagePrompt`/`runPdfPrompt` never saw the signal.
A run cancelled while the first provider request was in flight could still
issue the remaining ones — the image path dispatches `describeImage` once per
image in a sequential loop, so a dead run kept paying for every later image.

Both now take an optional `signal` and check it immediately before each
provider dispatch (3 sites in image-tool, 4 in pdf-tool).

Forwarding the signal further, into the provider transports themselves, is a
different seam and is deliberately left out of this PR.

Also declares `requestInit` on `ImageToolLoadWebMediaOptions`. The local facade
omitted it while the underlying loader accepts it (web-media.ts declares it and
forwards it to readRemoteMediaBuffer), so the option worked at runtime and only
compiled because spread properties skip excess-property checking. A non-spread
call site would not have.

tsgo core + core-test, oxlint, oxfmt clean; pdf-tool and image-tool suites 266
tests pass.

* test(agents): prove in-flight media aborts

* test(agents): narrow PDF loader options

* test(agents): reject abort mocks with errors

* fix(agents): propagate media cancellation

* test(agents): type PDF abort fixture

* test(agents): type prepared runtime snapshot

* fix(agents): normalize abort rejection errors

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
2026-07-25 05:17:09 -07:00
Peter Lee 5c98a12ab1 fix(provider-runtime): treat HTTP 429 as retryable in provider operations (#106365)
* fix(provider-runtime): unify HTTP 429 transient classifiers across retry and POST gate

Route 429 through the POST retry gate so retry-enabled POST helpers
throw into the retry loop for rate-limited responses instead of
returning them past the retry scope. Share the same transient status
set between isTransientProviderHttpStatus and
isTransientProviderOperationError.

Add a caller-level regression test that exercises the guarded POST
429-to-200 flow through postJsonRequest with retryStage read.

* fix(provider-runtime): centralize transient HTTP status predicate into one canonical export
2026-07-17 22:38:04 +03:00
Peter Lee fee89ff4cb fix(provider-runtime): align transient network error codes across retry paths (#101496)
* fix(provider-runtime): align transient network retries

Centralize the shared connection error policy, preserve the bounded provider-only ENOTFOUND retry, and keep gateway DNS/config failures and provider creates fail-fast.\n\nCo-authored-by: Peter Lee <li.xialong@xydigit.com>

* test(provider-runtime): cover root retry error codes

Exercise direct Node error.code and nested cause.code shapes through the public retry wrapper.

Co-authored-by: Peter Lee <li.xialong@xydigit.com>

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
2026-07-12 16:33:24 -07:00
Vincent Koc 3d206140f3 refactor: localize internal implementation constants (#101758) 2026-07-07 08:34:13 -07:00
Vincent Koc c8d95da14c refactor: localize file-private exports (#101701) 2026-07-07 06:57:19 -07:00
Peter Steinberger edb920b857 docs: document remaining src helpers 2026-06-04 20:34:26 -04:00
Peter Steinberger 714ff554fd fix: validate provider retry attempts 2026-05-28 16:28:07 -04:00
Peter Steinberger ecdad948b5 refactor(provider): centralize transient retry stages 2026-05-13 11:26:30 +01:00