-
Notifications
You must be signed in to change notification settings - Fork 219
fix(codex): detect the runtime approval prompt codex 0.147.0 actually renders #567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0c6c4e2
4105be6
a9ee828
7172100
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,9 +126,86 @@ | |
| UPDATE_DIALOG_FOOTER = TRUST_PROMPT_FOOTER | ||
| STARTUP_PROMPT_BOTTOM_LINES = 15 | ||
| STARTUP_ACTIVITY_PATTERN = r"^\s*•[^\S\n]+\S" | ||
| # Codex's runtime approval prompt as actually rendered by codex-cli 0.147.0, | ||
| # verified against a live tmux capture (test/providers/fixtures/ | ||
| # codex_approval_modal_raw.txt): | ||
| # | ||
| # Would you like to run the following command? | ||
| # | ||
| # Environment: local | ||
| # | ||
| # $ mkdir -p /private/tmp/codex-work-567 | ||
| # | ||
| # › 1. Yes, proceed (y) | ||
| # 2. Yes, and don't ask again for commands that start with `mkdir -p ...` (p) | ||
| # 3. No, and tell Codex what to do differently (esc) | ||
| # | ||
| # Press enter to confirm or esc to cancel | ||
| # | ||
| # It is NOT a box-drawn modal and carries no "[a] Accept"/"[d] Decline" keys: it | ||
| # is a numbered menu with a `›` selection cursor, structurally identical to the | ||
| # trust-v2, login, and update dialogs above -- hence the same question+footer | ||
| # corroboration shape. The three question variants are the exec, apply_patch, and | ||
| # permission-escalation approvals; all three block the TUI on a keystroke, and all | ||
| # three are present in the 0.147.0 binary's string table. | ||
| # | ||
| # Left un-anchored to the `›` cursor line on purpose: the cursor moves between the | ||
| # numbered options as the operator arrows around, so the question and the footer | ||
| # are the only two positionally stable rows. | ||
| APPROVAL_PROMPT_PATTERN = ( | ||
| r"Would you like to (?:run the following command" | ||
| r"|make the following edits" | ||
| r"|grant these permissions)\?" | ||
| ) | ||
| APPROVAL_PROMPT_FOOTER = r"Press enter to confirm" | ||
|
|
||
| # Codex's boxed command-approval modal: | ||
| # ╭─ Command Approval Required ─╮ | ||
| # │ [a] Accept [d] Decline │ | ||
| # ╰─────────────────────────────╯ | ||
| # WARNING: this copy is NOT emitted by codex-cli 0.147.0. `strings` over the | ||
| # vendored native binary finds zero occurrences of "Command Approval Required", | ||
| # "] Accept", or "] Decline" -- the live prompt is APPROVAL_PROMPT_PATTERN above. | ||
| # The two patterns are kept because this copy predates the numbered menu and is | ||
| # already load-bearing in STARTUP_BLOCKING_INPUT_PATTERN below, so dropping them | ||
| # would silently un-guard whichever older Codex builds still render it. Treat | ||
| # _has_approval_modal_in_bottom as legacy/defensive: APPROVAL_PROMPT_PATTERN is | ||
| # what fires on current Codex. | ||
| # | ||
| # Split into header and choice-key halves because the two paths that consume | ||
| # them need different strictness. The startup path (_has_startup_idle_composer) | ||
| # uses the permissive OR below as a NEGATIVE gate — any one token vetoes | ||
| # "ready", and a false veto merely keeps polling, so over-matching is free. | ||
| # get_status() uses them as a POSITIVE classifier where over-matching would | ||
| # strand a healthy pane in WAITING_USER_ANSWER, so it corroborates the two | ||
| # halves separately (see _has_approval_modal_in_bottom). Box-drawing characters | ||
| # are deliberately NOT required: the frame chrome has changed across Codex | ||
| # releases while this copy has not. | ||
| APPROVAL_MODAL_HEADER_PATTERN = r"Command Approval Required" | ||
| APPROVAL_MODAL_CHOICE_PATTERN = r"(?:\[[aA]\]\s+Accept\b|\[[dD]\]\s+Decline\b)" | ||
| # Box-drawing frame and padding stripped from a modal line before matching, so a | ||
| # framed line ("│ [a] Accept [d] Decline │") reduces to its text content. | ||
| # Stripped as a character SET from both ends, hence no ordering assumption about | ||
| # corner/edge glyphs. Light, heavy, and double variants are all covered because | ||
| # only light glyphs have been observed and the frame style is not contractual. | ||
| # | ||
| # ASCII frame characters (+ - |) are deliberately EXCLUDED. They are markdown | ||
| # table syntax, so including them would let a table the model wrote in its own | ||
| # reply ("| Command Approval Required |" / "| [a] Accept | [d] Decline |") | ||
| # reduce to the exact modal shape. No Codex release has been observed using | ||
| # ASCII frames, so that trade buys a hypothetical false negative at the cost of | ||
| # a plausible false positive. | ||
| # | ||
| # Note this set also strips leading whitespace, so an INDENTED plain-text quote | ||
| # reduces to the modal shape too. That look-alike is excluded positionally | ||
| # instead — see _has_approval_modal_in_bottom. | ||
| MODAL_FRAME_CHARS = "─│╭╮╰╯├┤━┃┏┓┗┛┣┫═║╔╗╚╝╠╣ \t" | ||
| # The same set minus padding, used to tell "this line began with box chrome" | ||
| # from "this line began with a prose indent". | ||
| MODAL_FRAME_GLYPHS = frozenset(MODAL_FRAME_CHARS) - frozenset(" \t") | ||
| STARTUP_BLOCKING_INPUT_PATTERN = ( | ||
| r"(?:Command Approval Required|\[[aA]\]\s+Accept\b|" | ||
| r"\[[dD]\]\s+Decline\b|Press enter to continue)" | ||
| rf"(?:{APPROVAL_MODAL_HEADER_PATTERN}|{APPROVAL_MODAL_CHOICE_PATTERN}|" | ||
| rf"{APPROVAL_PROMPT_PATTERN}|{APPROVAL_PROMPT_FOOTER}|{TRUST_PROMPT_FOOTER})" | ||
| ) | ||
| STARTUP_IDLE_PLACEHOLDER_PATTERN = ( | ||
| rf"^\s*{IDLE_PROMPT_PATTERN}[^\S\n]+(?:" | ||
|
|
@@ -278,6 +355,169 @@ def _has_update_dialog_in_bottom(clean_output: str) -> bool: | |
| ) | ||
|
|
||
|
|
||
| def _modal_line_content(line: str) -> Optional[str]: | ||
| """Reduce one line to its modal text, or None if the line reads as prose. | ||
|
|
||
| Strips frame glyphs and padding so ``"│ [a] Accept [d] Decline │"`` | ||
| reduces to ``"[a] Accept [d] Decline"``. Returns None when the leading run | ||
| removed was whitespace ONLY while being non-empty — i.e. the line is | ||
| indented plain text. | ||
|
|
||
| That indent test is the discriminator against the model quoting a modal | ||
| transcript back in its own reply: | ||
|
|
||
| • The terminal output showed: | ||
| Command Approval Required | ||
| [a] Accept [d] Decline | ||
| so it was waiting on approval. | ||
|
|
||
| Those quoted lines reproduce the modal's per-line structure exactly, so | ||
| line structure alone cannot separate them. Position can: Codex draws the | ||
| modal box flush at the left margin, whereas quoted or continuation prose is | ||
| indented under its bullet. So a leading run of frame glyphs is accepted, a | ||
| leading run of spaces/tabs is not, and column 0 is accepted either way | ||
| (an unframed modal would still start there). | ||
| """ | ||
| content = line.strip(MODAL_FRAME_CHARS) | ||
| if not content: | ||
| return None | ||
| lead = line[: len(line) - len(line.lstrip(MODAL_FRAME_CHARS))] | ||
| if lead and not (MODAL_FRAME_GLYPHS & set(lead)): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Keep framed assistant quotes out of modal detection This accepts a leading run whenever it contains any frame glyph, even when whitespace precedes that glyph. An assistant can quote the actual framed modal in an indented fenced block; the header and choice lines then both pass |
||
| return None | ||
| return content | ||
|
|
||
|
|
||
| def _is_frame_padding(line: str) -> bool: | ||
| """Return True when ``line`` carries nothing but frame glyphs and padding. | ||
|
|
||
| True of a box's top/bottom rule ("╰────╯"), of an empty interior row | ||
| ("│ │"), and of a blank line (space is in ``MODAL_FRAME_CHARS``). | ||
| """ | ||
| return not line.strip(MODAL_FRAME_CHARS) | ||
|
|
||
|
|
||
| def _is_chrome_only(line: str) -> bool: | ||
| """Return True when ``line`` is frame or TUI chrome rather than content. | ||
|
|
||
| The union of what may legitimately sit BELOW a live modal: the box's own | ||
| closing rule and interior padding, blank filler, the empty composer line | ||
| ("›" with nothing typed), and the status-bar footer. Anything else -- a | ||
| prose bullet, a spinner, a typed draft -- is content, which means the | ||
| modal is no longer the bottom of the pane. | ||
|
|
||
| The empty-composer and footer cases are matched explicitly rather than | ||
| folded into :func:`_is_frame_padding` because neither ``›`` nor the footer | ||
| text reduces to empty under ``MODAL_FRAME_CHARS``. | ||
| """ | ||
| if _is_frame_padding(line): | ||
| return True | ||
| if re.fullmatch(rf"\s*{IDLE_PROMPT_PATTERN}\s*", line): | ||
| return True | ||
| return re.search(TUI_FOOTER_PATTERN, line) is not None | ||
|
|
||
|
|
||
| def _is_transcript_marker(line: str) -> bool: | ||
| """Return True when ``line`` opens a new transcript cell (``›`` user / ``•`` bullet). | ||
|
|
||
| Used as the upward bound on the header search: Codex draws the modal as ONE | ||
| cell, so a user line or an assistant bullet is a hard boundary that the box | ||
| cannot span. This replaces a fixed line count, which could not express | ||
| "same box" and therefore failed open on a modal taller than the window. | ||
| """ | ||
| return bool( | ||
| re.match(USER_PREFIX_PATTERN, line, re.IGNORECASE) | ||
| or re.match(ASSISTANT_PREFIX_PATTERN, line, re.IGNORECASE) | ||
| ) | ||
|
|
||
|
|
||
| def _has_approval_modal_in_bottom(clean_output: str) -> bool: | ||
| """Return True when Codex's boxed command-approval modal is active at the bottom. | ||
|
|
||
| NOTE: this detects the LEGACY "Command Approval Required" / "[a] Accept" | ||
| modal, which codex-cli 0.147.0 does not render — see | ||
| APPROVAL_MODAL_HEADER_PATTERN's comment and | ||
| :func:`_has_approval_prompt_in_bottom` for the copy that is live today. | ||
|
|
||
| Anchored BOTTOM-UP on the last choice line, because the thing being tested | ||
| is an invariant about the bottom of the pane, not about a region of it: a | ||
| live modal blocks the TUI, so it must BE the bottom, with only frame rows | ||
| and footer chrome after it. Four guards: | ||
|
|
||
| 1. **Anchor.** The LAST line that reduces to a choice key. Taking the last | ||
| rather than the first is what lets an already-answered modal sitting in | ||
| scrollback above a live one be ignored instead of vetoing it. | ||
| 2. **Nothing but chrome below the anchor.** See :func:`_is_chrome_only`. | ||
| This subsumes the older spinner test (a spinner is not chrome) and also | ||
| rejects a modal transcript the model quoted mid-reply, since the reply | ||
| continues below the quote. It replaces "footer must NOT appear below", | ||
| which would have false-negatived every real modal: with | ||
| ``--no-alt-screen`` the footer renders at the bottom regardless. | ||
| 3. **Corroborating header above the anchor,** found by walking up and | ||
| stopping at the first :func:`_is_transcript_marker` — the box is one | ||
| transcript cell, so the header must be inside it. No fixed window, so an | ||
| arbitrarily tall modal still resolves; previously a >15-line modal lost | ||
| its header and failed open to COMPLETED. | ||
| 4. **Line structure and left-margin position.** Each half must own its line | ||
| (header an exact match, choice line a prefix match) and sit at the box's | ||
| margin rather than under a prose indent — see :func:`_modal_line_content`. | ||
|
|
||
| Known residual: a framed modal quote that ENDS a reply, with only the empty | ||
| composer and footer after it, satisfies all four guards and reads as live. | ||
| Distinguishing it needs semantics this detector does not have; it costs a | ||
| spurious WAITING_USER_ANSWER (work withheld) rather than a COMPLETED (work | ||
| pasted into a blocked pane), which is the safe direction to be wrong in. | ||
| """ | ||
| lines = clean_output.splitlines() | ||
|
|
||
| choice_idx = None | ||
| for index in range(len(lines) - 1, -1, -1): | ||
| content = _modal_line_content(lines[index]) | ||
| if content is not None and re.match(APPROVAL_MODAL_CHOICE_PATTERN, content, re.IGNORECASE): | ||
| choice_idx = index | ||
| break | ||
| if choice_idx is None: | ||
| return False | ||
|
|
||
| if not all(_is_chrome_only(line) for line in lines[choice_idx + 1 :]): | ||
| return False | ||
|
|
||
| for index in range(choice_idx - 1, -1, -1): | ||
| line = lines[index] | ||
| content = _modal_line_content(line) | ||
| if content is not None and re.fullmatch( | ||
| APPROVAL_MODAL_HEADER_PATTERN, content, re.IGNORECASE | ||
| ): | ||
| return True | ||
| if _is_transcript_marker(line): | ||
| return False | ||
| return False | ||
|
|
||
|
|
||
| def _has_approval_prompt_in_bottom(clean_output: str) -> bool: | ||
| """Return True when Codex's runtime approval prompt is active at the bottom. | ||
|
|
||
| This is the prompt codex-cli 0.147.0 actually renders (verified against a | ||
| live capture; see APPROVAL_PROMPT_PATTERN). Corroborates the question with | ||
| its footer inside the bottom region, exactly like the trust-v2, login, and | ||
| update dialogs — the prompt is a numbered menu of the same shape, so the | ||
| same guard against the copy surviving in scrollback applies. | ||
|
|
||
| BLANK LINES ARE DROPPED before the region is taken. The prompt is ~10 rows | ||
| of question, command preview, and options separated by blank filler, and | ||
| ``tmux capture-pane`` pads the pane to its full height with empty rows, so a | ||
| raw 15-line tail can land entirely inside the padding and see neither half. | ||
| Compacting first is what :meth:`CodexProvider.get_status_from_screen` | ||
| already does to the pyte viewport, so this makes the buffer path agree with | ||
| the screen path rather than inventing a second rule. | ||
| """ | ||
| rows = [line for line in clean_output.splitlines() if line.strip()] | ||
| bottom = "\n".join(rows[-STARTUP_PROMPT_BOTTOM_LINES:]) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Scan past long command previews The live approval is not bounded to 15 nonblank rows. Codex 0.147 appends every line of the command and then wraps the result (renderer); it does not truncate the preview to this detector's window. In CAO's 50-row pane, a 12-line heredoc leaves the complete question/menu/footer visible, but puts the question outside this slice: the helper returns |
||
| return ( | ||
| re.search(APPROVAL_PROMPT_PATTERN, bottom) is not None | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Require a live menu, not quoted prompt copy These two unanchored searches also match an ordinary completed assistant reply that quotes both UI strings—for example, a summary of this change containing |
||
| and re.search(APPROVAL_PROMPT_FOOTER, bottom) is not None | ||
| ) | ||
|
|
||
|
|
||
| def _has_startup_idle_composer(clean_output: str) -> bool: | ||
| """Return True when the bottom of the pane shows Codex's idle composer.""" | ||
| all_lines = clean_output.splitlines() | ||
|
|
@@ -893,6 +1133,44 @@ def get_status(self, output: str) -> TerminalStatus: | |
| ): | ||
| return TerminalStatus.WAITING_USER_ANSWER | ||
|
|
||
| # Boxed command-approval modal ("Command Approval Required" / "[a] Accept" | ||
| # / "[d] Decline"). Reuses the copy that STARTUP_BLOCKING_INPUT_PATTERN | ||
| # already vetoes readiness on at startup — the same modal can appear at | ||
| # RUNTIME under any approval-prompting codexProfile, and only the startup | ||
| # path used to notice it. | ||
| # | ||
| # Bottom-anchored like trust-v2 and the update dialog, and placed BEFORE | ||
| # the idle/COMPLETED classification for the same reason: the TUI composer | ||
| # and status bar keep rendering while the modal is up, so the idle-prompt | ||
| # check below would otherwise report COMPLETED (or PROCESSING when the | ||
| # composer has scrolled off) for a pane that is hard-blocked on a | ||
| # keystroke. A COMPLETED there is the dangerous case — it tells the | ||
| # conductor the agent is free and invites more work into a dead pane. | ||
| # | ||
| # NOT gated on `not assistant_after_last_user` (unlike WAITING_PROMPT_PATTERN | ||
| # below): the modal is raised mid-turn, after the model has already emitted | ||
| # bullets, so that gate would suppress every real occurrence. Prose that | ||
| # merely quotes the copy is excluded structurally instead — see | ||
| # _has_approval_modal_in_bottom. | ||
| if _has_approval_modal_in_bottom(clean_output): | ||
| return TerminalStatus.WAITING_USER_ANSWER | ||
|
|
||
| # Runtime approval prompt as codex-cli 0.147.0 actually renders it -- a | ||
| # numbered menu, not the boxed modal above. This is the check that fires on | ||
| # a current-Codex approval; without it a live prompt classified as IDLE | ||
| # (verified against the live capture in | ||
| # test/providers/fixtures/codex_approval_modal_raw.txt), because the | ||
| # prompt's own "› 1. Yes, proceed (y)" cursor line is both the last | ||
| # USER_PREFIX_PATTERN match and an idle-prompt match, so the classification | ||
| # below saw a user message with no reply after it. IDLE is as dangerous as | ||
| # COMPLETED here: both tell the conductor the pane is free. | ||
| # | ||
| # Placed after the legacy modal check and before the idle classification, | ||
| # for the same reason: the composer and status bar keep rendering while the | ||
| # prompt is up, so the idle-prompt check cannot see the block. | ||
| if _has_approval_prompt_in_bottom(clean_output): | ||
| return TerminalStatus.WAITING_USER_ANSWER | ||
|
|
||
| # Check bottom of captured output for idle prompt. | ||
| # With --no-alt-screen, scrollback contains history so we can't anchor | ||
| # to end-of-string. Instead, check only the last few lines. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| ╭──────────────────────────────────────────────────────╮ | ||
| │ >_ OpenAI Codex (v0.147.0) │ | ||
| │ │ | ||
| │ model: openai.gpt-5.6-sol low /model to change │ | ||
| │ directory: /private/tmp/codex-work-567 │ | ||
| ╰──────────────────────────────────────────────────────╯ | ||
|
|
||
| Tip: New Build faster with the Desktop app. Run 'codex app' or visit https://chatgpt.com/codex?app-landing-page=true | ||
|
|
||
|
|
||
| › Run this shell command now, do not explain first: mkdir -p /private/tmp/codex-work-567/subdir | ||
|
|
||
|
|
||
| ✗ You canceled the request to run mkdir -p /private/tmp/codex-work-567/subdir | ||
|
|
||
| • Ran mkdir -p /private/tmp/codex-work-567/subdir | ||
| └ (no output) | ||
|
|
||
| ■ Conversation interrupted - tell the model what to do differently. Something went wrong? Hit `/feedback` to report the | ||
| issue. | ||
|
|
||
|
|
||
| › Now edit note.txt and change hello to goodbye. Do it now, no explanation. | ||
|
|
||
|
|
||
| • Edited note.txt (+1 -1) | ||
| 1 -hello | ||
| 1 +goodbye | ||
|
|
||
|
|
||
| Would you like to make the following edits? | ||
|
|
||
|
|
||
| › 1. Yes, proceed (y) | ||
| 2. Yes, and don't ask again for these files (a) | ||
| 3. No, and tell Codex what to do differently (esc) | ||
|
|
||
| Press enter to confirm or esc to cancel | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| › run the deploy script | ||
| • I'll run the deploy script now. | ||
|
|
||
| • Called shell.exec({"command":"./scripts/deploy.sh"}) | ||
|
|
||
| ╭─ Command Approval Required ──────────────────╮ | ||
| │ │ | ||
| │ ./scripts/deploy.sh │ | ||
| │ │ | ||
| │ [a] Accept [d] Decline │ | ||
| ╰──────────────────────────────────────────────╯ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| › run the deploy script | ||
| • I'll run the deploy script now. | ||
|
|
||
| ┏━ Command Approval Required ━━━━━━━━━━━━━━━━━━┓ | ||
| ┃ ┃ | ||
| ┃ ./scripts/deploy.sh ┃ | ||
| ┃ ┃ | ||
| ┃ [a] Accept [d] Decline ┃ | ||
| ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| › why did the earlier run stall? | ||
| • The terminal output showed: | ||
|
|
||
| Command Approval Required | ||
| [a] Accept [d] Decline | ||
|
|
||
| so the pane was blocked waiting on approval. I have since set | ||
| approval_policy = "never" in the profile, so it will not recur. | ||
|
|
||
| › | ||
| openai.gpt-5.6-sol high · ~/wt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Cover the network exec-approval title
This alternation omits another blocking exec title emitted by the exact 0.147.0 renderer targeted here. When
network_approval_contextis present, https://github.com/openai/codex/blob/be6e8eac029b183056b7e4402879f15d2c85f61b/codex-rs/tui/src/bottom_pane/approval_overlay.rs#L252-L268 rendersDo you want to approve network access to "example.com"?with the same numbered choices and confirmation footer. CAO profiles can enable that experimental path throughcodexConfig(features.network_proxy=true). Replaying the upstream 0.147 snapshot makes_has_approval_prompt_in_bottom()returnFalseandget_status_from_screen()returnIDLE, recreating the queued-input loss this PR fixes. Please include this exec title and a regression case, or detect the approval menu structurally instead of enumerating only these three titles.