golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(workflows): assert HEAD on per-agent branch before worktree commits (#2924) (#2941)

Browse Source 
* fix(workflows): assert HEAD on per-agent branch before worktree commits

Worktree-mode setup could leave HEAD attached to a protected branch (master),
causing agent commits to land there. The previous response was a destructive
self-recovery via 'git update-ref refs/heads/master <sha>', which silently
rewinds the protected branch and destroys concurrent commits in multi-active
scenarios (parallel agents, user committing while agent runs).

- Reorder <worktree_branch_check> in execute-phase.md and quick.md to assert
  HEAD via 'git symbolic-ref' BEFORE any 'git reset --hard'. HALT with a
  blocker if HEAD is on main/master/develop/trunk/release/* or detached.
- Add a per-commit HEAD assertion (step 0) to gsd-executor.md
  <task_commit_protocol>; HEAD attachment can drift after 'git checkout <sha>'.
- Forbid 'git update-ref refs/heads/<protected>' in
  <destructive_git_prohibition>; surface the blocker rather than self-heal.
- Remove '--no-verify' as the worktree-mode default in execute-phase.md,
  execute-plan.md, quick.md, and references/git-integration.md. Hooks now
  run on every executor commit; opt out only via workflow.worktree_skip_hooks.
- Add regression test that parses the worktree_branch_check blocks structurally
  and asserts the symbolic-ref check precedes the reset --hard, no workflow
  performs update-ref on a protected ref, and --no-verify is no longer the
  default in any parallel-execution prompt.

* fix(#2924): address CodeRabbit review findings on worktree HEAD PR

- Add positive worktree-agent-* allow-list to <task_commit_protocol> step 0
  in gsd-executor.md and to <worktree_branch_check> in execute-phase.md and
  quick.md. The deny-list (main|master|develop|trunk|release/*) silently
  allowed feature/* and other arbitrary branches outside the agent namespace.
- Register workflow.worktree_skip_hooks in both config schemas
  (sdk/src/query/config-schema.ts and get-shit-done/bin/lib/config-schema.cjs)
  and document it in docs/CONFIGURATION.md so config-set accepts it.
- Fix stash lifecycle in execute-phase.md post-wave hook validation: stash
  under a named ref and pop after the hook run; warn on pop failure.
- Pre-dispatch PLAN.md commit in quick.md: gate on git diff --cached --quiet
  for idempotency and exit 1 with a clear error on commit failure (both the
  --no-verify and the normal branches) — no more swallowing real errors.
- Test fixes (tests/bug-2924-worktree-head-attachment.test.cjs):
  - Parse the protected-branch alternation structurally and require
    main, master, develop, trunk, release/.* (release/* was previously
    skipped by the \\b...\\b regex).
  - Use fs.readdirSync(dir, { recursive: true }) so workflows in nested
    subdirectories are also asserted against the update-ref ban.
  - Add allow-list assertions for execute-phase.md, quick.md, and
    gsd-executor.md to lock in the new positive namespace check.

* test(#2924): assert sub-section end marker exists before slicing

* test(#2924): use section boundary instead of fixed window for parallel-agents slice
This commit is contained in:
Tom Boucher
2026-05-01 09:23:02 -04:00
committed by GitHub
parent 2cc8796265
commit 8de8acee46
9 changed files with 583 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
get-shit-done/workflows/execute-phase.md
View File

@@ -506,40 +506,37 @@ increases monotonically across waves. `{status}` is `complete` (success),
</objective>
<worktree_branch_check>
FIRST ACTION before any other work: verify this worktree's branch is based on the correct commit.
Run:
FIRST ACTION: HEAD assertion MUST run before any reset/checkout. Worktrees
spawned by Claude Code's `isolation="worktree"` use the `worktree-agent-<id>`
namespace. If HEAD is on a protected ref (main/master/develop/trunk/release/*)
or detached, HALT — do NOT self-recover by force-rewinding via `git update-ref`,
that destroys concurrent commits in multi-active scenarios (#2924). Only after
Step 1 passes is `git reset --hard` safe (#2015 — affects all platforms).
```bash
ACTUAL_BASE=$(git merge-base HEAD {EXPECTED_BASE})
```
If `ACTUAL_BASE` != `{EXPECTED_BASE}` (i.e. the worktree branch was created from an older
base such as `main` instead of the feature branch HEAD), hard-reset to the correct base:
```bash
# Safe: this runs before any agent work, so no uncommitted changes to lose
git reset --hard {EXPECTED_BASE}
# Verify correction succeeded
if [ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ]; then
echo "ERROR: Could not correct worktree base — aborting to prevent data loss"
HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED")
ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD)
if [ "$HEAD_REF" = "DETACHED" ] || echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then
echo "FATAL: worktree HEAD on '$ACTUAL_BRANCH' (expected worktree-agent-*); refusing to self-recover via 'git update-ref' (#2924)." >&2
exit 1
fi
if ! echo "$ACTUAL_BRANCH" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then
echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace; refusing to commit (#2924)." >&2
exit 1
fi
ACTUAL_BASE=$(git merge-base HEAD {EXPECTED_BASE})
if [ "$ACTUAL_BASE" != "{EXPECTED_BASE}" ]; then
git reset --hard {EXPECTED_BASE}
[ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && { echo "ERROR: could not correct worktree base"; exit 1; }
fi
```
`reset --hard` is safe here because this is a fresh worktree with no user changes. It
resets both the HEAD pointer AND the working tree to the correct base commit (#2015).
If `ACTUAL_BASE` == `{EXPECTED_BASE}`: the branch base is correct, proceed immediately.
This check fixes a known issue where `EnterWorktree` creates branches from
`main` instead of the current feature branch HEAD (affects all platforms).
Per-commit HEAD assertion lives in `agents/gsd-executor.md` `<task_commit_protocol>` step 0.
</worktree_branch_check>
<parallel_execution>
You are running as a PARALLEL executor agent in a git worktree.
Use --no-verify on all git commits to avoid pre-commit hook contention
with other agents. The orchestrator validates hooks once after all agents complete.
For `gsd-sdk query commit` (or legacy `gsd-tools.cjs` commit): add --no-verify flag when needed.
For direct git commits: use git commit --no-verify -m "..."
Run `git commit` normally — hooks run by default. Do NOT pass `--no-verify`
unless the orchestrator surfaces `workflow.worktree_skip_hooks=true` in this
prompt; silent bypass violates project CLAUDE.md guidance (#2924).
IMPORTANT: Do NOT modify STATE.md or ROADMAP.md. execute-plan.md
auto-detects worktree mode (`.git` is a file, not a directory) and skips
@@ -656,13 +653,16 @@ increases monotonically across waves. `{status}` is `complete` (success),
**This fallback applies automatically to all runtimes.** Claude Code's Task() normally
returns synchronously, but the fallback ensures resilience if it doesn't.
5. **Post-wave hook validation (parallel mode only):**
When agents committed with `--no-verify`, run pre-commit hooks once after the wave:
5. **Post-wave hook validation (parallel mode only):** Hooks run on every executor commit by default (#2924); this post-wave run only fires when `workflow.worktree_skip_hooks=true` opted out of per-commit hooks:
```bash
# Run project's pre-commit hooks on the current state
git diff --cached --quiet || git stash # stash any unstaged changes
git hook run pre-commit 2>&1 || echo "⚠ Pre-commit hooks failed — review before continuing"
SKIP_HOOKS=$(gsd-sdk query config-get workflow.worktree_skip_hooks 2>/dev/null || echo "false")
if [ "$SKIP_HOOKS" = "true" ]; then
# Stash uncommitted changes under a named ref so we always pop (bare `git stash` strands them on hook/script failure).
STASHED=false
if (! git diff --quiet || ! git diff --cached --quiet) && git stash push -u -m "gsd-post-wave-hook-$$" >/dev/null 2>&1; then STASHED=true; fi
git hook run pre-commit 2>&1 || echo "⚠ Pre-commit hooks failed — review before continuing"
[ "$STASHED" = "true" ] && (git stash pop >/dev/null 2>&1 || echo "⚠ Could not pop gsd-post-wave-hook stash — recover manually")
fi
```
If hooks fail: report the failure and ask "Fix hook issues now?" or "Continue to next wave?"

9
get-shit-done/workflows/execute-plan.md
View File

@@ -81,7 +81,7 @@ Otherwise: Apply checkpoint-based routing below.
| Verify-only | B (segmented) | Segments between checkpoints. After none/human-verify → SUBAGENT. After decision/human-action → MAIN |
| Decision | C (main) | Execute entirely in main context |
**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → spawn Task(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `isolation="worktree"` only if `workflow.use_worktrees` is not `false`** (read via `config-get workflow.use_worktrees`). **When using `isolation="worktree"`, include a `<worktree_branch_check>` block in the prompt** instructing the executor to run `git merge-base HEAD {EXPECTED_BASE}` and, if the result differs from `{EXPECTED_BASE}`, hard-reset the branch with `git reset --hard {EXPECTED_BASE}` before starting work (safe — runs before any agent work), then verify with `[ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && exit 1`. This corrects a known issue where `EnterWorktree` creates branches from `main` instead of the feature branch HEAD (affects all platforms).
**Pattern A:** init_agent_tracking → capture `EXPECTED_BASE=$(git rev-parse HEAD)` → spawn Task(subagent_type="gsd-executor", model=executor_model) with prompt: execute plan at [path], autonomous, all tasks + SUMMARY + commit, follow deviation/auth rules, report: plan name, tasks, SUMMARY path, commit hash → track agent_id → wait → update tracking → report. **Include `isolation="worktree"` only if `workflow.use_worktrees` is not `false`** (read via `config-get workflow.use_worktrees`). **When using `isolation="worktree"`, include a `<worktree_branch_check>` block in the prompt** instructing the executor to: (1) FIRST assert `git symbolic-ref HEAD` resolves to a per-agent branch (NOT a protected ref like `main`/`master`/`develop`/`trunk`/`release/*`) and HALT with a blocker if not — never self-recover via `git update-ref refs/heads/<protected>` (#2924); (2) only after that assertion passes, run `git merge-base HEAD {EXPECTED_BASE}` and, if the result differs from `{EXPECTED_BASE}`, hard-reset the branch with `git reset --hard {EXPECTED_BASE}` before starting work, then verify with `[ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && exit 1`. The HEAD assertion (Step 1) MUST run before any reset/checkout. This corrects a known issue where `EnterWorktree` creates branches from `main` instead of the feature branch HEAD (affects all platforms — #2015) and prevents the destructive HEAD-on-master self-recovery path (#2924).
**Pattern B:** Execute segment-by-segment. Autonomous segments: spawn subagent for assigned tasks only (no SUMMARY/commit). Checkpoints: main context. After all segments: aggregate, create SUMMARY, commit. See segment_execution.
@@ -239,7 +239,12 @@ See `~/.claude/get-shit-done/references/tdd.md` for structure.
Your commits may trigger pre-commit hooks. Auto-fix hooks handle themselves transparently — files get fixed and re-staged automatically.
**If running as a parallel executor agent (spawned by execute-phase):**
Use `--no-verify` on all commits. Pre-commit hooks cause build lock contention when multiple agents commit simultaneously (e.g., cargo lock fights in Rust projects). The orchestrator validates once after all agents complete.
Run commits normally — let pre-commit hooks run. Do NOT use `--no-verify` by default
(#2924). Hooks should run so issues surface at the introducing commit, and silent
bypass violates project CLAUDE.md guidance. If a project explicitly opts out via
`workflow.worktree_skip_hooks=true`, the orchestrator will surface that flag in the
prompt; absent that signal, hooks run normally. If a hook fails, follow the
sequential-mode handling below.
**If running as the sole executor (sequential mode):**
If a commit is BLOCKED by a hook:

47
get-shit-done/workflows/quick.md
View File

@@ -637,7 +637,21 @@ if [ "${USE_WORKTREES}" != "false" ]; then
COMMIT_DOCS=$(gsd-sdk query config-get commit_docs 2>/dev/null || echo "true")
if [ "$COMMIT_DOCS" != "false" ]; then
git add "${QUICK_DIR}/${quick_id}-PLAN.md"
git commit --no-verify -m "docs(${quick_id}): pre-dispatch plan for ${DESCRIPTION}" -- "${QUICK_DIR}/${quick_id}-PLAN.md" || true
# No-op skip if nothing actually staged (idempotent re-runs).
if git diff --cached --quiet -- "${QUICK_DIR}/${quick_id}-PLAN.md"; then
echo "ℹ Pre-dispatch PLAN.md commit skipped (no staged changes)"
else
# Run hooks normally (#2924). If a project opts out via
# workflow.worktree_skip_hooks=true, honor that opt-in only.
SKIP_HOOKS=$(gsd-sdk query config-get workflow.worktree_skip_hooks 2>/dev/null || echo "false")
if [ "$SKIP_HOOKS" = "true" ]; then
git commit --no-verify -m "docs(${quick_id}): pre-dispatch plan for ${DESCRIPTION}" -- "${QUICK_DIR}/${quick_id}-PLAN.md" \
|| { echo "ERROR: pre-dispatch PLAN.md commit failed (--no-verify path). Aborting before executor dispatch." >&2; exit 1; }
else
git commit -m "docs(${quick_id}): pre-dispatch plan for ${DESCRIPTION}" -- "${QUICK_DIR}/${quick_id}-PLAN.md" \
|| { echo "ERROR: pre-dispatch PLAN.md commit failed — likely a pre-commit hook failure. Fix the hook output above (or set workflow.worktree_skip_hooks=true to bypass) and re-run." >&2; exit 1; }
fi
fi
fi
fi
```
@@ -660,12 +674,31 @@ Execute quick task ${quick_id}.
${USE_WORKTREES !== "false" ? `
<worktree_branch_check>
FIRST ACTION before any other work: verify this worktree branch is based on the correct commit.
Run: git merge-base HEAD ${EXPECTED_BASE}
If the result differs from ${EXPECTED_BASE}, hard-reset to the correct base (safe — runs before any agent work):
git reset --hard ${EXPECTED_BASE}
Then verify: if [ "$(git rev-parse HEAD)" != "${EXPECTED_BASE}" ]; then echo "ERROR: Could not correct worktree base"; exit 1; fi
This corrects a known issue where EnterWorktree creates branches from main instead of the feature branch HEAD (affects all platforms).
FIRST ACTION before any other work: verify this worktree's HEAD is bound to a per-agent
branch and that the branch is based on the correct commit.
Step 1 — HEAD attachment assertion (MANDATORY, runs before any reset/commit):
HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED")
ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD)
if [ "$HEAD_REF" = "DETACHED" ] || echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then
echo "FATAL: worktree HEAD is on '$ACTUAL_BRANCH' (expected per-agent branch like worktree-agent-*)." >&2
echo "Refusing to commit/reset on a protected ref. DO NOT self-recover via 'git update-ref refs/heads/$ACTUAL_BRANCH' — that destroys concurrent work (#2924)." >&2
echo "Aborting before any commits. Surface as a blocker for human review." >&2
exit 1
fi
if ! echo "$ACTUAL_BRANCH" | grep -Eq '^worktree-agent-[A-Za-z0-9._/-]+$'; then
echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the worktree-agent-* namespace (Claude Code's per-agent worktree branch namespace)." >&2
echo "Refusing to commit; surface as blocker (#2924)." >&2
exit 1
fi
Step 2 — Base correctness (only after Step 1 passes):
Run: git merge-base HEAD ${EXPECTED_BASE}
If the result differs from ${EXPECTED_BASE}, hard-reset to the correct base (safe — Step 1 confirmed HEAD is on a per-agent branch and the worktree is fresh):
git reset --hard ${EXPECTED_BASE}
Then verify: if [ "$(git rev-parse HEAD)" != "${EXPECTED_BASE}" ]; then echo "ERROR: Could not correct worktree base"; exit 1; fi
This corrects a known issue where EnterWorktree creates branches from main instead of the feature branch HEAD (#2015) and prevents the destructive HEAD-on-master self-recovery path (#2924).
</worktree_branch_check>
` : ''}