Skip to content

fix(claude): a restarted provider continues the subagent roster earlier runs journaled - #23758

Merged
brennanb2025 merged 10 commits into
mainfrom
brennanb2025/sta-8138-subagent-resume-id
Sep 30, 2026
Merged

brennanb2025 merged 10 commits into
mainfrom
brennanb2025/sta-8138-subagent-resume-id

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​932 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​926
Prod 18 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​500 $\color{#cf222e}{\Huge{\mathbf{−}}}$​99 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​401

ELI5

What broke. In Orca's Claude chat view, when the Claude process behind a chat restarts in the middle of a conversation (for example after a rate-limit error or a crash), Orca forgot which helper agents ("subagents") that chat had already started. If Claude then resumed one of those helpers, the chat showed that one helper as two: its named entry with nothing new in it, plus an unnamed "Subagent" section (or a second entry) holding the resumed work, with its run count reset. Helpers started outside a turn also lost their names, and a helper Claude reported as "didn't finish before the previous session ended" showed up as an extra unnamed background-task row.

Scope. Only Claude chats in Orca's structured chat view, and only after the Claude process restarts mid-conversation. Terminal Claude sessions, Codex, OpenCode and other agents are unaffected. Rows already written by older builds are left as they are.

Now. A restarted Claude process picks up its helper list from the chat's own saved history, so a resumed helper keeps its name, its single entry and its run count. A resumed helper also starts a fresh run, so its roster row's clock shows how long it actually ran, never the idle time since its last run.

What Changed

Before (mechanism). Orca keeps a "roster" of a chat's helpers: one entry per helper, grouped into one row per turn (plus one row for helpers started outside any turn), the link from the tool call that started a helper to the helper's own id, and how many times each helper has run. That roster lived only in the memory of one Claude process. When a new Claude process started for the same chat, it began empty:

  • A resumed helper's messages still name the ORIGINAL tool call that started it. The new process had no link from that call to the helper's id, so claude-subagent-linkage.ts stamped the helper's rows with the tool call's id instead of the helper's id. That is the unnamed second section.
  • The resume put the helper in a second roster row, and its run count restarted at 1.
  • The row for helpers started outside a turn was rewritten from empty, dropping every helper an earlier process had listed.
  • Claude's own restart notice for a helper it had lost ("didn't finish before the previous session ended") was dropped by the roster and instead became a separate unnamed background-task row.

After (mechanism). A new ClaudeJournaledRoster (src/main/claude/claude-subagent-journaled-roster.ts) re-reads, from the chat's saved history (the session journal), what earlier Claude processes of the same chat left: the tool-call → helper-id links, the roster row that last listed each helper and that row's entries, and each helper's latest run count. It reads once per loaded journal (and again only if the journal is replaced), and never before the journal is loaded. Nothing new is saved: the roster derives this from rows the journal already holds, so it cannot disagree with them. The roster then uses that reading:

  • a resumed helper's rows carry its real id, and it is reopened in its original entry with its run count continued;
  • an earlier row is taken over only when this process's own messages reach it, and taking it over writes nothing;
  • Claude's restart notice now settles the helper's entry as "stopped" (a muted, neutral state, not an error) instead of leaving it "unverifiable" beside a duplicate background-task row; the background-task lane asks the same reading whether a task is a rostered helper;
  • a helper stopped this way keeps the earlier process's stop time, or none if that process died without recording one;
  • a helper an older build listed in two rows resumes in the row it last ran in.

A resumed helper is the same helper on a new run. Resuming keeps the same entry in its original roster row, adds one to its run count, clears its old stop time and resets its start time to the new run's start (reopen in claude-subagent-roster-state.ts). This applies to every resume, with or without a restart. The saved startedAt field is documented as the start of the helper's latest run.

A resumed helper stays where it was started. Its new work shows under its original roster row, pinned where the helper was first started; the turn that resumed it gets no roster row of its own, so one helper is never listed twice.

The roster row's clock counts only running time. The row's duration is now the time covered by its helpers' current runs, with overlaps counted once and idle gaps not counted (summarizeSubagentGroup in src/shared/native-chat-subagent-summary.ts). For helpers that ran in parallel it reads exactly as before. The live clock still ticks while any helper works.

Renderer change. NativeChatSubagentRun.tsx reads that clock and now hides the roster row's run-length clock when any finished helper has no stop time. It already did this for "unverifiable" helpers; a helper "stopped" by the restart notice after its process died is the new case. Without it, the row would show a sibling helper's duration as the whole row's.

User-visible after. After a Claude restart mid-chat: a resumed helper keeps its name and one roster entry, its work sits with it, and its run count continues; helpers started outside a turn keep their names; a helper the previous process lost reads "stopped" in its own entry, with no extra unnamed row; and a resumed helper's clock counts its new run, not the hours since its last one.

Also changes outside a restart (intended).

  • A helper resumed in the same Claude process shows only its new run's time. Before, the idle gap since its first start was included.
  • A roster row whose helpers ran one after another with a gap between them, Claude or Codex, now shows the time they actually ran, not first start to last stop.
  • Resuming a helper restarts its part of the clock: a one-helper row that read "5m" restarts from 0s for the new run, because the row shows each helper's latest run only. Keeping earlier runs on the clock would need a new saved field (each helper's list of runs), which this PR does not add.

