From 8223983c80644807de66934369f7a22db98bcbcc Mon Sep 17 00:00:00 2001 From: Daniel Lok Date: Fri, 7 Aug 2026 18:08:21 +0800 Subject: [PATCH 1/6] fix(claude-native): make Claude's status file the source of truth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude's `sessions/.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 --- omnigent/claude_native_forwarder.py | 41 +++--- omnigent/claude_native_status_file.py | 58 ++++----- omnigent/runner/resource_registry.py | 60 ++++----- omnigent/server/routes/_sessions/helpers.py | 11 +- .../server/routes/sessions/routes_events.py | 19 +-- tests/runner/test_resource_registry.py | 117 ++++++++++-------- .../test_sessions_child_sessions.py | 25 ++-- tests/test_claude_native_forwarder.py | 15 +-- tests/test_claude_native_status_file.py | 66 ++++++---- web/src/store/chatStore.test.ts | 115 +++-------------- web/src/store/chatStore.ts | 69 ++++------- 11 files changed, 257 insertions(+), 339 deletions(-) diff --git a/omnigent/claude_native_forwarder.py b/omnigent/claude_native_forwarder.py index b364cb7eb1..15d464c664 100644 --- a/omnigent/claude_native_forwarder.py +++ b/omnigent/claude_native_forwarder.py @@ -250,17 +250,23 @@ def _hold_assistant_item_for_deltas( # published on the per-conversation SSE stream. Unmapped events emit # no status. # -# ``Stop`` → idle and ``StopFailure`` → failed are the authoritative -# turn-end edges (each fires once when Claude finishes / errors a turn); -# they drive sub-agent terminal delivery via the codex-shared -# ``external_session_status`` path (→ parent inbox + wake). The -# PTY-activity ``idle`` cannot: it is a ~1s-quiescence heuristic that -# oscillates on every mid-turn lull, so delivering on it fired a -# premature completion and idempotently locked out the real one. -# ``UserPromptSubmit`` → running stays PTY-derived — the pane watcher -# drives the UI running/idle badge and catches what ``Stop`` misses -# (interrupts, compaction failures, TUI edits). ``_publish_status`` -# keeps ``failed`` sticky against the trailing PTY idle. +# Claude's own ``sessions/.json`` owns the running/idle badge (see +# :mod:`omnigent.claude_native_status_file`), so these two hooks exist for +# what the file cannot express: +# +# - ``Stop`` → idle: the sub-agent terminal-delivery edge (→ parent inbox + +# wake, via the codex-shared ``external_session_status`` path). It fires +# exactly once per finished turn, where the PTY-activity ``idle`` was a +# ~1s-quiescence heuristic that oscillated on mid-turn lulls, firing a +# premature completion that idempotently locked out the real one. It also +# carries the background-shell count. It agrees with the file rather than +# competing with it, so arrival order does not matter — the shared edge +# dedup collapses the pair. +# - ``StopFailure`` → failed: the file has no failure literal (it returns to +# ``idle`` on a turn error exactly as on success), so this is the only +# source of the red pill, ``last_task_error``, and a failed scheduled run. +# ``_publish_status`` keeps it sticky against a trailing ``idle``; the +# file's next ``busy`` clears it on the following turn. _HOOK_EVENT_TO_STATUS: dict[str, str] = { "Stop": "idle", "StopFailure": "failed", @@ -3030,19 +3036,18 @@ async def _forward_available_status_events( retry_key = f"hook:{record.event_cursor}:{record.byte_offset}:{status}" if retry_tracker.retry_delay_s(retry_key) is not None: return durable - effective_status = status - if status == "idle" and record.background_task_count > 0: - effective_status = "waiting" try: await post_external_session_status( client, session_id=session_id, - status=effective_status, + status=status, response_id=response_id, - # Only the ``Stop`` (idle/waiting) edge carries an authoritative + # Only the ``Stop`` (idle) edge carries an authoritative # background-shell count — ``0`` clears the tally, ``N`` sets it. - # ``StopFailure`` (failed) clears it on the server regardless, so - # leave its count off the wire. + # This is the one thing the status file cannot report: its + # ``shell`` literal is a boolean, and the indicator renders a + # number. ``StopFailure`` (failed) clears it on the server + # regardless, so leave its count off the wire. background_task_count=( None if status == "failed" else record.background_task_count ), diff --git a/omnigent/claude_native_status_file.py b/omnigent/claude_native_status_file.py index c13df2b491..233a4d0c28 100644 --- a/omnigent/claude_native_status_file.py +++ b/omnigent/claude_native_status_file.py @@ -4,8 +4,10 @@ ``/sessions/.json`` (its internal "concurrentSessions" registry, present since v2.1.139 — the file that also backs ``claude agents``). For an interactive session it carries a ``status`` -field that flips ``idle`` ⇄ ``busy`` ⇄ ``waiting`` as the agent works, -which is a cleaner running/idle signal than diffing the tmux pane. +field that flips ``idle`` ⇄ ``busy`` ⇄ ``waiting`` as the agent works. It +reports what Claude is doing rather than inferring it from pane redraws, so +it — not the tmux pane diff — is the session's running/idle status whenever +it is readable. This module owns two pure pieces the claude-native status watcher builds on: @@ -49,9 +51,9 @@ "waiting": RUNNING, "idle": IDLE, # The turn ended but a background shell is still alive (Claude Code - # >= v2.1.197). The agent loop is idle, so this maps to ``idle`` — the - # Stop hook separately relabels its own ``idle`` to ``waiting`` with the - # shell tally, which is what keeps the spinner lit. Mapping ``shell`` to + # >= v2.1.197). The agent loop is idle, so this maps to ``idle``; the + # working indicator stays lit off the ``Stop`` hook's shell tally, which + # carries the count this boolean literal cannot. Mapping ``shell`` to # ``running`` would strand the composer on the "(queued)" placeholder, # since the session never reads idle while a background shell runs. "shell": IDLE, @@ -273,10 +275,12 @@ class SessionStatusPoller: - **Exhausted:** if resolution never succeeds, :attr:`active` stays ``False`` permanently and the file contributes nothing. - The poller never displaces the PTY watcher: it supplies an *additional* - status edge at Claude's real turn boundary, plus the freshness-bounded - :meth:`asserts_running` level the watcher consults before declaring a - quiet pane idle. + While :attr:`active` the poller *is* the session's status — it reports what + Claude is doing, where the pane diff only infers it from redraws — and the + PTY watcher publishes none. The watcher takes over when no file was ever + resolved (Claude older than v2.1.139) and always owns pane death, which the + file structurally cannot report: a killed Claude leaves its record behind + (see :meth:`retire`). :param on_status: Callback invoked as ``(runner_status, blocked_on)`` on each transition (and once on first read). Fires when either part @@ -350,35 +354,15 @@ def _try_resolve(self) -> None: if self._attempts >= _MAX_RESOLVE_ATTEMPTS: self._exhausted = True - def asserts_running(self, *, ttl_s: float, now: float | None = None) -> bool: - """Whether the file *recently* reported the session as running. - - The file is written only when its value changes, so its status is a - level that can outlive the truth — Claude keeps reporting ``busy`` - while a delegate or background task is active, long after the turn - itself ended. Callers therefore treat it as authoritative only for - *ttl_s* after the write, and fall back to the pane watcher once it - goes stale rather than pinning the session to ``running`` forever. - - :param ttl_s: How long after ``statusUpdatedAt`` the level is still - trusted, in seconds. - :param now: Wall-clock override (tests); uses :func:`time.time` - when ``None``. - :returns: ``True`` when the last read said running and is still fresh. + def retire(self) -> None: + """Stop reading the file, permanently. + + Called when the pane's process is gone: a killed Claude does not unlink + its file, so the record survives holding whatever it last said. Since + the file owns the session's status while it is readable, a dead pane + must retire it or that final value would pin the session forever. """ - status = self._last_status - if status is None or status.runner_status != RUNNING: - return False - # ``waiting`` does not decay: a dialog owns Claude's input until it - # closes, and closing it changes the value — so a new write is - # guaranteed. ``busy`` decays, because a delegate or background task - # keeps it set long after the turn it belongs to has ended. - if status.raw_status == "waiting": - return True - if status.status_updated_at is None: - return False - clock = time.time() if now is None else now - return clock - status.status_updated_at / 1000.0 <= ttl_s + self._exhausted = True @property def blocked_on(self) -> str | None: diff --git a/omnigent/runner/resource_registry.py b/omnigent/runner/resource_registry.py index 2bfbf9c31a..c1d73c2929 100644 --- a/omnigent/runner/resource_registry.py +++ b/omnigent/runner/resource_registry.py @@ -95,16 +95,6 @@ # don't 5x the capture-pane subprocess load on every terminal. _CLAUDE_NATIVE_STATUS_POLL_INTERVAL_SECONDS = 0.2 -# How long Claude's ``sessions/.json`` status stays trusted as a *level* -# after it was written. The file is rewritten only when its value changes, so -# a ``busy`` written for a delegate or background task outlives the turn that -# produced it; honouring it forever would pin the session to "Working…" with -# no way back. Inside this window a quiet pane is read as "parked on a prompt" -# (the case the pane diff genuinely cannot see) and the session stays running; -# past it the pane watcher decides. Comfortably above the 1s idle threshold so -# a real prompt is not lost to the gap between the write and the pane settling. -_CLAUDE_NATIVE_STATUS_FILE_LEVEL_TTL_SECONDS = 10.0 - # Minimum wall-clock interval (seconds) between consecutive # ``session.terminal.activity`` emissions for a single terminal. The # claude-native agent terminal polls its pane every 200ms @@ -1088,16 +1078,6 @@ def _start_terminal_activity_watcher( # means "never emitted", so the first changed tick always fires. last_activity_emit: dict[str, float | None] = {"value": None} - def _blocked_reason() -> str | None: - # The reason rides every edge, not just the poller's own, so a - # redrawing pane under a dialog doesn't publish a bare ``running`` - # that erases it. - if status_poller is None or not status_poller.asserts_running( - ttl_s=_CLAUDE_NATIVE_STATUS_FILE_LEVEL_TTL_SECONDS - ): - return None - return status_poller.blocked_on - def _publish_status(status: str, blocked_on: str | None = None) -> None: # Publish one running/idle edge: dedup against the last value, # memo for exit classification, and hop to the loop (publishers @@ -1113,6 +1093,15 @@ def _publish_status(status: str, blocked_on: str | None = None) -> None: self._set_session_status_memo(session_id, status) loop.call_soon_threadsafe(status_publisher, session_id, status, blocked_on) + def _file_owns_status() -> bool: + # Once Claude's own status file is readable it is the session's + # status: it reports what Claude is doing, where the pane diff only + # infers it from redraws. The pane keeps its activity-badge and + # pane-death jobs, but must not publish status alongside the file — + # two publishers is what made a post-turn redraw fight the file's + # ``idle`` and needed a freshness window to arbitrate. + return status_poller is not None and status_poller.active + # claude-native additionally reads Claude's own ``sessions/.json`` # status (present since Claude Code v2.1.139): it flips on the real # turn edge and knows when a dialog owns the input, neither of which @@ -1150,15 +1139,20 @@ def _on_activity() -> None: loop.call_soon_threadsafe(activity_publisher, session_id, resource_id) # Pane changed → the agent is working. Coalesce to the # idle→running edge so a continuously-redrawing pane doesn't - # re-emit ``running`` every poll. Always published, never deferred - # to the status file: that file is rewritten only when its value - # *changes*, so a turn starting while it already reads ``busy`` - # produces no write at all — and the session would sit on its - # stale ``idle`` for the whole turn with no working indicator. - if emit_status: - _publish_status("running", _blocked_reason()) + # re-emit ``running`` every poll. Skipped once the status file owns + # the session (see :func:`_file_owns_status`) — a post-turn prompt + # redraw is not a new turn, and the file already said so. + if emit_status and not _file_owns_status(): + _publish_status("running") def _on_exit() -> None: + # The pane's process is gone, which the status file cannot report — + # a killed Claude never unlinks it, so the record survives holding + # its last value. Retire the poller before classifying the exit so + # that stale value can't keep owning the session's status. + if status_poller is not None: + status_poller.retire() + def _schedule() -> None: task = asyncio.create_task( self._handle_terminal_exit( @@ -1210,16 +1204,12 @@ def _on_idle() -> None: # Pane quiet for the claude-native status threshold → the # agent has stopped. Edge-triggered: re-arms only after new # output mutates the pane (which flips back to ``running``). - # Held back only while the status file *freshly* reports running: - # a dialog owning the input quiets the pane without ending the - # turn, which the pane diff alone cannot tell from a finished one. - # Past that freshness window the pane decides, so a ``busy`` left - # standing by a background task can't pin the session to running. + # Skipped once the status file owns the session: a dialog owning + # the input quiets the pane without ending the turn, and only the + # file can tell that from a finished one. # Edge ordering: the watcher thread runs idle/exit serially, so # this idle commits before any later on_exit reads the memo. - if status_poller is None or not status_poller.asserts_running( - ttl_s=_CLAUDE_NATIVE_STATUS_FILE_LEVEL_TTL_SECONDS - ): + if not _file_owns_status(): _publish_status("idle") # Clear the activity throttle so the next working episode emits # its first pulse immediately, keeping the activity badge diff --git a/omnigent/server/routes/_sessions/helpers.py b/omnigent/server/routes/_sessions/helpers.py index a1907473c1..24fb087a14 100644 --- a/omnigent/server/routes/_sessions/helpers.py +++ b/omnigent/server/routes/_sessions/helpers.py @@ -6455,9 +6455,12 @@ async def _run_compact_locked( code=ErrorCode.INVALID_INPUT, ) task_id = f"compact_{int(time.time() * 1000)}" - _publish_status(session_id, "running") - # compact() publishes its own in_progress / completed SSE events - # when conversation_id is set — don't double-publish here. + # compact() publishes its own in_progress / completed SSE events when + # conversation_id is set, and the web UI's compaction bubble owns the + # busy state from those. Deliberately no ``session.status`` bracket: + # compaction is not an agent turn, so reporting running→idle would + # invent one — and its idle would land mid-turn on a session that is + # genuinely working, which clients then had to second-guess. from omnigent.runtime.workflow import compact_conversation_now try: @@ -6473,12 +6476,10 @@ async def _run_compact_locked( _logger.exception("Explicit session compaction failed for %s", session_id) detail = str(exc) or repr(exc) _publish_compaction_failed(session_id) - _publish_status(session_id, "idle") raise OmnigentError( f"Compaction failed while generating a summary: {detail}", code=ErrorCode.INTERNAL_ERROR, ) from exc - _publish_status(session_id, "idle") def _agent_provider_family(agent: Agent) -> str | None: diff --git a/omnigent/server/routes/sessions/routes_events.py b/omnigent/server/routes/sessions/routes_events.py index a3507f9b6e..b359f14fac 100644 --- a/omnigent/server/routes/sessions/routes_events.py +++ b/omnigent/server/routes/sessions/routes_events.py @@ -473,7 +473,6 @@ async def post_event( # deny sentinel on the session stream so the # client/REPL sees feedback. reason = _input_verdict.get("reason", "Denied by policy") - _publish_status(session_id, "running") _publish_policy_deny(session_id, reason) await _persist_policy_deny_sentinel( session_id, @@ -482,10 +481,13 @@ async def post_event( conversation_store, agent_store, ) - # Terminal response.completed before idle so live-tail - # consumers (the headless ``-p`` client) unblock. + # Terminal ``response.completed`` is what unblocks live-tail + # consumers (the headless ``-p`` client) and settles the web + # bubble. No ``session.status`` pair rides along: the agent + # never ran, so publishing running→idle would report a turn + # that did not happen — and a client seeing that phantom idle + # mid-turn had to second-guess it back to running. _publish_input_deny_terminal(session_id, conv, reason) - _publish_status(session_id, "idle") # Return the same shape the client expects from POST # /events so postEvent doesn't throw on an unexpected # response body. queued=False signals the event was @@ -505,7 +507,6 @@ async def post_event( ) if _input_verdict is not None: reason = _input_verdict.get("reason", "Denied by policy") - _publish_status(session_id, "running") _publish_policy_deny(session_id, reason) await _persist_policy_deny_sentinel( session_id, @@ -514,9 +515,9 @@ async def post_event( conversation_store, agent_store, ) - # Terminal response.completed before idle (see message branch). + # Terminal response.completed, no status pair (see the message + # branch above). _publish_input_deny_terminal(session_id, conv, reason) - _publish_status(session_id, "idle") return {"queued": False, "denied": True, "reason": reason} elif ( body.type == "message" @@ -883,7 +884,9 @@ async def post_event( # A background-task ``waiting`` marks an ended turn, so deliver it # as ``idle``: the session takes a new message now, and for a # sub-agent the terminal-delivery branch below must fire (otherwise - # the orchestrator hangs). The tally still drives the spinner. + # the orchestrator hangs). The tally still drives the indicator. + # The claude-native forwarder no longer sends ``waiting`` at all — + # this normalizes it for runners that predate that change. effective_status = _background_task_delivery_status(status, bg_count, conv) if effective_status != status: status = effective_status diff --git a/tests/runner/test_resource_registry.py b/tests/runner/test_resource_registry.py index 8739785034..d1ba0f9b3b 100644 --- a/tests/runner/test_resource_registry.py +++ b/tests/runner/test_resource_registry.py @@ -402,25 +402,25 @@ def _capture_watcher( class _FakeStatusPoller: """Controllable stand-in for the claude-native status-file poller. - Lets a test flip :attr:`active` (file resolved vs. falling back), flip - :attr:`running_level` (the file freshly reports running vs. its value - having gone stale), and fire status edges through the registry's - callback, without touching a real ``sessions/.json``. + Lets a test flip :attr:`active` (file resolved and therefore owning the + session's status, vs. the PTY watcher falling back) and fire status edges + through the registry's callback, without touching a real + ``sessions/.json``. """ def __init__(self, on_status: object) -> None: self._on_status = on_status self.active = False - self.running_level = False self.blocked_on: str | None = None self.ticks = 0 + self.retired = False def tick(self) -> None: self.ticks += 1 - def asserts_running(self, *, ttl_s: float, now: float | None = None) -> bool: - del ttl_s, now - return self.running_level + def retire(self) -> None: + self.retired = True + self.active = False def emit(self, status: str, blocked_on: str | None = None) -> None: """Simulate the file reporting a new status.""" @@ -498,53 +498,60 @@ async def test_claude_native_wires_status_poller_tick(tmp_path: Path) -> None: @pytest.mark.asyncio -async def test_pty_activity_publishes_running_even_with_active_status_file( - tmp_path: Path, -) -> None: - """Pane activity publishes ``running`` even while the file poller is active. - - Claude rewrites ``sessions/.json`` only when its value *changes*, so a - turn that starts while the file already reads ``busy`` produces no write at - all. If the file were allowed to mute the pane watcher, nothing could - publish ``running`` and the session would sit on a stale ``idle`` — no - working indicator, no stop button — for the whole turn. +async def test_pane_publishes_no_status_while_the_file_owns_it(tmp_path: Path) -> None: + """An active file poller is the only status source; the pane publishes none. + + Claude redraws its prompt after a turn and blinks a cursor, so the pane + keeps changing once the file has already said ``idle``. Letting both + publish is what made that redraw contradict the file and needed a + freshness window to arbitrate — so while the file is readable it decides, + and the pane's edges are dropped. """ callbacks, statuses, pollers, _registry = await _observe_native_with_fake_poller( - tmp_path, "conv_pre" + tmp_path, "conv_file_owns" ) poller = pollers[0] poller.active = True - # The file already holds ``busy`` from an earlier turn, so it emits nothing. - callbacks["on_activity"]() - - # Status edges publish via loop.call_soon_threadsafe; let them drain. + poller.emit("running") + callbacks["on_activity"]() # pane redraws mid-turn — no second edge await asyncio.sleep(0) assert statuses == ["running"] + poller.emit("idle") + callbacks["on_activity"]() # post-turn prompt redraw is not a new turn + callbacks["on_idle"]() # nor does a quiet pane re-assert idle + await asyncio.sleep(0) + assert statuses == ["running", "idle"] -@pytest.mark.asyncio -async def test_pty_idle_deferred_only_while_file_freshly_running(tmp_path: Path) -> None: - """A quiet pane goes idle unless the file *freshly* reports running. - A dialog owning Claude's input quiets the pane without ending the turn, so - a fresh ``running`` level holds the idle edge back. Once that level goes - stale — a ``busy`` left standing by a background task outlives its turn — - the pane decides again, so the session can never be pinned to ``running``. +@pytest.mark.asyncio +async def test_parked_pane_stays_running_then_recovers_on_pane_death(tmp_path: Path) -> None: + """A dialog keeps the session running; a dead pane still ends it. + + While Claude is parked on a prompt the pane is quiet but the turn is not + over, and only the file knows that — so the quiet pane must not publish + ``idle``. But a killed Claude leaves that ``waiting`` record behind, so + pane death retires the poller and the PTY side owns the outcome. Without + that, the session would spin forever. """ callbacks, statuses, pollers, _registry = await _observe_native_with_fake_poller( - tmp_path, "conv_idle_gate" + tmp_path, "conv_parked" ) poller = pollers[0] poller.active = True - poller.running_level = True - callbacks["on_activity"]() # → running - callbacks["on_idle"]() # suppressed: file freshly says running + poller.emit("running", "permission prompt") + callbacks["on_idle"]() # pane quiet under the dialog — turn is NOT over await asyncio.sleep(0) assert statuses == ["running"] - poller.running_level = False # the file's level went stale + # Claude is killed at the prompt. Its file survives holding ``waiting``. + callbacks["on_exit"]() + assert poller.retired is True + assert poller.active is False + + # The pane now owns status again, so the session can settle. callbacks["on_idle"]() await asyncio.sleep(0) assert statuses == ["running", "idle"] @@ -552,29 +559,37 @@ async def test_pty_idle_deferred_only_while_file_freshly_running(tmp_path: Path) @pytest.mark.asyncio async def test_hook_status_resyncs_watcher_dedup(tmp_path: Path) -> None: - """A forwarder's hook-derived edge rebases the watcher's dedup. + """A forwarder's hook-derived edge rebases the shared dedup baseline. ``Stop`` → ``idle`` is posted to the server by the claude-native forwarder, - bypassing this watcher. Without adopting it as the baseline the watcher - still believes ``running`` is live and swallows the next turn's genuine - ``running`` as a duplicate, leaving the session stuck on the hook's idle. + bypassing this watcher. Adopting it as the baseline is what makes the pair + idempotent: the file's own ``idle`` lands on the same edge and is collapsed, + so the two agree regardless of which arrives first. """ callbacks, statuses, pollers, registry = await _observe_native_with_fake_poller( tmp_path, "conv_resync" ) - pollers[0].active = True + poller = pollers[0] + poller.active = True - callbacks["on_activity"]() + poller.emit("running") await asyncio.sleep(0) assert statuses == ["running"] # The forwarder posts Stop → idle straight to the server. registry.note_external_session_status("conv_resync", "idle") - # Next turn: the pane moves again and must re-publish ``running``. - callbacks["on_activity"]() + # The file catches up moments later with the same edge — deduped away, so + # the user sees one idle rather than a flicker. + poller.emit("idle") + await asyncio.sleep(0) + assert statuses == ["running"] + + # Next turn: the file reports work again and must publish. + poller.emit("running") await asyncio.sleep(0) assert statuses == ["running", "running"] + del callbacks @pytest.mark.asyncio @@ -1284,13 +1299,13 @@ def test_resolve_environment_runner_workspace_overrides_absolute_spec_cwd( @pytest.mark.asyncio -async def test_blocked_reason_rides_pane_edges(tmp_path: Path) -> None: - """The parked reason travels with every edge, not just the file's own. +async def test_blocked_reason_survives_pane_redraws(tmp_path: Path) -> None: + """The parked reason survives the pane redrawing underneath the dialog. - Claude reports ``waitingFor`` once, when the dialog opens. The pane keeps - redrawing underneath it, so if a pane-derived ``running`` shipped without - the reason it would immediately erase what the file just said and the UI - would fall back to a bare spinner. + Claude reports ``waitingFor`` once, when the dialog opens, and the pane + keeps redrawing while it is up. Because the file owns status outright the + pane publishes nothing, so there is no bare ``running`` to erase the reason + — it stands until the file itself drops it. """ terminal_registry = TerminalRegistry() registry = SessionResourceRegistry(terminal_registry=terminal_registry) @@ -1335,15 +1350,15 @@ def _capture_watcher( poller = pollers[0] poller.active = True - poller.running_level = True poller.blocked_on = "permission prompt" poller.emit("running", "permission prompt") callbacks["on_activity"]() # pane redraw under the dialog + callbacks["on_idle"]() # and the quiet spells between redraws await asyncio.sleep(0) assert edges == [("running", "permission prompt")] - # Dialog answered: the file drops the reason and the pane keeps moving. + # Dialog answered: the file drops the reason on its own edge. poller.blocked_on = None poller.emit("running", None) await asyncio.sleep(0) diff --git a/tests/server/integration/test_sessions_child_sessions.py b/tests/server/integration/test_sessions_child_sessions.py index 8dcc39fea5..da12c48e39 100644 --- a/tests/server/integration/test_sessions_child_sessions.py +++ b/tests/server/integration/test_sessions_child_sessions.py @@ -1753,19 +1753,18 @@ async def _recover_spy(child_conv: Any, *_args: Any, **_kwargs: Any) -> Any: assert recovered_for == [child["id"]] -async def test_subagent_background_task_waiting_delivers_to_parent_as_idle( +async def test_subagent_background_task_count_still_delivers_to_parent( client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch, ) -> None: - """A sub-agent's background-task ``waiting`` still delivers terminal status. + """A lingering background shell must not strand the parent orchestrator. - Regression for the parent-orchestrator hang: a claude-native sub-agent - relabels its ``Stop`` turn-end ``idle`` to ``waiting`` when a background - shell lingers. The terminal-delivery branch only fires for - ``idle``/``failed``, so an un-collapsed ``waiting`` would skip delivery and - the parent would wait forever. The server must collapse the sub-agent's - background-task ``waiting`` to ``idle`` so delivery (here, the recovery - path) still runs for the child. + Regression for the parent-orchestrator hang. The ``Stop`` turn-end edge + carries the background-shell count, and the terminal-delivery branch fires + only for ``idle``/``failed`` — so the edge has to stay ``idle`` and let the + count ride alongside. (It used to be relabeled to ``waiting`` for the + spinner's sake, which skipped delivery and made the parent wait forever; + the spinner now stays lit off the count instead.) """ child = await _create_native_child(client, name="orch-bg-waiting") @@ -1788,13 +1787,13 @@ async def _recover_spy(child_conv: Any, *_args: Any, **_kwargs: Any) -> Any: f"/v1/sessions/{child['id']}/events", json={ "type": "external_session_status", - "data": {"status": "waiting", "background_task_count": 1}, + "data": {"status": "idle", "background_task_count": 1}, }, ) - # Delivery fired despite the incoming `waiting`: the collapse to `idle` - # let the terminal-status branch run for THIS child (recovery invoked, - # 202 Accepted) instead of silently skipping and stranding the parent. + # A positive count does not suppress delivery: the terminal-status branch + # ran for THIS child (recovery invoked, 202 Accepted) rather than silently + # skipping and stranding the parent. assert resp.status_code == 202, resp.text assert recovered_for == [child["id"]] diff --git a/tests/test_claude_native_forwarder.py b/tests/test_claude_native_forwarder.py index cd409e3a07..4739fc6bde 100644 --- a/tests/test_claude_native_forwarder.py +++ b/tests/test_claude_native_forwarder.py @@ -7698,16 +7698,17 @@ def handler(request: httpx.Request) -> httpx.Response: @pytest.mark.asyncio -async def test_forwarder_posts_waiting_when_stop_has_background_tasks( +async def test_forwarder_posts_idle_with_count_when_stop_has_background_tasks( tmp_path: Path, ) -> None: """ - ``Stop`` with ``background_tasks`` → ``waiting`` instead of ``idle``. + ``Stop`` with ``background_tasks`` posts ``idle`` plus the shell count. - When Claude Code's Stop hook carries a non-empty ``background_tasks`` - array (shells still running), the forwarder must publish ``waiting`` - so the web UI keeps showing the spinner. Without this, the chat - interface shows "idle" while the terminal shows "1 shell running". + The turn really has ended, so the status is ``idle`` — the spinner stays + lit off the count instead (``showsWorking`` is ``isWorking || tally > 0``). + The count is the one thing Claude's status file cannot report: its + ``shell`` literal is a boolean and the indicator renders a number, which + is why this hook still posts at all. """ bridge_dir = tmp_path / "bridge" transcript_path = tmp_path / "session.jsonl" @@ -7761,7 +7762,7 @@ async def test_forwarder_posts_waiting_when_stop_has_background_tasks( assert request["path"] == "/v1/sessions/conv_abc/events" assert request["body"] == { "type": "external_session_status", - "data": {"status": "waiting", "background_task_count": 1}, + "data": {"status": "idle", "background_task_count": 1}, } diff --git a/tests/test_claude_native_status_file.py b/tests/test_claude_native_status_file.py index dc81afc44c..5699f61f57 100644 --- a/tests/test_claude_native_status_file.py +++ b/tests/test_claude_native_status_file.py @@ -266,13 +266,13 @@ def test_unknown_status_clears_dedup_so_next_read_publishes(tmp_path: Path) -> N assert published == [RUNNING, RUNNING] -def test_asserts_running_only_while_fresh(tmp_path: Path) -> None: - """The running level is trusted only briefly after it was written. +def test_stale_busy_is_reported_as_written(tmp_path: Path) -> None: + """A long-standing ``busy`` still reads as running — no freshness window. - Claude keeps reporting ``busy`` while a delegate or background task is - active, long after the turn ended. Treating that as authoritative forever - would pin the session to "Working…"; past the window the pane watcher - decides again. + Claude reports ``busy`` while a delegate or background shell works, which + can outlast the turn that started it, and that is correct: it *is* still + doing work. The file says what Claude is doing, so we report it as written + rather than timing it out and second-guessing with a pane diff. """ sessions = tmp_path / "sessions" now = 1785480100.0 @@ -281,7 +281,7 @@ def test_asserts_running_only_while_fresh(tmp_path: Path) -> None: pid=1, session_id="s", status="busy", - status_updated_at=int((now - 2) * 1000), + status_updated_at=int((now - 3600) * 1000), ) published: list[str] = [] poller = SessionStatusPoller( @@ -291,19 +291,42 @@ def test_asserts_running_only_while_fresh(tmp_path: Path) -> None: config_dir=tmp_path, ) poller.tick() - assert poller.asserts_running(ttl_s=10.0, now=now) is True - assert poller.asserts_running(ttl_s=1.0, now=now) is False + assert published == [RUNNING] + assert poller.active is True + - # An idle level never asserts running, however fresh. +def test_retire_stops_the_file_owning_status(tmp_path: Path) -> None: + """A dead pane retires the poller, even with a readable file left behind. + + Claude does not unlink its status file when killed, so the record survives + holding whatever it last said — here a ``waiting`` that would otherwise + keep asserting the session is parked mid-turn forever. Pane death is the + one thing the file cannot report, so the watcher retires the poller and + the PTY side owns the outcome. + """ + sessions = tmp_path / "sessions" _write_session_file( - sessions, - pid=1, - session_id="s", - status="idle", - status_updated_at=int(now * 1000), + sessions, pid=1, session_id="s", status="waiting", blocked_on="input needed" + ) + published: list[tuple[str, str | None]] = [] + poller = SessionStatusPoller( + on_status=lambda status, reason: published.append((status, reason)), + pane_pid_getter=_StubPidGetter(1), + session_id_getter=lambda: "s", + config_dir=tmp_path, ) poller.tick() - assert poller.asserts_running(ttl_s=10.0, now=now) is False + assert published == [(RUNNING, "input needed")] + assert poller.active is True + + poller.retire() + assert poller.active is False + + # The file is still there and still readable; a retired poller reads it no + # more, so its stale value cannot keep owning the session's status. + _write_session_file(sessions, pid=1, session_id="s", status="busy") + poller.tick() + assert published == [(RUNNING, "input needed")] def test_waiting_carries_its_reason(tmp_path: Path) -> None: @@ -351,12 +374,12 @@ def test_poller_publishes_when_only_the_reason_changes(tmp_path: Path) -> None: assert poller.blocked_on == "dialog open" -def test_waiting_level_does_not_decay(tmp_path: Path) -> None: +def test_parked_session_stays_running_indefinitely(tmp_path: Path) -> None: """A dialog holds the session open however long it stays up. - Unlike ``busy`` — which a background task keeps set past its turn — a - ``waiting`` clears only when Claude writes a new status, so it is safe to - trust indefinitely and wrong to time out (the pane is quiet the whole time). + ``waiting`` clears only when Claude writes a new status, so an hour-old + parked record still reports running — the pane is quiet the whole time and + only the file can tell that from a finished turn. """ sessions = tmp_path / "sessions" now = 1785480100.0 @@ -376,4 +399,5 @@ def test_waiting_level_does_not_decay(tmp_path: Path) -> None: config_dir=tmp_path, ) poller.tick() - assert poller.asserts_running(ttl_s=10.0, now=now) is True + assert published == [(RUNNING, "input needed")] + assert poller.blocked_on == "input needed" diff --git a/web/src/store/chatStore.test.ts b/web/src/store/chatStore.test.ts index 4894a58acb..c40aa5b758 100644 --- a/web/src/store/chatStore.test.ts +++ b/web/src/store/chatStore.test.ts @@ -51,7 +51,6 @@ import { consumePendingInitialPrompt, handleSessionEvent, isStaleCompletedResponse, - reviveStrayCompletedResponse, initChatStore, pumpStreamEvents, setPendingInitialPrompt, @@ -2303,45 +2302,16 @@ describe("chatStore — send while streaming (queueing)", () => { error: null, }); - // The server's deny short-circuit still publishes a session-level - // running→idle pair for the denied out-of-band input, and the client - // trusts `session.status` 1:1 — so this stray idle flips - // `sessionStatus` AND finalizes the streaming turn (a bare terminal - // edge is the NORMAL turn-end shape for id-less emitters like the - // PTY-activity relay, so it must settle the bubble; see the bare-idle - // tests below). The deny corner is healed by the live-delta revive: - // the still-streaming turn's next delta reopens it - // (reviveStrayCompletedResponse), so the misread is a brief flicker, - // not a mid-turn fold. - handleSessionEvent({ - type: "session_status", - conversationId: "conv_abc", - status: "idle", - }); - const afterIdle = useChatStore.getState(); - expect(afterIdle.sessionStatus).toBe("idle"); - expect(afterIdle.status).toBe("idle"); - expect(afterIdle.activeResponse).toEqual({ - responseId: "resp_in_flight", - state: "completed", - error: null, - completedAt: expect.any(Number), - }); - - // The turn was actually still live — its next delta revives it, and - // the session's busy signal comes back with it: leaving - // sessionStatus "idle" let a mid-turn send bypass shouldQueueSend's - // queue gate. Local `status` stays "idle" — this client sent - // nothing, so no local send is in flight. - reviveStrayCompletedResponse(useChatStore.setState); - expect(useChatStore.getState().activeResponse).toEqual({ + // 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), }); - expect(useChatStore.getState().sessionStatus).toBe("running"); - expect(useChatStore.getState().status).toBe("idle"); }); it("finalizes a streaming turn on a bare idle edge (no response id)", () => { @@ -2386,7 +2356,7 @@ describe("chatStore — send while streaming (queueing)", () => { it("preserves a cancelled turn across a bare idle edge", () => { // The user's interrupt verdict must survive the trailing idle the - // teardown publishes — and the revive must never resurrect it. + // teardown publishes. useChatStore.setState({ conversationId: "conv_abc", status: "idle", @@ -2399,8 +2369,6 @@ describe("chatStore — send while streaming (queueing)", () => { status: "idle", }); expect(useChatStore.getState().activeResponse?.state).toBe("cancelled"); - reviveStrayCompletedResponse(useChatStore.setState); - expect(useChatStore.getState().activeResponse?.state).toBe("cancelled"); }); it("surfaces a failed send without settling the live turn", async () => { @@ -2443,54 +2411,6 @@ describe("chatStore — send while streaming (queueing)", () => { expect(state.status).toBe("streaming"); }); - it("revive is a no-op for failed and absent responses", () => { - useChatStore.setState({ - conversationId: "conv_abc", - sessionStatus: "idle", - activeResponse: { responseId: "resp_a", state: "failed", error: "boom" }, - }); - reviveStrayCompletedResponse(useChatStore.setState); - expect(useChatStore.getState().activeResponse?.state).toBe("failed"); - expect(useChatStore.getState().sessionStatus).toBe("idle"); - - useChatStore.setState({ activeResponse: null }); - reviveStrayCompletedResponse(useChatStore.setState); - expect(useChatStore.getState().activeResponse).toBeNull(); - expect(useChatStore.getState().sessionStatus).toBe("idle"); - }); - - it("gates the revive to a window after the finalize", () => { - // A scheduled wake's first deltas stream ahead of the batch that - // names the new turn; reviving the minutes-old finished turn - // popped its "Worked for" fold open at every /loop iteration. A - // finalize moments ago is a plausible stray idle and still revives. - useChatStore.setState({ - conversationId: "conv_abc", - sessionStatus: "idle", - activeResponse: { - responseId: "resp_prev_iter", - state: "completed", - error: null, - completedAt: Date.now() - 60_000, - }, - }); - reviveStrayCompletedResponse(useChatStore.setState); - expect(useChatStore.getState().activeResponse?.state).toBe("completed"); - expect(useChatStore.getState().sessionStatus).toBe("idle"); - - useChatStore.setState({ - activeResponse: { - responseId: "resp_live", - state: "completed", - error: null, - completedAt: Date.now() - 1_000, - }, - }); - reviveStrayCompletedResponse(useChatStore.setState); - expect(useChatStore.getState().activeResponse?.state).toBe("streaming"); - expect(useChatStore.getState().sessionStatus).toBe("running"); - }); - it("isStaleCompletedResponse: only an old finalize is stale", () => { const base = { responseId: "r", error: null } as const; expect( @@ -3351,12 +3271,11 @@ describe("chatStore — handleSessionEvent (session.* events)", () => { // sessionStatus tracks the server's session-level status 1:1 — a server // idle means idle, and the "Working…" indicator (which reads only // sessionStatus) must turn off. The bubble lifecycle settles on the - // same edge: a bare (id-less) idle is the NORMAL turn-end shape for - // the PTY-activity relay and orchestration teardown, and leaving the - // turn "streaming" hid its "Worked for" fold and Fork action until a - // reload. A stray idle for a turn that is actually still live (the - // policy-deny short-circuit) is healed by the live-delta revive — - // see reviveStrayCompletedResponse. + // same edge: a bare (id-less) idle is the NORMAL turn-end shape for the + // status file and orchestration teardown, and leaving the turn + // "streaming" hid its "Worked for" fold and Fork action until a reload. + // Every idle that reaches here is a real turn end — control signals no + // longer publish status. expect(state.sessionStatus).toBe("idle"); expect(state.status).toBe("idle"); expect(state.activeResponse).toEqual({ @@ -6931,7 +6850,10 @@ describe("chatStore — startStreamPump reconnect loop", () => { await loop; expect(opens).toBe(11); // 1 initial open + 10 retries, then gives up - expect(useChatStore.getState().sessionStatus).toBe("failed"); + // Giving up on OUR stream says nothing about what the agent is doing, so + // the session's own status is left alone — only the server may declare a + // session failed. The dropped stream surfaces as offline liveness. + expect(useChatStore.getState().sessionStatus).toBe("running"); expect(useChatStore.getState().abortController).toBeNull(); }); @@ -7633,10 +7555,7 @@ describe("chatStore — live delta streaming (claude-native)", () => { it("keeps the synthetic response id when no turn is tracked", async () => { // A preview that lands before any turn id is known keeps its own id, so - // it can't be grouped into an unrelated bubble. (A `completed` turn does - // NOT hit this path: a live delta proves that turn is still running, so - // `reviveStrayCompletedResponse` reopens it first and the preview - // correctly joins it.) + // it can't be grouped into an unrelated bubble. useChatStore.setState({ conversationId: "conv_live_norid", blocks: [], diff --git a/web/src/store/chatStore.ts b/web/src/store/chatStore.ts index 74bcdbd74b..302106cc28 100644 --- a/web/src/store/chatStore.ts +++ b/web/src/store/chatStore.ts @@ -3330,12 +3330,16 @@ export async function startStreamPump( // Release the unconsumed error-response body so the underlying fetch // connection is freed promptly rather than lingering across retries. void streamRes.body?.cancel().catch(() => {}); - // 401/403 won't fix themselves by retrying — give up and mark the - // session failed so the user isn't left on a silent spinner. + // 401/403 won't fix themselves by retrying — give up and settle the + // local send lifecycle so the user isn't left on a silent spinner. + // `sessionStatus` is NOT touched: losing our stream says nothing + // about what the agent is doing (it may well still be mid-turn), and + // only the server may declare a session failed. The dropped stream + // surfaces as offline liveness via ConnectionIndicator. if (streamRes.status === 401 || streamRes.status === 403) { console.warn(`Session ${id}: stream unavailable (${streamRes.status}), giving up`); finalizeActive(set, "failed", `stream unavailable (${streamRes.status})`, null); - set({ sessionStatus: "failed", status: "idle" }); + set({ status: "idle" }); break; } // A reverse proxy routinely serves 404 for the stream route while @@ -3348,8 +3352,10 @@ export async function startStreamPump( console.warn( `Session ${id}: stream unavailable (404) after ${consecutive404s} attempts, giving up`, ); + // Local lifecycle only — see the 401/403 branch above for why + // `sessionStatus` is left to the server. finalizeActive(set, "failed", "stream unavailable (404)", null); - set({ sessionStatus: "failed", status: "idle" }); + set({ status: "idle" }); break; } console.warn( @@ -3662,24 +3668,16 @@ async function* tapLiveDeltas( retired.add(ev.messageId); continue; } - reviveStrayCompletedResponse(set); applyLiveDelta(set, ev.messageId, ev.index ?? 0, ev.delta, lastIndex); } continue; } if (ev.type === "tool_output_delta") { if (get().conversationId === id && !isStaleCompletedResponse(get())) { - reviveStrayCompletedResponse(set); applyLiveToolOutputDelta(set, ev.callId, ev.delta); } continue; } - if ( - (ev.type === "text_delta" || ev.type === "reasoning_delta") && - get().conversationId === id - ) { - reviveStrayCompletedResponse(set); - } yield ev; } } @@ -3724,12 +3722,11 @@ export function adoptTrailingUnattributedBlocks( return next; } -// How long after a terminal edge a delta still revives the turn. A -// STRAY mid-turn idle is contradicted by the still-flowing stream -// within seconds; a scheduled wake (cron / wakeup fires at 60s -// minimum) streams its FIRST deltas ahead of the transcript batch that -// names the new turn — reviving the finished turn then popped its -// "Worked for" fold open at the start of every /loop iteration. +// 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; /** @@ -3744,23 +3741,6 @@ export function isStaleCompletedResponse(s: { activeResponse: ActiveResponse | n ); } -export function reviveStrayCompletedResponse(set: Setter): void { - set((s) => { - if (s.activeResponse?.state !== "completed") return {}; - if (isStaleCompletedResponse(s)) return {}; - // The delta also proves the SESSION is mid-turn: restore the busy - // signal the stray idle edge cleared, so send gating - // (shouldQueueSend) queues instead of firing into the live turn and - // the Working indicator comes back before the next running edge. - // Local `status` stays untouched — it means "this client's send is - // in flight", which is false for cross-client and TUI-typed turns. - return { - activeResponse: { ...s.activeResponse, state: "streaming" }, - sessionStatus: "running", - }; - }); -} - /** * Flip an auto-resolved ApprovalCard back to answerable, in place. * @@ -4600,17 +4580,14 @@ export function handleSessionEvent(event: StreamEvent): void { } } else { // Terminal edge without a matching response id. This is the - // NORMAL turn-end shape for most emitters — the PTY-activity - // relay's bare `idle`, orchestration teardown, and mismatched - // Stop-hook `waiting` all carry none — so a still-streaming - // turn is finalized here rather than left "streaming" forever - // (which hid the settled turn's "Worked for" fold and Fork - // action until a reload re-derived lifecycle from the - // snapshot). The one edge this can misread — the server's - // policy-deny short-circuit publishing a stray running→idle - // pair while a real turn streams — is healed by - // `reviveStrayCompletedResponse`: the live turn's next delta - // reopens it. A `cancelled` turn is preserved as-is. + // NORMAL turn-end shape for most emitters — the status file's + // bare `idle` and orchestration teardown carry none — so a + // still-streaming turn is finalized here rather than left + // "streaming" forever (which hid the settled turn's "Worked for" + // fold and Fork action until a reload re-derived lifecycle from + // the snapshot). Every terminal edge that reaches this point is + // now a real turn end: control signals (policy deny, compaction) + // no longer publish status. A `cancelled` turn is preserved as-is. patch.status = "idle"; if (s.activeResponse?.state === "streaming") { patch.activeResponse = { From 1c2debe37a4d595c043a87819685ce8186aba82a Mon Sep 17 00:00:00 2001 From: Daniel Lok Date: Fri, 7 Aug 2026 22:21:08 +0800 Subject: [PATCH 2/6] fix(claude-native): stop the transcript forwarder publishing session status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4344 made Claude's `sessions/.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 --- omnigent/claude_native_forwarder.py | 89 ++------------ tests/test_claude_native_forwarder.py | 164 ++++++++++++++++++-------- 2 files changed, 122 insertions(+), 131 deletions(-) diff --git a/omnigent/claude_native_forwarder.py b/omnigent/claude_native_forwarder.py index 15d464c664..79fdad2d8a 100644 --- a/omnigent/claude_native_forwarder.py +++ b/omnigent/claude_native_forwarder.py @@ -663,12 +663,6 @@ class _ForwardDedupeState: # sub-agent spend so the gate can block mid-turn. Separate baseline # because it can advance while ``posted_cost`` (S) is frozen. posted_policy_cost: float | None = None - # Response id of the last turn-start ``running`` status POSTed, so the - # id-bearing running edge fires exactly once per turn even when an - # assistant item is held across polls for delta ordering (which leaves - # ``state.current_response_id`` unadvanced). ``None`` until the first - # turn-start edge. Reset on /clear and /fork like the other baselines. - posted_running_response_id: str | None = None # Turn-settle latch driving the scheduled-wake boundary. The Stop edge # records the ended turn's id as PENDING; it activates (moves to # ``settled_response_id``) only once a fully-consumed transcript batch @@ -3182,31 +3176,6 @@ async def _ensure_state_for_transcript( return state -def _turn_has_assistant_output(items: list[ClaudeTranscriptItem], response_id: str) -> bool: - """ - Whether ``response_id`` has assistant-generated output among ``items``. - - The turn-start ``running`` edge should open a streaming turn only for an id - that a later ``Stop``/``StopFailure`` hook will close — i.e. one produced by - an actual LLM turn. Assistant text (``message`` with ``role=assistant``) and - tool calls (``function_call``) qualify; a ``slash_command`` (``/model``, - ``/effort``) or ``terminal_command`` (``!cmd``) item opens an id with no LLM - turn behind it, so it must not. - - :param items: Transcript items read this poll. - :param response_id: The current turn's response id. - :returns: ``True`` when an assistant-output item carries ``response_id``. - """ - for item in items: - if item.response_id != response_id: - continue - if item.item_type == "function_call": - return True - if item.item_type == "message" and item.data.get("role") == "assistant": - return True - return False - - def _promote_pending_settle( dedupe: _ForwardDedupeState, items: list[ClaudeTranscriptItem] ) -> bool: @@ -3465,55 +3434,15 @@ async def _forward_available_items( current_response_id = result.current_response_id seen_source_ids = list(state.seen_source_ids) seen = set(seen_source_ids) - # NOTE: the old "re-assert running on resumed agent output" hack lived - # here. It only existed to paper over the hook model's compaction - # blind spot (``Stop`` → idle, then an ``isCompactSummary`` resume that - # never fired ``UserPromptSubmit``). PTY-activity status makes it - # obsolete: the pane keeps changing through a mid-turn compaction, so - # the runner's watcher holds the session ``running`` directly. - # - # Turn-start edge: the first time we see a turn's response id, publish a - # ``running`` status carrying it. The PTY watcher already drives the - # running/idle BADGE with a bare (id-less) status; this id-bearing edge is - # what lets ap-web open a *streaming* ``activeResponse`` for the turn, so - # the forwarded tool-call cards (which carry the same response id) render - # LIVE — spinner + elapsed timer — instead of as static completed cards. - # Deduped on the persistent ``dedupe`` baseline (NOT ``state``): when an - # assistant item is held across polls for delta ordering, this function - # early-returns with ``state`` unadvanced, so a ``state``-based guard would - # re-fire ``running`` every poll of the hold window. Best-effort — a failed - # status post must not abort item forwarding (the items below are the - # primary payload); the turn-end idle/failed edge still carries the id to - # close the lifecycle, and the badge is unaffected either way. - # - # Only open the streaming turn for an id that has ASSISTANT output in this - # poll's items. A surfaced CLI built-in (``/model``, ``/effort``) or a - # ``!cmd`` becomes a slash_command / terminal_command item that opens its - # own response id but runs no LLM turn, so no ``Stop`` hook ever fires to - # close it — a ``running`` opened for it would strand the web composer in - # its "Stop"/busy state until the next real message. A skill that DOES - # trigger an LLM turn shares its id with the assistant text it produces, so - # ``running`` still fires — one poll later, when that output appears. - if ( - current_response_id is not None - and dedupe.posted_running_response_id != current_response_id - and _turn_has_assistant_output(items, current_response_id) - ): - try: - await post_external_session_status( - client, - session_id=session_id, - status="running", - response_id=current_response_id, - ) - dedupe.posted_running_response_id = current_response_id - except httpx.HTTPError: - _logger.warning( - "Failed to forward Claude turn-start running status; session=%s response_id=%s", - session_id, - current_response_id, - exc_info=True, - ) + # This function publishes no session status. Claude's own + # ``sessions/.json`` owns the running/idle badge (see + # :mod:`omnigent.claude_native_status_file`), and it reports the turn ending + # the moment Claude settles. A status edge derived from the transcript can + # only fire once a poll has parsed assistant output, so it lands *after* the + # file's ``idle`` on a short turn and re-asserts ``running`` on a session + # that already finished — the user sees idle → running → idle. Items carry + # their own ``response_id`` (see :func:`_post_external_conversation_item`), + # so the transcript's job here is items, not status. updated = state for item in items: if item.source_id in seen: diff --git a/tests/test_claude_native_forwarder.py b/tests/test_claude_native_forwarder.py index 4739fc6bde..cfb31429fa 100644 --- a/tests/test_claude_native_forwarder.py +++ b/tests/test_claude_native_forwarder.py @@ -1175,13 +1175,10 @@ async def test_forwarder_posts_visible_transcript_items(tmp_path: Path) -> None: ) ) try: - # Collect the seven transcript items. This transcript's final turn is a - # ``!bash`` command (a ``terminal_command``, no assistant output), so - # ``current_response_id`` lands on a turn that runs no LLM turn and thus - # gets no id-bearing ``running`` edge (that would strand the web UI busy - # with no ``Stop`` hook to close it). The turn-start ``running`` edge is - # asserted for a real assistant turn in - # ``test_forwarder_emits_turn_start_running_with_response_id``. + # Collect the seven transcript items. The transcript path publishes no + # session status at all — Claude's status file owns the badge — which + # ``test_forwarder_publishes_no_status_for_assistant_output`` asserts + # directly. requests = [await _get_recorded_item_request(server) for _index in range(7)] finally: task.cancel() @@ -1380,9 +1377,8 @@ async def test_forwarder_posts_web_injected_terminal_transcript_items(tmp_path: ) ) try: - # The turn-start ``running`` status posts first (the transcript has an - # assistant turn), then the assistant message item. - running = await _get_recorded_request(server) + # The item posts FIRST: the transcript path publishes no status at all + # (Claude's status file owns the badge), so nothing precedes it. request = await _get_recorded_request(server) finally: task.cancel() @@ -1392,8 +1388,6 @@ async def test_forwarder_posts_web_injected_terminal_transcript_items(tmp_path: server.server_close() thread.join(timeout=5.0) - assert running["body"]["type"] == "external_session_status" - assert running["body"]["data"]["status"] == "running" assert request["path"] == "/v1/sessions/conv_abc/events" assert request["body"]["type"] == "external_conversation_item" assert request["body"]["data"]["item_type"] == "message" @@ -3018,19 +3012,17 @@ def _handle_request(request: httpx.Request) -> httpx.Response: ) persisted = json.loads((bridge_dir / "transcript_forwarder.json").read_text("utf-8")) - # The turn-start ``running`` status (carrying the turn's response id) leads, - # then the poison item is attempted twice, then the forwarder-failed status. + # The poison item is attempted twice, then the forwarder-failed status. No + # status POST leads: the transcript path publishes none (Claude's status + # file owns the badge). assert [request["type"] for request in requests] == [ - "external_session_status", "external_conversation_item", "external_conversation_item", "external_session_status", ] - # The turn-start ``running`` edge carries the turn's response id, and the - # failed edge carries BOTH the drop reason as ``output`` (#1113 — the - # server surfaces it as the failure detail) and that same response id so + # The failed edge carries BOTH the drop reason as ``output`` (#1113 — the + # server surfaces it as the failure detail) and the turn's response id so # it closes the streaming turn instead of leaving its tool cards spinning. - assert requests[0]["data"]["status"] == "running" assert requests[-1]["data"] == { "status": "failed", "output": "transcript item poison-item:0:message rejected", @@ -5469,16 +5461,16 @@ def test_promote_pending_settle_waits_for_turn_quiescence() -> None: @pytest.mark.asyncio -async def test_scheduled_wake_forwards_marker_and_new_running_edge(tmp_path: Path) -> None: +async def test_scheduled_wake_forwards_marker_under_a_new_turn_id(tmp_path: Path) -> None: """ The full wake pipeline: settle → quiet-poll promote → marked new turn. Poll 1 forwards a turn; its Stop edge records the pending settle (covered by the status-events test — recorded directly here). Poll 2 is quiet and promotes the settle, persisting it. Poll 3 sees new - assistant entries — a cron firing writes no user entry — and must - POST a fresh turn-start ``running`` edge plus the scheduled-wake - marker ahead of the resumed output, all under a new response id. + assistant entries — a cron firing writes no user entry — and must POST + the scheduled-wake marker ahead of the resumed output, all under a new + response id. """ bridge_dir = tmp_path / "bridge" transcript_path = tmp_path / "session.jsonl" @@ -5571,13 +5563,15 @@ def _handle_request(request: httpx.Request) -> httpx.Response: dedupe=dedupe, ) - kinds = [request["type"] for request in requests] - assert kinds[0] == "external_session_status" - running = requests[0]["data"] - wake_turn_id = running["response_id"] - assert running["status"] == "running" - assert wake_turn_id != turn_one_id + # No status POST: the transcript path publishes none (Claude's status file + # owns the badge). The wake is observable entirely in the items — a fresh + # turn id plus the marker ahead of the resumed output. + assert [request["type"] for request in requests] == ["external_conversation_item"] * len( + requests + ) items = [r["data"] for r in requests if r["type"] == "external_conversation_item"] + wake_turn_id = items[0]["response_id"] + assert wake_turn_id != turn_one_id assert [item["item_data"]["role"] for item in items] == ["user", "assistant"] assert items[0]["item_data"]["content"] == [ {"type": "input_text", "text": "[System: scheduled prompt fired]"} @@ -7860,15 +7854,15 @@ def handler(request: httpx.Request) -> httpx.Response: @pytest.mark.asyncio -async def test_forwarder_emits_turn_start_running_with_response_id(tmp_path: Path) -> None: +async def test_forwarder_publishes_no_status_for_assistant_output(tmp_path: Path) -> None: """ - The first assistant output of a turn publishes ``running`` + its response id. + Assistant output forwards items and publishes NO session status. - Native Claude's running/idle BADGE stays PTY-derived; this id-bearing - ``running`` edge is the additional signal that lets ap-web open a streaming - ``activeResponse`` for the turn, so the forwarded tool cards (which share - the same response id) render LIVE rather than as static completed cards. - The running edge's response id must equal the forwarded items' response id. + Claude's ``sessions/.json`` owns the running/idle badge. A status edge + derived from the transcript can only fire once a poll has parsed assistant + output, so on a short turn it lands *after* the file's ``idle`` and + re-asserts ``running`` on a session that already finished — the user sees + idle → running → idle. The items still carry their own ``response_id``. """ bridge_dir = tmp_path / "bridge" transcript_path = tmp_path / "session.jsonl" @@ -7925,9 +7919,7 @@ async def test_forwarder_emits_turn_start_running_with_response_id(tmp_path: Pat ) ) try: - # First POST of the poll is the turn-start running status (it runs - # before the items in _forward_available_items); the two items follow. - running = await _get_recorded_request(server) + # Both POSTs of the poll are items — no status edge precedes them. item_a = await _get_recorded_request(server) item_b = await _get_recorded_request(server) finally: @@ -7938,20 +7930,90 @@ async def test_forwarder_emits_turn_start_running_with_response_id(tmp_path: Pat server.server_close() thread.join(timeout=5.0) - assert running["body"]["type"] == "external_session_status" - assert running["body"]["data"]["status"] == "running" - running_rid = running["body"]["data"]["response_id"] - assert isinstance(running_rid, str) and running_rid - # The running edge's response id matches the ASSISTANT turn's forwarded - # item (the function_call), so that bubble enters the streaming lifecycle - # on the client. The user message carries its own distinct response id. + # Neither POST is a status edge — the transcript path publishes none. + assert [body["body"]["type"] for body in (item_a, item_b)] == [ + "external_conversation_item", + "external_conversation_item", + ] + # The assistant turn's item still carries its own response id, which is + # what groups its bubble and its tool cards on the client. function_call = next( - body - for body in (item_a, item_b) - if body["body"]["type"] == "external_conversation_item" - and body["body"]["data"]["item_type"] == "function_call" + body for body in (item_a, item_b) if body["body"]["data"]["item_type"] == "function_call" ) - assert function_call["body"]["data"]["response_id"] == running_rid + rid = function_call["body"]["data"]["response_id"] + assert isinstance(rid, str) and rid + + +@pytest.mark.asyncio +async def test_short_turn_poll_posts_items_without_a_status_edge(tmp_path: Path) -> None: + """ + Regression: a short turn's poll must not re-assert ``running``. + + The status file reports the turn ending the moment Claude settles, but a + transcript-derived edge can only fire once a poll has parsed assistant + output — so it arrived *after* that ``idle`` and flipped the session back to + ``running``, then ``Stop`` closed it again: the user saw + idle → running → idle on every short turn. + """ + bridge_dir = tmp_path / "bridge" + transcript_path = tmp_path / "session.jsonl" + transcript_path.write_text( + "\n".join( + [ + json.dumps( + { + "type": "user", + "uuid": "u1", + "message": {"role": "user", "content": "i'll keep testing"}, + } + ), + json.dumps( + { + "type": "assistant", + "uuid": "a1", + "message": { + "role": "assistant", + "content": [{"type": "text", "text": "Sounds good."}], + }, + } + ), + ] + ) + + "\n", + encoding="utf-8", + ) + state = forwarder.TranscriptForwardState( + transcript_path=transcript_path, + line_cursor=0, + byte_offset=0, + cursor_fingerprint=forwarder._jsonl_cursor_fingerprint(transcript_path, 0), + ) + posted: list[dict[str, Any]] = [] + + def _handle_request(request: httpx.Request) -> httpx.Response: + """ + Record every forwarder POST body. + + :param request: Outbound HTTP request from the forwarder. + :returns: HTTP 202 for the mock Omnigent endpoint. + """ + posted.append(json.loads(request.content.decode("utf-8"))) + return httpx.Response(202, json={}) + + transport = httpx.MockTransport(_handle_request) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + await forwarder._forward_available_items( + client=client, + session_id="conv_abc", + bridge_dir=bridge_dir, + agent_name="claude-native-ui", + state=state, + retry_tracker=forwarder._PostRetryTracker(), + dedupe=forwarder._ForwardDedupeState(), + ) + + assert [body["type"] for body in posted] == ["external_conversation_item"] * 2 + assert not [body for body in posted if body["type"] == "external_session_status"] @pytest.mark.asyncio From f7475f616d40012a0496e0dd06ea1d3c60d18918 Mon Sep 17 00:00:00 2001 From: Daniel Lok Date: Fri, 7 Aug 2026 23:03:39 +0800 Subject: [PATCH 3/6] =?UTF-8?q?fix(web):=20light=20the=20chat=20"Working?= =?UTF-8?q?=E2=80=A6"=20indicator=20on=20send,=20like=20the=20sidebar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- web/src/pages/ChatPage.test.ts | 33 +++++++++++++++++++++++++++++++++ web/src/pages/ChatPage.tsx | 26 ++++++++++++++++++++------ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/web/src/pages/ChatPage.test.ts b/web/src/pages/ChatPage.test.ts index fcdcae3906..1dcff1149e 100644 --- a/web/src/pages/ChatPage.test.ts +++ b/web/src/pages/ChatPage.test.ts @@ -775,6 +775,39 @@ describe("computeShowsWorking", () => { false, ); }); + + it("an in-flight local send lights the indicator before any server edge", () => { + // Pressing Enter sets chatStore.status = "streaming" synchronously, while + // `sessionStatus` stays `idle` until the server publishes `running` (for + // claude-native, until the status file's next poll). The sidebar row + // already lights up off this flag, so the chat pane must too or the two + // disagree for the whole dispatch round-trip. + expect(computeShowsWorking("idle", opts({ localSendInFlight: true }))).toBe(true); + }); + + it("an in-flight local send survives a stale offline poll", () => { + // Sending to an asleep runner relaunches it; `/health` reads stale-offline + // during that window (10s cadence). The user dispatched, so the indicator + // must not be suppressed — same reasoning as live running/waiting above. + expect( + computeShowsWorking("idle", opts({ localSendInFlight: true, runnerOnline: false })), + ).toBe(true); + }); + + it("a pending elicitation still outranks an in-flight local send", () => { + // The elicitation prompt owns the in-progress slot; the two must never + // stack, so the suppression applies regardless of how the work was + // signalled. + expect( + computeShowsWorking("idle", opts({ localSendInFlight: true, hasPendingElicitation: true })), + ).toBe(false); + }); + + it("no local send in flight leaves an idle session idle", () => { + // The flag is opt-in: a cross-client or TUI-typed turn sets no local + // status here, so an idle session with no send stays dark. + expect(computeShowsWorking("idle", opts({ localSendInFlight: false }))).toBe(false); + }); }); // ── shouldShowWorkingIndicator ────────────────────────────────────────────── diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index 099935a41d..7c2b0344b6 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -1011,6 +1011,11 @@ export function ChatPage() { hasPendingElicitation, runnerOnline, backgroundTaskCount, + // Optimistic: light up the moment this client dispatches, without waiting + // for the server's ``running``. The sidebar row already reads this same + // flag (``isStartingUp`` in Sidebar.tsx), so the two agreed only once the + // server confirmed; now they agree immediately. + localSendInFlight: status === "streaming", }); // A fork of a coding session carries the source id in this label (set by @@ -5541,10 +5546,17 @@ export function computeIsWorking(sessionStatus: SessionStatus): boolean { * 10s cadence and reads stale-offline during the runner's connect window on a * fresh session's first turn (it would otherwise hide "Working…" for seconds). * @param options.backgroundTaskCount - Background shells still running after - * the turn ended. A claude-native turn settles to ``idle`` (the PTY-activity - * watcher's edge) even while shells run, so the bare status alone would hide - * the indicator; a positive count keeps it lit so "N background tasks still running" + * the turn ended. A claude-native turn settles to ``idle`` (the status file's + * edge) even while shells run, so the bare status alone would hide the + * indicator; a positive count keeps it lit so "N background tasks still running" * stays visible. + * @param options.localSendInFlight - This client's own send is in flight + * (``chatStore.status === "streaming"``). Lights the indicator optimistically + * the moment the user presses Enter, before any server edge confirms the turn + * — the sidebar row already does this (see ``isStartingUp`` in Sidebar.tsx), + * so without it the two disagree for the dispatch round-trip. Distinct from + * ``sessionStatus``, which mirrors the server: this one means "we asked", not + * "the agent is working". * @returns ``true`` when the main session's own status should render Working. */ export function computeShowsWorking( @@ -5553,6 +5565,7 @@ export function computeShowsWorking( hasPendingElicitation: boolean; runnerOnline: boolean | undefined; backgroundTaskCount?: number; + localSendInFlight?: boolean; }, ): boolean { if (options.hasPendingElicitation) return false; @@ -5560,9 +5573,10 @@ export function computeShowsWorking( // A running/waiting session is proof the runner is up, so a stale // poll-derived ``runnerOnline === false`` must not suppress it. Only gate on // known-offline for the not-actively-working case (e.g. a background-shell - // tally on an idle session). - if (options.runnerOnline === false && !isWorking) return false; - return isWorking || (options.backgroundTaskCount ?? 0) > 0; + // tally on an idle session). An in-flight local send is the same kind of + // proof — the user just dispatched — so it also survives the gate. + if (options.runnerOnline === false && !isWorking && !options.localSendInFlight) return false; + return isWorking || options.localSendInFlight === true || (options.backgroundTaskCount ?? 0) > 0; } /** From a54104b46a33f46eee97a5c1db841513523349b0 Mon Sep 17 00:00:00 2001 From: Daniel Lok Date: Sat, 8 Aug 2026 12:03:44 +0800 Subject: [PATCH 4/6] fix(runner): re-assert session status after the tunnel reconnects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/.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 --- omnigent/claude_native_status_file.py | 40 ++++++++++++++++- omnigent/runner/app.py | 12 +++++ omnigent/runner/resource_registry.py | 49 ++++++++++++++++++++ tests/runner/test_resource_registry.py | 60 +++++++++++++++++++++++++ tests/test_claude_native_status_file.py | 58 ++++++++++++++++++++++++ 5 files changed, 218 insertions(+), 1 deletion(-) diff --git a/omnigent/claude_native_status_file.py b/omnigent/claude_native_status_file.py index 233a4d0c28..b247d178c1 100644 --- a/omnigent/claude_native_status_file.py +++ b/omnigent/claude_native_status_file.py @@ -27,12 +27,15 @@ from __future__ import annotations import json +import logging import os import time from collections.abc import Callable from dataclasses import dataclass from pathlib import Path +_logger = logging.getLogger(__name__) + # Runner-side status vocabulary the file maps onto. ``busy`` and # ``waiting`` both mean "the turn is not finished" from the session's point # of view, so both map to ``running``; ``waiting`` is distinguished for the @@ -343,15 +346,31 @@ def tick(self) -> None: def _try_resolve(self) -> None: """Attempt one resolution, retiring to the PTY watcher on timeout.""" self._attempts += 1 + pane_pid = self._pane_pid_getter() path = resolve_status_file( - pane_pid=self._pane_pid_getter(), + pane_pid=pane_pid, expected_session_id=self._session_id_getter(), config_dir=self._config_dir, ) if path is not None: + # Log the hit: whether the file was found at all decides which + # source owns the session's status, and without this the answer is + # only reachable by re-deriving the resolution by hand. + _logger.info( + "claude status file resolved: path=%s attempts=%d pane_pid=%s", + path, + self._attempts, + pane_pid, + ) self._path = path return if self._attempts >= _MAX_RESOLVE_ATTEMPTS: + _logger.warning( + "claude status file never resolved after %d attempts " + "(pane_pid=%s); session status falls back to the pane watcher", + self._attempts, + pane_pid, + ) self._exhausted = True def retire(self) -> None: @@ -362,8 +381,27 @@ def retire(self) -> None: the file owns the session's status while it is readable, a dead pane must retire it or that final value would pin the session forever. """ + _logger.info("claude status file retired: path=%s", self._path) self._exhausted = True + def resync(self) -> None: + """Forget what was published so the next tick re-asserts the file. + + The file is written only when its value *changes*, so a poller that + already published ``running`` has nothing more to say until Claude's + status moves. That is a problem when the *listener* restarts: a server + recycle wipes its status cache, and the session would sit on a stale + ``idle`` for the rest of the turn because every source believes it + already reported. Dropping the edge/mtime baselines makes the next tick + publish the file's current value verbatim. + + Keeps the resolved path and the attempt count — this re-asserts a + working poller, it does not restart resolution. + """ + _logger.info("claude status file resync: path=%s", self._path) + self._last_mtime = None + self._last_edge = None + @property def blocked_on(self) -> str | None: """Why Claude is parked, when it is parked on a dialog. diff --git a/omnigent/runner/app.py b/omnigent/runner/app.py index d93e5bb458..9eb2046bfb 100644 --- a/omnigent/runner/app.py +++ b/omnigent/runner/app.py @@ -8593,6 +8593,18 @@ async def elicitation(elicitation_id: str, request: Request) -> Response: ) async def _catch_up_scan() -> None: + # The tunnel just reconnected, which usually means the SERVER restarted + # (deploy, crash, replica failover) and lost its in-memory session-status + # cache. This runner did not restart, so every status source still + # believes its last edge was delivered and nothing re-asserts — a + # native session mid-turn during the restart would sit on a stale + # ``idle`` for the rest of the turn. Re-arm them before the item scan + # below (which skips native harnesses entirely). + if resource_registry is not None: + try: + resource_registry.resync_session_statuses() + except Exception: # noqa: BLE001 — best-effort; never block catch-up. + _logger.warning("Session status resync failed after reconnect", exc_info=True) for session_id in list(_session_histories): if _is_native_harness(session_id): continue diff --git a/omnigent/runner/resource_registry.py b/omnigent/runner/resource_registry.py index c1d73c2929..b6df5c0256 100644 --- a/omnigent/runner/resource_registry.py +++ b/omnigent/runner/resource_registry.py @@ -308,6 +308,11 @@ def __init__( # which the turn-start hook also writes — deduping against that one # would swallow the turn's real ``running``. self._published_session_status: dict[str, tuple[str, str | None]] = {} + # Live claude-native status-file pollers, per session. Held so a + # reconnect can re-arm them (see :meth:`resync_session_statuses`) — the + # poller keeps its own edge/mtime baselines on the watcher thread, and + # clearing the registry's baseline alone would leave those intact. + self._status_pollers: dict[str, SessionStatusPoller] = {} # Optional callback invoked on the event loop when a watched terminal # disappears unexpectedly. The callback receives the terminal's # lifecycle relationship so the runner can decide whether the owning @@ -398,6 +403,7 @@ def _take_session_status_memo(self, session_id: str) -> str | None: """Pop and return the session's recorded PTY status (or ``None``).""" with self._lock: self._published_session_status.pop(session_id, None) + self._status_pollers.pop(session_id, None) return self._last_session_status.pop(session_id, None) def _claim_status_edge(self, session_id: str, status: str, blocked_on: str | None) -> bool: @@ -423,6 +429,42 @@ def _sync_status_edge(self, session_id: str, status: str) -> None: with self._lock: self._published_session_status[session_id] = (status, None) + def resync_session_statuses(self) -> None: + """Re-arm every status source so it republishes what it already sent. + + Called after the runner's tunnel reconnects. A server recycle (deploy, + crash, replica failover) restarts the *listener*, wiping its in-memory + status cache — but this runner keeps running, so every dedup baseline + still asserts the pre-restart edge was delivered. Nothing re-asserts on + its own: Claude's status file is written only when its value *changes*, + and the pane watcher's edges are coalesced, so a session mid-turn during + the restart would sit on a stale ``idle`` until its next turn boundary — + no spinner, no stop button, for the rest of the turn. + + Dropping the published-edge baselines here makes the next poll publish + the current status verbatim. The claude-native pollers are re-armed too: + they hold their own edge/mtime baselines on the watcher thread, so + clearing only this side would leave them silent. + + Deliberately does NOT clear ``_last_session_status`` — that memo + classifies terminal exits (clean vs mid-turn crash) and is unrelated to + what the server has heard. + """ + with self._lock: + sessions = sorted(self._published_session_status) + self._published_session_status.clear() + pollers = list(self._status_pollers.values()) + for poller in pollers: + poller.resync() + if sessions or pollers: + _logger.info( + "Re-arming session status after tunnel reconnect: " + "cleared_edges=%d pollers=%d sessions=%s", + len(sessions), + len(pollers), + sessions, + ) + def note_session_turn_started(self, session_id: str) -> None: """Mark a session as having an in-flight turn. @@ -1118,6 +1160,9 @@ def _file_owns_status() -> bool: if emit_status and resource_role == CLAUDE_NATIVE_TERMINAL_ROLE else None ) + if status_poller is not None: + with self._lock: + self._status_pollers[session_id] = status_poller def _on_activity() -> None: # Runs on the watcher daemon thread; hop to the loop so the @@ -1435,6 +1480,10 @@ async def transfer_terminal( moved_status = self._last_session_status.pop(source_session_id, None) if moved_status is not None and target_session_id not in self._last_session_status: self._last_session_status[target_session_id] = moved_status + # The watcher restart below rebuilds the poller under the + # target, so drop the source's entry rather than leaving a + # retired poller to be re-armed on every later reconnect. + self._status_pollers.pop(source_session_id, None) try: await entry.instance.set_conversation_link( self._terminal_registry.conversation_link_for_id(target_session_id) diff --git a/tests/runner/test_resource_registry.py b/tests/runner/test_resource_registry.py index d1ba0f9b3b..897bfcc8f5 100644 --- a/tests/runner/test_resource_registry.py +++ b/tests/runner/test_resource_registry.py @@ -414,6 +414,7 @@ def __init__(self, on_status: object) -> None: self.blocked_on: str | None = None self.ticks = 0 self.retired = False + self.resyncs = 0 def tick(self) -> None: self.ticks += 1 @@ -422,6 +423,9 @@ def retire(self) -> None: self.retired = True self.active = False + def resync(self) -> None: + self.resyncs += 1 + def emit(self, status: str, blocked_on: str | None = None) -> None: """Simulate the file reporting a new status.""" self._on_status(status, blocked_on) @@ -592,6 +596,62 @@ async def test_hook_status_resyncs_watcher_dedup(tmp_path: Path) -> None: del callbacks +@pytest.mark.asyncio +async def test_reconnect_resync_republishes_a_running_session(tmp_path: Path) -> None: + """A server restart mid-turn must not strand the session on a stale status. + + The tunnel reconnecting means the *listener* restarted and lost its status + cache. This runner did not, so its dedup baseline still asserts ``running`` + was delivered — and Claude's file is written only when its value *changes*, + so nothing re-asserts on its own. Without the resync the session would show + no spinner and no stop button for the rest of the turn. + """ + _callbacks, statuses, pollers, registry = await _observe_native_with_fake_poller( + tmp_path, "conv_restart" + ) + poller = pollers[0] + poller.active = True + + poller.emit("running") + await asyncio.sleep(0) + assert statuses == ["running"] + + # Mid-turn, the file's value is unchanged, so a re-read publishes nothing: + # this is exactly what leaves the restarted server on a stale status. + poller.emit("running") + await asyncio.sleep(0) + assert statuses == ["running"] + + registry.resync_session_statuses() + assert poller.resyncs == 1 + + # The same value now republishes, so the fresh server learns the truth. + poller.emit("running") + await asyncio.sleep(0) + assert statuses == ["running", "running"] + + +@pytest.mark.asyncio +async def test_reconnect_resync_keeps_the_exit_classification_memo(tmp_path: Path) -> None: + """The resync clears published edges, not the exit memo. + + ``_last_session_status`` decides whether a terminal exit reads as a clean + shutdown or a mid-turn crash. It tracks what the PANE last did, not what the + server has heard, so a reconnect must leave it alone — clearing it would make + a crash right after a reconnect look like a tidy exit. + """ + _callbacks, _statuses, pollers, registry = await _observe_native_with_fake_poller( + tmp_path, "conv_memo" + ) + pollers[0].active = True + pollers[0].emit("running") + await asyncio.sleep(0) + + registry.resync_session_statuses() + + assert registry._take_session_status_memo("conv_memo") == "running" + + @pytest.mark.asyncio async def test_pty_edges_drive_status_when_poller_inactive(tmp_path: Path) -> None: """With no file (poller inactive), the PTY pane edges remain the status diff --git a/tests/test_claude_native_status_file.py b/tests/test_claude_native_status_file.py index 5699f61f57..74112b98e6 100644 --- a/tests/test_claude_native_status_file.py +++ b/tests/test_claude_native_status_file.py @@ -329,6 +329,64 @@ def test_retire_stops_the_file_owning_status(tmp_path: Path) -> None: assert published == [(RUNNING, "input needed")] +def test_resync_republishes_an_unchanged_file(tmp_path: Path) -> None: + """A resync makes the next tick re-assert the file's current value. + + The file is rewritten only when its value *changes*, so a poller mid-turn + has nothing more to say — which strands the server when the SERVER is what + restarted and lost its cache. A resync drops the edge/mtime baselines so the + same value publishes again. + """ + sessions = tmp_path / "sessions" + _write_session_file(sessions, pid=1, session_id="s", status="busy") + published: list[tuple[str, str | None]] = [] + poller = SessionStatusPoller( + on_status=lambda status, reason: published.append((status, reason)), + pane_pid_getter=_StubPidGetter(1), + session_id_getter=lambda: "s", + config_dir=tmp_path, + ) + poller.tick() + assert published == [(RUNNING, None)] + + # Unchanged file: normally silent, which is the whole problem. + poller.tick() + assert published == [(RUNNING, None)] + + poller.resync() + poller.tick() + assert published == [(RUNNING, None), (RUNNING, None)] + # Still reading the same resolved file — a resync re-asserts a working + # poller rather than restarting resolution. + assert poller.active is True + + +def test_resync_does_not_revive_a_retired_poller(tmp_path: Path) -> None: + """A retired poller stays retired across a reconnect. + + Retirement means the pane's process is gone (or no file ever resolved), so + the PTY side owns the outcome. A reconnect must not hand ownership back to a + dead Claude's leftover record. + """ + sessions = tmp_path / "sessions" + _write_session_file(sessions, pid=1, session_id="s", status="busy") + published: list[str] = [] + poller = SessionStatusPoller( + on_status=lambda status, _reason: published.append(status), + pane_pid_getter=_StubPidGetter(1), + session_id_getter=lambda: "s", + config_dir=tmp_path, + ) + poller.tick() + poller.retire() + + poller.resync() + poller.tick() + + assert poller.active is False + assert published == [RUNNING] + + def test_waiting_carries_its_reason(tmp_path: Path) -> None: """``waiting`` exposes ``waitingFor`` so the UI can say what it is parked on.""" sessions = tmp_path / "sessions" From b659dc4027772a87da9b696f846620211c3494fc Mon Sep 17 00:00:00 2001 From: Daniel Lok Date: Sat, 8 Aug 2026 12:04:07 +0800 Subject: [PATCH 5/6] =?UTF-8?q?fix(web):=20let=20a=20spin-up=20keep=20the?= =?UTF-8?q?=20"Starting=20up=E2=80=A6"=20cue=20over=20the=20shimmer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 953187f9 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 --- web/src/pages/ChatPage.test.ts | 15 +++++++++++++++ web/src/pages/ChatPage.tsx | 15 ++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/web/src/pages/ChatPage.test.ts b/web/src/pages/ChatPage.test.ts index 1dcff1149e..a55fc670fe 100644 --- a/web/src/pages/ChatPage.test.ts +++ b/web/src/pages/ChatPage.test.ts @@ -794,6 +794,21 @@ describe("computeShowsWorking", () => { ).toBe(true); }); + it("a spin-up in flight yields the slot to the Starting-up cue", () => { + // ChatPage passes `localSendInFlight: status === "streaming" && !spinUpInFlight`. + // `RunnerStartingIndicator` renders only when the shimmer is absent, and its + // copy ("Starting up…" / "Cloning repository…") is strictly more informative + // than a generic shimmer — so during a boot the optimistic path stands down. + expect(computeShowsWorking("idle", opts({ localSendInFlight: false }))).toBe(false); + }); + + it("a server-confirmed running still wins during a spin-up", () => { + // Once the harness reports work the spin-up cue has self-gated to null, so + // suppressing the shimmer too would leave the turn with no indicator at all. + // `localSendInFlight` is the only thing the spin-up gate touches. + expect(computeShowsWorking("running", opts({ localSendInFlight: false }))).toBe(true); + }); + it("a pending elicitation still outranks an in-flight local send", () => { // The elicitation prompt owns the in-progress slot; the two must never // stack, so the suppression applies regardless of how the work was diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index 7c2b0344b6..586a729ac7 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -798,6 +798,9 @@ export function ChatPage() { // handling). Overrides the liveness-derived unreachable affordances // below, which misread the not-yet-host-bound session as stranded. const sandboxLaunching = sandboxStatus !== null && sandboxStatus.stage !== "failed"; + // Terminal-first spin-up state, read here (not just in the child surfaces) so + // the working-indicator gate below can defer to the "Starting up…" cue. + const chatTerminalFirst = useTerminalFirst(); // Read runner liveness from the app-level batch poller (see // RunnerHealthProvider). `undefined` = not yet polled — the indicator // stays hidden until the first poll for this session resolves. @@ -1007,6 +1010,16 @@ export function ChatPage() { // + shimmer/pill) for the main chat and is suppressed mid-elicitation or // when the runner is known offline. const isWorking = !hasPendingElicitation && computeIsWorking(sessionStatus); + // A spin-up in flight owns the in-progress slot with more specific copy + // ("Starting up…" / "Cloning repository…") than the generic shimmer, and + // `RunnerStartingIndicator` only renders when the shimmer is absent. So the + // OPTIMISTIC path must stand down here: a send that has to boot a runner is + // exactly when the user needs to know it's booting, not just that we asked. + // A server-confirmed `running`/`waiting` still wins — by then the harness is + // up and the spin-up cue has self-gated to null. + const spinUpInFlight = + sandboxLaunching || + Boolean(chatTerminalFirst?.isTerminalFirst && chatTerminalFirst.terminalStartingUp); const showsWorking = computeShowsWorking(sessionStatus, { hasPendingElicitation, runnerOnline, @@ -1015,7 +1028,7 @@ export function ChatPage() { // for the server's ``running``. The sidebar row already reads this same // flag (``isStartingUp`` in Sidebar.tsx), so the two agreed only once the // server confirmed; now they agree immediately. - localSendInFlight: status === "streaming", + localSendInFlight: status === "streaming" && !spinUpInFlight, }); // A fork of a coding session carries the source id in this label (set by From 821e05239cbe98b83df7ef28a154d6be00955155 Mon Sep 17 00:00:00 2001 From: Daniel Lok Date: Sat, 8 Aug 2026 21:58:25 +0800 Subject: [PATCH 6/6] fix(web): spin claude-native's in-flight tools off the session status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- web/src/lib/renderItems.test.ts | 104 +++++++++++++++++++++++++++++ web/src/lib/renderItems.ts | 113 ++++++++++++++++++++++++++++++-- web/src/pages/ChatPage.tsx | 12 +++- 3 files changed, 221 insertions(+), 8 deletions(-) diff --git a/web/src/lib/renderItems.test.ts b/web/src/lib/renderItems.test.ts index 6963fcff03..77913b827a 100644 --- a/web/src/lib/renderItems.test.ts +++ b/web/src/lib/renderItems.test.ts @@ -751,6 +751,110 @@ describe("buildBubbles — tool joining", () => { }); }); +describe("buildBubbles — session-driven trailing tool spinner", () => { + // claude-native has no streaming `activeResponse` — its running/idle lives + // in `sessionStatus`. The `sessionRunning` arg spins the newest turn's + // trailing tool phase so a dispatched-but-unresolved tool shows a spinner + // instead of "No output", without any bubble reaching lifecycle "streaming". + function toolState(bubbles: Bubble[]): string { + const asst = bubbles[bubbles.length - 1] as Extract; + const tool = asst.items.find( + (item): item is Extract => item.kind === "tool", + ); + expect(tool).toBeDefined(); + return tool!.state; + } + + const danglingToolTurn: AnyBlock[] = [ + { + type: "tool_group", + ctx: ctx({ itemId: "fc_1", responseId: "resp_1" }), + executions: [mkExec("Bash", "c1")], + iteration: 0, + }, + ]; + + it("spins the newest turn's trailing tool while the session is running", () => { + // No activeResponse (claude-native never opens one), sessionRunning=true. + const bubbles = buildBubbles(danglingToolTurn, null, undefined, [], true); + expect(toolState(bubbles)).toBe("input-available"); + }); + + it("does not spin when the session is idle (dangling tool resolves to no-output)", () => { + // The idle edge can land before the tool's result block — the tool must + // settle, not spin forever. This is the property the never-spin tests pin, + // now exercised through the session-driven path. + const bubbles = buildBubbles(danglingToolTurn, null, undefined, [], false); + expect(toolState(bubbles)).toBe("no-output"); + }); + + it("a resolved tool shows its output regardless of session running", () => { + const blocks: AnyBlock[] = [ + ...danglingToolTurn, + { + type: "tool_result", + ctx: ctx({ itemId: "fco_1", responseId: "resp_1" }), + name: "", + callId: "c1", + agentName: "test", + output: "done", + }, + ]; + const bubbles = buildBubbles(blocks, null, undefined, [], true); + expect(toolState(bubbles)).toBe("output-available"); + }); + + it("only the NEWEST turn spins — an earlier turn's dangling tool stays no-output", () => { + const blocks: AnyBlock[] = [ + { + type: "tool_group", + ctx: ctx({ itemId: "fc_old", responseId: "resp_old" }), + executions: [mkExec("Read", "c_old")], + iteration: 0, + }, + { type: "user_message", ctx: ctx({ itemId: "u1", responseId: "" }), content: [] }, + { + type: "tool_group", + ctx: ctx({ itemId: "fc_new", responseId: "resp_new" }), + executions: [mkExec("Bash", "c_new")], + iteration: 0, + }, + ]; + const bubbles = buildBubbles(blocks, null, undefined, [], true); + const assistants = bubbles.filter( + (b): b is Extract => b.kind === "assistant", + ); + const oldTool = assistants[0].items.find((item) => item.kind === "tool"); + const newTool = assistants[1].items.find((item) => item.kind === "tool"); + expect((oldTool as Extract).state).toBe("no-output"); + expect((newTool as Extract).state).toBe("input-available"); + }); + + it("a trailing user message means no live turn — nothing spins", () => { + // A just-sent prompt with no assistant output yet: newestAssistantTurnId + // returns null, so the earlier turn's tool does not spin. + const blocks: AnyBlock[] = [ + ...danglingToolTurn, + { type: "user_message", ctx: ctx({ itemId: "u1", responseId: "" }), content: [] }, + ]; + const bubbles = buildBubbles(blocks, null, undefined, [], true); + const asst = bubbles.find( + (b): b is Extract => b.kind === "assistant", + )!; + const tool = asst.items.find((item) => item.kind === "tool"); + expect((tool as Extract).state).toBe("no-output"); + }); + + it("the cache re-walks on a running→idle flip so a dangling tool stops spinning", () => { + // The flip carries no block change; the cache key must still see it move. + const cache = createBubbleCache(); + expect(toolState(buildBubbles(danglingToolTurn, null, cache, [], true))).toBe( + "input-available", + ); + expect(toolState(buildBubbles(danglingToolTurn, null, cache, [], false))).toBe("no-output"); + }); +}); + describe("buildBubbles — cross-bubble tool_result pairing", () => { function resultBlock( callId: string, diff --git a/web/src/lib/renderItems.ts b/web/src/lib/renderItems.ts index 76884140b3..48f3599492 100644 --- a/web/src/lib/renderItems.ts +++ b/web/src/lib/renderItems.ts @@ -229,6 +229,13 @@ export interface BubbleCache { blocks: AnyBlock[] | null; activeResponse: ActiveResponse | null; interruptedResponseIds: readonly string[] | null; + // Response id of the newest assistant turn while the SESSION is running, + // or null. Drives the trailing-tool spinner for harnesses whose + // running/idle lives in `sessionStatus` rather than a streaming + // `activeResponse` (see `newestAssistantTurnId`). Part of the cache key: a + // running→idle flip carries no block change, so the identity short-circuit + // must see it move or a dangling tool would spin forever. + liveTurnId: string | null; bubbles: Bubble[]; lastBubbleStart: number; lastBubbleCount: number; @@ -243,6 +250,7 @@ export function createBubbleCache(): BubbleCache { blocks: null, activeResponse: null, interruptedResponseIds: null, + liveTurnId: null, bubbles: [], lastBubbleStart: -1, lastBubbleCount: 1, @@ -250,6 +258,39 @@ export function createBubbleCache(): BubbleCache { }; } +/** + * Response id of the newest assistant turn — the turn whose trailing tool + * phase should spin while the SESSION is running. + * + * claude-native's running/idle lives in `sessionStatus` (the status file + * drives the badge; no streaming `activeResponse` is ever opened — see the + * transcript forwarder), so its in-flight tool calls can't key their spinner + * off `lifecycle === "streaming"` the way an in-process harness does. Instead + * the caller passes `sessionRunning` and this names which turn is live. + * + * Scans back from the end: the first assistant-side block carrying a real + * (non-anonymous) response id names that turn. A trailing user message (a + * just-sent prompt with no assistant output yet), a compaction/routing + * boundary, or an empty transcript yields `null` — there is no live turn. + */ +function newestAssistantTurnId(blocks: AnyBlock[]): string | null { + for (let i = blocks.length - 1; i >= 0; i -= 1) { + const b = blocks[i]!; + if ( + b.type === "user_message" || + b.type === "compaction" || + b.type === "compaction_loading" || + b.type === "routing_decision" + ) { + return null; + } + if (isNonRenderingBlock(b) || b.type === "tool_result") continue; + if (isAnonymousRid(b.ctx.responseId)) continue; + return b.ctx.responseId; + } + return null; +} + /** * Walk a flat block list and produce the bubble cluster list. * @@ -270,14 +311,24 @@ export function createBubbleCache(): BubbleCache { * and the unit tests rely on. * @param interruptedResponseIds - response ids whose bubbles should remain * labelled cancelled even after the active response sidecar has moved on. + * @param sessionRunning - whether the SESSION status is running/waiting. + * Lets the newest turn's trailing tool phase spin for a harness whose + * liveness lives in `sessionStatus` rather than a streaming `activeResponse` + * (claude-native). Does NOT change any bubble's `lifecycle` — fork, fold, + * cancelled, and failed are unaffected; it only reaches the tool-state gate. */ export function buildBubbles( blocks: AnyBlock[], activeResponse: ActiveResponse | null, cache?: BubbleCache, interruptedResponseIds: readonly string[] = EMPTY_INTERRUPTED_RESPONSE_IDS, + sessionRunning = false, ): Bubble[] { const interruptedResponses = new Set(interruptedResponseIds); + // The newest turn spins its trailing tools only while the session runs; an + // already-streaming `activeResponse` covers the in-process harnesses without + // it, so this is null unless the session is running. + const liveTurnId = sessionRunning ? newestAssistantTurnId(blocks) : null; // Resolved over the whole transcript before any reuse decision: a create-time // chip and the turn chip that repeats it verbatim are one verdict however far // apart they landed, and whether the pair is still visible decides whether the @@ -285,8 +336,16 @@ export function buildBubbles( const superseded = supersededRoutingChips(blocks); if (cache === undefined) { return markContinuedTurns( - walkBubbles(blocks, activeResponse, interruptedResponses, 0, [], new Map(), superseded) - .bubbles, + walkBubbles( + blocks, + activeResponse, + interruptedResponses, + 0, + [], + new Map(), + superseded, + liveTurnId, + ).bubbles, activeResponse, ); } @@ -295,14 +354,22 @@ export function buildBubbles( if ( cache.blocks === blocks && cache.activeResponse === activeResponse && - cache.interruptedResponseIds === interruptedResponseIds + cache.interruptedResponseIds === interruptedResponseIds && + cache.liveTurnId === liveTurnId ) { return cache.bubbles; } // Try the incremental path: reuse every finalized bubble (all but the // last) and rebuild only from where the last cached bubble started. - const reuse = reusablePrefix(blocks, activeResponse, interruptedResponses, cache, superseded); + const reuse = reusablePrefix( + blocks, + activeResponse, + interruptedResponses, + cache, + superseded, + liveTurnId, + ); if (reuse !== null) { const subIndexSeed = new Map(); for (const b of reuse.prefix) { @@ -318,10 +385,12 @@ export function buildBubbles( reuse.prefix, subIndexSeed, superseded, + liveTurnId, ); cache.blocks = blocks; cache.activeResponse = activeResponse; cache.interruptedResponseIds = interruptedResponseIds; + cache.liveTurnId = liveTurnId; cache.bubbles = markContinuedTurns(rest.bubbles, activeResponse); cache.lastBubbleStart = rest.lastBubbleStart; cache.lastBubbleCount = rest.lastBubbleCount; @@ -338,10 +407,12 @@ export function buildBubbles( [], new Map(), superseded, + liveTurnId, ); cache.blocks = blocks; cache.activeResponse = activeResponse; cache.interruptedResponseIds = interruptedResponseIds; + cache.liveTurnId = liveTurnId; cache.bubbles = markContinuedTurns(full.bubbles, activeResponse); cache.lastBubbleStart = full.lastBubbleStart; cache.lastBubbleCount = full.lastBubbleCount; @@ -486,6 +557,7 @@ function reusablePrefix( interruptedResponses: ReadonlySet, cache: BubbleCache, superseded: ReadonlySet, + liveTurnId: string | null, ): { prefix: Bubble[]; startBlock: number } | null { if (cache.blocks === null || cache.bubbles.length === 0 || cache.lastBubbleStart <= 0) { return null; @@ -540,6 +612,15 @@ function reusablePrefix( if (b.kind === "assistant" && b.responseId === activeId) return null; } } + // Same hazard for the session-driven spinner: the turn that WAS live (its + // trailing tool showed a spinner) or the one that IS now must be re-walked, + // not reused, so its tool state settles. Guard both — on an A→B transition A + // has moved into the prefix carrying its stale spinner, and on a + // running→idle flip the still-last bubble must drop it. + const liveIds = [liveTurnId, cache.liveTurnId]; + for (const b of prefix) { + if (b.kind === "assistant" && liveIds.includes(b.responseId)) return null; + } for (const b of prefix) { if (b.kind === "assistant" && interruptedResponses.has(b.responseId)) { return null; @@ -620,6 +701,7 @@ function walkBubbles( seedBubbles: Bubble[], subIndexByResp: Map, superseded: ReadonlySet, + liveTurnId: string | null = null, ): { bubbles: Bubble[]; lastBubbleStart: number; lastBubbleCount: number } { const bubbles: Bubble[] = [...seedBubbles]; // One cross-bubble result index per walk: the relay backdates a @@ -839,7 +921,15 @@ function walkBubbles( stableId, lifecycle, error, - items: buildAssistantItems(groupBlocks, lifecycle, crossBubbleResults), + // `sessionLive` spins this turn's trailing tools when the session is + // running and this is the newest turn — for a harness with no streaming + // `activeResponse`. `lifecycle` (and thus fork/fold) is untouched. + items: buildAssistantItems( + groupBlocks, + lifecycle, + crossBubbleResults, + liveTurnId !== null && groupResponseId === liveTurnId, + ), ...(workedForS !== undefined ? { workedForS } : {}), ...(lastActivityAtS !== undefined ? { lastActivityAtS } : {}), }); @@ -1185,11 +1275,12 @@ function buildAssistantItems( groupBlocks: AnyBlock[], lifecycle: ActiveResponse["state"], crossBubbleResults: Map, + sessionLive = false, ): RenderItem[] { // Results render only by folding into a call's card — strip them so // an absorbed out-of-band result can't split a text/reasoning run. const blocks = groupBlocks.filter((b) => b.type !== "tool_result"); - const liveToolCallIds = trailingLiveToolCallIds(blocks, lifecycle); + const liveToolCallIds = trailingLiveToolCallIds(blocks, lifecycle, sessionLive); // Pre-compute: is there any non-empty TextDone in this bubble? // Used to drop trailing-empty assistant messages — the server @@ -1374,9 +1465,17 @@ function buildAssistantItems( function trailingLiveToolCallIds( blocks: AnyBlock[], lifecycle: ActiveResponse["state"], + sessionLive = false, ): Set { const callIds = new Set(); - if (lifecycle !== "streaming") return callIds; + // Spin the trailing tool phase when EITHER this bubble is the streaming + // `activeResponse` (in-process harnesses) OR the session is running and this + // is its newest turn (`sessionLive`, for claude-native, whose liveness lives + // in `sessionStatus`). A settled turn — reloaded history, a finished or + // cancelled turn, a dead harness whose session reads idle — passes neither, + // so a result-less tool still resolves to `no-output`, never a perpetual + // spinner. + if (lifecycle !== "streaming" && !sessionLive) return callIds; for (let i = blocks.length - 1; i >= 0; i -= 1) { const b = blocks[i]!; diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index 586a729ac7..601fc85ae5 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -843,7 +843,16 @@ export function ChatPage() { // Both keep the prompt on top across the pending → approved flip. const committed = stripGatedSubagentRoutingChips( reorderCommittedRequestElicitations( - buildBubbles(blocks, activeResponse, bubbleCacheRef.current, interruptedResponseIds), + buildBubbles( + blocks, + activeResponse, + bubbleCacheRef.current, + interruptedResponseIds, + // Spin the newest turn's in-flight tools while the session runs — + // for claude-native, whose running/idle lives in `sessionStatus` + // and never opens a streaming `activeResponse`. + computeIsWorking(sessionStatus), + ), ), subagentRoutingOverride, ); @@ -863,6 +872,7 @@ export function ChatPage() { interruptedResponseIds, pendingUserMessages, subagentRoutingOverride, + sessionStatus, ]); // Picker selection. ChatPage stays mounted across `/` to `/c/:id`,