Skip to content

fix(codex): detect runtime command-approval modals in get_status - #567

Open
tedswinyar wants to merge 3 commits into
awslabs:mainfrom
tedswinyar:agent/caom-lq8-fix
Open

fix(codex): detect runtime command-approval modals in get_status#567
tedswinyar wants to merge 3 commits into
awslabs:mainfrom
tedswinyar:agent/caom-lq8-fix

Conversation

@tedswinyar

Copy link
Copy Markdown
Contributor

Problem

The STARTUP_BLOCKING_INPUT_PATTERN constant — Codex's boxed "Command Approval Required / [a] Accept / [d] Decline" modal — was consulted only on the startup path (one reference, inside _has_startup_idle_composer()). get_status() never reused it, and its own waiting checks (WAITING_PROMPT_PATTERN, trust-v1/v2, update-dialog) do not match that copy.

So a pane blocked on the modal mid-session classified as COMPLETED (when the TUI composer was still rendered above it) or PROCESSING (once it scrolled off). COMPLETED is the harmful one: it tells the conductor the agent is free, and the inbox service will type queued work into a pane hard-blocked on a keystroke — the same hazard class the update-dialog and bare-shell ERROR checks already exist to prevent.

This surfaces whenever a codex worker runs under a codexProfile with an approval policy (rather than the --yolo default).

Fix

Add _has_approval_modal_in_bottom(), called from get_status() before the idle/COMPLETED classification. STARTUP_BLOCKING_INPUT_PATTERN is split into APPROVAL_MODAL_HEADER_PATTERN + APPROVAL_MODAL_CHOICE_PATTERN and recomposed for the startup path (behaviourally identical — TRUST_PROMPT_FOOTER is exactly the old Press enter to continue alternative).

Five guards separate a live modal from look-alikes:

  1. Region — only the bottom STARTUP_PROMPT_BOTTOM_LINES are searched, so an answered modal scrolls out on its own.
  2. Corroboration — header AND a choice key must both be present, header above keys.
  3. Line structure — after reducing frame chrome, the header line is exactly the header and the choice line starts with a choice key, so prose naming the phrases mid-sentence fails.
  4. Left-margin position — a leading run of frame glyphs is chrome (accepted); a leading run of spaces/tabs is a prose indent (rejected). This is what separates a live modal from the model quoting a modal transcript in its own indented reply.
  5. No active spinner below the choice line — a live modal blocks execution, so nothing can be spinning below it; an answered modal whose work resumed but hasn't scrolled the box out yet renders • Working (Ns • esc to interrupt) there. Scoped to lines strictly below the choice line, because with --no-alt-screen a stale spinner can survive in scrollback above a genuinely live modal.

ASCII + - | frame characters are deliberately excluded from the strip set: they're markdown-table syntax, so stripping them would let a table the model wrote in its own reply reduce to the modal shape at column 0, defeating the position guard.

Tests

274 lines added in test_codex_provider_unit.py + 4 fixtures, including negative cases for:

  • indented quoted transcript in an assistant reply (must NOT be WAITING)
  • markdown-table quote of the modal copy
  • answered modal in scrollback
  • answered modal with work resumed below (not WAITING) and its controls: live modal with no spinner (IS WAITING), and a stale spinner above a live modal (still WAITING)
  • heavy- and double-box frame styles (detected)

test_codex_provider_unit.py: 184 passed, 3 skipped.

Review provenance

Adversarial panel of three independent reviewers (Claude ×2 + GPT-5.6 Sol, two harnesses). Round 1 caught a false-positive that single-review missed (indented quoted transcript); re-verification of that fix found a second narrow edge (answered-modal relatch while a spinner renders below); round 2 closed it. Every finding was reproduced against real code before acting.