Why

Evidence, from real journals on the development machine.

  • 2,057 helper rows in 18 chats were stamped with a raw tool-call id instead of the helper's id.
  • 67 helpers were listed in more than one roster row, with their run counts reset.
  • 34 helpers were dropped from the outside-a-turn row, leaving 545 of their rows without a name.
  • All 54 unnamed background-task rows that duplicate a roster helper are Claude's restart notice.
  • The newest affected row was written on 2026-09-28 by a build whose affected files match main.

One failure class, one mechanism. A first version recalled only the tool-call links. That fixed the ids but left the duplicate roster rows, the reset run counts, the erased outside-a-turn row and the duplicate background-task rows, which all have the same cause. So the roster continues from one reading of the journal, and every part of the fix uses that reading rather than a separate guard.

Alternatives considered.

  • Correct already-written rows when reading them. Every desktop, mobile and worker reader would have to join on the tool-call id, which the data contract forbids. It could also fix at most 709 of the 2,057 rows: chats from before 2026-09-25 saved nothing linking the call to the helper.
  • Rewrite old journals once. Rejected: it rewrites users' history for a label on past chats, with the same 709-row ceiling.
  • Rebuild the whole roster in memory as soon as the journal loads. It removes none of the special cases below, runs into the 32-row limit on groups held in memory (one real chat has 41), and still has to reconcile with messages that arrive before the journal loads.
  • Derive the roster purely from saved rows. The rows do not carry everything the roster tracks (which helpers are backgrounded, the tool calls that started them), and writes are queued, so a message could read a row whose previous write has not landed yet.
  • Keep the roster in memory only. That is the bug.

Why a fresh start on resume. A resumed helper is the same helper doing a new run, so it keeps its identity (entry, row, run count) and measures only that run. Orca's Codex roster already stamps a fresh start when a child runs again in a later turn (codex-subagent-roster.ts:333); the Claude roster now does the same. The row clock follows from it: first start to last stop counted the idle gap as soon as any helper ran twice, while the time the helpers' current runs cover never does and reads the same for ordinary parallel helpers.

Re-reading once per loaded journal follows the existing ClaudeBackgroundTaskIdentityResolver, which also re-derives from the journal whenever it is replaced. Unlike that resolver, this reads the whole journal once rather than one id at a time.

How this compares with other agent chat apps. Where the mechanism matches: Claude's "stopped" notice is recorded as a finished, neutral state (not an error); a resumed helper is the same helper on a new run, its time is its latest run only, and its work stays with the row that started it, which stays beside the turn that created it (a row inherited from an earlier process keeps that turn, taken from its journaled row). Where it differs, each difference is intended:

  • Recovery after a restart (intended). Comparable apps keep this helper state in memory and lose it when the agent process restarts, which is the exact failure Orca users hit (the 2,057 misfiled rows above). Orca re-derives it from the chat's own journal, which already holds every fact needed, so nothing new is stored and it cannot disagree with the rows.
  • One clock per roster row (intended). Comparable apps show a duration per helper only. Orca's roster row shows one duration for its group, so that duration is now the time the helpers' current runs cover: the same per-helper rule (latest run, no idle time), combined without double-counting parallel helpers.

