Add AbortSignal support to TypeScript SDK#6378
Conversation
This change allows users to cancel thread execution using an AbortController. Changes: - Add signal property to TurnOptions type - Pass signal through Thread methods to exec layer - Add signal parameter to CodexExecArgs - Leverage Node.js native spawn() signal support for clean cancellation - Add comprehensive test coverage for abort scenarios The implementation uses Node.js's built-in AbortSignal support in spawn(), which automatically handles process termination and cleanup when aborted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds a test that creates an infinite-streaming server to verify the abort signal actually kills a running process, not just catches pre-aborted signals. The test: - Creates a server that streams events indefinitely (every 100ms) - Starts a thread operation - Aborts after 200ms - Verifies completion in <500ms (proves process was killed) - Without signal support, test times out after 10s This ensures AbortSignal actually interrupts long-running operations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Improves the long-running abort test to use an infinite tool call loop instead of simple progress events. This better simulates a real scenario where the agent continuously executes tools and is more rigorous because it tests killing during active event processing, not just I/O wait. The test now: - Streams tool call events (in_progress -> completed) every 100ms - Simulates an agent stuck in an execution loop - Verifies abort kills the process during active tool execution - Hangs without signal support (verified) - Completes in ~203ms with signal support 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@danfhernandez, I noticed that you opened this PR in draft mode. When it's ready for review, please move it out of draft mode. Looks like there's a lint (formatting) error detected that needs to be addressed. |
- Update eslint config to allow underscore-prefixed unused vars - Fix abort.test.ts to use void pattern for consumed events
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The tests were passing async functions to expect().rejects instead of
invoking them and passing the resulting promises. This caused Jest to
fail immediately with "Received value must be a promise" without
running the test bodies.
Fixed by immediately invoking the async functions: (async () => {...})()
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| res.statusCode = status; | ||
| res.setHeader("content-type", "text/event-stream"); | ||
|
|
||
| const responseBody = responseBodies[Math.min(responseIndex, responseBodies.length - 1)]!; | ||
| responseIndex += 1; | ||
| const responseBody = responseBodies.next().value; | ||
| for (const event of responseBody.events) { |
There was a problem hiding this comment.
Handle exhaustion of custom response generators
The new ResponsesProxyOptions allows callers to supply a Generator<SseResponseBody>, but startResponsesTestProxy now blindly does const responseBody = responseBodies.next().value; and immediately dereferences responseBody.events. If a finite generator is provided (which the type signature allows), once it finishes .next() returns { done: true, value: undefined } and the server crashes with TypeError: Cannot read properties of undefined (reading 'events'), yielding a 500 response instead of a deterministic error. Arrays converted via createGenerator already throw a clear “not enough responses provided” error, so generators should get the same treatment by checking the .done flag and throwing an explicit error before touching events.
Useful? React with 👍 / 👎.
Summary
Adds AbortSignal support to the TypeScript SDK for canceling thread execution using AbortController.
Changes
signal?: AbortSignalproperty toTurnOptionstypeCodexExecArgsspawn()signal support for automatic cancellationImplementation
The implementation uses Node.js's built-in AbortSignal support in
spawn()(available since Node v15, SDK requires >=18), which automatically handles:This is a one-line change to the core implementation (
signal: args.signalpassed to spawn), making it simple, reliable, and maintainable.Usage Example
Testing
All tests pass (23 total across SDK):
Tests verified to fail correctly when signal support is removed (no false positives).