Skip to content

fix(sidebar): spin the worktree dot while Claude Code is thinking (#9040) - #10684

Merged
nwparker merged 4 commits into
mainfrom
nwparker/fix-9040-spinner-dot-attribution
Jul 27, 2026
Merged

nwparker merged 4 commits into
mainfrom
nwparker/fix-9040-spinner-dot-attribution

Conversation

@nwparker

@nwparker nwparker commented Jul 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #9040 — while Claude Code was visibly thinking, the left-nav worktree dot sat on the static emerald dot that also means "idle" and "done", instead of the yellow working spinner. So you could not tell which agents were still working.

Root cause. The dot's attribution gate titleStatusIsAgentAttributable() in src/renderer/src/lib/worktree-status.ts accepted a title only when resolveAgentTypeFromTerminalTitle() found a provider token in it. Claude's thinking title is a braille spinner plus free task text — ⠋ implementing the feature — and carries no provider token, so the gate rejected it and the worktree resolved to active instead of working. StatusIndicator.tsx renders active and done with the same bg-emerald-500 dot and only working as the yellow AgentWorkingSpinner ring — which is why the symptom reads as "no thinking indicator" rather than "no dot". Codex and the other providers put their name in the title, so they were unaffected.

That asymmetry is exactly what the reporter observed on 2026-07-22:

The same icons seem to work for Codex but they still don't seem to work properly for Claude code

The fix. When no provider token matches, fall back to the tab's launch identity:

return containsBrailleSpinner(title) && Boolean(launchAgent)

This is not a new heuristic — it mirrors the sidebar row builder, which has used this exact spinner→launchAgent fallback since #9647 (worktree-title-derived-agent-rows.ts:154). The dot and the row now agree instead of disagreeing.

Third commit — a latent trap found while verifying the fix was not inert. resolveTerminalTabActivityStatus (the tab-bar dot) declared its input as tab: Pick<TerminalTab, 'id' | 'title'>. The gate now reads launchAgent, but the type said that field is never consumed. It worked at runtime only because structural typing passes the whole object through (SortableTab hands it a full TerminalTab) — verified by probe, the tab-bar dot does return working. But since launchAgent is optional, re-narrowing that Pick or constructing an input literal would silently revert the tab-bar dot to pre-#9040 behavior with no type error. The Pick is widened to match what the resolver actually consumes, and the tab-bar path gets its own test.

ELI5

Each agent shouts its name while it works, and the little status light listens for a name. Claude doesn't shout its name — it just spins. So the light stayed off for Claude even though it was busy. Now, if we hear spinning and we remember we started Claude in that tab, we turn the light on. We already did this for the list of agents underneath; the light just wasn't listening the same way.

Screenshots

The real StatusIndicator component rendered from the real getWorktreeStatus() return value — not a mockup. One live terminal tab, OSC title ⠋ implementing the feature. The only difference between the first two rows is whether the tab carries a launch identity.

The third row is the control that makes the bug legible: a genuinely idle worktree renders the same emerald dot as the "Before" row. That collision is the defect — the user cannot tell a thinking Claude from an idle worktree.

#9040 worktree dot: before / after / control

Fix proof

The new tests fail without the fix and pass with it. Neutering just the new gate line (return containsBrailleSpinner(title) && Boolean(launchAgent) → return false, i.e. the pre-fix behavior) and re-running:

# src/renderer/src/lib/worktree-status-spinner-launch-agent.test.ts
AssertionError: expected 'active' to be 'working' // Object.is equality
Tests  5 failed | 2 passed (7)

# src/renderer/src/components/tab-bar/terminal-tab-spinner-launch-agent.test.ts
AssertionError: expected 'active' to be 'working' // Object.is equality
Tests  1 failed | 2 passed (3)

With the fix restored, both files pass in full (7 + 3).

The 2 tests per file that pass either way are deliberate negative controls — a spinner with no launch identity stays active, and a non-spinner title with a launch identity stays active. They pin that the gate did not simply become permissive, so they should pass on both branches.

Why the dot and the row now agree

Round-1 review raised "the dot could read working with 0 sidebar rows." I probed both resolvers on identical inputs rather than argue it:

input dot rows
⠋ implementing the feature + launchAgent, layout hydrated working 1
⠋ implementing the feature + launchAgent, no layout working 0
claude [working], no launchAgent, no layout working 0
⠹ codex fix flaky test, no launchAgent working —
⠋ … spinner, no launchAgent active 0

Rows 3 and 4 reproduce on origin/main with no launchAgent involved. "Dot without row" is a pre-existing property of the title heuristic for every provider whose title carries a token — it is what happens before layout hydration, not something this change introduces. The fix extends that already-shipped behavior to Claude, which alone carries no token. Row 1 is the case this PR fixes, and there the dot and the row agree.

That parity is now pinned by a test (#9040 spinner attribution matches named-provider dot/row agreement) with the named-provider path as an in-test control, so a future change cannot silently give Claude worse agreement than Codex gets.

Proof of no regressions

  • pnpm typecheck — exit 0 (all three projects: node, cli, web)
  • Full suites for every directory that owns this code — src/renderer/src/lib/, src/renderer/src/components/sidebar/, src/renderer/src/components/tab-bar/: 545 test files, 4807 passed, 1 skipped, 0 failed
  • oxfmt clean.
  • pnpm lint has a pre-existing failure on origin/main (unlocalized Ghostty keyword strings from Show Terminal and Window settings by default in Appearance pane #10628, commit 912c2495fa) in files this PR does not touch. Nothing in this diff is flagged.

Trade-offs — one real false positive, measured

launchAgent is the tab's launch identity, written when the tab spawns. It usually clears when the agent exits — clearTabLaunchAgent fires from use-tab-agent.ts whenever resolveLaunchedAgentExitEvidence proves exit — but that proof is not available on every path:

exit path clears? why
local, OSC 133;D shell-foreground ✅ title-independent exit evidence
local, agent signal vanishes + shell-process title ✅ !isRemote && hasObservedAgentSignal
any host, completion hook fires ✅ hasCompletedHook
remote, hookless agent ❌ both signal paths are gated on !isRemote, and there is no completion hook

So the honest consequence, narrowed to that last row:

If you launch a hookless agent on a remote/SSH host, it exits without a completion hook, and you then run something else in that same tab whose OSC title carries a braille spinner — pnpm install, vite, cargo, any ora-based CLI — the dot will show working for that non-agent process.

Measured matrix (real getWorktreeStatus()):

scenario status
pnpm spinner title, launchAgent set, live PTY working ← the false positive
pnpm spinner title, no launchAgent, live PTY active
pnpm spinner title, launchAgent set, no live PTY inactive
npm run dev (no spinner), launchAgent set active
⠋ implementing the feature, launchAgent set working ← the fix
VITE v7 ready (no spinner), launchAgent set active

Scope bounds worth noting: it requires all three of an agent-launched tab, a live PTY, and a braille spinner in the title. It cannot fire in a tab you never launched an agent in, and it clears when the PTY dies.

One mitigation applies on the remote side even for the hookless case: web-session-tabs-sync.ts:640 rebuilds the tab from the host snapshot each sync and omits launchAgent when the host stops reporting it ("once the host omits it, don't resurrect stale startup intent"). So the stale-identity window closes as soon as the host drops the surface's launchAgent — the false positive lives only in the gap between agent exit and that snapshot change.

This is not a new trade-off — it is the exact one the sidebar row builder has shipped since #9647. Before this PR the dot and the row disagreed; now they agree. Accepting it means a dot that occasionally over-reports in a tab you did start an agent in; rejecting it means Claude users keep getting no thinking indicator at all, which is the reported bug. I judged the former strictly better, but flagging it explicitly since it is a genuine behavior change and the alternative (clearing launchAgent on agent exit) is a larger change that would also affect native-chat eligibility and cold-restore paths.

Performance

containsBrailleSpinner runs only on the path where resolveAgentTypeFromTerminalTitle() already returned null — i.e. the branch that previously returned false immediately. Measured on the real inputs:

case ns/call
⠋ implementing the feature (the #9040 hit) 6.0
bash (short miss) 13.1
65-char non-spinner title (worst-case miss) 196.2

Worst realistic aggregate — 50 worktrees × 4 tabs, every title a long non-spinner miss — is 0.038 ms added per full status pass. Not measurable in practice.

Cross-platform

Pure string predicate over a Unicode range (U+2800–U+28FF) plus a boolean field read. No paths, no shell, no platform APIs, no keyboard handling — nothing in the diff branches on platform. Behaves identically on macOS, Linux and Windows.

Local, WSL, SSH and relay sessions all reach the same gate, since it consumes OSC titles that already arrive normalized through the shared title pipeline and launchAgent is threaded through the remote transport (remote-runtime-pty-transport.ts:1531) and the web/session tab sync (web-session-tabs-sync.ts:591). Folder workspaces are unaffected — nothing here touches git or worktree layout; folder-workspace-composer-submit.ts:258 sets launchAgent the same way.

Testing

  • pnpm typecheck
  • Unit tests added (10: 7 sidebar/dot + 3 tab-bar) covering the fix, its bounds and its negative controls
  • Full regression run over affected directories (4807 tests)
  • Before/after proof captured by reverting the fix
  • Perf measured
  • Cross-platform reviewed

AI Review Report

Reviewed by two independent frontier sub-agents (GPT-5.6-Sol at xhigh effort, and Fable), looped until clean. 2 rounds. Final verdict: NON-BLOCKING from both.

Round 1 — GPT: BLOCKING (2 claims), both refuted with measurements

# Claim Resolution
a "The dot can spin while the sidebar shows 0 agent rows — a regression introduced by this fix." Refuted. Built a dot-vs-rows probe over identical inputs. claude [working] with no layout snapshot already yields dot=working, rows=0 on origin/main, with no launchAgent involved. The asymmetry is a pre-existing property of the title heuristic for any provider whose title carries a token — buildTitleDerivedAgentRows needs a resolvable UUID leaf id, getWorktreeStatus does not. This change sits exactly at parity. Rather than widen scope, I pinned the invariant as a test (6ae5fed24b).
b "launchAgent is never cleared, so the fallback is permanent." Refuted as stated (too broad) — see round 2, where I was the one who had it wrong.

Round 2 — GPT: NON-BLOCKING (3 nits) · Fable: NON-BLOCKING (1 LOW + 3 nits)

# Finding Action
1 GPT: the collapsed-section projection (visible-worktree-activity-inputs.ts:5, worktree-section-activity.ts:32) also narrows away launchAgent — same class of bug as the tab-bar one I fixed in e841974b42. Verified independently as latent, deferred. getWorktreeSectionTerminalActivityTabs has only test consumers today — WorktreeList.tsx imports the sibling getVisible* functions, not this one. Out of scope for a P2 bug fix; flagged here so whoever activates the collapsed-section summary widens it first.
2 GPT: my trade-off comment claimed launchAgent "is never cleared" — overstated. Correct, and my error. Fixed. clearTabLaunchAgent (store/slices/terminals.ts:1676) fires from use-tab-agent.ts:278 whenever resolveLaunchedAgentExitEvidence proves exit. I had also written the local/remote direction backwards in this PR body. Both corrected — see the trade-off table above, which now names the one path that genuinely does not clear (hookless remote).
3 Fable (LOW): the residual false positive is real but bounded, and matches what the #9647 row builder already shipped. Also confirmed a spinner can only ever reach working, never permission (agent-title-status.ts:167). Accepted and pinned as a test with both bounds (103e9acc7d).
4 Fable: the two new Why comments are 3 lines each vs AGENTS.md's "1 line if possible". Judged borderline-acceptable by the reviewer; kept, as they match the surrounding files' existing multi-line Why idiom.

Independent verification the reviewers performed

  • Mutation testing. Fable neutered the gate to return false and confirmed the new tests actually fail. I re-measured at final HEAD: 6 of 10 tests killed, 4 surviving by design as negative controls (no-launch-identity spinner ×2, non-spinner-with-identity, named-provider control). The mutated file was restored byte-identically — shasum 3b0be7ba… verified by both of us independently.
  • No test asserted the old behavior. Fable grepped every test referencing getWorktreeStatus / resolveWorktreeStatus / resolveTerminalTabActivityStatus and ran them: 5 files / 100 tests, plus pty-connection + orca-runtime at 1355 tests. The nearest pre-existing tests (worktree-status.test.ts:114,124) use tabs without launchAgent, so they exercise the unchanged half of the gate — none pass "for a different reason".
  • All production callers checked. use-worktree-activity-status.ts:14, worktree-section-activity.ts:105, WorktreeJumpPalette.tsx:1747, SortableTab.tsx:94 — every one passes a complete TerminalTab, so the Pick widening is the whole integration.
  • GPT independently re-derived the full dot/row table with its own probe script, including valid-UUID and hydrated/unhydrated layout cases, and reproduced my numbers exactly.

Security Audit

  • Attack surface: none added. The change reads two values already in the renderer store (tab.title, tab.launchAgent) and returns a boolean. No new IPC, no filesystem, no network, no shell, no eval, no dependency added.
  • Untrusted input: tab.title is attacker-influenceable — a terminal program can emit any OSC title, including one containing braille characters. The consequence of a malicious title is bounded to "the status dot spins": a purely cosmetic, non-privileged UI state. It cannot escalate, execute, or exfiltrate. containsBrailleSpinner is a bounded codepoint-range scan with no regex backtracking, so a hostile title cannot cause ReDoS.
  • Identity source: the fallback deliberately requires launchAgent, which is set by Orca when it launches the tab — not derived from terminal output. So a title alone cannot manufacture agent attribution; it must be paired with an identity Orca itself assigned.
  • Third-party code: none introduced. No new imports beyond one existing in-repo shared helper.

Notes

Does not conflict with #9048 (fix: restore worktree status and resource indicators), which is open but last touched 2026-07-20. That PR works at the agent-hook/normalizer layer and shares zero files with this diff. The two are complementary: #9048 fixes status lost at normalization; this fixes status lost at title attribution for the one provider whose title has no name in it.

Per the request that drove this work, do not merge — this is up for review only.

Closes #9040

Made with Orca 🐋

nwparker and others added 4 commits July 25, 2026 21:46
…es (#9040)

The worktree activity dot required a provider token in the terminal title
before it would attribute activity to an agent. Claude's thinking title is a
braille spinner plus free-form task text ("⠋ implementing the feature") with
no provider token, so the gate rejected it and the worktree resolved to
'active' the whole time Claude was working. StatusIndicator renders 'active'
and 'done' with the same emerald dot and only 'working' as the yellow spinner
ring, so the symptom reads as a missing thinking indicator.

The sidebar row builder already solved this in #9647 by falling back to the
tab's launchAgent for spinner-only titles. Thread the same launch identity
into the dot's attribution gate so the dot and the row agree.

The #9647 invariant is preserved: a spinner with no launch identity still
cannot spin the dot, so a frozen spinner frame left behind by an exited agent
does not produce a permanent "working, 0 agents" state.
…t/row agreement (#9040)

Review raised two concerns: a spinner tab could show a yellow dot with no
sidebar row, and a frozen spinner frame could spin forever.

Probing both resolvers against origin/main shows these are pre-existing
properties of the title heuristic, not something this fix introduces:
`claude [working]` with an unhydrated layout already yields dot=working /
rows=0, and `⠹ codex fix flaky test` already spins with no launchAgent at all.

Pin the actual contract instead: Claude's spinner title reaches the same
dot/row agreement a named provider already gets — no better, no worse.
…9040)

resolveTerminalTabActivityStatus narrows its tab with Pick<'id'|'title'>, so the
type said launchAgent is never read even though the status gate now reads it.
Structural typing means the real caller (SortableTab passes a whole TerminalTab)
still worked at runtime, but the contract was wrong: re-narrowing or building an
input object literal would silently drop the tab-bar dot back to pre-#9040
behavior with no type error, because launchAgent is optional.

Widen the Pick to match what the resolver actually consumes, and pin the tab-bar
path with its own test so the regression cannot land silently.
…epts (#9040)

launchAgent usually clears when the agent exits — clearTabLaunchAgent fires from
use-tab-agent.ts once resolveLaunchedAgentExitEvidence proves it via OSC 133;D
shell-foreground, a vanished local agent signal, or a completion hook. But both
signal paths are gated on !isRemote and a hookless remote agent has no completion
hook, so its launch identity can outlive it. A later non-agent process in that tab
whose title carries an ora-style braille spinner (pnpm, vite, cargo) then reads as
working.

That is the trade-off #9647 already made for the sidebar row; this makes the dot
match rather than diverge. Pin it with its two bounds — no launch identity cannot
spin, and a dead PTY drops out — so it stays a deliberate choice under review
instead of drifting into an unexamined default.

Also drop an overstated comment: the gate stops a spinner in a tab no agent was
launched in, not every frozen spinner frame.

Co-authored-by: Orca <help@stably.ai>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 440e1103-3816-489b-9f78-3848e377a3ce

📥 Commits

Reviewing files that changed from the base of the PR and between 438d387 and 103e9ac.

📒 Files selected for processing (4)
  • src/renderer/src/components/tab-bar/terminal-tab-activity-status.ts
  • src/renderer/src/components/tab-bar/terminal-tab-spinner-launch-agent.test.ts
  • src/renderer/src/lib/worktree-status-spinner-launch-agent.test.ts
  • src/renderer/src/lib/worktree-status.ts

📝 Walkthrough

Walkthrough

Terminal activity and worktree status inputs now include launchAgent. Spinner titles are attributed only when a launch identity is present, including pane-title checks. New Vitest coverage verifies Claude spinner handling, missing identities, live and dead PTYs, and consistency between spinner and named-provider sidebar statuses.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: enabling the worktree dot to spin while Claude Code is thinking.
Description check ✅ Passed The description follows the template with summary, screenshots, testing, AI review, security audit, and notes, with substantial detail.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker
nwparker merged commit a065db1 into main Jul 27, 2026
2 checks passed
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] No processing indicators

1 participant