mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(core): stop AbortSignal listener leak in long sessions (MaxListenersExceededWarning) (#4366)
* fix(core): consolidate AbortController handling to stop listener leaks in long sessions
Users hit `MaxListenersExceededWarning: 1509 abort listeners added to
[AbortSignal]` in long interactive sessions. The agent runtime nests
parent→child controllers (masterAbortController → per-message round →
per-API-call round → tool execution) and each layer registered its own
`addEventListener('abort', ...)` on the parent without `{once:true}` or
reverse cleanup, so listeners accumulated on long-lived parents across
hundreds of model turns.
Add `utils/abortController.ts` with three helpers:
- `createAbortController(maxListeners = 50)` — factory that pre-caps the
signal so the warning never fires on per-request signals.
- `createChildAbortController(parent)` — WeakRef-based parent→child
propagation with `{once:true}` on the parent listener AND a reverse-cleanup
listener on the child that detaches the parent listener when the child
aborts. This is the key mechanism — short-lived children stop accumulating
dead listeners on long-lived parents.
- `combineAbortSignals(signals, {timeoutMs})` — N-way combiner that replaces
the existing one-input `combinedAbortSignal.ts` (kept as a `@deprecated`
shim so `httpHookRunner.ts` doesn't churn).
Migrate every production `new AbortController()` in `packages/core/src` (24
sites) to the helper. Wrap `_runReasoningLoopInner` per-iteration body and
`AgentHeadless.execute` in `try/finally` so the round controller is aborted
(triggering reverse cleanup) even when the model stream or tool execution
throws. Add `{once:true}` to the manual abort listeners in `hookRunner`,
`functionHookRunner`, and `message-bus` that were missing it. Remove the
`raiseAbortListenerCap` band-aid from `openaiContentGenerator/pipeline.ts` —
no longer needed now that the per-round signal carries `maxListeners=50`.
Add `cli/utils/warningHandler.ts` as a belt-and-suspenders: hides
`MaxListenersExceededWarning.*AbortSignal` from end users in production
(any shape Node ≥20 emits), keeps it visible under `DEBUG`/`QWEN_DEBUG`/
`NODE_ENV=development`. Uses `process.on('warning', ...)` without
`removeAllListeners` so third-party warning subscribers stay intact.
Direct reproducer in `docs/verification/abort-controller-refactor/` proves
the old pattern accumulates 2000 listeners over 2000 rounds while the new
pattern stays at 0.
* fix(core): address PR #4366 review feedback
Four issues from the Copilot review:
1. combineAbortSignals — add a per-iteration `aborted` check inside the
for-loop so we short-circuit if an input signal flips aborted between
the initial scan and listener registration. In single-threaded JS this
can't actually interleave, but the defensive check makes correctness
obvious and protects against signals whose `aborted` getter has side
effects. New test exercises the path via a Proxy that flips after the
initial scan.
2. warningHandler docstring — was stale: said "AbortSignal / EventTarget"
while the regex was tightened to AbortSignal-only in the previous review.
3. README.md — replace personal absolute path with `$WT` placeholder so
the verification recipe is shareable.
4. README.md — replace the markdown table with per-scenario headed
sections. Prettier had interpreted an inline `ps -ef | grep sleep`
pipe character as a column separator, breaking the table rendering on
GitHub. Per-section format is also easier to scan and edit.
* test(core): fix abortController race-defense test to actually hit the loop check
The previous version set the Proxy's `aborted` to true before calling
combineAbortSignals, so the initial `find` scan caught it and we took the
fast path — not the per-iteration check the test was meant to validate.
Switch to an access counter so `aborted` is false on the first read (during
`find`) and true on subsequent reads (inside the loop). This forces the
loop to enter, then catches the flip via the defensive per-iteration check
before any listener is attached to the next input.
Verified the test fails if the per-iteration check is removed.
* fix(lint): include docs/**/*.mjs in the script ESLint block so the AbortController repro passes lint
CI Lint flagged 11 no-undef errors in
docs/verification/abort-controller-refactor/listener-accumulation-repro.mjs
(AbortController, console, process) because the project's flat config
only declared Node globals for ./scripts/**/*.mjs.
The reviewer's suggestion (`/* eslint-env node */`) doesn't work under
ESLint 9 flat config — env directives are deprecated there. The proper
fix is to extend the existing script-globals block to also cover the
verification repro script under docs/.
* fix(core,cli): address PR #4366 critical review findings
Two real bugs the reviewer caught and I confirmed locally:
1. warningHandler.ts didn't actually suppress anything. Adding a
`process.on('warning')` listener does NOT prevent Node's default
onWarning printer from writing to stderr — the default is just an
ordinary listener registered in `lib/internal/process/warning.js`.
My previous code therefore:
- failed to suppress targeted AbortSignal warnings (they still hit
stderr via the default printer)
- produced a SECOND copy of every non-suppressed warning (default
printer + my handler's own stderr.write)
The unit tests missed it because they synthesised a fake warning and
called `process.listeners('warning')` directly rather than going
through `process.emitWarning`.
Fix: snapshot the existing `'warning'` listeners (which include the
default printer and any third-party telemetry hooks) BEFORE replacing
them. Install ours as the sole listener. For non-suppressed warnings
fan out to the captured set so the default printer + telemetry still
fire; for suppressed warnings stop here. Tests now use
`process.emit('warning', ...)` to drive the real listener chain, plus
a spawned-child integration test that asserts the real stderr from
`process.emitWarning` is empty for AbortSignal warnings and still
contains DeprecationWarning text.
2. abortController.createChildAbortController kept a WeakRef to the
child controller. A natural usage pattern — pass `child.signal` into
an async API and drop the controller object — could let the
controller be GC'd while the signal is still in use, after which
`parent.abort()` would no longer propagate. Reproduced with
`node --expose-gc`.
Fix: hold the child strongly via the parent's listener closure. The
reverse-cleanup listener still removes the closure when child aborts
(closure releases child → GC-eligible), and the parent's `{once:true}`
listener self-removes when parent fires (same effect). Net listener
accounting on long-lived parents is unchanged; the only difference is
the controller now stays alive long enough for propagation to reach
downstream consumers that hold only the signal. Tests updated: drop
the old `--expose-gc`-dependent assertion that abandoned children
GC immediately (that was a property of the OLD contract); add a
signal-only-retention test that verifies propagation under the new
contract without needing GC at all.
Verified: 32 helper/warning tests pass (incl. spawned-child stderr
integration); 363 affected caller tests pass; typecheck + prettier +
eslint clean for the touched files.
* fix(core,cli): address PR #4366 review — fix combineAbortSignals orphan listeners + runtime DEBUG toggle
Two real bugs the reviewer caught:
1. combineAbortSignals registered its cleanup listener on
controller.signal AFTER the for-loop. Node does NOT fire 'abort'
listeners added to an already-aborted signal, so when the
per-iteration defensive check aborted the controller mid-loop, the
cleanup never ran — orphaning every input-signal listener registered
before the break, and leaving the (also-registered-after-the-break)
setTimeout uncleared.
Fix: skip timeout scheduling when controller.signal.aborted is
already true post-loop, and when it's true call cleanup()
synchronously instead of registering a doomed listener. Existing
test for the mid-iteration path now also asserts that the
pre-break input signal (a) has zero abort listeners — that's the
assertion that catches the orphan bug. New test for the
already-aborted-input + timeoutMs combination confirms the timer
isn't scheduled (would otherwise overwrite the abort reason).
2. warningHandler captured isDebugMode() in a closure at init time, so
toggling DEBUG / QWEN_DEBUG at runtime (e.g. via a /debug slash
command) didn't update suppression behavior. Moved the check inside
the handler — warnings are rare so the per-emit env-lookup cost is
negligible. New test asserts a mid-stream DEBUG=1 flip starts
forwarding suppressed warnings to the prior-listener chain.
* test(core): strengthen the timeout-guard test in combineAbortSignals to actually exercise the new !aborted check
Reviewer correctly pointed out that the previous version of this test
took the pre-loop fast path (since `a.abort('pre')` ran before
`combineAbortSignals`), so it never reached the in-loop guard at
abortController.ts:138.
Switched to the Proxy `aborted`-getter pattern from the sibling
mid-iteration test (so the loop genuinely re-checks `aborted` and
short-circuits inside the for-loop), and added a `setTimeout` spy that
asserts the timer was never scheduled — this is the only observable
difference from "scheduled then immediately cleared by synchronous
cleanup()", which is what the timer-advance assertion alone couldn't
distinguish.
Verified by mutation testing: removing the guard makes the new test
fail; restoring it makes it pass. Refs PR #4366.
* test(core): cover timeout-triggered cleanup of input-signal listeners in combineAbortSignals
Reviewer noted the timeout path only had an empty-input test, leaving
the leak-sensitive case uncovered: when timeoutMs fires with a
long-lived source signal in the input list, do the input-side
listeners get released? They do (the timeout callback aborts the
combined controller, which fires the auto-cleanup listener registered
on its signal, which calls the per-input removeEventListener), but
that path wasn't tested.
Adds a test that snapshots the source listener count before, asserts
it increased by 1 after combineAbortSignals returns, advances fake
timers past timeoutMs, and asserts the count returns to baseline.
Refs PR #4366.
* fix(test): use pathToFileURL for the warning-handler e2e import on Windows
CI failure on windows-latest:
AssertionError: expected '\r\nnode:internal/modules/run_main:12…'
to match /DeprecationWarning.*Plain deprecation/
Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in:
file, data, and node are supported by the default ESM loader. On
Windows, absolute paths must be valid file:// URLs. Received protocol 'd:'
The e2e test wrote a child script with an `import "<helperPath>"` where
helperPath was a raw Windows absolute path (`D:\a\qwen-code\...`). Node's
ESM loader parses that as a URL on Windows and rejects the `D:` "scheme".
Converted the helper path to a `file://` URL via `pathToFileURL`. macOS
test still passes; the Windows-specific schemes-must-be-URL behavior is
now honored. Refs PR #4366.
* fix(core,cli): address PR #4366 review batch — onAbort leak, migrate missed sites, tighten tests
Adopted 6 of the 7 review threads (skipping the debug-logging suggestion).
1. processFunctionCalls onAbort leak (CRITICAL): the new
`finally { roundAbortController.abort(); }` in _runReasoningLoopInner
would fire the `onAbort` handler in `processFunctionCalls` if
scheduler.schedule or batchDone threw (the explicit
removeEventListener at the old happy-path exit would be skipped),
emitting spurious "Tool call cancelled by user abort." TOOL_RESULT
events for every un-emitted callId — corrupting the transcript and
misleading the model on the next round. Fixed by wrapping schedule
+ batchDone in their own try/finally so removeEventListener always
runs before the outer finally's abort.
2. Migrate 3 new-from-main `new AbortController()` sites that this
PR's audit missed (they came in via the merge from main):
- goals/goalHook.ts (2 sites: judgeController, fallback signal) —
consistency
- hooks/promptHookRunner.ts (1 site: internalAbortController) —
real leak (manual addEventListener without {once:true} or
cleanup, exactly the pattern this PR exists to fix). Switched to
createChildAbortController + finally `internalAbortController.abort()`
for reverse cleanup on the success path.
3. Repro script (`listener-accumulation-repro.mjs`): inlined helper
diverged from production — used WeakRef on child, while production
was changed to strong-ref earlier in this PR. Updated the inlined
copy to match production exactly, with a comment noting the
intentional WeakRef-on-parent-only pattern.
4. warningHandler.ts: documented the snapshot-and-replace trade-offs
in the JSDoc (late-added listeners bypass our filter; late
`removeListener` calls have no effect on our fan-out). Tried the
re-snapshot-per-warning approach the reviewer suggested but it
doesn't work — `removeAllListeners('warning')` permanently removes
the snapshot from Node's tracking, so a `process.listeners('warning')`
filter at fan-out time always returns empty for prior listeners.
The current design is the right trade-off; documentation is the
correct fix.
5. abortController.test.ts: added three coverage gaps the reviewer
identified —
- createChildAbortController forwards custom maxListeners
- manual cleanup() before scheduled timeout fires cancels it
- timeoutMs <= 0 is treated as "no timeout"
6. Migrated `httpHookRunner.ts:202` (the lone caller of the deprecated
`createCombinedAbortSignal`) to `combineAbortSignals` directly,
then deleted `combinedAbortSignal.ts` + its test. All semantics
covered by `combineAbortSignals` tests in abortController.test.ts.
Refreshed `migration-completeness.txt` (now empty — clean grep).
Tests: 194 pass across abortController/warningHandler/agent-runtime/
followup/hooks/goal/promptHook suites. Typecheck + prettier clean.
* docs(verification): commit the headless-scenario scripts referenced by the PR body
The PR body's "End-to-end scenarios I drove locally" section points at
docs/verification/abort-controller-refactor/scripts/02-lite.sh and 06-headless-sigint.sh.
These are the actual reproducible commands behind the EXIT codes /
warning counts reported there — checking them in so anyone can replay
without copy-pasting from the PR description.
Refs PR #4366.
* docs(verification): sync automated-results with current state
Two doc fixes the reviewer flagged:
- migration-completeness.txt was a 0-byte file with a confusing
cross-reference. Populated with the actual grep command + its
"(no output)" result so the empty-output state is explicit.
- automated-results.md still referenced combinedAbortSignal.test.ts (8
tests, @deprecated shim) — both files were deleted in 94e8c5812 when
httpHookRunner.ts migrated to combineAbortSignals directly. Replaced
the line with a reference to httpHookRunner.test.ts. Also updated
the test counts to reflect current state (26 abortController, 13
warningHandler — both grew with the review cycle) and removed the
stale combinedAbortSignal.ts entry from the prettier-check command.
Refs PR #4366.
* test(core): pin two abort-cascade behaviors PR #4366 introduced
Adopting 2 of 3 new review threads (the third — automated-results.md
drift — was already fixed in 5aa7110e4).
1. packages/core/src/agents/arena/ArenaManager.test.ts: pin the
master→agent abort cascade introduced by switching per-agent
controllers to `createChildAbortController(this.masterAbortController)`.
New test spawns ≥2 agents, calls `manager.cancel()`, and asserts every
`agentState.abortController.signal.aborted === true`. Existing cancel
test only checked backend + status; if a future refactor re-introduced
independent controllers, the cascade would silently regress.
2. packages/core/src/followup/speculation.test.ts: cover the
`startSpeculation` abort wiring introduced when the manual
addEventListener + .finally removeEventListener pattern got replaced
by createChildAbortController + .finally abort(). Three tests:
- parent abort propagates to state.abortController (lifetime contract)
- parent-already-aborted fast path returns aborted state
- parent-signal listener count returns to baseline after the fire-and-
forget loop settles (reverse-cleanup proof)
Mocked `runWithForkedChatModel` and `OverlayFs` so the background
loop is a no-op — these tests only assert the synchronous wiring,
not the loop's content.
* fix(test): speculation.test.ts TS errors + sync verification doc counts
Two real CI blockers in the just-added speculation tests (TS2554 and
TS2339) plus stale doc counts the reviewer flagged.
1. saveCacheSafeParams takes 3 positional args (generationConfig,
history, model), not a single object. Compile error on every
platform. Fixed by switching to the correct shape; also moved
getEventListeners to a static `import` at the top of the file
(dynamic `await import('node:events')` exposes EventEmitter's
static method via the namespace type rather than as a direct
property, so destructuring fails type-check).
2. docs/verification/abort-controller-refactor/README.md still claimed
"18 + 1 GC" tests for abortController and "9" for warningHandler;
actual current counts are 26 and 13. Also dropped the stale
combinedAbortSignal reference and added a note about the new
ArenaManager cascade + startSpeculation wiring pin tests.
Refreshed smoke-boot.log against current built bin (still 0.15.11,
which is what package.json reports on this branch).
Refs PR #4366.
* refactor(core): narrow PR #4366 scope per yiliang's review — revert independent-controller migrations
Adopting @yiliang114's review feedback (#4366 review comment, 2026-05-22):
keep only the migrations that fix the real leak path (the agent-runtime
parent→child chain that accumulates listeners on a long-lived parent
signal in long sessions) and revert the consistency-only migrations on
independent short-lived controllers.
Issue #4423 confirms the user-visible bug is the nested-chain
accumulation — the reverted sites do not contribute to that bug.
Migrations KEPT:
- agents/runtime/agent-interactive.ts (master + per-message round)
- agents/runtime/agent-core.ts (per-iteration + wait + processFunctionCalls)
- agents/runtime/agent-headless.ts (external → execution)
- hooks/promptHookRunner.ts (real cleanup leak: addEventListener without
{once:true}, never removed)
- hooks/httpHookRunner.ts → combineAbortSignals direct (shim deleted)
- hookRunner.ts / functionHookRunner.ts / message-bus.ts: {once:true} only
- openaiContentGenerator/pipeline.ts band-aid removal (per-request signals
are children of the per-round controller, which carries maxListeners=50)
- warningHandler.ts belt-and-suspenders
Migrations REVERTED (independent short-lived controllers; restored to
`new AbortController()` + their original cleanup patterns):
- agents/arena/ArenaManager.ts (master + per-agent)
- agents/background-agent-resume.ts (3 sites)
- core/client.ts (recall — restored manual addEventListener + finally
removeEventListener pattern from main)
- followup/speculation.ts (restored parentAbortHandler + finally
removeEventListener)
- goals/goalHook.ts (judgeController + fallback signal)
- memory/manager.ts (dream controller)
- services/chatCompressionService.ts (fallback signal)
- services/chatRecordingService.ts (autoTitle controller)
- tools/agent/agent.ts (fg + bg subagent controllers — restored manual
onParentAbort + finally removeEventListener)
- tools/monitor.ts (entryAc)
- tools/shell.ts (promote + 3 entryAc)
- utils/fetch.ts (fetchWithTimeout)
Tests removed alongside the reverts:
- ArenaManager.test.ts "cancels cascades..." — the cascade itself was an
intentional behavioral improvement that's now reverted, so the
pin-test belongs with it
- speculation.test.ts "startSpeculation — abort-controller wiring" block
(3 tests) — they tested helper-wired behavior we reverted
Verification docs updated to reflect the narrower scope.
Net change: 19 raw `new AbortController()` remain (intentional, per
migration-completeness.txt rationale); previously was 0.
Refs PR #4366, issue #4423.
This commit is contained in:
parent
60dbbaaed4
commit
174e8de179
25 changed files with 1699 additions and 563 deletions
120
docs/verification/abort-controller-refactor/README.md
Normal file
120
docs/verification/abort-controller-refactor/README.md
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
# AbortController refactor — verification plan
|
||||
|
||||
Scenarios used to validate the change manually before opening the PR. Each
|
||||
scenario captures its tmux pane via `tmux pipe-pane -o 'cat >> <log>'`.
|
||||
|
||||
## Setup once
|
||||
|
||||
```sh
|
||||
# Point WT at your local checkout of the branch under review.
|
||||
WT=/path/to/qwen-code/worktree
|
||||
LOGDIR=$WT/docs/verification/abort-controller-refactor/logs
|
||||
mkdir -p "$LOGDIR"
|
||||
|
||||
# Build the CLI once (skip sandbox image, skip vscode).
|
||||
( cd "$WT" && npm run build:packages )
|
||||
```
|
||||
|
||||
## Scenarios
|
||||
|
||||
For each scenario:
|
||||
|
||||
```sh
|
||||
tmux new-session -d -s qwen-verify-XX
|
||||
tmux pipe-pane -t qwen-verify-XX -o "cat >> $LOGDIR/XX-name.log"
|
||||
tmux send-keys -t qwen-verify-XX "cd /path/to/your/test/workspace && exec node $WT/packages/cli/dist/index.js" C-m
|
||||
tmux attach -t qwen-verify-XX
|
||||
```
|
||||
|
||||
Then drive the session manually per the matrix below. Hit `C-b d` to detach
|
||||
when done; `tmux kill-session -t qwen-verify-XX` to stop the pane.
|
||||
|
||||
### 00 — Baseline (PRE-fix)
|
||||
|
||||
- **Setup:** check out `main`, build, run with `NODE_OPTIONS=--trace-warnings`.
|
||||
- **Input:** long 50-round mixed-tool session (shell + edit + grep + agent).
|
||||
- **Expected:** after ~30–40 rounds, `MaxListenersExceededWarning: ... 1500+ abort listeners added to [AbortSignal]` printed to stderr.
|
||||
- **Log:** `00-baseline-reproduction.log`.
|
||||
|
||||
### 01 — Long-session, DEBUG mode (this branch)
|
||||
|
||||
- **Setup:** `NODE_OPTIONS=--trace-warnings DEBUG=1 qwen`.
|
||||
- **Input:** same 50-round script as #00.
|
||||
- **Expected:** no `MaxListenersExceededWarning` printed; any other warnings still print.
|
||||
- **Log:** `01-long-session-debug.log`.
|
||||
|
||||
### 02 — Long-session, prod mode (this branch)
|
||||
|
||||
- **Setup:** `qwen` (no debug env).
|
||||
- **Input:** same 50-round script.
|
||||
- **Expected:** clean output; a temporary `console.error` probe inside the handler (added then removed) confirms the filter fires.
|
||||
- **Log:** `02-long-session-prod.log`.
|
||||
|
||||
### 03 — Ctrl-C mid-stream abort
|
||||
|
||||
- **Setup:** this branch, interactive.
|
||||
- **Input:** ask for a long generation (>30s); press Ctrl-C mid-stream.
|
||||
- **Expected:** stream stops within ~200ms, "Cancelled" banner shown, next prompt accepts input. `process._getActiveHandles()` count returns to baseline (use `:debug handles`).
|
||||
- **Log:** `03-ctrlc-streaming.log`.
|
||||
|
||||
### 04 — Cancel long-running shell
|
||||
|
||||
- **Setup:** this branch.
|
||||
- **Input:** run `sleep 60` via the shell tool; cancel mid-execution.
|
||||
- **Expected:** child process killed (verify with `pgrep -f sleep` returning empty), tool result shows cancellation, agent accepts next prompt.
|
||||
- **Log:** `04-shell-cancel.log`.
|
||||
|
||||
### 05 — Subagent cancellation
|
||||
|
||||
- **Setup:** this branch.
|
||||
- **Input:** spawn a long agent task via the agent tool; cancel from parent.
|
||||
- **Expected:** subagent's in-flight tool calls abort, subagent's model stream stops, parent receives cancellation event.
|
||||
- **Log:** `05-subagent-cancel.log`.
|
||||
|
||||
### 06 — Headless / non-interactive abort
|
||||
|
||||
- **Setup:** `qwen --prompt "do a long task"`; send `SIGINT` from outside via `kill -INT <pid>`.
|
||||
- **Expected:** clean shutdown, exit code 130, no warnings.
|
||||
- **Log:** `06-headless-abort.log`.
|
||||
|
||||
### 07 — Background agent flow
|
||||
|
||||
- **Setup:** interactive.
|
||||
- **Input:** spawn a background agent (`run_in_background: true`); let it complete; spawn a second one; cancel the second mid-flight.
|
||||
- **Expected:** first agent completes normally; second aborts cleanly; no listener leak across the two.
|
||||
- **Log:** `07-background-agent.log`.
|
||||
|
||||
### 08 — Memory baseline
|
||||
|
||||
- **Setup:** `qwen --inspect`, attach Chrome devtools.
|
||||
- **Input:** 100-round session.
|
||||
- **Expected:** heap snapshots at round 0/50/100. `AbortSignal` instance count and per-signal listener count stable (no monotonic growth).
|
||||
- **Log:** `08-memory-snapshots/`.
|
||||
|
||||
### 09 — Existing combinedAbortSignal consumer
|
||||
|
||||
- **Setup:** trigger an HTTP hook with both an external signal and timeout.
|
||||
- **Input:** (a) cancel external signal mid-hook; (b) let timeout fire in a separate run.
|
||||
- **Expected:** hook aborts cleanly in both cases; deprecation shim path is exercised.
|
||||
- **Log:** `09-http-hook-shim.log`.
|
||||
|
||||
## Automated (non-interactive) verifications
|
||||
|
||||
The automated checks below were run during development and recorded in
|
||||
`automated-results.md`:
|
||||
|
||||
- All abortController unit tests pass (`abortController.test.ts`, 26 tests; 1 GC test skipped under non-`--expose-gc`).
|
||||
- All warningHandler tests pass (`warningHandler.test.ts`, 13 tests including a spawned-child stderr integration test).
|
||||
- All `combineAbortSignals` consumer tests pass (`httpHookRunner.test.ts`); the deprecated `createCombinedAbortSignal` shim plus its own test file were removed once the lone caller migrated.
|
||||
- All agent runtime / followup / openaiContentGenerator / hooks tests pass.
|
||||
- Migration scope (intentional): only the agent-runtime parent→child chain (`agent-interactive.ts`, `agent-core.ts`, `agent-headless.ts`) plus `promptHookRunner.ts` (real cleanup leak) was switched to the helper. Independent short-lived controllers (per-shell-command, per-fetch, per-recall, etc.) stay on raw `new AbortController()` — they're GC'd quickly and don't accumulate listeners on a long-lived parent. See `migration-completeness.txt` for the captured grep + rationale.
|
||||
- TypeScript strict-mode typecheck passes for both `packages/core` and `packages/cli`.
|
||||
- Prettier check passes on all modified files.
|
||||
|
||||
See `automated-results.md` for the actual command output.
|
||||
|
||||
## How to capture the artifacts for the PR body
|
||||
|
||||
After running each scenario, attach the transcript file (or relevant excerpt)
|
||||
to the PR. For #08 (memory), export the heap snapshots and include the
|
||||
listener-count delta between snapshots.
|
||||
139
docs/verification/abort-controller-refactor/automated-results.md
Normal file
139
docs/verification/abort-controller-refactor/automated-results.md
Normal file
|
|
@ -0,0 +1,139 @@
|
|||
# Automated verification results
|
||||
|
||||
Captured 2026-05-20 during the AbortController refactor.
|
||||
|
||||
## 1. Listener-accumulation reproducer
|
||||
|
||||
Direct simulation of the listener-accumulation pattern observed in long
|
||||
sessions (1500+ abort listeners on a single AbortSignal). The script lives
|
||||
at `listener-accumulation-repro.mjs`.
|
||||
|
||||
```text
|
||||
$ node docs/verification/abort-controller-refactor/listener-accumulation-repro.mjs
|
||||
Simulating 2000 rounds for each pattern.
|
||||
|
||||
OLD pattern listener count on long-lived parent: 2000
|
||||
NEW pattern listener count on long-lived parent: 0
|
||||
PASS: OLD pattern accumulated >1500 listeners (reproduces the bug).
|
||||
PASS: NEW pattern kept listener count at 0 — the helper prevents accumulation.
|
||||
```
|
||||
|
||||
This is a self-contained proof: the OLD pattern (raw `addEventListener`
|
||||
without `{once:true}` or reverse cleanup) accumulates 2000 listeners over
|
||||
2000 rounds — well past the 1500 threshold the user observed. The NEW
|
||||
pattern (`createChildAbortController` from `packages/core/src/utils/abortController.ts`)
|
||||
keeps the parent listener count at 0 across 2000 rounds because each child's
|
||||
reverse-cleanup listener removes the parent listener when the child aborts.
|
||||
|
||||
## 2. Migration scope (intentional)
|
||||
|
||||
Only the agent-runtime parent→child chain that actually accumulates listeners
|
||||
on a long-lived parent signal is migrated to the helper:
|
||||
|
||||
- `packages/core/src/agents/runtime/agent-interactive.ts` (master + per-message round)
|
||||
- `packages/core/src/agents/runtime/agent-core.ts` (per-iteration round + waitForExternalInputs + processFunctionCalls try/finally)
|
||||
- `packages/core/src/agents/runtime/agent-headless.ts` (external → execution)
|
||||
- `packages/core/src/hooks/promptHookRunner.ts` (had a real cleanup leak: manual addEventListener without `{once:true}` and never removed)
|
||||
|
||||
Plus three `{once:true}`-only fixes (no helper switch, just defensive
|
||||
correctness):
|
||||
|
||||
- `packages/core/src/hooks/hookRunner.ts`
|
||||
- `packages/core/src/hooks/functionHookRunner.ts`
|
||||
- `packages/core/src/confirmation-bus/message-bus.ts`
|
||||
|
||||
Independent short-lived controllers (per-shell-command in `tools/shell.ts`,
|
||||
per-monitor in `tools/monitor.ts`, per-arena-session in
|
||||
`agents/arena/ArenaManager.ts`, per-recall in `core/client.ts`,
|
||||
per-fetch in `utils/fetch.ts`, per-dream / per-title / per-judge / per-resume,
|
||||
etc.) stay on raw `new AbortController()` — they're GC'd at end of use and
|
||||
do not accumulate on a long-lived parent.
|
||||
|
||||
See `migration-completeness.txt` for the actual grep + rationale.
|
||||
|
||||
## 3. Affected test suites
|
||||
|
||||
All 71 affected test files / 2085 tests pass (3 skipped — 1 is the GC test
|
||||
that requires `--expose-gc`, 2 are pre-existing skips in the headless suite).
|
||||
|
||||
```text
|
||||
Test Files 71 passed (71)
|
||||
Tests 2085 passed | 3 skipped (2088)
|
||||
Duration 16.71s
|
||||
```
|
||||
|
||||
Coverage:
|
||||
|
||||
- `packages/core/src/utils/abortController.test.ts` — 26 tests: factory cap (default + custom), child propagation, reverse cleanup, fast path, undefined parent, custom-maxListeners passthrough, `combineAbortSignals` semantics (incl. cleanup-cancels-timeout, timeout-cleans-input-listeners, `timeoutMs <= 0` boundary, mid-iteration defensive check), GC safety (best-effort).
|
||||
- `packages/cli/src/utils/warningHandler.test.ts` — 13 tests: idempotency, AbortSignal suppression (including `[AbortSignal{...}]` shape), generic EventTarget NOT suppressed, debug-mode passthrough, fan-out to prior listeners, spawned-child end-to-end stderr integration.
|
||||
- `packages/core/src/hooks/httpHookRunner.test.ts` — covers the migrated `combineAbortSignals` consumer (the deprecated `createCombinedAbortSignal` shim plus its test file were removed once the lone caller migrated).
|
||||
- `packages/core/src/agents/runtime/{agent-core,agent-interactive,agent-headless,agent-context,agent-statistics}.test.ts` — 102 tests covering the high-impact migrated files.
|
||||
- `packages/core/src/core/openaiContentGenerator/**` — 280+ tests including the pipeline that lost the `raiseAbortListenerCap` band-aid.
|
||||
- `packages/core/src/followup/**` — 100+ tests including the migrated speculation controller.
|
||||
- `packages/core/src/tools/agent/**`, `packages/core/src/tools/shell.test.ts`, `packages/core/src/services/**`, `packages/core/src/hooks/**`, `packages/core/src/confirmation-bus/**` — all migrated tool/hook/service files.
|
||||
|
||||
## 4. TypeScript strict-mode typecheck
|
||||
|
||||
```sh
|
||||
$ node_modules/.bin/tsc -p packages/core/tsconfig.json --noEmit
|
||||
(no output, exit 0)
|
||||
|
||||
$ node_modules/.bin/tsc -p packages/cli/tsconfig.json --noEmit
|
||||
(no output, exit 0)
|
||||
```
|
||||
|
||||
## 5. Prettier formatting
|
||||
|
||||
```sh
|
||||
$ node_modules/.bin/prettier --check packages/core/src/agents/runtime/agent-core.ts \
|
||||
packages/core/src/agents/runtime/agent-headless.ts \
|
||||
packages/cli/src/utils/warningHandler.ts \
|
||||
packages/cli/src/utils/warningHandler.test.ts \
|
||||
packages/core/src/utils/abortController.ts \
|
||||
packages/core/src/utils/abortController.test.ts
|
||||
Checking formatting...
|
||||
All matched files use Prettier code style!
|
||||
```
|
||||
|
||||
## 6. Build + binary smoke test
|
||||
|
||||
```sh
|
||||
$ npm run build:packages
|
||||
(succeeds for all 5 workspace packages)
|
||||
|
||||
$ NODE_OPTIONS=--trace-warnings node packages/cli/dist/index.js --version
|
||||
0.15.11
|
||||
EXIT=0
|
||||
|
||||
$ node packages/cli/dist/index.js --help
|
||||
Usage: qwen [options] [command]
|
||||
...
|
||||
```
|
||||
|
||||
No warnings emitted during boot with `--trace-warnings`.
|
||||
|
||||
## 7. Codex independent review
|
||||
|
||||
Two full passes via the `codex:codex-rescue` agent (independent context each
|
||||
time). First pass surfaced 3 issues — all addressed in subsequent commits:
|
||||
|
||||
1. **Throw between controller creation and explicit abort leaks listener** in
|
||||
`agent-core.ts`'s per-iteration body and `agent-headless.ts`'s
|
||||
pre-try-block setup. Fixed by wrapping each in `try { ... } finally {
|
||||
abortController.abort(); }`.
|
||||
2. **Warning suppressor regex `EventTarget` too broad**. Tightened to match
|
||||
only `AbortSignal` (any shape Node ≥20 produces).
|
||||
3. **`process.removeAllListeners('warning')` strips third-party listeners**.
|
||||
Removed — rely on Node's "no listeners → default printer fires" semantics
|
||||
so adding our handler implicitly disables the default print path while
|
||||
keeping third-party telemetry listeners intact.
|
||||
|
||||
Second pass confirmed all fixes correct, no further blockers.
|
||||
|
||||
## What remains for interactive verification
|
||||
|
||||
The scenarios in `README.md` numbered 00–09 require a real interactive
|
||||
session against the model API (long mixed-tool conversations, Ctrl-C
|
||||
mid-stream, subagent cancellation, heap snapshots). Those are documented
|
||||
for human execution and the transcripts should be attached to the PR body
|
||||
when run.
|
||||
|
|
@ -0,0 +1,107 @@
|
|||
#!/usr/bin/env node
|
||||
/**
|
||||
* Direct simulation of the listener-accumulation pattern the agent runtime
|
||||
* exhibits in long sessions. Builds a deep parent → child chain to a depth
|
||||
* the user observed (>1500 listeners) and asserts:
|
||||
*
|
||||
* 1. The OLD pattern (plain new AbortController + manual addEventListener
|
||||
* without {once:true} or reverse cleanup) accumulates listeners on the
|
||||
* long-lived parent — reproducing the warning.
|
||||
*
|
||||
* 2. The NEW pattern (createChildAbortController from the helper) keeps the
|
||||
* parent listener count bounded by 1, regardless of how many short-lived
|
||||
* children come and go.
|
||||
*
|
||||
* Run:
|
||||
* node docs/verification/abort-controller-refactor/listener-accumulation-repro.mjs
|
||||
*/
|
||||
|
||||
import { getEventListeners, setMaxListeners } from 'node:events';
|
||||
|
||||
// Inline copy of the production helper (packages/core/src/utils/abortController.ts)
|
||||
// so this script has no build-step dependency on @qwen-code/qwen-code-core.
|
||||
// Kept in sync — the child is held STRONGLY by the parent's listener closure
|
||||
// (no WeakRef on child) so propagation works even when a caller drops the
|
||||
// controller and keeps only the signal. WeakRef is used only on the PARENT,
|
||||
// to keep child cleanup from pinning a long-lived parent in memory.
|
||||
function createAbortController(maxListeners = 50) {
|
||||
const c = new AbortController();
|
||||
setMaxListeners(maxListeners, c.signal);
|
||||
return c;
|
||||
}
|
||||
function createChildAbortController(parent) {
|
||||
const child = createAbortController();
|
||||
if (!parent) return child;
|
||||
const parentSignal = parent.signal ?? parent;
|
||||
if (parentSignal.aborted) {
|
||||
child.abort(parentSignal.reason);
|
||||
return child;
|
||||
}
|
||||
const weakParent = new WeakRef(parentSignal);
|
||||
const handler = () => {
|
||||
child.abort(weakParent.deref()?.reason);
|
||||
};
|
||||
parentSignal.addEventListener('abort', handler, { once: true });
|
||||
child.signal.addEventListener(
|
||||
'abort',
|
||||
() => {
|
||||
weakParent.deref()?.removeEventListener('abort', handler);
|
||||
},
|
||||
{ once: true },
|
||||
);
|
||||
return child;
|
||||
}
|
||||
|
||||
const ROUNDS = 2000;
|
||||
|
||||
console.log(`Simulating ${ROUNDS} rounds for each pattern.\n`);
|
||||
|
||||
// ─── OLD pattern: plain new AbortController + manual addEventListener ───
|
||||
const oldParent = new AbortController();
|
||||
setMaxListeners(0, oldParent.signal); // disable warning so we can measure cleanly
|
||||
for (let i = 0; i < ROUNDS; i++) {
|
||||
const child = new AbortController();
|
||||
// No {once:true}, no reverse cleanup — accumulates on oldParent.
|
||||
oldParent.signal.addEventListener('abort', () => child.abort());
|
||||
}
|
||||
const oldCount = getEventListeners(oldParent.signal, 'abort').length;
|
||||
|
||||
// ─── NEW pattern: createChildAbortController ───
|
||||
const newParent = createAbortController();
|
||||
for (let i = 0; i < ROUNDS; i++) {
|
||||
const child = createChildAbortController(newParent);
|
||||
child.abort(); // simulate end-of-round cleanup via try/finally
|
||||
}
|
||||
const newCount = getEventListeners(newParent.signal, 'abort').length;
|
||||
|
||||
console.log(`OLD pattern listener count on long-lived parent: ${oldCount}`);
|
||||
console.log(`NEW pattern listener count on long-lived parent: ${newCount}`);
|
||||
|
||||
const expectations = {
|
||||
oldShouldExceed: 1500,
|
||||
newMustBe: 0,
|
||||
};
|
||||
|
||||
let pass = true;
|
||||
if (oldCount <= expectations.oldShouldExceed) {
|
||||
console.error(
|
||||
`FAIL: OLD pattern should accumulate >${expectations.oldShouldExceed} listeners; got ${oldCount}`,
|
||||
);
|
||||
pass = false;
|
||||
} else {
|
||||
console.log(
|
||||
`PASS: OLD pattern accumulated >${expectations.oldShouldExceed} listeners (reproduces the bug).`,
|
||||
);
|
||||
}
|
||||
if (newCount !== expectations.newMustBe) {
|
||||
console.error(
|
||||
`FAIL: NEW pattern must have exactly ${expectations.newMustBe} listeners; got ${newCount}`,
|
||||
);
|
||||
pass = false;
|
||||
} else {
|
||||
console.log(
|
||||
`PASS: NEW pattern kept listener count at ${expectations.newMustBe} — the helper prevents accumulation.`,
|
||||
);
|
||||
}
|
||||
|
||||
process.exit(pass ? 0 : 1);
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
$ grep -rn "new AbortController" packages/core/src --include="*.ts" \
|
||||
| grep -v test | grep -v abortController.ts
|
||||
|
||||
# Scoped to the nested parent→child chain that actually accumulates listeners
|
||||
# (the agent-runtime loop in long sessions, plus promptHookRunner which had a
|
||||
# real cleanup leak). Independent short-lived controllers (per-shell-command,
|
||||
# per-fetch, per-recall, per-arena-session etc.) intentionally stay on raw
|
||||
# `new AbortController()` — they're GC'd at the end of their use and do not
|
||||
# accumulate listeners on a long-lived parent signal.
|
||||
packages/core/src/followup/speculation.ts:100: const abortController = new AbortController();
|
||||
packages/core/src/tools/agent/agent.ts:1722: const bgAbortController = new AbortController();
|
||||
packages/core/src/tools/agent/agent.ts:2116: const fgAbortController = new AbortController();
|
||||
packages/core/src/tools/shell.ts:1514: const promoteAbortController = new AbortController();
|
||||
packages/core/src/tools/shell.ts:2364: const entryAc = new AbortController();
|
||||
packages/core/src/tools/shell.ts:2772: const entryAc = new AbortController();
|
||||
packages/core/src/tools/monitor.ts:306: const entryAc = new AbortController();
|
||||
packages/core/src/core/client.ts:1199: const controller = new AbortController();
|
||||
packages/core/src/memory/manager.ts:936: const abortController = new AbortController();
|
||||
packages/core/src/goals/goalHook.ts:70: const judgeController = new AbortController();
|
||||
packages/core/src/goals/goalHook.ts:169: const signal = context?.signal ?? new AbortController().signal;
|
||||
packages/core/src/agents/arena/ArenaManager.ts:305: this.masterAbortController = new AbortController();
|
||||
packages/core/src/agents/arena/ArenaManager.ts:817: abortController: new AbortController(),
|
||||
packages/core/src/agents/background-agent-resume.ts:421: abortController: new AbortController(),
|
||||
packages/core/src/agents/background-agent-resume.ts:493: const bgAbortController = new AbortController();
|
||||
packages/core/src/agents/background-agent-resume.ts:922: abortController: new AbortController(),
|
||||
packages/core/src/utils/fetch.ts:64: const controller = new AbortController();
|
||||
packages/core/src/services/chatRecordingService.ts:963: const controller = new AbortController();
|
||||
packages/core/src/services/chatCompressionService.ts:387: abortSignal: signal ?? new AbortController().signal,
|
||||
23
docs/verification/abort-controller-refactor/scripts/02-lite.sh
Executable file
23
docs/verification/abort-controller-refactor/scripts/02-lite.sh
Executable file
|
|
@ -0,0 +1,23 @@
|
|||
#!/usr/bin/env bash
|
||||
# Scenario 02-lite — single real-Qwen prompt under --trace-warnings.
|
||||
# Demonstrates the steady-state path emits no MaxListenersExceededWarning.
|
||||
set -uo pipefail
|
||||
WT="${WT:-$(git rev-parse --show-toplevel)}"
|
||||
LOG="$WT/docs/verification/abort-controller-refactor/logs/02-lite-short-prompt.log"
|
||||
mkdir -p "$(dirname "$LOG")"
|
||||
|
||||
NODE_OPTIONS=--trace-warnings node "$WT/packages/cli/dist/index.js" \
|
||||
--prompt "Reply with exactly 'OK' and nothing else." > "$LOG" 2>&1 &
|
||||
PID=$!
|
||||
for i in $(seq 1 90); do
|
||||
if ! kill -0 $PID 2>/dev/null; then break; fi
|
||||
sleep 1
|
||||
done
|
||||
if kill -0 $PID 2>/dev/null; then kill -9 $PID; echo "TIMEOUT"; exit 1; fi
|
||||
wait $PID 2>/dev/null
|
||||
EC=$?
|
||||
|
||||
echo "EXIT=$EC"
|
||||
echo "MaxListenersExceededWarning count: $(grep -c MaxListenersExceededWarning "$LOG")"
|
||||
echo "--- log ---"
|
||||
cat "$LOG"
|
||||
18
docs/verification/abort-controller-refactor/scripts/06-headless-sigint.sh
Executable file
18
docs/verification/abort-controller-refactor/scripts/06-headless-sigint.sh
Executable file
|
|
@ -0,0 +1,18 @@
|
|||
#!/usr/bin/env bash
|
||||
# Scenario 06 — headless --prompt + SIGINT. Verifies the agent shuts down
|
||||
# cleanly when an external signal aborts the in-flight stream.
|
||||
set -uo pipefail
|
||||
WT="${WT:-$(git rev-parse --show-toplevel)}"
|
||||
LOG="$WT/docs/verification/abort-controller-refactor/logs/06-headless-sigint.log"
|
||||
mkdir -p "$(dirname "$LOG")"
|
||||
|
||||
NODE_OPTIONS=--trace-warnings node "$WT/packages/cli/dist/index.js" \
|
||||
--prompt "Please write a detailed essay about the history of distributed systems, at least 500 words." > "$LOG" 2>&1 &
|
||||
PID=$!
|
||||
sleep 6
|
||||
kill -INT $PID
|
||||
wait $PID 2>/dev/null
|
||||
EC=$?
|
||||
|
||||
echo "EXIT_CODE=$EC (expected 130)"
|
||||
echo "MaxListenersExceededWarning count: $(grep -c MaxListenersExceededWarning "$LOG")"
|
||||
|
|
@ -0,0 +1,2 @@
|
|||
0.15.11
|
||||
EXIT=0
|
||||
10
docs/verification/abort-controller-refactor/test-summary.txt
Normal file
10
docs/verification/abort-controller-refactor/test-summary.txt
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
✓ |@qwen-code/qwen-code-core| src/core/openaiContentGenerator/provider/minimax.test.ts (9 tests) 2ms
|
||||
✓ |@qwen-code/qwen-code-core| src/followup/suggestionGenerator.test.ts (16 tests) 2ms
|
||||
✓ |@qwen-code/qwen-code-core| src/followup/speculation.test.ts (7 tests) 2ms
|
||||
✓ |@qwen-code/qwen-code| src/utils/warningHandler.test.ts (9 tests) 5ms
|
||||
|
||||
Test Files 71 passed (71)
|
||||
Tests 2085 passed | 3 skipped (2088)
|
||||
Start at 02:35:37
|
||||
Duration 16.71s (transform 1.77s, setup 92ms, collect 14.79s, tests 3.03s, environment 319ms, prepare 2.20s)
|
||||
|
||||
|
|
@ -191,7 +191,14 @@ export default tseslint.config(
|
|||
},
|
||||
// extra settings for scripts that we run directly with node
|
||||
{
|
||||
files: ['./scripts/**/*.js', './scripts/**/*.mjs', 'esbuild.config.js', 'packages/*/scripts/**/*.js'],
|
||||
files: [
|
||||
'./scripts/**/*.js',
|
||||
'./scripts/**/*.mjs',
|
||||
'esbuild.config.js',
|
||||
'packages/*/scripts/**/*.js',
|
||||
// Verification reproducer scripts under docs/ also run with `node`.
|
||||
'docs/**/*.mjs',
|
||||
],
|
||||
languageOptions: {
|
||||
globals: {
|
||||
...globals.node,
|
||||
|
|
|
|||
|
|
@ -78,6 +78,7 @@ import { start_sandbox } from './utils/sandbox.js';
|
|||
import { getStartupWarnings } from './utils/startupWarnings.js';
|
||||
import { getUserStartupWarnings } from './utils/userStartupWarnings.js';
|
||||
import { getCliVersion } from './utils/version.js';
|
||||
import { initializeWarningHandler } from './utils/warningHandler.js';
|
||||
import { writeStderrLine } from './utils/stdioHelpers.js';
|
||||
import { getHeadlessYoloSafetyWarning } from './utils/headlessSafetyWarnings.js';
|
||||
import { computeWindowTitle } from './utils/windowTitle.js';
|
||||
|
|
@ -403,6 +404,7 @@ export async function main() {
|
|||
setStartupEventSink((name, attrs) => recordStartupEvent(name, attrs));
|
||||
}
|
||||
setupUnhandledRejectionHandler();
|
||||
initializeWarningHandler();
|
||||
|
||||
if (process.argv.includes('--bare')) {
|
||||
process.env[QWEN_CODE_SIMPLE_ENV_VAR] = '1';
|
||||
|
|
|
|||
253
packages/cli/src/utils/warningHandler.test.ts
Normal file
253
packages/cli/src/utils/warningHandler.test.ts
Normal file
|
|
@ -0,0 +1,253 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
initializeWarningHandler,
|
||||
resetWarningHandlerForTests,
|
||||
} from './warningHandler.js';
|
||||
|
||||
const ENV_KEYS = ['NODE_ENV', 'DEBUG', 'QWEN_DEBUG'] as const;
|
||||
|
||||
describe('initializeWarningHandler', () => {
|
||||
const originalEnv: Partial<Record<(typeof ENV_KEYS)[number], string>> = {};
|
||||
let originalListeners: NodeJS.WarningListener[] = [];
|
||||
// Mock prior listener — installed before initializeWarningHandler so it
|
||||
// becomes one of the captured "priorListeners" the handler fans out to.
|
||||
// This is the channel the real Node default printer travels on, so
|
||||
// asserting fan-out here is equivalent to asserting "the default printer
|
||||
// would have fired" without coupling tests to internal Node behavior.
|
||||
let priorListener: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
for (const k of ENV_KEYS) originalEnv[k] = process.env[k];
|
||||
for (const k of ENV_KEYS) delete process.env[k];
|
||||
originalListeners = [...process.listeners('warning')];
|
||||
process.removeAllListeners('warning');
|
||||
resetWarningHandlerForTests();
|
||||
priorListener = vi.fn();
|
||||
process.on('warning', priorListener);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
resetWarningHandlerForTests();
|
||||
process.removeAllListeners('warning');
|
||||
for (const l of originalListeners) process.on('warning', l);
|
||||
for (const k of ENV_KEYS) {
|
||||
if (originalEnv[k] === undefined) delete process.env[k];
|
||||
else process.env[k] = originalEnv[k];
|
||||
}
|
||||
});
|
||||
|
||||
function makeWarning(name: string, message: string): Error {
|
||||
const err = new Error(message);
|
||||
err.name = name;
|
||||
return err;
|
||||
}
|
||||
|
||||
function emit(warning: Error): void {
|
||||
// Drive the real listener chain (process.emit dispatches synchronously
|
||||
// to every registered 'warning' listener). After initializeWarningHandler,
|
||||
// the only listener is ours; it decides whether to fan out to the
|
||||
// captured priorListener mock.
|
||||
process.emit('warning', warning);
|
||||
}
|
||||
|
||||
it('suppresses MaxListenersExceededWarning for AbortSignal in production', () => {
|
||||
initializeWarningHandler();
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 1509 abort listeners added to [AbortSignal].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT suppress generic [EventTarget] warnings — only AbortSignal', () => {
|
||||
initializeWarningHandler();
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 11 listeners added to [EventTarget].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('suppresses AbortSignal warnings with class metadata, e.g. [AbortSignal{aborted: false}]', () => {
|
||||
initializeWarningHandler();
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal{aborted: false}].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('fans out unrelated warnings to captured prior listeners (e.g. Node default printer)', () => {
|
||||
initializeWarningHandler();
|
||||
const warning = makeWarning('DeprecationWarning', 'Some legacy thing');
|
||||
emit(warning);
|
||||
expect(priorListener).toHaveBeenCalledTimes(1);
|
||||
expect(priorListener).toHaveBeenCalledWith(warning);
|
||||
});
|
||||
|
||||
it('preserves third-party warning listeners — they still fire for non-suppressed warnings', () => {
|
||||
const telemetryHook = vi.fn();
|
||||
process.on('warning', telemetryHook);
|
||||
initializeWarningHandler();
|
||||
emit(makeWarning('DeprecationWarning', 'X'));
|
||||
expect(priorListener).toHaveBeenCalledTimes(1);
|
||||
expect(telemetryHook).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('a buggy prior listener does not break the chain for the rest', () => {
|
||||
const buggy = vi.fn(() => {
|
||||
throw new Error('boom');
|
||||
});
|
||||
const downstream = vi.fn();
|
||||
process.on('warning', buggy);
|
||||
process.on('warning', downstream);
|
||||
initializeWarningHandler();
|
||||
emit(makeWarning('DeprecationWarning', 'X'));
|
||||
expect(buggy).toHaveBeenCalledTimes(1);
|
||||
expect(downstream).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('keeps suppressed warnings visible when DEBUG is set', () => {
|
||||
process.env['DEBUG'] = '1';
|
||||
initializeWarningHandler();
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 1500 abort listeners added to [AbortSignal].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('treats DEBUG=0 and DEBUG=false as not set', () => {
|
||||
process.env['DEBUG'] = '0';
|
||||
initializeWarningHandler();
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 1500 abort listeners added to [AbortSignal].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps warnings visible when QWEN_DEBUG is set', () => {
|
||||
process.env['QWEN_DEBUG'] = '1';
|
||||
initializeWarningHandler();
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 1500 abort listeners added to [AbortSignal].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('keeps warnings visible when NODE_ENV=development', () => {
|
||||
process.env['NODE_ENV'] = 'development';
|
||||
initializeWarningHandler();
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 1500 abort listeners added to [AbortSignal].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('is idempotent — repeated calls install only one listener', () => {
|
||||
initializeWarningHandler();
|
||||
initializeWarningHandler();
|
||||
initializeWarningHandler();
|
||||
expect(process.listeners('warning').length).toBe(1);
|
||||
});
|
||||
|
||||
it('honors runtime DEBUG toggles — debug check is evaluated per warning', () => {
|
||||
initializeWarningHandler();
|
||||
// Initially DEBUG unset → suppression active.
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 1500 abort listeners added to [AbortSignal].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).not.toHaveBeenCalled();
|
||||
|
||||
// Flip DEBUG at runtime → next suppressed-pattern warning passes through.
|
||||
process.env['DEBUG'] = '1';
|
||||
emit(
|
||||
makeWarning(
|
||||
'MaxListenersExceededWarning',
|
||||
'Possible EventTarget memory leak detected. 1500 abort listeners added to [AbortSignal].',
|
||||
),
|
||||
);
|
||||
expect(priorListener).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('initializeWarningHandler — end-to-end stderr behavior', () => {
|
||||
// Integration test: spawn a child Node process to verify the real
|
||||
// emitWarning → default printer path is actually suppressed. The unit
|
||||
// tests above can't catch this because the default printer lives inside
|
||||
// Node and writes to stderr via internal mechanisms, not via the same
|
||||
// process.stderr.write spy.
|
||||
it('a child process with the handler installed does not print suppressed AbortSignal warnings to stderr', async () => {
|
||||
const { spawn } = await import('node:child_process');
|
||||
const { fileURLToPath, pathToFileURL } = await import('node:url');
|
||||
const { dirname, join } = await import('node:path');
|
||||
const { writeFile, mkdtemp, rm } = await import('node:fs/promises');
|
||||
const { tmpdir } = await import('node:os');
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
// Convert the absolute path to a `file://` URL — on Windows, Node's ESM
|
||||
// loader rejects raw absolute paths (it treats `D:` as a URL scheme),
|
||||
// so import specifiers MUST be file URLs.
|
||||
const helperImportSpecifier = pathToFileURL(
|
||||
join(here, 'warningHandler.ts'),
|
||||
).href;
|
||||
|
||||
const dir = await mkdtemp(join(tmpdir(), 'warning-handler-e2e-'));
|
||||
try {
|
||||
const script = `
|
||||
import { initializeWarningHandler } from ${JSON.stringify(helperImportSpecifier)};
|
||||
delete process.env.DEBUG; delete process.env.QWEN_DEBUG;
|
||||
process.env.NODE_ENV = 'production';
|
||||
initializeWarningHandler();
|
||||
process.emitWarning(
|
||||
'Possible EventTarget memory leak detected. 1509 abort listeners added to [AbortSignal].',
|
||||
'MaxListenersExceededWarning'
|
||||
);
|
||||
process.emitWarning('Plain deprecation', 'DeprecationWarning');
|
||||
`;
|
||||
const scriptPath = join(dir, 'run.mjs');
|
||||
await writeFile(scriptPath, script);
|
||||
|
||||
const child = spawn(process.execPath, ['--import', 'tsx', scriptPath], {
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
});
|
||||
let stderr = '';
|
||||
child.stderr.on('data', (b) => {
|
||||
stderr += b.toString();
|
||||
});
|
||||
await new Promise<void>((resolve) => child.on('exit', () => resolve()));
|
||||
|
||||
expect(stderr).not.toMatch(/abort listeners added to \[AbortSignal\]/);
|
||||
// Deprecation should still print via the fanned-out default printer.
|
||||
expect(stderr).toMatch(/DeprecationWarning.*Plain deprecation/);
|
||||
} finally {
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
}, 15_000);
|
||||
});
|
||||
107
packages/cli/src/utils/warningHandler.ts
Normal file
107
packages/cli/src/utils/warningHandler.ts
Normal file
|
|
@ -0,0 +1,107 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
/**
|
||||
* Warnings we know about and want to keep out of the user-facing terminal.
|
||||
* Listener accumulation on long-lived AbortSignals during multi-round agent
|
||||
* sessions is structural, not a real memory leak — the listeners are removed
|
||||
* (via {once:true} + reverse-cleanup in utils/abortController.ts) but a few
|
||||
* extreme cases (e.g. OpenAI retry storms layered with multiple wrappers) can
|
||||
* still graze the per-signal cap. Match any MaxListenersExceededWarning that
|
||||
* mentions AbortSignal so we cover every shape Node ≥20 emits — `[AbortSignal]`,
|
||||
* `[AbortSignal{...}]`, `[AbortSignal { ... }]`. We deliberately don't match
|
||||
* the generic `EventTarget` token so unrelated EventTarget leaks stay visible.
|
||||
*/
|
||||
const SUPPRESSED_WARNINGS: RegExp[] = [
|
||||
/MaxListenersExceededWarning.*AbortSignal/,
|
||||
];
|
||||
|
||||
function isSuppressed(warning: Error): boolean {
|
||||
const text = `${warning.name}: ${warning.message}`;
|
||||
return SUPPRESSED_WARNINGS.some((re) => re.test(text));
|
||||
}
|
||||
|
||||
function isDebugMode(): boolean {
|
||||
if (process.env['NODE_ENV'] === 'development') return true;
|
||||
const truthy = (v: string | undefined) =>
|
||||
!!v && v !== '0' && v.toLowerCase() !== 'false';
|
||||
return truthy(process.env['DEBUG']) || truthy(process.env['QWEN_DEBUG']);
|
||||
}
|
||||
|
||||
let installedHandler: ((warning: Error) => void) | null = null;
|
||||
|
||||
/**
|
||||
* For tests only — uninstall the handler and reset internal state.
|
||||
*/
|
||||
export function resetWarningHandlerForTests(): void {
|
||||
if (installedHandler) {
|
||||
process.removeListener('warning', installedHandler);
|
||||
installedHandler = null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Install a process-level `warning` handler that swallows the well-known
|
||||
* `MaxListenersExceededWarning` for AbortSignal while letting every other
|
||||
* warning through — including generic EventTarget leak warnings, which we
|
||||
* leave visible because they likely indicate a real leak elsewhere. In
|
||||
* debug mode (NODE_ENV=development, or DEBUG / QWEN_DEBUG set), all
|
||||
* warnings are forwarded so developers can still see them.
|
||||
*
|
||||
* Implementation note: simply adding a `warning` listener does NOT prevent
|
||||
* Node's default printer from writing to stderr — the default handler is
|
||||
* registered as an ordinary listener (`lib/internal/process/warning.js`).
|
||||
* To actually suppress targeted warnings, we capture the existing listeners
|
||||
* (which include the default printer and any third-party telemetry hooks),
|
||||
* remove them, then install ours as the sole listener. Non-suppressed
|
||||
* warnings get fanned out to the captured listeners so the default printer
|
||||
* still fires for them; suppressed warnings stop here.
|
||||
*
|
||||
* Idempotent — repeated calls are a no-op.
|
||||
*/
|
||||
export function initializeWarningHandler(): void {
|
||||
if (installedHandler) return;
|
||||
|
||||
// Snapshot everything currently listening on 'warning' (Node's default
|
||||
// onWarning printer + any third-party telemetry subscribers). We will fan
|
||||
// out non-suppressed warnings back to them.
|
||||
//
|
||||
// Trade-offs to be aware of (documented for future readers):
|
||||
// - Listeners ADDED via `process.on('warning', ...)` after this init are
|
||||
// independent of our snapshot. They receive `process.emit('warning')`
|
||||
// directly and bypass the suppression filter. Node's default printer
|
||||
// is in our snapshot (not added later), so stderr stays clean; late
|
||||
// telemetry will see the full warning stream including AbortSignal
|
||||
// leaks. This is intentional — telemetry should see what's happening.
|
||||
// - Listeners REMOVED via `process.removeListener('warning', fn)` after
|
||||
// this init have no effect: we hold our own strong reference in the
|
||||
// snapshot. Re-snapshotting per warning doesn't fix this because the
|
||||
// listeners are already removed from Node's list (we called
|
||||
// `process.removeAllListeners` to disable Node's default printing of
|
||||
// suppressed warnings). Callers who need conditional fan-out should
|
||||
// install BEFORE initializeWarningHandler.
|
||||
const priorListeners = [...process.listeners('warning')] as Array<
|
||||
(warning: Error) => void
|
||||
>;
|
||||
|
||||
installedHandler = (warning: Error) => {
|
||||
// Evaluate isDebugMode() per warning so DEBUG / QWEN_DEBUG can be
|
||||
// toggled at runtime (e.g. via a `/debug` slash command) without
|
||||
// re-running initializeWarningHandler.
|
||||
if (!isDebugMode() && isSuppressed(warning)) return;
|
||||
for (const fn of priorListeners) {
|
||||
try {
|
||||
fn(warning);
|
||||
} catch {
|
||||
// Don't let a misbehaving prior listener (e.g. a buggy telemetry
|
||||
// hook) take down warning delivery for the rest of the chain.
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
process.removeAllListeners('warning');
|
||||
process.on('warning', installedHandler);
|
||||
}
|
||||
|
|
@ -17,6 +17,7 @@
|
|||
*/
|
||||
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { createChildAbortController } from '../../utils/abortController.js';
|
||||
import { reportError } from '../../utils/errorReporting.js';
|
||||
import { subagentNameContext } from '../../utils/subagentNameContext.js';
|
||||
import type { Config } from '../../config/config.js';
|
||||
|
|
@ -591,252 +592,253 @@ export class AgentCore {
|
|||
break;
|
||||
}
|
||||
|
||||
// Create a new AbortController per round to avoid listener accumulation
|
||||
// in the model SDK. The parent abortController propagates abort to it.
|
||||
const roundAbortController = new AbortController();
|
||||
const onParentAbort = () => roundAbortController.abort();
|
||||
abortController.signal.addEventListener('abort', onParentAbort);
|
||||
if (abortController.signal.aborted) {
|
||||
roundAbortController.abort();
|
||||
}
|
||||
// Per-round child controller so model-SDK retry layers don't accumulate
|
||||
// listeners on the long-lived parent. createChildAbortController handles
|
||||
// parent propagation; the try/finally below guarantees reverse-cleanup
|
||||
// fires for every exit (success, break, return, throw).
|
||||
const roundAbortController = createChildAbortController(abortController);
|
||||
|
||||
const promptId = `${this.runtimeContext.getSessionId()}#${this.subagentId}#${turnCounter++}`;
|
||||
try {
|
||||
const promptId = `${this.runtimeContext.getSessionId()}#${this.subagentId}#${turnCounter++}`;
|
||||
|
||||
const messageParams = {
|
||||
message: currentMessages[0]?.parts || [],
|
||||
config: {
|
||||
abortSignal: roundAbortController.signal,
|
||||
tools: [{ functionDeclarations: toolsList }],
|
||||
},
|
||||
};
|
||||
const messageParams = {
|
||||
message: currentMessages[0]?.parts || [],
|
||||
config: {
|
||||
abortSignal: roundAbortController.signal,
|
||||
tools: [{ functionDeclarations: toolsList }],
|
||||
},
|
||||
};
|
||||
|
||||
const roundStreamStart = Date.now();
|
||||
const responseStream = await chat.sendMessageStream(
|
||||
this.modelConfig.model ||
|
||||
this.runtimeContext.getModel() ||
|
||||
DEFAULT_QWEN_MODEL,
|
||||
messageParams,
|
||||
promptId,
|
||||
);
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_START, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
promptId,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundEvent);
|
||||
|
||||
const functionCalls: FunctionCall[] = [];
|
||||
let roundText = '';
|
||||
let roundThoughtText = '';
|
||||
let lastUsage: GenerateContentResponseUsageMetadata | undefined =
|
||||
undefined;
|
||||
let currentResponseId: string | undefined = undefined;
|
||||
let wasOutputTruncated = false;
|
||||
|
||||
for await (const streamEvent of responseStream) {
|
||||
if (roundAbortController.signal.aborted) {
|
||||
abortController.signal.removeEventListener('abort', onParentAbort);
|
||||
return {
|
||||
text: finalText,
|
||||
terminateMode: AgentTerminateMode.CANCELLED,
|
||||
turnsUsed: turnCounter,
|
||||
};
|
||||
}
|
||||
|
||||
// Handle retry events — reset all per-attempt state so a successful
|
||||
// retry does not inherit stale data (e.g. wasOutputTruncated) from a
|
||||
// previous attempt that may have hit MAX_TOKENS.
|
||||
if (streamEvent.type === 'retry') {
|
||||
functionCalls.length = 0;
|
||||
roundText = '';
|
||||
roundThoughtText = '';
|
||||
lastUsage = undefined;
|
||||
currentResponseId = undefined;
|
||||
wasOutputTruncated = false;
|
||||
continue;
|
||||
}
|
||||
|
||||
// GeminiChat already mutated its own history; surface to the debug
|
||||
// log so subagent compactions show up alongside the main session's.
|
||||
if (streamEvent.type === 'compressed') {
|
||||
this.runtimeContext
|
||||
.getDebugLogger()
|
||||
.debug(
|
||||
`[AGENT-COMPACT] subagent=${this.subagentId} round=${turnCounter} ` +
|
||||
`tokens ${streamEvent.info.originalTokenCount} -> ${streamEvent.info.newTokenCount}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Handle chunk events
|
||||
if (streamEvent.type === 'chunk') {
|
||||
const resp = streamEvent.value;
|
||||
// Track the response ID for tool call correlation
|
||||
if (resp.responseId) {
|
||||
currentResponseId = resp.responseId;
|
||||
}
|
||||
if (resp.functionCalls) functionCalls.push(...resp.functionCalls);
|
||||
if (resp.candidates?.[0]?.finishReason === FinishReason.MAX_TOKENS) {
|
||||
wasOutputTruncated = true;
|
||||
}
|
||||
const content = resp.candidates?.[0]?.content;
|
||||
const parts = content?.parts || [];
|
||||
for (const p of parts) {
|
||||
const txt = p.text;
|
||||
const isThought = p.thought ?? false;
|
||||
if (txt && isThought) roundThoughtText += txt;
|
||||
if (txt && !isThought) roundText += txt;
|
||||
if (txt)
|
||||
this.eventEmitter?.emit(AgentEventType.STREAM_TEXT, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
text: txt,
|
||||
thought: isThought,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
}
|
||||
if (resp.usageMetadata) lastUsage = resp.usageMetadata;
|
||||
}
|
||||
}
|
||||
|
||||
if (roundText || roundThoughtText) {
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_TEXT, {
|
||||
const roundStreamStart = Date.now();
|
||||
const responseStream = await chat.sendMessageStream(
|
||||
this.modelConfig.model ||
|
||||
this.runtimeContext.getModel() ||
|
||||
DEFAULT_QWEN_MODEL,
|
||||
messageParams,
|
||||
promptId,
|
||||
);
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_START, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
text: roundText,
|
||||
thoughtText: roundThoughtText,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundTextEvent);
|
||||
}
|
||||
|
||||
this.executionStats.rounds = turnCounter;
|
||||
this.stats.setRounds(turnCounter);
|
||||
|
||||
durationMin = (Date.now() - startTime) / (1000 * 60);
|
||||
if (options?.maxTimeMinutes && durationMin >= options.maxTimeMinutes) {
|
||||
abortController.signal.removeEventListener('abort', onParentAbort);
|
||||
terminateMode = AgentTerminateMode.TIMEOUT;
|
||||
break;
|
||||
}
|
||||
|
||||
// Update token usage if available
|
||||
if (lastUsage) {
|
||||
this.recordTokenUsage(lastUsage, turnCounter, roundStreamStart);
|
||||
}
|
||||
|
||||
if (functionCalls.length > 0) {
|
||||
currentMessages = await this.processFunctionCalls(
|
||||
functionCalls,
|
||||
roundAbortController,
|
||||
promptId,
|
||||
turnCounter,
|
||||
toolsList,
|
||||
currentResponseId,
|
||||
wasOutputTruncated,
|
||||
);
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundEvent);
|
||||
|
||||
const externalInputs = this.drainExternalInputs(options);
|
||||
if (externalInputs.length > 0) {
|
||||
// Append to the tool-response user message so external input rides
|
||||
// alongside the tool results the model is about to see.
|
||||
// processFunctionCalls always returns exactly one user-role entry.
|
||||
const last = currentMessages[currentMessages.length - 1];
|
||||
last.parts!.push(...this.externalInputsToParts(externalInputs, true));
|
||||
// Emit one event per injection so observers (e.g. the JSONL
|
||||
// transcript writer) can persist each external message as a
|
||||
// user-role record. The framing prefix is stripped — the prefix
|
||||
// is a model-facing detail, not part of the original message.
|
||||
this.emitExternalInputEvents(externalInputs);
|
||||
}
|
||||
} else {
|
||||
const immediateExternalInputs = this.drainExternalInputs(options);
|
||||
if (immediateExternalInputs.length > 0) {
|
||||
currentMessages = this.externalInputsToContent(
|
||||
immediateExternalInputs,
|
||||
);
|
||||
this.emitExternalInputEvents(immediateExternalInputs);
|
||||
} else if (options?.shouldWaitForExternalMessages?.()) {
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_END, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
promptId,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundEvent);
|
||||
abortController.signal.removeEventListener('abort', onParentAbort);
|
||||
const functionCalls: FunctionCall[] = [];
|
||||
let roundText = '';
|
||||
let roundThoughtText = '';
|
||||
let lastUsage: GenerateContentResponseUsageMetadata | undefined =
|
||||
undefined;
|
||||
let currentResponseId: string | undefined = undefined;
|
||||
let wasOutputTruncated = false;
|
||||
|
||||
const waitResult = await this.waitForExternalInputs(
|
||||
options,
|
||||
abortController,
|
||||
startTime,
|
||||
turnCounter,
|
||||
);
|
||||
if (waitResult.terminateMode) {
|
||||
finalText = roundText.trim();
|
||||
terminateMode = waitResult.terminateMode;
|
||||
break;
|
||||
for await (const streamEvent of responseStream) {
|
||||
if (roundAbortController.signal.aborted) {
|
||||
return {
|
||||
text: finalText,
|
||||
terminateMode: AgentTerminateMode.CANCELLED,
|
||||
turnsUsed: turnCounter,
|
||||
};
|
||||
}
|
||||
if (waitResult.inputs.length > 0) {
|
||||
currentMessages = this.externalInputsToContent(waitResult.inputs);
|
||||
this.emitExternalInputEvents(waitResult.inputs);
|
||||
|
||||
// Handle retry events — reset all per-attempt state so a successful
|
||||
// retry does not inherit stale data (e.g. wasOutputTruncated) from a
|
||||
// previous attempt that may have hit MAX_TOKENS.
|
||||
if (streamEvent.type === 'retry') {
|
||||
functionCalls.length = 0;
|
||||
roundText = '';
|
||||
roundThoughtText = '';
|
||||
lastUsage = undefined;
|
||||
currentResponseId = undefined;
|
||||
wasOutputTruncated = false;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (roundText && roundText.trim().length > 0) {
|
||||
finalText = roundText.trim();
|
||||
break;
|
||||
// GeminiChat already mutated its own history; surface to the debug
|
||||
// log so subagent compactions show up alongside the main session's.
|
||||
if (streamEvent.type === 'compressed') {
|
||||
this.runtimeContext
|
||||
.getDebugLogger()
|
||||
.debug(
|
||||
`[AGENT-COMPACT] subagent=${this.subagentId} round=${turnCounter} ` +
|
||||
`tokens ${streamEvent.info.originalTokenCount} -> ${streamEvent.info.newTokenCount}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Handle chunk events
|
||||
if (streamEvent.type === 'chunk') {
|
||||
const resp = streamEvent.value;
|
||||
// Track the response ID for tool call correlation
|
||||
if (resp.responseId) {
|
||||
currentResponseId = resp.responseId;
|
||||
}
|
||||
if (resp.functionCalls) functionCalls.push(...resp.functionCalls);
|
||||
if (
|
||||
resp.candidates?.[0]?.finishReason === FinishReason.MAX_TOKENS
|
||||
) {
|
||||
wasOutputTruncated = true;
|
||||
}
|
||||
const content = resp.candidates?.[0]?.content;
|
||||
const parts = content?.parts || [];
|
||||
for (const p of parts) {
|
||||
const txt = p.text;
|
||||
const isThought = p.thought ?? false;
|
||||
if (txt && isThought) roundThoughtText += txt;
|
||||
if (txt && !isThought) roundText += txt;
|
||||
if (txt)
|
||||
this.eventEmitter?.emit(AgentEventType.STREAM_TEXT, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
text: txt,
|
||||
thought: isThought,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
}
|
||||
if (resp.usageMetadata) lastUsage = resp.usageMetadata;
|
||||
}
|
||||
}
|
||||
|
||||
if (roundText || roundThoughtText) {
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_TEXT, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
text: roundText,
|
||||
thoughtText: roundThoughtText,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundTextEvent);
|
||||
}
|
||||
|
||||
this.executionStats.rounds = turnCounter;
|
||||
this.stats.setRounds(turnCounter);
|
||||
|
||||
durationMin = (Date.now() - startTime) / (1000 * 60);
|
||||
if (options?.maxTimeMinutes && durationMin >= options.maxTimeMinutes) {
|
||||
terminateMode = AgentTerminateMode.TIMEOUT;
|
||||
break;
|
||||
}
|
||||
|
||||
// Update token usage if available
|
||||
if (lastUsage) {
|
||||
this.recordTokenUsage(lastUsage, turnCounter, roundStreamStart);
|
||||
}
|
||||
|
||||
if (functionCalls.length > 0) {
|
||||
currentMessages = await this.processFunctionCalls(
|
||||
functionCalls,
|
||||
roundAbortController,
|
||||
promptId,
|
||||
turnCounter,
|
||||
toolsList,
|
||||
currentResponseId,
|
||||
wasOutputTruncated,
|
||||
);
|
||||
|
||||
const externalInputs = this.drainExternalInputs(options);
|
||||
if (externalInputs.length > 0) {
|
||||
// Append to the tool-response user message so external input rides
|
||||
// alongside the tool results the model is about to see.
|
||||
// processFunctionCalls always returns exactly one user-role entry.
|
||||
const last = currentMessages[currentMessages.length - 1];
|
||||
last.parts!.push(
|
||||
...this.externalInputsToParts(externalInputs, true),
|
||||
);
|
||||
// Emit one event per injection so observers (e.g. the JSONL
|
||||
// transcript writer) can persist each external message as a
|
||||
// user-role record. The framing prefix is stripped — the prefix
|
||||
// is a model-facing detail, not part of the original message.
|
||||
this.emitExternalInputEvents(externalInputs);
|
||||
}
|
||||
currentMessages = [
|
||||
{
|
||||
role: 'user',
|
||||
parts: [
|
||||
{
|
||||
text: 'Please provide the final result now and stop calling tools.',
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
continue;
|
||||
} else {
|
||||
// No tool calls — treat this as the model's final answer.
|
||||
if (roundText && roundText.trim().length > 0) {
|
||||
finalText = roundText.trim();
|
||||
// Emit ROUND_END for the final round so all consumers see it.
|
||||
// Previously this was skipped, requiring AgentInteractive to
|
||||
// compensate with an explicit flushStreamBuffers() call.
|
||||
const immediateExternalInputs = this.drainExternalInputs(options);
|
||||
if (immediateExternalInputs.length > 0) {
|
||||
currentMessages = this.externalInputsToContent(
|
||||
immediateExternalInputs,
|
||||
);
|
||||
this.emitExternalInputEvents(immediateExternalInputs);
|
||||
} else if (options?.shouldWaitForExternalMessages?.()) {
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_END, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
promptId,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundEvent);
|
||||
// Clean up before breaking
|
||||
abortController.signal.removeEventListener('abort', onParentAbort);
|
||||
// null terminateMode = normal text completion
|
||||
break;
|
||||
|
||||
const waitResult = await this.waitForExternalInputs(
|
||||
options,
|
||||
abortController,
|
||||
startTime,
|
||||
turnCounter,
|
||||
);
|
||||
if (waitResult.terminateMode) {
|
||||
finalText = roundText.trim();
|
||||
terminateMode = waitResult.terminateMode;
|
||||
break;
|
||||
}
|
||||
if (waitResult.inputs.length > 0) {
|
||||
currentMessages = this.externalInputsToContent(waitResult.inputs);
|
||||
this.emitExternalInputEvents(waitResult.inputs);
|
||||
continue;
|
||||
}
|
||||
|
||||
if (roundText && roundText.trim().length > 0) {
|
||||
finalText = roundText.trim();
|
||||
break;
|
||||
}
|
||||
currentMessages = [
|
||||
{
|
||||
role: 'user',
|
||||
parts: [
|
||||
{
|
||||
text: 'Please provide the final result now and stop calling tools.',
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
continue;
|
||||
} else {
|
||||
// No tool calls — treat this as the model's final answer.
|
||||
if (roundText && roundText.trim().length > 0) {
|
||||
finalText = roundText.trim();
|
||||
// Emit ROUND_END for the final round so all consumers see it.
|
||||
// Previously this was skipped, requiring AgentInteractive to
|
||||
// compensate with an explicit flushStreamBuffers() call.
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_END, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
promptId,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundEvent);
|
||||
// null terminateMode = normal text completion
|
||||
break;
|
||||
}
|
||||
// Otherwise, nudge the model to finalize a result.
|
||||
currentMessages = [
|
||||
{
|
||||
role: 'user',
|
||||
parts: [
|
||||
{
|
||||
text: 'Please provide the final result now and stop calling tools.',
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
}
|
||||
// Otherwise, nudge the model to finalize a result.
|
||||
currentMessages = [
|
||||
{
|
||||
role: 'user',
|
||||
parts: [
|
||||
{
|
||||
text: 'Please provide the final result now and stop calling tools.',
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_END, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
promptId,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundEvent);
|
||||
} finally {
|
||||
// Reverse-cleanup fires whether the iteration ended normally, broke,
|
||||
// returned, or threw — preventing parent-listener accumulation on
|
||||
// long-running parents like the per-message roundAbortController in
|
||||
// AgentInteractive or the session-lived externalSignal in headless.
|
||||
roundAbortController.abort();
|
||||
}
|
||||
|
||||
this.eventEmitter?.emit(AgentEventType.ROUND_END, {
|
||||
subagentId: this.subagentId,
|
||||
round: turnCounter,
|
||||
promptId,
|
||||
timestamp: Date.now(),
|
||||
} as AgentRoundEvent);
|
||||
|
||||
// Clean up the per-round listener before the next iteration
|
||||
abortController.signal.removeEventListener('abort', onParentAbort);
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
@ -943,12 +945,7 @@ export class AgentCore {
|
|||
return { inputs: [] };
|
||||
}
|
||||
|
||||
const waitAbortController = new AbortController();
|
||||
const onAbort = () => waitAbortController.abort();
|
||||
abortController.signal.addEventListener('abort', onAbort, { once: true });
|
||||
if (abortController.signal.aborted) {
|
||||
waitAbortController.abort();
|
||||
}
|
||||
const waitAbortController = createChildAbortController(abortController);
|
||||
let timedOut = false;
|
||||
let timeout: ReturnType<typeof setTimeout> | undefined;
|
||||
if (remainingTimeMs !== undefined) {
|
||||
|
|
@ -985,7 +982,9 @@ export class AgentCore {
|
|||
throw error;
|
||||
} finally {
|
||||
if (timeout) clearTimeout(timeout);
|
||||
abortController.signal.removeEventListener('abort', onAbort);
|
||||
// Aborting the child fires reverse-cleanup of its listener on the
|
||||
// parent; no-op if it already aborted from the parent or the timeout.
|
||||
waitAbortController.abort();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1325,17 +1324,23 @@ export class AgentCore {
|
|||
}
|
||||
};
|
||||
abortController.signal.addEventListener('abort', onAbort, { once: true });
|
||||
try {
|
||||
// If already aborted before the listener was registered, resolve
|
||||
// immediately to avoid blocking forever.
|
||||
if (abortController.signal.aborted) {
|
||||
onAbort();
|
||||
}
|
||||
|
||||
// If already aborted before the listener was registered, resolve
|
||||
// immediately to avoid blocking forever.
|
||||
if (abortController.signal.aborted) {
|
||||
onAbort();
|
||||
await scheduler.schedule(requests, abortController.signal);
|
||||
await batchDone;
|
||||
} finally {
|
||||
// Always remove `onAbort` — otherwise a throw from scheduler.schedule
|
||||
// or batchDone would leak it on the round controller, and the round's
|
||||
// outer try/finally `.abort()` would later fire spurious cancellation
|
||||
// TOOL_RESULT events for every un-emitted callId (corrupting the
|
||||
// transcript and misleading the model on the next round).
|
||||
abortController.signal.removeEventListener('abort', onAbort);
|
||||
}
|
||||
|
||||
await scheduler.schedule(requests, abortController.signal);
|
||||
await batchDone;
|
||||
|
||||
abortController.signal.removeEventListener('abort', onAbort);
|
||||
}
|
||||
|
||||
// If all tool calls failed, inform the model so it can re-evaluate.
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@
|
|||
import type { Content } from '@google/genai';
|
||||
import type { Config } from '../../config/config.js';
|
||||
import type { RuntimeContentGeneratorView } from './agent-context.js';
|
||||
import { createChildAbortController } from '../../utils/abortController.js';
|
||||
import { createDebugLogger } from '../../utils/debugLogger.js';
|
||||
import type {
|
||||
AgentEventEmitter,
|
||||
|
|
@ -225,116 +226,117 @@ export class AgentHeadless {
|
|||
return;
|
||||
}
|
||||
|
||||
// Set up abort signal propagation
|
||||
const abortController = new AbortController();
|
||||
const onExternalAbort = () => {
|
||||
abortController.abort();
|
||||
};
|
||||
if (externalSignal) {
|
||||
externalSignal.addEventListener('abort', onExternalAbort);
|
||||
}
|
||||
if (externalSignal?.aborted) {
|
||||
abortController.abort();
|
||||
}
|
||||
|
||||
const toolsList = await this.core.prepareTools();
|
||||
|
||||
const initialMessages =
|
||||
initialMessagesOverride && initialMessagesOverride.length > 0
|
||||
? initialMessagesOverride
|
||||
: [{ role: 'user' as const, parts: [{ text: initialTaskText }] }];
|
||||
|
||||
const startTime = Date.now();
|
||||
this.core.executionStats.startTimeMs = startTime;
|
||||
this.core.stats.start(startTime);
|
||||
// Child controller propagates from optional externalSignal and auto-cleans
|
||||
// its parent listener when aborted (see utils/abortController.ts).
|
||||
const abortController = createChildAbortController(externalSignal);
|
||||
|
||||
try {
|
||||
// Emit start event
|
||||
this.core.eventEmitter?.emit(AgentEventType.START, {
|
||||
subagentId: this.core.subagentId,
|
||||
name: this.core.name,
|
||||
model:
|
||||
this.core.modelConfig.model ||
|
||||
this.core.runtimeContext.getModel() ||
|
||||
DEFAULT_QWEN_MODEL,
|
||||
tools: (this.core.toolConfig?.tools || ['*']).map((t) =>
|
||||
typeof t === 'string' ? t : t.name,
|
||||
),
|
||||
timestamp: Date.now(),
|
||||
} as AgentStartEvent);
|
||||
const toolsList = await this.core.prepareTools();
|
||||
|
||||
// Log telemetry for subagent start
|
||||
const startEvent = new SubagentExecutionEvent(this.core.name, 'started');
|
||||
logSubagentExecution(this.core.runtimeContext, startEvent);
|
||||
const initialMessages =
|
||||
initialMessagesOverride && initialMessagesOverride.length > 0
|
||||
? initialMessagesOverride
|
||||
: [{ role: 'user' as const, parts: [{ text: initialTaskText }] }];
|
||||
|
||||
// Delegate to AgentCore's reasoning loop
|
||||
const result = await this.core.runReasoningLoop(
|
||||
chat,
|
||||
initialMessages,
|
||||
toolsList,
|
||||
abortController,
|
||||
{
|
||||
maxTurns: this.core.runConfig.max_turns,
|
||||
maxTimeMinutes: this.core.runConfig.max_time_minutes,
|
||||
startTimeMs: startTime,
|
||||
getExternalMessages: this.externalMessageProvider,
|
||||
waitForExternalMessages: this.externalMessageWaiter,
|
||||
shouldWaitForExternalMessages: this.externalMessageWaitPredicate,
|
||||
},
|
||||
);
|
||||
const startTime = Date.now();
|
||||
this.core.executionStats.startTimeMs = startTime;
|
||||
this.core.stats.start(startTime);
|
||||
|
||||
this.finalText = result.text;
|
||||
this.terminateMode = result.terminateMode ?? AgentTerminateMode.GOAL;
|
||||
} catch (error) {
|
||||
debugLogger.error('Error during subagent execution:', error);
|
||||
this.terminateMode = AgentTerminateMode.ERROR;
|
||||
this.core.eventEmitter?.emit(AgentEventType.ERROR, {
|
||||
subagentId: this.core.subagentId,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
timestamp: Date.now(),
|
||||
} as AgentErrorEvent);
|
||||
|
||||
throw error;
|
||||
} finally {
|
||||
if (externalSignal) {
|
||||
externalSignal.removeEventListener('abort', onExternalAbort);
|
||||
}
|
||||
this.core.executionStats.totalDurationMs = Date.now() - startTime;
|
||||
const summary = this.core.stats.getSummary(Date.now());
|
||||
this.core.eventEmitter?.emit(AgentEventType.FINISH, {
|
||||
subagentId: this.core.subagentId,
|
||||
terminateReason: this.terminateMode,
|
||||
timestamp: Date.now(),
|
||||
rounds: summary.rounds,
|
||||
totalDurationMs: summary.totalDurationMs,
|
||||
totalToolCalls: summary.totalToolCalls,
|
||||
successfulToolCalls: summary.successfulToolCalls,
|
||||
failedToolCalls: summary.failedToolCalls,
|
||||
inputTokens: summary.inputTokens,
|
||||
outputTokens: summary.outputTokens,
|
||||
totalTokens: summary.totalTokens,
|
||||
} as AgentFinishEvent);
|
||||
|
||||
const completionEvent = new SubagentExecutionEvent(
|
||||
this.core.name,
|
||||
this.terminateMode === AgentTerminateMode.GOAL ? 'completed' : 'failed',
|
||||
{
|
||||
terminate_reason: this.terminateMode,
|
||||
result: this.finalText,
|
||||
execution_summary: this.core.stats.formatCompact(
|
||||
'Subagent execution completed',
|
||||
try {
|
||||
// Emit start event
|
||||
this.core.eventEmitter?.emit(AgentEventType.START, {
|
||||
subagentId: this.core.subagentId,
|
||||
name: this.core.name,
|
||||
model:
|
||||
this.core.modelConfig.model ||
|
||||
this.core.runtimeContext.getModel() ||
|
||||
DEFAULT_QWEN_MODEL,
|
||||
tools: (this.core.toolConfig?.tools || ['*']).map((t) =>
|
||||
typeof t === 'string' ? t : t.name,
|
||||
),
|
||||
},
|
||||
);
|
||||
logSubagentExecution(this.core.runtimeContext, completionEvent);
|
||||
timestamp: Date.now(),
|
||||
} as AgentStartEvent);
|
||||
|
||||
await this.core.hooks?.onStop?.({
|
||||
subagentId: this.core.subagentId,
|
||||
name: this.core.name,
|
||||
terminateReason: this.terminateMode,
|
||||
summary: summary as unknown as Record<string, unknown>,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
// Log telemetry for subagent start
|
||||
const startEvent = new SubagentExecutionEvent(
|
||||
this.core.name,
|
||||
'started',
|
||||
);
|
||||
logSubagentExecution(this.core.runtimeContext, startEvent);
|
||||
|
||||
// Delegate to AgentCore's reasoning loop
|
||||
const result = await this.core.runReasoningLoop(
|
||||
chat,
|
||||
initialMessages,
|
||||
toolsList,
|
||||
abortController,
|
||||
{
|
||||
maxTurns: this.core.runConfig.max_turns,
|
||||
maxTimeMinutes: this.core.runConfig.max_time_minutes,
|
||||
startTimeMs: startTime,
|
||||
getExternalMessages: this.externalMessageProvider,
|
||||
waitForExternalMessages: this.externalMessageWaiter,
|
||||
shouldWaitForExternalMessages: this.externalMessageWaitPredicate,
|
||||
},
|
||||
);
|
||||
|
||||
this.finalText = result.text;
|
||||
this.terminateMode = result.terminateMode ?? AgentTerminateMode.GOAL;
|
||||
} catch (error) {
|
||||
debugLogger.error('Error during subagent execution:', error);
|
||||
this.terminateMode = AgentTerminateMode.ERROR;
|
||||
this.core.eventEmitter?.emit(AgentEventType.ERROR, {
|
||||
subagentId: this.core.subagentId,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
timestamp: Date.now(),
|
||||
} as AgentErrorEvent);
|
||||
|
||||
throw error;
|
||||
} finally {
|
||||
this.core.executionStats.totalDurationMs = Date.now() - startTime;
|
||||
const summary = this.core.stats.getSummary(Date.now());
|
||||
this.core.eventEmitter?.emit(AgentEventType.FINISH, {
|
||||
subagentId: this.core.subagentId,
|
||||
terminateReason: this.terminateMode,
|
||||
timestamp: Date.now(),
|
||||
rounds: summary.rounds,
|
||||
totalDurationMs: summary.totalDurationMs,
|
||||
totalToolCalls: summary.totalToolCalls,
|
||||
successfulToolCalls: summary.successfulToolCalls,
|
||||
failedToolCalls: summary.failedToolCalls,
|
||||
inputTokens: summary.inputTokens,
|
||||
outputTokens: summary.outputTokens,
|
||||
totalTokens: summary.totalTokens,
|
||||
} as AgentFinishEvent);
|
||||
|
||||
const completionEvent = new SubagentExecutionEvent(
|
||||
this.core.name,
|
||||
this.terminateMode === AgentTerminateMode.GOAL
|
||||
? 'completed'
|
||||
: 'failed',
|
||||
{
|
||||
terminate_reason: this.terminateMode,
|
||||
result: this.finalText,
|
||||
execution_summary: this.core.stats.formatCompact(
|
||||
'Subagent execution completed',
|
||||
),
|
||||
},
|
||||
);
|
||||
logSubagentExecution(this.core.runtimeContext, completionEvent);
|
||||
|
||||
await this.core.hooks?.onStop?.({
|
||||
subagentId: this.core.subagentId,
|
||||
name: this.core.name,
|
||||
terminateReason: this.terminateMode,
|
||||
summary: summary as unknown as Record<string, unknown>,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
// Outer finally guarantees the child's parent-signal listener is
|
||||
// detached even if prepareTools or initialMessages prep throws before
|
||||
// the inner try runs.
|
||||
abortController.abort();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,6 +11,10 @@
|
|||
* state (messages, pending approvals, live outputs) that the UI reads.
|
||||
*/
|
||||
|
||||
import {
|
||||
createAbortController,
|
||||
createChildAbortController,
|
||||
} from '../../utils/abortController.js';
|
||||
import { createDebugLogger } from '../../utils/debugLogger.js';
|
||||
import { type AgentEventEmitter, AgentEventType } from './agent-events.js';
|
||||
import type {
|
||||
|
|
@ -57,7 +61,7 @@ export class AgentInteractive {
|
|||
private error: string | undefined;
|
||||
private lastRoundError: string | undefined;
|
||||
private executionPromise: Promise<void> | undefined;
|
||||
private masterAbortController = new AbortController();
|
||||
private masterAbortController = createAbortController();
|
||||
private roundAbortController: AbortController | undefined;
|
||||
private chat: GeminiChat | undefined;
|
||||
private toolsList: FunctionDeclaration[] = [];
|
||||
|
|
@ -150,14 +154,9 @@ export class AgentInteractive {
|
|||
this.setStatus(AgentStatus.RUNNING);
|
||||
this.lastRoundError = undefined;
|
||||
this.roundCancelledByUser = false;
|
||||
this.roundAbortController = new AbortController();
|
||||
|
||||
// Propagate master abort to round
|
||||
const onMasterAbort = () => this.roundAbortController?.abort();
|
||||
this.masterAbortController.signal.addEventListener('abort', onMasterAbort);
|
||||
if (this.masterAbortController.signal.aborted) {
|
||||
this.roundAbortController.abort();
|
||||
}
|
||||
this.roundAbortController = createChildAbortController(
|
||||
this.masterAbortController,
|
||||
);
|
||||
|
||||
try {
|
||||
const initialMessages = [
|
||||
|
|
@ -196,10 +195,10 @@ export class AgentInteractive {
|
|||
debugLogger.error('AgentInteractive round error:', err);
|
||||
this.addMessage('info', errorMessage, { metadata: { level: 'error' } });
|
||||
} finally {
|
||||
this.masterAbortController.signal.removeEventListener(
|
||||
'abort',
|
||||
onMasterAbort,
|
||||
);
|
||||
// Helper's reverse-cleanup detaches the parent listener automatically
|
||||
// when the round controller aborts; abort here so cleanup fires whether
|
||||
// or not the round was already cancelled.
|
||||
this.roundAbortController?.abort();
|
||||
this.roundAbortController = undefined;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -120,7 +120,7 @@ export class MessageBus extends EventEmitter {
|
|||
};
|
||||
|
||||
if (signal) {
|
||||
signal.addEventListener('abort', abortHandler);
|
||||
signal.addEventListener('abort', abortHandler, { once: true });
|
||||
}
|
||||
|
||||
const responseHandler = (response: TResponse) => {
|
||||
|
|
|
|||
|
|
@ -4,7 +4,6 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { setMaxListeners } from 'node:events';
|
||||
import type OpenAI from 'openai';
|
||||
import {
|
||||
type GenerateContentParameters,
|
||||
|
|
@ -20,23 +19,6 @@ import type { PipelineConfig, RequestContext } from './types.js';
|
|||
import { redactProxyError } from '../../utils/runtimeFetchOptions.js';
|
||||
import { runtimeDiagnostics } from '../../utils/runtimeDiagnostics.js';
|
||||
|
||||
/**
|
||||
* The OpenAI SDK adds an abort listener for every `chat.completions.create`
|
||||
* call, and several layers (retryWithBackoff, LoggingContentGenerator, the
|
||||
* SDK's internal stream/fetch wrappers) each register their own listeners
|
||||
* on the same per-request AbortSignal. With 5 retries the count comfortably
|
||||
* exceeds Node's default 10-listener leak warning — and on top of that,
|
||||
* concurrent code paths (e.g., recap + followup speculation) can share or
|
||||
* compose signals, pushing it past any small cap.
|
||||
*
|
||||
* These signals are per-request and short-lived (GC'd when the request
|
||||
* settles), so accumulation here is structural, not a memory leak. Disable
|
||||
* the warning entirely for them. Idempotent.
|
||||
*/
|
||||
function raiseAbortListenerCap(signal: AbortSignal | undefined): void {
|
||||
if (signal) setMaxListeners(0, signal);
|
||||
}
|
||||
|
||||
/**
|
||||
* Error thrown when the API returns an error embedded as stream content
|
||||
* instead of a proper HTTP error. Some providers (e.g., certain OpenAI-compatible
|
||||
|
|
@ -65,7 +47,6 @@ export class ContentGenerationPipeline {
|
|||
request: GenerateContentParameters,
|
||||
userPromptId: string,
|
||||
): Promise<GenerateContentResponse> {
|
||||
raiseAbortListenerCap(request.config?.abortSignal);
|
||||
return this.executeWithErrorHandling(
|
||||
request,
|
||||
userPromptId,
|
||||
|
|
@ -93,7 +74,6 @@ export class ContentGenerationPipeline {
|
|||
request: GenerateContentParameters,
|
||||
userPromptId: string,
|
||||
): Promise<AsyncGenerator<GenerateContentResponse>> {
|
||||
raiseAbortListenerCap(request.config?.abortSignal);
|
||||
return this.executeWithErrorHandling(
|
||||
request,
|
||||
userPromptId,
|
||||
|
|
|
|||
|
|
@ -1,111 +0,0 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen Team
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { createCombinedAbortSignal } from './combinedAbortSignal.js';
|
||||
|
||||
describe('createCombinedAbortSignal', () => {
|
||||
it('should return a non-aborted signal by default', () => {
|
||||
const { signal, cleanup } = createCombinedAbortSignal();
|
||||
expect(signal.aborted).toBe(false);
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it('should abort after timeout', async () => {
|
||||
const { signal, cleanup } = createCombinedAbortSignal(undefined, {
|
||||
timeoutMs: 50,
|
||||
});
|
||||
expect(signal.aborted).toBe(false);
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
expect(signal.aborted).toBe(true);
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it('should abort when external signal is aborted', () => {
|
||||
const externalController = new AbortController();
|
||||
const { signal, cleanup } = createCombinedAbortSignal(
|
||||
externalController.signal,
|
||||
);
|
||||
expect(signal.aborted).toBe(false);
|
||||
|
||||
externalController.abort();
|
||||
expect(signal.aborted).toBe(true);
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it('should abort immediately if external signal is already aborted', () => {
|
||||
const externalController = new AbortController();
|
||||
externalController.abort();
|
||||
|
||||
const { signal, cleanup } = createCombinedAbortSignal(
|
||||
externalController.signal,
|
||||
);
|
||||
expect(signal.aborted).toBe(true);
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it('should cleanup timeout timer', async () => {
|
||||
const { signal, cleanup } = createCombinedAbortSignal(undefined, {
|
||||
timeoutMs: 50,
|
||||
});
|
||||
|
||||
cleanup();
|
||||
|
||||
// Wait longer than timeout - should not abort because timer was cleared
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
expect(signal.aborted).toBe(false);
|
||||
});
|
||||
|
||||
it('should remove external abort listener on cleanup', () => {
|
||||
const externalController = new AbortController();
|
||||
const removeListenerSpy = vi.spyOn(
|
||||
externalController.signal,
|
||||
'removeEventListener',
|
||||
);
|
||||
const { signal, cleanup } = createCombinedAbortSignal(
|
||||
externalController.signal,
|
||||
);
|
||||
|
||||
cleanup();
|
||||
externalController.abort();
|
||||
|
||||
expect(removeListenerSpy).toHaveBeenCalledWith(
|
||||
'abort',
|
||||
expect.any(Function),
|
||||
);
|
||||
expect(signal.aborted).toBe(false);
|
||||
});
|
||||
|
||||
it('should work with both external signal and timeout', async () => {
|
||||
const externalController = new AbortController();
|
||||
const { signal, cleanup } = createCombinedAbortSignal(
|
||||
externalController.signal,
|
||||
{ timeoutMs: 200 },
|
||||
);
|
||||
|
||||
// Abort external signal before timeout
|
||||
externalController.abort();
|
||||
expect(signal.aborted).toBe(true);
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it('should timeout before external signal', async () => {
|
||||
const externalController = new AbortController();
|
||||
const { signal, cleanup } = createCombinedAbortSignal(
|
||||
externalController.signal,
|
||||
{ timeoutMs: 50 },
|
||||
);
|
||||
|
||||
// Wait for timeout
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
expect(signal.aborted).toBe(true);
|
||||
|
||||
// External signal is still not aborted
|
||||
expect(externalController.signal.aborted).toBe(false);
|
||||
cleanup();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,57 +0,0 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen Team
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
/**
|
||||
* Create a combined AbortSignal that aborts when either:
|
||||
* - The provided external signal is aborted, OR
|
||||
* - The timeout is reached
|
||||
*
|
||||
* @param externalSignal - Optional external AbortSignal to combine
|
||||
* @param timeoutMs - Timeout in milliseconds
|
||||
* @returns Object containing the combined signal and a cleanup function
|
||||
*/
|
||||
export function createCombinedAbortSignal(
|
||||
externalSignal?: AbortSignal,
|
||||
options?: { timeoutMs?: number },
|
||||
): { signal: AbortSignal; cleanup: () => void } {
|
||||
const controller = new AbortController();
|
||||
|
||||
const timeoutMs = options?.timeoutMs;
|
||||
|
||||
// Set up timeout
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
if (timeoutMs !== undefined && timeoutMs > 0) {
|
||||
timeoutId = setTimeout(() => {
|
||||
controller.abort();
|
||||
}, timeoutMs);
|
||||
}
|
||||
|
||||
// Listen to external signal
|
||||
let abortHandler: (() => void) | undefined;
|
||||
if (externalSignal) {
|
||||
if (externalSignal.aborted) {
|
||||
controller.abort();
|
||||
} else {
|
||||
abortHandler = () => {
|
||||
controller.abort();
|
||||
};
|
||||
externalSignal.addEventListener('abort', abortHandler, { once: true });
|
||||
}
|
||||
}
|
||||
|
||||
const cleanup = () => {
|
||||
if (timeoutId !== undefined) {
|
||||
clearTimeout(timeoutId);
|
||||
timeoutId = undefined;
|
||||
}
|
||||
if (externalSignal && abortHandler) {
|
||||
externalSignal.removeEventListener('abort', abortHandler);
|
||||
abortHandler = undefined;
|
||||
}
|
||||
};
|
||||
|
||||
return { signal: controller.signal, cleanup };
|
||||
}
|
||||
|
|
@ -234,7 +234,7 @@ export class FunctionHookRunner {
|
|||
abortHandler = () => {
|
||||
reject(new Error('Function hook execution aborted'));
|
||||
};
|
||||
signal.addEventListener('abort', abortHandler);
|
||||
signal.addEventListener('abort', abortHandler, { once: true });
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -614,7 +614,7 @@ export class HookRunner {
|
|||
};
|
||||
|
||||
if (signal) {
|
||||
signal.addEventListener('abort', abortHandler);
|
||||
signal.addEventListener('abort', abortHandler, { once: true });
|
||||
}
|
||||
|
||||
// Send input to stdin
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@
|
|||
import { createDebugLogger } from '../utils/debugLogger.js';
|
||||
import { interpolateHeaders, interpolateUrl } from './envInterpolator.js';
|
||||
import { UrlValidator } from './urlValidator.js';
|
||||
import { createCombinedAbortSignal } from './combinedAbortSignal.js';
|
||||
import { combineAbortSignals } from '../utils/abortController.js';
|
||||
import { isBlockedAddress } from './ssrfGuard.js';
|
||||
import { lookup as dnsLookup } from 'dns';
|
||||
import type {
|
||||
|
|
@ -199,8 +199,8 @@ export class HttpHookRunner {
|
|||
const timeout = hookConfig.timeout
|
||||
? hookConfig.timeout * 1000
|
||||
: DEFAULT_HTTP_TIMEOUT;
|
||||
const { signal: combinedSignal, cleanup } = createCombinedAbortSignal(
|
||||
signal,
|
||||
const { signal: combinedSignal, cleanup } = combineAbortSignals(
|
||||
[signal],
|
||||
{ timeoutMs: timeout },
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
*/
|
||||
|
||||
import { z } from 'zod';
|
||||
import { createChildAbortController } from '../utils/abortController.js';
|
||||
import { createDebugLogger } from '../utils/debugLogger.js';
|
||||
import type {
|
||||
PromptHookConfig,
|
||||
|
|
@ -229,21 +230,14 @@ export class PromptHookRunner {
|
|||
},
|
||||
];
|
||||
|
||||
// Create internal AbortController to abort the request on timeout
|
||||
const internalAbortController = new AbortController();
|
||||
// Internal AbortController to abort the request on timeout. Use
|
||||
// createChildAbortController so parent-signal propagation gets `{once:true}`
|
||||
// + reverse cleanup automatically — the old manual addEventListener path
|
||||
// had no `{once:true}` and never removed the listener, leaking one
|
||||
// listener per prompt-hook invocation on the long-lived parent.
|
||||
const internalAbortController = createChildAbortController(signal);
|
||||
const internalSignal = internalAbortController.signal;
|
||||
|
||||
// Chain external signal to internal abort controller
|
||||
if (signal) {
|
||||
if (signal.aborted) {
|
||||
internalAbortController.abort();
|
||||
} else {
|
||||
signal.addEventListener('abort', () => {
|
||||
internalAbortController.abort();
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Create timeout promise that also aborts the request
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
const timeoutPromise = new Promise<never>((_, reject) => {
|
||||
|
|
@ -320,6 +314,9 @@ export class PromptHookRunner {
|
|||
if (timeoutId) {
|
||||
clearTimeout(timeoutId);
|
||||
}
|
||||
// Trigger reverse-cleanup of the parent-signal listener on the
|
||||
// success path; no-op if already aborted via parent/timeout.
|
||||
internalAbortController.abort();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
340
packages/core/src/utils/abortController.test.ts
Normal file
340
packages/core/src/utils/abortController.test.ts
Normal file
|
|
@ -0,0 +1,340 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { getEventListeners, getMaxListeners } from 'node:events';
|
||||
import {
|
||||
combineAbortSignals,
|
||||
createAbortController,
|
||||
createChildAbortController,
|
||||
} from './abortController.js';
|
||||
|
||||
describe('createAbortController', () => {
|
||||
it('sets a default max-listener cap of 50 on the signal', () => {
|
||||
const controller = createAbortController();
|
||||
expect(getMaxListeners(controller.signal)).toBe(50);
|
||||
});
|
||||
|
||||
it('honors a custom max-listener cap', () => {
|
||||
const controller = createAbortController(200);
|
||||
expect(getMaxListeners(controller.signal)).toBe(200);
|
||||
});
|
||||
|
||||
it('produces a working, abortable controller', () => {
|
||||
const controller = createAbortController();
|
||||
expect(controller.signal.aborted).toBe(false);
|
||||
controller.abort('done');
|
||||
expect(controller.signal.aborted).toBe(true);
|
||||
expect(controller.signal.reason).toBe('done');
|
||||
});
|
||||
});
|
||||
|
||||
describe('createChildAbortController', () => {
|
||||
it('aborts when the parent aborts and propagates the reason', () => {
|
||||
const parent = createAbortController();
|
||||
const child = createChildAbortController(parent);
|
||||
parent.abort('parent-reason');
|
||||
expect(child.signal.aborted).toBe(true);
|
||||
expect(child.signal.reason).toBe('parent-reason');
|
||||
});
|
||||
|
||||
it('does not abort the parent when the child aborts', () => {
|
||||
const parent = createAbortController();
|
||||
const child = createChildAbortController(parent);
|
||||
child.abort('child-reason');
|
||||
expect(child.signal.aborted).toBe(true);
|
||||
expect(parent.signal.aborted).toBe(false);
|
||||
});
|
||||
|
||||
it('aborts synchronously when the parent is already aborted (fast path)', () => {
|
||||
const parent = createAbortController();
|
||||
parent.abort('pre-aborted');
|
||||
const child = createChildAbortController(parent);
|
||||
expect(child.signal.aborted).toBe(true);
|
||||
expect(child.signal.reason).toBe('pre-aborted');
|
||||
// No listener should have been registered on the parent in the fast path.
|
||||
expect(getEventListeners(parent.signal, 'abort').length).toBe(0);
|
||||
});
|
||||
|
||||
it('removes its parent listener once the child has aborted (reverse cleanup)', () => {
|
||||
const parent = createAbortController();
|
||||
const child = createChildAbortController(parent);
|
||||
expect(getEventListeners(parent.signal, 'abort').length).toBe(1);
|
||||
child.abort();
|
||||
expect(getEventListeners(parent.signal, 'abort').length).toBe(0);
|
||||
});
|
||||
|
||||
it('removes its parent listener after parent abort fires (once: true)', () => {
|
||||
const parent = createAbortController();
|
||||
createChildAbortController(parent);
|
||||
expect(getEventListeners(parent.signal, 'abort').length).toBe(1);
|
||||
parent.abort();
|
||||
// The {once: true} listener should self-remove after firing.
|
||||
expect(getEventListeners(parent.signal, 'abort').length).toBe(0);
|
||||
});
|
||||
|
||||
it('does not accumulate listeners on a long-lived parent across many short-lived children', () => {
|
||||
const parent = createAbortController();
|
||||
for (let i = 0; i < 1000; i++) {
|
||||
const child = createChildAbortController(parent);
|
||||
child.abort();
|
||||
}
|
||||
expect(getEventListeners(parent.signal, 'abort').length).toBe(0);
|
||||
});
|
||||
|
||||
it('accepts an AbortSignal directly as the parent', () => {
|
||||
const parent = createAbortController();
|
||||
const child = createChildAbortController(parent.signal);
|
||||
parent.abort();
|
||||
expect(child.signal.aborted).toBe(true);
|
||||
});
|
||||
|
||||
it('returns a plain controller when the parent is undefined', () => {
|
||||
const child = createChildAbortController(undefined);
|
||||
expect(child.signal.aborted).toBe(false);
|
||||
child.abort('manual');
|
||||
expect(child.signal.aborted).toBe(true);
|
||||
});
|
||||
|
||||
it('forwards a custom maxListeners through to the child signal', () => {
|
||||
const parent = createAbortController();
|
||||
const child = createChildAbortController(parent, 123);
|
||||
expect(getMaxListeners(child.signal)).toBe(123);
|
||||
});
|
||||
});
|
||||
|
||||
describe('combineAbortSignals', () => {
|
||||
it('aborts when any input signal aborts', () => {
|
||||
const a = createAbortController();
|
||||
const b = createAbortController();
|
||||
const { signal } = combineAbortSignals([a.signal, b.signal]);
|
||||
expect(signal.aborted).toBe(false);
|
||||
b.abort('from-b');
|
||||
expect(signal.aborted).toBe(true);
|
||||
expect(signal.reason).toBe('from-b');
|
||||
});
|
||||
|
||||
it('aborts synchronously when an input is already aborted', () => {
|
||||
const a = createAbortController();
|
||||
a.abort('pre');
|
||||
const { signal, cleanup } = combineAbortSignals([a.signal]);
|
||||
expect(signal.aborted).toBe(true);
|
||||
expect(signal.reason).toBe('pre');
|
||||
expect(() => cleanup()).not.toThrow();
|
||||
});
|
||||
|
||||
it('ignores undefined entries', () => {
|
||||
const a = createAbortController();
|
||||
const { signal } = combineAbortSignals([undefined, a.signal, undefined]);
|
||||
a.abort();
|
||||
expect(signal.aborted).toBe(true);
|
||||
});
|
||||
|
||||
it('fires the timeout when no signal aborts first', async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const { signal } = combineAbortSignals([], { timeoutMs: 50 });
|
||||
vi.advanceTimersByTime(50);
|
||||
expect(signal.aborted).toBe(true);
|
||||
expect((signal.reason as DOMException).name).toBe('TimeoutError');
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('auto-cleans input-signal listeners when the timeout fires', async () => {
|
||||
// Timeout-driven aborts must run the same auto-cleanup as source-driven
|
||||
// aborts — otherwise long-lived input signals (e.g. a session-lived
|
||||
// AbortSignal) accumulate dead listeners across many short-lived
|
||||
// combinedSignal calls. Verifies cleanup is wired to the COMBINED
|
||||
// controller abort path, not just to source-signal events.
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const source = createAbortController();
|
||||
const before = getEventListeners(source.signal, 'abort').length;
|
||||
const { signal } = combineAbortSignals([source.signal], {
|
||||
timeoutMs: 50,
|
||||
});
|
||||
expect(getEventListeners(source.signal, 'abort').length).toBe(before + 1);
|
||||
vi.advanceTimersByTime(50);
|
||||
expect(signal.aborted).toBe(true);
|
||||
expect((signal.reason as DOMException).name).toBe('TimeoutError');
|
||||
expect(getEventListeners(source.signal, 'abort').length).toBe(before);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('cleanup removes listeners from inputs', () => {
|
||||
const a = createAbortController();
|
||||
const before = getEventListeners(a.signal, 'abort').length;
|
||||
const { cleanup } = combineAbortSignals([a.signal]);
|
||||
expect(getEventListeners(a.signal, 'abort').length).toBe(before + 1);
|
||||
cleanup();
|
||||
expect(getEventListeners(a.signal, 'abort').length).toBe(before);
|
||||
});
|
||||
|
||||
it('cleanup is idempotent', () => {
|
||||
const a = createAbortController();
|
||||
const { cleanup } = combineAbortSignals([a.signal]);
|
||||
cleanup();
|
||||
expect(() => cleanup()).not.toThrow();
|
||||
});
|
||||
|
||||
it('manual cleanup() cancels a pending timeout so it never fires', () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const { signal, cleanup } = combineAbortSignals([], { timeoutMs: 50 });
|
||||
cleanup();
|
||||
vi.advanceTimersByTime(100);
|
||||
// Without the clearTimeout in cleanups[], the timer would still fire
|
||||
// and abort the (already-cleaned) signal with TimeoutError.
|
||||
expect(signal.aborted).toBe(false);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('treats timeoutMs <= 0 as "no timeout"', () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const zero = combineAbortSignals([], { timeoutMs: 0 });
|
||||
const negative = combineAbortSignals([], { timeoutMs: -1 });
|
||||
vi.advanceTimersByTime(1_000_000);
|
||||
expect(zero.signal.aborted).toBe(false);
|
||||
expect(negative.signal.aborted).toBe(false);
|
||||
zero.cleanup();
|
||||
negative.cleanup();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('aborts and stops registering listeners once an input is found aborted mid-iteration', () => {
|
||||
const a = createAbortController();
|
||||
const b = createAbortController();
|
||||
const c = createAbortController();
|
||||
// Simulate a signal whose `aborted` getter returns false during the initial
|
||||
// `find` scan and true on subsequent accesses, exercising the per-iteration
|
||||
// defensive check inside the for-loop (not the fast path).
|
||||
let accessCount = 0;
|
||||
const proxied = new Proxy(b.signal, {
|
||||
get(target, prop, recv) {
|
||||
if (prop === 'aborted') {
|
||||
accessCount++;
|
||||
return accessCount > 1; // false on first access, true thereafter
|
||||
}
|
||||
return Reflect.get(target, prop, recv);
|
||||
},
|
||||
}) as AbortSignal;
|
||||
const { signal } = combineAbortSignals([a.signal, proxied, c.signal]);
|
||||
// Per-iteration check fires when the loop reaches proxied (2nd `aborted`
|
||||
// access) and short-circuits → controller aborts, loop breaks before c.
|
||||
expect(signal.aborted).toBe(true);
|
||||
// a was iterated before the break and DID get a listener — cleanup must
|
||||
// run synchronously (since adding to an already-aborted signal is a no-op),
|
||||
// otherwise the listener leaks on the long-lived input.
|
||||
expect(getEventListeners(a.signal, 'abort').length).toBe(0);
|
||||
// c never had a listener attached (we broke out of the loop before it).
|
||||
expect(getEventListeners(c.signal, 'abort').length).toBe(0);
|
||||
});
|
||||
|
||||
it('does not schedule a timeout when the per-iteration check aborts the controller mid-loop', () => {
|
||||
// Drives the `!controller.signal.aborted` guard inside the timeout
|
||||
// block (not the pre-loop fast path): the Proxy reports `aborted=false`
|
||||
// on the initial scan and `aborted=true` once the loop re-checks it.
|
||||
// Spy on setTimeout so we can distinguish "guard skipped scheduling"
|
||||
// from "scheduled then immediately cleared by synchronous cleanup" —
|
||||
// the latter would be observationally indistinguishable via timer
|
||||
// advancement alone since cleanup() runs synchronously and clears the
|
||||
// timer it just scheduled.
|
||||
vi.useFakeTimers();
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout');
|
||||
try {
|
||||
const a = createAbortController();
|
||||
const b = createAbortController();
|
||||
let accessCount = 0;
|
||||
const proxied = new Proxy(b.signal, {
|
||||
get(target, prop, recv) {
|
||||
if (prop === 'aborted') {
|
||||
accessCount++;
|
||||
return accessCount > 1;
|
||||
}
|
||||
return Reflect.get(target, prop, recv);
|
||||
},
|
||||
}) as AbortSignal;
|
||||
const { signal } = combineAbortSignals([a.signal, proxied], {
|
||||
timeoutMs: 50,
|
||||
});
|
||||
expect(signal.aborted).toBe(true);
|
||||
// The guard must prevent setTimeout from being called at all.
|
||||
expect(setTimeoutSpy).not.toHaveBeenCalled();
|
||||
// Belt-and-suspenders: even if a timer somehow snuck through,
|
||||
// advancing past it must not change the abort reason.
|
||||
const reasonAfterAbort = signal.reason;
|
||||
vi.advanceTimersByTime(100);
|
||||
expect(signal.reason).toBe(reasonAfterAbort);
|
||||
} finally {
|
||||
setTimeoutSpy.mockRestore();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('auto-cleans listeners on inputs when the combined signal aborts', () => {
|
||||
const a = createAbortController();
|
||||
const b = createAbortController();
|
||||
combineAbortSignals([a.signal, b.signal]);
|
||||
expect(getEventListeners(a.signal, 'abort').length).toBe(1);
|
||||
expect(getEventListeners(b.signal, 'abort').length).toBe(1);
|
||||
a.abort();
|
||||
expect(getEventListeners(a.signal, 'abort').length).toBe(0);
|
||||
expect(getEventListeners(b.signal, 'abort').length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('lifetime contract', () => {
|
||||
it('parent abort propagates to a signal whose controller the caller has dropped', () => {
|
||||
// Real-world pattern: caller pipes child.signal into an async API and
|
||||
// does not hold the controller object itself. The parent listener
|
||||
// closure keeps the controller alive long enough for parent abort to
|
||||
// reach the signal — verified WITHOUT --expose-gc because we don't
|
||||
// depend on GC behavior at all, only on the strong reference inside
|
||||
// the listener closure.
|
||||
const parent = createAbortController();
|
||||
let signal: AbortSignal;
|
||||
(() => {
|
||||
const child = createChildAbortController(parent);
|
||||
signal = child.signal;
|
||||
})();
|
||||
expect(signal!.aborted).toBe(false);
|
||||
parent.abort('parent-reason');
|
||||
expect(signal!.aborted).toBe(true);
|
||||
expect(signal!.reason).toBe('parent-reason');
|
||||
});
|
||||
});
|
||||
|
||||
describe('GC safety (best-effort, requires --expose-gc)', () => {
|
||||
const maybeGc = (globalThis as { gc?: () => void }).gc;
|
||||
const itGc = maybeGc ? it : it.skip;
|
||||
|
||||
itGc('controller becomes GC-eligible after the child aborts', async () => {
|
||||
// After child.abort(), the reverse-cleanup listener removes the
|
||||
// parent's handler closure — which was the strong holder of the
|
||||
// controller. With no other refs, the controller is collectable.
|
||||
const parent = createAbortController();
|
||||
let weakChild: WeakRef<AbortController>;
|
||||
(() => {
|
||||
const child = createChildAbortController(parent);
|
||||
weakChild = new WeakRef(child);
|
||||
child.abort();
|
||||
})();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
maybeGc!();
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
maybeGc!();
|
||||
expect(weakChild!.deref()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
165
packages/core/src/utils/abortController.ts
Normal file
165
packages/core/src/utils/abortController.ts
Normal file
|
|
@ -0,0 +1,165 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { setMaxListeners } from 'node:events';
|
||||
|
||||
/**
|
||||
* Default per-signal listener cap. Sized generously so OpenAI SDK retries +
|
||||
* internal stream/fetch wrappers + per-tool listeners can coexist on a single
|
||||
* short-lived per-request signal without warning.
|
||||
*/
|
||||
const DEFAULT_MAX_LISTENERS = 50;
|
||||
|
||||
/**
|
||||
* Create an AbortController with its signal pre-configured to allow a sane
|
||||
* number of listeners. Use this in place of `new AbortController()` everywhere
|
||||
* in production code.
|
||||
*/
|
||||
export function createAbortController(
|
||||
maxListeners: number = DEFAULT_MAX_LISTENERS,
|
||||
): AbortController {
|
||||
const controller = new AbortController();
|
||||
setMaxListeners(maxListeners, controller.signal);
|
||||
return controller;
|
||||
}
|
||||
|
||||
function asSignal(
|
||||
parent: AbortController | AbortSignal | undefined,
|
||||
): AbortSignal | undefined {
|
||||
if (!parent) return undefined;
|
||||
return parent instanceof AbortController ? parent.signal : parent;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a child AbortController that aborts when its parent aborts.
|
||||
* Aborting the child does NOT abort the parent.
|
||||
*
|
||||
* Three invariants keep listener accumulation bounded on long-lived parents
|
||||
* even when many short-lived children come and go:
|
||||
* - The parent's abort listener is registered with `{once: true}` so it
|
||||
* removes itself when the parent fires.
|
||||
* - When the child aborts (from any source — parent propagation, manual
|
||||
* abort, etc.), the listener it registered on the parent is actively
|
||||
* removed. This is the key to preventing dead-listener accumulation on
|
||||
* long-lived parents.
|
||||
* - The parent is held via `WeakRef` from the child's reverse-cleanup
|
||||
* closure, so a child being kept alive does not pin its parent.
|
||||
*
|
||||
* Lifetime contract: the child controller is held strongly by the parent's
|
||||
* listener closure until either the parent fires (closure released by
|
||||
* `{once: true}` self-removal) or the child aborts (closure released by
|
||||
* reverse-cleanup). This means callers can safely pass `child.signal` into
|
||||
* async APIs and drop the controller object — the controller will stay
|
||||
* alive long enough for parent abort to propagate to the signal.
|
||||
*
|
||||
* Accepts an `AbortController`, an `AbortSignal`, or `undefined`. Undefined
|
||||
* returns a fresh controller with no parent propagation.
|
||||
*/
|
||||
export function createChildAbortController(
|
||||
parent: AbortController | AbortSignal | undefined,
|
||||
maxListeners?: number,
|
||||
): AbortController {
|
||||
const child = createAbortController(maxListeners);
|
||||
const parentSignal = asSignal(parent);
|
||||
|
||||
if (!parentSignal) return child;
|
||||
|
||||
// Fast path: parent already aborted, no listener setup needed.
|
||||
if (parentSignal.aborted) {
|
||||
child.abort(parentSignal.reason);
|
||||
return child;
|
||||
}
|
||||
|
||||
// WeakRef on the parent only — the handler closure strongly retains the
|
||||
// child so that propagation works even if the caller passes child.signal
|
||||
// to an async API and drops the controller object. See the contract
|
||||
// docstring above.
|
||||
const weakParent = new WeakRef(parentSignal);
|
||||
const handler = (): void => {
|
||||
child.abort(weakParent.deref()?.reason);
|
||||
};
|
||||
|
||||
parentSignal.addEventListener('abort', handler, { once: true });
|
||||
|
||||
child.signal.addEventListener(
|
||||
'abort',
|
||||
() => {
|
||||
// `{once: true}` on the parent listener already self-removes when
|
||||
// parent fires; this branch covers the child-aborts-first case so
|
||||
// we don't leave a dead listener on a long-lived parent.
|
||||
weakParent.deref()?.removeEventListener('abort', handler);
|
||||
},
|
||||
{ once: true },
|
||||
);
|
||||
|
||||
return child;
|
||||
}
|
||||
|
||||
/**
|
||||
* Combine N input signals (any undefined entries are ignored) plus an optional
|
||||
* timeout into a single child AbortSignal. The returned `cleanup` releases all
|
||||
* listeners and clears the timeout — call it on the success path so listeners
|
||||
* don't linger on long-lived input signals. Cleanup is idempotent and is also
|
||||
* invoked automatically when the returned signal aborts.
|
||||
*/
|
||||
export function combineAbortSignals(
|
||||
signals: ReadonlyArray<AbortSignal | undefined>,
|
||||
options?: { timeoutMs?: number; maxListeners?: number },
|
||||
): { signal: AbortSignal; cleanup: () => void } {
|
||||
const controller = createAbortController(options?.maxListeners);
|
||||
|
||||
const alreadyAborted = signals.find((s) => s?.aborted);
|
||||
if (alreadyAborted) {
|
||||
controller.abort(alreadyAborted.reason);
|
||||
return { signal: controller.signal, cleanup: () => {} };
|
||||
}
|
||||
|
||||
const cleanups: Array<() => void> = [];
|
||||
|
||||
for (const sourceSignal of signals) {
|
||||
if (!sourceSignal) continue;
|
||||
// Re-check aborted state per iteration. Single-threaded JS can't actually
|
||||
// interleave aborts between the initial scan above and this point, but
|
||||
// making the check obvious here keeps the function correct even if a
|
||||
// future caller passes signals whose `aborted` getter has side effects.
|
||||
if (sourceSignal.aborted) {
|
||||
controller.abort(sourceSignal.reason);
|
||||
break;
|
||||
}
|
||||
const handler = () => controller.abort(sourceSignal.reason);
|
||||
sourceSignal.addEventListener('abort', handler, { once: true });
|
||||
cleanups.push(() => sourceSignal.removeEventListener('abort', handler));
|
||||
}
|
||||
|
||||
// Skip timeout if the loop already aborted the controller — its cleanup
|
||||
// wouldn't fire via the post-loop auto-cleanup path below.
|
||||
const timeoutMs = options?.timeoutMs;
|
||||
if (timeoutMs !== undefined && timeoutMs > 0 && !controller.signal.aborted) {
|
||||
const timeoutId = setTimeout(() => {
|
||||
controller.abort(new DOMException('Operation timed out', 'TimeoutError'));
|
||||
}, timeoutMs);
|
||||
cleanups.push(() => clearTimeout(timeoutId));
|
||||
}
|
||||
|
||||
let done = false;
|
||||
const cleanup = () => {
|
||||
if (done) return;
|
||||
done = true;
|
||||
for (const fn of cleanups) fn();
|
||||
};
|
||||
|
||||
// Node does not fire 'abort' listeners added to an already-aborted signal,
|
||||
// so if the per-iteration check aborted controller mid-loop we'd orphan
|
||||
// every input listener that was registered before the break. Run cleanup
|
||||
// synchronously instead.
|
||||
if (controller.signal.aborted) {
|
||||
cleanup();
|
||||
} else {
|
||||
controller.signal.addEventListener('abort', cleanup, { once: true });
|
||||
}
|
||||
|
||||
return { signal: controller.signal, cleanup };
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue