docs(orchestration): bind worker placement to the Run, not the task

[Tauri-EPO]

ELI5

When a coordinator starts several workers for one goal, the guide never said when it picks where those workers run. So it decided again for every task, and one Run ended up with some lanes as child worktrees and others as top-level ones. This adds one rule — pick the placement once when you open the Run and reuse it — and states the one fact the guide was missing: leaving --worktree off means current.

What Changed

Documentation only, in skill-guides/orchestration/references/placement-and-remote.md:

• Placement is decided once per Run and reused for every later worker-start, not re-derived per Task. The reason is spelled out: tasks decomposed from one objective read as dependent and independent at the same time, so a per-Task reading splits one Run across both lineages.
• An omitted --worktree resolves to current, the coordinator's own workspace.
• --on gets the matching caveat: the resolved current default is rejected there too, so a remote start must name placement explicitly.

Two follow-on edits: src/cli/bundled-skill-guides.ts is regenerated with pnpm generate:bundled-skill-guides (generated file, not hand-edited), and config/scripts/orchestration-skill-guidance.test.mjs gains one it that pins the new sentences. No existing assertion was changed or removed.

Why

#14840 reported two separate gaps behind the mixed-lineage Run: the missing worktree-creation gate on the preferred path, and the missing rule about when placement is chosen.

The first gap is already closed on main. The orchestration guide rewrite in #16904 moved the gate into references/placement-and-remote.md, which now opens with "A fresh worker means a fresh agent terminal, not a new Git worktree… Create a worktree only when the user requested one or a concrete checkout or filesystem conflict makes sharing unsafe." Nothing here touches that.

What is still absent after that rewrite is the timing rule and the default. Grepping the current guide and its references returns no statement that placement is a Run-level decision, and none that --worktree is optional. Both are load-bearing:

• Without the timing rule, the surviving criterion is under-determined for the DAG case it governs. Every task in a decomposed DAG is derived from one objective (reads as stacked → new-child) and parallelizable (reads as independent → new-top-level). Both readings are true of the same task, so the outcome tracks task wording. Deciding once per Run removes the judgment call instead of asking the model to apply a criterion that cannot discriminate.
• Without the default, every example passes --worktree explicitly, which teaches by example that a lineage decision is always required. The default is real: src/main/runtime/rpc/methods/orchestration-workers.ts:57 reads const requestedWorktree = params.worktree ?? 'current', and current then resolves to the coordinator terminal's own workspace. The remote path applies the same default at src/main/runtime/rpc/methods/orchestration-federated-worker-start.ts:45 and immediately rejects it, which is why the --on caveat is stated rather than left implicit.

The original version of this PR also deleted --worktree current from the examples, per the third bullet of the issue's proposal. That part is dropped: after #16904 those spellings are a test contract (orchestration-skill-guidance.test.mjs asserts the reference contains --worktree current --agent codex), and stating the default in prose is enough without churning the examples.

Scope notes: the change stays inside the conditional reference, so the always-loaded kernel is untouched and its 202-line budget is unaffected. The wording is placement-neutral for folder workspaces — it constrains when the choice is made, not what can be chosen — and adds nothing to any wire, RPC, or client-host exchange.

Linked Issue

Fixes #14840

Visual Proof

N/A — Markdown guide text plus its generated CLI bundle and a guidance test. No UI, no runtime behavior change.

Testing

Windows 11, Node 24.19.0, pnpm 12.0.0, on upstream/main at 9d29e6878e.

$ node config/scripts/generate-bundled-skill-guides.mjs --write
$ node config/scripts/generate-bundled-skill-guides.mjs --check
verify:bundled-skill-guides EXIT=0

$ node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts config/scripts/orchestration-skill-guidance.test.mjs
 Test Files  1 failed (1)
      Tests  1 failed | 29 passed (30)

$ pnpm tc:cli
$ tsc --noEmit -p config/tsconfig.tc.cli.json
(exit 0)

$ node_modules/.bin/oxlint config/scripts/orchestration-skill-guidance.test.mjs
(exit 0)
$ node_modules/.bin/oxfmt --check config/scripts/orchestration-skill-guidance.test.mjs src/cli/bundled-skill-guides.ts
All matched files use the correct format.

The new it passes. The one failing test — keeps heartbeat and worker_done recipes bound to the injected capability, which reads worker-contract.md — fails identically on a clean upstream/main checkout with no changes applied (28 passed / 1 failed there, 29 passed / 1 failed here). It is a local Windows artifact: that test extracts fenced blocks with a regex anchored on \n, and this checkout has core.autocrlf=true, so the file's \r\n line endings make the match return undefined. Nothing in this PR touches that file or that test.

pnpm verify:skill-bundle-manifest also fails here, and likewise fails on the unmodified upstream tree (verified with git stash, exit 1 both times, same three stale artifacts: current-manifest.json, snapshot-registry.json, release-mapping.json). Pre-existing and unrelated, so the generated manifest was deliberately left alone.

• I manually tested these changes locally
• Automated tests added/updated, or explained why not below

AI Disclosure

