Repository navigation
Conversation
…n cards Codex 0.158 asks with request_user_input_async: an async agentMessage carrying questions, no server request, then sleep items until a user message steers the turn. Orca rendered it as plain text, so no card appeared and the session never went to attention. Register the async ask in the existing prompt registry and journal it through the existing question path. Answering a card steers the answer into the turn via turn/start instead of connection.respond; a typed user message resolves the open cards with its text; turn end cancels them as before. Blocking requestUserInput is unchanged.
…uestion async ask A typed reply closes every async ask on the thread and dropped answers already given on its cards, though the journal showed them resolved. Steer those collected answers after the reply so Codex receives what the cards show.
…ync ask Codex 0.158 ends a turn on its own while an async ask waits. The turn end forgot the prompt, so answers already given on its cards were never sent though the journal showed them resolved. Steer them once the turn completes; a Stop or failed turn is not restarted for them.
 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe change adds handling for Codex async user-input questions. It parses completed async question items into prompts, records card and typed-message answers in the journal, and sends applicable answers through Codex turn steering. Integration tests cover single- and multi-question flows, turn completion and interruption, and the blocking server-request path. Priority: ➖ Normal Merge Risk: 🔵 Low · up to Partially answered async question cards can occasionally have their answers lost or delivered at the wrong time when several threads or turns are active. The blocking question path is unchanged. Scoping the drain to the completed turn before merge would remove the issue. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Answers remain scoped to their conversation, with existing ownership checks preserved. However, automatically resumed work may briefly lack the tracking needed for Stop to cancel it before the new turn is announced. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 5578a01c-2d8f-456f-b9dd-4eb53f7489cb
