Skip to content

fix(status): self-heal a stuck-PROCESSING terminal via a fresh capture-pane read - #558

Open
klabulan wants to merge 2 commits into
awslabs:mainfrom
klabulan:fix-status-stale-processing-selfheal
Open

fix(status): self-heal a stuck-PROCESSING terminal via a fresh capture-pane read#558
klabulan wants to merge 2 commits into
awslabs:mainfrom
klabulan:fix-status-stale-processing-selfheal

Conversation

@klabulan

@klabulan klabulan commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Problem

StatusMonitor.get_status()'s existing stale-PROCESSING re-check re-derives from the same
rolling self._buffers[terminal_id] the FIFO push pipeline feeds. Once the underlying process
goes genuinely idle and stops emitting output, that buffer stops changing too — re-running
detection against it produces the same PROCESSING/UNKNOWN result forever, even though the real
pane already shows a ready state.

Live-reproduced twice in one operator session on a real production deployment: a real chat
message queued behind a PROCESSING status sat undelivered for ~10 minutes, only delivered after
a manual tmux resize-window forced a fresh redraw. There was no automatic self-healing path for
this at all — a genuinely-idle terminal could get stuck showing PROCESSING indefinitely.

Fix

Adds a rate-limited fallback: when the cheap buffer-based re-check still can't resolve a cached
PROCESSING status, read the pane directly via get_backend().get_history() — a real tmux capture-pane, not the FIFO-fed buffer — and re-run provider detection against that. This is the
same reliable source providers/codex.py's _handle_trust_prompt already uses for init-time
dialog detection: tmux always holds the correct, current rendered pane state regardless of
output volume, so this can see a genuine ready state the stale buffer cannot.

Rate-limited via STALE_PROCESSING_CAPTURE_INTERVAL_S (default 3s, per-terminal) because
get_status() is a hot path (every poll, across the whole fleet) and a capture-pane read is a
real subprocess call, unlike the existing cheap buffer re-check — unbounded, it would repeat the
"fork storm freezes the server" class of problem run()'s own docstring already documents for
status detection in general.

Testing

  • 9 new tests (TestStaleProcessingCapturePane) covering: self-heals to the real status,
    correctly stays PROCESSING when the fresh capture-pane read also shows PROCESSING, non-fatal on
    a capture-pane read failure, non-fatal when no provider / get_provider() raises, rate-limiting
    (two immediate polls only shell out once; retried after the window elapses), and that the
    capture-pane fallback is skipped entirely when the existing cheap buffer re-check already
    resolves the status (no added overhead in the common case).
  • test/services/test_status_monitor.py: 38 passed (29 pre-existing + 9 new), 0 failures.
  • Full repo suite: 6095 passed, 35 skipped, 111 deselected, 1 xfailed, 0 failed (257.67s).
  • black --check / isort --check-only: clean.
  • Cherry-picked cleanly onto current main (a2c5afe) as a single self-contained commit.

🤖 Generated with Claude Code

…e-pane read

get_status()'s stale-PROCESSING re-check re-derives from the SAME rolling self._buffers[id]
the FIFO push pipeline feeds -- once the underlying process goes genuinely idle and stops
emitting output, that buffer stops changing too, so re-running detection on it produces the
same PROCESSING/UNKNOWN result forever even though the real pane already shows a ready state.

Live-reproduced twice in one operator session on a real production box (app.workain.ai): a real
chat message queued behind PROCESSING sat undelivered for ~10 minutes until a manual tmux resize
(forcing a fresh redraw) unstuck it. No automatic self-healing existed for this case at all.

Adds a rate-limited fallback (STALE_PROCESSING_CAPTURE_INTERVAL_S, default 3s) that reads the
pane directly via get_backend().get_history() -- a real tmux capture-pane, not the FIFO-fed
buffer -- the same reliable source codex.py's _handle_trust_prompt already uses for init-time
dialog detection. tmux always holds the correct, current rendered pane state regardless of
output volume, so this can see a genuine ready state the stale buffer cannot.

