Repository navigation
Running-tasks strip and per-subagent Stop from the shared subagent tracker (Grok, Claude Stop) - #27585
Running-tasks strip and per-subagent Stop from the shared subagent tracker (Grok, Claude Stop)#27585brennanb2025 wants to merge 5 commits into
Conversation
…e, Codex and Grok subagents Recorded on unchanged code so the move onto the shared subagent tracker shows each strip or Stop change it makes as a snapshot diff.
… shared subagent tracker The tracker keeps every subagent, listing only the first 64 of a turn in its row, and projects each change for the running-tasks strip after the row write that made it is accepted. Grok's strip, sidebar and Stop now come from that projection instead of the saved row, so a 65th subagent is shown and stoppable. An acknowledged per-subagent Stop is reported to the tracker for Grok and Claude as a provisional stopped that only the stopped run's own ending can replace.
A subagent past a row's limit is let go only once it reports its own ending, so a later verdict still corrects lost contact or an acknowledged Stop; the cap makes room from settled ones. Claude binds an acknowledged Stop to the run current when the CLI acknowledges it, and a subagent row the journal refused is written again once it drains.
Review summaryThe problem. Grok's running-tasks strip, sidebar and per-subagent Stop were built by reading the saved chat row back, and a row lists at most 64 subagents, so a 65th Grok subagent could not be seen or stopped (for the row outside any turn, over the whole chat). Separately, after a per-subagent Stop the agent confirmed, the chat row kept saying "working" until the subagent's own final report, which Claude sometimes never sends. What changes for the user.
Review loop.
Deferred. Claude's tracker does not report re-keyed or removed subagents to the strip (no effect until the follow-up moves Claude's strip onto the tracker; it must handle them first); subagents past the row limit show no "2" suffix in the strip; Grok rewrites an unchanged row to carry an update for a subagent past the limit (same cost as a listed one's update). Verified. Golden tests recorded on unchanged code (strip records, Stop targets and rows per agent), with this PR's snapshot diff limited to the listed changes; new tracker tests; targeted suites (31 files); real Claude CLI suite on the final head; tsc node/web/cli; changed-code quality gate; CI on 830f0d2: 15 pass, 17 path-gated skips, 0 fail. Live QA ran the after build at 46b13e0; 830f0d2 adds one fix with no visible effect (covered by a test). Live QA on hidden test builds, before and after: real Claude, Codex and Grok for one and two helpers, a helper outliving its turn, parent Stop, per-helper Stop, strip/sidebar vs row, quit and reopen: all the same on both builds, no regressions. Each real agent sent its own stop report within ~0.4 s, so "stopped on confirmation" has no visible window with them. A labelled stand-in Grok showed both intended changes: 66 helpers all in the strip with Stop (64 before), and a stopped helper reading "stopped" ~1 s after the confirmation (before: ~45 s later, at its own end). A first stand-in run seemed to miss the second change; the stand-in had answered Stop in the wrong shape, which Orca rejects (reproduced in a test), and the corrected run passed. Not verified. SSH/remote, Windows and Linux live runs (no platform or wire changes; source review only). Codex live behaviour is unchanged by design and was checked only as a no-difference row. |
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to If Claude restarts after acknowledging a helper Stop but before the helper reports its outcome, the helper may remain incorrectly marked stopped. This is a bounded issue that warrants a fix or explicit acceptance before merge. Pre-merge checks |
|
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:
12fbbc33-2ea2-43e6-869e-e8c89e80a2fd
⛔ Files ignored due to path filters (4)
src/main/acp/__snapshots__/acp-subagent-child-work-characterization.test.ts.snapis excluded by!**/*.snapsrc/main/acp/__snapshots__/acp-subagent-timeline-characterization.test.ts.snapis excluded by!**/*.snapsrc/main/claude/__snapshots__/claude-subagent-child-work-characterization.test.ts.snapis excluded by!**/*.snapsrc/main/codex/__snapshots__/codex-subagent-child-work-characterization.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (27)
src/main/acp/acp-structured-child-session-end.test.tssrc/main/acp/acp-structured-child-stop.tssrc/main/acp/acp-structured-child-work-admission.test.tssrc/main/acp/acp-structured-child-work.test.tssrc/main/acp/acp-subagent-child-work-characterization.test-fixture.tssrc/main/acp/acp-subagent-child-work-characterization.test.tssrc/main/acp/acp-subagent-child-work.tssrc/main/acp/acp-subagent-timeline-characterization.test.tssrc/main/acp/acp-subagent-timeline.tssrc/main/acp/acp-timeline-event.tssrc/main/acp/acp-timeline-translator.tssrc/main/claude/claude-journal-translator-contract.tssrc/main/claude/claude-structured-control-actions.tssrc/main/claude/claude-structured-journal-translation.tssrc/main/claude/claude-subagent-child-work-characterization.test.tssrc/main/claude/claude-subagent-roster.tssrc/main/codex/codex-subagent-child-work-characterization.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.tssrc/main/native-chat/subagent-tracker/subagent-child-work-characterization.test-fixture.tssrc/main/native-chat/subagent-tracker/subagent-child-work.tssrc/main/native-chat/subagent-tracker/subagent-row-writes.tssrc/main/native-chat/subagent-tracker/subagent-tracker-child-work.test.tssrc/main/native-chat/subagent-tracker/subagent-tracker-entries.tssrc/main/native-chat/subagent-tracker/subagent-tracker-groups.tssrc/main/native-chat/subagent-tracker/subagent-tracker-types.tssrc/main/native-chat/subagent-tracker/subagent-tracker.tssrc/shared/native-chat-subagent-summary.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| /** The provider acknowledged a Stop of this child's `run` (null: whichever is current). It shows | ||
| * stopped at once; the acknowledgement can precede the run's own ending, which still replaces it, | ||
| * and nothing else can. An acknowledgement for a run since superseded changes nothing. */ | ||
| stopAcknowledged(id: string, run: string | null = null): StructuredAgentSessionSinkAdmission { | ||
| const located = this.groups.locate(id) | ||
| if (!located || (run !== null && run !== located.tracked.run)) { | ||
| return ADMITTED | ||
| } | ||
| const stopped = acknowledgedStop(located.tracked, this.now()) | ||
| if (!stopped) { | ||
| return ADMITTED | ||
| } | ||
| located.group.entries.set(id, stopped) | ||
| return this.write(located.group) | ||
| } | ||
|
|
||
| /** Rows a sink refused that no later report rewrote, written again once it drains. */ | ||
| retryRefused(): StructuredAgentSessionSinkAdmission { | ||
| return this.write(...[...this.groups.values()].filter((group) => group.lastSerialized === null)) | ||
| } | ||
|
|
||
| /** What changed for the strip since the last drain, in the order the rows were written. */ | ||
| drainChildWork(): SubagentChildWorkChange[] { | ||
| return this.childWork.splice(0) | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '125,165p' src/main/native-chat/subagent-tracker/subagent-tracker.ts
sed -n '45,90p' src/main/native-chat/subagent-tracker/subagent-tracker-entries.ts
sed -n '180,220p' src/main/native-chat/subagent-tracker/subagent-tracker-groups.tsRepository: stablyai/orca
Length of output: 5124
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- tracker files ---'
rg -n -F --glob 'src/main/native-chat/subagent-tracker/**' \
-e 'stopAcknowledged' -e 'subagentGroupJournalBody' -e 'journaled' -e 'inherit(' -e 'report' src/main/native-chat/subagent-tracker
printf '%s\n' '--- Claude Stop bindings ---'
rg -n -F --glob 'src/**' \
-e 'stopAcknowledged(' -e 'acknowledgedStop' -e 'Claude' src/main/native-chat | head -240
printf '%s\n' '--- journal serialization and types ---'
rg -n -F --glob 'src/main/native-chat/subagent-tracker/**' \
-e 'function subagentGroupJournalBody' -e 'interface TrackedSubagent' -e 'type TrackedSubagent' -e 'NativeChatSubagentEntry' -e 'SubagentGroupJournal' -e 'claimGroup' -e 'report('
printf '%s\n' '--- changed diff ---'
git diff --unified=35 3c1141f23372d26abe9a66bd4074c61139b34175 -- \
src/main/native-chat/subagent-tracker src/main/native-chat | sed -n '1,1200p'Repository: stablyai/orca
Length of output: 42809
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- tracker files ---'
rg -n -F --glob 'src/main/native-chat/subagent-tracker/**' -e 'stopAcknowledged' -e 'subagentGroupJournalBody' -e 'journaled' -e 'inherit(' -e 'report' src/main/native-chat/subagent-tracker
printf '%s\n' '--- Claude Stop bindings ---'
rg -n -F --glob 'src/**' -e 'stopAcknowledged(' -e 'acknowledgedStop' src/main/native-chat
printf '%s\n' '--- journal serialization and types ---'
rg -n -F --glob 'src/main/native-chat/subagent-tracker/**' -e 'function subagentGroupJournalBody' -e 'interface TrackedSubagent' -e 'type TrackedSubagent' -e 'NativeChatSubagentEntry' -e 'SubagentGroupJournal' -e 'claimGroup' -e 'report('
printf '%s\n' '--- changed diff ---'
git diff --unified=35 3c1141f23372d26abe9a66bd4074c61139b34175 -- src/main/native-chat/subagent-tracker src/main/native-chat | sed -n '1,1200p'Repository: stablyai/orca
Length of output: 43050
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- all tracker construction and stop calls ---'
rg -n -F --glob 'src/**' -e 'new SubagentTracker' -e '.stopAcknowledged(' -e 'stopAcknowledged(' -e 'acknowledgedStop' src
printf '%s\n' '--- journal row serialization ---'
sed -n '1,180p' src/main/native-chat/agent-session-journal/journal-subagent-group-body.ts
sed -n '1,90p' src/main/native-chat/subagent-tracker/subagent-row-writes.ts
printf '%s\n' '--- complete tracker report/revise/state path ---'
sed -n '55,115p' src/main/native-chat/subagent-tracker/subagent-tracker.ts
sed -n '185,380p' src/main/native-chat/subagent-tracker/subagent-tracker.ts
printf '%s\n' '--- Claude-specific surrounding matches ---'
rg -n -i --glob '*.{ts,tsx}' 'claude.*stop|stop.*claude|claude.*subagent|subagent.*claude' src/main src/shared | head -240Repository: stablyai/orca
Length of output: 41167
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- Claude roster construction and stop ---'
sed -n '1,235p' src/main/claude/claude-subagent-roster.ts
printf '%s\n' '--- Claude journaled roster ---'
sed -n '1,280p' src/main/claude/claude-subagent-journaled-roster.ts
printf '%s\n' '--- Claude translation and control call sites ---'
sed -n '270,325p' src/main/claude/claude-structured-journal-translation.ts
sed -n '55,100p' src/main/claude/claude-structured-control-actions.ts
printf '%s\n' '--- restart and Stop-race test assertions ---'
rg -n -F --glob 'src/main/claude/*test.ts' -e 'stopped' -e 'restart' -e 'stopAcknowledged' -e 'before' src/main/claude/claude-subagent-resume-after-restart.test.ts src/main/claude/claude-stop-ack-race.test.ts src/main/claude/claude-subagent-child-work-characterization.test.ts
sed -n '1,280p' src/main/claude/claude-subagent-resume-after-restart.test.tsRepository: stablyai/orca
Length of output: 31287
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- exact state transition and entry schema ---'
rg -n -F --glob 'src/shared/**' -e 'function canReplaceSubagentState' -e 'canReplaceSubagentState' -e 'isSubagentVerdict' -e 'isTerminalSubagentState' -e 'export type NativeChatSubagentEntry' -e 'type NativeChatSubagentEntry'
sed -n '1,180p' src/shared/native-chat-subagent-summary.ts
rg -n -F --glob 'src/shared/native-chat-types.ts' -e 'NativeChatSubagentEntry'
sed -n '1,120p' src/shared/native-chat-types.ts
printf '%s\n' '--- journaled group reader ---'
sed -n '1,260p' src/main/native-chat/subagent-tracker/journaled-subagent-groups.ts
printf '%s\n' '--- exact restart test blocks around stopped rows ---'
nl -ba src/main/claude/claude-subagent-resume-after-restart.test.ts | sed -n '350,500p'
printf '%s\n' '--- exact tracker hydration and stop transition ---'
nl -ba src/main/native-chat/subagent-tracker/subagent-tracker-entries.ts | sed -n '1,120p'
nl -ba src/main/native-chat/subagent-tracker/subagent-tracker-groups.ts | sed -n '175,220p'Repository: stablyai/orca
Length of output: 32097
Persist the provisional Stop marker across hydration.
Claude records stopAcknowledged only in TrackedSubagent. The journal serializes tracked.entry, and hydration resets the flag to false. After a restart, stopped is therefore treated as a final verdict. canReplaceSubagentState then rejects the child’s later completed or failed report, so the row remains incorrectly stopped.
Suggested fix
diff --git a/src/shared/native-chat-types.ts b/src/shared/native-chat-types.ts
@@
export type NativeChatSubagentEntry = {
+ /** True when `stopped` came from an acknowledged Stop and remains provisional. */
+ stopAcknowledged?: boolean
}
diff --git a/src/main/native-chat/subagent-tracker/subagent-tracker-entries.ts b/src/main/native-chat/subagent-tracker/subagent-tracker-entries.ts
@@
export function acknowledgedStop(tracked: TrackedSubagent, now: number): TrackedSubagent | null {
const stopped = tracked.stopAcknowledged ? null : withSubagentState(tracked, 'stopped', now)
- return stopped ? { ...stopped, stopAcknowledged: true } : null
+ return stopped
+ ? {
+ ...stopped,
+ entry: { ...stopped.entry, stopAcknowledged: true },
+ stopAcknowledged: true
+ }
+ : null
}
@@
if (tracked.stopAcknowledged) {
// The run's own verdict confirms or corrects an acknowledged Stop, keeping when it ended;
// nothing else may move it.
- return isSubagentVerdict(state)
- ? { ...tracked, stopAcknowledged: false, entry: { ...tracked.entry, state } }
- : null
+ if (!isSubagentVerdict(state)) {
+ return null
+ }
+ const { stopAcknowledged: _provisional, ...entry } = tracked.entry
+ return { ...tracked, stopAcknowledged: false, entry: { ...entry, state } }
}diff --git a/src/main/native-chat/subagent-tracker/subagent-tracker-groups.ts b/src/main/native-chat/subagent-tracker/subagent-tracker-groups.ts
@@
- stopAcknowledged: false
+ stopAcknowledged: entry.stopAcknowledged === true|
Closing without merging. Review found that saving subagents as one grouped row per turn, rewritten on every change, is what forces the per-row limit and most of the extra code here. The replacement saves one record per subagent, which removes those limits; it comes in two new PRs that will link back here. |
ELI5
When an agent in native chat starts helper agents ("subagents"), Orca shows them in two places: a row in the chat that lists them, and the running-tasks strip above the message box (also the sidebar), where each running helper has its own Stop button. PR #27228 put the chat rows for Claude, Codex and Grok on one shared subagent tracker. This PR starts feeding the strip and Stop from that same tracker, so the strip no longer loses helpers the chat row has no room for, and a helper you stop reads "stopped" in the chat as soon as the agent confirms the Stop.
What Changed
The problems.
What you see now.
The mechanism. All in
src/main/native-chat/subagent-tracker/:subagent-child-work.ts). After a row write is accepted, the tracker reports each helper whose state, name, run, token count or Stop status changed. Grok publishes those changes only when the row carrying them reaches the chat's saved history, as it already did, so the strip never runs ahead of the chat. When only unlisted helpers changed, the unchanged row is written again to carry the update in order.stopAcknowledged) the same way for Grok and Claude. Each agent still makes its own Stop request to its own provider. The "stopped" it records is provisional: only the stopped run's own final report can replace it, which matches how the host's records already treated an acknowledged Stop. If the journal is busy and refuses that row write, Claude writes it again once the journal catches up.Why
The tracker already decides what each helper's row says. Building the strip from the same entries removes the second copy of that state, which is where Grok's limit came from. Building it from the saved row would keep tying what you can stop to what fits in the row. The common pattern keeps one record per helper that the chat reads, and does not limit running helpers by count. This PR follows it, with these differences:
Considered and rejected: moving all three agents in one PR. Claude's change alters when its strip shows a helper as finished, which needs its own before and after check; doing both at once would hide which part caused a regression.
Linked Issue
N/A (follow-up to #27228, which listed this as its temporary item).
Visual Proof
Hidden test builds of Orca, before =
main(3c1141f), after = this PR. Both images use a stand-in Grok (a small proxy in front of the real Grok that turns one prompt into 66 background helpers and answers their Stop like Grok; a helper's own end report arrives 45 s after its Stop). Real Grok, Claude and Codex could not show these: none can start 66 helpers on demand, and each sent its own stop report within a fraction of a second, so before and after look the same with them (see Testing). Every screenshot was checked by hand against the time of the action it shows.1. 66 helpers in one turn (stand-in Grok). Before, the running-tasks strip lists 64 and helpers 65 and 66 have no entry and no Stop. After, it lists all 66, each with Stop. The chat row lists 64 on both.
2. One second after pressing Stop on Helper 10 (stand-in Grok confirms the Stop at once). Before, the row still says "working" and the strip still lists it (it changes only when the helper's own end arrives, 45 s later). After, the row says "stopped" ("63 working +1 stopped") and it has left the strip; its siblings keep running.
Testing
subagent-tracker-child-work.test.ts): helpers past the row limit (including one that lost contact or was stopped, then reported its own ending), the 256 cap and making room in it, refused writes and their retry, restarts, earlier-process helpers never shown as running, and the provisional Stop.orca-ci-checks: passes apart from its local bun typecheck runner, which fails here on a tooling error ("Script not found check");tscfor node, web and CLI passes through the normal queue.Notes