Written with Claude (Opus). Every claim about current main and about the CLI default was checked against the tree and the linked source lines before it was written.

Review

• Cross-platform: text and a generated TypeScript string constant. No paths, shells, or processes.
• SSH / remote: the added --on sentence describes existing runtime behavior (orchestration-federated-worker-start.ts:45-51 rejects a resolved current), and does not change the execution boundary or the live / unverifiable / exited vocabulary.
• Folder workspaces: the new rule constrains when placement is decided, not what a workspace may be; the existing folder-workspace paragraphs are untouched.
• Wire compatibility: none. No RPC params, stream opcodes, or host-published content.
• Git providers: no provider-specific content.
• Performance / security: none. Four prose sentences in a conditionally loaded reference.
• Backwards compatibility: additive to a reference that is already fetched on demand; the always-loaded kernel is byte-identical.

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.

Checklist

• This PR is small and focused
• I explained what changed and why (including ELI5)
• 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) — ran the gates listed under Testing; full pnpm lint and pnpm build left to CI

Author

X/Twitter: @EnricoPin

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: d3f5d072-308d-4268-8a40-a7a6f0930ce8

📥 Commits

Reviewing files that changed from the base of the PR and between 9d29e68 and 2515171.

📒 Files selected for processing (3)

• config/scripts/orchestration-skill-guidance.test.mjs
• skill-guides/orchestration/references/placement-and-remote.md
• src/cli/bundled-skill-guides.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• config/scripts/orchestration-skill-guidance.test.mjs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The change documents that worker placement is selected once per Run and reused for later worker-start operations. It defines omitted --worktree as the coordinator's current workspace. Remote starts reject this default and require an explicit remote workspace or new-top-level with a remote repository selector. Tests cover these rules and worker lineage consistency.

Merge Risk: ⚪ Minimal · up to 25151

Worker placement guidance now stays consistent for an entire Run, including explicit remote requirements, with regression coverage for the documented behavior. The change is ready to merge.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address issue [#14840] by documenting Run-level placement selection, the current default, remote placement requirements, and regression coverage. The existing worktree-isolation guidance a…
Out of Scope Changes check	✅ Passed	The changes are limited to orchestration documentation, its generated bundle, and focused regression coverage. No unrelated runtime, wire, or product behavior changes are included.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Title check	✅ Passed	The title clearly and concisely summarizes the primary change: worker placement is bound to the Run instead of individual tasks.
Description check	✅ Passed	The description is complete and relevant. It covers the change, rationale, linked issue, testing, generated-file updates, compatibility considerations, AI disclosure, and checklist status. Known test …

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 26e8149b-cc7c-4805-903d-7b157de9e21a

📥 Commits

Reviewing files that changed from the base of the PR and between 1f483c3 and 54a3eb742284988743e0840b9052841c8e1dc364.

📒 Files selected for processing (3)

• config/scripts/orchestration-skill-guidance.test.mjs
• skill-guides/orchestration.md
• src/cli/bundled-skill-guides.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[Tauri-EPO]

Rewritten on upstream/main (9d29e6878e) after the orchestration guide rewrite in #16904. The change now lands in skill-guides/orchestration/references/placement-and-remote.md instead of the old orchestration.md section, which no longer exists.

Scope shrank. #16904 already carries the worktree-creation gate ("A fresh worker means a fresh agent terminal, not a new Git worktree… Create a worktree only when the user requested one or a concrete checkout or filesystem conflict makes sharing unsafe"), so this PR no longer restates it. What is left is the part that is still missing on main: placement is decided once per Run rather than re-derived per Task, and an omitted --worktree resolves to current. Grepping skill-guides/ on 9d29e6878e returns no hit for either.

Dropped: removing --worktree current from the examples. After #16904 that spelling is a test contract — config/scripts/orchestration-skill-guidance.test.mjs asserts the reference contains --worktree current --agent codex — so the default is stated in prose and the examples stay as they are.

The default is confirmed in the runtime, not assumed: src/main/runtime/rpc/methods/orchestration-workers.ts:57 is const requestedWorktree = params.worktree ?? 'current', and src/main/runtime/rpc/methods/orchestration-federated-worker-start.ts:45 applies the same default before rejecting it for --on, which is why the remote caveat is spelled out.

Head is 251517160c. Locally on Windows: generate-bundled-skill-guides.mjs --check exits 0 after regeneration, pnpm tc:cli exits 0, oxlint and oxfmt --check are clean, and the guidance suite is 29 passed / 1 failed. That one failure (keeps heartbeat and worker_done recipes bound to the injected capability, on worker-contract.md) reproduces on a clean upstream/main tree with no changes applied, and is a core.autocrlf=true artifact in this checkout. Details are in the PR description.

[Tauri-EPO]

Closing: skill-guides/orchestration/references/placement-and-remote.md (from #16904, v1.4.199) already fixes the default this PR asked for: a fresh worker is a fresh agent terminal in the current or exact workspace, and a new worktree only on user request or a concrete checkout conflict. Deciding placement once per Run follows from that rule without a second paragraph. Branch kept on the fork.