Rate-limited (not on every poll) because get_status() is a hot path across the whole fleet and
a capture-pane read is a real subprocess call, unlike the existing cheap buffer re-check --
unbounded, it would repeat the "fork storm freezes the server" class of problem run()'s own
docstring already documents for status detection in general.

@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 gap is real and your incident analysis is right — the quiescence re-detect and the cheap stale-PROCESSING re-check both re-derive from the same rolling buffer the FIFO pipeline feeds, so a ready marker evicted from the window (or a pyte screen corrupted by a truncated escape) wedges PROCESSING forever, and the #397 watchdog deliberately treats "FIFO delivered bytes" as healthy so it can't see this. The plumbing details are careful too: the lock-guarded check-and-set rate limit is correctly race-free, every failure path fails soft, cleanup is complete, and routing the result through _apply_detection instead of a second status cache was exactly the right call. Requesting changes on three things that are all fixable inside the PR's existing shape:

1. It forks tmux on the event loop — the exact pattern this codebase keeps excising. get_status() is called inline from async code at wait_until_status (utils/terminal.py:182, 1s poll), agent_step._wait_for_completion, and GET /sessions/{name} (per-terminal loop — a session with N processing terminals forks N capture-panes serially on the loop per request, and the web UI polls that endpoint). The codebase documents this hazard one endpoint over: GET /terminals/{id} wraps the same call in asyncio.to_thread with a comment naming the fork-storm history (api/main.py:2218-2224). Before this PR, the unconditional path was pure regex for claude; now every tmux provider forks. Either offload the remaining async call-sites or — better — see (3b).

2. It fires throughout every normal turn, not when stuck. The capture branch's only gates are cached == PROCESSING and the 3s window — the still-PROCESSING cheap re-check falls through into it (status_monitor.py:616-632), so every awaited busy terminal gets a capture-pane every 3s for its entire turn, on top of the fifo watchdog's every-4s probe. Your incident's actual signature is "rolling buffer stopped changing" — make the code require it: a timestamp bumped in _process_chunk, capture only when the buffer has been byte-identical ≥N seconds. One precondition, and the steady-state cost drops to ~zero.

3. A single mid-burst frame can sticky-latch a false ready — the premature-completion class with the blast radius amplified. The detection design's own comments forbid sampling mid-burst ("Detection NEVER runs mid-burst… eliminates the flaps", status_monitor.py:314-322); this fallback samples the pane at arbitrary mid-turn moments, and Ink repaints by clear/rewrite — a capture between writes can miss the spinner line while the previous response box parses COMPLETED. _apply_detection then latches ready sticky and disarms _allow_processing_revert on the upgrade (:251-252), so the agent's genuine PROCESSING is latch-blocked until the next input: agent_step returns early with partial output and the inbox pastes into a busy agent. Fixes that compose with (2): (a) require two consecutive identical captures before honoring a ready flip — claude_code.wait_until_input_ready already uses exactly this pattern; (b) consider moving the read into the quiescence callbacks (_on_raw_quiescent/_on_screen_quiescent) — already off-loop, already once-per-settle on a settled frame, which solves (1) and (2) structurally and would also cover the #463 burst→stall→settle watchdog blind spot you'll remember from that thread. If you keep it in get_status() deliberately, a should-fix either way: re-validate _last_status is PROCESSING under the lock before applying, since the subprocess widens the sample→apply TOCTOU window from microseconds to tens of milliseconds.

Smaller notes: provider.get_status isn't pure everywhere — kimi latches _has_received_input from whatever text it's fed and codex forks a second subprocess inside detection, so feeding a different source can perturb pipeline-shared state; under default-ON pyte the pipeline's status comes from get_status_from_screen while this consults the raw detector (antigravity's own comment calls its raw detector unreliable) — the asymmetry deserves a sentence and one screen-provider test; PR body says 9 new tests, there are 8.

