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.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI 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:
📝 WalkthroughWalkthroughSubmission verification now recognizes queued agent input, requires positive evidence for non-spawn deliveries, and accepts working or thinking states after pending input clears. Busy relays also verify submissions without recovery retries. Tests cover Codex transitions, screen-read failures, and fail-closed responses. ChangesSubmission Verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AgentRelay
participant SubmissionVerifier
participant TerminalSurface
AgentRelay->>SubmissionVerifier: submit agent input
SubmissionVerifier->>TerminalSurface: read screen evidence
TerminalSurface-->>SubmissionVerifier: queued input or working/thinking state
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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| ); | ||
| } | ||
|
|
||
| function screenShowsQueuedAgentInput(screenText: string): boolean { |
There was a problem hiding this comment.
🟠 High src/server.ts:1744
screenShowsQueuedAgentInput returns true whenever the heading Messages to be submitted after next tool call and a ↳ line appear anywhere in the full transcript, including in prior conversation output. If a submitted message or visible reply itself contains those lines (e.g. while discussing or relaying this UI), the function keeps reporting pending queued input after the real composer has cleared, causing submit verification to falsely fail. The regex searches the entire screenText without any boundary tying the heading and ↳ marker to the current composer area, so stale matches from earlier output are accepted. Consider anchoring the search to the most recent occurrence of the heading, or otherwise scoping it to the live UI region rather than the whole transcript.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 1744:
`screenShowsQueuedAgentInput` returns `true` whenever the heading `Messages to be submitted after next tool call` and a `↳` line appear anywhere in the full transcript, including in prior conversation output. If a submitted message or visible reply itself contains those lines (e.g. while discussing or relaying this UI), the function keeps reporting pending queued input after the real composer has cleared, causing submit verification to falsely fail. The regex searches the entire `screenText` without any boundary tying the heading and `↳` marker to the current composer area, so stale matches from earlier output are accepted. Consider anchoring the search to the most recent occurrence of the heading, or otherwise scoping it to the live UI region rather than the whole transcript.
| /(?:^|\n)[ \t]*Messages to be submitted after next tool call[ \t]*(?=\n|$)/i.exec( | ||
| normalized, | ||
| ); |
There was a problem hiding this comment.
🟠 High src/server.ts:1747
The heading regex requires the entire string Messages to be submitted after next tool call on one line. In a narrow terminal pane, Codex wraps that heading across two screen rows, so the regex never matches and screenShowsQueuedAgentInput returns false — even though a queued ↳ entry is present. A simultaneous working status is then accepted as a successful submission even though the input is still queued. The regex should tolerate line wrapping within the heading text (or detect the ↳ queue entry independently of the heading).
| /(?:^|\n)[ \t]*Messages to be submitted after next tool call[ \t]*(?=\n|$)/i.exec( | |
| normalized, | |
| ); | |
| /(?:^|\n)[ \t]*Messages\s+to\s+be\s+submitted\s+after\s+next\s+tool\s+call[ \t]*(?=\n|$)/i.exec( |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around lines 1747-1749:
The heading regex requires the entire string `Messages to be submitted after next tool call` on one line. In a narrow terminal pane, Codex wraps that heading across two screen rows, so the regex never matches and `screenShowsQueuedAgentInput` returns `false` — even though a queued `↳` entry is present. A simultaneous `working` status is then accepted as a successful submission even though the input is still queued. The regex should tolerate line wrapping within the heading text (or detect the `↳` queue entry independently of the heading).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
| if (!snapshot) { | ||
| return { submit_verified: null, retry_count: retryCount }; | ||
| return { | ||
| submit_verified: noSubmitEvidenceResult, | ||
| retry_count: retryCount, | ||
| }; |
There was a problem hiding this comment.
When the first post-Return readScreen call has a transient socket/connection failure, readParsedSurface converts it to null and this branch immediately reports submit_verified: false instead of polling for the configured verification window. The input may already have been successfully delivered, so callers receive an error and may retry it, producing a duplicate; reserve the failure result for when no later poll provides positive evidence.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| function screenShowsQueuedAgentInput(screenText: string): boolean { |
There was a problem hiding this comment.
Because this helper accepts only the full screen and does not identify a Codex UI region or correlate the queue entry with the submitted text, ordinary visible transcript content can trigger it. For example, a successfully submitted multiline prompt that quotes Messages to be submitted after next tool call followed by an indented ↳ line remains visible while the agent is working; it is then classified as pending, suppressing both working-status and cleared-composer success until the operation falsely times out.
Useful? React with 👍 / 👎.
Independent Claude review — head
|
1 participant
Summary
allow_busysends, without issuing a recovery Return for busy/queued delivery.working/thinkingstatus.nullonly when verification was not requested or for the documentedspawn_agentlauncher probe.Real incident coverage
allow_busyfollow-up that received success while remaining in the Codex composer.Messages to be submitted after next tool call/↳frame, followed by the message moving into the composer without submission.send_commandand unavailable-screen aliases cannot retain the same false-green receipt.Focus evidence and scope boundary
Live evidence shows workspace selection and global pane focus are distinct: newly spawned panes remained
focused:falsewhile explicit surface-addressed boot and prompt delivery completed. The source path does not transiently focus and restore the new spawn surface. Focus placement remains a follow-up and is intentionally not changed here.Probe D's stale surface/registry cleanup and misleading resync diff are also intentionally excluded from this first PR.
Live evidence came from the installed Homebrew cmuxlayer
0.4.16bundle at/opt/homebrew/Cellar/cmuxlayer/0.4.16/libexec/dist/index.js, which has no commit stamp and is not identified ase8021c4. This PR's source base isorigin/mainate8021c4.TDD receipts
191b963.git diff --check: exit 0.Review state
Local CodeRabbit review was bounded and stopped after approximately three minutes while still emitting heartbeats. Its one MAJOR finding—requested verification could return
nullwithout positive screen evidence—was fixed with dedicated RED/GREEN coverage. No CRITICAL finding was emitted. PR bot reviews are requested after opening.Remaining gate
Do not merge or deploy yet. MCP/agent lifecycle code still requires parent-coordinated independent Claude review and real fresh-client runtime verification of the reviewed artifact.
Note
Medium Risk
Changes core relay delivery success semantics for MCP/agent tools; incorrect heuristics could reject valid submits or still miss edge cases, but scope is verification-only with broad test coverage.
Overview
Tightens agent input submit verification so successful delivery is only reported when the terminal screen shows real evidence of submission, addressing false-green receipts (including live Codex
allow_busyincidents).Adds
screenShowsQueuedAgentInputto treat Codex’s “Messages to be submitted after next tool call” /↳chrome as pending input. Verification now treats queued UI and composer text as stronger than a transientworkingstatus, and only accepts “verified” when there is no pending evidence.Fail-closed behavior: when verification is requested, missing/empty/unreadable screens return
submit_verified: falseinstead of ambiguousnull(except the advisoryspawn_agentlauncher probe).send_tonow runs submit verification forallow_busypresses Enter, but still skips recovery Return on busy sends.Tests add Codex queue/composer probes, unreadable-screen failure, and update
send_commandexpectations to error when verification cannot prove submission.Reviewed by Cursor Bugbot for commit 191b963. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Reject false submit receipts in relay when on-screen verification evidence is absent
false) when no positive on-screen evidence is found, except forspawn_agentevents which returnnullas before.screenShowsQueuedAgentInputto detect a 'Messages to be submitted after next tool call' section; queued or pending input now blocks a verified result.allow_busysubmissions now request submit verification (previously suppressed it), without enabling recovery-enter retry.submit_verified=falseinstead of succeeding.📊 Macroscope summarized 191b963. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Summary by CodeRabbit
Bug Fixes
Tests