* fix(#3021): recognize worktree-wf_* branch namespace in all guards The Claude-orchestration Workflow backend (#1143) creates per-plan worktrees on branches named worktree-wf_<runid>-<n>. Four independent copies of the agent branch allow-list regex (^(worktree-)?agent-...) never learned this namespace: - hooks/gsd-worktree-path-guard.js:176 — FAILED OPEN (process.exit(0)), silently disabling path containment for exactly the concurrent dispatch mode where cross-worktree writes are most likely - src/worktree-safety.cts:21 — silently dropped cleanup-wave manifest entries - agents/gsd-executor.md:503 — FATAL halt on branch check - gsd-core/references/worktree-branch-check.md:33 — same FATAL halt Extended all four to ^((worktree-)?agent-|worktree-wf_)[A-Za-z0-9._/-]+$. The path guard now correctly blocks cross-worktree writes for Workflow- backend branches instead of no-op'ing. * chore(#3021): backfill changeset PR number 3109 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/wise-eagles-gather.md
Normal file
5
.changeset/wise-eagles-gather.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3109
|
||||
---
|
||||
**Workflow-backend worktree branches (`worktree-wf_*`) are now recognized by all worktree guards** — the Claude-orchestration Workflow backend created worktrees on branches none of the four guards recognized, causing the path-containment hook to fail open and the cleanup/executor commands to reject or silently drop entries. All four sites now accept the `worktree-wf_` namespace alongside `agent-*` / `worktree-agent-*`. (#3021)
|
||||
@@ -500,8 +500,8 @@ if [ -f .git ]; then # worktree
|
||||
# Positive allow-list: HEAD must be on a per-agent branch (`agent-<id>` or
|
||||
# legacy `worktree-agent-<id>`). This catches feature/* and any other
|
||||
# arbitrary branch that the deny-list would silently allow (#2924, #1995).
|
||||
if ! echo "$ACTUAL_BRANCH" | grep -Eq '^(worktree-)?agent-[A-Za-z0-9._/-]+$'; then
|
||||
echo "FATAL: refusing to commit — worktree HEAD '$ACTUAL_BRANCH' is not in the agent-* / worktree-agent-* namespace." >&2
|
||||
if ! echo "$ACTUAL_BRANCH" | grep -Eq '^((worktree-)?agent-|worktree-wf_)[A-Za-z0-9._/-]+$'; then
|
||||
echo "FATAL: refusing to commit — worktree HEAD '$ACTUAL_BRANCH' is not in the agent-* / worktree-agent-* / worktree-wf_* namespace." >&2
|
||||
echo "Agent commits must live on per-agent branches; surface as blocker (#2924)." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -30,8 +30,8 @@ if [ "$HEAD_REF" = "DETACHED" ] || echo "$ACTUAL_BRANCH" | grep -Eq '^(main|mast
|
||||
echo "FATAL: worktree HEAD on '$ACTUAL_BRANCH' (expected agent-* or worktree-agent-*); refusing to commit or self-recover via 'git update-ref' (#2924)." >&2
|
||||
exit 42
|
||||
fi
|
||||
if ! echo "$ACTUAL_BRANCH" | grep -Eq '^(worktree-)?agent-[A-Za-z0-9._/-]+$'; then
|
||||
echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the agent-* / worktree-agent-* namespace; refusing to commit (#2924)." >&2
|
||||
if ! echo "$ACTUAL_BRANCH" | grep -Eq '^((worktree-)?agent-|worktree-wf_)[A-Za-z0-9._/-]+$'; then
|
||||
echo "FATAL: worktree HEAD '$ACTUAL_BRANCH' is not in the agent-* / worktree-agent-* / worktree-wf_* namespace; refusing to commit (#2924)." >&2
|
||||
exit 42
|
||||
fi
|
||||
ACTUAL_BASE=$(git rev-parse HEAD)
|
||||
|
||||
@@ -479,7 +479,7 @@ cwd inside an agent worktree (or a subdirectory of one). Every subsequent
|
||||
orchestrator-side git call would then target the wrong tree — this is how a wrong-base
|
||||
merge nearly shipped ~1000 files. Resolve the *worktree root* (so a subdirectory cwd
|
||||
cannot skew the check) and refuse if it is an agent worktree. The discriminator is the
|
||||
per-agent branch namespace `agent-*` / `worktree-agent-*`, NOT the `.claude/worktrees/` path: the
|
||||
per-agent branch namespace `agent-`/`worktree-agent-`/`worktree-wf_`, NOT the path: the
|
||||
orchestrator may itself be legitimately invoked from a feature worktree under
|
||||
`.claude/worktrees/`, so a path-substring refusal would break legitimate runs. Do NOT
|
||||
pin to `git worktree list`'s first entry — that is the main worktree, the wrong target
|
||||
@@ -490,7 +490,7 @@ when the orchestrator legitimately runs from a feature worktree.
|
||||
ORCHESTRATOR_WT=$(git rev-parse --show-toplevel 2>/dev/null) || {
|
||||
echo "FATAL: execute_waves entry is not inside a git worktree (#48)." >&2; exit 1; }
|
||||
ORCH_BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null)
|
||||
if printf '%s' "$ORCH_BRANCH" | grep -Eq '^(worktree-)?agent-'; then
|
||||
if printf '%s' "$ORCH_BRANCH" | grep -Eq '^((worktree-)?agent-|worktree-wf_)'; then
|
||||
echo "FATAL: orchestrator cwd is inside an agent worktree (branch '$ORCH_BRANCH', root '$ORCHESTRATOR_WT') — refusing to execute waves (#48). A prior isolation=\"worktree\" dispatch drifted the cwd; re-run from the orchestrator's own worktree." >&2
|
||||
# #1856 handoff: the refusal above is correct, but on its own it is a dead end —
|
||||
# this worktree may hold committed fixes AND uncommitted work, and "re-run from
|
||||
|
||||
@@ -173,7 +173,8 @@ process.stdin.on('end', () => {
|
||||
// / error → not GSD-managed → no-op.
|
||||
const branchResult = git(['symbolic-ref', '--short', 'HEAD'], cwd);
|
||||
const branch = branchResult.status === 0 && branchResult.stdout ? branchResult.stdout.trim() : '';
|
||||
if (!/^(worktree-)?agent-[A-Za-z0-9._/-]+$/.test(branch)) {
|
||||
// #3021: accept worktree-wf_<runid>-<n> branches (Workflow backend's naming).
|
||||
if (!/^((worktree-)?agent-|worktree-wf_)[A-Za-z0-9._/-]+$/.test(branch)) {
|
||||
process.exit(0); // not a GSD-managed executor worktree — no-op
|
||||
}
|
||||
|
||||
|
||||
@@ -18,7 +18,10 @@ import { execGit as execGitSeam, posixNormalize, type SpawnResultOutput } from '
|
||||
// remote, stalled NFS mount, etc.). Callers can override via deps.timeout.
|
||||
const DEFAULT_GIT_TIMEOUT_MS = 10000;
|
||||
|
||||
const WORKTREE_AGENT_BRANCH_RE = /^(worktree-)?agent-[A-Za-z0-9._/-]+$/;
|
||||
// #3021: accept the Workflow tool's worktree-wf_<runid>-<n> naming convention
|
||||
// (claude-orchestration's isolation:"worktree" emission) alongside the
|
||||
// existing agent-<id> / worktree-agent-<id> shapes.
|
||||
const WORKTREE_AGENT_BRANCH_RE = /^((worktree-)?agent-|worktree-wf_)[A-Za-z0-9._/-]+$/;
|
||||
const WORKTREE_AGENT_BRANCH_PATTERN = WORKTREE_AGENT_BRANCH_RE.source;
|
||||
|
||||
// GitResult now aliases the canonical SpawnResultOutput (shell-command-projection.cts),
|
||||
@@ -1026,7 +1029,7 @@ function planWorktreeRecordAgent(manifestRaw: string, fields: RecordAgentFields)
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'invalid_entry',
|
||||
hint: `Entry failed cleanup-manifest validation: --path/--branch/--base must be non-empty and --branch must match ${WORKTREE_AGENT_BRANCH_PATTERN} (accepts both agent-<id> and worktree-agent-<id> namespaces; got branch="${branch}"). Fix the field and re-run.`,
|
||||
hint: `Entry failed cleanup-manifest validation: --path/--branch/--base must be non-empty and --branch must match ${WORKTREE_AGENT_BRANCH_PATTERN} (accepts agent-<id>, worktree-agent-<id>, and worktree-wf_<runid> namespaces; got branch="${branch}"). Fix the field and re-run.`,
|
||||
entry: null,
|
||||
manifest: null,
|
||||
};
|
||||
@@ -1252,7 +1255,7 @@ function planWorktreeCreate(fields: WorktreeCreateFields): WorktreeCreatePlan {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'invalid_entry',
|
||||
hint: `Entry failed cleanup-manifest validation: --path/--branch/--base must be non-empty and --branch must match ${WORKTREE_AGENT_BRANCH_PATTERN} (accepts both agent-<id> and worktree-agent-<id> namespaces; got branch="${branch}"). Fix the field and re-run.`,
|
||||
hint: `Entry failed cleanup-manifest validation: --path/--branch/--base must be non-empty and --branch must match ${WORKTREE_AGENT_BRANCH_PATTERN} (accepts agent-<id>, worktree-agent-<id>, and worktree-wf_<runid> namespaces; got branch="${branch}"). Fix the field and re-run.`,
|
||||
entry: null,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"execute-phase.md": "#2930 (epic #1671 Phase 3): pilots the in-file `<!-- gsd:section -->` marker grammar by wrapping the --wave/gap-closure/regression-gate branch sections (partial-wave, gap-closure-artifacts, regression-gate) in marker pairs, proving the composeWorkflow seam runs at install time before per-runtime rewrites. Retargeted from plan-phase.md (chore/2930 review): plan-phase.md sits only 36 B under the ADR-857 Phase-6 PRE_PHASE6 gate (tests/phase6-capstone-conformance.test.cjs) and cannot absorb marker overhead, so the maintainer retargeted the pilot to execute-phase.md, which has 728 B of headroom under its own PRE_PHASE6 cap. SOURCE grows by exactly 275 marker bytes (6 marker lines); the EMITTED artifact composeWorkflow produces at install is byte-identical to the pre-#2930 file (markers are stripped, never shipped). See .gsd/phase/chore-2930-fragmentize-xl-workflow/40-design.md 'Known limits' item 5. #2639: handle_branching now warns when local is ahead of origin (+376 B condensed one-line WARNING + rev-list --count check). #2993 (epic #1671 Phase 6.2): fixes the sibling gap #2932 shipped — `flag:--wave` section gating was added to the WHEN_VOCABULARY and to init.execute-phase's section manifest, but execute-phase.md never actually parsed `--wave` out of `$ARGUMENTS` or forwarded it on the `gsd_run query init.execute-phase` line, so the flag could never fire. This diff adds a WAVE_PARAM extraction (`--wave <value>` via BASH_REMATCH) and appends it to the init call, growing the file 163 bytes (89,507 -> 89,670). #2830: the discover_and_group_plans step gained the halt-aware skip rule — plans whose blocked_by is non-empty are skipped in addition to the existing has_summary skip, and each is reported BY NAME with its cause (\"Skipping {id}: blocked by halted {…}\") rather than silently vanishing from the executable list. Growth is the added rule plus the blocked_by/runnable fields in the documented phase-plan-index parse contract, growing the file 518 bytes (89,670 -> 90,188); no other content changed. #2868: the discover_and_group_plans step no longer exits unconditionally when every plan is filtered out — it now checks whether the phase ever produced a VERIFICATION.md and, when one is missing on an unfiltered run, resumes at the tail gates (code_review_gate -> close_parent_artifacts -> regression_gate -> verify_phase_goal) instead of stranding the phase. Growth is that rule plus its filter-active and already-verified guards; no other content changed."
|
||||
"execute-phase.md": "#2930 (epic #1671 Phase 3): pilots the in-file `<!-- gsd:section -->` marker grammar by wrapping the --wave/gap-closure/regression-gate branch sections (partial-wave, gap-closure-artifacts, regression-gate) in marker pairs, proving the composeWorkflow seam runs at install time before per-runtime rewrites. Retargeted from plan-phase.md (chore/2930 review): plan-phase.md sits only 36 B under the ADR-857 Phase-6 PRE_PHASE6 gate (tests/phase6-capstone-conformance.test.cjs) and cannot absorb marker overhead, so the maintainer retargeted the pilot to execute-phase.md, which has 728 B of headroom under its own PRE_PHASE6 cap. SOURCE grows by exactly 275 marker bytes (6 marker lines); the EMITTED artifact composeWorkflow produces at install is byte-identical to the pre-#2930 file (markers are stripped, never shipped). See .gsd/phase/chore-2930-fragmentize-xl-workflow/40-design.md 'Known limits' item 5. #2639: handle_branching now warns when local is ahead of origin (+376 B condensed one-line WARNING + rev-list --count check). #2993 (epic #1671 Phase 6.2): fixes the sibling gap #2932 shipped \u2014 `flag:--wave` section gating was added to the WHEN_VOCABULARY and to init.execute-phase's section manifest, but execute-phase.md never actually parsed `--wave` out of `$ARGUMENTS` or forwarded it on the `gsd_run query init.execute-phase` line, so the flag could never fire. This diff adds a WAVE_PARAM extraction (`--wave <value>` via BASH_REMATCH) and appends it to the init call, growing the file 163 bytes (89,507 -> 89,670). #2830: the discover_and_group_plans step gained the halt-aware skip rule \u2014 plans whose blocked_by is non-empty are skipped in addition to the existing has_summary skip, and each is reported BY NAME with its cause (\"Skipping {id}: blocked by halted {\u2026}\") rather than silently vanishing from the executable list. Growth is the added rule plus the blocked_by/runnable fields in the documented phase-plan-index parse contract, growing the file 518 bytes (89,670 -> 90,188); no other content changed. #2868: the discover_and_group_plans step no longer exits unconditionally when every plan is filtered out \u2014 it now checks whether the phase ever produced a VERIFICATION.md and, when one is missing on an unfiltered run, resumes at the tail gates (code_review_gate -> close_parent_artifacts -> regression_gate -> verify_phase_goal) instead of stranding the phase. Growth is that rule plus its filter-active and already-verified guards; no other content changed. #3021: orchestrator-cwd branch check regex widened to accept worktree-wf_*."
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"gsd-executor.md": "#2943: the context7 doc-lookup block's ctx7 CLI-fallback rationale was rewritten to describe the real mechanism (custom subagents cannot see project-scoped .mcp.json; they inherit only user-scoped ~/.claude/mcp.json), replacing the wrong anthropics/claude-code#13898 'tools: frontmatter restriction' attribution. The tool name was also corrected (get-library-docs -> query-docs) and the params renamed (context7CompatibleLibraryId/topic -> libraryId/query). The +95 bytes is the longer-but-accurate mechanism description; it is the literal fix for the mis-attribution, not incidental prose growth, and the rationale must be correct because agents read it to decide when to fall back to the CLI."
|
||||
"gsd-executor.md": "#2943: the context7 doc-lookup block's ctx7 CLI-fallback rationale was rewritten to describe the real mechanism (custom subagents cannot see project-scoped .mcp.json; they inherit only user-scoped ~/.claude/mcp.json), replacing the wrong anthropics/claude-code#13898 'tools: frontmatter restriction' attribution. The tool name was also corrected (get-library-docs -> query-docs) and the params renamed (context7CompatibleLibraryId/topic -> libraryId/query). The +95 bytes is the longer-but-accurate mechanism description; it is the literal fix for the mis-attribution, not incidental prose growth, and the rationale must be correct because agents read it to decide when to fall back to the CLI. #3021 amendment: branch allow-list regex widened to accept worktree-wf_* (Workflow backend naming) alongside agent-*/worktree-agent-*."
|
||||
}
|
||||
}
|
||||
|
||||
@@ -187,10 +187,11 @@ describe('bug #2924: worktree HEAD attachment + destructive recovery', () => {
|
||||
const scripts = codeBlocks.map(({ body }) => body).join('\n');
|
||||
// Allow-list must reference the canonical Claude Code worktree-agent-<id>
|
||||
// namespace via a regex assertion (grep -Eq '^worktree-agent-...').
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(worktree-\)\?agent-/;
|
||||
// #3021: allow-list now includes worktree-wf_* alongside agent-*/worktree-agent-*
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(\(worktree-\)\?agent-\|worktree-wf_/;
|
||||
assert.ok(
|
||||
allowListRe.test(scripts),
|
||||
'worktree_branch_check must enforce a positive allow-list matching ^(worktree-)?agent-* (#2924 hardening)'
|
||||
'worktree_branch_check must enforce a positive allow-list matching ^((worktree-)?agent-|worktree-wf_) (#2924/#3021 hardening)'
|
||||
);
|
||||
});
|
||||
|
||||
@@ -322,10 +323,10 @@ describe('bug #2924: worktree HEAD attachment + destructive recovery', () => {
|
||||
});
|
||||
|
||||
test('block enforces positive worktree-agent-* allow-list (#2924 hardening)', () => {
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(worktree-\)\?agent-/;
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(\(worktree-\)\?agent-\|worktree-wf_/;
|
||||
assert.ok(
|
||||
allowListRe.test(block),
|
||||
'quick.md worktree_branch_check must enforce a positive allow-list matching ^(worktree-)?agent-* (#2924 hardening)'
|
||||
'quick.md worktree_branch_check must enforce a positive allow-list matching ^((worktree-)?agent-|worktree-wf_) (#2924/#3021 hardening)'
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -412,10 +413,10 @@ describe('bug #2924: worktree HEAD attachment + destructive recovery', () => {
|
||||
test('step 0 enforces positive worktree-agent-* allow-list (#2924 hardening)', () => {
|
||||
const codeBlocks = extractFencedCodeBlocks(block);
|
||||
const scripts = codeBlocks.map(({ body }) => body).join('\n');
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(worktree-\)\?agent-/;
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(\(worktree-\)\?agent-\|worktree-wf_/;
|
||||
assert.ok(
|
||||
allowListRe.test(scripts),
|
||||
'task_commit_protocol step 0 must enforce a positive allow-list matching ^(worktree-)?agent-* in addition to the protected-ref deny-list (#2924 hardening)'
|
||||
'task_commit_protocol step 0 must enforce a positive allow-list matching ^((worktree-)?agent-|worktree-wf_) in addition to the protected-ref deny-list (#2924/#3021 hardening)'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1138,6 +1138,9 @@ describe('worktree branch namespace boundaries (#1995)', () => {
|
||||
'worktree-agent-a1',
|
||||
'worktree-agent-abc123',
|
||||
'worktree-agent-session.42',
|
||||
// #3021: Workflow backend naming convention
|
||||
'worktree-wf_run123-1',
|
||||
'worktree-wf_execute-phase-71-env-vars-3',
|
||||
];
|
||||
const REJECTED_BRANCHES = [
|
||||
'feature-x',
|
||||
@@ -1195,7 +1198,8 @@ describe('worktree branch namespace boundaries (#1995)', () => {
|
||||
agentId: 'a1', worktreePath: '/repo/wt', branch: 'feature-x', base: 'abc123',
|
||||
});
|
||||
assert.equal(plan.ok, false);
|
||||
assert.match(plan.hint, /\(worktree-\)\?agent-\[A-Za-z0-9\._\/-\]\+/);
|
||||
// #3021: hint must now mention the worktree-wf_ namespace too
|
||||
assert.match(plan.hint, /worktree-wf_/);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -167,9 +167,9 @@ describe('canonical worktree-branch-check fragment is the single source of truth
|
||||
assert.ok(/\brelease\b/.test(block), 'fragment protected-ref alternation must include release');
|
||||
});
|
||||
|
||||
test('fragment block positive allow-list matches ^(worktree-)?agent- pattern', () => {
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(worktree-\)\?agent-/;
|
||||
assert.ok(allowListRe.test(block), 'fragment block must enforce a positive allow-list matching ^(worktree-)?agent-');
|
||||
test('fragment block positive allow-list matches ^((worktree-)?agent-|worktree-wf_) pattern', () => {
|
||||
const allowListRe = /grep\s+-Eq?\s+'\^\(\(worktree-\)\?agent-\|worktree-wf_/;
|
||||
assert.ok(allowListRe.test(block), 'fragment block must enforce a positive allow-list matching ^((worktree-)?agent-|worktree-wf_) (#3021)');
|
||||
});
|
||||
|
||||
test('fragment block contains update-ref prohibition text', () => {
|
||||
|
||||
Reference in New Issue
Block a user