Contract change. The providerParentRef field (the tool call a row's message named) was documented as "re-minted on every resume, never a join key". The first part is false for helper rows, so the comment is corrected. No reader joins on it; now its own producer reads it back to recall which helper an earlier process linked it to. Rows without it simply yield no link.

Performance. Measured on all 1,316 local journals: the first read after a journal loads takes at most 5 ms (a 48 MB journal with 8,260 items); medians are under 2 ms.

Known limitations.

  • If a helper's message reached Orca before the journal loaded, nothing is recalled for it. Not observed: the journals' timing suggests Claude's restart notices arrive after the journal loads, but no captured restart transcript proves it.
  • The 64-helper limit on the outside-a-turn row now applies across the whole chat rather than per process. The largest observed is 55.
  • The run count is a lower bound: a run that wrote no rows is not counted.

Linked Issue

Linear: STA-8138. Follow-up to #23605. Found while restructuring the transcript in #23752, where these rows landed in an unnamed section instead of with their helper.

Visual Proof

Electron QA on a remote macOS machine with real Claude, main (bd5dca4406f) against this branch. The chat's Claude process was restarted by stopping its recorded process id; the next message started a new process resuming the same chat. The resume scenario was run on 1a0e431cf09, re-run on 166fd51fd2d, and re-run on the final head 8f11ab7dcdf with the same roster result; the background-helper screenshots are from 166fd51fd2d (nothing on that path changed since).

A helper resumed after a restart. Before: turn 2 lists the same helper in a second roster row, and its resumed work is captioned only "Subagent". After: one roster entry, and the resumed work is captioned with the helper's name.

Before (main) After (this PR)
Before: duplicate roster row and bare Subagent captions After: one roster entry, resumed work captioned with the helper's name

A background helper still running when Claude restarts. Before: the entry reads "unverifiable", plus a separate unnamed "Background task · stopped" row. After: one named entry reading "stopped" (muted), no extra row.

Before (main) After (this PR)
Before: unverifiable entry plus a separate Background task row After: one entry reading stopped, no extra row

Orca itself killed and relaunched while a background helper ran. After the next message: "stopped", with no run-length clock (the dead process recorded no stop time).

After: stopped with no clock after the app was killed

A resumed helper's clock no longer counts the idle gap (final head 8f11ab7dcdf, resumed 165 s after its first run). Before this change the reopened row read "working · 5m 14s" and settled at "completed · 5m 19s" for about 5 s of work; now it counts the new run from 0s and settles at "completed · 4s".

Before the clock change (idle gap counted) After (new run only)
Before: the reopened row's clock includes the idle gap, 5m 19s After: the resumed helper counts its new run, completed 4s

After: while resumed, the row counts the new run (working 2s)

Testing

  • Automated tests added/updated
  • Manually tested: Electron QA on a remote macOS machine (see Visual Proof)

New tests replay scrubbed real journal sequences with synthetic ids across two Claude processes:

  • the resumed helper keeps its id, one roster row names it, and its rows carry run 2;
  • the outside-a-turn row keeps the earlier process's helper;
  • Claude's restart notice settles the helper as "stopped", adds no background-task row, keeps the earlier stop time, and a later resume still reopens it with run 2;
  • a helper whose process died without a stop time gets none from the notice;
  • a helper listed in two rows resumes in the row it last ran in, in either message order, and survives the older row being evicted;
  • the roster row hides its clock when a finished helper has no stop time;
  • a resume (after a restart and in the same process) resets the helper's start time, keeps its entry and row, and counts run 2;
  • the row's clock counts the union of its helpers' current runs: unchanged for parallel helpers, no idle gap for a resumed or sequential helper, live while working;
  • a control that no id is invented.

Every new test was run red with the code it covers broken and green once restored; the clock tests that pin unchanged behaviour (parallel helpers) pass on both, by design. The clock arithmetic was also checked against a brute-force count on 20,000 random groups and for no jump at settle or at a new helper's start.

Gates: the root typecheck (node, CLI, web); src/main/claude (real-CLI test excluded), the journal, session-wire and native-chat renderer suites: Claude and Codex, journal, session-wire, native-chat renderer and shared suites pass (real-CLI test excluded); oxfmt, oxlint and the changed-code quality gate.

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

  • Cross-platform: no platform-specific code.
  • SSH/remote: host-side only; it runs on the machine that runs Claude and owns the journal.
  • Wire and persistence: no wire change, no schema change, nothing new saved. Older clients read the same rows, now correctly attributed.
  • Mobile: no mobile code change; a "stopped" helper reads "(1 stopped)" in the mobile summary line.
  • Performance: one pass over the journal's in-memory items per loaded journal, at most 5 ms measured.

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)