The boxed "Command Approval Required / [a] Accept / [d] Decline" modal was
only consulted on the STARTUP path: STARTUP_BLOCKING_INPUT_PATTERN had exactly
one reference, inside _has_startup_idle_composer(). get_status() never reused
it, and its own waiting checks (WAITING_PROMPT_PATTERN, trust-v1/v2,
update-dialog) do not match that copy.

So a pane blocked on the modal MID-SESSION classified as COMPLETED when the TUI
composer was still rendered above it, or PROCESSING once the composer scrolled
off. COMPLETED is the harmful one: it tells the conductor the agent is free, and
the inbox service will type queued work into a pane hard-blocked on a keystroke
— the same hazard class the update-dialog and bare-shell ERROR checks exist to
prevent.

Split the startup constant into APPROVAL_MODAL_HEADER_PATTERN and
APPROVAL_MODAL_CHOICE_PATTERN and add _has_approval_modal_in_bottom(), called
from get_status() before the idle/COMPLETED classification. The startup constant
is recomposed from the halves and is behaviourally unchanged — it is a permissive
negative gate there, where over-matching only costs another poll.

get_status() needs the opposite strictness (over-matching would strand a healthy
pane in WAITING_USER_ANSWER), so three guards separate the live modal from
look-alikes: the bottom-region anchor used by trust-v2 and the update dialog;
header AND choice-key corroboration in header-above-keys order; and a
line-structure requirement that each half own its line once frame glyphs are
stripped. The check is deliberately NOT gated on assistant_after_last_user —
a real modal is raised mid-turn after the model has emitted bullets, so that
gate would suppress every true positive; quoted prose is excluded structurally
instead.

Frame box-drawing characters are not required for detection: the chrome has
changed across Codex releases while the copy has not.
…styles

Follow-up to the runtime approval-modal check, addressing two defects found in
adversarial review. Both were reproduced against the real helper before
changing anything.

FALSE POSITIVE: an indented modal transcript quoted in the model's OWN reply
was classified WAITING_USER_ANSWER on a pane that was actually COMPLETED,
stalling a ready worker. Root cause: whitespace is in MODAL_FRAME_CHARS, so
line.strip() erased a prose indent exactly as it erases box chrome. The quote
reproduces the modal's per-line structure, so the region, corroboration, and
line-structure guards all pass on it.

Add a fourth guard on left-margin position, via _modal_line_content(): a
leading run of frame glyphs is chrome and is accepted (so an indented box still
matches), a leading run of spaces/tabs is a prose indent and is rejected, and
column 0 is accepted either way. Codex draws the box flush at the margin while
quoted and continuation prose sits indented under its bullet, so the model
would have to emit the header un-indented and outside any bullet to fool this.

Considered and rejected the alternative discriminator "require the composer to
be absent below the box": get_status's own TUI-progress comment records that
with --no-alt-screen the footer renders at the bottom even while processing, so
that test is likely true of real modals and would miss all of them.

FALSE NEGATIVE: MODAL_FRAME_CHARS covered only light box-drawing glyphs, so a
heavy- or double-framed box was not stripped and the header fullmatch failed.
Widen to cover all three styles. This is defensive, not observed — no evidence
any Codex release uses anything but light glyphs — but missing a real modal is
the costlier direction.

ASCII "+ - |" are deliberately still excluded: they are markdown table syntax,
so stripping them would let a table the model wrote in its own reply reduce to
the modal shape AT column 0, defeating every guard including the new one. That
trades a hypothetical ASCII-framed modal for a plausible false positive.

Widening the strip set does not weaken the new guard, which keys on
glyph-vs-whitespace in the leading run rather than on the set's contents.
Third defect from adversarial review, reproduced before changing anything. A
modal that was ANSWERED and whose work has resumed, but which has not yet
scrolled out of the bottom region, satisfies all four existing guards. Status
came back WAITING_USER_ANSWER while codex was actively running, so the conductor
withheld work from a busy pane — the same "worker looks stuck" symptom as the
original bug, in the opposite direction.

