* test(#4254): sequential executor root pin — failing-first regression + matrix The new suite executes the shipped supplied-root-pin guard against real git fixtures (drifted primary-checkout cwd halts before the write and the FATAL names both roots; matching cwd permits it; unexpanded/empty pins halt; normalization forms; submodule and sibling boundaries; metacharacter quoting; drive-letter form gate) and locks the dispatch contract across execute-phase.md, its sequential-root-pin step fragment, and worktree-path-safety.md. The #2772 per-plan serialization assertion retargets to the fragment that now carries those rules (ADR-857 Phase 6 ceiling), plus the host-step wiring. * fix(#4254): pin sequential executor to the orchestrator's validated root Sequential-mode dispatch told the executor to self-derive PROJECT_ROOT from its own cwd; every existing guard is worktree-mode-only or self-referential, so an executor spawned with a drifted cwd committed onto the wrong checkout silently. - worktree-path-safety.md step 0p: mode-agnostic supplied-root pin guard, composed by the orchestrator at build time with the literal $ORCHESTRATOR_WT (git-vs-git comparison on both sides — representation-safe on Windows, the #4296 lesson), fail-closed on empty/unexpanded pins, registered-submodule allowance, warn-and-proceed only when the dispatch carries no pin block. - execute-phase.md sequential branch: build-time embed of the bound <project_root_pin> via the new execute-phase/steps/sequential-root-pin.md fragment (ADR-857 Phase 6 frozen ceiling — the host step cannot grow; the wave serialization rules move with the fragment, verbatim in substance) plus the per-write/commit pin instruction in <sequential_execution>. Worktree-mode dispatch untouched (its self-derived toplevel IS correct there). - INVENTORY rows (5 locales) + INVENTORY-MANIFEST + install-tree goldens regenerated for the new fragment; changeset added. * chore(#4254): backfill changeset PR number * fix(#4254): accept backslash-separated Windows drive pins CI on windows-latest showed every permit-path test failing with "Actual root: <none>": pins composed from Node's path.join arrive in the backslash drive form (C:\Users\RUNNER~1\...), which the guard's absolute-form gate rejected before the cwd-side root was ever computed — a legitimate matching pin could never pass. The gate now accepts either separator ([A-Za-z]:[\\/]); git -C resolves both forms (and 8.3 short names) to the same canonical toplevel, so the git-vs-git comparison is unaffected. Form-gate tests cover the emitted (C:/…) and produced (C:\…) spellings plus short names. * fix(#4254): portable drive-form gate for MSYS bash The bracket class [\\/] that accepted backslash drive pins parses inconsistently on MSYS bash (the Windows CI leg still rejected C:\ pins — every permit-path test red with "Actual root: <none>"). Replace it with standard pattern escaping outside brackets: [A-Za-z]:/*|[A-Za-z]:\\* — the escape form is version- and build-portable. Verified across all forms: both drive spellings accepted; bare "C:", relative, empty, and unexpanded rejected. * fix(#4254): runtime-generated backslash comparator + self-describing FATAL The Windows CI legs failed every #4254 permit-path row with 'Actual root: <none>' across two prior pattern spellings ([\\/] and \\*). Stage misattribution: <none> appears whenever the FATAL fires BEFORE the cwd-side capture assigns ACTUAL_ROOT — the absolute-form gate was what fired. Mechanism: the test harness spawns bash -c <script> through the Windows command-line boundary; that round-trip applies one extra shell-quoting pass with double-quote semantics — a backslash written twice in the script text arrives halved, while a lone backslash survives (the pin displays intact; row 9's pure-bash gate independently showed the halved pattern rejecting C:\ while C:/ still passed its surviving arm). On windows-latest every pin carries backslashes (os.tmpdir() is the 8.3 short form C:\Users\RUNNER~1\...), so the gate ate every pin before the actual root was ever computed. Fix, robust by construction: - the drive-form gate generates its backslash comparator at RUNTIME (BS=$(printf '\134'); match [A-Za-z]:"$BS"*) — the shipped guard now contains no doubled backslash anywhere, enforced by a regression assertion on the extracted guard text; - the FATAL self-describes: Guard stage (pin-unbound / form-gate / actual-capture / pinned-capture / root-mismatch) plus a Diagnostic line carrying git's own stderr for capture failures and both compared values for mismatches — future platform failures name their stage in the log; - row 9's hand-rolled duplicate case gate (transit-fragile copy, #4296 Minor 1 duplication smell) is replaced by driving the SHIPPED guard and asserting the stage; rows 2/4 pin the new stage machinery. Validated on darwin across drift/match/relative/unbound/empty/bare-drive/ forward-and-backslash drive forms, each also re-run under a simulated Windows transit (every doubled backslash halved) with identical outcomes. * fix(#4254): close the empty-comparator fail-open seam in the drive-form gate Self-review of the runtime-generated backslash comparator: if printf's octal escape ever returned empty, the drive arm [A-Za-z]:"$BS"* would widen to drive-RELATIVE pins (C:foo) — the construction's one theoretical fail-open path. Fail closed with a self-describing diagnostic instead of trusting the shell's printf. --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
@@ -1,7 +1,117 @@
|
||||
# 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.
|
||||
Guards for executor agents running inside Claude Code worktrees. The
|
||||
supplied-root pin (step 0p) runs in EVERY mode; the remaining checks run before
|
||||
any staging, Edit, or Write operation in worktree mode.
|
||||
|
||||
---
|
||||
|
||||
## Supplied-root pin — step 0p (#4254, EVERY mode)
|
||||
|
||||
Sequential-mode dispatch (no `isolation="worktree"`) gives the executor no
|
||||
spawn-time cwd guarantee, and the worktree-only guards below do not apply — so
|
||||
a sequential executor whose process cwd resolved to a different checkout of
|
||||
the same repo would self-derive that checkout as its root and commit there,
|
||||
silently. Step 0p closes that hole by comparing the executor's actual root
|
||||
against a root the ORCHESTRATOR already validated — never against anything the
|
||||
executor derives itself.
|
||||
|
||||
**Runtime contract (executor):** if your prompt contains a `<project_root_pin>`
|
||||
block, run its guard script verbatim before your first Edit/Write and again
|
||||
before every commit, in the same cwd as that write or commit. On FATAL, halt
|
||||
and report — recovery (moving commits between checkouts) is an
|
||||
orchestrator/human decision, never agent self-repair. If your prompt contains
|
||||
NO `<project_root_pin>` block (worktree/isolated dispatch, or a legacy
|
||||
orchestrator), emit one warning line and continue with steps 0a/0b below — do
|
||||
not fail closed on dispatches that never carried a pin. **Never bind
|
||||
`{PINNED_ROOT}` yourself**: if this template reaches you unbound it is
|
||||
reference prose, not your pin — only the orchestrator's build-time
|
||||
substitution produces a valid guard.
|
||||
|
||||
**Composition contract (orchestrator — build time, NOT a sub-agent runtime
|
||||
step):** copy the guard below into the dispatched prompt inside a
|
||||
`<project_root_pin>` block, substituting `{PINNED_ROOT}` with the literal value
|
||||
of `$ORCHESTRATOR_WT` captured at execute_waves entry, shell-single-quoted:
|
||||
wrap the path in `'…'` and escape any embedded `'` as `'\''`. A path that
|
||||
cannot be quoted this way must halt the phase (surface a blocker) rather than
|
||||
ship a pin that could mis-parse. The comparison is git-vs-git on BOTH sides —
|
||||
`git -C` resolves the pinned path to its repo's canonical toplevel in git's
|
||||
own path representation, so symlink aliases, trailing slashes, `/var` vs
|
||||
`/private/var` spellings, and Windows drive-letter forms — forward- or
|
||||
backslash-separated, `RUNNER~1`-style short names included — compare equal by
|
||||
construction (shell `pwd -P` normalization does NOT match git's emission on
|
||||
Windows — do not re-introduce it).
|
||||
|
||||
Two portability rules baked into the guard below, learned from the #4254 CI
|
||||
Windows legs: (1) a backslash comparator must be GENERATED at runtime
|
||||
(`printf '\134'`), because a backslash written twice in the script text does
|
||||
not survive the Windows command-line round-trip into bash — the doubled form
|
||||
arrives halved, which silently rewrites any escape pattern that relies on it;
|
||||
(2) every FATAL names its `Guard stage` and, where a git capture failed,
|
||||
git's own stderr in a `Diagnostic` line, so a platform failure self-describes
|
||||
instead of surfacing as a bare `Actual root: <none>`.
|
||||
|
||||
```bash
|
||||
# gsd:guard=supplied-root-pin (#4254) — run before the first Edit/Write and before every commit.
|
||||
PINNED_ROOT='{PINNED_ROOT}' # orchestrator build-time substitution — the only valid source of this value
|
||||
PIN_STAGE=''
|
||||
PIN_DIAG=''
|
||||
gsd_pin_fail() {
|
||||
echo "FATAL: executor root does not match the orchestrator-supplied PROJECT_ROOT pin (#4254)." >&2
|
||||
echo " Pinned root: ${PINNED_ROOT:-<empty or unexpanded>}" >&2
|
||||
echo " Actual root: ${ACTUAL_ROOT:-<none>}" >&2
|
||||
echo " Guard stage: ${PIN_STAGE:-<unset>}" >&2
|
||||
if [ -n "$PIN_DIAG" ]; then echo " Diagnostic: $PIN_DIAG" >&2; fi
|
||||
echo " No writes or commits are permitted from this checkout. HALT and report; recovery is an" >&2
|
||||
echo " orchestrator/human decision. Only the IMMEDIATE submodule of the pinned checkout is a" >&2
|
||||
echo " legitimate other cwd — nested submodules must surface as a blocker, not self-route." >&2
|
||||
exit 1
|
||||
}
|
||||
# Backslash comparator, generated at runtime: a backslash written twice in this
|
||||
# script does not survive the Windows spawn path into bash (the command-line
|
||||
# round-trip halves the doubled form), which rejected every C:\ pin at the form
|
||||
# gate on the #4254 CI Windows legs. printf's octal escape is a lone backslash,
|
||||
# which does survive; the quoted expansion below is literal in a case pattern.
|
||||
BS=$(printf '\134')
|
||||
# Fail closed if the comparator could not be generated: an empty BS would widen
|
||||
# the drive-form arm below to drive-RELATIVE pins (C:foo) — the one fail-open
|
||||
# seam in this construction, closed loudly rather than trusted to the shell.
|
||||
if [ -z "$BS" ]; then
|
||||
PIN_STAGE=form-gate
|
||||
PIN_DIAG='backslash comparator generation failed (printf octal escape returned empty)'
|
||||
gsd_pin_fail
|
||||
fi
|
||||
case "$PINNED_ROOT" in
|
||||
''|'{PINNED_ROOT}') PIN_STAGE=pin-unbound; gsd_pin_fail ;; # empty or unexpanded pin — fail closed, never warn-and-proceed
|
||||
/*) ;; # absolute POSIX form
|
||||
[A-Za-z]:/*|[A-Za-z]:"$BS"*) ;; # Windows drive form, forward- or backslash-separated
|
||||
*) PIN_STAGE=form-gate; gsd_pin_fail ;; # relative pin — never trustworthy across cwds
|
||||
esac
|
||||
ACTUAL_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
|
||||
if [ -z "$ACTUAL_ROOT" ]; then
|
||||
PIN_STAGE=actual-capture
|
||||
PIN_DIAG="git rev-parse --show-toplevel from the cwd failed: $(git rev-parse --show-toplevel 2>&1 1>/dev/null)"
|
||||
gsd_pin_fail
|
||||
fi
|
||||
PINNED_TL=$(git -C "$PINNED_ROOT" rev-parse --show-toplevel 2>/dev/null)
|
||||
if [ -z "$PINNED_TL" ]; then
|
||||
PIN_STAGE=pinned-capture
|
||||
PIN_DIAG="git -C <pinned root> rev-parse --show-toplevel failed: $(git -C "$PINNED_ROOT" rev-parse --show-toplevel 2>&1 1>/dev/null)"
|
||||
gsd_pin_fail
|
||||
fi
|
||||
if [ "$ACTUAL_ROOT" != "$PINNED_TL" ]; then
|
||||
# Registered-submodule allowance: sub_repos plans legitimately commit inside an
|
||||
# immediate submodule of the pinned checkout. The superproject working tree is
|
||||
# git-emitted in the same representation as PINNED_TL, so the equality is
|
||||
# representation-safe on every platform.
|
||||
SUPER_TL=$(git rev-parse --show-superproject-working-tree 2>/dev/null)
|
||||
if [ "$SUPER_TL" != "$PINNED_TL" ]; then
|
||||
PIN_STAGE=root-mismatch
|
||||
PIN_DIAG="actual=${ACTUAL_ROOT} pinned=${PINNED_TL} superproject=${SUPER_TL:-<none>}"
|
||||
gsd_pin_fail
|
||||
fi
|
||||
fi
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -815,14 +815,23 @@ increases monotonically across waves. `{status}` is `complete` (success),
|
||||
|
||||
**Sequential mode** (`USE_WORKTREES_FOR_PLAN` is `false` — either project-level `USE_WORKTREES=false`, or per-plan submodule intersection forced it false in step 2.5):
|
||||
|
||||
Omit `isolation="worktree"` from the Agent call. Replace the `<parallel_execution>` block with:
|
||||
Omit `isolation="worktree"` from the Agent call. Before composing the prompt, read and execute
|
||||
`execute-phase/steps/sequential-root-pin.md` (#4254) — it owns the sequential root-pin build-time
|
||||
embed and the wave serialization rules.
|
||||
|
||||
Replace the `<parallel_execution>` block with:
|
||||
|
||||
```
|
||||
<sequential_execution>
|
||||
You are running as a SEQUENTIAL executor agent on the main working tree.
|
||||
Use normal git commits (with hooks). Do NOT use --no-verify.
|
||||
Run the `<project_root_pin>` guard before your first Edit/Write and before every commit (#4254).
|
||||
REQUIRED ORDER: Write SUMMARY.md → commit → only then any narration. No text between Write and commit (truncation risk; #2070 rescue is not primary defense).
|
||||
</sequential_execution>
|
||||
|
||||
<project_root_pin>
|
||||
{ORCHESTRATOR build-time embed: bound step-0p guard per sequential-root-pin.md — never this note}
|
||||
</project_root_pin>
|
||||
```
|
||||
|
||||
The sequential mode Agent prompt uses the same structure as worktree mode but with these differences in success_criteria — since there is only one agent writing at a time, there are no shared-file conflicts:
|
||||
@@ -837,8 +846,6 @@ increases monotonically across waves. `{status}` is `complete` (success),
|
||||
</success_criteria>
|
||||
```
|
||||
|
||||
When worktrees are disabled for a plan (per-plan or project-level), that plan's executor runs on the main working tree. If **any** plan in the current wave dropped to sequential mode, execute the affected plan(s) **one at a time** to avoid concurrent writes to the main working tree — plans in the same wave that retained worktree isolation can still run in parallel alongside the sequential ones, but two non-worktree plans in the same wave must serialize. When the project-level `USE_WORKTREES=false`, all plans in the wave serialize regardless of the `PARALLELIZATION` setting.
|
||||
|
||||
4. **Wait for all agents in wave to complete.**
|
||||
|
||||
**Plan-complete heartbeat (#2410):** as each executor returns (or is verified
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
# Sequential root pin (#4254)
|
||||
|
||||
Apply response_language to all user-facing prose — narration between tool calls, status
|
||||
updates, progress notes, and findings included; preserve code, paths, and identifiers.
|
||||
|
||||
Read and execute this fragment from `execute-phase.md`'s **Sequential mode** branch before
|
||||
composing any sequential dispatch prompt. It owns the sequential root-pin build-time embed,
|
||||
the `<required_reading>` root substitution, and the wave serialization rules.
|
||||
|
||||
## Root pin — ORCHESTRATOR build-time embed (#4254; NOT a sub-agent runtime step)
|
||||
|
||||
Before this dispatch, read `gsd-core/references/worktree-path-safety.md` step 0p
|
||||
("Supplied-root pin") and copy its guard template into this prompt inside a
|
||||
`<project_root_pin>` block, substituting `{PINNED_ROOT}` with the literal value of
|
||||
`$ORCHESTRATOR_WT` resolved at execute_waves entry, shell-single-quoted per that section's
|
||||
composition contract — the dispatched prompt must carry the bound, runnable guard verbatim;
|
||||
do not pass this instruction through in its place.
|
||||
|
||||
In this dispatch's `<required_reading>`, also replace the self-derivation line
|
||||
`PROJECT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)` with
|
||||
`PROJECT_ROOT='<the same literal $ORCHESTRATOR_WT>'` — a sequential executor must never
|
||||
re-derive its root from its own (possibly drifted) cwd. This substitution is sequential-mode
|
||||
ONLY: worktree-mode dispatches keep the self-derived line — an isolated executor's own
|
||||
toplevel IS its correct (and intentionally different) worktree, and substituting the
|
||||
orchestrator's root there would break every worktree-mode dispatch.
|
||||
|
||||
## Wave serialization (moved verbatim from the host step — ADR-857 Phase 6 ceiling, #1168)
|
||||
|
||||
When worktrees are disabled for a plan (per-plan or project-level), that plan's executor runs
|
||||
on the main working tree. If **any** plan in the current wave dropped to sequential mode,
|
||||
execute the affected plan(s) **one at a time** to avoid concurrent writes to the main working
|
||||
tree — plans in the same wave that retained worktree isolation can still run in parallel
|
||||
alongside the sequential ones, but two non-worktree plans in the same wave must serialize.
|
||||
When the project-level `USE_WORKTREES=false`, all plans in the wave serialize regardless of
|
||||
the `PARALLELIZATION` setting.
|
||||
Reference in New Issue
Block a user