From 9efbb36f9be70056c5b4599fece44ed3f4b51387 Mon Sep 17 00:00:00 2001 From: Justin Puah Date: Thu, 3 Sep 2026 00:21:39 +1000 Subject: [PATCH 1/5] fix(implement): correct worktree-detection and cd-persistence gaps [#QR221-01, #QR221-02, #QR221-03] - ai-agents/skills/implement/SKILL.md: bare-worktree detection now requires more than one git worktree list entry, not just the current worktree showing (always true) - ai-agents/skills/implement/SKILL.md: scope the EnterWorktree path claim to the main session's own launch directory; add the pinned-subagent fallback (stop and hand back, or EnterWorktree name:) - ai-agents/skills/implement/SKILL.md: add an explicit stop-and-ask fallback when Codex CLI/Pi cd persistence can't be confirmed and no runtime primitive exists Refs: AB#221 --- ai-agents/skills/implement/SKILL.md | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/ai-agents/skills/implement/SKILL.md b/ai-agents/skills/implement/SKILL.md index dedf63ad..48351d53 100644 --- a/ai-agents/skills/implement/SKILL.md +++ b/ai-agents/skills/implement/SKILL.md @@ -6,6 +6,39 @@ disable-model-invocation: true Implement the work described by the user in the spec or tickets. +**Before anything else, make sure this isn't landing on the default branch.** Determine the +default branch (`git symbolic-ref --short refs/remotes/origin/HEAD`, stripped of the `origin/` +prefix — don't assume `main`/`master`). If that's unset (e.g. a fresh or `--single-branch` +clone with no `origin/HEAD`), stop and ask which branch is the default rather than guessing or +running an untested network fallback. If the current branch is the default branch, branch off +first, automatically, without asking: + +- **Bare-worktree layout** (sibling `.bare/`, or `git worktree list` shows more than one entry — + see [git-worktrees.md](../../AGENTS.d/git-worktrees.md)): from the current worktree, + `git worktree add ../ -b ` (flatten any `/` in `` for + ``), then move into that new worktree and continue all further work there. + **Claude Code:** a plain shell `cd` to a sibling worktree directory does not reliably persist + across tool calls in the sandboxed Bash tool (verified: it silently resets to the original + directory) — use `EnterWorktree` with `path` set to the new worktree's directory instead. From + the main session's own launch directory, its first-entry check accepts any path already + registered in `git worktree list`, so the bare-worktree layout's sibling directory qualifies + even though it isn't under `.claude/worktrees/`. From a pinned subagent (working directory + pinned at launch — subagent isolation or an explicit cwd, e.g. a `dispatch-implement` child) + that finds itself on the default branch, `EnterWorktree`'s contract requires the target to be + a worktree already under `.claude/worktrees/` of the same repo, so the bare-worktree sibling + directory does not qualify there: either stop and hand back rather than guess, or use + `EnterWorktree name:` instead of `git worktree add` + `path` — it creates the new worktree + under `.claude/worktrees/` and is accepted from a pinned agent. **Codex CLI / Pi:** unverified + whether their shell tools persist a `cd` across calls the same way — check before relying on + it, and use their own worktree/session primitive if one exists rather than assuming a bare + `cd` holds. If persistence can't be confirmed and no such primitive exists, stop and ask the + user to switch directories manually rather than proceeding on an unverified `cd`. +- **Normal clone**: `git checkout -b ` — no directory change involved, so no `cd`/session + caveat applies here. + +Name `` as `/-` (conventional-commit ``, e.g. `feat`/`fix`; +slug from the ticket/spec title) — drop the `-` segment when there's no ticket. + Use `/tdd` where possible, at pre-agreed seams. Run the project's fast checks (typecheck / lint / analyzer) regularly and single test files as you go; run the full test suite once at the end. From 5bbe5b092e538b2f1b5d6a424aff1b82b6597e77 Mon Sep 17 00:00:00 2001 From: Justin Puah Date: Thu, 3 Sep 2026 00:21:53 +1000 Subject: [PATCH 2/5] fix(dispatch-implement): specify branch naming for multi-ticket dispatch [#QR221-04] - ai-agents/skills/dispatch-implement/SKILL.md: step 5's parent branch-off now names the branch explicitly for a multi-ticket run (epic/spec slug when shared, else the first ticket listed) instead of relying on /implement's single-ticket naming rule Refs: AB#221 --- ai-agents/skills/dispatch-implement/SKILL.md | 44 +++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/ai-agents/skills/dispatch-implement/SKILL.md b/ai-agents/skills/dispatch-implement/SKILL.md index 1deccc5f..b636e40a 100644 --- a/ai-agents/skills/dispatch-implement/SKILL.md +++ b/ai-agents/skills/dispatch-implement/SKILL.md @@ -56,21 +56,35 @@ checks, review, commit. (see Override below). 5. **Dispatch**, one subagent per ticket, each prompted to run `/implement` on that ticket and nothing else. Parallel units go out in a single message so they actually run concurrently; - sequential units wait for the previous unit's result before the next dispatch. -6. **Integrate parallel children** (sequential children skip this — they already committed to the - current branch, in turn). An isolated child commits to its own branch in its own worktree, so its - work is not on the invoking branch until you bring it over. Before dispatch, capture each child's - pre-dispatch `HEAD` — that commit is the range base for that child's cherry-pick. Once every - parallel child has returned, for each in ticket order cherry-pick `base..tip` (its own branch tip) - onto the invoking branch — the full range, since `/implement` does not promise exactly one commit — - then run the project's fast checks once over the integrated result. The full suite is not re-run - post-integration: each child already ran it in isolation before its own commit, per `/implement`'s - own contract; this is an accepted tradeoff for a thin wrapper, not an oversight. On a cherry-pick - conflict, run `git cherry-pick --abort` to leave the invoking branch clean, then stop and hand back - the child branch and worktree names; do not resolve it silently, since a conflict here means the - non-overlapping judgment in step 3 was wrong. Once integration succeeds, remove the child worktrees - and delete the child branches — their commits are now fully represented on the invoking branch via - the cherry-picks, so there is no reason to keep them. + sequential units wait for the previous unit's result before the next dispatch. **Any run — + parallel or sequential — starting on the default branch:** branch off once, here, before + dispatching the first child — same detection as `/implement`'s own first step + (`implement/SKILL.md`, `AGENTS.d/git-worktrees.md`), named `/-` when the + invocation shares a spec/epic, else `/-` off the first ticket listed, in + invocation order. For a sequential run this makes each + child's own `/implement` call find itself already off the default branch and commit straight + to it in turn, as originally designed. For a parallel run this matters just as much: step 6 + cherry-picks each isolated child's range onto *the invoking branch*, so if the invoking branch + were still the default branch, every ticket's commits would land there regardless of the + child's own isolated branch being fine. Skipping this and leaving the invoking branch as the + default branch would spawn throwaway worktrees per ticket and land every cherry-pick back onto + the default branch in step 6 — the exact outcome this whole mechanism exists to prevent. +6. **Integrate parallel children** (sequential children skip this — step 5 already put the + invoking branch off the default branch before the first one was dispatched, so each commits + straight to the invoking branch in turn, same as before this ADR). An isolated child commits to + its own branch in its own worktree, so its work is not on the invoking branch until you bring it + over. Before dispatch, capture each child's pre-dispatch `HEAD` — that commit is the range base + for that child's cherry-pick. Once every parallel child has returned, for each in ticket order + cherry-pick `base..tip` (its own branch tip) onto the invoking branch — the full range, since + `/implement` does not promise exactly one commit — then run the project's fast checks once over + the integrated result. The full suite is not re-run post-integration: each child already ran it + in isolation before its own commit, per `/implement`'s own contract; this is an accepted + tradeoff for a thin wrapper, not an oversight. On a cherry-pick conflict, run + `git cherry-pick --abort` to leave the invoking branch clean, then stop and hand back the child + branch and worktree names; do not resolve it silently, since a conflict here means the + non-overlapping judgment in step 3 was wrong. Once integration succeeds, remove the child + worktrees and delete the child branches — their commits are now fully represented on the invoking + branch via the cherry-picks, so there is no reason to keep them. 7. **Report** per ticket: model used, parallel or sequential and why, the child's outcome, and for a parallel run the integration result (integrated cleanly, or which branches are left for the user). From ec7222797cabe7764c1565675f69d9df75ac4d51 Mon Sep 17 00:00:00 2001 From: Justin Puah Date: Thu, 3 Sep 2026 00:32:22 +1000 Subject: [PATCH 3/5] docs(adr): remove negative-parallelism prose [#QR221-05] - docs/adr/implement-enforces-worktree-per-ticket-in-its-own-first-step.md: reword the title and three body instances of 'X, not Y' phrasing to state the positive directly, per AGENTS.md; leave the Rejected alternatives section's contrast untouched - align the title's wording with the filename ('worktree-per-ticket') Refs: AB#221 --- ...rktree-per-ticket-in-its-own-first-step.md | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 docs/adr/implement-enforces-worktree-per-ticket-in-its-own-first-step.md diff --git a/docs/adr/implement-enforces-worktree-per-ticket-in-its-own-first-step.md b/docs/adr/implement-enforces-worktree-per-ticket-in-its-own-first-step.md new file mode 100644 index 00000000..6d90cf28 --- /dev/null +++ b/docs/adr/implement-enforces-worktree-per-ticket-in-its-own-first-step.md @@ -0,0 +1,109 @@ +# `/implement` enforces worktree-per-ticket in its own first step + +## Status + +Accepted. Governs how `ai-agents/skills/implement/SKILL.md` prevents ticket work from landing +on the default branch. + +## Context + +`AGENTS.d/git-worktrees.md` documents the bare-worktree layout's core rule — a new branch +means a new worktree, never `git checkout -b` inside the default-branch worktree — but the rule +was advisory only: nothing stopped `/implement` from running directly in the `main` worktree. + +A prior attempt enforced this with a `PreToolUse` hook plus a marker file +(`.active-ticket.json`) written by the implement skill to tell the hook "a ticket is in play." +It was ported three times across Claude Code, Codex CLI, and Pi with diverging capability: +Claude Code's hooks support an ask/confirm decision, Codex and Pi only support hard allow/deny. +No research turned up precedent for a marker/state-file mechanism used this way — every real +example of branch-per-task enforcement in AI coding agents and worktree tooling is either an +orchestration step that just creates the worktree up front (Codex Branch Workspaces, +OpenHands' worktree-per-task convention, Claude Code's/Cursor's opt-in worktree session modes), +or a hook intercepting git commands directly (with well-documented friction: absolute-path +requirements, deprecated schemas, false positives on legitimate branch-then-commit sequences). Git itself has no per-worktree hook — a hook applies repo-wide across every worktree, so a hook +scoped to "block commits when a ticket is active" still needs external state to know a ticket +is active, which is exactly what the marker file was for. + +The actual mistake in scope is narrower than "any commit on main": it's `/implement` itself +running while the current worktree/clone sits on the default branch. `dispatch-implement` +already isolates parallel children into their own worktree (`isolation: "worktree"`); its +sequential children and any standalone `/implement` invocation inherit whatever branch the +invoking session is already on, which is the actual gap. + +## Decision + +Move the check into `/implement`'s own first step, as plain skill-instruction text — no hook, +no marker file, no new state: + +1. Detect the default branch (`git symbolic-ref --short refs/remotes/origin/HEAD`), since it + varies per repo — verified in this repo, resolves to `origin/main`. If it's unset, stop and + ask rather than run an untested network fallback + (`git remote set-head`/`git remote show origin` were considered and dropped for this reason). +2. If the current branch is the default branch, branch off automatically before doing anything + else — `git worktree add` in the bare-worktree layout, `git checkout -b` in a normal clone — + then continue the rest of `/implement` from there. On Claude Code, moving into the new + worktree needs `EnterWorktree` with `path`; a plain shell `cd` resets on the next tool call — + verified live that a `cd` to a sibling worktree directory in the Bash tool silently resets to + the original directory on the next call, so the switch would not actually hold without it. + Codex CLI's and Pi's shell + persistence for this case is unverified and flagged as such in `implement/SKILL.md` rather + than assumed. +3. Name the branch `/-` (conventional-commit type, ticket number when one + exists, slug from the ticket/spec title). + +This covers a standalone `/implement` call directly. For `dispatch-implement`, it exposed a gap +in that skill's own assumptions, for both dispatch modes: sequential children were assumed to +always commit straight to the invoking branch (true only as long as `/implement` never moved the +workspace itself), and parallel children's step-6 cherry-picks target *the invoking branch*, +which would be the default branch if the parent session started there. `dispatch-implement`'s +step 5 is updated alongside this ADR: whenever a dispatch run — parallel or sequential — would +start on the default branch, the *parent* branches off once, before dispatching the first child, +using the same detection and naming `/implement` uses. Every sequential child's own `/implement` +call then finds itself already off the default branch and commits straight to it in turn, and +every parallel child's integration cherry-picks land on that same feature branch instead of the +default branch — preserving `dispatch-implement`'s original assumptions for both modes rather +than replacing them with a wider integration path. The alternative (let each child branch off +independently and integrate the results one at a time) was rejected: it would spawn one +throwaway worktree per ticket and still land cherry-picks back onto the default branch, which is +the exact outcome this whole mechanism exists to prevent. + +This resolves the prior design's capability split for free: the skill follows this as prompt +text, so it behaves identically across runtimes — "detect and just do it" works the same way on +Claude Code, Codex CLI, and Pi, with no ask/confirm-vs-hard-block distinction to reconcile since +no hook is involved. + +This nudges against `docs/adr/implement-stays-minimal-dispatch-in-wrapper-skill.md`, which kept +dispatch logic (ticket parsing, parallel/sequential judgment, per-runtime dispatch mechanics, +model selection) out of `/implement` and in the `dispatch-implement` wrapper instead. The +worktree check isn't dispatch logic — it doesn't parse tickets, judge parallelism, or pick a +dispatch primitive — it's a workspace precondition, the same category as the fast-checks step +`/implement` already runs before committing. It stays in scope for that ADR's boundary. + +## Rejected alternatives + +### Advisory-only (status quo) + +Already in place via `AGENTS.d/git-worktrees.md` and clearly insufficient — that's the problem +being solved. + +### `PreToolUse` hook + marker file, per runtime + +The prior attempt. Rejected: three ports for one rule, diverging ask/deny behavior across +runtimes, a marker file that's new state with its own lifecycle (write, read, staleness, who +clears it), and no precedent anywhere for this shape solving this problem. + +### Hard-blocking hook without a marker file + +Would need to distinguish "this commit is ticket work" from any other commit on the default +branch (e.g. a docs fix, a version bump) without state to say a ticket is in play — the same +problem the marker file existed to solve, or a hook broad enough to block *every* commit on the +default branch regardless of intent, which is a different and unwanted rule (the grilling +session that produced this ADR confirmed the mistake in scope is `/implement` on the default +branch specifically, not commits on the default branch in general). + +### Confirm before creating the worktree, instead of auto-creating + +Considered and rejected in favor of auto-create-then-proceed, matching Codex Branch Workspaces +and OpenHands' pattern: the first step of ticket work just creates the workspace, no +interruption. Creating a worktree/branch is cheap and reversible (`git worktree remove`, +`git branch -D`), so it doesn't need a confirm gate the way a destructive action would. From 594157f849f24fd165ca637380759854b332c6c1 Mon Sep 17 00:00:00 2001 From: Justin Puah Date: Thu, 3 Sep 2026 00:32:31 +1000 Subject: [PATCH 4/5] docs(dispatch-implement): trim duplicated rationale, fix unlinked ADR reference [#QR221-06, #QR221-07] - ai-agents/skills/dispatch-implement/SKILL.md: step 5's branch-off paragraph now states the instruction plus one why-sentence, with a resolvable pointer to the ADR instead of restating its rejected-alternative rationale inline - ai-agents/skills/dispatch-implement/SKILL.md: step 6 drops the changelog-style 'same as before this ADR' phrasing (unlinked, references history rather than current state) Refs: AB#221 --- ai-agents/skills/dispatch-implement/SKILL.md | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/ai-agents/skills/dispatch-implement/SKILL.md b/ai-agents/skills/dispatch-implement/SKILL.md index b636e40a..57676df0 100644 --- a/ai-agents/skills/dispatch-implement/SKILL.md +++ b/ai-agents/skills/dispatch-implement/SKILL.md @@ -61,17 +61,12 @@ checks, review, commit. dispatching the first child — same detection as `/implement`'s own first step (`implement/SKILL.md`, `AGENTS.d/git-worktrees.md`), named `/-` when the invocation shares a spec/epic, else `/-` off the first ticket listed, in - invocation order. For a sequential run this makes each - child's own `/implement` call find itself already off the default branch and commit straight - to it in turn, as originally designed. For a parallel run this matters just as much: step 6 - cherry-picks each isolated child's range onto *the invoking branch*, so if the invoking branch - were still the default branch, every ticket's commits would land there regardless of the - child's own isolated branch being fine. Skipping this and leaving the invoking branch as the - default branch would spawn throwaway worktrees per ticket and land every cherry-pick back onto - the default branch in step 6 — the exact outcome this whole mechanism exists to prevent. -6. **Integrate parallel children** (sequential children skip this — step 5 already put the - invoking branch off the default branch before the first one was dispatched, so each commits - straight to the invoking branch in turn, same as before this ADR). An isolated child commits to + invocation order. Step 6 cherry-picks each parallel child's range onto the invoking branch, so + the invoking branch must not be the default branch; see + `docs/adr/implement-enforces-worktree-per-ticket-in-its-own-first-step.md` for the full + rationale. +6. **Integrate parallel children** (sequential children skip this — they commit straight to the + invoking branch in turn, which step 5 guarantees is never the default branch). An isolated child commits to its own branch in its own worktree, so its work is not on the invoking branch until you bring it over. Before dispatch, capture each child's pre-dispatch `HEAD` — that commit is the range base for that child's cherry-pick. Once every parallel child has returned, for each in ticket order From b0e6c51dc4a21a5b52cb053f18536d79c97c4c7e Mon Sep 17 00:00:00 2001 From: Justin Puah Date: Thu, 3 Sep 2026 08:50:14 +1000 Subject: [PATCH 5/5] docs(git-worktrees): document dynamic default-branch detection and /implement enforcement - ai-agents/AGENTS.d/git-worktrees.md: don't hardcode the default branch name; note that /implement's first step now enforces the branch-per-ticket rule automatically Refs: AB#221 --- ai-agents/AGENTS.d/git-worktrees.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ai-agents/AGENTS.d/git-worktrees.md b/ai-agents/AGENTS.d/git-worktrees.md index 1a297bc2..528df37e 100644 --- a/ai-agents/AGENTS.d/git-worktrees.md +++ b/ai-agents/AGENTS.d/git-worktrees.md @@ -2,7 +2,9 @@ Applies when the repo uses the **bare-worktree layout**: the parent directory holds `.bare/` plus one directory per branch (e.g. `E:\Personal Projects\dotfiles\{.bare, main}`). **Check before creating a branch** — look for a sibling `.bare/`, or run `git worktree list`. In a normal clone, branch as usual and ignore the rest of this section. -- **A new branch means a new worktree. Never `git checkout -b` inside the `main` worktree.** From inside an existing worktree (e.g. `main`), not the bare-repo parent dir itself: `git worktree add ../ -b ` (flatten any `/` in the branch name for the directory), then change the working directory to that worktree and do the work there. `git worktree add`'s path is resolved against the shell's cwd, not the repo root — running it from the bare-repo parent dir (the one holding `.bare` + `main`) instead of from `main` lands the new worktree one level too high, as a sibling of the bare-repo dir rather than inside it. -- **Why:** the point of the layout is that every worktree is a stable, always-available checkout of its branch. `main` must stay on `main` so it remains browsable and buildable while feature work happens elsewhere — and so anything pointing at that path (tooling, another session, a live config) keeps seeing main. +- **A new branch means a new worktree. Never `git checkout -b` inside the default-branch worktree (e.g. `main`).** From inside an existing worktree (e.g. `main`), not the bare-repo parent dir itself: `git worktree add ../ -b ` (flatten any `/` in the branch name for the directory), then change the working directory to that worktree and do the work there. `git worktree add`'s path is resolved against the shell's cwd, not the repo root — running it from the bare-repo parent dir (the one holding `.bare` + `main`) instead of from `main` lands the new worktree one level too high, as a sibling of the bare-repo dir rather than inside it. +- **Why:** the point of the layout is that every worktree is a stable, always-available checkout of its branch. The default-branch worktree must stay on the default branch so it remains browsable and buildable while feature work happens elsewhere — and so anything pointing at that path (tooling, another session, a live config) keeps seeing it. +- **Don't hardcode the default branch name.** It's `main` in this repo but not necessarily elsewhere — detect it with `git symbolic-ref --short refs/remotes/origin/HEAD` (strip the `origin/` prefix) rather than assuming `main`/`master`. If it's unset, stop and ask which branch is the default rather than guessing. +- **Enforced automatically by `/implement`.** [`implement/SKILL.md`](../skills/implement/SKILL.md)'s first step checks whether the current branch is the default branch and, if so, branches off before doing anything else — in both layouts (`git worktree add` here, `git checkout -b` in a normal clone). This page stays the mechanics/rationale reference; the skill is what actually fires it. - **After the PR is merged:** change back to the main worktree → `git pull` → `git worktree remove ` (drops the registration *and* the directory) → **then** delete the branch. That order is forced: git refuses to delete a branch while a worktree still holds it, so the worktree always goes first. - **Squash-merged PRs need `-D`, not `-d`.** GitHub's squash-merge rewrites the branch's commits, so `git branch -d` refuses with "not fully merged" even though every change landed. Verify nothing is lost with `git diff main --stat` (empty ⇒ safe), then force-delete. `git branch -D` is denied by a Claude Code deny hook, so ask me to run that one.