Add a fifth guard: no active progress spinner below the choice line. A live modal
blocks execution, so nothing can be spinning beneath it; a resumed one renders
"• Working (Ns • esc to interrupt)" there. Guard 1 already clears this case once
enough output scrolls the box out, so this closes a transient window rather than
a permanent misread.

Scoped to lines strictly BELOW the choice line rather than across the whole
region, which is the part that matters. With --no-alt-screen a spinner from
earlier in the same turn can survive in scrollback ABOVE the box, so a
region-wide test would match that stale line and suppress a genuinely blocked
pane — reintroducing the false negative this bead exists to fix. Both directions
are now covered by tests.

The guard lives inside _has_approval_modal_in_bottom rather than at the call site
because correct scoping needs the choice line's index, which only the helper
knows; a call-site guard can see only the flat tail, i.e. exactly the
region-wide variant that breaks. The helper remains pure and testable.
@codecov-commenter

codecov-commenter commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@2a6f20c). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #567   +/-   ##
=======================================
  Coverage        ?   90.99%           
=======================================
  Files           ?      179           
  Lines           ?    23300           
  Branches        ?        0           
=======================================
  Hits            ?    21201           
  Misses          ?     2099           
  Partials        ?        0           
Flag Coverage Δ
unittests 90.99% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Codex terminal status detection by recognizing the “Command Approval Required / [a] Accept / [d] Decline” modal when it appears mid-session, preventing get_status() from misclassifying a blocked pane as COMPLETED (or PROCESSING) and allowing the orchestrator to keep sending work into a hard-blocked TUI.

Changes:

  • Split the startup modal regex into header/choice components and add _has_approval_modal_in_bottom() for stricter runtime detection.
  • Invoke approval-modal detection in CodexProvider.get_status() before idle/COMPLETED classification, returning WAITING_USER_ANSWER when active.
  • Add extensive unit tests and fixtures covering true positives and multiple false-positive lookalikes (prose quotes, indented transcript, markdown-table transcription, scrollback, spinner-below “answered” case).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/cli_agent_orchestrator/providers/codex.py Adds bottom-anchored runtime approval-modal detection and wires it into get_status() ahead of COMPLETED classification.
test/providers/test_codex_provider_unit.py Adds a dedicated test suite validating correct WAITING/PROCESSING/COMPLETED outcomes across modal and lookalike scenarios.
test/providers/fixtures/codex_approval_modal.txt Fixture for an active runtime approval modal.
test/providers/fixtures/codex_approval_modal_scrollback.txt Fixture ensuring an already-answered modal in scrollback doesn’t latch.
test/providers/fixtures/codex_approval_modal_quoted_in_reply.txt Fixture ensuring an indented quoted transcript in an assistant reply doesn’t latch.
test/providers/fixtures/codex_approval_modal_heavy_box.txt Fixture ensuring heavy box-drawing frames are still detected as modal chrome.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@haofeif haofeif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two reproducible P2 edge cases remain in the new approval-modal detector. One can hide a newer live modal behind stale buffered output; the other can latch a completed assistant reply as WAITING_USER_ANSWER.

content = _modal_line_content(line)
if content is not None and re.match(APPROVAL_MODAL_CHOICE_PATTERN, content, re.IGNORECASE):
choice_idx = header_idx + 1 + offset
return not _has_active_spinner(bottom_lines[choice_idx + 1 :])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Continue past stale modal candidates

get_status() parses an accumulated raw pipe-pane buffer, so two approval boxes can coexist in these 15 lines. For old modal -> Working spinner -> new live modal, this selects the first header/choice and immediately returns False because the spinner is below that old choice; it never examines the newer live box. With no more output arriving, the blocked pane remains PROCESSING indefinitely. Please evaluate the newest complete header/choice pair (or continue scanning after a spinner-vetoed pair) and cover the two-modal sequence.

