Repository navigation
test(cursor): pin the empty-turn producer path end to end - #2598
Conversation
#2472 was closable only once the producer side was proved, not assumed. A Cursor turnEnded with no text and no committed tool call finalizes to a bare done: a successful terminal carrying no content, which is exactly the shape a client records as a completed turn that said nothing. Pins three distinctions that matter: that shape, an incomplete tool call (already a stated error, so NOT this failure mode), and a turn reporting output tokens without emitting content - usage counters are not content. Joins the halves too: the adapter's real output is fed through observeEmptyCompletion and asserted to be both flagged and passed through unchanged, so a future change to either side cannot quietly break the pairing.
|
✅ Deterministic PR hygiene checks passed. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds Cursor event tests for silent turns. The tests cover empty successful completions, incomplete tool-call errors, token-only turns, and ChangesCursor completion validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a19178c838
ℹ️ 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".
| */ | ||
| test("a Cursor turnEnded with no output is flagged by the observer", async () => { | ||
| const state = createCursorProtobufEventState(); | ||
| const produced = mapCursorProtobufServerMessage(turnEndedFrame(), state) as unknown as AdapterEvent[]; |
There was a problem hiding this comment.
Pass producer output through the Cursor message mapper
This cast skips the mapCursorServerMessage step that the real adapter executes at src/adapters/cursor.ts:212, so the test feeds CursorServerMessage objects directly into an observer that accepts AdapterEvents. If the mapper drops or changes the terminal event, the production observer will no longer flag the turn while this purported end-to-end test continues to pass; map each produced message through the real mapper (or exercise runTurn) before calling observeEmptyCompletion.
Useful? React with 👍 / 👎.
…2598) lidge-jun#2472 was closable only once the producer side was proved, not assumed. A Cursor turnEnded with no text and no committed tool call finalizes to a bare done: a successful terminal carrying no content, which is exactly the shape a client records as a completed turn that said nothing. Pins three distinctions that matter: that shape, an incomplete tool call (already a stated error, so NOT this failure mode), and a turn reporting output tokens without emitting content - usage counters are not content. Joins the halves too: the adapter's real output is fed through observeEmptyCompletion and asserted to be both flagged and passed through unchanged, so a future change to either side cannot quietly break the pairing.
…2598) lidge-jun#2472 was closable only once the producer side was proved, not assumed. A Cursor turnEnded with no text and no committed tool call finalizes to a bare done: a successful terminal carrying no content, which is exactly the shape a client records as a completed turn that said nothing. Pins three distinctions that matter: that shape, an incomplete tool call (already a stated error, so NOT this failure mode), and a turn reporting output tokens without emitting content - usage counters are not content. Joins the halves too: the adapter's real output is fed through observeEmptyCompletion and asserted to be both flagged and passed through unchanged, so a future change to either side cannot quietly break the pairing.
Summary
Completes #2472, together with #2597.
#2597 added the observer. This proves the producer side rather than assuming it: a Cursor
turnEndedwith no text and no committed tool call finalizes to a baredone(
finalizeTurnEvents) — a successful terminal carrying no content, which is exactly theshape a client records as a completed turn that said nothing.
That matters because the earlier investigation of this issue leaned toward "not an OpenCodex
defect". The envelope fields it named (
wall_time_seconds) really are host-owned, but thatwas never evidence that the proxy could not produce an empty successful turn. It can, and
now there is a test saying so.
Three distinctions are pinned deliberately:
this failure mode and not to be conflated with it;
which is why a turn can look non-empty in metering and still be blank to the client.
The two halves are then joined: the adapter's real output is fed through
observeEmptyCompletionand asserted to be both flagged and passed through unchanged, so afuture change to either side cannot quietly break the pairing.
Verification
Tests only; no runtime behaviour changes in this PR.
Checklist
devdevheadSummary by CodeRabbit
Bug Fixes
Tests