Skip to content

docs(orchestration): clean up created_child worktrees after worker-release #19404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LesleyMurfin wants to merge 1 commit into
base: main
Choose a base branch
from
+43 −3
Open

docs(orchestration): clean up created_child worktrees after worker-release #19404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions skill-guides/orchestration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ Release is post-settlement cleanup, not cancellation. Only an accepted
settlement authorizes it; no other observation does. If release is uncertain,
follow its exact recovery receipt and never substitute `terminal close`.

Release also owes one more check: if that Dispatch's own `effects` recorded a
`created_child` worktree, remove it once release has closed the terminal —
see the worktree-cleanup gate in `references/recovery-and-cleanup.md` for the
exact command and its fail-closed conditions.

A valid `worker_done` settles the Task and Dispatch automatically; do not follow
it with `task-update --status completed`. Enumerate the terminals still owing a
decision with `worker-list --run <run_id> --terminal-state reclaimable --json`,
Expand All @@ -194,6 +199,7 @@ older CLI rejects `--full`, keep this kernel's safety floor, use that command's
| New worktree, exact workspace, SSH, WSL, or connected-server placement | `references/placement-and-remote.md` |
| Inbox replay, follow-up messages, group addresses, or decision gates | `references/messaging-and-gates.md` |
| Failed/stopped/unknown attempts, retry, stop, abandon, retain, or uncertain release | `references/recovery-and-cleanup.md` |
| A released Dispatch's start effect created a child worktree still needing removal | `references/recovery-and-cleanup.md` |
| Custom argv or terminal topology that `worker-start` cannot express | `references/low-level-topology.md` |
| Any legacy label, adopted Run, compatibility receipt, or takeover | `references/legacy-contract-migration.md` |

Expand Down
34 changes: 34 additions & 0 deletions skill-guides/orchestration/references/recovery-and-cleanup.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,37 @@ or `release_unknown`, follow its exact recovery action. Never substitute

`orchestration reset` is destructive recovery. Do not run it during active
coordination unless the user explicitly abandons that state.

## Worktree cleanup after release

After `worker-release` accepts, check whether that Dispatch's own `effects`
recorded `{ "kind": "worktree", "action": "created_child" }`:
Comment on lines +163 to +164

@coderabbitai coderabbitai Bot Sep 8, 2026

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Require a terminal-closed release state before cleanup.

orchestration.workerRelease can return state: "retained" while the terminal remains retained. Command acceptance is not proof that the terminal closed. Require state: "released" or "already_released" before worker-show and worktree rm; route retained, release_pending, and release_unknown through recovery. This guard is necessary because worktree rm --force also sets allowUnverifiedPtyStop.

Also applies to: 177-177


```text
ORCA orchestration worker-show --dispatch <dispatch_id> --json
```

If it did, remove the checkout Orca created for it, using the exact path from
that same response's `terminal.worktreePath`:

@pullfrog pullfrog Bot Sep 8, 2026

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.

terminal.worktreePath won't be present in the response this step is told to read. worker-show returns terminal: observation.exact ? observation.terminal : null (src/main/runtime/rpc/methods/orchestration/worker/worker-control.ts:78), and exact requires the Dispatch's recorded process incarnation to still match (isDispatchProcessCurrent). Release's job is to close/kill that process, so terminal is null afterwards — and for a structured worker it's null unconditionally (inspectWorkerTerminal returns terminal: null on the structured branch). Use the always-exposed worker.worktreeId from that same response instead and drive removal with id:<worktreeId>; worktree rm accepts id: as a first-class selector.

Technical details 
# Worktree cleanup path source

## Affected sites
- skill-guides/orchestration/references/recovery-and-cleanup.md:171 — reads terminal.worktreePath from a post-release worker-show response

## Required outcome
- The recipe must obtain the created_child worktree identity from a field that survives release. Prefer `worker.worktreeId` (and/or the `id` on the `created_child` effect) from the `worker-show --json` response, then `ORCA worktree rm --force --worktree id:<worktreeId> --json`.
- If keeping `terminal.worktreePath`, note it must be read from an inspection run before release (when the terminal is still exact), and is never available for structured workers.

## Suggested approach (optional)
- `exposeWorker` already publishes `worktreeId` (parsed from `worker.worktree_id`) and `effects`; both are durable DB values, independent of process liveness. The `id:` selector resolves via `orca-runtime-resolve-worktree-selector.ts`.


```text
ORCA worktree rm --force --worktree path:<worktreePath> --json

@coderabbitai coderabbitai Bot Sep 8, 2026

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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve local work before force removal.

--force removes a dirty checkout without confirmation. The current rule protects only unpushed commits after STOP or failure and only says to run git status; it does not require checking the exact terminal.worktreePath, and it does not cover uncommitted or untracked changes or a successful report. The worker contract does not require a commit when --files-modified is applicable. Inspect the exact child worktree before every removal and leave/report whenever it is dirty or commit visibility is unproven.

Also applies to: 187-188

```

`--force` skips the dirty-tree confirmation prompt; it never force-deletes the
underlying Git branch. Leave the worktree in place and report it instead when
any of the following hold:

- The user asked to keep the workspace, or the Dispatch recorded
`worker-retain`.
- The terminal was user-taken-over.
- The recorded start effect was `reused`, or the worker started with
`--worktree current` or an exact pre-existing workspace — never delete a
checkout this Dispatch did not create.
- The worker STOP'd or failed with unpushed, local-only commits (check
`git status` first).
- `worktree rm` itself errors — a locked worktree or unverifiable Git state.

This is coordinator hygiene layered on top of release, not a substitute for
it: run it only after `worker-release` has already closed the terminal, and
only for the exact `created_child` worktree that Dispatch created.
Loading
Loading