if not content:
return None
lead = line[: len(line) - len(line.lstrip(MODAL_FRAME_CHARS))]
if lead and not (MODAL_FRAME_GLYPHS & set(lead)):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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 _modal_line_content(), and an otherwise completed pane latches as WAITING_USER_ANSWER. Please distinguish assistant indentation from a real viewport gutter and add a framed quoted-transcript regression case.

@call-me-ram call-me-ram left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying bug is real and worth fixing. I confirmed it independently: STARTUP_BLOCKING_INPUT_PATTERN had exactly one consumer (_has_startup_idle_composer), and none of get_status's own waiting checks — WAITING_PROMPT_PATTERN, trust-v1/v2, update-dialog — match that copy. So a mid-session block on the modal fell through to the idle/COMPLETED branch. COMPLETED really is the dangerous outcome: deliver_pending gates on status in (IDLE, COMPLETED), so the inbox pastes into a pane that cannot consume it. Classifying it WAITING_USER_ANSWER is also the right target status, not just a safe one — it's the gate answer_user_prompt requires, so it converts a silent wedge into something the conductor can actually resolve with a/d. Good find, and the reasoning captured in the commit messages (why not assistant_after_last_user, why not "composer absent below the box", why ASCII +-| stays out of the strip set) is the kind of thing that saves the next reader an hour.

The recomposition is genuinely behaviour-neutral — I checked that empirically rather than by eye (below), so no concerns there.

What I'm blocking on is the detector's scan strategy, not its guards. _has_approval_modal_in_bottom commits to the first header it finds and the first choice line below it, inside a fixed 15-line window. That single choice produces three separate reproducible defects — one of which I believe is the more likely production trigger than any of the three the adversarial panel found, and it fails in the exact direction the bead exists to prevent. All three collapse into one fix, and I've prototyped it against every assertion this PR makes.

I independently confirm both of haofeif's items (must-fix 2 and 3 below) — both reproduce on head. I'd raise the severity of the second one; see the note there.

Must-fix

1. A live modal taller than 15 lines fails open to COMPLETED — the original hazard, unmitigated

src/cli_agent_orchestrator/providers/codex.py:372bottom_lines = clean_output.splitlines()[-STARTUP_PROMPT_BOTTOM_LINES:], then :374-383 searches the header only inside that slice.

Guard 1's docstring names the assumption ("This assumes the modal is at most that tall") but nothing enforces or degrades gracefully when it doesn't hold. When the box body exceeds ~13 lines, the header is outside the window while the choice line is inside it, guard 2 fails, and the helper returns False. With the TUI footer rendered below — which --no-alt-screen always does, per get_status's own comment — the pane classifies COMPLETED.

Concrete scenario: a apply_patch / multi-file-write approval, where Codex renders the patch body inside the box, or any long command that wraps. Reproduced:

# 14-line box body, header at index 2 of 19 lines
_has_approval_modal_in_bottom(...)                    -> False
get_status(...)                                       -> PROCESSING
get_status(... + "› \n  ? for shortcuts   88% context left\n") -> COMPLETED

COMPLETED on a pane hard-blocked on a keystroke is precisely the failure this PR is fixing, and a patch approval is not an exotic case — it's the single most common thing an approval policy gates.

Note this is not fixable by bumping STARTUP_PROMPT_BOTTOM_LINES: guard 1 is doing double duty (bounding modal height and expiring answered modals via the header scrolling out), and those two jobs want the window sized in opposite directions. Trust-v2 and the update dialog get away with the same 15-line window because those dialogs are fixed-height and short; this one isn't.

