fix(claude-native): make Claude's status file the source of truth - #4344
fix(claude-native): make Claude's status file the source of truth#4344daniellok-db wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes Claude-native’s sessions/<pid>.json status file the single source of truth for session running/idle (while readable), eliminating prior cross-source arbitration and several client/server-side “synthetic status” control signals that could cause stuck spinners or mid-turn UI flicker.
Changes:
- Runner: prefer Claude’s status file for running/idle; suppress pane-derived status while the file is active, and retire file polling on pane death.
- Server: stop publishing synthetic
session.statusbrackets for policy-deny and/compact, and keep background-task status asidlewith a count (normalizing legacywaiting). - Web: stop forging
sessionStatus: "failed"on stream-open failures and remove the client-side “revive stray completed response” hack; update unit tests accordingly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/store/chatStore.ts | Stops setting sessionStatus on stream-open failures; removes stray-response revive logic; updates comments for revised lifecycle assumptions. |
| web/src/store/chatStore.test.ts | Removes tests for the deleted revive behavior; updates expectations around policy-deny and stream reconnect behavior. |
| tests/test_claude_native_status_file.py | Updates poller tests to reflect “file owns status while readable” and adds retirement behavior for dead panes / orphaned files. |
| tests/test_claude_native_forwarder.py | Updates expectations so Stop posts idle with background_task_count (no longer relabeling to waiting). |
| tests/server/integration/test_sessions_child_sessions.py | Ensures background-task counts don’t suppress terminal-delivery behavior (keeps status idle). |
| tests/runner/test_resource_registry.py | Updates watcher/poller interaction tests: no pane status while file owns, and retire-on-exit behavior. |
| omnigent/server/routes/sessions/routes_events.py | Removes policy-deny synthetic status edges; clarifies/extends background-task normalization rationale. |
| omnigent/server/routes/_sessions/helpers.py | Removes /compact synthetic session.status bracketing in favor of compaction SSE events only. |
| omnigent/runner/resource_registry.py | Implements “file owns status” gating for pane status emissions and retires the poller on pane exit. |
| omnigent/claude_native_status_file.py | Documents file-as-authority model and replaces freshness-based running assertion with an explicit retire() mechanism. |
| omnigent/claude_native_forwarder.py | Stops relabeling Stop-with-background-tasks from idle to waiting; keeps count delivery semantics. |
| // The deny publishes no session-status pair at all: the agent never ran, | ||
| // so there is no turn to report. The live turn streaming alongside it is | ||
| // therefore untouched — no stray idle to fold its bubble, and no | ||
| // client-side revive needed to undo one. | ||
| expect(state.sessionStatus).toBe("running"); | ||
| expect(state.activeResponse).toEqual({ | ||
| responseId: "resp_in_flight", | ||
| state: "streaming", | ||
| error: null, | ||
| completedAt: expect.any(Number), | ||
| }); |
| // How long after a terminal edge a delta still belongs to the finished | ||
| // turn. A scheduled wake (cron / wakeup fires at 60s minimum) streams | ||
| // its FIRST deltas ahead of the transcript batch that names the new | ||
| // turn; attributing those to the previous turn popped its "Worked for" | ||
| // fold open at the start of every /loop iteration. | ||
| const REVIVE_WINDOW_MS = 15_000; | ||
|
|
|
…status #4344 made Claude's `sessions/<pid>.json` the source of truth for claude-native running/idle, but missed a publisher: the transcript forwarder still posted `running` when it first saw a turn's assistant output. That produced a visible flicker on every short turn — session.status idle <- the file; the turn really ended session.status running <- the transcript forwarder, late session.status idle <- Stop because the file flips the instant Claude settles, while a transcript-derived edge can only fire once a poll has parsed assistant output. It lands after the file's `idle` and re-asserts `running` on a session that already finished. That POST never existed to report status. #1499 added it to carry `response_id` so the web store opens a streaming `activeResponse`; it carried `running` only because `_publish_status` gates the id on it. Same shape as the policy-deny and `/compact` pairs #4344 removed: a bubble-lifecycle signal multiplexed onto `session.status`. Deleting it needs nothing in its place. The items are a separate POST (`external_conversation_item`) and already carry their own `response_id`, so they still forward and still group. `posted_running_response_id` and `_turn_has_assistant_output` become dead and go with it. Accepted cost: `activeResponse.state === "streaming"` is now unreachable for claude-native on the live path, so a tool call renders `no-output` rather than `input-available` between dispatch and result — no spinner in that gap. Once the result lands, `output !== null` wins and the card renders normally. This also preserves for free the property three tests pin (`renderItems.test.ts:704`, `:720`, `:736`): a tool whose result never arrives must not spin forever. A follow-up should derive tool liveness from `sessionStatus` + newest-turn instead of `activeResponse`, which restores the spinner and drops the turn-id dependency for good — deferred because it touches the renderer every harness shares. claude-native only. `_forward_available_items` has one entry point (`forward_claude_transcript_to_session`); goose, hermes, and codex post their own id-bearing `running` from their own forwarders, where it is their only status source. `post_external_session_status` keeps its signature and the web `session.status` handler stays generic, so those harnesses are untouched (170 of their tests pass unchanged). Co-authored-by: Isaac
Claude's `sessions/<pid>.json` reports what Claude is doing; the tmux pane diff only infers it from redraws. Both were publishing session status, and union (either source asserts it), `idle` an intersection (both must agree, via a 10s `asserts_running` freshness window). You could not state what a session's status *was* without replaying which edge landed last, and the window let a `SIGKILL`ed Claude parked on a permission prompt pin the spinner forever: `waiting` was exempt from the TTL, and the poller only retires when the file *vanishes*, which a killed process never does. The file now decides while it is readable. Precedence is one rule: the file, unless no file resolved (Claude < v2.1.139), unless the pane is dead. - resource_registry: the pane publishes no status while the poller is active — it keeps the activity badge and owns pane death. Deletes `_blocked_reason` and the freshness-window constant. - status_file: `asserts_running` is gone; a new `retire()` is called from the watcher's exit path, since a killed Claude leaves its record behind holding a value that would otherwise keep owning the session. - forwarder: `Stop` no longer decides status. It carries the two things the file cannot express — the background-shell count (its `shell` literal is a boolean; the indicator renders a number) and the sub-agent delivery edge. `StopFailure` stays: the file has no failure literal, so it is the only source of the red pill and a failed scheduled run. - Ordering stopped mattering: `Stop`'s idle and the file's idle are the same edge and share a dedup baseline, so whichever lands second is collapsed. One idle reaches the client, no flicker. This removes the `waiting` relabel at its source, where #4266 normalized it at server ingress. That normalization stays — it covers runners that predate this change and still post `waiting`. Also stop publishing status as a control signal. Policy-deny and `/compact` bracketed themselves with synthetic `running`→`idle` pairs, so a denied tool call reported a turn that never ran — and its stray idle folded a live turn's bubble mid-stream. The terminal `response.completed` already unblocks live-tail consumers and the compaction bubble owns its own spinner. With the cause gone, `reviveStrayCompletedResponse` — the client-side hack that flipped `sessionStatus` back to `running` on the next delta — goes too. The web client also stops forging `sessionStatus: "failed"` when its own stream fails to open: losing our stream says nothing about what the agent is doing. No other harness changes behaviour — the poller is claude-native only, so `_file_owns_status()` is always false for the seven other PTY-watched roles and they publish exactly as before. Co-authored-by: Isaac
…status #4344 made Claude's `sessions/<pid>.json` the source of truth for claude-native running/idle, but missed a publisher: the transcript forwarder still posted `running` when it first saw a turn's assistant output. That produced a visible flicker on every short turn — session.status idle <- the file; the turn really ended session.status running <- the transcript forwarder, late session.status idle <- Stop because the file flips the instant Claude settles, while a transcript-derived edge can only fire once a poll has parsed assistant output. It lands after the file's `idle` and re-asserts `running` on a session that already finished. That POST never existed to report status. #1499 added it to carry `response_id` so the web store opens a streaming `activeResponse`; it carried `running` only because `_publish_status` gates the id on it. Same shape as the policy-deny and `/compact` pairs #4344 removed: a bubble-lifecycle signal multiplexed onto `session.status`. Deleting it needs nothing in its place. The items are a separate POST (`external_conversation_item`) and already carry their own `response_id`, so they still forward and still group. `posted_running_response_id` and `_turn_has_assistant_output` become dead and go with it. Accepted cost: `activeResponse.state === "streaming"` is now unreachable for claude-native on the live path, so a tool call renders `no-output` rather than `input-available` between dispatch and result — no spinner in that gap. Once the result lands, `output !== null` wins and the card renders normally. This also preserves for free the property three tests pin (`renderItems.test.ts:704`, `:720`, `:736`): a tool whose result never arrives must not spin forever. A follow-up should derive tool liveness from `sessionStatus` + newest-turn instead of `activeResponse`, which restores the spinner and drops the turn-id dependency for good — deferred because it touches the renderer every harness shares. claude-native only. `_forward_available_items` has one entry point (`forward_claude_transcript_to_session`); goose, hermes, and codex post their own id-bearing `running` from their own forwarders, where it is their only status source. `post_external_session_status` keeps its signature and the web `session.status` handler stays generic, so those harnesses are untouched (170 of their tests pass unchanged). Co-authored-by: Isaac
Pressing Enter sets `chatStore.status = "streaming"` synchronously, but
leaves `sessionStatus` alone — the two fields mean different things
("this client's send is in flight" vs "the server says the agent is
working"). The sidebar row opted into the local one and lights up
immediately (`isStartingUp` in Sidebar.tsx reads `s.status`); the chat
pane read only `sessionStatus`, so its spinner waited for the server's
`running` edge and the two surfaces disagreed for the whole dispatch
round-trip.
`computeShowsWorking` now takes `localSendInFlight` and treats it as
working. It also survives the `runnerOnline === false` gate for the same
reason a live running/waiting status does: sending to an asleep runner
relaunches it, and `/health` reads stale-offline during that window at
its 10s cadence. A pending elicitation still outranks it, so the prompt
and the shimmer never stack.
The flag is opt-in, so a cross-client or TUI-typed turn — which sets no
local status here — still shows nothing until the server speaks.
Co-authored-by: Isaac
A server restart mid-turn left the session with no working indicator and
no stop button for the rest of the turn.
The tunnel reconnecting usually means the *listener* restarted — a
deploy, a crash, a replica failover — which wipes the server's in-memory
`_session_status_cache`. This runner keeps running, so every dedup
baseline still asserts its last edge was delivered, and nothing
re-asserts on its own: Claude's `sessions/<pid>.json` is written only
when its value *changes*, and the pane watcher's edges are coalesced to
the idle->running transition. So the restarted server never learns the
session is running.
Nothing else covers it. The server's cache-miss fallback polls the
runner, but `GET /v1/sessions/{id}` derives status from `_active_turns`,
which is empty for native harnesses. And `_catch_up_scan` — the existing
`on_reconnect` hook — skips native harnesses outright.
`resource_registry.resync_session_statuses()` drops the published-edge
baselines so the next poll republishes the current value verbatim. The
claude-native pollers are re-armed too: they hold their own edge/mtime
baselines on the watcher thread, so clearing only the registry side would
leave them silent. The exit-classification memo (`_last_session_status`)
is deliberately untouched — it tracks what the PANE last did, not what
the server has heard, and clearing it would make a crash right after a
reconnect read as a clean shutdown. A retired poller stays retired, so a
reconnect can't hand status back to a dead Claude's leftover record.
Pre-existing, but recently more exposed: while the pane watcher published
`running` on every fresh redraw it papered over this within a second. Now
that the file owns the status, the file is the only publisher — and it has
nothing to say.
Also adds the first logging to `claude_native_status_file` (resolve hit,
resolve give-up, retire, resync). The module had none, so "did the poller
ever find the file?" was only answerable by re-deriving the resolution by
hand against a live session — which is exactly what diagnosing this took.
Co-authored-by: Isaac
953187f lit the chat pane's "Working…" shimmer optimistically on send, which took the in-thread slot that `RunnerStartingIndicator` used to own (it renders only when the shimmer is absent). A send that has to boot a runner then read "Working…" instead of "Starting up…" / "Cloning repository…" — dropping the more specific copy at exactly the moment the user needs it, since booting is the slow part. `ChatPage` now stands the optimistic path down while a terminal-first spin-up or a managed-sandbox launch stage is in flight. Only `localSendInFlight` is gated: a server-confirmed `running`/`waiting` still lights the shimmer, and by then the spin-up cue has self-gated to null, so the turn is never left with no indicator at all. Co-authored-by: Isaac
An in-flight tool card showed "No output" instead of a spinner for claude-native. The spinner is gated on the bubble's lifecycle reaching `streaming`, which is only reachable through a streaming `activeResponse` — and claude-native never opens one: its running/idle lives in Claude's status file (`sessionStatus`), the transcript forwarder no longer posts a turn-start `running`, so no bubble is ever `streaming` and `trailingLiveToolCallIds` returns nothing. Widen the gate: the trailing tool phase spins when EITHER the bubble is the streaming `activeResponse` (unchanged, in-process harnesses) OR the session is running and the bubble is its newest turn. `buildBubbles` takes a `sessionRunning` flag and computes the newest turn id (`newestAssistantTurnId`, scanning back from the end); `ChatPage` passes `computeIsWorking(sessionStatus)`. This is the same "last assistant bubble + session running" liveness `BlockRenderer` already uses to keep the trace expanded, so the two agree. `lifecycle` itself is untouched — fork, fold, cancelled, and failed all read it as before, and the in-process harnesses are unaffected (the new condition only ADDs the session-driven case). The property the three never-spin tests pin is preserved: a settled turn — reloaded history, a finished turn, a dead harness whose session reads idle — is neither streaming nor the running session's newest turn, so a result-less tool still resolves to `no-output`, never a perpetual spinner. The one subtlety is the reuse cache: a running→idle flip carries no block change, so `liveTurnId` joins the cache key and `reusablePrefix` refuses to reuse a bubble matching the previous or current live turn — otherwise a dangling tool would keep its stale spinner after the turn settled. Co-authored-by: Isaac
75bc337 to
821e052
Compare
Related issue
Closes #
Summary
Claude's
sessions/<pid>.jsonreports what Claude is doing. The tmux panediff only infers it from redraws. Both were publishing session status, and
#4195 reconciled them with opposite rules per direction —
runningwas aunion (either source asserts it),
idlean intersection (both must agree, viaa 10s
asserts_runningfreshness window). You could not state what a session'sstatus was without replaying which edge landed last.
That asymmetry had a live bug:
asserts_runningnever decayed while the rawstatus was
waiting, on the theory that closing a dialog guarantees a newwrite.
SIGKILLClaude at a permission prompt and the file keepswaiting—the poller only retires when the file vanishes, which a killed process never
does. The
idleedge was suppressed permanently and the spinner spun untilterminal exit.
The file now decides while it is readable. One precedence rule:
The hooks stop competing and keep only what the file cannot express:
sessions/<pid>.jsonblocked_onStopStopFailurefailededge (the file has no failure literal)Stopstill posts because its count is a number ("2 background tasks stillrunning") where the file's
shellliteral is a boolean.StopFailurestillposts because the interactive file returns to
idleon a turn error exactly ason success — it is the only source of the red pill,
last_task_error, and afailed scheduled run.
Ordering stopped mattering.
Stop'sidleand the file'sidleare thesame edge and share one dedup baseline, so whichever lands second is collapsed.
The pane publishes nothing while the file is readable, so a post-turn prompt
redraw cannot contradict either. One
idlereaches the client regardless ofarrival order — no flicker.
This removes the
waitingrelabel at its source, where #4266 normalized it atserver ingress. That normalization stays — it covers runners that predate
this change and still post
waiting.Also: status stops being used as a control signal. Policy-deny and
/compactbracketed themselves with syntheticrunning→idlepairs, so adenied tool call reported a turn that never ran — and its stray
idlefolded alive turn's bubble mid-stream. The terminal
response.completedalreadyunblocks live-tail consumers, and the compaction bubble owns its own spinner.
With the cause gone,
reviveStrayCompletedResponse— the client-side hack thatflipped
sessionStatusback torunningon the next text delta — goes too.The web client also stops forging
sessionStatus: "failed"when its ownstream fails to open: losing our stream says nothing about what the agent is
doing.
Net -82 lines, and every deletion is a rule that no longer needs to exist.
Second commit: the transcript forwarder was still publishing status
The first commit claimed the file is the source of truth but left one publisher
behind, which showed up as a flicker on every short turn:
The file flips the instant Claude settles, but a transcript-derived edge can
only fire once a poll has parsed assistant output — so it lands after the
file's
idleand re-assertsrunningon a session that already finished.That POST never existed to report status. #1499 added it to carry
response_idso the web store opens a streaming
activeResponse; it carriedrunningonlybecause
_publish_statusgates the id on it. Same shape as the policy-deny and/compactpairs removed above — a bubble-lifecycle signal multiplexed ontosession.status.Deleting it needs nothing in its place: the items are a separate POST
(
external_conversation_item) and already carry their ownresponse_id, sothey still forward and still group.
posted_running_response_idand_turn_has_assistant_outputbecome dead and go with it (-79 lines).Accepted cost.
activeResponse.state === "streaming"is now unreachable forclaude-native on the live path, so a tool call renders
no-outputrather thaninput-availablebetween dispatch and result — no spinner in that gap. Once theresult lands,
output !== nullwins and the card renders normally. This alsopreserves for free the property three tests pin (
renderItems.test.ts:704,:720,:736): a tool whose result never arrives must not spin forever.Follow-up, not here: derive tool liveness from
sessionStatus+newest-turn instead of
activeResponse. That restores the spinner and dropsthe turn-id dependency for good, but it touches the renderer every harness
shares, so it deserves its own review.
ELI5
Two people were reporting whether the agent was busy: one reads Claude's own
status file, the other watches the terminal for flickering pixels. They
disagreed, so we wrote tie-break rules — and the tie-break rules had a bug that
could leave the spinner on forever. Now the file reports, the pixel-watcher
only fills in for old Claude versions and notices when the process dies, and
the tie-break rules are gone.
Other harnesses
Unaffected. The poller is built only for
CLAUDE_NATIVE_TERMINAL_ROLE, so_file_owns_status()is alwaysFalsefor the seven other PTY-watched roles(pi, cursor, kiro, goose, qwen, kimi, hermes) — they publish exactly as before.
post_external_session_statuskeeps its signature,_HOOK_EVENT_TO_STATUSisclaude-only, and
_publish_turn_status's harness gate is untouched.Same for the second commit:
_forward_available_itemshas one entry point(
forward_claude_transcript_to_session). goose, hermes, and codex post theirown id-bearing
runningfrom their own forwarders, where it is their onlystatus source — so it must keep publishing, and it does. The web
session.statushandler stays generic, so they keep opening their bubble offrunning+response_id. 170 of their forwarder tests pass unchanged.Test Plan
Rebased on
origin/main(57ff1b3) and re-run after reconciling with #4266.Tests replacing the deleted arbitration:
test_pane_publishes_no_status_while_the_file_owns_it— a mid-turn redraw anda post-turn prompt redraw both publish nothing.
test_parked_pane_stays_running_then_recovers_on_pane_death— the bugabove: quiet pane under a dialog stays
running, then a dead pane retiresthe poller so the session settles instead of spinning forever.
test_retire_stops_the_file_owning_status— a readable file left behind by akilled Claude stops being read.
test_stale_busy_is_reported_as_written— an hour-oldbusystill readsrunning (a delegate/background shell is work; no freshness window).
test_hook_status_resyncs_watcher_dedup— now asserts the idempotencedirectly:
Stop's idle then the file's idle produce one edge.Pre-commit: ruff format, ruff check, prettier, oxlint, tsc all pass. Pyrefly
reports 9
missing-importerrors inbedrock.py/s3.py/hindsight.py— untouched files, optional deps absent from this worktree's.venv; present in CI.Manual verification of the fixed bugs
Second commit's coverage:
test_short_turn_poll_posts_items_without_a_status_edge— replays the exactreported sequence. Verified it FAILS without the fix (reports
external_session_statusfirst), so it genuinely guards the regression.test_forwarder_publishes_no_status_for_assistant_output— was the testasserting the deleted edge; now asserts its absence, and that items still
carry their own
response_id.test_forwarder_does_not_leave_running_open_for_slash_command_only_turnpasses unchanged — it already asserted
running_ids == set(), which nowholds for a stronger reason.
Demo
N/A — no visual change. The indicator renders identically; the fix is that it
now stops when it should.
Type of change
Test coverage
Coverage notes
The crash-while-parked recovery is covered by
test_parked_pane_stays_running_then_recovers_on_pane_death, which drives thewatcher callbacks directly. The manual
kill -9steps above were listedbecause reproducing a real
SIGKILLagainst a live tmux pane is not somethingthe unit harness does — the unit test asserts the callback ordering the real
pane-death path invokes.
Two follow-ups deliberately out of scope:
_MAX_RESOLVE_ATTEMPTS(~8s) now decides which source runs the session, soa slow boot silently degrades to PTY-only with no signal. Worth a one-time
log and possibly a longer retry.
blocked_onis still not durable across a reload (bindStreamhardcodesblockedOn: null), so "Blocked on: permission prompt" vanishes on refresheven though the runner polls it every 200ms.
Changelog
A claude-native session no longer shows "Working…" forever when Claude exits
while parked on a permission prompt, and no longer flickers idle → running →
idle at the end of a short turn.