fix(relay): reject false submit receipts - #343
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a994601e-b4cf-4e79-8b40-544e623feb20) |
|
@coderabbitai review @greptileai review |
|
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughSubmission verification now detects queued Codex input, correlates fresh Cursor responses, and fails closed when evidence is unavailable. Relay and supersession flows use verified delivery results, bounded busy timeouts, and non-retryable reconciliation errors. ChangesSubmission verification
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AgentRelay
participant SubmissionVerifier
participant TerminalSurface
AgentRelay->>SubmissionVerifier: submit agent input
SubmissionVerifier->>TerminalSurface: capture pre-Return screen evidence
TerminalSurface-->>SubmissionVerifier: parsed screen snapshot
SubmissionVerifier->>TerminalSurface: send Return and poll verification evidence
TerminalSurface-->>SubmissionVerifier: queued input, working state, or fresh Cursor response
SubmissionVerifier-->>AgentRelay: verified success or fail-closed error
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 191b963cde
ℹ️ 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".
Independent Claude review — head
|
These sat UNCOMMITTED in this worktree from the round-3 iteration that hit the 3-attempt cap. Not reviewed, not verified, NOT claimed working — the lane's known blocker (Cursor retained-composer receipt matcher) was never resolved. Committed only so a worktree prune or cleanup cannot delete them. #343 is still CONFLICTING against main and needs a rebase on merit before any of this is trusted. Preserved by the outgoing cmuxlayer lead; authored by the #343 worker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol <noreply@anthropic.com>
191b963 to
52fb19d
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_91ce0c41-a761-44b0-b300-42814e71606b) |
|
All NOT_SHIP items were reconciled on the rebased head:
Fresh local and push-hook gates pass: typecheck, build, 107 Vitest files, 2,473 tests passed with 1 skipped, whitespace check, nightly contract receipts, and terminal-state regressions. Local CodeRabbit's post-fix review produced no valid correctness defect: its supersede-reason suggestion was disproved by an added assertion (the shared error serializer already emits — cmuxlayerCodex (worker) · codex/gpt-5.6-sol |
|
@coderabbitai review @codex review Please review the rebased final head, including the raw-runtime Cursor retained-composer regression and the Codex queue correlation fixes. — cmuxlayerCodex (worker) · codex/gpt-5.6-sol |
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52fb19d687
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/enter-reliability.test.ts (1)
825-830: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the verification-window boundary from a shared constant.
The test pins the window to the open interval between 4_900 ms and 5_900 ms with literal values. The PR objectives record a discrepancy between the documented timeout and the code default. If the default changes, this test fails with an opaque "settled === false" mismatch instead of a clear signal.
Export the verification timeout from
src/server.ts(or expose it on__submitEvidenceTestHooks) and compute the two advance amounts from it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/enter-reliability.test.ts` around lines 825 - 830, Replace the hard-coded timer advances in the test around resultPromise with values derived from a shared exported verification-timeout constant. Export that timeout from server.ts or expose it through __submitEvidenceTestHooks, then compute the pre-boundary and post-boundary advances from it while preserving the existing assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitattributes:
- Around line 1-3: Update the .gitattributes rules for terminal-capture fixtures
to use a glob covering all relevant files under the fixture directories, and pin
their line endings to LF so Git does not normalize them to CRLF on Windows.
Preserve the existing whitespace handling for trailing blank-line padding while
applying both attributes consistently to future captures.
In `@tests/enter-reliability.test.ts`:
- Around line 280-282: Update the branch using postReturnPendingScreenText so it
also requires returnCount > 0 before returning the queued fixture. Preserve the
existing pendingText and non-null checks, ensuring pre-Return baseline reads
remain queue-free while post-Return reads still return the pending screen.
- Around line 1312-1321: Extend the assertions for the failed submit result and
emitted event in this test to verify both submit_verification_reason and
retry_safe, matching the expected unavailable-screen values used by the parallel
tests. Keep the existing submit_verified and retry_count assertions unchanged.
- Around line 1270-1295: Make the fake Codex frame realistic in the test
“ignores adjacent Codex queue chrome for another sender's visible prefix” by
setting client.requiredReturns to 1 so pendingText clears before the queued
read. Preserve the unrelated queued text and existing assertions, ensuring the
empty composer represents a successful Return rather than an unsubmitted
message.
- Around line 1468-1470: Update the Cursor screen parser to recognize the “⠀⠞
Working” spinner as the working status, add or extend parser coverage for this
fixture, and change the assertion on parsed.screen?.status to expect “working”
while preserving the existing screen-read assertions.
In `@tests/server.test.ts`:
- Line 3027: Update the test setup around vi.useFakeTimers() to register
vi.useRealTimers() before processScopedTmpDir("cmuxlayer-f8-send-command"), or
use Vitest’s test-finished cleanup hook, so setup failures cannot leak fake
timers into later tests.
---
Outside diff comments:
In `@tests/enter-reliability.test.ts`:
- Around line 825-830: Replace the hard-coded timer advances in the test around
resultPromise with values derived from a shared exported verification-timeout
constant. Export that timeout from server.ts or expose it through
__submitEvidenceTestHooks, then compute the pre-boundary and post-boundary
advances from it while preserving the existing assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bf4f644b-adbe-45f3-b711-4d1890e1760a
📒 Files selected for processing (9)
.gitattributessrc/server.tstests/enter-reliability.test.tstests/fixtures/cursor-pr343-live-accepted-response.txttests/fixtures/cursor-pr343-v2-immediate-working-response.txttests/fixtures/painpoints/codex-pr343-live-queued-followup.txttests/painpoint-replay.test.tstests/server-agent-tools.test.tstests/server.test.ts
💤 Files with no reviewable changes (2)
- tests/server-agent-tools.test.ts
- src/server.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Macroscope - Correctness Check
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-15T10:42:35.917Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 1
File: tests/quality-tracking.test.ts:171-200
Timestamp: 2026-03-15T10:42:35.917Z
Learning: In tests/quality-tracking.test.ts for the cmuxlayer project, ensure that at or above 80% context quality degradation, behavior depends on depth: depth-0 agents receive a /compact command; depth > 0 agents are killed and logged (kill + log). Respawn of non-root agents is out of scope for v1. Treat the design doc quality tracking section as the authoritative source for this behavior, and align test expectations accordingly.
Applied to files:
tests/server.test.tstests/painpoint-replay.test.tstests/enter-reliability.test.ts
🪛 LanguageTool
tests/fixtures/cursor-pr343-v2-immediate-working-response.txt
[grammar] ~3-~3: Ensure spelling is correct
Context: ... Duration 235ms (transform 59ms, setup 7ms, collect 56ms, tests 22ms, environment ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~30-~30: Ensure spelling is correct
Context: ... Duration 246ms (transform 57ms, setup 5ms, collect 60ms, tests 22ms, environment ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~57-~57: Ensure spelling is correct
Context: ... Duration 236ms (transform 61ms, setup 6ms, collect 59ms, tests 24ms, environment ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (15)
tests/enter-reliability.test.ts (10)
5-58: LGTM!
78-93: LGTM!
118-138: LGTM!
228-269: LGTM!
496-533: LGTM!
645-646: LGTM!Also applies to: 718-718, 735-736, 860-861, 876-876, 892-893, 908-908, 925-926
967-1128: LGTM!
1130-1268: LGTM!
1349-1448: LGTM!
1480-1603: LGTM!tests/fixtures/cursor-pr343-live-accepted-response.txt (1)
1-22: LGTM!tests/fixtures/painpoints/codex-pr343-live-queued-followup.txt (1)
1-81: LGTM!tests/painpoint-replay.test.ts (1)
70-70: LGTM!Also applies to: 177-177
tests/server.test.ts (1)
3060-3082: LGTM!tests/fixtures/cursor-pr343-v2-immediate-working-response.txt (1)
1-101: 🗄️ Data Integrity & IntegrationConfirm the CI/former workflow preserves terminal-panic fixture whitespace.
This fixture has right-padding spaces on many lines, and
.gitattributesonly suppressesgit diffblank-at-eol warnings. If Prettier/editorconfig/pre-commit/formatted commit hooks strip trailing whitespace, the fixture will stop reproducing the real pane andCURSOR_PR343_V2_PRE_RETURN_SCREENwill change. Add an ignore or config exception, or keep this capture without trailing-space-sensitive formatting.
Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_aeff1d52-948c-461e-8098-5102d4b98cf5) |
Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_811398e7-6bfc-44b3-85bf-a0b22fa15bbb) |
|
@coderabbitai review @codex review Please review final head — cmuxlayerCodex (worker) · codex/gpt-5.6-sol |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- PR metadata ---'
gh pr view 343 --repo EtanHey/cmuxlayer --json headRefOid,baseRefOid,files,title,url \
--jq '{title, url, baseRefOid, headRefOid, files: [.files[].path]}'
printf '%s\n' '--- Recent commits on the PR branch ---'
git log --oneline --decorate -12
printf '%s\n' '--- Changed files relative to base ---'
git diff --name-status "$(git merge-base HEAD origin/HEAD)" HEAD 2>/dev/null || git diff --name-status HEAD~1 HEAD
printf '%s\n' '--- Outline: primary implementation ---'
ast-grep outline src/server.ts --items all --match 'deliverInputChunks|verify|screenShowsQueuedAgentInput|screenShowsPendingInput|hasClearedAgentComposer|supersede_agent_goal' --view expanded || true
printf '%s\n' '--- Outline: targeted tests ---'
ast-grep outline tests/enter-reliability.test.ts --items all --match 'Cursor|queue|queued|retained|supersede|allow_busy' --view expanded || trueLength of output: 3164 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/HEAD)"
printf '%s\n' '--- Implementation diff: server and screen parser ---'
git diff --unified=80 "$base" HEAD -- src/server.ts src/screen-parser.ts
printf '%s\n' '--- Test diff: verification and supersede paths ---'
git diff --unified=60 "$base" HEAD -- \
tests/enter-reliability.test.ts \
tests/screen-parser.test.ts \
tests/server-agent-tools.test.ts \
tests/server.test.ts \
tests/painpoint-replay.test.ts
printf '%s\n' '--- Helper and call-site locations ---'
rg -n -C 4 -P '\b(screenShowsPendingInput|screenShowsQueuedAgentInput|hasClearedAgentComposer|verify[A-Za-z]*Submit|deliverInputChunks|supersedePatch|supersede_agent_goal)\b' \
src/server.ts src/screen-parser.ts
printf '%s\n' '--- Server verification helper region ---'
sed -n '1840,2140p' src/server.ts
sed -n '3360,3740p' src/server.ts
sed -n '10480,10680p' src/server.ts
printf '%s\n' '--- New raw fixtures ---'
for file in \
tests/fixtures/cursor-pr343-live-accepted-response.txt \
tests/fixtures/cursor-pr343-v2-immediate-working-response.txt \
tests/fixtures/painpoints/codex-pr343-live-queued-followup.txt
do
printf '\n--- %s ---\n' "$file"
cat -n "$file"
doneLength of output: 208591 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Composer and CLI inference primitives ---'
rg -n -C 8 -P '\b(function (inferComposerCli|matchComposerPromptLine|normalizeTerminalText|isSubmitVerifiedStatus)|const (CODEX|CURSOR).*PROMPT)' src/server.ts
printf '%s\n' '--- Exact source windows for the primitives and verification loop ---'
sed -n '1540,1845p' src/server.ts
sed -n '3930,4170p' src/server.ts
printf '%s\n' '--- Timeout and allow_busy documentation references ---'
rg -n -C 2 -i 'CMUXLAYER_SUBMIT_VERIFY_TIMEOUT_MS|submit.verify.timeout|verify timeout|allow_busy|queued-but-unsubmitted' \
README.md CLAUDE.md docs src tests 2>/dev/null || true
printf '%s\n' '--- Final commit delta ---'
git diff --check HEAD~1 HEAD
git diff --stat HEAD~1 HEAD
git diff --unified=40 HEAD~1 HEAD -- \
src/server.ts src/screen-parser.ts \
tests/enter-reliability.test.ts tests/screen-parser.test.ts \
tests/server-agent-tools.test.ts
printf '%s\n' '--- Fixture-backed test inventory ---'
rg -n -C 2 -P '(CURSOR_PR343|CODEX_PR343|PR343_LIVE_QUEUE|retained-composer|wrapped.*Codex|supersede_agent_goal reports)' \
tests/enter-reliability.test.ts tests/server-agent-tools.test.ts tests/screen-parser.test.ts
printf '%s\n' '--- File-level diff check ---'
git diff --check "$(git merge-base HEAD origin/HEAD)" HEADLength of output: 76826
The Cursor path now uses baseline-deduped response evidence. It accepts a verified post-submit response when Cursor retains composer text. It rejects unchanged historical response evidence and unavailable baselines. The Codex path now scopes queue detection to the active composer region. It reconstructs wrapped queue headings. It requires the queued prefix to match the submitted text. It rejects matched queued input before a working-status receipt. Busy relays use the bounded 500 ms verification window. They do not issue a recovery Return. The tests use the added raw Cursor and Codex fixtures. The PR diff passes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4fd7103a7
ℹ️ 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".
| const remaining = baselineCounts.get(signature) ?? 0; | ||
| if (remaining === 0) { | ||
| return true; |
There was a problem hiding this comment.
Track Cursor response blocks instead of mutable signatures
When allow_busy repeats text that already appears in a Cursor transcript while the earlier response is still evolving, a missed Return can still be reported as verified: the pre-Return baseline contains the old response's current signature, but a subsequent Working transition or additional streamed thought text produces a different signature, so this branch treats the same historical response as fresh and the pending composer is ignored. Compare stable response-block identity or require evidence created after the pre-Return boundary rather than treating any unseen content signature as a new response.
Useful? React with 👍 / 👎.
Summary
Related: #364
Review fixes
mainWorkingtransitionWorkingspinner as workingVerification
bun run typecheckbun run buildbun run test— 107 files passed; 2,477 tests passed, 1 skippedgit diff --checkEvidence boundary
Delivery is accepted only from evidence newer than the pre-submit baseline and correlated with the submitted content. Unchanged historical Cursor responses, unrelated queue blocks, dirty composers without fresh activity, and unverified supersede side effects remain failures.
— cmuxlayerCodex (worker) · codex/gpt-5.6-sol
Note
Reject false submit receipts in relay by verifying Codex queue and Cursor response evidence
cursorSubmittedResponseEvidenceSignaturesto detect Cursor Working/Thinking/Running activity and thought blocks as a pre/post-Return baseline, treating fresh post-Return evidence as positive submit verification even when the composer still shows text.screenShowsQueuedAgentInputto detect Codex live queue chrome correlated to the submitted text and fail submit verification immediately when a match is found.normalizeKnownPlaceholderComposerInputto collapse known placeholder texts (CodexImplement {feature}, CursorPlan, search, build anything) to empty so they do not falsely indicate pending input.supersede_agent_goalnow fails closed on unverified submits, returning a structuredsupersede_submit_unverifiederror without patching the agent registry.allow_busysends are now subject to submit verification using a shorter 500 ms timeout (BUSY_AGENT_SUBMIT_VERIFY_TIMEOUT_MS), andmarkAgentWorkingis only called when verification succeeds.CURSOR_BRAILLE_WORKING_RE) so Cursor screens with the braille spinner are classified as working status ininferStatus.Macroscope summarized a4fd710.