2. Answered modal above a live modal: the earliest pair wins and vetoes the real one (confirms haofeif's [P2])

codex.py:375-383 breaks on the first header; :386-391 returns on the first choice line below it. There is no continuation. So for answered modal -> spinner -> live modal, guard 5 vetoes on the stale pair and the newer live box is never examined. Reproduced on head:

╭─ Command Approval Required ─╮   │ ./step1.sh │   │ [a] Accept  [d] Decline │  ╰──╯
• Working (3s • esc to interrupt)
• step1 done, now step2
╭─ Command Approval Required ─╮   │ ./step2.sh │   │ [a] Accept  [d] Decline │  ╰──╯

_has_approval_modal_in_bottom -> False;  get_status -> PROCESSING

Codex does not opt into supports_screen_detection, so this runs over the accumulated raw pipe-pane buffer (not a rendered viewport) — haofeif's premise about two boxes coexisting in the region is correct for the path that actually executes. The pane then sits at PROCESSING with no further output to trigger re-detection, i.e. stuck until timeout.

3. Indented framed quote latches WAITING_USER_ANSWER on a finished pane (confirms haofeif's [P2] — and I'd raise the severity)

codex.py:316-318 accepts the leading run whenever it contains any frame glyph, so whitespace-then-glyph passes. test_has_approval_modal_accepts_framed_box_with_whitespace_gutter deliberately locks that in, which is why this needs a decision rather than a one-line patch — guard 4 currently only catches the unframed indented quote (the codex_approval_modal_quoted_in_reply.txt shape). Reproduced:

› why did the last run stall?
• The pane was blocked. It showed:

    ╭─ Command Approval Required ─╮
    │ ./scripts/deploy.sh         │
    │ [a] Accept  [d] Decline     │
    ╰──────────────────────────────╯

  I have set approval_policy = "never" so it will not recur.
›
  ? for shortcuts                     90% context left

_has_approval_modal_in_bottom -> True;  get_status -> WAITING_USER_ANSWER

Severity is worse than "an otherwise completed pane latches", because of two things downstream:

  • WAITING_USER_ANSWER is in status_monitor._STICKY_READY_STATUSES, so _apply_detection refuses the WAITING -> PROCESSING regression while unarmed.
  • deliver_pending refuses anything outside (IDLE, COMPLETED), and codex leaves accepts_input_while_processing at the base False, so eager delivery can't rescue it either.

A finished pane emits no further output, so there is no new chunk to re-detect against. The result is a permanent wedge: the worker is ready, and the conductor will never send it anything. Quoting terminal captures back is a routine thing for a CAO worker to do, so this is not a stretch shape.

Suggested fix for all three — anchor bottom-up on the choice line

Same root cause, one change. Anchor on the last choice line rather than the first header, drop the window bound on the header search, and take over guard 1's expiry job with a stronger, cheaper invariant: a live modal is the bottom of the pane, so nothing but frame/footer chrome may follow it. Guard 5's spinner test then becomes a special case of that.

def _has_approval_modal_in_bottom(clean_output: str) -> bool:
    lines = clean_output.splitlines()
    # Anchor on the LAST choice line: a second live modal below an answered one
    # must win, and the header search must not be bounded by modal height.
    for choice_idx in range(len(lines) - 1, -1, -1):
        content = _modal_line_content(lines[choice_idx])
        if content is not None and re.match(APPROVAL_MODAL_CHOICE_PATTERN, content, re.IGNORECASE):
            break
    else:
        return False
    # A live modal blocks execution, so only frame rows and TUI footer chrome
    # can follow it. Subsumes the old spinner guard and the bottom-region anchor.
    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):
        content = _modal_line_content(lines[index])
        if content is not None and re.fullmatch(
            APPROVAL_MODAL_HEADER_PATTERN, content, re.IGNORECASE
        ):
            return True
    return False

with _is_chrome_only(line) = blank, or reduces to nothing under MODAL_FRAME_CHARS (a frame row), or an empty composer (^\s*IDLE_PROMPT_PATTERN\s*$), or matches TUI_FOOTER_PATTERN.