The regression pin is genuine (the core test fails on main) and black/isort are clean; CI hasn't run at this head — I've approved the workflow runs. With the buffer-quiet gate, the double-capture confirm, and the async call-sites handled (or the quiescence-callback move), this closes a real hole the watchdog can't see — happy to re-review quickly.

@codecov-commenter

codecov-commenter commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
.../cli_agent_orchestrator/services/status_monitor.py 94.44% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #558   +/-   ##
=======================================
  Coverage        ?   91.01%           
=======================================
  Files           ?      179           
  Lines           ?    23340           
  Branches        ?        0           
=======================================
  Hits            ?    21242           
  Misses          ?     2098           
  Partials        ?        0           
Flag Coverage Δ
unittests 91.01% <94.73%> (?)

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.

… gate, double-capture confirm

call-me-ram's review requested changes on three real gaps in the stale-PROCESSING
capture-pane fallback, all fixable inside the PR's existing shape:

1. Forks tmux on the event loop. get_status() is called inline from async code at
   wait_until_status (utils/terminal.py), agent_step._wait_for_completion (both its
   poll-loop and deadline-check call sites), and GET /sessions/{name}
   (session_service.get_session, called per-terminal in a loop) -- none offloaded,
   unlike GET /terminals/{id}'s own established asyncio.to_thread pattern for the
   identical hazard. All four now wrapped the same way.

2. Fires throughout every normal turn, not just when stuck. The capture branch's only
   gates were cached==PROCESSING and the 3s rate-limit interval -- a busy terminal got
   a real capture-pane subprocess call every ~3s for its entire turn. Added
   STALE_PROCESSING_BUFFER_QUIET_S: a timestamp bumped in _process_chunk on every real
   chunk, consulted before even attempting the fallback -- it now only runs once the
   buffer has genuinely gone quiet, matching the incident's own signature ('the rolling
   buffer stopped changing'), not on every poll during active streaming.

3. A single mid-burst capture-pane sample can sticky-latch a false ready. This
   fallback's own detection design elsewhere (_schedule_screen_detection) deliberately
   never samples mid-burst for exactly this reason (Ink repaints by clear-then-rewrite).
   Added a double-capture confirm: a ready candidate is only honored once the SAME
   status is read on two consecutive (rate-limit-interval-apart) capture-pane reads --
   same 'confirm, don't trust one sample' pattern claude_code.py's own
   wait_until_input_ready already uses. Also added the requested TOCTOU re-validation:
   since the capture-pane read runs outside the lock (a real subprocess call), re-check
   under the lock that the terminal is still the same stale-PROCESSING terminal before
   applying a capture that may now be stale -- and return the fresher real status
   instead of a stale cached one when discarding.

Two smaller notes from the same review (provider.get_status() purity for kimi/codex;
raw-vs-screen detector asymmetry) are disclosed in a docstring rather than fixed --
both are pre-existing properties of a fallback that only ever runs once a terminal is
already stuck, not something this round's scope should expand to cover.

Tests: rewrote TestStaleProcessingCapturePane's existing 6 tests for the new gates
(buffer-quiet + confirm), added 5 new tests (recently-changed-buffer gate, unset
buffer_changed_at gate, differing-second-read never confirms, TOCTOU discard, and the
two-confirming-reads self-heal path itself) -- 11 total in that class, all passing.
Added 3 to_thread-dispatch pins (wait_until_status, agent_step's completion poll,
GET /sessions/{name}) using  so real execution continues
while the dispatch itself is inspected -- every other test in these areas mocks
status_monitor/session_service entirely and can't see HOW the call was made, so a
regression back to a bare synchronous call would otherwise stay green.

Full suite: 6102 passed, 35 skipped, 111 deselected, 1 xfailed, 0 failed.
black/isort clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@klabulan

klabulan commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — all three requested changes pushed (b4230fd), plus the smaller notes disclosed (not fixed, see below).

1. Forks tmux on the event loop

