feat(orchestrator): archive and delete owned subagent subtrees#3878
Conversation
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).
|
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
Mobile UI for the owned-subtree lifecycle cascade in pingdotgg#3878: 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#3878.
The recursive lifecycle for owned subagent subtrees is enforced server-side (pingdotgg#3878): 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.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| (candidate) => candidate.id === run.providerThreadId, | ||
| ); | ||
| if (projection.thread.archivedAt !== null || projection.thread.deletedAt !== null) { | ||
| if ( |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderTurnStartService.ts:86
The early archived/deleted fast path detaches the provider session based on the initially-read projection, without revalidating before the call. If the thread is unarchived and a new turn reattaches the same providerSessionId before this outbox effect runs, providerSessions.detach removes the now-current attachment and may release the single-thread session, disrupting the in-flight turn. Consider rechecking the thread lifecycle and current attachment state transactionally before detaching, or conditioning detachment on a lifecycle generation so it only applies to the archived/deleted state it was scheduled for.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderTurnStartService.ts around line 86:
The early archived/deleted fast path detaches the provider session based on the initially-read projection, without revalidating before the call. If the thread is unarchived and a new turn reattaches the same `providerSessionId` before this outbox effect runs, `providerSessions.detach` removes the now-current attachment and may release the single-thread session, disrupting the in-flight turn. Consider rechecking the thread lifecycle and current attachment state transactionally before detaching, or conditioning detachment on a lifecycle generation so it only applies to the archived/deleted state it was scheduled for.
| }, | ||
| } | ||
| : {}), | ||
| writeIfRunCurrent: { |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/RunExecutionService.ts:656
expectedStatus: ["running", "waiting"] is applied to every routed provider event, not just child-drain events. When a child keeps ingestion open after the root turn.terminal, a late mutable root event arriving after the run has moved to waiting is still committed and can overwrite terminal root provider state — for example, reverting the provider turn back to running. The waiting admission should be restricted to post-terminal child artifacts only, not applied unconditionally to all events.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/RunExecutionService.ts around line 656:
`expectedStatus: ["running", "waiting"]` is applied to every routed provider event, not just child-drain events. When a child keeps ingestion open after the root `turn.terminal`, a late mutable root event arriving after the run has moved to `waiting` is still committed and can overwrite terminal root provider state — for example, reverting the provider turn back to `running`. The `waiting` admission should be restricted to post-terminal child artifacts only, not applied unconditionally to all events.
| const throughOrdinal = Math.max( | ||
| 0, | ||
| ...projection.runs.map((run) => run.ordinal), | ||
| ...projection.checkpoints.map((checkpoint) => checkpoint.ordinalWithinScope), | ||
| ); |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/CheckpointCleanupService.ts:43
cleanup computes throughOrdinal by spreading every run and checkpoint ordinal into Math.max, so a thread with enough runs and checkpoints exceeds the JavaScript engine's function-argument limit and throws RangeError before any scope refs are removed. Compute the maximum iteratively instead of spreading the arrays into Math.max.
| const throughOrdinal = Math.max( | |
| 0, | |
| ...projection.runs.map((run) => run.ordinal), | |
| ...projection.checkpoints.map((checkpoint) => checkpoint.ordinalWithinScope), | |
| ); | |
| const throughOrdinal = [ | |
| ...projection.runs.map((run) => run.ordinal), | |
| ...projection.checkpoints.map((checkpoint) => checkpoint.ordinalWithinScope), | |
| 0, | |
| ].reduce((a, b) => Math.max(a, b), 0); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/CheckpointCleanupService.ts around lines 43-47:
`cleanup` computes `throughOrdinal` by spreading every run and checkpoint ordinal into `Math.max`, so a thread with enough runs and checkpoints exceeds the JavaScript engine's function-argument limit and throws `RangeError` before any scope refs are removed. Compute the maximum iteratively instead of spreading the arrays into `Math.max`.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 225ba2d. Configure here.
| const descendants = ownedSubagentDescendants(command.threadId, [ | ||
| ...shell.threads, | ||
| ...shell.archivedThreads, | ||
| ]); |
There was a problem hiding this comment.
Delete cascade misses shell gaps
Medium Severity
dispatchOwnedThreadLifecycle builds the owned subtree only from getShellSnapshot() threads, which omit deleted rows. If an intermediate owned subagent was already deleted, the walk cannot reach deeper descendants, so a later thread.delete on an ancestor may leave active grandchild threads undeleted and off the affectedThreadIds list used to cancel process-bound effects. planOwnedSubagentLifecycleRepairs runs only at orchestrator startup, so those orphans can keep running until restart.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 225ba2d. Configure here.
ApprovabilityVerdict: Needs human review 3 blocking correctness issues found. This PR introduces a significant new feature for cascading archive/delete operations across owned subagent subtrees. The changes involve substantial new orchestration logic, new services, and modifications to core event handling. Four unresolved medium-severity review comments identify potential correctness issues including race conditions and orphaned thread scenarios. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Applying archive, unarchive, or delete to a thread now cascades through the subagent threads it recursively owns, entirely server-side:
thread.archive/thread.unarchive/thread.delete, applies the lifecycle transition to every subtree member with a single timestamp (so "archived together" is exact), and cancels active runs and pending attempts across the subtree.Server-only: no client, web, or mobile changes. The sidebar (#3861) and mobile (#3871) UI work layer on top of this behavior and should land after it.
All 282 orchestration-v2 tests pass (276 passed, 6 pre-existing skips), including new coverage for the cascade, repair, cleanup, turn-start gating on archived/deleted threads, and foundation persistence.
Note
High Risk
Large orchestration-v2 changes to thread lifecycle, event commit guards, effect cancellation, and checkpoint deletion—areas where races or incomplete cascades could leave inconsistent state or drop in-flight provider work.
Overview
Thread archive, unarchive, and delete now drive a server-side cascade over recursively owned subagent threads (subagent lineage only—not forks), using a shared archive timestamp so “archived together” is exact and unarchive restores only that cohort. The orchestrator cancels active work across the subtree, detaches sessions, enqueues terminal/attachment cleanup, and on delete adds
checkpoint.cleanupvia newCheckpointCleanupServiceanddeleteScopeRefs(including deterministic ordinals through interrupted baselines).Concurrency and repair: dispatch is serialized under a root-thread graph lock; archive/delete cancel process-bound (and capture) effects across all affected thread IDs.
OwnedSubagentLifecycleRepairplusgetThreadLifecycleRecordsreconcile inconsistent parent/child lifecycle on startup and when new subagent threads appear under archived/deleted parents.Persistence races:
writeIfRunCurrentrejects events for archived/deleted threads (including routed child-thread events), accepts multiple expected run statuses, and optionally admits terminal artifacts from superseded attempts. Provider turn start and run execution detach or gate when lifecycle wins; delegated-task and subagent finalization skip inactive parents.Reviewed by Cursor Bugbot for commit 225ba2d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Archive and delete owned subagent subtrees with cascading lifecycle propagation
OwnedSubagentTree.ts), cancelling runs, attempts, nodes, provider turns, and effects for all affected threads.CheckpointCleanupServicedeletes checkpoint refs for each scope in a thread up to the latest known ordinal; it is enqueued as acheckpoint.cleanupeffect during archive/delete.writeIfRunCurrentinEventSink.tsnow rejects writes if any referenced thread is archived/deleted, and can accept multiple expected run statuses and terminal artifacts for superseded attempts.thread.createdevents, the orchestrator runs lifecycle repair viaOwnedSubagentLifecycleRepair.tsto align child threads whose lifecycle disagrees with an archived/deleted parent.startNextQueuedRunare now serialized by a graph-level lock keyed onrootThreadId, preventing races across related threads.📊 Macroscope summarized 225ba2d. 13 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.