Repository navigation
test(codex): pin the stopped-send receipt rule where #30403 moved it (fix red main) - #30865
Conversation
…since #30403 #28046's dispatch-boundary tests pinned that the Codex adapter drops an echo for a stopped or refused send. #30403 moved that rule to the journal's queued late-accept write. Pin it there through the host's Stop path, and keep the adapter tests asserting what still holds at the adapter: no fallback or second turn/start write, and the earlier send's receipt survives an abandoned wait.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the test-only change and the stopped-send receipt rule it covers. It is specific and related to the main changes. |
| Description check | |
The description covers the main required sections, including the plain-language summary, changes, rationale, testing, visual proof, and scope notes. It does not provide a linked issue and does not inc… |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description covers the main required sections, including the plain-language summary, changes, rationale, testing, visual proof, and scope notes. It does not provide a linked issue and does not include the repository checklist, but the description is otherwise complete and focused.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
ELI5
Two tests on main went red because two PRs that landed the same day disagreed about where a rule is checked. The rule: a message you stopped or that Codex refused must never later be shown as delivered. That rule still holds in the app. One PR moved the check to a different place, and the other PR's tests were still looking for it in the old place. This PR updates those tests and adds a test that pins the rule where it now lives.
What Changed
Before (main):
src/main/codex/codex-structured-dispatch-boundary.test.tsfails "stops a refused steer before its fallback write" and "abandons only the stopped message while waiting for a previous turn to open". Each asserts that the Codex adapter passes on nothing when Codex echoes the id of a stopped send.Why they fail: #30403 changed the Codex adapter so that every exact echo of a message id is passed on, which lets a second echo retry a receipt whose first save failed. The "a send that is already settled stays settled" decision moved into the chat journal's queued write (
JournalSubmissionWriter.acceptLate), which writes nothing for a send that is already rejected or accepted. #30403 deleted the matching adapter-level assertions in the other Codex test files. #28046 added these two tests in parallel, so the two changes crossed.Is this a production bug? No. A Stop during a send makes the host record that send as a final
rejected(cancelled) answer (dispatchSafely/cancelledDispatchinstructured-agent-session-handover.ts). A later receipt for it is dropped by the journal. A send Stop withdrew before it was ever written can't be echoed by Codex, because its id never reached Codex. The one remaining race (an echo arriving before the host writes the cancelled row) can only recordacceptedwhen Codex has actually proved it holds the message, which is the honest answer. That ordering is already intended and covered ("publishes acceptance during a pending send and never reopens it for retry").Changes (tests only):
turn/startafter a refused steer, only oneturn/startwhile waiting) and now expect the echo to be passed on, following fix(native-chat): save a Codex message's receipt from either of Codex's reports of it, not only the first #30403's convention. The second test now echoes the earlier message, so it checks what its title says: the abandoned wait leaves the earlier send's receipt intact.structured-agent-session-late-settlement.test.ts: Stop a send whose dispatch is waiting on its abort signal, then deliver a receipt naming it. The send staysrejected/cancelledand the journal adds no dispatch row. I removed theacceptLateguard temporarily to confirm the test fails without it.Why
Both PRs' guarantees survive: #28046's "a stopped or refused send is never delivered or confirmed" is now pinned end to end through the host's Stop path, and #30403's "either of Codex's echoes saves the receipt" is unchanged. Restoring the adapter-level filter was rejected: it would bring back the bug #30403 fixed (a failed first receipt save could not be retried), and it would duplicate the journal's single source of truth.
Both #28046 and #30403 are by @brennanb2025. Credit for both behaviours goes to them; this PR only reconciles the tests.
Linked Issue
None. Fixes red main (introduced by 43abd59, #30403, crossing 5319125, #28046).
Visual Proof
N/A, test-only change with no UI or behaviour change.
Testing
src/main/codex/codex-structured-dispatch-boundary.test.tsandstructured-agent-session-late-settlement.test.ts: 13/13 pass. The new host test fails when the journal'sacceptLateguard is removed.All of
src/main/codex/single-worker: 282 files, 2884 tests pass.The local static analysis job steps from
pr.ymlall pass (import boundaries, lint, anti-slop, native and type-aware code quality, changed-code gate, React Doctor,typecheck:ci, reliability gates, every ratchet including focus-read).Codex review (gpt-6.1-sol, high) found nothing on both heads. An independent Opus review agreed it is not a production bug. Its low finding (the test did not pin the writer guard itself) and its naming nits are fixed.
Nothing was verified live: test-only.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Agent skill upstream boundary
Notes
Test-only. No effect on security, platforms, SSH, mobile or wire compatibility.