Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Claude adapter now treats subprocess termination by ChangesClaude interruption handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The interruption path correctly completes signal-terminated Claude turns without reporting a runtime stream failure. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Approved 8f898ff Straightforward bug fix that adds two string patterns to an existing pattern-matching function. The change correctly classifies SIGTERM/SIGINT subprocess terminations as 'interrupted' rather than runtime errors, following the same pattern already used for similar cases. Includes comprehensive test coverage. You can customize Macroscope's approvability policy. Learn more. |
- ClaudeAdapter now recognizes "terminated by signal SIGTERM/SIGINT" from the Claude Agent SDK's ProcessTransport as a graceful interruption instead of surfacing a runtime.error - Covers the case where the OS delivers the signal directly to the subprocess (e.g. desktop app quit) ahead of our own stopSession()-driven close - Add regression test asserting no runtime.error is emitted and the turn completes with state "interrupted"
8f898ff to
e6c7331
Compare
|
This PR should fix #5323 |
|
Thanks for the PR. We're not taking changes to the orchestration and provider layers right now: that part of the server is being rewritten for V2, and merging into the current code would either conflict with or be thrown away by that work. Closing for now. If this is still an issue once V2 lands, please reopen (or open a fresh PR against the new code) and we'll take a proper look. |
What Changed
ClaudeAdapternow recognizes "terminated by signal SIGTERM/SIGINT" messages from the Claude Agent SDK'sProcessTransportas a graceful interruption instead of surfacing aruntime.error. Added a regression test asserting noruntime.erroris emitted and the turn completes with stateinterrupted.Why
When the OS delivers SIGTERM/SIGINT directly to the Claude subprocess (e.g. on desktop app quit) ahead of our own
stopSession()-driven close, the SDK's error message for the signal-killed process was being treated as a runtime error instead of a normal interruption.UI Changes
When Cmd+Q app and reopen a Claude Code thread it looks like screenshot
Checklist
Note
[!NOTE]
Fix
isClaudeInterruptedMessageto treat SIGTERM/SIGINT-killed subprocesses as interruptedWhen the Claude CLI subprocess is killed by SIGTERM or SIGINT before
stopSession()completes, the Claude Agent SDK surfaces an error message containing'terminated by signal sigterm'or'terminated by signal sigint'. Previously these were treated as runtime errors; nowisClaudeInterruptedMessagematches these substrings and returnsinterruptedstate instead.Macroscope summarized 8021acf.
Note
Low Risk
Small change to interruption heuristics in the Claude adapter with a focused regression test; no auth, data, or API surface changes.
Overview
When the desktop app quits, the OS can SIGTERM/SIGINT the Claude CLI child before
stopSession()finishes. The Agent SDK then reports "terminated by signal SIGTERM/SIGINT" — which used to surface asruntime.errorand a broken thread on reopen.isClaudeInterruptedMessageinClaudeAdapternow treats those SDKProcessTransportmessages like other graceful interruptions, so the turn ends with stateinterruptedand no runtime error.A regression test simulates external subprocess kill and asserts no
runtime.errorandturn.completedwithinterrupted.Reviewed by Cursor Bugbot for commit 8021acf. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
SIGTERMorSIGINTare now handled as interrupted turns rather than runtime errors.runtime.errorfrom being emitted when Claude is stopped externally.Tests