A backgrounded Claude subagent resumed by a message after its session's provider
restarted parents its frames to its ORIGINAL spawn call, while the announcement the
new provider sees names only the resuming call. The spawn call's alias lived only
in the old provider's memory, so every row the resumed child wrote fell through to
its raw call id: one child shown as two, a named roster entry with no rows and an
unnamed section holding them.

The alias table now also recalls what an earlier run of the session resolved,
re-derived from the agent rows it journaled (canonical id beside the call its frames
arrived under), read once per bound journal epoch. Nothing new is persisted.
…er runs journaled

The roster's state lived in one provider process while everything it writes is
the session's. After a restart a resumed child was re-rostered in a second group
row with its attempts restarted, its resumed frames lost the alias they still
carry, and the one group row no turn owns was rewritten from empty, erasing the
children an earlier run had listed there.

A run now reads what earlier runs journaled, once per bound journal epoch: group
rows give each child's entry and group, agent rows give the spawn-call aliases
and the latest attempt. A group is inherited only when this run's own events
reach it, and inheriting writes nothing. An inherited child is reopened by an
announcement exactly as an in-process resume reopens it, and by nothing else.
The alias recall is one facet of that read. Nothing new is persisted.
… in one row

At restart Claude reports each agent the previous session left running as
stopped ("didn't finish before the previous session ended"). The roster ignored
it, leaving the entry unverifiable, while the background-task lane, which never
saw that agent announced, opened a second, unnamed row for the same agent.

The roster now records the verdict on the inherited entry, keeping the time the
earlier run lost contact, and still reopens the entry on an announcement. The
background-task lane leaves any agent an earlier run rostered to the roster,
from the same journal-derived reading.
…stop time

A journal reopened after its host died leaves a child unverifiable with no stop
time. Stamping Claude's later restart verdict with the current time would show
the whole outage as how long the child ran, so the earlier run's stamp, or its
absence, is kept. The journal-liveness comment no longer describes the roster as
unable to continue from the journal.
An older build could re-roster a resumed child in a later turn's row, so two
group rows list it (67 children in 39 local sessions). Inheriting the second
row re-pointed the child to that row's copy, so the copy the resume had
reopened was left at working and swept to unverifiable, while the outcome
landed on the stale copy. The first row reached keeps the child.
A subagent whose host died without sweeping it has no stop time. After a
restart, Claude's own verdict on it ("stopped") is now recorded, but the rule
that hides the group's duration only covered `unverifiable`, so a mixed group
showed the sibling's duration as the whole group's. The rule now keys on the
missing stop time, whatever the settled state.
…ing chose

A child an older build listed in two rows was placed in whichever row this
run's frames reached first, so a sibling's frame reaching the older row made
the resumed child reopen there. Placement now follows the reading's one
tie-break, and evicting a row drops only placements that point at it.
@brennanb2025

brennanb2025 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor Author

Review status: ready for review

Verdict. The fix works and the review loop is clean. After Claude's process restarts mid-chat, a resumed helper keeps its name, its single roster entry, its real id and its run count; a helper Claude reports as lost reads "stopped" in its own entry with no extra unnamed row; and a resumed helper's clock counts its new run, never the idle time since its last one. Marked ready for review; not merged, auto-merge not enabled.