I ran this against all 19 behaviours this PR asserts plus the three defects above. It preserves all 19 — including the four fixtures, answered modal in scrollback (rejected: bullets below the box are not chrome), answered modal with work resumed (rejected by the same rule, so the spinner guard's intent survives), stale spinner above a live modal (still accepted), the markdown-table quote, and both mixed framed/indented cases — and fixes defects 1, 2 and 3. Score: head 5/24 wrong, candidate 1/24.

The one residual is worth calling out honestly: an indented framed quote that ends the reply with nothing but the composer after it still reads as True. That's the narrow case where haofeif's "distinguish assistant indentation from a real viewport gutter" is doing work the chrome rule can't. If you take the candidate above, that residual is what the extra discriminator (or an explicit accepted-risk note plus a regression test pinning current behaviour) needs to cover.

Non-blocking

  1. Rebase. mergeStateStatus: DIRTY. Main moved 124 commits since the 2a6f20c base and now inserts a LOGIN_MENU_PATTERN check at exactly the insertion point used at codex.py:840. git merge-tree shows one conflict hunk and it's trivially resolvable — just needs doing, and the CI signal isn't trustworthy until it is.

  2. re.fullmatch on the header is brittle in the one dimension the PR argues matters. The design rationale is "the chrome has changed across releases while the copy has not" — but fullmatch on the reduced header line means any decoration inside the title kills detection outright:

    '╭─ Command Approval Required (1/2) ─╮'      -> not detected
    '╭─ Command Approval Required — shell.exec ─╮' -> not detected
    

    A per-batch counter or the tool name in the title is a very ordinary TUI thing to add. re.match on the reduced content instead of fullmatch would tolerate both without re-admitting the prose cases (the bullet isn't in the strip set, so bulleted prose still fails at the start-of-line test) — worth checking against the suite if you agree.

  3. The per-line guards are unverified on the stream get_status actually receives. Codex doesn't set supports_screen_detection, so this runs on the raw pipe-pane buffer through strip_terminal_escapes, which only newline-normalises CUP to column 1 (\d+;1H). utils/text.py:11-15 documents that Codex lays out its bottom prompt via CUP rather than CHA. If the box rows are positioned with CUP to a column > 1, the sequence is stripped outright and every row glues into one logical line — at which point all five guards fail:

    rows joined by \x1b[<row>;1H  -> 4 logical lines -> WAITING_USER_ANSWER   ✓
    rows joined by \x1b[<row>;3H  -> 1 logical line  -> COMPLETED             ✗
    rows joined by \x1b[1A\x1b[1G -> 4 logical lines -> WAITING_USER_ANSWER   ✓
    

    Medium confidence — I have no capture of how Codex actually emits this box, so I can't say which branch it lands in. Worth noting that trust-v2 and the update dialog re.search over the joined region precisely because they don't depend on line structure surviving the raw stream, and this is the first codex detector that does.

  4. Selection cursor on the choice line. re.match requires a choice key at the start of the reduced content, so a cursor glyph outside MODAL_FRAME_CHARS breaks it — ❯ [a] Accept [d] Decline, ▸ [a] Accept …, > [a] Accept … all return False. Mechanically verified; whether Codex renders one is speculation on my part. Cheap to absorb if the header check is loosened per (2) — allow an optional leading non-alphanumeric marker.

Asks

  1. One real capture. All four fixtures are hand-authored, and the whole detector is validated against text written by the same reasoning that designed it — which is how defects 1 and (possibly) 3 in the non-blocking list survived three review rounds. The repo already has real raw captures for other providers' permission surfaces (opencode_cli_permission.ansi.txt, kimi_code_tui_idle_raw.txt, cursor_cli_v2026_idle_output.txt), so the precedent and the harness both exist. A single codex_approval_modal_raw.txt from a live codexProfile with an approval policy would settle non-blocking (2), (3) and (4) at once, and would confirm the box height for must-fix 1.

  2. Has the copy itself ever been confirmed against a live binary? git log -S "Command Approval Required" traces the string to a824cee (the codexProfile PR), where it entered as a startup negative gate — the position where, as your own comment notes, over-matching is free and a wrong string costs nothing observable. Promoting it to a positive classifier changes that: if the copy is wrong or has since changed, this bead is a no-op that reads as fixed. Same question for the [a] Accept / [d] Decline key letters.

  3. blocks_orchestrated_input_while_waiting_user_answer for codex. The PR frames the hazard as "queued work typed into a pane hard-blocked on a keystroke", and deliver_pending closes the inbox half. But terminal_service.send_input:465-476 only pauses assign/handoff when the provider opts into that property, and CodexProvider overrides no properties at all — so it inherits False from base.py:191 and orchestrated task delivery still pastes into the WAITING pane. hermes.py:136 and antigravity_cli.py:189 both opt in for exactly this surface. That's a one-line property plus a test, and it's what makes the error message at terminal_service.py:473 ("Use answer_user_prompt …") actually reachable for codex. Happy to see it as a follow-up if you'd rather keep this diff focused — just don't want it lost, since without it the stated hazard is only half closed.

What I verified

Head d2736a3, base 2a6f20c, compared against upstream/main e592b21. Isolated worktree, nothing run in the main tree.

  • Tests, PR head: pytest test/providers/test_codex_provider_unit.py test/services/test_status_monitor.py -q -p no:randomly --no-cov214 passed, 3 skipped in 141s. The new TestCodexProviderApprovalModal class is green, and no pre-existing codex or status-monitor test regressed. (test_initialize_with_trust_prompt_v2 is just slow — 22s of real sleeps, also slow on upstream/main; not this PR.)
  • Lint: black --check and isort --check-only clean on both changed files.
  • Startup-path neutrality — empirical, not eyeballed. Compiled the pre-PR literal and the recomposed STARTUP_BLOCKING_INPUT_PATTERN side by side under re.IGNORECASE. Identical match spans on all 70 files in test/providers/fixtures/; 0 boolean divergences over 400,000 fuzzed strings drawn from an alphabet seeded with the pattern's own tokens; 0 divergences on 11 hand-picked edge cases ([a]Accept, [A] Accept, [d]\tDecline, Press enter to continue, case variants). The two regexes differ only by a redundant non-capturing group. TRUST_PROMPT_FOOTER is r"Press enter to continue", exactly the old fourth alternative. Claim holds.
  • Must-fix 1 — 19-line output, 14-line box body: _has_approval_modal_in_bottomFalse; get_statusPROCESSING, and COMPLETED once the TUI footer is appended.
  • Must-fix 2answered modal / spinner / live modal in 10 lines: FalsePROCESSING.
  • Must-fix 3 — indented framed quote followed by prose + composer: TrueWAITING_USER_ANSWER.
  • Suggested fix — prototyped and run against a 24-case table: the 19 behaviours this PR asserts (all four fixtures plus every inline case in TestCodexProviderApprovalModal), plus the three defects. Head wrong on 5/24; candidate wrong on 1/24 (the residual framed-quote-ending-the-reply case, called out above).
  • Raw-stream behaviour — built the box with three different escape layouts, ran through strip_terminal_escapes then get_status; results in non-blocking (3). Confirmed no codex fixture in the repo contains any escape sequence, while ten other providers' fixtures do.
  • Downstream status semantics — read inbox_service.deliver_pending (gates on IDLE/COMPLETED; eager path needs accepts_input_while_processing, which codex doesn't set), status_monitor._STICKY_READY_STATUSES / _apply_detection (WAITING is sticky against PROCESSING while unarmed), terminal_service.send_input:465-476, and mcp_server/server.py:_send_user_prompt_answer (requires WAITING_USER_ANSWER, accepts single-character answers).
  • Event loop — the new helpers are pure string/regex; no tmux, subprocess, filesystem or sleep reached from them, so no asyncio.to_thread obligation. Confirmed by inspection of codex.py:290-391.
  • Merge stategit merge-tree against upstream/main: one conflict hunk at the get_status insertion point (main added the login-menu check there), plus clean adds for the four fixtures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants