Repository navigation
fix(codex): allow explicit resolution of indeterminate delivery blocking forks - #2051
techotaku39 wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Review mode: initial
Requirement — Pass
Explicitly resolving unknown delivery provides a justified recovery path when an idle Codex session remains blocked from Fork despite unsuccessful Cancel and Retry.
Evidence
- Issue #2050 documents an empty native queue, absent consumption history, persistent indeterminate delivery, and Fork returning HTTP 409 after recovery.
- cli/src/codex/shared/root.ts:134 preserves uncertainty rather than treating missing history as proof that replay is safe.
Approach — Pass
A separate abandonment state, confirmed by the user and persisted by the worker before Hub resolution, addresses the blocker without fabricating consumption or replaying potentially executed input.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes native state and checks turn activity before abandonment.
- cli/src/codex/shared/queue.ts:200 verifies queue absence and persists abandonment before acknowledging it.
- hub/src/store/messages.ts:875 retains the original correlation row and creates an explanatory note transactionally.
Code — Reviewed
Reviewed the full fixed-head merge-base diff. Two client defects remain in resolution-note rendering and concurrent cancellation recovery.
- [Minor] Preserve distinct abandonment notes on native clients —
web/src/chat/reducerEvents.ts:75 - [Minor] Reconcile absent rows before restoring a busy cancellation —
web/src/hooks/mutations/useCancelQueuedMessage.ts:100
Testing
Not run (automation; PR code execution is prohibited).
- At inspection, fixed-head CI passed drift-gate, integration, and windows-codex-mcp; the main test job remained in progress.
- Added tests cover persistence failures, lost acknowledgements, consumption races, and Web note deduplication, but omit native resolution-note conformance and concurrent Cancel/Abandon recovery.
- No PR code, tests, builds, or scripts were executed during this read-only review.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicit resolution provides a justified recovery path when unknown delivery blocks Fork despite an idle session and unsuccessful Cancel/Retry.
Evidence
- Issue #2050 documents the persistent blocker with an empty native queue and no matching consumption history.
- Base cli/src/codex/shared/queue.ts preserves uncertainty, while hub/src/sync/syncEngine.ts blocks history actions on uninvoked messages.
Approach — Pass
A confirmed, durable abandonment state resolves HAPI's blocker without replaying input or fabricating consumption.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes native state and checks execution activity.
- cli/src/codex/shared/queue.ts:200 persists abandonment before acknowledgement.
- hub/src/store/messages.ts:875 transactionally retains correlation and creates an explanatory note.
Code — Reviewed
Reviewed the full fixed-head merge-base diff. Both previous findings are addressed; one native synchronization defect remains.
- [Minor] Keep duplicate sends from broadcasting abandoned rows —
hub/src/sync/messageService.ts:934
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed during this read-only review.
- Fixed-head CI passed test, integration, fixture drift-gate, iOS package-tests, Android build, and both Android transcript-layout jobs. iOS app-build remained in progress.
- windows-codex-mcp failed with a shim-response timeout in unchanged cli/src/codex/utils/codexMcpProxy.test.ts:146; causation by this diff is not established.
- Added coverage addresses both previous findings, persistence failures, consumption races, and restart behavior; abandoned duplicate-send SSE coverage is missing.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicit resolution provides a justified recovery path when indeterminate delivery persists in an idle session and Cancel/Retry cannot clear the Fork blocker.
Evidence
- Issue #2050 documents an empty native queue, absent matching consumption history, and persistent Fork rejection after Cancel and Retry.
- Base cli/src/codex/shared/queue.ts preserves unknown outcomes; hub/src/sync/syncEngine.ts rejects history actions while uninvoked messages remain.
Approach — Pass
Confirmed, durable abandonment resolves HAPI's blocker without replaying input or fabricating consumption. Retaining correlation permits later authoritative reconciliation.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes native state and checks execution activity; queue.ts:200 persists abandonment before acknowledgement.
- hub/src/store/messages.ts:875 transactionally retains the original row and creates an explanatory note.
- hub/src/sync/messageService.ts:927 reconciles abandoned duplicate sends through cancellation events instead of pending-message broadcasts.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and relevant surrounding code. The previous duplicate-send finding is addressed with regression coverage. No remaining actionable defects were identified; runtime behavior was not independently exercised.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed during this read-only review.
- Fixed-head CI passed integration, windows-codex-mcp, fixture drift-gate, iOS package-tests, Android build, and both Android transcript-layout jobs. The test job and iOS app-build remained in progress when checked.
- Added coverage addresses durable abandonment, restart behavior, persistence failures, consumption races, confirmation handling, concurrent Cancel reconciliation, duplicate-send cancellation events, and cross-client note deduplication.
- The snapshot reports successful original-session recovery acceptance, but explicitly records a cold-reopen HTTP 500/timeout before subsequent recovery; startup deadlines remain outside this fix.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicit resolution provides a justified recovery path when uncertain Codex delivery leaves an idle session blocked after Cancel and Retry fail.
Evidence
- Issue #2050 documents absent native queue/history entries, persistent indeterminate delivery, and rejected Fork attempts.
- Base cli/src/codex/shared/queue.ts preserves unknown outcomes; hub/src/sync/syncEngine.ts blocks history actions while uninvoked messages remain.
Approach — Pass
Confirmed, durable abandonment clears the blocker without replaying input or claiming consumption, while retaining correlation for later authoritative acknowledgements.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes native state and checks execution activity; queue.ts:200 persists abandonment before acknowledgement.
- hub/src/store/messages.ts:875 retains the original row, advances the message epoch, and excludes abandoned delivery from pending messages.
Suggestions
- Update docs/api/client-contract/messages.md:26: it still promises a separate note, contradicting the final implementation and pagination contract.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. One client reconciliation defect remains; runtime behavior was not independently exercised.
- [Minor] Reconcile abandoned retries using their local ID —
hub/src/sync/messageService.ts:931
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed during this read-only review.
- Fixed-head CI passed test, integration, windows-codex-mcp, fixture drift-gate, Android build and both transcript-layout jobs, and iOS package-tests. iOS app-build remained in progress.
- The duplicate-send regression test checks the emitted event but does not exercise its web SSE consumer against an optimistic row.
- The snapshot reports real-Codex recovery acceptance, with a cold-reopen HTTP 500/timeout before subsequent recovery; startup deadlines remain outside this fix.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicit resolution is justified when uncertain Codex delivery leaves an idle session blocked after Cancel and Retry fail.
Evidence
- Issue #2050 documents absent native queue/history entries, persistent indeterminate delivery, and rejected Fork attempts.
- Base hub/src/sync/syncEngine.ts:1199 blocks history actions while uninvoked messages remain.
Approach — Pass
Confirmed, durable abandonment provides recovery without replaying input or falsely claiming consumption.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes native state and checks execution activity; queue.ts:200 persists abandonment before acknowledgement.
- hub/src/store/messages.ts:875 retains correlation while excluding abandoned rows from pending delivery and visible history.
Code — Reviewed
Reviewed the full fixed-head merge-base diff. The previous SSE finding and documentation mismatch are addressed; one cancellation regression remains.
- [Minor] Preserve cancellation when indeterminate input remains natively queued —
web/src/components/AssistantChat/QueuedMessagesBar.tsx:368
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed during this read-only review.
- Fixed-head CI passed test, integration, windows-codex-mcp, fixture drift-gate, Android build and both transcript-layout jobs, and iOS package-tests. iOS app-build remained in progress.
- Added tests cover refusal to abandon native queued input, but do not verify that the UI retains a working cancellation path for that state.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicit resolution is justified when unknown Codex delivery leaves an idle session blocked after Cancel and Retry fail.
Evidence
- Issue #2050 documents empty native queue/history, retained indeterminate delivery, unsuccessful Cancel/Retry, and rejected Fork attempts.
- Base hub/src/sync/syncEngine.ts:1199 rejects history actions while uninvoked messages remain.
Approach — Pass
Confirmed, durable abandonment provides recovery without replaying input or falsely claiming consumption. Trying normal cancellation first preserves recovery for input still natively queued.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes history and checks execution activity; queue.ts:200 persists abandonment before acknowledgement.
- hub/src/store/messages.ts:875 retains correlation while excluding abandoned rows from pending delivery and visible history.
- web/src/hooks/mutations/useAbandonIndeterminateMessage.ts:13 attempts normal cancellation before abandonment.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff. The previous cancellation finding is addressed; one late-consumption race remains.
- [Minor] Preserve consumption received before the abandonment response —
web/src/hooks/mutations/useAbandonIndeterminateMessage.ts:25
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed during this read-only review.
- Fixed-head CI passed integration, windows-codex-mcp, fixture drift-gate, Android build and both transcript-layout jobs, and iOS package-tests. The test job and iOS app-build remained in progress.
- Added tests cover cancellation after reconnection, persistence failures, duplicate sends, and consumption races, but not authoritative consumption arriving before a delayed abandonment HTTP response.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicit recovery is justified when indeterminate Codex delivery leaves an idle session blocked from Fork after Cancel and Retry fail.
Evidence
- Issue #2050 documents an empty native queue, absent matching history, retained unknown delivery, and unsuccessful Cancel/Retry.
- Base hub/src/sync/syncEngine.ts:1199 blocks history actions while uninvoked messages remain.
Approach — Pass
Confirmed, durable abandonment clears the blocker without replaying input or claiming consumption. Trying normal cancellation first handles input still queued after reconnection.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes native history and checks execution activity; queue.ts:200 persists abandonment before acknowledgement.
- hub/src/store/messages.ts:875 retains correlation while excluding abandoned input from pending delivery and visible history.
- web/src/hooks/mutations/useAbandonIndeterminateMessage.ts:13 attempts cancellation first and preserves consumption received before a delayed response.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and relevant surrounding code. No actionable defects found. The previous delayed-response consumption finding is addressed with regression coverage. Runtime behavior was not independently exercised.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed during this read-only review. The diff whitespace check passed.
- Fixed-head CI passed integration, windows-codex-mcp, fixture drift-gate, Android build, both transcript-layout jobs, and iOS package-tests. The test job and iOS app-build remained in progress.
- Added coverage addresses persistence failures, duplicate-send suppression, cancellation after reconnection, both Fork modes, and authoritative consumption arriving before a delayed abandonment response.
HAPI Bot
There was a problem hiding this comment.
Review mode: follow-up after new commits
Requirement — Pass
Explicit recovery is justified when indeterminate Codex delivery blocks Fork after Cancel and Retry cannot resolve it.
Evidence
- Issue #2050 documents an idle restored session with an empty native queue, absent matching history, and unsuccessful Cancel/Retry.
- Base hub/src/sync/syncEngine.ts:1199 blocks history actions while uninvoked messages remain.
Approach — Pass
Confirmed, durable abandonment clears the blocker without replaying input or claiming consumption. Retained correlation allows later authoritative reconciliation.
Evidence
- cli/src/codex/shared/root.ts:125 refreshes native history and checks activity; queue.ts:200 persists abandonment before acknowledgement.
- hub/src/store/messages.ts:875 retains the original row while excluding it from pending delivery and visible history.
- web/src/hooks/mutations/useAbandonIndeterminateMessage.ts:13 attempts normal cancellation first; web/src/hooks/useSSE.ts:41 preserves established consumption across delayed cancellation events.
Code — Reviewed
Reviewed the entire fixed-head merge-base diff and relevant surrounding code. No actionable defects found. Runtime behavior was not independently exercised.
No reportable code issues found.
Testing
Not run (automation; PR code execution is prohibited).
- No PR code, tests, builds, or scripts were executed. The read-only diff whitespace check passed.
- Fixed-head CI passed integration, windows-codex-mcp, fixture drift-gate, Android build, both transcript-layout jobs, and iOS package-tests. The test job and iOS app-build remained in progress.
- Added coverage addresses persistence failures, duplicate-send suppression, both Fork modes, cancellation after reconnection, and consumption preceding delayed HTTP responses or cancellation events in both SSE paths.
HAPI Bot
Summary
Interrupted Codex compaction can leave accepted input absent from native queue and user-message history. HAPI retains the uncertain delivery, but Cancel and Retry cannot resolve it, leaving current and historical Fork blocked.
abandonedledger state before acknowledging resolution.The native input-persistence problem is tracked separately in openai/codex#54822.
Validation
hub:bun test src/sync/abandonMessage.test.ts src/web/routes/messages.test.ts— 37 passed. Updated no-note assertions failed against the previous implementation and passed after the change. Covers persistence failure, consumption races, no additional stored/SSE chat message, duplicate-send suppression, and current/historical Fork.bun run test:web -- src/hooks/mutations/useAbandonIndeterminateMessage.race.test.tsx src/hooks/useSSE.test.ts src/hooks/mutations/useCancelQueuedMessage.test.tsx src/hooks/mutations/useAbandonIndeterminateMessage.test.tsx src/components/AssistantChat/QueuedMessagesBar.test.tsx src/lib/message-window-store.test.ts— 156 passed. Both global and full SSE paths reproduce and fix cancellation of optimistic attachment retries; server-echoed rows, older events without local IDs, unrelated messages, and consumption received before delayed removal responses are covered.bun typecheck,bun run test:web(3,380 passed),bun run test:shared(327 passed),bun run gen:fixtures, andgit diff --check— passed.bun run typecheck:webandbun run buildalso passed after the final dialog changes.pwsh -NoProfile -File .\scripts\Invoke-HapiTaskPlaywright.ps1 -Name investigate-session-fork-queue -Suite Root terminal-wrap-fidelity.spec.ts— 2 passed.-Suite Root queued-resolution.spec.ts --retries=0— 14 passed in Chromium desktop English (1440×900) and touch/mobile Chinese emulation (390×844). Uses actual UI, hooks, API client and store with mocked API responses. Covers exact copy, centered title, red confirmation, dismissal without writes, pending acknowledgement, unavailable-response retry, removal, offline Cancel followed by reconnection with native input still queued, consumption before a delayed response, and ordinary/scheduled cancellation. No page errors or horizontal overflow.Original-session acceptance: Before the UI/no-note follow-up, independent copies of all 3,863 affected Hub rows, the complete ledger, and three native rollout files were tested with real Codex 0.162.1. Baseline main
317e39a0reproduced the blocker; recovery verified both Fork modes, persisted abandonment after restart, no replay, and no false consumption. Original source hashes stayed unchanged. The final no-note behavior is covered by the Hub and browser tests above; the real-Codex clone was not rerun for that presentation change.Validation limit: The fixed clone's cold-reopen request returned HTTP 500/timeout before native execution subsequently became active; assertions explicitly waited for recovery. This PR does not fix startup deadlines or establish completion of abandoned input.
Compatibility
No dependency or database schema-version change. Update CLI/Runner and Hub together: older CLI versions cannot read ledger entries containing the new
abandonedstate.Related Issues
Fixes #2050
Refs openai/codex#54822
AI assistance: OpenAI Codex (GPT-6).