Skip to content

fix(native-chat): a long reply keeps its Working bar and Stop after a reload, from the host's turn - #25751

Merged
brennanb2025 merged 6 commits into
mainfrom
brennanb2025/nc-host-turn-state
Oct 6, 2026
Merged

brennanb2025 merged 6 commits into
mainfrom
brennanb2025/nc-host-turn-state

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 10 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​690 $\color{#cf222e}{\Huge{\mathbf{−}}}$​16 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​674
Prod 23 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​331 $\color{#cf222e}{\Huge{\mathbf{−}}}$​146 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​185

ELI5

If you opened, reloaded, or reconnected a structured chat while the agent was deep in a long reply, the chat could forget that the reply was still running. The sidebar and tab still said Working, but inside the chat:

  • there was no "Working for …" bar;
  • the composer showed Send instead of Stop, so you could not stop a runaway agent from the chat;
  • on an approval or question card, X and Escape did nothing;
  • on the phone, the Stop button was missing while the reply ran, and a card's X answered "Stop not sent".

Now the computer running the agent (the "host") tells the chat directly whether a reply is running. The chat no longer has to work that out from the part of the history it happens to have loaded.

What Changed

The problem. Every chat client decided "is a reply running" by scanning the messages it had loaded for the newest "turn" record (one record per agent reply, updated as the reply runs and ends). Two things break that:

  • A turn record stays at the place in the history where the reply started, even as it is updated.
  • A chat only loads a recent window of history, so a long reply's turn record can fall out of it. Counting the rows (tool calls, messages) the reply has produced since it started:
    • Phone: opening or reconnecting the chat past about 200 rows is enough. The phone loads older history only when you scroll up or tap "Load earlier messages".
    • Desktop: opening, reloading or restarting the chat past about 300 rows (desktop first loads about 300). It also keeps loading older history on its own while the loaded part is shorter than the window, so it only stays lost when those rows draw taller than the window (for example, text between tool calls; a run of back-to-back commands folds into one row and does not trigger it) and you have not scrolled up. Scrolling up loads the start of the reply, and Working and Stop come back.
    • Desktop, under the same height condition: when the host resends the chat from scratch mid-reply (a snapshot or reset) past about 200 rows, or while watching one reply live past 1024 rows.

When that happens, the chat decides nothing is running. The sidebar never had this problem, because the host computes it from the whole history. A new test proves this end to end on the path a phone open or reconnect (and a desktop snapshot or reset) takes: a running reply with 250 rows after its start → host says running, chat said idle.

What you see now, on desktop and phone, however long the reply:

  • The chat shows Working for … with the real elapsed time, on the first loaded row of the reply.
  • The composer shows Stop (on the phone too), and it stops the reply.
  • An approval or question card's X and Escape cancel it.
  • The "Thinking" label shows while the agent is reasoning, as it does for a short reply.
  • When the reply ends, all of that goes away, even if its turn record is still out of the window.

The mechanism.

  • Host: every history page and every live update that carries rows now includes latestTurn, the newest turn record taken from the whole history (its id, state, start time and so on). It is computed once per history snapshot.
  • One shared reader: runningStructuredAgentSessionTurnId answers "which reply is running" from latestTurn. Desktop and mobile both use it for Working, Stop, card cancel, the conversation-command gate, the elapsed-time clock and the "Thinking" label. The old scan of loaded rows remains only for hosts too old to send latestTurn.
  • Never a stale answer: the client keeps latestTurn across updates that carry no rows (nothing about a reply changed). An update that carries rows always restates it, so one that arrives without it came from an older host (for example a remote Orca server that was rolled back while the chat stayed open), and the client drops back to the loaded rows instead of holding the last answer. The step that merges rapid live updates applies the same rule.
  • Turn grouping: while a reply runs, the code that puts transcript rows under their reply also uses latestTurn. That is what lets the Working bar appear when neither the reply's turn record nor your message that started it is loaded. Such a reply shows no "N changed files" total, because only its newest rows are loaded and the total would undercount; once the reply ends, its loaded rows are left ungrouped, as they were before this change.
  • The phone's Stop and card cancel are otherwise unchanged: they name the reply the host says is running.
  • Two files hit the line limit, so pieces moved without changes: the turn-completion feed types moved to their own file, and the submission-retention helper joined the existing retention module.

Why

The common pattern — and Orca's own sidebar — computes "a reply is running", when it started, and what Stop acts on from server-side state that travels with the chat, while only the timeline itself is paged. Following it removes the whole class of bug: whether a reply is running no longer depends on how much history happens to be loaded or how tall it draws on screen. Live QA has not yet reproduced the bug on desktop with a real agent; that QA follows in the review loop.

Alternatives considered:

  • Attach the turn record row to every page. Rejected: the loaded history must be one unbroken run of rows (paging back relies on it), and an extra old row breaks that.
  • Have the chat read the sidebar's status feed. Rejected: it is a separate stream with its own timing, carries no turn id, and carries no start time.
  • Load a bigger window. Rejected: that only moves the threshold.
  • Send latestTurn only when it changes. Rejected: that needs extra bookkeeping per client, to save a few hundred bytes on frames that already carry whole messages.

Differences from the common pattern

  • Intended: latestTurn rides the chat's own live stream rather than a shared thread record. It is computed by the same host from the same history as the sidebar, and it arrives in order with the rows it governs.
  • Temporary: clients still scan loaded rows when the host is too old to send latestTurn. Remove that once such hosts are no longer supported.
  • Temporary: a card's cancel goes through the running reply: it names the host's running turn, because the host's cancel request requires one. So a card that outlives its reply (no reply running) still cannot be cancelled from the card. Follow-up: a host cancel that accepts a prompt by its own id.
  • Temporary: pending approval and question cards, and the check that holds messages behind a running /compact, still read the loaded rows. Both sit at the newest end of the history while they matter, so no case was found where they fall off the page. Follow-up: deliver pending prompts beside the timeline, as the reply state now is.

Compatibility (remote-wire rules):

  • latestTurn is a new optional field. Older clients ignore it.
  • When it is missing (an older host), the new clients fall back to the old behavior. A null value means "no reply recorded".
  • The legacy turn-row rewrite for old clients does not touch it.

Overlap: #25670 and #25675 (drafts) change how a finished or cut-off reply is worded. This PR changes no wording; expect small merge conflicts in use-structured-agent-session.ts and the mobile session hook.

Linked Issue

N/A — maintainer change from a native chat audit

Visual Proof

Live Electron QA (before = main, after = this PR; reload partway through a long reply): #25751 (comment)

before (main): no Working bar, Send, no way to stop after (this PR): Stop shown

Testing

  • New: structured-agent-session-live-turn-window.test.ts (real host history → live subscriber → shared client state). Covers:

    • a long running reply off the page;
    • the reply ending off the page;
    • the next reply opening;
    • an older history page not undoing newer live state;
    • an older host's fallback.
    • With the host field removed, 5 of 7 cases fail.
  • New: NativeChatMessageList.unloaded-live-turn.test.tsx (desktop render): "Working for 1m 3s" draws with neither the turn record nor the opening message loaded, with no partial "changed files" total; without the host record, no bar draws.

  • Added cases to:

    • desktop Stop: use-structured-agent-session-stop.test.tsx;
    • mobile card cancel and plain Stop with the turn record not loaded: use-mobile-structured-agent-session-prompt-cancel.test.tsx;
    • mobile turn bar: use-mobile-native-chat-turn-disclosure.test.tsx;
    • client state: structured-agent-session-reducer.test.ts (kept across row-less updates; an older host's rows drop it) and structured-agent-session-coalescer.test.ts (merging follows the same rule);
    • grouping: native-chat-turn-membership.test.ts (grouped only while running, whole once the record loads);
    • "Thinking": structured-agent-session-live-turn.test.ts.
    • Each new case fails with its fix removed.
  • Existing suites around every touched module: 55 files / 581 tests green (desktop, host and shared), 15 mobile session files / 138 tests green.

  • Typecheck and lint: tc:node, tc:web, tc:cli, mobile typecheck and its test-typecheck ratchet, oxlint, and the changed-code quality and React Doctor gates.

  • The cross-version agent-session wire test could not start locally, because its extracted older-release checkout needs a package this checkout no longer installs. CI covers it.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH and remote hosts: the host computes the value and the client reads it, so local, SSH and paired hosts behave the same.
  • Folder workspaces: unaffected.
  • Mobile: covered.
  • Performance: one backwards scan to the newest turn record per history snapshot, shared across a catch-up's pages, plus a few hundred bytes per row-carrying live update.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…the host's turn when its rows are not loaded
…running turn

Drops the conversation-wide phone Stop: the phone shows Stop only once a turn
is running, so a Stop naming no turn could not be reached and only changed the
ordinary Stop's request. Stop and card cancel still name the turn the host says
is running.
… a half-loaded turn shows no edit totals

- A batch carrying rows restates the host's newest turn record, so one without
  it (an older host after a downgrade) drops back to the loaded rows instead of
  holding the last answer; the event coalescer merges by the same rule.
- A turn whose record is above the loaded rows is grouped only while it runs,
  and its loaded tail gets no "N changed files" total, which would undercount.
- "Thinking" reads the host's running turn too, so it shows when the turn's
  record is not loaded.
- The wire doc for observedAt says what the journal stores: creation time.
…turn-state

# Conflicts:
#	src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx
…ing the transcript list under its line limit

Also drops the chat zoom prop main removed from the unloaded live turn test.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review summary

The problem. A structured chat decides whether a reply is still running by looking through the history it has loaded for the newest "turn" record. That record stays where the reply started, and a chat loads only a recent window of history. After a reload, an app restart or a phone reconnect partway through a long reply, the record can sit outside that window. When it does, the chat decides nothing is running: no "Working for …" bar, Send instead of Stop, and an approval card's X or Escape does nothing. Meanwhile the sidebar still says Working.

When it happens:

  • Phone: opening or reconnecting once a reply has more than about 200 rows.
  • Desktop: opening, reloading or restarting once a reply has more than about 300 rows and those rows draw taller than the window (text between tool calls, for example), as long as you haven't scrolled up. Scrolling up loads older history and brings Stop back.

What changes for you. The computer running the agent now tells the chat directly whether a reply is running, so the chat stops guessing from whatever history happens to be loaded. After a reload partway through a long reply, Stop is there and works, and the Working state and the "Thinking" label follow the real reply. Card X and Escape target the running reply. The phone gets the same fix. Older hosts behave exactly as before.

What review found and fixed

Two rounds of adversarial review, an architecture challenge and a readiness checklist. The architecture review landed on the same design independently: the host computes running state from the whole history, and only the timeline is paged.

  • Fixed:
    • If a remote host was rolled back to an older version while a chat stayed open, the chat could stay stuck on "running". It now drops back to reading its loaded rows.
    • A reply whose start isn't loaded showed an undercounted "N changed files" total. That total is no longer shown for such a reply.
    • The "Thinking" label never showed once the reply's start was off the page. It now follows the host.
    • The PR body's wording and thresholds are corrected.
  • Removed as out of scope: a phone "stop the whole conversation" change. It wasn't needed for this fix, and its promise (Stop before a reply opens) wasn't reachable on the phone.
  • Deferred:
    • A card that outlives its reply still can't be cancelled from the card, because the host's cancel needs a reply id. The follow-up is a host cancel that takes the card's own id.
    • Pending cards and the /compact queue check still read loaded rows.
    • When a reply whose start is off the page ends, it gets no "Worked for" summary or file totals.
    • Smaller nits.

Verified

  • Unit and render tests: new tests on desktop and phone fail without the change. Each review fix was also checked by reverting it.
  • Typechecks and CI: typechecks (node, web, cli, mobile) pass. CI passed on the current head e962eef6440, which merges in current main. The merge only had to keep main's new wrapper around the message list, and move one hook out to stay under the file length limit.
  • Live Electron QA on a remote Mac (on 05d7d01ba7c, before the main merge), with a scripted stand-in agent that runs a long reply (170 text-and-command steps) and keeps it running, then reloads the chat mid-reply:
before (main) after (this PR)
After reload mid-reply no Working bar, Send, no way to stop; reply keeps running Stop shown, "Running a command"
Press Stop — reply stops ("Interrupted"), Send returns

Screenshots, before (main): after the reload, and 30 s later.

Screenshots, after (this PR): after the reload, and after pressing Stop.

Not verified

  • Phone UI: not checked live. The emulator on the QA machine goes through that machine's own running Orca, which was off limits. Phone behavior is covered by unit and render tests only.
  • Approval card X/Escape: not checked live, because the stand-in agent shows no cards. Unit tests cover it.
  • "Working for" clock after a reload: after the reload it isn't on screen without scrolling up. That matches any long reply, where the bar draws at the reply's first row, but its exact position in this run wasn't captured.
  • Earlier QA attempts: they did not reproduce the bug on main, because back-to-back commands fold into one row and the chat loads older history to fill the screen. That is what led to the narrower conditions above.
  • SSH, Windows and Linux: not run live. The change is platform-neutral wire and state code.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 6, 2026 13:52
@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4e6bee27-40e3-4220-8683-b9d7003a193f
📥 Commits

Reviewing files that changed from the base of the PR and between 5cf3585 and e962eef.

📒 Files selected for processing (33)
  • mobile/src/session/mobile-structured-agent-session-cancel.ts
  • mobile/src/session/use-mobile-native-chat-turn-disclosure.test.tsx
  • mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx
  • mobile/src/session/use-mobile-structured-agent-session.ts
  • mobile/src/session/use-mobile-structured-agent-turn-timing.ts
  • mobile/src/session/use-mobile-structured-send-with-outcome.ts
  • src/main/codex/codex-structured-journal-subagent-readers.test.ts
  • src/main/native-chat/agent-session-wire/agent-session-history-page.ts
  • src/main/native-chat/agent-session-wire/agent-session-subscriber-catch-up.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-live-turn-window.test.ts
  • src/renderer/src/components/native-chat/NativeChatMessageList.tsx
  • src/renderer/src/components/native-chat/NativeChatMessageList.unloaded-live-turn.test.tsx
  • src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx
  • src/renderer/src/components/native-chat/native-chat-turn-diffs.ts
  • src/renderer/src/components/native-chat/use-native-chat-transcript-projection.ts
  • src/renderer/src/components/native-chat/use-native-chat-turn-diffs.ts
  • src/renderer/src/components/native-chat/use-native-chat-turn-membership.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-stop.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-transport-state.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session.ts
  • src/renderer/src/components/native-chat/use-structured-agent-turn-timing.ts
  • src/shared/agent-session-turn-completion-wire.ts
  • src/shared/agent-session-wire.ts
  • src/shared/native-chat-turn-membership.test.ts
  • src/shared/native-chat-turn-membership.ts
  • src/shared/structured-agent-session-coalescer.test.ts
  • src/shared/structured-agent-session-coalescer.ts
  • src/shared/structured-agent-session-item-retention.ts
  • src/shared/structured-agent-session-live-turn.test.ts
  • src/shared/structured-agent-session-live-turn.ts
  • src/shared/structured-agent-session-reducer.test.ts
  • src/shared/structured-agent-session-reducer.ts
  • src/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; 7 remain after this review.


📝 Walkthrough

Walkthrough

The host now supplies the latest turn record independently of the loaded journal page. The client carries this value through page reads, live batches, and session state. Turn membership, timing, thinking status, and stop or cancellation checks use it when the turn record is not loaded. Chat rendering also identifies partial turns and excludes them from turn-diff summaries.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to e962e

The latest-turn state is carried through live updates, and no remaining issue is established that would prevent merging after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 33 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: preserving the Working bar and Stop control for long replies after reload by using the host’s turn state. It is specific and related to the changeset.
Description check ✅ Passed The description is detailed and covers the user impact, implementation, rationale, compatibility, visual proof, testing, and cross-platform considerations. It identifies the linked-issue section as no…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@brennanb2025
brennanb2025 merged commit ccab2a3 into main Oct 6, 2026
56 checks passed
Jinwoo-H added a commit that referenced this pull request Oct 6, 2026
…25949)

A test still called isStructuredAgentSessionThinking with a bare array after
#25751 changed it to take { items }, and #24369 left the session host one line
over its max-lines budget. Both fail CI on every PR.
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.

1 participant