Skip to content

fix: Honor research transport deadlines - #6

Merged
wagnercosta merged 3 commits into
mainfrom
em/research-timeout
Sep 14, 2026
Merged

wagnercosta merged 3 commits into
mainfrom
em/research-timeout

Conversation

@wagnercosta

@wagnercosta wagnercosta commented Sep 14, 2026

Copy link
Copy Markdown
Member

Requested by @wagnercosta

What / why

Buffered DeepNews research has a 900,000 ms (15-minute) CLI budget, but the unconfigured transport can expire while waiting for headers or body data after 300,000 ms.

  • Add pinned undici@7.29.1 and a request-scoped Agent/interceptor. Set finite headersTimeout and bodyTimeout at dispatch time, after native fetch has constructed its options, so request-level values cannot override the effective CLI budget.
  • Keep a separate, cancellable buffered deadline across headers and the complete body. Body inactivity is not a total-duration limit; receiving chunks does not extend the buffered deadline.
  • Preserve the 15-minute research default, explicit flag/environment overrides, 60-second ordinary default, SSE chunk/heartbeat inactivity resets, and the existing single 401 refresh retry. No automatic timeout/network retry or routing/model/stream-default change.
  • Cancel discarded 401 bodies, cancel/release SSE readers, clear timers, destroy the scoped dispatcher on success/failure, and retain actionable timeout versus transport/API errors.
  • Document timeout semantics and the optional local wall-clock regression command in existing docs.

Validation

  • pnpm install --frozen-lockfile --ignore-scripts

  • pnpm check: lint, strict types, 118 tests passed, 3 intentionally skipped (2 live tests and the opt-in long local smoke). Coverage thresholds pass.

  • pnpm build:bundle: passed, using the committed schema. The broader pnpm build refreshes the live schema and was intentionally not used for this transport-only change.

  • Node 22.15.0 / bundled Undici 6.21.2: 35 focused tests passed. Node 24.19.0 / bundled Undici 7.29.0: full suite and actual 305-second smoke passed.

  • Real loopback server, four concurrent requests, no fake clock:

    Wait Old/default transport Fixed transport
    Delayed headers UND_ERR_HEADERS_TIMEOUT at 300.762 s JSON success at 305.020 s
    Delayed body UND_ERR_BODY_TIMEOUT at 300.757 s JSON success at 305.016 s

    Both fixed calls retain the default 900-second research budget. Reproduce with:
    ASKNEWS_LOCAL_TIMEOUT_SMOKE=1 pnpm exec vitest run test/integration/http-timeout.integration.test.ts -t wall-clock (outer process budget ≥360 seconds).

  • Short real-transport tests inspect actual dispatched values and cover true total expiry despite body chunks, delayed/stalled bodies, SSE comment heartbeats and idle aborts, closed sockets, 401-only retry, and error distinctions. Mock tests exercise conflicting fetch-supplied timer values and cleanup. Dispatch and cleanup regressions fail against the unchanged main implementation and pass with this change.

  • Built CLI against loopback: original Reddit research flags still use POST /v1/chat/deepnews, with model/stream flags omitted; flag-over-environment timeout precedence and exits 0/4/5/6 verified.

Native-fetch caveat: larger Agent defaults alone are honored on the tested Node 24 runtime; the interceptor does not assume otherwise. It explicitly wins over request-level timers if supplied, verified with a conflicting-options test.

Risk / boundaries

  • New runtime dependency and request-scoped connection pools; connections are not reused across separate operation executions. Node 22 minimum-runtime compatibility is covered, but this does not exhaust all runtime/proxy/TLS environments.
  • The buffered budget is per HTTP attempt, as before; OAuth refresh and the one authenticated retry are not a single total wall-clock budget. Connection-establishment protections remain at Undici defaults. SSE has an inactivity limit, not a total-duration cap.
  • Local HTTP proof, not a live research run or a full 900-second wall-clock test. No live/billable API requests, global installation, deployment, schema refresh, installed-skill/config edits, or workflow changes.
  • Revert this commit to roll back. A human merge to main feeds release-please; merging its separate Release PR tags/publishes. This PR does not merge, release, tag, or publish anything.

CI

  • Adds a Node 24 compatibility job (typecheck, tests, bundle) so the npm undici dispatcher ↔ bundled-fetch contract is exercised on both supported Node majors. Both jobs pass on this branch.

@wagnercosta

Copy link
Copy Markdown
Member Author

Follow-up commit pins undici 7.29.1 instead of 7.29.0: 7.29.1 (2026-09-04) is a security release (GHSA-w293-vg96-wgc3, GHSA-rfgv-xxqx-mfg5, GHSA-3wwx-pv8p-q78v). None of the affected code paths (BalancedPool, WebSocket, permessage-deflate) is used by the CLI, but a public npm package should not pin a version with open high-severity advisories. Re-verified locally on the bumped lockfile: pnpm check passes (118 tests, coverage thresholds met) and the 305-second wall-clock smoke passes (old transport fails at ~300 s, fixed transport completes at ~305 s).

@wagnercosta
wagnercosta merged commit 62ec298 into main Sep 14, 2026
2 checks passed
@wagnercosta
wagnercosta deleted the em/research-timeout branch September 14, 2026 19:03
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.

1 participant