diff --git a/.changeset/serene-bears-travel.md b/.changeset/serene-bears-travel.md new file mode 100644 index 000000000..367f3aac1 --- /dev/null +++ b/.changeset/serene-bears-travel.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 4343 +--- +**Executor commits now refuse to land on the planning repo's default/protected branch** — the pre-commit guard in the executor agent widened to run in every isolation mode (not just Claude Code worktrees) and now resolves the repository's actual default branch instead of a hardcoded five-name list, with a new `git.allow_default_branch_commits` config escape hatch for projects that intentionally execute on their default branch. (#3819) diff --git a/agents/gsd-executor.md b/agents/gsd-executor.md index eab2ac008..0f54fd324 100644 --- a/agents/gsd-executor.md +++ b/agents/gsd-executor.md @@ -479,19 +479,30 @@ Prefer **relative paths** for all Edit/Write operations inside a worktree. When 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/`: +**0. Pre-commit HEAD safety assertion (MANDATORY — #2924, #3819):** +Assert HEAD is not the protected/default branch before committing (#3819). If drifted onto it, HALT — never self-recover via `git update-ref refs/heads/`: ```bash -if [ -f .git ]; then # worktree - HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED") - ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD) - # Deny-list: never commit on a protected ref. - if [ "$HEAD_REF" = "DETACHED" ] || \ - echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then - echo "FATAL: refusing to commit — worktree HEAD is on '$ACTUAL_BRANCH' (expected per-agent branch)." >&2 - echo "DO NOT use 'git update-ref' to rewind the protected branch — surface as blocker (#2924)." >&2 - exit 1 +HEAD_REF=$(git symbolic-ref --quiet HEAD || echo "DETACHED") +ACTUAL_BRANCH=$(git rev-parse --abbrev-ref HEAD) +if [ "$HEAD_REF" = "DETACHED" ]; then + echo "FATAL: refusing to commit — HEAD is detached." >&2 + exit 1 +fi +# #3819: real default branch; override git.allow_default_branch_commits; else five-name fallback. +IS_PROTECTED=$(gsd_run query git.base-branch --is-protected "$ACTUAL_BRANCH" 2>/dev/null) || IS_PROTECTED="__GSD_RUN_UNAVAILABLE__" +if [ "$IS_PROTECTED" = "__GSD_RUN_UNAVAILABLE__" ] || [ -z "$IS_PROTECTED" ]; then + if echo "$ACTUAL_BRANCH" | grep -Eq '^(main|master|develop|trunk|release/.*)$'; then + IS_PROTECTED="true" + else + IS_PROTECTED="false" fi +fi +if [ "$IS_PROTECTED" != "false" ]; then + echo "FATAL: refusing to commit — HEAD is on '$ACTUAL_BRANCH' (protected/default branch)." >&2 + echo "Re-home onto a phase/agent branch (#2924, #3819); override: git.allow_default_branch_commits:true in .planning/config.json." >&2 + exit 1 +fi +if [ -f .git ]; then # worktree # Positive allow-list: HEAD must be on a per-agent branch (`agent-` or # legacy `worktree-agent-`). This catches feature/* and any other # arbitrary branch that the deny-list would silently allow (#2924, #1995). @@ -583,8 +594,7 @@ back, those deletions appear on the main branch, destroying prior-wave work (#20 - `git rm` on files not explicitly created by the current task - `git checkout -- .` or `git restore .` (blanket working-tree resets that discard files) - `git reset --hard` except inside the `` step at agent startup -- `git update-ref refs/heads/` (where protected is `main`, `master`, - `develop`, `trunk`, or `release/*`). This is an absolute prohibition (#2924). +- `git update-ref refs/heads/` (resolved protected branch, #2924, #3819). Prohibited. If you discover that your worktree HEAD is attached to a protected branch and your commits landed there, **DO NOT** "recover" by force-rewinding the protected ref — that silently destroys concurrent commits in multi-active scenarios (parallel @@ -802,6 +812,7 @@ gsd_run query state.add-blocker --text "Blocker description" +This commit must re-run the Step 0 assertion above (#3819). ```bash gsd_run query commit "docs({phase}-{plan}): complete [plan-name] plan" --files \ .planning/phases/XX-name/{phase}-{plan}-SUMMARY.md .planning/STATE.md .planning/ROADMAP.md .planning/REQUIREMENTS.md diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 0b695edad..41cc2a3f0 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1194,6 +1194,7 @@ All four fields are **optional and additive** — STATE.md files without them ke | `git.branching_strategy` | enum | `none` | `none`, `phase`, or `milestone` | | `git.base_branch` | string | `main` | The integration branch that phase/milestone branches are created from and merged back into. Override when your repo uses `master` or a release branch | | `git.protected_branches` | array of non-empty strings | (none) | Optional additional shared branches that should trigger protected-branch warnings alongside the resolved base branch | +| `git.allow_default_branch_commits` | boolean | `false` | Escape hatch (#3819): when `true`, the executor's pre-commit guard no longer refuses to commit on the resolved default branch. Explicitly configured `git.protected_branches` names are still enforced. | | `git.create_tag` | boolean | `true` | Create a git tag (`v[X.Y]`) on milestone completion. Set to `false` for projects with their own release flow | | `git.phase_branch_template` | string | `gsd/phase-{phase}-{slug}` | Branch name template for phase strategy | | `git.milestone_branch_template` | string | `gsd/{milestone}-{slug}` | Branch name template for milestone strategy | @@ -1224,6 +1225,25 @@ still apply. } ``` +### Escape Hatch: Committing on the Default Branch + +Some projects intentionally run GSD directly on their default branch (no branch-per-phase +workflow). For those, `git.allow_default_branch_commits: true` tells the executor's pre-commit +guard (see `agents/gsd-executor.md`) to stop refusing commits on the resolved default branch. +It narrows only the *automatic* default-branch protection — any branch name explicitly listed +in `git.protected_branches` stays protected regardless of this flag. + +```json +{ + "git": { + "allow_default_branch_commits": true + } +} +``` + +This does not change what gets committed, commit message format, or behavior on any +non-default branch — see issue #3819. + ### Strategy Comparison | Strategy | Creates Branch | Scope | Merge Point | Best For | @@ -2213,6 +2233,7 @@ Two different rules apply, and the difference is deliberate ([#3532](https://git global `effort` block keeps working in projects and does not trigger the warning. - **The whole `git.*` namespace is project-scoped and never resolves from the global file**, in either directory shape — not `git.base_branch`, not `git.protected_branches`, not + `git.allow_default_branch_commits`, not `git.branching_strategy` or the branch templates. Branch policy is a property of the repository, not of the machine, so it is read only from that project's `.planning/config.json`. A `git` block in `~/.gsd/defaults.json` still seeds new projects diff --git a/gsd-core/bin/shared/config-schema.manifest.json b/gsd-core/bin/shared/config-schema.manifest.json index 2ea6033c9..8313928d5 100644 --- a/gsd-core/bin/shared/config-schema.manifest.json +++ b/gsd-core/bin/shared/config-schema.manifest.json @@ -45,6 +45,7 @@ "git.branching_strategy", "git.base_branch", "git.protected_branches", + "git.allow_default_branch_commits", "git.create_tag", "git.phase_branch_template", "git.milestone_branch_template", diff --git a/src/config-loader.cts b/src/config-loader.cts index 111630551..fdafaaee3 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -902,6 +902,7 @@ function loadConfigResolved(cwd: string, options: Record = {}): branching_strategy: get('branching_strategy', { section: 'git', field: 'branching_strategy' }) ?? defaults.branching_strategy, base_branch: get('base_branch', { section: 'git', field: 'base_branch' }), protected_branches: getNested('git', 'protected_branches'), + allow_default_branch_commits: getNested('git', 'allow_default_branch_commits'), phase_branch_template: get('phase_branch_template', { section: 'git', field: 'phase_branch_template' }) ?? defaults.phase_branch_template, milestone_branch_template: get('milestone_branch_template', { section: 'git', field: 'milestone_branch_template' }) ?? defaults.milestone_branch_template, quick_branch_template: get('quick_branch_template', { section: 'git', field: 'quick_branch_template' }) ?? defaults.quick_branch_template, diff --git a/src/git-base-branch.cts b/src/git-base-branch.cts index 7c22943d1..5edbb463e 100644 --- a/src/git-base-branch.cts +++ b/src/git-base-branch.cts @@ -77,6 +77,15 @@ interface EffectiveGitConfig { protectedBranches: string[]; /** Rendered form of every entry rejected as unusable, for the caller to report. */ rejectedProtectedBranches: string[]; + /** + * `git.allow_default_branch_commits` escape hatch (#3819). When `true`, the + * resolved base branch is no longer auto-added to `protectedBranches` — for + * a project that legitimately runs GSD directly on its default branch. + * Explicitly configured `protected_branches` entries are unaffected: this + * flag narrows only the automatic base-branch protection, never a name the + * project named on purpose. + */ + allowDefaultBranchCommits: boolean; } /** Render a rejected config value for a diagnostic without throwing on exotic input. */ @@ -150,6 +159,7 @@ function readEffectiveGitConfig( config = { base_branch: _readGitKey(normalized, 'base_branch'), protected_branches: _readGitNested(normalized, 'protected_branches'), + allow_default_branch_commits: _readGitNested(normalized, 'allow_default_branch_commits'), }; } } catch { /* malformed direct edit contributes no policy values */ } @@ -186,7 +196,11 @@ function readEffectiveGitConfig( rejectedProtectedBranches.push(renderRejected(rawProtectedBranches)); } - return { baseBranch, protectedBranches, rejectedProtectedBranches }; + // #3819: nested-only read (mirrors protected_branches — new key, no legacy + // flat form, so a top-level spelling must not become an undocumented alias). + const allowDefaultBranchCommits = config.allow_default_branch_commits === true; + + return { baseBranch, protectedBranches, rejectedProtectedBranches, allowDefaultBranchCommits }; } /** @@ -374,6 +388,8 @@ export interface ProtectedBranchStatus { rejectedProtectedBranches: string[]; isProtected: boolean; verified: boolean; + /** Mirrors the `git.allow_default_branch_commits` config value (#3819). */ + allowDefaultBranchCommits: boolean; } /** Resolve the base branch plus configured protected-branch extensions. */ @@ -388,13 +404,17 @@ export function resolveProtectedBranchStatus( effectiveConfig.baseBranch, deps, ); - const protectedBranches = [...new Set([baseBranch, ...effectiveConfig.protectedBranches])]; + const protectedBranches = [...new Set([ + ...(effectiveConfig.allowDefaultBranchCommits ? [] : [baseBranch]), + ...effectiveConfig.protectedBranches, + ])]; return { baseBranch, protectedBranches, rejectedProtectedBranches: effectiveConfig.rejectedProtectedBranches, isProtected: protectedBranches.includes(currentBranch), verified, + allowDefaultBranchCommits: effectiveConfig.allowDefaultBranchCommits, }; } diff --git a/tests/git-base-branch.test.cjs b/tests/git-base-branch.test.cjs index 46120320e..bd2ec7bfc 100644 --- a/tests/git-base-branch.test.cjs +++ b/tests/git-base-branch.test.cjs @@ -609,6 +609,7 @@ describe('#3552: configured protected branches', () => { rejectedProtectedBranches: [], isProtected: true, verified: true, + allowDefaultBranchCommits: false, }); assert.strictEqual(control.isProtected, false); assert.notStrictEqual(match.isProtected, control.isProtected, @@ -1087,6 +1088,355 @@ describe('#3552: configured protected branches', () => { }); }); +// ─── #3819: allow_default_branch_commits escape hatch ──────────────────────── + +describe('#3819: allow_default_branch_commits escape hatch', () => { + test('#3819 allow_default_branch_commits:true + currentBranch on base branch → not protected', () => { + const loadConfig = () => ({ base_branch: 'main', allow_default_branch_commits: true }); + const status = gitBaseBranch.resolveProtectedBranchStatus('/repo', 'main', { loadConfig }); + + assert.strictEqual(status.isProtected, false, + 'the escape hatch must exempt the base branch from auto-included protection'); + assert.deepStrictEqual(status.protectedBranches, [], + 'the base branch must not appear in protectedBranches when the escape hatch is on'); + assert.strictEqual(status.allowDefaultBranchCommits, true); + }); + + test('#3819 explicit protected_branches still applies with the escape hatch on', () => { + const loadConfig = () => ({ + base_branch: 'main', + allow_default_branch_commits: true, + protected_branches: ['develop'], + }); + const onBase = gitBaseBranch.resolveProtectedBranchStatus('/repo', 'main', { loadConfig }); + const onExplicit = gitBaseBranch.resolveProtectedBranchStatus('/repo', 'develop', { loadConfig }); + + assert.strictEqual(onBase.isProtected, false, + 'the base branch escapes protection even though an explicit list is also configured'); + assert.strictEqual(onExplicit.isProtected, true, + 'an explicitly configured protected branch must still be enforced despite the escape hatch'); + assert.notStrictEqual(onBase.isProtected, onExplicit.isProtected, + 'the two cases must disagree — otherwise the explicit list is not actually being enforced'); + }); + + test('#3819 negative control: allow_default_branch_commits absent → base-branch protection unchanged', () => { + const loadConfig = () => ({ base_branch: 'main' }); + const status = gitBaseBranch.resolveProtectedBranchStatus('/repo', 'main', { loadConfig }); + + assert.strictEqual(status.allowDefaultBranchCommits, false, + 'default must be false/off when the key is not set at all'); + assert.strictEqual(status.isProtected, true, + 'without the escape hatch, the base branch must remain protected as before #3819'); + }); + + test('#3819 allow_default_branch_commits:false explicitly → same as absent', () => { + const loadConfig = () => ({ base_branch: 'main', allow_default_branch_commits: false }); + const status = gitBaseBranch.resolveProtectedBranchStatus('/repo', 'main', { loadConfig }); + + assert.strictEqual(status.allowDefaultBranchCommits, false); + assert.strictEqual(status.isProtected, true, + 'an explicit false must behave identically to leaving the key unset'); + }); + + test('#3819 CLI: git.allow_default_branch_commits:true makes --is-protected report false on the base branch', (t) => { + const dir = createGitRepo({ prefix: 'gsd-3819-cli-', defaultBranch: 'main' }); + t.after(() => cleanup(dir)); + addPlanning(dir); + setGsdConfig(dir, 'git.allow_default_branch_commits', true); + + const escaped = runGsdTools(['query', 'git.base-branch', '--is-protected', 'main'], dir); + assert.ok(escaped.success, escaped.error); + assert.strictEqual(escaped.output, 'false', + 'the escape hatch must make the CLI report the base branch as not protected'); + + // Negative control: a sibling fixture without the config key must still + // report the base branch as protected — proves the flag, not some other + // difference between the two fixtures, drives the result above. + const controlDir = createGitRepo({ prefix: 'gsd-3819-cli-control-', defaultBranch: 'main' }); + t.after(() => cleanup(controlDir)); + addPlanning(controlDir); + const control = runGsdTools(['query', 'git.base-branch', '--is-protected', 'main'], controlDir); + assert.ok(control.success, control.error); + assert.strictEqual(control.output, 'true'); + assert.notStrictEqual(escaped.output, control.output, + 'CLI negative control must disagree with the escape-hatch fixture'); + }); +}); + +// ─── #3819: gsd-executor.md pre-commit protected-branch guard ──────────────── + +/** + * Extract the FIRST ```bash fenced block that follows the "Pre-commit + * protected-branch safety assertion" heading in agents/gsd-executor.md. + * Keyed off the heading text (not a `` tag — this step has no + * such wrapper) so a future rewording of the heading fails the test loudly + * instead of silently extracting nothing. + */ +function extractExecutorPreCommitBash() { + const executorPath = path.join(__dirname, '..', 'agents', 'gsd-executor.md'); + const content = readFileNormalized(executorPath); + const lines = content.split('\n'); + const headingIndex = lines.findIndex( + (line) => line.includes('Pre-commit HEAD safety assertion'), + ); + if (headingIndex === -1) { + throw new Error( + 'agents/gsd-executor.md: could not find the "Pre-commit HEAD safety ' + + 'assertion" heading — has it been reworded?', + ); + } + let inBash = false; + const buffer = []; + for (let i = headingIndex + 1; i < lines.length; i++) { + const line = lines[i]; + if (!inBash && /^\s*```bash\s*$/.test(line)) { + inBash = true; + continue; + } + if (inBash && /^\s*```\s*$/.test(line)) { + return buffer.join('\n'); + } + if (inBash) buffer.push(line); + } + throw new Error( + 'agents/gsd-executor.md: no ```bash block found after the pre-commit protected-branch ' + + 'heading — has the step been restructured?', + ); +} + +/** + * Write a standalone script that mocks `git` and `gsd_run`, then runs the + * extracted pre-commit guard bash verbatim. Deliberately does NOT `set -e`: + * the guard's fatal paths call `exit 1` explicitly, and this script must run + * to completion on the non-fatal path to print the GUARD_PASSED sentinel. + * + * `isWorktree` controls whether `scriptDir` gets a `.git` FILE (worktree) or + * a `.git` DIRECTORY (ordinary checkout) — the guard's own `[ -f .git ]` + * branch reads this from the script's cwd, so the test runs the script with + * `cwd: scriptDir`. + */ +function writeExecutorGuardScript(prefix, bash, { branch, headRef, isWorktree, queryResult, queryExit = 0 }) { + const scriptDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const gitPath = path.join(scriptDir, '.git'); + if (isWorktree) { + fs.writeFileSync(gitPath, 'gitdir: /nonexistent\n'); + } else { + fs.mkdirSync(gitPath); + } + const scriptPath = path.join(scriptDir, 'guard.sh'); + fs.writeFileSync(scriptPath, [ + '#!/usr/bin/env bash', + 'git() {', + ' if [ "$1" = symbolic-ref ]; then', + headRef === 'DETACHED' + ? ' return 1' + : ` printf "%s\\n" "${headRef}"\n return 0`, + ' elif [ "$1" = rev-parse ] && [ "$2" = --abbrev-ref ]; then', + ` printf "%s\\n" "${branch}"`, + ' return 0', + ' else', + ' printf "unexpected git invocation: %s\\n" "$*" >&2', + ' return 97', + ' fi', + '}', + 'gsd_run() {', + ` if [ "$#" -ne 4 ] || [ "$1" != query ] || [ "$2" != git.base-branch ] || ` + + `[ "$3" != --is-protected ] || [ "$4" != "${branch}" ]; then`, + ' printf "unexpected gsd_run invocation: %s\\n" "$*" >&2', + ' return 0', + ' fi', + queryExit + ? ` return ${queryExit}` + : ` printf "%s\\n" "${queryResult}"`, + '}', + bash, + 'printf "GUARD_PASSED\\n"', + ].join('\n'), { mode: 0o755 }); + return { scriptDir, scriptPath }; +} + +describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => { + const bash = extractExecutorPreCommitBash(); + + test('#3819 non-worktree, branch is protected (query says true) → HALT with exit 1, no GUARD_PASSED', (t) => { + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-nonwt-protected-', bash, { + branch: 'main', + headRef: 'main', + isWorktree: false, + queryResult: 'true', + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 1, result.stderr); + assert.match(result.stderr, /protected\/default branch/); + assert.doesNotMatch(result.stdout, /GUARD_PASSED/); + }); + + test('#3819 non-worktree, branch is not protected (query says false) → continues, GUARD_PASSED', (t) => { + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-nonwt-clean-', bash, { + branch: 'feature-x', + headRef: 'feature-x', + isWorktree: false, + queryResult: 'false', + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 0, result.stderr); + assert.match(result.stdout, /GUARD_PASSED/); + }); + + test('#3819 worktree, branch is protected → HALT with exit 1 (protected check fires before the allow-list check)', (t) => { + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-wt-protected-', bash, { + branch: 'main', + headRef: 'main', + isWorktree: true, + queryResult: 'true', + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 1, result.stderr); + assert.match(result.stderr, /protected\/default branch/); + }); + + test('#3819 worktree, branch not protected but outside the agent-*/worktree-agent-*/worktree-wf_* namespace → HALT', (t) => { + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-wt-outside-namespace-', bash, { + branch: 'some-random-branch', + headRef: 'some-random-branch', + isWorktree: true, + queryResult: 'false', + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 1, result.stderr); + assert.match(result.stderr, /not in the agent-\* \/ worktree-agent-\* \/ worktree-wf_\* namespace/); + }); + + test('#3819 worktree, branch not protected AND in the agent-* namespace → continues, GUARD_PASSED', (t) => { + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-wt-clean-', bash, { + branch: 'agent-42', + headRef: 'agent-42', + isWorktree: true, + queryResult: 'false', + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 0, result.stderr); + assert.match(result.stdout, /GUARD_PASSED/); + }); + + test('#3819 detached HEAD (non-worktree) → HALT with exit 1 before any protected-branch query', (t) => { + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-detached-', bash, { + branch: 'HEAD', + headRef: 'DETACHED', + isWorktree: false, + queryResult: 'false', + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 1, result.stderr); + assert.match(result.stderr, /detached/); + }); + + test('#3819 HOSTILE: gsd_run RUNS but reports the branch as protected (resolver-internal fail-closed) → HALT', (t) => { + // This is the scenario that must stay fail-closed unconditionally: + // gsd_run itself succeeds (exit 0) — e.g. because cmdGitBaseBranch's own + // pre-existing #3057 B4 logic already answered "true" when it could not + // VERIFY the real default branch — and the bash guard must never + // second-guess that answer. It is deliberately indistinguishable, at + // this bash layer, from an ordinary "yes this branch is protected" + // answer (see the "non-worktree, branch is protected" test above) — + // that IS the fail-closed contract: the guard trusts what gsd_run says + // when gsd_run actually says something. + // + // This is a DIFFERENT failure mode from "gsd_run could not be invoked at + // all" (nonzero exit / no output), which is covered by the two + // "gsd_run itself is unavailable" tests below and — per the #3819 + // approval's item 2 ("keep the existing names as a fallback where the + // default cannot be resolved") — deliberately does NOT fail closed for + // every branch; it falls back to the pre-#3819 five-name list instead. + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-resolver-unverified-', bash, { + branch: 'feature-x', + headRef: 'feature-x', + isWorktree: false, + queryResult: 'true', + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 1, result.stderr); + assert.match(result.stderr, /protected\/default branch/); + assert.doesNotMatch(result.stdout, /GUARD_PASSED/); + }); + + test('#3819 gsd_run itself is unavailable (query cannot run) + branch matches the old five-name list → still HALTs via the fallback', (t) => { + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-unavailable-listed-', bash, { + branch: 'develop', + headRef: 'develop', + isWorktree: false, + queryResult: 'false', + queryExit: 1, + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 1, result.stderr); + assert.match(result.stderr, /protected\/default branch/); + }); + + test('#3819 gsd_run itself is unavailable (query cannot run) + branch does NOT match the old five-name list → falls back to allowing it', (t) => { + // Deliberate #3819-approved tradeoff: when gsd-tools itself cannot even be + // invoked, an unlisted branch is allowed through rather than blocking every + // branch outright — unlike the "HOSTILE: the protected-branch query itself + // fails" test above (gsd-tools runs fine but the resolver's own internal + // verification fails), which stays fail-closed. + const { scriptDir, scriptPath } = writeExecutorGuardScript('gsd-3819-guard-unavailable-unlisted-', bash, { + branch: 'my-feature-branch', + headRef: 'my-feature-branch', + isWorktree: false, + queryResult: 'false', + queryExit: 1, + }); + t.after(() => cleanup(scriptDir)); + + const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 }); + + assert.strictEqual(result.exitCode, 0, result.stderr); + assert.match(result.stdout, /GUARD_PASSED/); + }); + + // allow-test-rule: source-text-is-the-product + // Justification: this test's whole point is asserting on the prose text of the + // block itself — the workflow .md IS the product surface here, + // same rationale as Guard G (see the top-of-file exemption above). + test('#3819 block instructs the executor to re-run the Step 0 guard before the final commit', () => { + const executorPath = path.join(__dirname, '..', 'agents', 'gsd-executor.md'); + const content = readFileNormalized(executorPath); + const startIndex = content.indexOf(''); + assert.notStrictEqual(startIndex, -1, 'agents/gsd-executor.md: could not find tag — has it been renamed?'); + const fenceIndex = content.indexOf('```bash', startIndex); + const excerpt = fenceIndex === -1 + ? content.slice(startIndex, startIndex + 2000) + : content.slice(startIndex, fenceIndex); + + assert.match(excerpt, /re-run the Step 0/); + assert.match(excerpt, /#3819/); + }); +}); + // ─── #3648 Major 1: negative space for readEffectiveGitConfig's readFile seam ─ // // The #3057 W3 suite that pinned readConfigBaseBranch's unusable-config arms diff --git a/tests/no-bare-gsd-tools-command-position.test.cjs b/tests/no-bare-gsd-tools-command-position.test.cjs index 0bf31e207..355272893 100644 --- a/tests/no-bare-gsd-tools-command-position.test.cjs +++ b/tests/no-bare-gsd-tools-command-position.test.cjs @@ -103,7 +103,7 @@ const BARE_COMMAND_RE = new RegExp( // Each entry MUST carry a one-line reason; the test prints the allowlist on // failure so a reviewer can see exactly what is sanctioned. const PROSE_ALLOWLIST = [ - { file: 'agents/gsd-executor.md', line: 812, reason: 'describes the SDK return envelope of `gsd-tools query commit`; not an instruction to run the bare word' }, + { file: 'agents/gsd-executor.md', line: 823, reason: 'describes the SDK return envelope of `gsd-tools query commit`; not an instruction to run the bare word' }, { file: 'agents/gsd-phase-researcher.md', line: 33, reason: 'package-legitimacy provenance rule names the command as the source of an OK verdict; descriptive' }, { file: 'agents/gsd-roadmapper.md', line: 647, reason: 'parenthetical "e.g." naming SDK queries a user *could* run; not an agent instruction' }, { file: 'agents/gsd-intel-updater.md', line: 40, reason: 'cross-platform note names the `gsd-tools intel ` CLI surface descriptively ("CLI invocations go through..."); not an agent instruction' },