Skip to content

fix(status): probe a bursting screen for PROCESSING so a spinner that never goes quiet is not read as idle - #766

Merged
haofeif merged 1 commit into
awslabs:mainfrom
mastash3ff:fix/status-midburst-processing-probe
Sep 11, 2026
Merged

haofeif merged 1 commit into
awslabs:mainfrom
mastash3ff:fix/status-midburst-processing-probe

Conversation

@mastash3ff

@mastash3ff mastash3ff commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #757. Related: #659, #735, #714.

Summary

Rendered-screen detection runs only on the rising edge and at quiescence. Codex repaints its spinner every second while a command runs, so the terminal never goes quiescent, the detector is never invoked mid-turn, and the terminal reads idle for its whole busy turn (the rising-edge frame is composited before the spinner is drawn). The provider detector is right on every one of those frames; it just never gets asked. Consequences: inbox delivery pastes into a busy turn, the deferred-assign confirm reads "idle, never working", and external coordinators wait out the handoff timeout.

Change

  • StatusMonitor._midburst_processing_probe: while a terminal is bursting and its last status is not PROCESSING, probe the composited pyte screen at most every PYTE_MIDBURST_PROBE_S (1.0 s, env CAO_PYTE_MIDBURST_PROBE_S) and apply only a PROCESSING verdict. Ready statuses still wait for quiescence, so the anti-flap rule that ruled out per-chunk detection is preserved: a half-drawn frame can upgrade to busy, never settle to ready. The probe stops once PROCESSING is latched, and _apply_detection's sticky-latch/arm rules are untouched.
  • The probe asks a new side-effect-free predicate, BaseProvider.probe_processing_from_screen(), and only of providers that opt in with supports_midburst_processing_probe — the same fail-closed shape as supports_screen_detection and supports_direct_status_probe. It never calls the full detector: discarding a verdict does not undo the turn bookkeeping a detector commits while reaching it. minimax_code opts into screen detection and commits _last_completion_identity / _last_completion_buffer_epoch and clears _awaiting_turn inside get_status, so a mid-redraw frame would register as a new completion and the next settled frame would report a pending turn complete on the previous turn's output.
  • CodexProvider opts in and requires positive evidence of work: the progress row must be drawn, and only then does the full detector get the final say, keeping the trust-prompt, login-menu, approval-dialog and error guards. Inheriting the detector's no-composer catch-all would be wrong here, because a partial redraw that erases the composer while the previous response is still on screen carries no evidence of a new turn; taken as busy it spends the monitor's dispatch arm, the restored old completion latches, and the genuine spinner that follows is refused.
  • Tests (TestMidBurstProcessingProbe): the Codex spinner transition through the real provider; the minimax bookkeeping unchanged across a burst with the settled frame still IDLE and _awaiting_turn set; a provider without the opt-in never probed and no detector run; the throttle; no probe once PROCESSING. The probe tests feed a real pyte screen rather than stubbing detection.
  • Docs: docs/event-driven-architecture.md StatusMonitor section, docs/configuration.md env note.

Live verification

Same sleep 30 task on codex-cli 0.153.4, polled every 4 s: before, idle ×12 then completed; after, processing ×9 then completed.

Tests

test/services/ and test/providers/: 4817 passed, 9 skipped, 1 xfailed, against 4811 passed on main at the same commit with the identical 43 pre-existing AG-UI failures in this environment. black and isort clean on touched files. Rebased onto main (405750f).

@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.

Reviewed the standalone PR at 8a81b77d4ea472c382c04d6b9f5c2545b7eeb8ea (reported base c581b3be5555f925adcc3b0d7023182405695c88). Changes requested for one P2: discarding non-PROCESSING verdicts does not discard state changes performed by the detector. The intended Codex continuous-spinner transition, throttling boundaries, and timer cleanup work independently of the separate glyph PR; the blocker concerns another provider already opted into screen detection.

Current-head GitHub workflow runs require approval and do not establish a passing CI result.

Comment thread src/cli_agent_orchestrator/services/status_monitor.py Outdated
@codecov-commenter

codecov-commenter commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@4ad5e6b). Learn more about missing BASE report.

Files with missing lines Patch % Lines
.../cli_agent_orchestrator/services/status_monitor.py 87.50% 4 Missing ⚠️
src/cli_agent_orchestrator/providers/base.py 66.66% 1 Missing ⚠️
src/cli_agent_orchestrator/providers/codex.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #766   +/-   ##
=======================================
  Coverage        ?   91.89%           
=======================================
  Files           ?      207           
  Lines           ?    29266           
  Branches        ?        0           
=======================================
  Hits            ?    26895           
  Misses          ?     2371           
  Partials        ?        0           
Flag Coverage Δ
unittests 91.89% <86.66%> (?)

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.

@mastash3ff
mastash3ff force-pushed the fix/status-midburst-processing-probe branch from 8a81b77 to 9614ac6 Compare September 11, 2026 04:52

@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.

Re-reviewed 9614ac61e337310ad070bd1a02b30ddf6dc1ef9a against 405750f8374314f861bfa3c315b671b3f989c8d5, covering all seven changed files and the full provider/monitor state path.

The original MiniMax P2 is fixed. The opt-in predicate prevents discarded detection from changing its completion identity, buffer epoch or awaiting-turn flag; the restored settled frame remains IDLE with the pending turn intact. Providers without the capability remain unprobed, as intended.

One separate P2 remains in Codex's predicate, documented inline with a rectification recommendation. It accepts the normal detector's missing-composer fallback as positive work evidence. A partial redraw can therefore consume the dispatch arm, re-latch the previous completion, and suppress the subsequent genuine processing transition. The comparison with the exact base uses real Codex, pyte, monitor routing, worker-thread execution and quiescence timers, with both a current-style frame and the existing complex-response fixture. This is a monitor-state regression in the PR, not an additional shared-poller requirement or a demand for unsupported providers to opt in.

Comment thread src/cli_agent_orchestrator/providers/codex.py Outdated
@mastash3ff
mastash3ff force-pushed the fix/status-midburst-processing-probe branch from 9614ac6 to 73bcc83 Compare September 11, 2026 07:21

@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.

Re-reviewed 73bcc830f8eb63b470a9d9342e61388afdae4d23 against base 4ad5e6bf0f94ad34aa528eb08dc88362476d11ce. Both prior findings are fixed; no significant current-diff issue remains.

The Codex mid-burst predicate now requires a positive progress row before consulting the full detector. The normal missing-composer fallback no longer consumes the dispatch arm during a partial redraw. Real provider/pyte/monitor replays with two different composer frames preserve the arm through erase and restored old completion, then accept the genuine new spinner; the previous reviewed head reproduces the earlier failure under the same conditions. Trust, login, update, approval, error and exited-shell guards remain effective, including frames containing a spinner.

The MiniMax repair remains intact: non-opted-in providers are not probed, partial redraw does not mutate the turn/completion bookkeeping, and restored state remains armed and awaiting the dispatched turn. Existing throttling and sticky-latch behavior is preserved.

151 focused existing cases passed, with additional bounded composed controls through actual worker-thread routing and quiescence timers. External I/O alone was substituted; no live-provider acceptance is claimed. All 23 current-head checks returned SUCCESS at publication preparation. The updated Codecov comment was read; approval is not a claim of complete patch coverage.

@haofeif
haofeif merged commit aacd835 into awslabs:main Sep 11, 2026
23 checks passed
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.

Rendered-screen status detection never runs while a TUI keeps repainting, so a Codex worker reads IDLE for its whole turn

3 participants