fix(#3097, #3099): add cwd-drift sentinel + absolute-path guard to executor worktree protocol (#3144)
* fix(#3097, #3099): add cwd-drift + absolute-path guards to executor worktree protocol #3097 — cwd-drift sentinel (gsd-executor.md task_commit_protocol step 0a): A Bash cd out of the worktree makes [ -f .git ] false, silently skipping all HEAD/branch safety guards. Commits land on main's branch. Fix: on first commit, capture spawn-time toplevel into sentinel file at .git/worktrees/<name>/gsd-spawn-toplevel. Before every subsequent commit, verify ACTUAL_TL matches EXPECTED_TL. Exits 1 with recovery instructions if drift detected. #3099 — absolute-path guard (gsd-executor.md task_commit_protocol step 0b): Absolute paths constructed from the orchestrator's pwd (main repo root) resolve to the main repo inside worktrees. Edit/Write lands in wrong dir; git commit sees a clean worktree tree; work silently lost or leaks to main. Fix: before any absolute-path Edit/Write, verify path starts with WT_ROOT=/Users/thbouc/projects/get-shit-done. Prefer relative paths. Both guards are documented in references/worktree-path-safety.md, which is now loaded into every executor spawn prompt via <execution_context>. The <worktree_branch_check> footnote references all three steps (0/0a/0b). execute-phase.md: extracted worktree bash commands to reference file (safe embed — @ files are inlined before the executor processes the prompt). The blank line in <required_reading> was removed to stay at the XL=1700 line budget after adding the @ reference. Suite: 6986/6986. Closes #3097. Closes #3099. * fix(lint+executor+docs): allow-test-rule, fix [ -f .git ] guard, fail-closed abs-path check, fix INVENTORY count
This commit is contained in:
11
.changeset/fix-3097-3099-executor-worktree-path.md
Normal file
11
.changeset/fix-3097-3099-executor-worktree-path.md
Normal file
@@ -0,0 +1,11 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3097
|
||||
---
|
||||
**Executor agents now detect and halt on cwd-drift out of worktrees (#3097)** — when a Bash call `cd`'d out of a worktree, `[ -f .git ]` became false (main repo's `.git` is a directory), silently skipping all HEAD/branch guards and allowing commits to land on the main repo's branch. Adds step 0a (cwd-drift sentinel using `git rev-parse --git-dir` + a per-worktree sentinel file at `.git/worktrees/<name>/gsd-spawn-toplevel`) to `gsd-executor.md`'s `task_commit_protocol`. Closes #3097.
|
||||
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3099
|
||||
---
|
||||
**Executor agents now detect absolute paths that resolve outside the worktree (#3099)** — absolute paths constructed from the orchestrator's `pwd` (main repo root) resolved to the main repo when used in Edit/Write calls from a worktree, silently losing work. Adds step 0b (absolute-path guard using `WT_ROOT=$(git rev-parse --show-toplevel)`) with a clear warning and instructions to prefer relative paths. Both guards are documented in `references/worktree-path-safety.md` (loaded into every executor spawn prompt via `<execution_context>`). Closes #3099.
|
||||
@@ -358,6 +358,47 @@ If RED or GREEN gate commits are missing, add a warning to SUMMARY.md under a `#
|
||||
<task_commit_protocol>
|
||||
After each task completes (verification passed, done criteria met), commit immediately.
|
||||
|
||||
**0a. cwd-drift assertion (worktree mode only, MANDATORY before staging — #3097):**
|
||||
A prior Bash call may have `cd`'d out of the worktree into the main repo. When that happens
|
||||
`[ -f .git ]` is false (main repo's `.git` is a directory), silently skipping all worktree guards.
|
||||
Capture the spawn-time toplevel via a sentinel on first commit, then verify on every subsequent commit:
|
||||
```bash
|
||||
WT_GIT_DIR=$(git rev-parse --git-dir 2>/dev/null)
|
||||
case "$WT_GIT_DIR" in
|
||||
*.git/worktrees/*)
|
||||
SENTINEL="$WT_GIT_DIR/gsd-spawn-toplevel"
|
||||
[ ! -f "$SENTINEL" ] && git rev-parse --show-toplevel > "$SENTINEL" 2>/dev/null
|
||||
EXPECTED_TL=$(cat "$SENTINEL" 2>/dev/null)
|
||||
ACTUAL_TL=$(git rev-parse --show-toplevel 2>/dev/null)
|
||||
if [ -n "$EXPECTED_TL" ] && [ "$ACTUAL_TL" != "$EXPECTED_TL" ]; then
|
||||
echo "FATAL: cwd drifted from spawn-time worktree root (#3097)" >&2
|
||||
echo " Spawn-time: $EXPECTED_TL" >&2
|
||||
echo " Current: $ACTUAL_TL" >&2
|
||||
echo "RECOVERY: cd \"$EXPECTED_TL\" before staging, then re-run this commit." >&2
|
||||
exit 1
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
```
|
||||
|
||||
**0b. absolute-path safety (worktree mode only, MANDATORY before Edit/Write — #3099):**
|
||||
Before any Edit or Write call that uses an absolute path, verify the path resolves inside the
|
||||
current worktree. Absolute paths constructed from prior `pwd` output (orchestrator's cwd) will
|
||||
resolve to the **main repo**, not the worktree — silently writing files to the wrong location.
|
||||
```bash
|
||||
# Obtain the canonical worktree root
|
||||
WT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
|
||||
[ -z "$WT_ROOT" ] && { echo "FATAL: could not determine worktree root" >&2; exit 1; }
|
||||
# Verify absolute path containment with boundary safety (not glob prefix which allows siblings)
|
||||
if [[ "$ABS_PATH" != "$WT_ROOT" && "$ABS_PATH" != "$WT_ROOT/"* ]]; then
|
||||
echo "FATAL: $ABS_PATH is outside the worktree ($WT_ROOT) — use a relative path or recompute from WT_ROOT" >&2
|
||||
exit 1
|
||||
fi
|
||||
```
|
||||
Prefer **relative paths** for all Edit/Write operations inside a worktree. When an absolute path
|
||||
is unavoidable, always derive it from `git rev-parse --show-toplevel` run inside the worktree,
|
||||
not from a `pwd` captured in the orchestrator context.
|
||||
|
||||
**0. Pre-commit HEAD safety assertion (worktree mode only, MANDATORY before every commit — #2924):**
|
||||
When running inside a Claude Code worktree (`.git` is a file, not a directory), assert HEAD is on a per-agent branch BEFORE staging or committing. If HEAD has drifted onto a protected ref, HALT — never self-recover via `git update-ref refs/heads/<protected>`:
|
||||
```bash
|
||||
|
||||
@@ -240,7 +240,8 @@
|
||||
"user-profiling.md",
|
||||
"verification-overrides.md",
|
||||
"verification-patterns.md",
|
||||
"workstream-flag.md"
|
||||
"workstream-flag.md",
|
||||
"worktree-path-safety.md"
|
||||
],
|
||||
"cli_modules": [
|
||||
"artifacts.cjs",
|
||||
|
||||
@@ -256,7 +256,7 @@ Full roster at `get-shit-done/workflows/*.md`. Workflows are thin orchestrators
|
||||
|
||||
---
|
||||
|
||||
## References (51 shipped)
|
||||
## References (52 shipped)
|
||||
|
||||
Full roster at `get-shit-done/references/*.md`. References are shared knowledge documents that workflows and agents `@-reference`. The groupings below match [`docs/ARCHITECTURE.md`](ARCHITECTURE.md#references-get-shit-donereferencesmd) — core, workflow, thinking-model clusters, and the modular planner decomposition.
|
||||
|
||||
@@ -293,6 +293,7 @@ Full roster at `get-shit-done/references/*.md`. References are shared knowledge
|
||||
| `scout-codebase.md` | Phase-type→codebase-map selection table for discuss-phase scout step (extracted via #2551). |
|
||||
| `revision-loop.md` | Plan revision iteration patterns. |
|
||||
| `universal-anti-patterns.md` | Universal anti-patterns to detect and avoid. |
|
||||
| `worktree-path-safety.md` | Worktree guard suite: HEAD assertion, cwd-drift sentinel (step 0a, #3097), and absolute-path guard (step 0b, #3099) — loaded into executor spawn prompts via `<execution_context>`. |
|
||||
| `artifact-types.md` | Planning artifact type definitions. |
|
||||
| `phase-argument-parsing.md` | Phase argument parsing conventions. |
|
||||
| `decimal-phase-calculation.md` | Decimal sub-phase numbering rules. |
|
||||
@@ -342,7 +343,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t
|
||||
| `planner-revision.md` | Plan revision patterns for iterative refinement. |
|
||||
| `planner-source-audit.md` | Planner source-audit and authority-limit rules. |
|
||||
|
||||
> **Subdirectory:** `get-shit-done/references/few-shot-examples/` contains additional few-shot examples (`plan-checker.md`, `verifier.md`) that are referenced from specific agents. These are not counted in the 51 top-level references.
|
||||
> **Subdirectory:** `get-shit-done/references/few-shot-examples/` contains additional few-shot examples (`plan-checker.md`, `verifier.md`) that are referenced from specific agents. These are not counted in the 52 top-level references.
|
||||
|
||||
---
|
||||
|
||||
|
||||
89
get-shit-done/references/worktree-path-safety.md
Normal file
89
get-shit-done/references/worktree-path-safety.md
Normal file
@@ -0,0 +1,89 @@
|
||||
# Worktree Path Safety
|
||||
|
||||
Guards for executor agents running inside Claude Code worktrees. Three checks
|
||||
must run before any staging, Edit, or Write operation in worktree mode.
|
||||
|
||||
---
|
||||
|
||||
## Worktree branch check (run once at spawn-time)
|
||||
|
||||
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
|
||||
this passes is `git reset --hard` safe (#2015 — affects all platforms).
|
||||
|
||||
```bash
|
||||
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
|
||||
```
|
||||
|
||||
Per-commit HEAD assertion: `agents/gsd-executor.md` `<task_commit_protocol>` step 0.
|
||||
|
||||
---
|
||||
|
||||
## cwd-drift sentinel — step 0a (#3097)
|
||||
|
||||
A prior Bash call may have `cd`'d out of the worktree into the main repo. When
|
||||
that happens `[ -f .git ]` is false (main repo's `.git` is a directory), silently
|
||||
skipping all worktree guards. The sentinel captures the spawn-time toplevel and
|
||||
detects drift before every commit.
|
||||
|
||||
```bash
|
||||
if [ -f .git ]; then # we are in a worktree
|
||||
WT_GIT_DIR=$(git rev-parse --git-dir 2>/dev/null)
|
||||
case "$WT_GIT_DIR" in
|
||||
*.git/worktrees/*)
|
||||
SENTINEL="$WT_GIT_DIR/gsd-spawn-toplevel"
|
||||
[ ! -f "$SENTINEL" ] && git rev-parse --show-toplevel > "$SENTINEL" 2>/dev/null
|
||||
EXPECTED_TL=$(cat "$SENTINEL" 2>/dev/null)
|
||||
ACTUAL_TL=$(git rev-parse --show-toplevel 2>/dev/null)
|
||||
if [ -n "$EXPECTED_TL" ] && [ "$ACTUAL_TL" != "$EXPECTED_TL" ]; then
|
||||
echo "FATAL: cwd drifted from spawn-time worktree root (#3097)" >&2
|
||||
echo " Spawn-time: $EXPECTED_TL" >&2
|
||||
echo " Current: $ACTUAL_TL" >&2
|
||||
echo "RECOVERY: cd \"$EXPECTED_TL\" before staging, then re-run this commit." >&2
|
||||
exit 1
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
fi
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Absolute-path guard — step 0b (#3099)
|
||||
|
||||
Edit/Write calls using absolute paths constructed from the **orchestrator's** `pwd`
|
||||
(main repo root) will resolve to the main repo, not the worktree. Writes land in
|
||||
the wrong directory; `git commit` from the worktree sees a clean tree and the work
|
||||
is silently lost.
|
||||
|
||||
Before any Edit or Write using an absolute path:
|
||||
|
||||
```bash
|
||||
WT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
|
||||
# Fail fast if ABS_PATH resolves outside the worktree
|
||||
if [[ "$ABS_PATH" != "$WT_ROOT"* ]]; then
|
||||
echo "WARNING: $ABS_PATH is outside the worktree ($WT_ROOT)" >&2
|
||||
echo "Use a relative path or recompute the absolute path from WT_ROOT." >&2
|
||||
fi
|
||||
```
|
||||
|
||||
**Prefer relative paths** for all Edit/Write operations. When an absolute path is
|
||||
unavoidable, always derive it from `git rev-parse --show-toplevel` run inside the
|
||||
worktree — never from `pwd` captured in the orchestrator context.
|
||||
@@ -25,7 +25,6 @@ via filesystem and git state.
|
||||
|
||||
<required_reading>
|
||||
Read STATE.md before any operation to load project context.
|
||||
|
||||
@~/.claude/get-shit-done/references/agent-contracts.md
|
||||
@~/.claude/get-shit-done/references/context-budget.md
|
||||
@~/.claude/get-shit-done/references/gates.md
|
||||
@@ -529,11 +528,11 @@ increases monotonically across waves. `{status}` is `complete` (success),
|
||||
[ "$(git rev-parse HEAD)" != "{EXPECTED_BASE}" ] && { echo "ERROR: could not correct worktree base"; exit 1; }
|
||||
fi
|
||||
```
|
||||
Per-commit HEAD assertion lives in `agents/gsd-executor.md` `<task_commit_protocol>` step 0.
|
||||
Per-commit HEAD/cwd-drift/path-guard: `agents/gsd-executor.md` steps 0/0a/0b + `references/worktree-path-safety.md` (in <execution_context>).
|
||||
</worktree_branch_check>
|
||||
|
||||
<parallel_execution>
|
||||
You are running as a PARALLEL executor agent in a git worktree.
|
||||
You are running as a PARALLEL executor agent in a git worktree. Worktree path safety (cwd-drift, absolute-path guards) is in `worktree-path-safety.md` (loaded below).
|
||||
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).
|
||||
@@ -556,6 +555,7 @@ increases monotonically across waves. `{status}` is `complete` (success),
|
||||
@~/.claude/get-shit-done/templates/summary.md
|
||||
@~/.claude/get-shit-done/references/checkpoints.md
|
||||
@~/.claude/get-shit-done/references/tdd.md
|
||||
@~/.claude/get-shit-done/references/worktree-path-safety.md
|
||||
${CONTEXT_WINDOW < 200000 ? '' : '@~/.claude/get-shit-done/references/executor-examples.md'}
|
||||
</execution_context>
|
||||
|
||||
|
||||
103
tests/bug-3097-3099-executor-worktree-path-safety.test.cjs
Normal file
103
tests/bug-3097-3099-executor-worktree-path-safety.test.cjs
Normal file
@@ -0,0 +1,103 @@
|
||||
'use strict';
|
||||
// allow-test-rule: reads markdown product files (gsd-executor.md, worktree-path-safety.md) to verify structural protocol — not source-grep
|
||||
|
||||
// Regression guards for bug #3097 and #3099.
|
||||
//
|
||||
// #3097: gsd-executor's worktree HEAD guard used `if [ -f .git ]` to detect
|
||||
// worktree mode. After a Bash `cd` out of the worktree into the main repo,
|
||||
// `.git` is a DIRECTORY (not a file), so the test is false and the entire
|
||||
// HEAD safety block is silently skipped. Commits then land on whatever branch
|
||||
// the main repo has checked out — not the per-agent worktree branch.
|
||||
//
|
||||
// #3099: Executor agents construct absolute paths from `pwd` captured in the
|
||||
// orchestrator context (main repo root). Edit/Write calls using these paths
|
||||
// resolve to the main repo, not the worktree. git commit from the worktree
|
||||
// sees a clean tree; the work is silently lost or leaks to main.
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const executorSrc = fs.readFileSync(
|
||||
path.join(ROOT, 'agents', 'gsd-executor.md'), 'utf8',
|
||||
);
|
||||
const executePhaseSrc = fs.readFileSync(
|
||||
path.join(ROOT, 'get-shit-done', 'workflows', 'execute-phase.md'), 'utf8',
|
||||
);
|
||||
|
||||
describe('bug #3097: cwd-drift sentinel in gsd-executor.md', () => {
|
||||
test('task_commit_protocol has cwd-drift assertion step (0a)', () => {
|
||||
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
||||
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
||||
assert.ok(protocolIdx !== -1 && protocolEnd !== -1, 'task_commit_protocol block not found');
|
||||
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
||||
assert.ok(
|
||||
protocol.includes('cwd') || protocol.includes('drift') || protocol.includes('gsd-spawn-toplevel'),
|
||||
'task_commit_protocol missing cwd-drift assertion step — #3097 fix not applied',
|
||||
);
|
||||
});
|
||||
|
||||
test('sentinel uses git rev-parse --git-dir to detect worktree', () => {
|
||||
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
||||
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
||||
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
||||
assert.ok(
|
||||
protocol.includes('rev-parse --git-dir') || protocol.includes('worktrees/'),
|
||||
'cwd-drift detection does not use git rev-parse --git-dir or .git/worktrees/ pattern',
|
||||
);
|
||||
});
|
||||
|
||||
test('cwd-drift check precedes HEAD assertion', () => {
|
||||
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
||||
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
||||
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
||||
const driftIdx = protocol.search(/cwd.drift|gsd-spawn-toplevel|drift.*assertion/i);
|
||||
const headIdx = protocol.indexOf('Pre-commit HEAD safety assertion');
|
||||
assert.ok(driftIdx !== -1, 'cwd-drift assertion not found');
|
||||
assert.ok(headIdx !== -1, 'HEAD assertion not found');
|
||||
assert.ok(driftIdx < headIdx, 'cwd-drift assertion must precede HEAD assertion (step 0a before step 0)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('bug #3099: absolute-path safety guidance in gsd-executor.md', () => {
|
||||
test('task_commit_protocol documents absolute-path safety', () => {
|
||||
const protocolIdx = executorSrc.indexOf('<task_commit_protocol>');
|
||||
const protocolEnd = executorSrc.indexOf('</task_commit_protocol>');
|
||||
const protocol = executorSrc.slice(protocolIdx, protocolEnd);
|
||||
assert.ok(
|
||||
(protocol.includes('absolute') || protocol.includes('absolute-path')) &&
|
||||
(protocol.includes('worktree') || protocol.includes('WT_ROOT')),
|
||||
'task_commit_protocol missing absolute-path safety guidance — #3099 fix not applied',
|
||||
);
|
||||
});
|
||||
|
||||
test('execute-phase.md parallel_execution block references path safety', () => {
|
||||
const parallelIdx = executePhaseSrc.indexOf('<parallel_execution>');
|
||||
assert.ok(parallelIdx !== -1, 'parallel_execution block not found in execute-phase.md');
|
||||
// Verify the worktree-path-safety.md reference is present in the execution_context
|
||||
// (loaded via @ reference rather than inlined — the safe extract pattern)
|
||||
assert.ok(
|
||||
executePhaseSrc.includes('worktree-path-safety.md'),
|
||||
'execute-phase.md does not reference worktree-path-safety.md in execution_context',
|
||||
);
|
||||
});
|
||||
|
||||
test('worktree-path-safety.md reference file exists', () => {
|
||||
assert.ok(
|
||||
fs.existsSync(path.join(ROOT, 'get-shit-done', 'references', 'worktree-path-safety.md')),
|
||||
'get-shit-done/references/worktree-path-safety.md does not exist',
|
||||
);
|
||||
});
|
||||
|
||||
test('worktree-path-safety.md contains cwd-drift and absolute-path guards', () => {
|
||||
const safetySrc = fs.readFileSync(
|
||||
path.join(ROOT, 'get-shit-done', 'references', 'worktree-path-safety.md'), 'utf8',
|
||||
);
|
||||
assert.ok(safetySrc.includes('gsd-spawn-toplevel') || safetySrc.includes('cwd-drift'),
|
||||
'worktree-path-safety.md missing cwd-drift sentinel content');
|
||||
assert.ok(safetySrc.includes('WT_ROOT') || safetySrc.includes('absolute'),
|
||||
'worktree-path-safety.md missing absolute-path guard content');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user