fix(cursor): surface a cancel we did not request instead of dropping the turn - #1994
Conversation
…the turn An unexpected NGHTTP2_CANCEL produced a turn with zero adapter events. The transport already got this right - both failure exits check `this.expectedClose` before swallowing a cancel - but the adapter re-decided the same question one layer up from the error CODE alone (cursor.ts:181 via isCursorBenignCancelError), with no provenance in scope. A cancel Cursor sent us was therefore indistinguishable from one we sent, and the turn vanished: streaming degraded to adapter_eof, non-streaming returned "completed". The transport now raises a typed CursorUnexpectedCancelError when the cancel is not ours and no terminal has been emitted yet. Two details the audit forced: - The guard includes !emittedTerminal. Tagging on !expectedClose alone would fire after a done/error was already queued, and buffered JSON processes both - flipping a completed turn to failed. - The error carries its own message rather than the raw NGHTTP2_CANCEL text. Left raw, it is re-matched by classifyCursorError and reported as "Cursor stream suspended", so a turn that failed unexpectedly would claim an intentional suspension. Untagged cancels keep their existing benign classification, so nothing that returns silently today starts erroring without evidence. Verified red-before-green.
Review follow-up: wrapping the raw cancel hid its code from the per-turn `turn-failed` diagnostic, so the one summary that exists to explain this failure was the one with no errorCode. The wrapper now re-exposes the originating code. The added test also guards the trap this creates: carrying NGHTTP2_CANCEL back onto the error must NOT make isCursorBenignCancelError match it again. Provenance is checked first, so the class stays non-benign.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
Summary
Surface a Cursor CANCEL we did not request instead of dropping the turn.
An unexpected
NGHTTP2_CANCELproduced a turn with zero adapter events. The transport already had this right — both failure exits checkthis.expectedClosebefore swallowing a cancel — but the adapter re-decided the same question one layer up from the error code alone (cursor.ts:181viaisCursorBenignCancelError), with no provenance in scope. A cancel Cursor sent us was indistinguishable from one we sent, and the turn vanished: streaming degraded toadapter_eof, non-streaming returnedcompleted.The transport now raises a typed
CursorUnexpectedCancelErrorwhen the cancel is not ours and no terminal has been emitted. Two details review forced:!emittedTerminal— the flag PR1 introduces. Tagging on!expectedClosealone would fire after adone/errorwas already queued, flipping a completed buffered turn to failed.NGHTTP2_CANCELtext. Left raw, it is re-matched byclassifyCursorErrorand reported as "Cursor stream suspended" — a turn that failed unexpectedly would claim an intentional suspension.Untagged cancels keep their existing benign classification, so nothing that returns silently today starts erroring without evidence.
Stacked on #1993 — this layer reads the
emittedTerminalflag that PR introduces.Verification
Covered by the stack-tip gate run on
ssh lidgeatff4b0bb4e(typecheck, privacy:scan, audit:high, build:gui, full isolated suite — all green). Verified red-before-green when the provenance tag was disabled.Checklist
devlog/_plan/260817_cursor_toolcall_decode/040_*.md; failure-path correctness, not a documented feature.)cursor-errorsredaction path.)