fix(ce-work): codify worktree isolation for parallel subagent dispatch#698
Merged
fix(ce-work): codify worktree isolation for parallel subagent dispatch#698
Conversation
Phase 1 Step 4's parallel-subagent flow described shared-directory dispatch with reactive collision recovery, leaving worktree isolation as emergent model behavior. Direct subagent dispatch to use isolation: "worktree" on Claude Code's Agent tool (with run_in_background: true), reframe the existing constraints as the shared-directory fallback for platforms without worktree support, and add a worktree-aware merge path that resolves overlap at merge time instead of via the discovered-collision cross-check. Stable/beta sync: applied identically to ce-work and ce-work-beta — the parallel-subagent body is shared.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f569c294b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Phase 2's "Parallel subagent mode" note now splits commit ownership by isolation mode, matching Phase 1 Step 4: worktree-isolated subagents commit inside their own worktree branch (orchestrator merges after the batch); shared-directory fallback subagents do not commit (orchestrator stages and commits after the batch). Resolves the contradiction where Phase 2 flatly forbade subagent commits while Phase 1 Step 4's new worktree-isolated branch allowed them. - Worktree cleanup step now uses the absolute path returned in the subagent's result, with a `git rev-parse --show-toplevel` fallback for platforms whose subagent primitive does not return a path. The prior `git worktree remove .claude/worktrees/agent-<id>` was CWD-relative and failed when /ce-work ran from a subdirectory. Both edits propagated to ce-work-beta — the affected sections are shared verbatim between stable and beta.
…allel mode The previous worktree-isolated wording had two gaps surfaced during post-merge validation of #698: 1. Post-batch step 3 said "resolve conflicts at merge time" without saying how. Hand-resolving silently picks a side and discards one unit's intent. Now: abort the merge and re-dispatch the conflicting unit serially against the now-merged tree — the worktree-mode equivalent of shared-directory's "re-run affected units serially" recovery. 2. The Parallel Safety Check unconditionally downgraded to serial on any file overlap. That rule was written for shared-directory write races. With worktree isolation, overlap is no longer a write race — it's a predictable merge conflict the orchestrator handles. Split the rule: downgrade to serial only when isolation is unavailable; when it's available, proceed in parallel and log the predicted overlap so the post-batch flow knows where to expect conflicts. Stable/beta sync: applied identically to ce-work and ce-work-beta.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70d5c5ef4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A live probe with isolation: "worktree" surfaced three issues with the post-batch worktree cleanup step: 1. The Claude Code harness locks per-subagent worktrees with the reason "claude agent agent-<id> (pid <n>)". Plain `git worktree remove <path>` fails with "cannot remove a locked working tree." The skill now uses `--force --force` to override the lock; safe because the work is already merged at that point. 2. The previous fallback `git rev-parse --show-toplevel` returned the *orchestrator's worktree*, not the main repo, when the orchestrator itself ran from a worktree (e.g., one created by /ce-worktree). The resulting path was wrong and cleanup silently failed. The skill now uses `dirname "$(git rev-parse --path-format=absolute --git-common-dir)"`, which returns the main-repo root from both main checkouts and worktrees. 3. `git worktree remove` removes the working directory but leaves the `worktree-agent-<id>` branch behind. Without explicit cleanup, these accumulate (the compound-engineering repo currently has 26+ such orphan branches). The skill now adds `git branch -D <branch-name>` after worktree removal, using the branch name the harness returns alongside the path. Verified empirically: dispatched a real subagent with isolation: "worktree" from inside .claude/worktrees/hidden-cuddling-brooks; the harness placed the per-subagent worktree at <main-repo>/.claude/ worktrees/agent-<id> (sibling to the orchestrator's worktree, not nested), and returned both the absolute path and branch name in the result. Stable/beta sync: applied identically to ce-work and ce-work-beta.
Self-review surfaced four follow-ups on the worktree-isolated post-batch flow that landed in 666c669: 1. Step 6 was a single dense bullet covering five concerns (worktree removal, lock override, branch deletion, fallback path derivation, `--show-toplevel` rationale). Split into three sub-bullets, one command each. 2. `git worktree remove --force --force` and `git branch -D` together bypass git's "uncommitted changes" check, "is this worktree locked" check, and "is this branch merged" check. If a future flow change ever invokes cleanup before merging, the work would silently disappear. Switched to `git worktree unlock <path>` then plain `git worktree remove <path>` (overrides only the lock, not other safety checks), and `git branch -d <name>` (lowercase: refuses unmerged branches). All three commands fail loudly if state is wrong rather than discarding work. 3. Removed the cross-platform fallback path derivation (`dirname "$(git rev-parse --path-format=absolute --git-common-dir)"`). It only applied to a hypothetical platform that exposes worktree isolation but doesn't return the worktree path — no platform we target hits that path. Codex/Pi already use the shared-directory fallback, where worktrees aren't created at all. 4. Trimmed the trailing audience-enumeration sentence from Phase 2's "Parallel subagent mode" note. The bullet split above already covers commit ownership; restating "applies to inline, serial, worktree-isolated subagents, and shared-directory orchestrator commits" reads as defensive boilerplate. Stable/beta sync: applied identically to ce-work and ce-work-beta.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isolation: "worktree"(an emergent Opus behavior, not a skill instruction).isolation: "worktree"andrun_in_background: trueon Claude Code'sAgenttool, with a documented fallback for platforms without built-in worktree primitives (Codexspawn_agent, Pisubagent).Stable/beta sync: applied identically to
ce-workandce-work-beta— the parallel-subagent body is shared verbatim between them.Test plan
bun run release:validate— passes (no manifest drift; doc-only edits)bun test— 939 pass, 0 failSKILL.mdfilesThere is no code-level regression test for skill behavioral instructions; the verification is editorial review of the new isolation directive and the worktree-mode post-batch flow.
Fixes #682