feat(codex): answer asynchronous Codex questions from ChatMux - #158
Yoonwoo-Ha wants to merge 3 commits into
Conversation
## Summary - Codex can queue a question mid-turn instead of blocking the composer: the pane shows a `? N questions … to answer` summary and the question panel only opens on Shift+Left, so ChatMux saw no prompt and the user had to answer in the terminal. - The transcript now surfaces such a question as its own tool call, tagged so the relay composer renders it even though it carries no options until the panel is open. - Answering opens the queue when the panel is closed (Shift+Left), then navigates and submits like any other selection; skipping sends Shift+Right in place of Escape, and free-text answers go through the existing custom input path. - The pane's activity state reflects a queued question, so a session waiting on one is not reported as idle. ## Test plan - [x] `tmux-ask-selection.service.test.ts`, `tmux-interactive-prompt.service.test.ts`, `codex-sessions.test.ts`, `external-session-activity.service.test.ts`, `pendingRelayAsk.test.ts` - [x] `npm test` (the 6 `server/self-update.test.ts` failures also fail on an unmodified `main` worktree on this host), `npm run typecheck`, `npm run lint` Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
devswha
left a comment
There was a problem hiding this comment.
Review: REQUEST_CHANGES\n\nVerified against PR head 01e23fe. Five blocking issues, most around the "fail closed when identity is stale or uncertain" invariant and the M5B approval contract:\n\nBlocking\n\n1. Free-text async questions send typed text into the pane without live-widget evidence. 's branch ( :286-295) accepts a capture when a loose hint line () appears anywhere plus the question text within 20 preceding lines — no selection cursor, no focused input row. (:478-487) delegates to exactly that check, so 'menu collapsed' vs 'answer box focused' are indistinguishable, and then pastes text + Enter. Since capture includes 80 lines of scrollback, a stale rendering can authorize typing into whatever is actually focused. Every other provider's custom-input parser requires live state (codex selected, gjc selected + row, omp + , claude delta 0). This also widens M5B §1.2 ("표시된 선택지만 허용") — see point 2's contract note.\n\n2. A key is sent before the pending prompt is matched on screen. :568-576: when the pending question fails to parse, the only gate before is — two unrelated regexes matching any two lines — with zero relation to the pending question's identity; it also fires on an out-of-range (route accepts up to 32), so a bad parameter moves native focus. M5B §6 is explicit: a broken parse must make the UI disappear, not emit keys. Gate the expansion on the pending question's own text in the collapsed summary, or fail closed. Also: M5B §1.1 enumerates supported parsers/responders and was revised (rev.3/rev.4) per provider addition — this PR adds a responder plus a pre-answer navigation keystroke with no contract revision, which AGENTS.md forbids for normative docs.\n\n3. The expansion path synchronizes on a fixed 80ms sleep (:573-575): → → one re-capture. A slower repaint yields a stale capture and a 409 after the pane's focus already changed, leaving the TUI expanded while ChatMux reports the prompt gone. Needs a bounded re-capture loop keyed on parse success. The new test flips synchronously in the fake runner, so it cannot catch this.\n\n4. Zero-row async prompts are unanswerable dead ends. supports (, ), but throws for every request when ( fires before the cancel branch) — yet the client renders both direct input and cancel. Every action on such a card 400s. Bound on or don't emit zero-row prompts.\n\n5. ** overrides terminal outcomes and has no clearing path other than an exact string match.** ~:360 returns before the reverse scan, so /// are suppressed while an async question lacks a reply starting exactly with . A session that fails or is aborted with a question outstanding reports ; a natively skipped question (, which the hint line itself advertises) pins the session to until the record scrolls out of the 128KB tail. Needs an explicit clear on skip/abort/failure and must not outrank failed/aborted.\n\nNon-blocking highlights: the dedupe and the framing are load-bearing but only proven by hand-written test fixtures — a captured rollout fixture would make them evidence; the answered card renders "Skipped" ( is set but reads ); wire contract is hardcoded in three places instead of ; stacked questions deadlock fail-closed (transcript surfaces only the newest, native TUI answers oldest first) — safe but worth a UI hint.\n\nHolding merge until these are addressed. The scroll-free detection approach in the rest of the PR is solid; the fixes look localized.
devswha
left a comment
There was a problem hiding this comment.
Review: REQUEST_CHANGES
Verified against PR head 01e23fe. Five blocking issues, most around the "fail closed when identity is stale or uncertain" invariant and the M5B approval contract.
Blocking
-
Free-text async questions send typed text into the pane without live-widget evidence.
parseCodexAsyncAskSelectionScreen'slabels.length === 0branch (server/modules/providers/services/tmux-ask-selection.service.ts:286-295) accepts a capture when a loose hint line (:478-487) delegates to exactly that check, so "menu collapsed" vs "answer box focused" are indistinguishable, andCODEX_ASYNC_HINT_RE) appears anywhere plus the question text within the preceding 20 lines — no selection cursor, no focused input row.parseCodexAsyncAskCustomInputScreen(submitPendingTmuxAskCustomResponsethen pastes text and presses Enter. Capture includes 80 lines of scrollback, so a stale rendering of the same question can authorize typing into whatever is actually focused. Every other provider's custom-input parser requires live state (codex› Add notesselected, gjc selectedOther+>row, ompCustom answer:+>, claude delta 0). -
A key is sent before the pending prompt is matched on screen. ~:568-576: when the pending question fails to parse, the only gate before
sendTmuxSelectionKeys(target, ['S-Left'])iscodexAsyncQuestionSummaryIsVisible— two unrelated regexes matching any two lines of the capture, with zero relation to the pending question's identity. It also fires on a merely out-of-rangeoptionIndex(the route accepts up to 32), so a bad parameter moves native focus. M5B §6 is explicit that a broken parse must make the approval UI disappear rather than emit keys. Gate the expansion on the pending question's own text being present in the collapsed summary, or fail closed. Also: M5B §1.1 enumerates the supported parsers/responders and was revised (rev.3, rev.4) each time a provider path was added — this PR adds acodex-async-questionresponder and a pre-answer navigation keystroke with no contract revision, which AGENTS.md forbids for normative documents. -
The expansion path synchronizes on a fixed 80ms sleep (~:573-575):
S-Left,setTimeout(80), one re-capture. A slower repaint yields a stale capture and a 409TMUX_ASK_PROMPT_STALEafter the pane's focus has already changed, leaving the TUI expanded while ChatMux reports the prompt gone. This needs a bounded re-capture loop keyed on the parse succeeding. The new test flipsexpandedsynchronously inside the fake runner, so it cannot catch this. -
Zero-row async prompts are unanswerable dead ends.
parseCodexAsyncQuestionsupportsrows.length === 0(options: [],customOptionNumber: 1), butvalidateChoicesthrowsTMUX_INTERACTIVE_CHOICE_INVALIDfor every request whenoptions.length === 0(choices.length > prompt.options.lengthfires before the cancel branch) — yet the client renders both direct input and cancel for such cards. Bound oncustomOptionNumber ?? options.lengthor do not emit zero-row prompts. -
hasPendingCodexAsyncQuestionoverrides terminal outcomes and has no clearing path other than an exact string match.server/modules/providers/services/external-session-activity.service.ts~:360 returnsasking_userbefore the reverse scan, soturn_aborted,turn_failed,error, andtask_complete/turn_completeare all suppressed while any async question title lacks a reply beginning exactly with> {title}\n\n. A session that fails or is aborted with a question outstanding reportsasking_userinstead of the terminal state; a question skipped natively (ctrl+] skip, which the hint line itself advertises) or answered with any other framing pins the session toasking_useruntil the record scrolls out of the 128KB tail. Needs an explicit clear on skip/abort/failure, and it must not outrank failed/aborted.
Non-blocking highlights
- The
item.id === call_iddedupe and the> {title}\n\nanswer framing are load-bearing but only proven by hand-written test fixtures that construct both sides — a captured rollout fixture would turn them into evidence. - The answer never becomes visible: linkage sets
toolResult, but the AskUserQuestion card readsinput.answers, so an answered async card renders "Skipped". - The
_chatmux.kindwire contract is hardcoded as a bare string in three places instead of living inshared/. - Stacked questions deadlock fail-closed: the transcript surfaces only the newest async message while the native TUI presents the oldest first — safe, but worth a UI hint.
The overall structure (async question synthesis, session pinning, screen parsers reusing the existing ask-selection machinery) is sound; the fixes look localized. Holding merge until the blocking items are addressed.
## Summary Prepares the v1.10.0 release. Version bookkeeping only - no runtime code changes. - `package.json` version -> 1.10.0 - both `package-lock.json` version fields -> 1.10.0 - exact `packaging/release/update-compatibility.json` entry for 1.10.0 ## Schema generation and rollback The migration registry is unchanged since v1.9.2 (`git diff v1.9.2 main -- server/modules/database` is empty). Schema generation therefore stays at 20 and rollback stays exact, carrying forward every version declared by 1.9.2 plus 1.9.2 itself: 1.8.17, 1.8.18, 1.8.19, 1.9.0, 1.9.1 and 1.9.2. `npm run release:check-metadata` passes with exactly this declaration. ## Release contents since v1.9.2 - #157 - feat(chat): load older messages by click, not only by scrolling - #159 - fix(claude): detect dangerous rm approvals Minor bump (1.9.x -> 1.10.0) because #157 lands a user-facing feature, matching the 1.8.19 -> 1.9.0 precedent. #158 (feat(codex): answer asynchronous Codex questions) is intentionally not part of this release: review requested changes on five blocking issues (unsafe free-text send path, keystroke before prompt match, fixed-delay resync, unanswerable zero-option prompts, terminal-outcome suppression). ## Verification - [x] `npm run release:check-metadata` - passed (schema generation 20, 6 rollback-compatible versions) - [x] `npm run check:identity` - passed (3230 source, 1317 generated, 12 archive files) - [x] version alignment asserted across all three files - [ ] CI Verify Node 22 / 24, canonical server bundle
|
리뷰 반영 완료했습니다 (
검증: |
Summary
Codex can queue a question mid-turn rather than blocking its composer. The pane then shows only a summary line —
? 2 questions … to answer— and the question panel opens on Shift+Left. ChatMux saw no prompt in that state, so the only way to answer was to attach to the terminal and press the key there.This upstreams the handling that has been running locally:
codex-async:<messageId>), marked so the relay composer renders it even though it carries no options until the panel is open.Shift+Leftopens the queue, the pane is re-captured, and the choice is then navigated and submitted like any other selection. Skipping sendsShift+Rightwhere other CLIs takeEscape, and a free-text answer reuses the existing custom-input path.S-Left/S-Rightjoin the selection key vocabulary.A stale summary fails closed with the usual stale-prompt error rather than sending keys into a pane whose state has moved on.
Test plan
tmux-ask-selection.service.test.ts— async selection/other/cancel parsing and the_chatmuxmarkertmux-interactive-prompt.service.test.ts,codex-sessions.test.ts,external-session-activity.service.test.ts,pendingRelayAsk.test.tsnpm test,npm run typecheck,npm run lintNote:
npm testreports 6 failures inserver/self-update.test.ts(403 vs 409) that also fail on an unmodifiedmainworktree on this host; unrelated to this change.🤖 Generated with Claude Code