Repository navigation
fix(native-chat): keep a turn's bar on the prompt that opened it - #23573
Conversation
A message sent while a structured turn runs appears in the transcript at once, so "the newest user message" is not the running turn's owner. The live "Working for" bar moved to the mid-turn message and counted from the earlier prompt's start, and a send queued behind the running turn counted its wait twice: once in the previous turn and again from its own send. Derive both from the host's turn records in one ordered pass: - The running turn's bar belongs to the user message its lifecycle row names (resolved exactly as settled timing resolves it). A message sent mid-turn gets no bar until its own turn opens; a send folded into the running turn never gets one. Surfaces fall back to the latest user message only when the host names no opener. - A turn counts from its send, but never before the previous turn in the journal ended (its recorded end, else its row's last host revision), capped at the turn's own start. The same origin feeds the live counter and the settled duration. Desktop and mobile share the derivation; no wire, host, or storage change.
Review status: ready for reviewHead: Origin: found in live QA of #23537 on desktop and phone. When a message is sent while an earlier turn is still running, the live bar jumped to the new message and counted from the earlier turn's start, and the settled times double-counted the wait. The host journal from that repro is replayed in the new tests. Review: one full review loop came back clean with no changes. It confirmed:
Live QA:
Already on main, not changed here:
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughShared turn timing now records queue boundaries and opener identity. Renderer and mobile chat pass the opener through to turn-status logic and use user-message keys to place active and settled bars. Tests cover overlapping prompts, opener changes, fallback attribution, and timing. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to Queued-turn durations remain anchored to the prompt submission or later, and opener attribution uses transcript message keys. No actionable merge-blocking risk is established. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change appears limited to which chat message displays a turn’s status and duration. No new security capability was identified, but some transcript lifecycle behavior remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 47f6b4e8-b84b-46d0-a14b-5ac96cd101ef
📒 Files selected for processing (22)
mobile/src/session/MobileNativeChatOverlay.tsxmobile/src/session/MobileNativeChatView.tsxmobile/src/session/mobile-native-chat-controller-contract.tsmobile/src/session/use-mobile-native-chat-controller.tsmobile/src/session/use-mobile-native-chat-turn-disclosure.test.tsxmobile/src/session/use-mobile-native-chat-turn-disclosure.tsmobile/src/session/use-mobile-native-chat-turn-status.tsmobile/src/session/use-mobile-structured-agent-turn-timing.tssrc/renderer/src/components/native-chat/NativeChatMessageList.tsxsrc/renderer/src/components/native-chat/NativeChatMessageList.turn-indicator.test.tsxsrc/renderer/src/components/native-chat/NativeChatStructuredSession.tsxsrc/renderer/src/components/native-chat/native-chat-message-rail-items.test.tssrc/renderer/src/components/native-chat/native-chat-rail-outline-parity.test.tssrc/renderer/src/components/native-chat/native-chat-transcript-slots.test.tssrc/renderer/src/components/native-chat/native-chat-transcript-slots.tssrc/renderer/src/components/native-chat/use-native-chat-message-rail.test.tssrc/renderer/src/components/native-chat/use-native-chat-turn-status.tssrc/renderer/src/components/native-chat/use-structured-agent-session.tssrc/renderer/src/components/native-chat/use-structured-agent-turn-timing.tssrc/shared/native-chat-turn-status.tssrc/shared/structured-agent-session-turn-bars.test.tssrc/shared/structured-agent-session-turn-timing.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| // writing a ref during render, which react-freeze can discard. | ||
| turnKey: turnKey && turnStatus?.workedSeconds != null ? turnKey : undefined, | ||
| // With no user boundary at all, the session's working state stays authoritative. | ||
| activeTurnIsWorking: | ||
| enabled && | ||
| isWorking && | ||
| (turnKey === activeTurnKey || | ||
| (turnKey === undefined && activeTurnKey === MOBILE_UNANCHORED_TURN_KEY)) | ||
| activeTurnIsWorking: enabled && isWorking && turnKey === latestTurnKey | ||
| } | ||
| }, | ||
| [turnKeys, enabled, activeTurnKey, active, completedByTurn, expandedTurnIds, isWorking] | ||
| [ | ||
| turnKeys, | ||
| enabled, | ||
| activeTurnKey, | ||
| latestTurnKey, | ||
| active, | ||
| completedByTurn, | ||
| expandedTurnIds, | ||
| isWorking | ||
| ] | ||
| ) | ||
|
|
||
| return { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '75,155p' mobile/src/session/use-mobile-native-chat-turn-disclosure.ts
rg -n 'activeTurnIsWorking|latestTurnKey|UNANCHORED' mobile/src/session/use-mobile-native-chat-turn-disclosure.ts mobile/src/session/MobileNativeChatView.tsxRepository: stablyai/orca
Length of output: 3161
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- focused diff ---'
git diff --unified=35 080c56289863d45bd7cf76325e178273fafa2357 268fc6e066d5c4c0f87ee51a61d9201897f4d684 -- mobile/src/session/use-mobile-native-chat-turn-disclosure.ts
printf '%s\n' '--- consumers and related symbols ---'
rg -n -C 5 'activeTurnIsWorking|useMobileNativeChatTurnDisclosure|useMobileNativeChatTurnStatus|MOBILE_UNANCHORED_TURN_KEY' mobile/src
printf '%s\n' '--- hook outline ---'
ast-grep outline mobile/src/session/use-mobile-native-chat-turn-disclosure.tsRepository: stablyai/orca
Length of output: 38246
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions
Length of output: 25767
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- status hook ---'
cat -n mobile/src/session/use-mobile-native-chat-turn-status.ts | sed -n '1,240p'
printf '%s\n' '--- view call and row render ---'
cat -n mobile/src/session/MobileNativeChatView.tsx | sed -n '245,390p'
cat -n mobile/src/session/MobileNativeChatMessage.tsx | sed -n '95,180p'
printf '%s\n' '--- focused disclosure tests ---'
cat -n mobile/src/session/use-mobile-native-chat-turn-disclosure.test.tsx | sed -n '130,275p'
printf '%s\n' '--- current view tests around grouping ---'
cat -n mobile/src/session/MobileNativeChatView.test.ts | sed -n '680,920p'
printf '%s\n' '--- base disclosure test diff ---'
git diff --unified=12 080c56289863d45bd7cf76325e178273fafa2357 268fc6e066d5c4c0f87ee51a61d9201897f4d684 -- mobile/src/session/use-mobile-native-chat-turn-disclosure.test.tsx mobile/src/session/MobileNativeChatView.test.tsRepository: stablyai/orca
Length of output: 35774
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- shared turn selector and grouping ---'
rg -n -C 12 'selectNativeChatActiveTurnKey|NATIVE_CHAT_UNANCHORED_TURN_KEY|selectNativeChatTurnStatuses' src/shared mobile
printf '%s\n' '--- active tool selector ---'
rg -n -C 12 'function selectActiveToolCall|const selectActiveToolCall|selectActiveToolCall' mobile/srcRepository: stablyai/orca
Length of output: 42003
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- active tool implementation ---'
rg -l 'function selectActiveToolCall|selectActiveToolCall' src/shared mobile | sort
file=$(rg -l 'function selectActiveToolCall|selectActiveToolCall' src/shared mobile | head -n 1)
printf 'selected=%s\n' "$file"
cat -n "$file" | sed -n '1,180p'
printf '%s\n' '--- active tool tests ---'
rg -n -C 12 'selectActiveToolCall|activeTurnIsWorking|running.*tool|live.*tool' src/shared mobile/src --glob '*test*'Repository: stablyai/orca
Length of output: 41876
🏁 Script executed:
#!/bin/bash
set -e
cat -n src/shared/native-chat-tool-activity.ts | sed -n '1,130p'
printf '%s\n' '--- shared mid-turn contract ---'
cat -n src/shared/structured-agent-session-status-tool-line.test.ts | sed -n '100,125p'Repository: stablyai/orca
Length of output: 6208
Keep the running tool row live after a mid-turn send.
If turn A owns a running tool call and the user submits turn B, the tool row remains grouped under A. The new latestTurnKey check marks it non-working because B is latest. selectActiveToolCall then returns null, so MobileNativeChatMessage can hide the collapsed running tool row.
Suggested fix
-import type { NativeChatSettledTurns } from '../../../src/shared/native-chat-turn-status'
+import {
+ NATIVE_CHAT_UNANCHORED_TURN_KEY,
+ type NativeChatSettledTurns
+} from '../../../src/shared/native-chat-turn-status'
...
- const latestTurnKey = turnKeys.at(-1)
const activeActivityText = enabled && isWorking ? (activityText ?? null) : null
...
- activeTurnIsWorking: enabled && isWorking && turnKey === latestTurnKey
+ activeTurnIsWorking:
+ enabled &&
+ isWorking &&
+ (turnKey === activeTurnKey ||
+ (turnKey === undefined && activeTurnKey === NATIVE_CHAT_UNANCHORED_TURN_KEY))
...
- latestTurnKey,![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one edge case in the new owner-resolution fallback.
Reviewed changes
- Turn ownership from the host record — desktop and mobile now take the running turn's bar owner from the host's
userItemId(resolved through the submission alias), so a message sent mid-turn no longer steals the "Working for" bar from the prompt that opened the turn. - Queued-send origin —
readTimingrecordsqueuedUntil(previous turn's end, clamped to this turn's start), andstructuredAgentTurnOrigin/completedStructuredAgentTurnSecondscount from it, so live and settled bars sum to wall time. - Single journal pass —
readStructuredAgentJournalTurnsreplaces the separate settled/run scans, andselectStructuredAgentTurnBarsbecomes the one entry point for settled durations, running timing, and owner. - Mobile liveness split — row liveness follows the transcript's latest prompt (
latestTurnKey), decoupled from the bar's owner. - Consolidated unanchored key + tests —
NATIVE_CHAT_UNANCHORED_TURN_KEYreplaces the duplicated per-surface literals; new shared, desktop, and mobile tests cover the queued, Codex-fold, older-host, and unverifiable cases.
I verified the new shared suite (structured-agent-session-turn-bars.test.ts), the desktop turn-indicator suite, and the mobile disclosure suite pass, and that the new assertions exercise exact durations and placements rather than loose bounds.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| return ( | ||
| activeTurnOpenedBy ?? | ||
| messages.findLast((message) => message.role === 'user')?.id ?? | ||
| NATIVE_CHAT_UNANCHORED_TURN_KEY | ||
| ) |
There was a problem hiding this comment.
activeTurnOpenedBy is returned without checking it is still among messages. Client-side trimming keeps the last 1024 items but the last 256 submissions (structured-agent-session-reducer.ts:53-56,148-169), so a turn's opener item can be trimmed while its turn row and submission survive — then activeTurnOpenedBy resolves through the surviving submission alias to an id no longer in the transcript. No row then satisfies message.id === activeTurnKey, so the live "Working for" bar disappears on both surfaces instead of falling back to the newest prompt (which is what it did before this PR). Reachable in a long session where a mid-turn send's turn only opens >1024 rows after the send.
Technical details
# Fall back when the host-named opener is not in the transcript
## Affected sites
- `src/shared/native-chat-turn-status.ts:116-120` — returns `activeTurnOpenedBy` unconditionally.
- `src/renderer/src/components/native-chat/native-chat-transcript-slots.ts:133-140` — live status only attaches when `message.id === activeTurnKey`, so a non-present key renders no bar.
- `mobile/src/session/use-mobile-native-chat-turn-disclosure.ts:113-118` — same on mobile.
## Required outcome
- When `activeTurnOpenedBy` is non-null but not present among `messages`, fall back to the latest user message (and then to the unanchored key), so a bar is always rendered while the turn works.
## Suggested approach
- Guard the first branch of the `??` chain on membership, e.g. only use `activeTurnOpenedBy` when `messages.some((message) => message.id === activeTurnOpenedBy)`; otherwise let the existing newest-user fallback run. This also absorbs the id-universe divergence where `readStructuredAgentJournalTurns` builds `itemIds` from all items while `projectStructuredAgentSessionMessages` hides rejected ones.
- Consider whether `readStructuredAgentJournalTurns` should validate the alias target against `itemIds` as well, though the client-side membership guard is the behaviourally important one.
## Open questions for the human
- Is a missing opener a case you want to show under the newest loaded prompt, or to leave barless as the PR body's "paged out" note implies? The code currently does the latter, while the note describes the queued message counting from its own send.…essage # Conflicts: # src/renderer/src/components/native-chat/NativeChatMessageList.tsx
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| isWorking && | ||
| (turnKey === activeTurnKey || | ||
| (turnKey === undefined && activeTurnKey === MOBILE_UNANCHORED_TURN_KEY)) | ||
| activeTurnIsWorking: enabled && isWorking && turnKey === latestTurnKey |
There was a problem hiding this comment.
Knowledge Base Used: Mobile sessions and terminal interaction
| const timing = readTiming(item, precedingTurnEndedAt) | ||
| precedingTurnEndedAt = timing?.completedAt ?? item.observedAt |
There was a problem hiding this comment.
item.observedAt is still the time its row was created, not the time of its last revision. For a message queued during that turn, this timestamp usually predates the send, so queuedUntil is omitted and the next turn’s duration includes time spent waiting for recovery.
Knowledge Base Used: Agent workspaces and sessions
| activeTurnOpenedBy?: string | null | ||
| ): string { | ||
| return ( | ||
| activeTurnOpenedBy ?? |
There was a problem hiding this comment.
Knowledge Base Used: Agent workspaces and sessions
|
Update (head |
…3700, #22943, #23720, #23793, #23776, #23796) into #23059 #23573 (keep a turn's bar on the prompt that opened it) is ported onto the turn-record membership instead of kept as a second owner lookup: - The running turn's bar comes from nativeChatTurnMembership, which now returns barTurnKey beside liveTurnKey. Where the host states each row's turn they are the same key: the running root turn's anchor, and a message handed over mid-turn is already scoped to that turn. Where the host states no scope, rows still group by position, so barTurnKey is the user entry the running record names (through the same anchors) while liveTurnKey keeps the positional live group, as #23573 kept row liveness on the newest prompt. - activeTurnOpenedBy, selectStructuredAgentTurnBars' openedBy and selectNativeChatActiveTurnKey are removed; the desktop list, the slot builder and both turn-status hooks read barTurnKey. - The queued-send clock (queuedUntil: a turn counts from the previous turn's end when its send waited behind it) is kept, read in the same one-pass walk over root turn records that keys timing by anchor. - Its tests are adapted to pass a journal: each runs on a host that states no scope (the original expectations), and the desktop and mobile bar tests also run on a scoped host. #23621 (a message Claude folds into its running turn no longer splits the turn) is host-only and merged as is: the translator contract carries both openTurnInLiveProviderCycle and #23059's command-turn members. Both sides grew the Claude translator past its line budget, so its result branch moves unchanged into claude-result-journaling.ts, beside claude-message-journaling.ts (the pre-commit lint refuses the merge without it). #23801's Codex child path (no error-driven end) and #23059's Codex /compact guard (a command turn ends only on turn/completed) both stand.
Where this fits: the plan for messages sent while the agent is working
The problem. In a structured (native) chat, a message you sent while the agent was still working went straight to the agent. What happened next depended on the agent:
Orca then drew the conversation wrong:
The plan. A message sent mid-turn waits as a card above the message box, with three actions:
Otherwise it sends by itself when the turn ends. Only Steer puts a message into a running turn. With queueing turned off in Settings, mid-turn messages still go straight in, as they do today.
The work is split into six PRs:
Steer on Codex also needs Codex's turn-steer support from #21062, which is still open.
The queue stays switched off, meaning the host does not advertise it and every client behaves as today, until PRs 1–6 and #21062 have landed. Turning it on is then a one-line change.
This PR is step 1. It keeps the running turn's bar under the message that opened the turn, as the host records it, instead of under whichever message is newest. It is needed with the queue on (a steered message) and with it off (every mid-turn send).
ELI5
If you send a second message while the agent is still working on your first one, the "Working for" timer now stays under the message the agent is actually working on, and each message's "Worked for" time no longer counts the time it spent waiting in line.
What Changed
Follow-up to #23537, found in live QA of that PR on desktop and phone.
Before: you send prompt A, and a few seconds later, while A is still running, you send message B.
After:
Mechanism: both desktop and mobile picked "the newest user message" as the owner of the running turn, and dated every turn from its own send. A mid-turn send shows in the transcript immediately, so it became the "newest message" while the earlier turn was still running. The host already records, on each turn, which message opened it. The owner now comes from that record, resolved through the same alias lookup the settled durations already used. A turn's clock now starts at its send, or at the end of the previous turn if that is later. The live counter and the settled duration read that same start, so they still agree. Everything is derived on the client from journal rows that already exist: there is no host, wire, or storage change. Hosts that do not name an opener, and chats that are not structured, keep the old "newest message" rule.
Why
The bug came from deciding ownership by position when the host already knows the answer. Reading the host's turn record removes the cause, and it fixes the Codex double bar with the same change.
Alternative considered: hold a mid-turn message out of the transcript as a "queued" row until its turn starts, so the newest message is always the running turn's opener. That does not fit Orca. The host has already delivered the message to the agent, and a message Codex folds into the running turn never opens a turn of its own, so it would stay hidden forever. Orca keeps showing the message immediately and derives its bar and clock from the host's record instead.
Known limits, not introduced here:
Linked Issue
None; found during QA of #23537.
Visual Proof
Live structured Claude chat. Prompt A runs four
sleep 15tool calls, and message B ("Also say the word banana at the end.") is sent a few seconds later while A is still running. Claude ends A's turn after the first sleep and runs the rest in B's turn.Desktop (dev build; before = main, after = this PR):
Before: the live bar sits under B and counts from A's start while A is still running, and A has no bar. B settles at 1m 4s, so the two bars add up to 81 s for about 70 s of real time. After: the bar stays under A ("Working for 10s") while B waits. Sampled every second, A settled in place at 17 s (same position), B's bar started at 0 s, and B settled at 52 s. 17 + 52 matches the time from sending A to the end.
Phone (iOS Simulator, paired to a dev host running this PR, messages sent from the phone):
B waiting ("Working for 12s" under A, nothing under B) → B's turn opens (A "Worked for 17s", B "Working for 0s") → settled (17s and 51s). Each bar expands only its own turn's commands (1 sleep under A, 3 under B), and an ordinary single turn afterward still shows its live bar and settles in place.
Testing
New
structured-agent-session-turn-bars.test.tsreplays the host journal recorded in the live repro. It checks where the bar sits while B waits, A settling at 17 s, B counting from A's end, the two durations summing to wall time, a Codex fold getting a single bar, and unchanged fallbacks for older hosts, provider-opened turns and unverifiable turns. The desktop turn-indicator and mobile turn-disclosure suites cover the same ownership sequence. Reverting the production change fails all the new ownership and timing tests. Desktop native-chat and shared suites pass (2,025 tests), mobile session suites pass (1,836), andpnpm tc, mobiletsc, oxlint, the changed-code quality gate and React Doctor all pass.AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Renderer, mobile and shared client code only. No wire, host, or storage change, so SSH hosts, mixed-version remotes and paired phones need nothing new. Performance: the one pass over the journal replaces the old full pass plus a reverse scan, is memoized on the same inputs, and adds no timers.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)