Decision taken during review: a resumed helper is the same helper on a new run. QA found that after a restart the reopened roster row's clock counted the idle gap ("completed · 5m 19s" for about 5 s of work; real journals show gaps of about an hour typically, up to 22 hours). Resolved in this PR (8f11ab7dcdf): a resume keeps the same entry and row, adds one to the run count, clears the old stop time and resets the start time; the row's clock is the time its helpers' current runs cover, overlaps once and gaps never. Orca's Codex roster already restarts a re-run child's start. Re-verified live: the same resume now reads "working · 0s…3s" and settles at "completed · 4s".

Intended changes outside a restart. A helper resumed in the same process shows only its new run's time. A row whose helpers (Claude or Codex) ran one after another with a gap shows the time they actually ran. Resuming a helper restarts its part of the row's clock (a one-helper row that read "5m" restarts from 0s), because the row shows each helper's latest run; keeping earlier runs would need a new saved field. Parallel helpers read exactly as before. A resumed helper's work stays under the roster row where it was first started; the turn that resumed it gets no second row, by design.

Review loops (4, plus an architecture challenge).

  • Loop 1: Claude's restart notice ("didn't finish before the previous session ended") was dropped by the roster and became a duplicate unnamed background-task row (54 such rows in 29 real chats). Fixed: it settles the entry as "stopped" (muted, not an error), and the background-task lane asks the same journal reading. The earlier process's stop time is kept, or none if that process died.
  • Loop 2: a helper an older build listed in two rows could flip from running to "unverifiable" after it finished; a "stopped" helper with no stop time showed a sibling's duration. Both fixed.
  • Architecture challenge: keep the current design; rebuilding the whole roster at load time or deriving it purely from saved rows was rejected with reasons. Its one suggested change (a helper listed in two rows resumes in the row it last ran in, whatever order messages arrive) is in 166fd51fd2d.
  • Loop 3: no further defects. Loop 4 (after the clock change): clean; the clock arithmetic matched a brute-force count on 20,000 random groups and never jumps at settle or when a helper starts.

Checks at 8f11ab7dcdf (before the merges). Root typecheck passes locally. Claude, Codex, journal, session-wire, native-chat renderer and shared suites pass (the real-CLI test excluded by design). Every new test was run red with its code broken and green restored. oxfmt, oxlint and the changed-code gate pass. The first journal read after a journal loads measured at most 5 ms across 1,316 real journals.

Merged up to current main (2026-09-29), CI green. Two merge commits (no rebase, no force-push). Main's #23059 now files every roster row beside a turn; this branch had moved group creation, so a group it creates takes the open turn's scope and a group inherited from an earlier Claude process keeps the scope its journaled row was created with, carried through the journal reading (each of those three edits was run red with the change removed, green restored). Main's #23613 moved every chat's journal into one database per host; the restart test now opens its journals through the host-database test support. Root typecheck passes; 62 explicit test files (this PR's and main's own for every file both touched), 792 tests pass; CI at 749666b3c10: 22 pass, 13 skipped, 0 failing; mergeable.

Electron QA (remote macOS, real Claude). Main reproduces the bug (duplicate roster row, "Subagent" captions, rows filed under the raw tool-call id). This branch fixes it, including a background helper when only Claude's process dies and when Orca itself is killed and relaunched, and the resumed helper's clock. Screenshots are embedded in the PR body.

Open items decided. Recording Claude's "stopped" verdict: yes, neutral styling. The providerParentRef comment: accurate; no reader joins on it. The background-task duplicate row: fixed with the same journal reading. The 64-helper limit on the outside-a-turn row now applies per chat: acceptable (largest seen: 55).

Not verified. A captured transcript of a real restart proving Claude's first messages always arrive after the journal loads (the journals' timing suggests they do; if not, only that notice falls back to main's behaviour). Mobile UI (no mobile code change; mobile shows counts only).

