feat(sidebar): reveal nested subagent threads#3861
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 3 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
| : rootProjectThreads.filter((thread) => | ||
| visibleRootThreadKeys.has(sidebarThreadKey(thread)), | ||
| ); | ||
| return flattenSidebarSubagentTree({ |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:3693
The sidebar thread preview limit (sidebarThreadPreviewCount) is applied to root threads only, not to the final flattened row list, so a project can render far more visible rows than the configured limit. When subagent branches are expanded, flattenSidebarSubagentTree expands each previewed root into all of its visible descendants, so a project configured to show 3 visible threads can render 3 roots plus every expanded descendant. This breaks the "Visible threads" setting and skews downstream features like jump-hint numbering and sidebar prewarming, which now treat those extra rows as visible. The preview slicing should be applied after flattening (or the flatten result should be truncated to sidebarThreadPreviewCount) so the total visible row count respects the configured limit.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 3693:
The sidebar thread preview limit (`sidebarThreadPreviewCount`) is applied to root threads only, not to the final flattened row list, so a project can render far more visible rows than the configured limit. When subagent branches are expanded, `flattenSidebarSubagentTree` expands each previewed root into all of its visible descendants, so a project configured to show 3 visible threads can render 3 roots plus every expanded descendant. This breaks the "Visible threads" setting and skews downstream features like jump-hint numbering and sidebar prewarming, which now treat those extra rows as visible. The preview slicing should be applied after flattening (or the flatten result should be truncated to `sidebarThreadPreviewCount`) so the total visible row count respects the configured limit.
There was a problem hiding this comment.
This is intentional: the "Visible threads" setting controls how many conversations (root threads) are previewed per project, and subagent children only render when the user explicitly expands a branch — so the extra rows are opt-in rather than a regression of the limit. Truncating the flattened list instead would cut a tree mid-branch, leaving a parent marked expanded with some of its children silently missing. Prewarming and jump hints are independently capped, so expanded rows don't create unbounded work downstream.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
|
Note on the failing Macroscope - Effect Service Conventions check: its own agent log concludes this PR is frontend React code and pure helpers with no Effect-service changes to flag, but the run then fails on an output-format conflict inside the check-run agent (JSON schema vs a literal "All clear" reply — the activity log cuts off mid-sentence). The failure is a check-tooling issue on this repo's |
Extract the environment-scoped subagent tree projection with cycle and orphan fallbacks so every client renders the same rows. The web sidebar adopts it separately in pingdotgg#3861; this copy carries only the shared helpers the mobile tree needs.
Web half of the owned-subtree lifecycle work: sidebar thread actions archive, unarchive, and delete a root together with its recursively owned subagent threads, using the shared subtree helpers and confirmation copy from client-runtime, with an app setting to skip the archive confirmation. The client-runtime helpers are byte-identical with the mobile-subagent-tree branch so the shared file merges cleanly in either order.
github-actions
Bot
added
size:XXL
| scopedThreadKey(scopeThreadRef(thread.environmentId, thread.id)), | ||
| ? rootProjectThreads | ||
| : rootProjectThreads.slice(0, sidebarThreadPreviewCount); | ||
| const visibleRootThreadKeys = new Set( |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:3804
When the current route points at a nested subagent thread inside a collapsed project, pinnedCollapsedThread walks up to the topmost ancestor and only that root key is added to visibleSidebarThreadKeys. The active child thread's own key is missing from the list, so resolveAdjacentThreadId cannot find currentThreadId and thread.previous / thread.next return null from that route. Consider including the active thread key itself (and its ancestor chain) in the visible set when a pinned-collapsed thread is used.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 3804:
When the current route points at a nested subagent thread inside a collapsed project, `pinnedCollapsedThread` walks up to the topmost ancestor and only that root key is added to `visibleSidebarThreadKeys`. The active child thread's own key is missing from the list, so `resolveAdjacentThreadId` cannot find `currentThreadId` and `thread.previous` / `thread.next` return `null` from that route. Consider including the active thread key itself (and its ancestor chain) in the visible set when a pinned-collapsed thread is used.
Review follow-ups: deleting a root now stops sessions and closes terminals for every thread in the owned subtree rather than only the root; the environment-thread fallback read merges loaded archived shells so subtree confirmations are not undercounted before the snapshot arrives; and sidebar descendant counts and recovery-root badges are computed over unarchived threads only, matching the archive confirmation flow.
| const archived = readArchivedThreadShells(environmentId) | ||
| .filter((thread) => !activeIds.has(thread.id)) | ||
| .map((thread) => presentThreadShell(environmentId, thread)); | ||
| return [...active, ...archived]; |
There was a problem hiding this comment.
Stale archived cache skews subtree
Medium Severity
When the environment shell snapshot is unavailable, readEnvironmentThreads merges archived threads only from readArchivedThreadShells, which returns an empty list until that archived snapshot atom has already succeeded. Subtree reads for delete/archive confirmations and the new per-subtree stopThreadSession / closeTerminal loops can then omit archived owned subagents, so dialogs undercount and client teardown skips them even if the server still removes the tree.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 83f95f5. Configure here.
| renderedThreads.map((row) => { | ||
| const { thread } = row; | ||
| const threadKey = scopedThreadKey(scopeThreadRef(thread.environmentId, thread.id)); | ||
| const subagentDescendants = getOwnedSubagentDescendants(unarchivedProjectThreads, thread); |
There was a problem hiding this comment.
🟡 Medium components/Sidebar.tsx:1155
getOwnedSubagentDescendants is called with unarchivedProjectThreads, which is scoped to the current sidebar project. Subagent lineage is environment-wide, so a parent thread can own descendants in other projects. This causes subagentDescendantCount to undercount the subtree, but SidebarThreadRow still calls attemptArchiveThread(threadRef, { confirmed: true }) with that count. The Confirm +N UI therefore discloses fewer threads than the archive operation actually affects, so the user can archive more descendant threads than the confirmation message showed. Consider passing the full set of environment-wide thread shells to getOwnedSubagentDescendants so the count matches the archive scope.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/Sidebar.tsx around line 1155:
`getOwnedSubagentDescendants` is called with `unarchivedProjectThreads`, which is scoped to the current sidebar project. Subagent lineage is environment-wide, so a parent thread can own descendants in other projects. This causes `subagentDescendantCount` to undercount the subtree, but `SidebarThreadRow` still calls `attemptArchiveThread(threadRef, { confirmed: true })` with that count. The `Confirm +N` UI therefore discloses fewer threads than the archive operation actually affects, so the user can archive more descendant threads than the confirmation message showed. Consider passing the full set of environment-wide thread shells to `getOwnedSubagentDescendants` so the count matches the archive scope.
Archive, delete, and unarchive now dispatch a command for every thread in the owned subagent subtree instead of only the selected root, so the recursive behavior promised by the confirmation copy holds without any server-side changes. Unarchive matches descendants archived together with the root using a small timestamp tolerance, since each member is archived by its own command.
The recursive lifecycle for owned subagent subtrees is enforced server-side in this PR: one command per root, one timestamp per cascade, run cancellation included. Archive, delete, and unarchive go back to a single root dispatch, and "archived with it" matching returns to exact timestamp equality, which the server-side cascade guarantees.
Applying archive, unarchive, or delete to a thread now cascades through the subagent threads it recursively owns: the orchestrator walks the owned subtree (OwnedSubagentTree), applies the lifecycle transition to every member with a single timestamp, and cancels their active runs and pending attempts. Half-applied cascades are repaired on startup (OwnedSubagentLifecycleRepair), and checkpoints orphaned by deleted threads are cleaned up (CheckpointCleanupService).
PixPMusic
force-pushed
the
pixpmusic/subagent-sidebar-threads
branch
from
50628fd to
c050552
Compare
Mobile UI for the owned-subtree lifecycle cascade in pingdotgg#3861: thread list actions archive, unarchive, and delete a root together with its recursively owned subagent threads using the shared subtree helpers and confirmation copy from client-runtime, and thread lists flag recovery roots whose subagent parent is no longer available. Depends on the server-side cascade in pingdotgg#3861.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c050552. Configure here.
| const descendants = ownedSubagentDescendants(command.threadId, [ | ||
| ...shell.threads, | ||
| ...shell.archivedThreads, | ||
| ]); |
There was a problem hiding this comment.
Root cascade skips orphan subagents
Medium Severity
dispatchOwnedThreadLifecycle builds the owned subtree only from active and archived shell threads via ownedSubagentDescendants. That BFS starts at the command thread and follows subagent parent links present in that list. A descendant whose direct parent was already deleted is not linked from the root, so it is omitted from archive, delete, and related cleanup even when its lineage.rootThreadId still points at the root being mutated.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c050552. Configure here.
1 participant
Summary
Validation
vp checkvp run typecheckvp test apps/web/src/components/Sidebar.logic.test.ts packages/client-runtime/src/state/threadRelationships.test.ts packages/contracts/src/settings.test.ts apps/desktop/src/settings/DesktopClientSettings.test.tsStacked on #2829.
Note
High Risk
Large orchestration changes to thread lifecycle, event commit guards, and checkpoint deletion affect core concurrency and data cleanup; incorrect guards could drop events or over-delete refs.
Overview
Orchestration v2 now treats archive, unarchive, and delete as owned-subagent subtree operations: the orchestrator walks
ownedSubagentDescendants, applies lifecycle mutations with coordinated timestamps, cancels in-flight runs/attempts/nodes/subagents/provider turns, detaches sessions, and enqueues terminal, attachment, andcheckpoint.cleanupeffects on delete. Commands run under a root-graph lock (lineagerootThreadId) alongside per-thread locks; archive/delete cancel process-bound effects across all affected thread IDs. Startup and livethread.createdrepair (planOwnedSubagentLifecycleRepairs) fixes parent/child lifecycle mismatches. Unarchive blocks when the parent is archived/deleted; archiving a subagent cancels the parent’s active subagent task artifacts.EventSink
writeIfRunCurrentis stricter: target threads must be active (not archived/deleted),expectedStatuscan be an array, andallowSupersededAttemptTerminalArtifactsadmits late terminal provider-turn / interrupt-result writes from superseded attempts. Routed events into archived child threads are rejected. Provider turn start and run execution detach sessions and gate writes when threads go inactive mid-flight.New
CheckpointCleanupServiceanddeleteScopeRefs(through latest run/checkpoint ordinal) remove checkpoint refs when threads are deleted.getThreadLifecycleRecordskeeps deleted thread headers for repair.Web sidebar logic adds nested subagent tree helpers (flatten, roots, ancestors, expansion merge for deep links, range selection order) wired through client-runtime
thread-relationships. Desktop settings tests add defaultsidebarShowSubagentThreads: false.Reviewed by Cursor Bugbot for commit c050552. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Reveal nested subagent threads in the sidebar with expand/collapse controls
uiStateStoreand auto-expands ancestors of the currently active thread on first paint.CheckpointCleanupServiceV2that deletes checkpoint refs per scope up to the latest known ordinal, wired into the effect worker as a newcheckpoint.cleanupeffect type.EventSinkV2.writenow rejects writes targeting archived/deleted threads and accepts multiple expected run statuses; provider adapter events are only persisted when the root run is current.useThreadActionslayer; cascade deletes affect all owned subagent descendants, not just the selected thread.Macroscope summarized c050552.
Archiving, unarchiving, or deleting a thread now applies to the subagent subtree it recursively owns, enforced server-side in this PR: the orchestrator walks the owned subtree (OwnedSubagentTree), applies the lifecycle transition to every member with a single timestamp, and cancels active runs and pending attempts. Half-applied cascades are repaired on startup (OwnedSubagentLifecycleRepair) and checkpoints orphaned by deleted threads are cleaned up (CheckpointCleanupService). The web sidebar layers on top: subtree-aware confirmation copy and counts, recovery-root badges, navigation, and client-side draft/terminal cleanup. No mobile code in this PR — the mobile UI for the same behavior follows in #3871.