📒 Files selected for processing (11)
src/main/codex/codex-async-user-input.tssrc/main/codex/codex-prompt-registry.tssrc/main/codex/codex-structured-async-question.test.tssrc/main/codex/codex-structured-journal-prompts.tssrc/main/codex/codex-structured-journal-translation.tssrc/main/codex/codex-structured-prompt-ownership.tssrc/main/codex/codex-structured-prompt-replies.tssrc/main/codex/codex-structured-provider-events.tssrc/main/codex/codex-structured-session-adapter.tssrc/main/codex/codex-structured-session-state.tssrc/main/codex/codex-structured-turn-start.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| const abandoned = session.prompts.takeAbandonedAsyncAnswers() | ||
| // Codex 0.158 ends a turn on its own while an async ask waits; a Stop or failure is not resumed. | ||
| if (readCodexTurnEnd(method, params)?.status === 'completed') { | ||
| for (const answered of abandoned) { | ||
| steerCodexPartialAnswers(session, answered, input.requestTimeoutMs) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -nP -C4 'clearPromptTurn' src/main/codexRepository: stablyai/orca
Length of output: 12707
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files 'src/main/codex/*' | rg 'codex-(structured-provider-events|structured-journal-settlement|structured-journal-translation-turn-boundaries|structured-journal-translation-turn-lifecycle|structured-session-acquire|structured-prompts|prompt)'
printf '%s\n' '--- clearTurn definitions and calls ---'
rg -n -C8 'clearTurn|clearPromptTurn|turn/completed|turnId|threadId' src/main/codex/codex-structured-prompt* src/main/codex/codex-structured-journal-settlement.ts src/main/codex/codex-structured-journal-translation-turn-boundaries.ts src/main/codex/codex-structured-provider-events.ts src/main/codex/codex-structured-session-acquire.ts 2>/dev/null
printf '%s\n' '--- changed provider-events file ---'
cat -n src/main/codex/codex-structured-provider-events.ts
printf '%s\n' '--- relevant diff ---'
git diff --unified=25 d74388f8a2dad2bd4bbfe3b937aba66e6648258b 80890da2573dce387649bb4332a8128b66e511ec -- src/main/codex/codex-structured-provider-events.ts src/main/codex/codex-structured-journal-settlement.ts src/main/codex/codex-structured-journal-translation-turn-boundaries.ts src/main/codex/codex-structured-session-acquire.tsRepository: stablyai/orca
Length of output: 43213
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- prompt registry ---'
cat -n src/main/codex/codex-prompt-registry.ts
printf '%s\n' '--- settlement path ---'
sed -n '1,230p' src/main/codex/codex-structured-journal-settlement.ts
printf '%s\n' '--- turn boundary path ---'
sed -n '1,240p' src/main/codex/codex-structured-journal-translation-turn-boundaries.ts
printf '%s\n' '--- relevant registry tests ---'
rg -n -C8 'clearTurn|abandoned|takeAbandoned|thread|turn' src/main/codex/codex-prompt-registry-retention.test.ts src/main/codex/codex-structured-journal-translation-turn-lifecycle.test.tsRepository: stablyai/orca
Length of output: 43091
Scope abandoned async answers to the completed turn that created them.
clearTurn matches prompts by thread and turn, but it appends answered async prompts to a session-wide queue. Every accepted notification drains that queue before checking its own turn status. A non-completed notification can therefore discard answers, and a completed notification from another thread can steer them based on the wrong turn. Drain only answers belonging to the current thread and turn after matching a completed turn notification.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return null | ||
| } | ||
| // Forget first, as for a blocking reply: a second answer must not steer twice. | ||
| registry.forget(prompt) |
There was a problem hiding this comment.
When the user answers the final card, the code removes the pending prompt before Codex confirms the turn/start request. If that request is rejected or times out, the card is already resolved and cannot be answered again. Orca shows an “answer unconfirmed” status, but Codex may still be waiting without a way to retry the answer. Keep the answer recoverable until delivery is confirmed.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two delivery gaps in the new async-answer path: a machine-generated steer can resolve an unrelated open card with the wrong text, and a failed turn end can silently drop an already-committed card answer. Neither is covered by the added tests.
Reviewed changes
- Async question recognition (
codex-async-user-input.ts): a completedagentMessagewithdelivery: 'async'and aquestionslist is reshaped into the existingrequestUserInputparams (title doubles as question id, anisOtherfree-text option is added), so the existing card/status machinery renders it. Also addsreadCodexUserMessageReply,codexAsyncAnswerText, andcodexAsyncPartialAnswerText. - Registry (
codex-prompt-registry.ts):registertakes adelivery: 'async'marker;forgetAsyncreturns/forgets every async prompt on a thread;clearTurncollects partly answered async prompts intoabandonedAsyncAnswersfortakeAbandonedAsyncAnswers. - Answer routing (
codex-structured-prompt-ownership.ts,-replies.ts,codex-structured-turn-start.ts): an async card answer is recorded, then delivered by steering aturn/startuser message (steerCodexTurn, noclientUserMessageId) instead of a JSON-RPC reply. - Typed replies and turn end (
codex-structured-journal-translation.ts,codex-structured-journal-prompts.ts,codex-structured-provider-events.ts): a live user message resolves open async cards with its text, and partly answered asks are steered when a typed reply or acompletedturn end closes them. - Test (
codex-structured-async-question.test.ts): 9 replayed-0.158-frame cases covering the card, both answer paths, multi-question partial delivery, turn-end cancellation, and the unchanged blocking path.
ℹ️ Nitpicks
steerCodexPartialAnswersis fire-and-forget (console.warnon failure) while the full-answer path awaitssteerCodexTurnand lets the host recordanswerUnconfirmed; a transient failure on the partial path leaves the cardresolvedwith no marker. Worth aligning or at least documenting.register's byte-cap eviction andtrim()delete async prompts without pushing them toabandonedAsyncAnswers, so recorded partial answers can be dropped under registry pressure — same resolved-but-undelivered shape, lower severity.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| export function readCodexUserMessageReply( | ||
| method: string, | ||
| params: unknown | ||
| ): { text: string | null } | null { | ||
| if (method !== 'item/started' && method !== 'item/completed') { | ||
| return null | ||
| } | ||
| const item = readCodexThreadItem(readRecord(params).item) | ||
| if (!item || item.type !== 'userMessage') { | ||
| return null | ||
| } | ||
| return { text: readTextContent(item, 'content') ?? readString(item, 'text') } | ||
| } |
There was a problem hiding this comment.
readCodexUserMessageReply accepts any userMessage item, which includes the echo of Orca's own steerCodexTurn message (it deliberately sends no clientUserMessageId). If a second async ask is still open on the thread when that steer echo lands, answerAsync resolves its card with the first ask's synthesized text and forgetAsync drops it, so the card shows an answer the user never gave and can no longer be answered. The "no stray user row" live check does not clear this: CodexJournalItems.handle suppresses live userMessage rows regardless of arming, so the reply path fires even with no visible row.
Technical details
# Steered answer echo resolves an unrelated async card
## Affected sites
- `src/main/codex/codex-async-user-input.ts:78` — matches any `userMessage`, not only a real send.
- `src/main/codex/codex-structured-journal-translation.ts:244` — calls `prompts.answerAsync(event.threadId, reply.text)` for every `item/started`/`item/completed`.
- `src/main/codex/codex-structured-provider-events.ts:49` — `forgetAsync(threadId)` then drops the other ask.
- `src/main/codex/codex-structured-turn-start.ts:127` — `steerCodexTurn` omits `clientUserMessageId`.
## Reachability
Precondition: two async asks open on the same thread (the design explicitly supports this; `answerAsync`/`forgetAsync` iterate all of them). Answer ask A's card → `steerCodexTurn` → Codex coalesces that input into the open turn and surfaces it as a `userMessage`. The echo is not distinguished from a real user reply, so `answerAsync` rewrites ask B's card to `resolved` with A's answer and `forgetAsync` forgets B.
## Required outcome
A machine-generated steer must not count as a live user reply for answering other async cards. Either correlate the reply to an armed submit (only real sends have a matching dispatch echo / client id) or mark steered inputs so the reply path ignores them.
## Open questions for the human
- Does 0.158 echo a `turn/start` that carries no `clientUserMessageId` as a `userMessage` item? No repo fixture captures the steer echo; the "no stray row" observation is explained by `CodexJournalItems.handle` suppressing live userMessage rows even while it routes them.| const abandoned = session.prompts.takeAbandonedAsyncAnswers() | ||
| // Codex 0.158 ends a turn on its own while an async ask waits; a Stop or failure is not resumed. | ||
| if (readCodexTurnEnd(method, params)?.status === 'completed') { | ||
| for (const answered of abandoned) { | ||
| steerCodexPartialAnswers(session, answered, input.requestTimeoutMs) | ||
| } | ||
| } |
There was a problem hiding this comment.
takeAbandonedAsyncAnswers() drains on every notification, but the answers are only steered when the end is completed. For interrupted that is the intended Stop behavior, but for a failed end a card the user already answered stays resolved in the journal while Codex never receives it, and unlike the awaited full-answer path there is no "unconfirmed" marker. The added regression test only covers interrupted.
Technical details
# Failed turn end drops a committed async card answer
## Affected sites
- `src/main/codex/codex-structured-provider-events.ts:54` — drains `takeAbandonedAsyncAnswers()` unconditionally.
- `src/main/codex/codex-structured-provider-events.ts:56` — steers only when `readCodexTurnEnd(...).status === 'completed'`.
- `src/main/codex/codex-prompt-registry.ts:250` — `clearTurn` abandons partly answered async prompts on any turn end.
- Host commit path — a clicked card is committed `resolved` immediately, before the ask is complete.
## Reachability
1. Multi-question async ask; user answers one card (committed `resolved`, held until the ask completes).
2. Codex ends the turn with `status: 'failed'` (not a user Stop).
3. `clearTurn` abandons the prompt, `takeAbandonedAsyncAnswers` drains it, the `completed` gate is false → no steer and no `answerUnconfirmed` marker.
4. Card stays `resolved`; Codex never receives the answer.
## Required outcome
On a `failed` end, either deliver the held answers (as for `completed`) or leave/cancel the card so the UI does not present an undelivered answer as accepted. `interrupted` (Stop) is a documented exception; `failed` is not.
## Open questions for the human
- Was `failed` folded into "not resumed" deliberately? It is untested; the regression test covers only `interrupted`.
|
@OrcaWin I've opened #30062 with an overlapping fix for structured Codex async questions, plus desktop/mobile support for chat backed by a terminal. This PR is linked in its description. One deliberate difference is the lifecycle: async questions stay separate from blocking approvals and survive turn completion or Stop. The host derives them from history and exposes them through the current-request transport merged in #29121. Answers use the existing immediate message path, and failed or unconfirmed delivery does not resolve the shared pending set. 384 focused tests and the build pass; fork CI approval and physical-device/live SSH checks remain pending. I'd appreciate a comparison against the real app-server behavior you captured, particularly delivery failures and turn-end handling. |
|
Thank you for this PR. Codex async questions now show as answerable cards on main via #31652, which covers what this PR targeted. Since this branch is conflicting, we are closing it as superseded. We appreciate the work. |
ELI5
Newer Codex (0.158) asks you a question in a new way: it writes the question as a normal chat message with a hidden tag saying "this is a question, here are the choices", then waits. Orca only knew the old way, where Codex asks through a separate "please answer this" request. So Orca showed the question as plain text, no answer buttons appeared, and the chat just looked stuck while Codex waited. This PR teaches Orca to spot the new-style question and turn it into the same answer card it already shows for old-style questions. When you pick an answer, Orca sends it to Codex as your next chat message, which is how Codex expects answers to these questions.
What Changed
Before (Codex 0.158): Codex asks "Which color do you prefer: red or blue?". Orca shows it as text, no question card appears, and the session stays "working" instead of "needs attention". Codex quietly sends
sleepsteps in the background while it waits, so from the user's side the chat looks frozen.After: the same question shows up as a normal pending question card (options Red / Blue, plus free text), and the session goes to "attention". Picking an option or typing a reply answers it, and Codex carries on.
Mechanism. It reuses the existing question-card system instead of adding a second one.
src/main/codex/codex-async-user-input.ts: recognises a completed CodexagentMessagewithdelivery: 'async'and aquestionslist, and reshapes it into the same parameters an old-style blockingitem/tool/requestUserInputcarries. The question title becomes the question id. Each string option becomes{label}, and an "Other" (free text) choice is added, because any message the user types answers an async question.codex-structured-provider-events.ts: registers the async question in the existingCodexPromptRegistry(markeddelivery: 'async') and emits the normalpromptevent. The existing question-card builder, pending-question tracking and status projection then do the rest.codex-structured-prompt-ownership.ts,codex-structured-prompt-replies.ts, newsteerCodexTurnincodex-structured-turn-start.ts): the answer is written to the journal first, as before. For an async question, Orca then sends it as a user message (turn/starton the question's thread) instead of a JSON-RPC reply, since there is no server request to reply to. A multi-question ask sends one message ("Title: answer" per line) once all cards are answered.codex-structured-journal-prompts.ts, newanswerAsync): a live user message on the thread resolves every open async card there, recording the typed text as the free-text answer. Any card answers already given on a partly answered ask are still delivered.requestUserInputpath is unchanged and covered by a test.Why
Found in a crash-report scan: a user note on Slack report ts
1790733136.887819(Orca 1.4.217, Codex CLI 0.158) said interactive questions from Codex still didn't work. I reproduced it against the realcodex app-server0.158.0 and captured its frames.Alternatives considered:
respondToPromptpath.CLEAR UNDENIABLE fix evidence
Repro command:
The test replays frames captured from the real Codex 0.158.0 app-server: an
item/completedagentMessage withdelivery: 'async'andquestions: [{title:'Which color do you prefer: red or blue?', options:['Red','Blue']}], followed bysleepitems.Real 0.158.0 frames (captured by driving
codex app-serverdirectly):Before (main source files restored, new module removed, first version of the test file):
The one passing test covers the older blocking
requestUserInputpath, which should pass both before and after. With the final 9-test file against main's source, the adversarial reviewer saw 8 of 9 fail (only the blocking-path test passes).After (this branch, rebased on latest
main):Live end to end with real
codex app-server0.158.0 through Orca's real host, journal and Codex adapter (a scratch harness, not committed):Trade-offs (negative user-facing changes)
Adversarial review
3 loops, run until clean. The final loop came back clean.
76dcbbcbce, with a regression test.80890da257, with a regression test and a test that Stop does not restart work. The reviewer also checked two things that are not defects: the steered echo creates no stray user row, and a second ask in the same turn is resolved by the next user message, which matches the design.Linked Issue
None. Found by the crash-report scan (user note on Slack report ts
1790733136.887819).Visual Proof
N/A for screenshots: this is a main-process protocol change that makes the existing question card appear. The live run against real Codex 0.158.0 above shows the pending card row, the 'attention' status and the resolved answer.
Testing
Checks rerun after rebasing on latest
origin/main:pnpm test src/main/codex/codex-structured-async-question.test.ts: 9/9 pass.pnpm test src/main/codex src/main/native-chat: Test Files 447 passed, 3 skipped (450); Tests 4314 passed, 13 skipped.pnpm tc:node: clean.oxlinton all 11 changed files: exit 0.check:code-quality:changed: passed (0 findings) on the original fix commit.Live run against real
codex app-server0.158.0 on macOS (above).I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)