Not caused by this PR, seen in QA. Killing Claude's process shows no notice in the chat. When Orca itself is force-killed, its Claude process and that process's shell keep running as orphans, and the relaunched Orca resumes the same chat in a second Claude process.

A reopened Claude child now starts a new run: its startedAt is reset to the
reopening frame's time on every reopen path (resume after a restart and a
same-process reactivation). The group clock reads the union of the children's
latest runs, so an idle gap between runs is never counted; an ordinary
overlapping fan-out reads the same as before.
@brennanb2025
brennanb2025 marked this pull request as ready for review September 29, 2026 04:41
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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 configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f5db8ea9-eea6-401d-9d88-a70d7dfc0b30

📥 Commits

Reviewing files that changed from the base of the PR and between 59e31e2 and 749666b.

📒 Files selected for processing (2)
  • src/main/claude/claude-subagent-resume-after-restart.test.ts
  • src/main/native-chat/agent-session-journal/journal-store.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The translator now reads journal linkage to restore Claude subagent aliases, groups, and attempts across provider runs. Resumed entries reopen with a new attempt and start time, while some earlier settlement times remain unchanged during updates. The shared subagent summary calculates elapsed duration from child run intervals, and the renderer hides duration when a non-working child has no recorded settlement time.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 74966

The change adds a journal visitor and restart tests for Claude subagent roster continuity. No merge-blocking issue was found in the reviewed changes.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 74966

The change preserves helper identity within a conversation after restart. No new permission or external access path was identified. Simultaneous restarts and recovery after failed saves remain incompletely established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is attribution and visible lifecycle state within the bound conversation journal. The identified restoration path does not select another session or grant tool capabilities.

Security Findings and Attack Paths

  • inferred — Provider-supplied task identifiers influence restored aliases and roster updates. Traced consumers use them for attribution, grouping, and state correlation, not authorization or tool execution; this path does not establish an introduced privilege-escalation finding.

Trust Boundaries and Controls

  • observed — The new traversal does not perform writes. Durable writes continue through serialized journal operations that check writability and fencing, commit transactionally, and adopt rows only after the database commit.

Resilience and Maintainability Implications

  • observed — The sink latches write failure, reports it through its error callback and barriers, discards queued work, and stops pumping. This provides failure containment, but does not prove atomic recovery between an in-memory roster transition and its durable row.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 47.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 27 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 Claude subagent roster across provider restarts. The wording is somewhat awkward but remains specific and understandable.
Description check ✅ Passed The description is comprehensive and follows the required template. It explains the user impact, mechanism, rationale, alternatives, testing, visual evidence, compatibility considerations, and checkli…
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

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.

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Handles subagent roster recovery after provider restart.

The PR is not yet safe to merge because an inherited Claude roster group can still be lost after eviction.

Findings

  1. P1 Inherited groups cannot be recovered ▶

Summary

The PR restores Claude subagent identity, roster entries, and run counts from saved chat history after a provider restart. It also changes the roster clock to exclude idle gaps. The changes since the previous review merge in the host-wide journal migration; no distinct new actionable issue was established from that integration.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  J[Saved chat journal] --> R[Read prior roster and linkage]
  R --> C[Claim a helper's original group]
  C --> U[Update its entry and run count]
  U --> J
Loading

Reviews (3) · Last reviewed commit: "Merge origin/main into brennanb2025/sta-..."

/** Once per group: after that this run's copy is the newer one. */
private inherit(groupId: string): RosterGroup | null {
const journaled = this.deps.journaled
const entries = journaled?.claimGroup(groupId) ?? null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Inherited groups cannot be recovered When more than 32 groups are admitted, an inherited group can be evicted from memory. Its saved entries have already been consumed by claimGroup, so a later task_started for one of its helpers creates a new entry in the current turn, resets the run count, and leaves the original entry behind. If the evicted group is outside-turn, a later admission can recreate it empty and overwrite its earlier helpers. The inherited group needs to remain recoverable after eviction.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This run read the complete 27-file diff at 8f11ab7 and traced the roster, journal-reading, background-task and clock paths against the current tree.

  • Journaled roster derivation — ClaudeJournaledRoster re-reads the journal once per bound epoch to recover tool-call → agent id aliases, the latest attempt per child, and the group each child last appeared in. claimGroup hands a group over once.
  • Group inheritance — ClaudeSubagentRosterGroups owns the in-memory group map and inherits an earlier run's group on demand; inheritedClaudeSubagentGroup seeds entries as invokedInEarlierRun with the journaled attempt.
  • Resume semantics — reopen advances the attempt, resets startedAt to the new run's start and clears settledAt; revise preserves an earlier run's stop time when a verdict lands on an inherited child.
  • Restart-verdict routing — ClaudeBackgroundTaskRows drops a non-started task already listed in a journaled roster row, so Claude's restart notice settles the roster child instead of opening a duplicate unnamed row.
  • Clock — summarizeSubagentGroup returns a synthetic clockStartedAt yielding the union of the children's latest runs (overlaps once, idle gaps excluded); the renderer hides the clock when any non-working child has no settledAt.
  • Tests — journal-reading, group-placement and two-process resume suites, plus clock arithmetic tests.

Verification: subagentClockStart brute-forced against a union-of-intervals reference with zero mismatches (settled-only and working cases); the new/updated suites pass (journaled-roster, roster-groups, resume-after-restart, roster, shared summary, background-tasks, reading-control, renderer NativeChatSubagentRun); pnpm run check:code-quality:changed reports zero new findings.

ℹ️ Derived-state edges already documented in the PR

Two consequences of "derived, never stored" are worth a reader's awareness but need no change here. attempt is a lower bound — a run that reopens a child without journaling a row for it resets the next epoch's read to 1. And evicting an inherited group at MAX_SUBAGENT_GROUPS runs onEvicted → sweep(group, true), which can write unverifiable into an evicted row's copy of a child still working in a second (legacy two-row) group. Both are noted in the PR body.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Main (#23059) gave every roster group the turn it is written beside. This
branch moved group creation into ClaudeSubagentRosterGroups, so a group this
run creates takes the open turn's scope there, and a group inherited from an
earlier run keeps the scope its journaled row was created with, which the
journal reading now carries.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review at 8f11ab7: a single merge of origin/main (59e31e2) that reconciles the branch with main's per-group turn scope (#23059). The branch's original eight commits are unchanged; the merge's conflict resolution is the only new code, and it is now part of the presented diff.

  • Turn-scope inheritance for journaled rows — JournaledClaudeSubagentGroup now carries { entries, turnScope }, and inheritedClaudeSubagentGroup keeps the journaled row's turnScope, so a group resumed after a restart is revised beside the turn that created it rather than the live turn.
  • Journal reading records the scope — ClaudeJournaledRoster.readJournaledRoster reads attribution.turnScope ?? AGENT_JOURNAL_THREAD_SCOPE from visitItemsWithLinkage, defaulting rows from hosts that predate turn scope to the thread.
  • Live group creation takes the open turn — ClaudeSubagentRosterGroups gained currentTurnScope, wired from ClaudeSubagentRosterDeps.currentTurnScope through createClaudeJournalTranslator, so a group this run creates belongs to the turn open at creation.
  • Tests — the merge adaptation is pinned by a new case in claude-subagent-roster-groups.test.ts ("keeps an inherited row beside the turn that wrote it, and puts a new row beside the open turn") and turn-scope fixtures in the journaled-roster and resume-after-restart suites.

Verification: read the full 27-file diff and the merge's combined (--cc) resolution hunks; ran vitest run over the roster, journaled-roster, roster-groups, resume-after-restart and shared-summary suites — 5 files / 77 tests pass. The adaptation leaves the PR's resumed-roster semantics intact: inherited groups keep their journaled scope, the newly created outside-a-turn group stays thread-scoped, and no prior open threads remain.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Main (#23613) moved every chat's journal into one database per host and
removed the per-chat test opener; the restart test opens its journals through
the host-database test support instead.
@brennanb2025
brennanb2025 merged commit f406874 into main Sep 30, 2026
35 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/sta-8138-subagent-resume-id branch September 30, 2026 06:55
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