Fixed at all the sites you named plus the one you predicted ("or better, see (3b)" wasn't taken, but I traced every call site as requested): wait_until_status (utils/terminal.py), both call sites inside agent_step._wait_for_completion (the poll loop AND the deadline-check), and GET /sessions/{session_name} (session_service.get_session, which loops status_monitor.get_status() once per terminal). All four now dispatch via asyncio.to_thread, matching GET /terminals/{id}'s own established pattern for the identical hazard.

2. Fires throughout every normal turn

Added STALE_PROCESSING_BUFFER_QUIET_S: a timestamp bumped in _process_chunk on every real chunk, consulted before the fallback even attempts a capture-pane read. It now only runs once the buffer has genuinely gone quiet for that long — a terminal mid-burst never reaches the gate at all, only one that's actually stopped producing output does. Matches the incident's own signature directly, as you suggested.

3. Single mid-burst sample can sticky-latch a false ready

Took your first framing (double-capture confirm) over the quiescence-callback move (3b) — the quiescence timers only fire on new chunks arriving, so they can't help the genuinely-stuck case this whole fallback exists for (no new chunks = no timer ever re-arms), and moving the read there would mean it doesn't run in exactly the situation that matters. Instead: a ready candidate is only honored once the same status is read on two consecutive (rate-limit-interval-apart) capture-pane reads — same "confirm, don't trust one sample" pattern claude_code.py's own wait_until_input_ready already uses for an analogous settle-race.

Also took the TOCTOU re-validation you flagged: since the capture-pane read runs outside the lock (a real subprocess call), re-checks under the lock that the terminal is still the same stale-PROCESSING terminal before applying — and now returns the fresher real status instead of a stale cached one when it discards a race.

Smaller notes

Disclosed rather than fixed this round (both are pre-existing properties of a fallback that only ever runs once a terminal is already stuck, not new scope this round should absorb):

  • provider.get_status() purity (kimi/codex) — noted in _fresh_capture_pane_status's docstring.
  • Raw-vs-screen detector asymmetry — also documented there; since this fallback only runs when both the pipeline's normal path (raw or screen, whichever the provider uses) has already failed to resolve the terminal, the raw-detector choice here is a best-effort second opinion either way, not the primary signal.
  • Test count: 8 in the original PR, not 9 as the body said — apologies, miscounted. Now 13 across both rounds (11 in TestStaleProcessingCapturePane for the fallback itself, 3 to_thread-dispatch pins for the offloaded call sites).

Tests

Rewrote the existing 6 tests for the new gates, added 5 new (recently-changed-buffer gate, unset-timestamp gate, differing-second-read-never-confirms, TOCTOU discard, the two-confirming-reads self-heal path) — 11 total, plus 3 to_thread dispatch pins using wraps=asyncio.to_thread so real execution continues while the dispatch call itself is inspected (every other test in these areas mocks status_monitor/session_service entirely and can't see how the call was made — a regression back to a bare synchronous call would otherwise stay green).

Full suite: 6102 passed, 35 skipped, 111 deselected, 1 xfailed, 0 failed. black/isort clean.

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

Round-2 re-review at b4230fd. Item 2 is properly fixed and I verified the gate through the real _process_chunk path, not just the tests' hand-set dict. Item 1 is fixed at the four sites I named but two on-loop call sites remain. Item 3 is not closed, and the reason is structural rather than an oversight: the buffer-quiet gate you added for (2) selects precisely for a pane that is not changing, so the "two consecutive identical reads" confirm from (3) is byte-identical by construction and carries no information — it costs one extra poll of latency and rejects nothing. Any deterministic misread confirms itself. I proved that with the real KiroCliProvider detector: a plausible rendered snapshot of a busy kiro pane returns COMPLETED, and driven end-to-end through get_status() it latches sticky-ready, disarms _allow_processing_revert, and blocks the agent's genuine PROCESSING for the rest of the turn. Details and a minimal fix that uses predicates the codebase already ships are in must-fix 2 below.

What genuinely improved since round 1: the quiet gate is exactly the right shape (timestamp bumped at the one place output actually lands, status_monitor.py:206, consulted before anything forks — :665-671) and it works; the four offloads match GET /terminals/{id}'s pattern with comments that name the hazard accurately; the three to_thread dispatch pins are the right kind of test and would catch a regression the surrounding tests structurally cannot; the TOCTOU re-validation under the lock plus returning the fresher status instead of the entry-time snapshot (:690-704) is better than what I asked for; and the None-vs-0.0 sentinel reasoning on both new timestamp maps is careful. Fail-soft on every error path and complete cleanup in both clear_terminal and reset_buffer still hold.

Must-fix

1. Item 1 is a partial offload — two async call sites still fork tmux on the loop.

src/cli_agent_orchestrator/providers/copilot_cli.py:318, inside async def initialize (:280), polls status_monitor.get_status(self.terminal_id) inline every 1s for up to 60s. That function's own docstring, added by the #494 fork-storm fix, ends with (:289-290):

status_monitor.get_status stays inline -- it is in-memory only, no blocking I/O.

At this head that sentence is unconditionally false (it was already optimistic — copilot's own get_status falls back to self._history() when the buffer has no visible text, copilot_cli.py:456-461), and copilot init is a regime that can trip the new branch: cached status is PROCESSING while the CLI boots, and if the pane goes quiet for >3s (auth, MCP boot) it forks a capture-pane on the loop every ~3s for up to ~20 forks per init, multiplied by concurrent session creations. I haven't reproduced that live — medium confidence on the frequency, none needed on the hazard, since it's the same call in the same shape you already agreed to offload elsewhere. Second site: src/cli_agent_orchestrator/services/flow_service.py:206 via _is_terminal_busy at :243, inside async def execute_flow — lower frequency, and that function already blocks the loop with an inline subprocess.run(timeout=30), so I'd take the two-line fix rather than leave the count wrong.

Minimal fix: await asyncio.to_thread(status_monitor.get_status, ...) at both, and update copilot's docstring — it is load-bearing documentation that the next person will trust.

2. Item 3: the double-capture confirm cannot see the failure it was added for, and a busy kiro pane still latches a false COMPLETED.

src/cli_agent_orchestrator/services/status_monitor.py:765 reads the pane and :775 feeds it to provider.get_status(fresh_output) for every provider. The codebase already has a predicate for exactly this operation — capture-pane snapshot in, provider.get_status() out — and it says most providers must not do it (providers/base.py:161-166):

Opt-in for the deferred-init direct status probe (capture-pane bypass). Set True on providers whose get_status() detector is line-oriented and works correctly on a rendered capture-pane snapshot. Providers whose get_status() relies on dispatch bookkeeping (e.g. kiro_cli) must leave this False — their COMPLETED/IDLE split is not screen-detectable.

Only opencode_cli sets it True; terminal_service.py:729-730 honors it before its own direct probe, and :652 names kiro_cli, antigravity_cli and cursor_cli as the providers it protects. The new fallback ignores the flag, and it errs in the false-ready direction. With the real detector (no mocking of get_status), a rendered snapshot of a busy kiro pane — previous turn's transcript, ▸ Credits: line, live Kiro is working..., and the always-rendered composer placeholder at the bottom (kiro_cli.py:73 NEW_TUI_IDLE_PATTERN, which kiro_cli.py:438's own comment notes is drawn even when kiro is not idle) — resolves through Check 2 → Check 4/5 to COMPLETED, because in a rendered frame the placeholder sits physically below the working line and below the credits line, whereas in the raw FIFO byte stream the ordering those checks were tuned against is the opposite.

Concrete, mundane failure: kiro is running a silent tool call (npm test, a network wait — anything ≥3s with no pane repaint). Buffer quiet ≥3s ⇒ gate opens (:671). Read 1 → COMPLETED candidate. Read 2, three seconds later, sees the same bytespending == detected at :790 ⇒ confirmed. _apply_detection(COMPLETED) latches it and, since last was PROCESSING, sets _allow_processing_revert = False (:275-276). The agent's real PROCESSING is then latch-blocked for the whole turn: agent_step returns early with partial output and send_input — which guards only ERROR/WAITING_USER_ANSWER (terminal_service.py:987-1008) — pastes the next queued message into a working pane. Verified end-to-end at head; test source and output in "What I verified".

The same argument applies in the other direction for the four supports_screen_detection providers: their raw detector is explicitly not the one calibrated for rendered content (base.py:153-159, "ONLY when it ships a purpose-built get_status_from_screen() calibrated for a composited fixed-height viewport (not the raw byte stream)"), yet the fallback consults the raw one and never get_status_from_screen. Disclosing that in the docstring isn't enough now that the confirm can't catch a systematic misread — it is the failure mode.

Minimal fix, using what's already there:

fresh = get_backend().get_history(
    provider.session_name, provider.window_name,
    tail_lines=PYTE_SCREEN_ROWS, strip_escapes=True,
)
if getattr(provider, "supports_screen_detection", False):
    detected = provider.get_status_from_screen(fresh.splitlines())
elif getattr(provider, "supports_direct_status_probe", False):
    detected = provider.get_status(fresh)
else:
    return None  # detector is raw-stream-tuned; a rendered snapshot can't be trusted

That fails closed for kiro/cursor, keeps the self-heal for the providers that can actually read a snapshot (and for agy the snapshot is a better source than the append-only log — antigravity_cli.py:740-746), and gets the input shape right: PYTE_SCREEN_ROWS == 200 matches the viewport get_status_from_screen expects, and strip_escapes=True matches its escape-free contract. Keep the confirm — it still helps in the wedged-FIFO case where the pane is moving between reads.

If you'd rather not touch detector routing in this PR, the other way to close it is to make "stuck" mean stuck: record _processing_since when _apply_detection latches PROCESSING and require e.g. ≥30s of continuous PROCESSING in addition to buffer-quiet. A genuinely wedged terminal has been PROCESSING for minutes; a silent tool call has not. That doesn't fix the wrong-detector problem but it removes the ordinary-turn exposure, and it's ~6 lines.

3. The pending candidate never expires and survives a new turn, so one read can confirm a candidate recorded much earlier.

_pending_stale_capture (status_monitor.py:788-798) is cleared only by an intervening PROCESSING/UNKNOWN capture, clear_terminal, or reset_buffer. notify_input_sent, clear_rolling_buffer and a PROCESSING re-latch all leave it armed — verified: after a full simulated turn boundary (notify_input_sent + clear_rolling_buffer + _apply_detection(PROCESSING)), _pending_stale_capture["t1"] is still IDLE, and the next single ready read (one get_history call) is honored immediately. So "two consecutive reads, an interval apart" is really "two consecutive attempts, unbounded in time" — a stale candidate from a turn ten minutes ago can latch a lone mid-repaint frame now, which is the exact single-frame latch the confirm was supposed to prevent.

Minimal fix: store (status, monotonic) and require the confirming read within ~2× STALE_PROCESSING_CAPTURE_INTERVAL_S, and drop the candidate in notify_input_sent (a new turn invalidates it).

Non-blocking

  • get_history at :765 takes the default tail_lines=TMUX_HISTORY_LINES (200), i.e. the visible pane plus scrollback, which is a third input shape — neither the 8 KB stream tail the raw detectors were tuned on nor the viewport the screen detectors expect. Narrow but nasty instance: kiro's error check is a whole-text any(indicator in clean_output) over a single specific string (kiro_cli.py:143, :519), so that string surviving in scrollback from an earlier turn yields ERROR — which is in _STICKY_READY_STATUSES and makes agent_step raise StepExecutionError(kind="error"). Bounding the read (as the fix above does, and as _worker_is_started_direct already does deliberately) also bounds this.
  • The quiet gate is never exercised through its own write site: every test sets _buffer_changed_at directly, so deleting status_monitor.py:206 would silently disable the entire feature with the suite still green. One test that pushes chunks through _process_chunk and then asserts the fallback becomes eligible after the window would pin it (I wrote that test to verify this item; happy to hand it over).
  • Still no screen-provider test from last round's smaller note. The docstring sentence is there, but the behaviour it describes is now the mechanism in must-fix 2 — it wants a test, not a paragraph.
  • asyncio.to_thread isn't cancellable, so a cancel_event set while a capture-pane hangs waits the subprocess out in _wait_for_completion. Same shape as the existing offloads, just worth knowing now that a subprocess can sit under it.
  • PR comment says 11 tests in TestStaleProcessingCapturePane; --collect-only says 12. Third round of counting — worth just running the collector before writing the number.

Asks

  • CI at this head: Unit Tests green on 3.10/3.11/3.12, Code Quality green. The only red check is Security Scan / Trivy, which fails in the scanner step with no finding attached (looks like a DB/rate-limit infra failure, unrelated to this diff) — re-run it before merge so the signal is clean.
  • The two constants stay module-level rather than server settings, which I think is right for now; if either ever needs to differ per deployment, please route it through ConfigService rather than an env var.

What I verified

At b4230fd, in a detached worktree (git worktree add … refs/remotes/pr/558), .venv interpreter, PYTHONPATH=$PWD/src:

  • pytest test/services/test_status_monitor.py test/utils/test_terminal.py test/services/test_agent_step.py test/api/test_api_endpoints.py::TestGetSession -q144 passed. test/services/test_status_monitor.py alone → 42 passed, 12 collected in TestStaleProcessingCapturePane.
  • Item 1: enumerated every status_monitor.get_status( call site at head (git grep): the four you were told about are offloaded (utils/terminal.py:188, agent_step.py:186 and :221, api/main.py:2038); inbox_service.py:79 is already reached only via to_thread (main.py:176/:194, inbox_service.py:54); session_service.py:139, terminal_service.py:906/:987 and memory_service.py:2941/:2955 are reached from async only through offloaded wrappers. Remaining on-loop: copilot_cli.py:318 (async initialize, 1s poll ≤60s) and flow_service.py:206 via :243 (async execute_flow).
  • Item 2: drove the real _process_chunk (patched get_server_settings, real lock/bump) with five chunks while cached status was PROCESSINGbackend.get_history.call_count == 0, i.e. no fork mid-burst; then set _buffer_changed_at back past STALE_PROCESSING_BUFFER_QUIET_S → exactly one get_history. The gate works, through its real write site.
  • Item 3: (a) real KiroCliProvider("test1234","s1","w1","developer")._initialized = True, get_status(<synthetic rendered busy pane>)TerminalStatus.COMPLETED. (b) Same provider wired into StatusMonitor with _last_status=PROCESSING, _buffers="", _buffer_changed_at = now-30, backend.get_history returning that pane: poll 1 → PROCESSING (candidate), reset the rate-limit key, poll 2 → COMPLETED, _last_status == COMPLETED, and a following _apply_detection(PROCESSING) leaves it at COMPLETED. (c) Both confirming reads were byte-identical (get_history.call_args_list identical), and provider.get_status_from_screen was never called for a supports_screen_detection=True provider. (d) Turn-boundary probe: notify_input_sent + clear_rolling_buffer + _apply_detection(PROCESSING) leave _pending_stale_capture == IDLE, and the next single read is honored (one get_history call). All five probe tests pass at head; the pane text in (a)/(b) is synthetic — shaped from kiro's own patterns and comments, not a captured live pane — so treat the realism of that exact frame as high-but-not-certain and the mechanism (probe-unsafe detector + vacuous confirm + sticky latch + disarmed revert) as confirmed.
  • Read base.py:153-186, terminal_service.py:641-680/:729-730, antigravity_cli.py:739-747, clients/tmux.py:543-583, constants.py:70/:240-241 for the flag contracts, the capture-pane defaults and the raw-vs-rendered asymmetry.

The self-heal is still the right feature and the incident is still real — item 2's gate is the piece I most wanted and it landed well. It's the source-of-truth question in must-fix 2 that has to be closed before this can go in, and routing through the two existing opt-in flags is a smaller change than what you already did this round.

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.

3 participants