Skip to content

test(cursor): pin the empty-turn producer path end to end - #2598

Merged
lidge-jun merged 1 commit into
devfrom
codex/empty-turn-producer-2472
Aug 25, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/empty-turn-producer-2472

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Aug 25, 2026 •

Copy link
Copy Markdown
Owner

Summary

Completes #2472, together with #2597.

#2597 added the observer. This proves the producer side rather than assuming it: a Cursor
turnEnded with no text and no committed tool call finalizes to a bare done
(finalizeTurnEvents) — a successful terminal carrying no content, which is exactly the
shape 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 that
was 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:

  • the empty-turn shape itself;
  • an incomplete tool call, which finalizes to an error — already a stated failure, so not
    this failure mode and not to be conflated with it;
  • a turn reporting output tokens while emitting no content — usage counters are not content,
    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
observeEmptyCompletion and asserted to be both flagged and passed through unchanged, so a
future change to either side cannot quietly break the pairing.

Verification

$ bun test tests/cursor-protobuf-events.test.ts
 48 pass, 0 fail

$ bun test tests/cursor-protobuf-events.test.ts tests/empty-completion-guard.test.ts tests/cursor-adapter.test.ts
 95 pass, 0 fail, 281 expect() calls

$ bun x tsc --noEmit
(clean)

Tests only; no runtime behaviour changes in this PR.

Checklist

  • Targets dev
  • Based on the current dev head
  • Producer path proved, not assumed
  • Typecheck clean
  • No secrets or account identifiers in the diff

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of responses that finish without generated content.
    • Incomplete tool interactions now report an appropriate error instead of appearing successfully completed.
    • Usage-only responses are no longer incorrectly treated as having meaningful content.
  • Tests

    • Added regression and end-to-end coverage for silent completions, incomplete tool calls, and event pass-through behavior.

#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.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner August 25, 2026 17:57
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@lidge-jun
lidge-jun merged commit 7ead799 into dev Aug 25, 2026
7 of 9 checks passed
@lidge-jun
lidge-jun deleted the codex/empty-turn-producer-2472 branch August 25, 2026 17:57
@coderabbitai

coderabbitai Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d0ca3913-0e32-46d3-990d-07035db7f830

📥 Commits

Reviewing files that changed from the base of the PR and between 316043e and a19178c.

📒 Files selected for processing (1)
  • tests/cursor-protobuf-events.test.ts

📝 Walkthrough

Walkthrough

The PR adds Cursor event tests for silent turns. The tests cover empty successful completions, incomplete tool-call errors, token-only turns, and observeEmptyCompletion event pass-through.

Changes

Cursor completion validation

Layer / File(s) Summary
Silent completion coverage
tests/cursor-protobuf-events.test.ts
Adds imports and tests for empty turn finalization, incomplete tool calls, token-only turns, and end-to-end empty-completion observation. The integration test confirms that events pass through unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: luvs01

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/empty-turn-producer-2472

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Aug 25, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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[];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

tarunravi pushed a commit to tarunravi/opencodex that referenced this pull request Sep 14, 2026
…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.
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant