Files
msd-core/tests/git-base-branch.test.cjs
Tom Boucher 53ea8e0664 fix(#3057): make a guard's failure distinguishable from its benign result — Wave 1 (#3088)
* fix(#3057): refuse the write when the duplicate scan cannot complete

writeManifest documents itself as a fail-closed duplicate guard: if any
existing manifest shares plan_id with a different, non-terminal job_id it must
refuse, because dispatching again would duplicate the external job.

It could not honour that. The scan reads every sibling manifest looking for the
duplicate, and an unreadable or unparseable sibling was `continue`d past. If
the corrupt file was the one holding the live duplicate, the scan found nothing
and a duplicate external job dispatched.

The asymmetry is what gives it away: a malformed TARGET refused with
malformed_existing because clobbering is unacceptable, while a malformed
SIBLING was skipped — yet siblings are the only thing the duplicate check
reads.

Adds a scan_incomplete verdict that refuses and names the offending file, so an
operator can quarantine or repair it. Fail-closed alone would let one stale
corrupt manifest wedge every dispatch for that planning dir permanently; naming
the file is what makes refusing survivable. malformed_existing is untouched, so
the target/sibling distinction stays visible. The docstring is updated — it
previously stated a rule the function did not keep.

memFs() gains an optional failReads map so these branches are reachable at all;
they had zero coverage because the fake could not express a per-file read
fault. The signature is additive and every existing caller is unchanged.

The regression is proved by a pair, not a single test. A control writes a
readable sibling holding a genuine non-terminal duplicate and asserts
duplicate_plan_id, establishing the scenario is real; the regression then makes
that same path unreadable and asserts scan_incomplete. A first draft of this
test used a corrupt-JSON fixture containing no plan_id at all while its comment
claimed otherwise — it duplicated the unparseable-sibling case and proved
nothing, which is the defect class this phase exists to remove.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3057): make a guard's failure distinguishable from its benign result

Wave 1 of the negative-space backfill: the branches where a guard that could
not verify something reported the same value it reports when everything is
fine. That indistinguishability is the defect; every fix here makes the two
states tellable apart, and every test proves it with a pair — one for the
failure, one for the benign case. A single test cannot establish that two
states are distinguishable, which is the whole property being fixed.

state.cts phaseInventoryProvider returned null for both a real disk-scan
failure and a genuinely empty phases dir, so `state rebuild` could report
success while phase-table reconciliation never ran. It now returns a
discriminated result and the CLI surfaces phase_inventory_scan_failed plus a
reason. The reason field turned out never to have been wired into the emitted
JSON at all — it existed only as an internal variable — so a test could only
assert on the operator-facing note. It is a real field now.

state.cts treated an unreadable lock body the same as an empty one, applying
the 1-second stealable floor. A lock we cannot read is not a lock we know is
stale; an unreadable body is now held to the deadman ceiling like a live
holder.

verification.cts findStaleVerificationSummary returned null on any fs, scan or
clock failure — meaning "not stale". It now returns a discriminated
StaleCheckResult and the caller records that the check was indeterminate.

git-base-branch resolveBaseBranch returned 'main' both when no candidate branch
existed and when every git tier timed out. A diagnostics variant now reports
whether the answer was verified, and the CLI writes an unverified-fallback note
to stderr. The stdout contract five workflows parse is untouched.

worktree-safety snapshotWorktreeInventory left exists:true when statSync threw,
so a guard that could not check reported the worktree present; exists is now
tri-state and a stat failure surfaces as an 'unverified' finding.
planWorktreePrune reported 'no_worktrees' for a parse failure, which is not the
same as an empty list — and it drives a prune. It now reports 'parse_failed'.

Fixing the inventory change exposed a second fail-open in verify.cts: the
validate-health consumer silently dropped findings whose kind it did not
recognise, so the new kind would have vanished. That is closed too — worth
noting that the survey enumerated producers of degraded verdicts, not consumers
that discard them.

worktree-base-ref and state-transition gain the distinguishing signal without
changing what they do: headAbsenceVerified, and a phase-inventory scan meta.
Whether those guards should ACT differently is a product question this change
does not answer, and both are flagged rather than quietly settled.

rescueSummaryArtifacts is left alone: rescuing on an uncertain cat-file is
deliberate per #2556. It now has tests proving it, and a recorded negative
finding — git cat-file -e returns 128 for both "absent from HEAD" and a fatal
error, so "uncertain" and "certain-and-fine" are not separable at the git
level.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3057): assert typed values, not rendered text

Ten assertions in the rebuild CLI suite matched substrings of produced output —
STATE.md body fields, a markdown table row, an audit-log heading, and JSON keys
read as text. CONTRIBUTING prohibits that: if the code under test produces
text, the test asserts on its structured surface instead.

No production surface had to be built. Every one already existed and was
already compiled into bin/lib: stateExtractField for body fields,
parseMarkdownTable for the phase table, collectSection for the audit-log
section, and result.data.log — already a typed RebuildLogEntry[]. The tests
were matching rendered text sitting next to the structured data.

One of those assertions was passing for the wrong reason. `stdout.includes
('rebuilt')` matched the JSON KEY name, not a value: the dry-run path emits
`mutated` and the real path emits `rebuilt`, so it would have passed whether
the value was true or false. It now asserts the value.

external-job's refusal already had to name the offending file — that naming is
why the fail-closed variant is survivable rather than a permanent wedge — but
the tests proved it by substring of a prose message. The failure result now
carries offendingPath as its own field and the tests assert it by value. The
human message is unchanged; operators read it.

Array membership is left alone. `phaseIds.includes('99')` and
`result.updated.includes('Completed Phases')` are membership checks on real
arrays, not text matching, and converting them would weaken nothing and clarify
nothing.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3057): execute acquireStateLock instead of grepping its source

The non-EEXIST lock test asserted on the TEXT of the built .cjs and never
called acquireStateLock. It carried an allow-test-rule: architectural-invariant
exemption to permit that. A source grep proves a literal is present in a file,
not that the behaviour works — it is weaker than a liveness test, which at
least runs the code, and it was the only coverage the fatal-errno path had.

Replaced with tests that inject the errno through fs and assert what actually
happens: a fatal EACCES propagates out of acquireStateLock with zero backoff
sleeps, while EAGAIN/EINTR/EINVAL/EIO/ENOENT/ESTALE/EPERM/EBUSY retry once and
succeed. The exemption is removed and its allowlist entry with it.

One old assertion is deliberately not carried over: it checked the retryable
errnos were expressed as a Set rather than an inline literal. That is a shape
check with no runtime signature; the behavioural tests fail if the code reverts
to the old inline check, which is the regression it was really guarding.

The #3057 lock-body tests move into that same file rather than a new one, which
is what lint-test-file-count asks for and puts every acquireStateLock test in
one place.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3057): surface an indeterminate staleness check to its callers

An isolated review caught an inconsistency inside this wave. Two of the three
"add the distinguishing signal" fixes wire through to something a user sees:
git base-branch writes an unverified-fallback diagnostic to stderr, and an
unverifiable worktree surfaces as a W020 finding. The third set
staleCheckIndeterminate on readVerificationStatus's result and nothing read it.

A signal nobody consumes leaves the fail-open exactly as silent as before: the
staleness check could fail and the operator saw precisely what they would see
if the answer were genuinely "not stale". That is the defect this issue exists
to remove, so it is not defensible as scaffolding when its two siblings in the
same change already wire through.

All five callers now surface it, each through the channel it already had rather
than a mechanism imposed uniformly: phase complete adds it to its existing
warnings array and, on the blocked path, as an additive note on the error text;
init and roadmap carry it as a field on output they already emit; the UAT
report carries it without ever gating passed/blockers; workstream inventory
takes an injectable writeDiagnostic mirroring the git base-branch idiom,
because its return shape had nowhere to hang a per-phase field without
rippling the builder's types.

The routing decision is unchanged everywhere. What changes is only that a
caller and an operator can now tell a failed check from a completed one.

That diagnostic carries structured meta rather than being asserted by regex —
the default still writes only the human message to stderr, but tests assert
phaseDir and reason by value. Two earlier assertions in this branch were
converted the same way; this was the last raw-text assertion left.

Also records a scope correction: the completePhaseCore guards now compare
stateReplaceField's result to the body instead of testing truthiness, so a
field whose substitution produced identical text no longer reports as updated.
That is a real behaviour fix, not the signal-only change this file was
described as carrying, and its tests cover both the changed and unchanged
cases.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3057): bound two heavy subprocesses for a loaded bench, not an idle one

The remote matrix surfaced three failures unrelated to this branch's changes.
All were bad tests, and a re-run would have hidden every one of them.

The reviewer-flags parse block bounded bash -> node -> a full gsd-tools cold
start at 5 seconds. On a bench running thirty thousand tests in parallel that
is not a hang, it is a busy machine. Raised to 30s, matching the convention
sibling suites already use for script invocations, with a comment saying what
the budget covers so nobody tightens it back. Two further copies of the same
5-second spawn in the same file had the identical defect and are raised too —
they were not in the failure report, but they will be next time.

The fragment-propagation test bounded npm run regen:derived — a full build plus
eight generators, the heaviest subprocess in the suite — at five minutes, and
node22 was killed near the end. The captured output proves it: every generator
had written its files and gen:install-tree had emitted all fifteen runtimes
before the kill. Raised to fifteen minutes.

That failure read as `null !== 0`, which says nothing. status null means killed,
not a non-zero exit, and the two want different responses: one is a timeout to
size correctly, the other is a real build break. The assertion now distinguishes
them and names the signal.

Neither test's assertions were weakened and no retry was added. A retry here
would suppress exactly the signal the timeout exists to produce.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3057): capture fd 1 through the mock tracker, not a raw reassignment

The phase suite reported zero test results on both lanes while running for five
and a half minutes and exiting 1. No assertion text, no stderr, four events for
the whole file: enqueue, start, dequeue, complete. That shape is not a failing
assertion — it is the runner being unable to read the child at all, because it
parses its event stream from the child's stdout.

The cause was the capture helper reassigning fs.writeSync directly. Proven
rather than assumed: a standalone probe patched fs.writeSync and called
process.stdout.write, and the interception fired only when fd 1 resolved to a
FILE, not when it was a pipe. The remote runner captures the event stream to a
file, so a helper that was invisible against a pipe swallowed the reporter's own
output on the bench. That is also why the two sibling suites wired the same way
in this change pass cleanly — they use the mock tracker, the seam io.test.cjs
established for this exact function.

The helper now uses t.mock.method with an explicit restore after each call, so
teardown belongs to node:test rather than a second hand-rolled implementation,
and the interception cannot outlive the one synchronous call it wraps even if
that call throws. Ten call sites thread the test context through; three test
callbacks gained the parameter they lacked.

The three B3 tests are untouched — same assertions, same fault injection. Only
how the context reaches the helper changed.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(#3057): capture phase-complete output from a subprocess, not fd 1

Two attempts to make in-process fd-1 interception safe both failed on the
bench. The suite reported zero test results on either lane while exiting 1 —
four events for the whole file — because the runner parses its event stream
from the child's stdout, and process.stdout.write routes through fs.writeSync
whenever fd 1 resolves to a file, which is how the runner captures. Patching
that seam anywhere in a file can therefore destroy the file's own reporting,
and tightening the window only moved the runtime from 326s to 125s without
recovering a single event.

So the interception is gone rather than tuned. The helper now spawns gsd-tools
as a real subprocess and reads stdout the way the OS already gives it to us,
which is what the rest of the suite does. It asserts the command succeeded
before parsing, so a genuine failure can no longer present as a JSON parse
error.

The two fault-injecting tests could not survive that move as written: a
subprocess cannot see a mock installed in the parent. Instead of reinstating
the interception they now produce the fault on disk — the summary artifact is
created as a dangling symlink, so the staleness check's real statSync throws
inside the child. That is a more honest fixture than a mock in any case, since
it is a condition a user's tree can actually be in. Skipped on Windows, matching
the existing symlink precedent in the write-guard suite.

Three further call sites turned out to depend on parent-process writeFileSync
mocks the subprocess could not see. Those call the CJS function directly, which
is what they always wanted — they never needed stdout at all.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(#3057): one name for one signal, one encoding for one distinction

Standards review found four things this branch introduced, all of them
inconsistencies with itself rather than with the repo.

One upstream bit reached its consumers under three names —
verification_stale_check_indeterminate in two modules, the same value with
"stale" dropped in a third, and stderr only in the fourth. Standardised on the
long name wherever it is a field. The workstream inventory keeps its stderr
channel, since its return shape has nowhere to hang a per-phase field without
rippling the builder's types, but it now says the same word for the same thing.

worktree-safety encoded one three-way distinction two ways in a single file: a
named union for a finding's kind, and boolean|null for an inventory entry's
existence. The second is now a named union too.

Two assertions matched human prose because the blocked and non-blocked
completion paths carried no typed field for the signal. Both now assert typed
values. The first round of this fix added the field but left the regex beside
it, which is the banned pattern sitting next to its own replacement; the second
removed it and added an assertion on the reason enum so nothing was lost.

The remaining two were reasoned away before being fixed, and both reasons were
bad. "No typed surface exists" is the condition CONTRIBUTING says to fix by
adding one — it took three lines. "The file already does this dozens of times"
is not licence to add instance number thirty-one; a convention that violates a
documented rule is debt, not precedent.

Vocabulary differing across DIFFERENT modules is left alone: CONTEXT.md rejects
a single shared result envelope, so per-module shapes are precedented, and a
baseline smell does not outrank a documented standard.

A census of every line this branch adds to a test file now finds no regex or
substring assertion on produced prose: 87 strictEqual, 25 ok (all non-empty or
shape guards), 12 equal, 3 throws (all typed err.code predicates), 3
deepStrictEqual, 2 notStrictEqual.

Refs #3051

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore(#3057): backfill changeset pr number to 3088

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-05 16:00:52 -04:00

773 lines
34 KiB
JavaScript

'use strict';
/**
* #1146: git.base-branch resolver — single source of truth for default-branch detection.
*
* Tests:
* A. Config override wins (git.base_branch set → returned as-is, no git calls needed)
* B. origin/HEAD symref resolves → used
* C. origin/HEAD unset but git remote show origin knows HEAD → AUTHORITATIVE fallback
* (key regression: master repo with no origin/HEAD → must return "master", NOT "main")
* D. No origin/HEAD, no remote show, local branch "master" present → returns "master"
* E. No origin/HEAD, no remote show, local branch "main" present → returns "main"
* F. No origin/HEAD, no remote show, no local branches → returns "main" (last resort)
* G. Anti-regression guard: five affected workflows must NOT contain the
* duplicated bare `:-main` / `:-master` fallback pattern that was the root cause.
* They must call `gsd_run query git.base-branch` instead.
* (allow-test-rule: runtime-contract-is-the-product — the workflow .md content IS
* the runtime surface; the absence of the bad pattern is what ships to agents.)
*/
// allow-test-rule: runtime-contract-is-the-product
// Justification: the workflow .md files ARE the product surface — agents read and
// execute them directly. Guard G asserts that the resolved command appears in all five
// workflows, which requires reading those workflow files. Per TESTING-STANDARDS.md §6.
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { execSync } = require('node:child_process');
const { runGsdTools, cleanup, readFileNormalized } = require('./helpers.cjs');
const { makeFaultyGit } = require('./helpers/faulty-deps.cjs');
// ─── helpers ──────────────────────────────────────────────────────────────────
/**
* Create a minimal git repo in a temp dir, optionally setting up a remote
* and local branches.
*/
function createGitRepo(opts = {}) {
const { prefix = 'gsd-1146-', defaultBranch = 'master' } = opts;
const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
execSync(`git init -b ${defaultBranch}`, { cwd: dir, stdio: 'pipe' });
execSync('git config user.email "test@test.com"', { cwd: dir, stdio: 'pipe' });
execSync('git config user.name "Test"', { cwd: dir, stdio: 'pipe' });
execSync('git config commit.gpgsign false', { cwd: dir, stdio: 'pipe' });
// Need at least one commit so branches exist
fs.writeFileSync(path.join(dir, 'README.md'), '# test\n');
execSync('git add README.md', { cwd: dir, stdio: 'pipe' });
execSync('git commit -m "init"', { cwd: dir, stdio: 'pipe' });
return dir;
}
/**
* Create a .planning dir so gsd-tools resolveProjectRoot doesn't bail.
*/
function addPlanning(dir) {
fs.mkdirSync(path.join(dir, '.planning', 'phases'), { recursive: true });
}
/**
* Write a gsd config.json with git.base_branch set.
*/
function setGsdConfig(dir, key, value) {
const cfgDir = path.join(dir, '.planning');
fs.mkdirSync(cfgDir, { recursive: true });
const cfgPath = path.join(cfgDir, 'config.json');
let cfg = {};
try { cfg = JSON.parse(fs.readFileSync(cfgPath, 'utf8')); } catch (_) { /* new file */ }
// Set nested key (dot notation). Guard every segment against prototype
// pollution with inline literal checks at each write site — mirrors the
// production guard in src/config.cts. A Set/pre-loop guard is NOT recognised
// by CodeQL's js/prototype-pollution-utility query (see PR #752 / alert #40).
const parts = key.split('.');
let obj = cfg;
for (let i = 0; i < parts.length - 1; i++) {
const k = parts[i];
if (k === '__proto__' || k === 'prototype' || k === 'constructor') {
throw new Error(`setGsdConfig: unsafe config key segment '${k}'`);
}
if (typeof obj[k] !== 'object' || obj[k] === null) obj[k] = {};
obj = obj[k];
}
const lastKey = parts[parts.length - 1];
if (lastKey === '__proto__' || lastKey === 'prototype' || lastKey === 'constructor') {
throw new Error(`setGsdConfig: unsafe config key segment '${lastKey}'`);
}
obj[lastKey] = value;
fs.writeFileSync(cfgPath, JSON.stringify(cfg, null, 2) + '\n');
}
// Paths to the five affected workflow files
const WORKFLOW_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows');
const AFFECTED_WORKFLOWS = [
path.join(WORKFLOW_DIR, 'execute-phase.md'),
path.join(WORKFLOW_DIR, 'quick.md'),
path.join(WORKFLOW_DIR, 'ship.md'),
path.join(WORKFLOW_DIR, 'complete-milestone.md'),
path.join(WORKFLOW_DIR, 'pr-branch.md'),
];
// ─── Test suite ───────────────────────────────────────────────────────────────
describe('#1146: git.base-branch resolver', () => {
test('A. config override git.base_branch → returned immediately', (t) => {
const dir = createGitRepo({ prefix: 'gsd-1146-a-', defaultBranch: 'master' });
t.after(() => cleanup(dir));
addPlanning(dir);
setGsdConfig(dir, 'git.base_branch', 'develop');
const result = runGsdTools(['query', 'git.base-branch'], dir);
assert.ok(result.success, `git.base-branch with config override failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'develop',
`Expected config override 'develop', got: '${branch}'`);
});
test('B. origin/HEAD symref resolves → returned', (t) => {
// Create an "origin" bare repo with main branch
const originDir = createGitRepo({ prefix: 'gsd-1146-b-origin-', defaultBranch: 'main' });
const worktreeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1146-b-wt-'));
t.after(() => { cleanup(originDir); cleanup(worktreeDir); });
// Clone from origin — this sets origin/HEAD
execSync(`git clone "${originDir}" "${worktreeDir}"`, { stdio: 'pipe' });
execSync('git config user.email "test@test.com"', { cwd: worktreeDir, stdio: 'pipe' });
execSync('git config user.name "Test"', { cwd: worktreeDir, stdio: 'pipe' });
addPlanning(worktreeDir);
// Verify origin/HEAD is set (it should be after clone)
const symref = execSync('git symbolic-ref refs/remotes/origin/HEAD', { cwd: worktreeDir, encoding: 'utf8' }).trim();
assert.ok(symref.includes('origin/main'), `Expected origin/HEAD→origin/main, got: ${symref}`);
const result = runGsdTools(['query', 'git.base-branch'], worktreeDir);
assert.ok(result.success, `git.base-branch symref test failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'main',
`Expected 'main' from origin/HEAD, got: '${branch}'`);
});
test('C. KEY REGRESSION — master repo, origin/HEAD unset → returns "master" not "main"', (t) => {
// This is the bug: git init + remote add without git remote set-head → no origin/HEAD
// Current code falls back to :-main → wrong. Fixed code uses `git remote show origin`.
const originDir = createGitRepo({ prefix: 'gsd-1146-c-origin-', defaultBranch: 'master' });
const cloneDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1146-c-clone-'));
t.after(() => { cleanup(originDir); cleanup(cloneDir); });
// Manually add remote WITHOUT cloning (so origin/HEAD is never set)
execSync('git init', { cwd: cloneDir, stdio: 'pipe' });
execSync('git config user.email "test@test.com"', { cwd: cloneDir, stdio: 'pipe' });
execSync('git config user.name "Test"', { cwd: cloneDir, stdio: 'pipe' });
execSync('git config commit.gpgsign false', { cwd: cloneDir, stdio: 'pipe' });
execSync(`git remote add origin "${originDir}"`, { cwd: cloneDir, stdio: 'pipe' });
execSync('git fetch origin', { cwd: cloneDir, stdio: 'pipe' });
// Explicitly delete origin/HEAD in case git fetch auto-set it (newer git versions may do this)
try {
execSync('git remote set-head origin --delete', { cwd: cloneDir, stdio: 'pipe' });
} catch (_) { /* ignore — may not exist */ }
addPlanning(cloneDir);
// Confirm origin/HEAD is unset
let hasSymref = true;
try {
execSync('git symbolic-ref refs/remotes/origin/HEAD', { cwd: cloneDir, stdio: 'pipe' });
} catch (_) {
hasSymref = false;
}
assert.strictEqual(hasSymref, false, 'Test setup: origin/HEAD must be unset for this test case');
const result = runGsdTools(['query', 'git.base-branch'], cloneDir);
assert.ok(result.success, `git.base-branch regression test failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'master',
`BUG REGRESSION: master repo with origin/HEAD unset must return 'master', got: '${branch}'`);
});
test('D. No remote, local branch "master" present, "main" absent → returns "master"', (t) => {
const dir = createGitRepo({ prefix: 'gsd-1146-d-', defaultBranch: 'master' });
t.after(() => cleanup(dir));
addPlanning(dir);
// No remote configured — falls through to local branch detection
const result = runGsdTools(['query', 'git.base-branch'], dir);
assert.ok(result.success, `git.base-branch local branch test failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'master',
`Expected 'master' from local branch detection, got: '${branch}'`);
});
test('E. No remote, local branch "main" present → returns "main"', (t) => {
const dir = createGitRepo({ prefix: 'gsd-1146-e-', defaultBranch: 'main' });
t.after(() => cleanup(dir));
addPlanning(dir);
const result = runGsdTools(['query', 'git.base-branch'], dir);
assert.ok(result.success, `git.base-branch main branch test failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'main',
`Expected 'main' from local branch detection, got: '${branch}'`);
});
test('F. No remote, no main/master local branch → returns "main" (last resort default)', (t) => {
const dir = createGitRepo({ prefix: 'gsd-1146-f-', defaultBranch: 'develop' });
t.after(() => cleanup(dir));
addPlanning(dir);
// Branch named "develop" — neither main nor master
const result = runGsdTools(['query', 'git.base-branch'], dir);
assert.ok(result.success, `git.base-branch default fallback test failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'main',
`Expected 'main' as last resort default, got: '${branch}'`);
});
test('A2. config override with flat base_branch key (legacy form) → returned immediately', (t) => {
const dir = createGitRepo({ prefix: 'gsd-1146-a2-', defaultBranch: 'master' });
t.after(() => cleanup(dir));
addPlanning(dir);
// Write flat base_branch directly to config root (legacy form, not nested under "git")
const cfgPath = require('node:path').join(dir, '.planning', 'config.json');
require('node:fs').writeFileSync(cfgPath, JSON.stringify({ base_branch: 'release' }, null, 2) + '\n');
const result = runGsdTools(['query', 'git.base-branch'], dir);
assert.ok(result.success, `git.base-branch with flat config key failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'release',
`Expected flat config override 'release', got: '${branch}'`);
});
test('H. No remote, both "main" and "master" local branches exist → returns "main" (main wins tie-break)', (t) => {
// Tier-4 tie-break: when both main and master exist locally and no remote info is available,
// "main" wins (documented in tryLocalBranch JSDoc — modern default).
const dir = createGitRepo({ prefix: 'gsd-1146-h-', defaultBranch: 'master' });
t.after(() => cleanup(dir));
addPlanning(dir);
// Create a "main" branch alongside the existing "master"
const { execSync: exec } = require('node:child_process');
exec('git branch main', { cwd: dir, stdio: 'pipe' });
// No remote configured — falls to tier-4 (local branch existence)
const result = runGsdTools(['query', 'git.base-branch'], dir);
assert.ok(result.success, `git.base-branch both-branches test failed:\n${result.error}`);
const branch = result.output.trim();
assert.strictEqual(branch, 'main',
`Expected 'main' to win when both main and master exist locally, got: '${branch}'`);
});
test('G. Anti-regression: all five affected workflows use gsd_run query git.base-branch, not bare :-main / :-master', () => {
// The root-cause pattern: DEFAULT_BRANCH=${DEFAULT_BRANCH:-main} or BASE_BRANCH="${BASE_BRANCH:-main}"
// After fix: workflows call gsd_run query git.base-branch and remove the bare fallback.
const BAD_PATTERN = /\$\{(?:DEFAULT_BRANCH|BASE_BRANCH):-(?:main|master)\}/;
const RESOLVER_CALL = /gsd_run query git\.base-branch/;
for (const wfPath of AFFECTED_WORKFLOWS) {
const name = path.basename(wfPath);
const content = fs.readFileSync(wfPath, 'utf8');
assert.ok(
!BAD_PATTERN.test(content),
`${name} still contains the bare :-main/:-master fallback pattern. ` +
'Must be replaced with gsd_run query git.base-branch (Issue #1146).',
);
assert.ok(
RESOLVER_CALL.test(content),
`${name} does not call \`gsd_run query git.base-branch\`. ` +
'All five affected workflows must delegate to the single resolver (Issue #1146).',
);
}
});
});
// ─── gitWorktreeInfoInternal: behaviour (#1268 T0, T1 #1277) ─────────────────
const gitBaseBranch = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'git-base-branch.cjs'));
const { createTempGitProject, createTempDir } = require('./helpers.cjs');
describe('#1268 gitWorktreeInfoInternal: relocation to git-base-branch', () => {
test('gitWorktreeInfoInternal(createTempGitProject()) returns {inside:true, worktreeRoot:<non-empty string>}', (t) => {
const dir = createTempGitProject('gsd-wt-info-');
t.after(() => cleanup(dir));
const result = gitBaseBranch.gitWorktreeInfoInternal(dir);
assert.strictEqual(result.inside, true, 'inside must be true for a git project dir');
assert.ok(typeof result.worktreeRoot === 'string' && result.worktreeRoot.length > 0,
`worktreeRoot must be a non-empty string, got: ${JSON.stringify(result.worktreeRoot)}`);
});
test('gitWorktreeInfoInternal(createTempDir()) returns {inside:false, worktreeRoot:null} for a non-git dir', (t) => {
const dir = createTempDir('gsd-wt-info-nongit-');
t.after(() => cleanup(dir));
const result = gitBaseBranch.gitWorktreeInfoInternal(dir);
assert.strictEqual(result.inside, false, 'inside must be false for a non-git dir');
assert.strictEqual(result.worktreeRoot, null, 'worktreeRoot must be null for a non-git dir');
});
test('gitWorktreeInfoInternal never throws (non-git dir)', (t) => {
const dir = createTempDir('gsd-wt-info-nothrow-');
t.after(() => cleanup(dir));
assert.doesNotThrow(() => gitBaseBranch.gitWorktreeInfoInternal(dir));
});
});
// ─── #3057 B4: last-resort "main" — verified vs unverified ───────────────────
//
// `resolveBaseBranch` alone collapses two very different situations into the
// same `'main'` string: a repository that genuinely has no candidate branch
// (every git query on tiers 2-4 completed and cleanly answered "nothing"),
// and a total resolution failure (every query timed out). `resolveBaseBranchDiagnostics`
// exposes `verified` so a caller can tell them apart; `cmdGitBaseBranch`
// surfaces the unverified case as a stderr diagnostic without touching its
// stdout contract (five workflows parse that stdout literally).
describe('#3057 B4: resolveBaseBranchDiagnostics — verified vs unverified last-resort default', () => {
test('every tier-2/3/4 git query TIMES OUT → last-resort "main" is UNVERIFIED', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3057-b4-fault-'));
t.after(() => cleanup(dir));
// No .planning/config.json in this dir → the config-override tier is
// skipped naturally (readConfigBaseBranch's real-fs read misses cleanly).
const faultyGit = makeFaultyGit({ faults: [{ kind: 'timeout' }] });
const result = gitBaseBranch.resolveBaseBranchDiagnostics(dir, { execGit: faultyGit });
assert.strictEqual(result.branch, 'main');
assert.strictEqual(result.verified, false);
});
test('every tier-2/3/4 git query cleanly reports no candidate → last-resort "main" is VERIFIED', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3057-b4-clean-'));
t.after(() => cleanup(dir));
// Default passthrough: exitCode 0, empty stdout for every call — a real,
// completed "no answer" from git, not a failure (timedOut:false, error:null).
const faultyGit = makeFaultyGit();
const result = gitBaseBranch.resolveBaseBranchDiagnostics(dir, { execGit: faultyGit });
assert.strictEqual(result.branch, 'main');
assert.strictEqual(result.verified, true);
});
test('resolveBaseBranch (string-returning) is unaffected — both cases still return "main"', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3057-b4-compat-'));
t.after(() => cleanup(dir));
assert.strictEqual(
gitBaseBranch.resolveBaseBranch(dir, { execGit: makeFaultyGit({ faults: [{ kind: 'timeout' }] }) }),
'main',
);
assert.strictEqual(
gitBaseBranch.resolveBaseBranch(dir, { execGit: makeFaultyGit() }),
'main',
);
});
test('cmdGitBaseBranch writes an unverified-fallback diagnostic to stderr ONLY when unverified', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3057-b4-cmd-'));
t.after(() => cleanup(dir));
let stdoutText = '';
let stderrText = '';
gitBaseBranch.cmdGitBaseBranch(dir, [], {
execGit: makeFaultyGit({ faults: [{ kind: 'timeout' }] }),
write: (s) => { stdoutText += s; },
writeDiagnostic: (s) => { stderrText += s; },
});
assert.strictEqual(stdoutText, 'main\n');
assert.ok(stderrText.length > 0, 'the unverified fallback must write a stderr diagnostic');
stdoutText = '';
stderrText = '';
gitBaseBranch.cmdGitBaseBranch(dir, [], {
execGit: makeFaultyGit(),
write: (s) => { stdoutText += s; },
writeDiagnostic: (s) => { stderrText += s; },
});
assert.strictEqual(stdoutText, 'main\n');
assert.strictEqual(stderrText, '', 'a verified fallback must not write any diagnostic');
});
});
// ─── setGsdConfig prototype-pollution guard (#1406) ───────────────────────────
describe('#1406: setGsdConfig prototype-pollution guard', () => {
test('rejects __proto__ as a key segment', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1406-'));
t.after(() => cleanup(dir));
assert.throws(() => setGsdConfig(dir, '__proto__', 'x'), /unsafe config key segment/);
assert.throws(() => setGsdConfig(dir, '__proto__.polluted', true), /unsafe config key segment/);
});
test('rejects constructor / prototype chain segments', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1406-'));
t.after(() => cleanup(dir));
assert.throws(() => setGsdConfig(dir, 'constructor.prototype.polluted', true), /unsafe config key segment/);
assert.throws(() => setGsdConfig(dir, 'safe.__proto__', true), /unsafe config key segment/);
assert.throws(() => setGsdConfig(dir, 'a.prototype.b', true), /unsafe config key segment/);
});
test('does not pollute Object.prototype after rejected attempts', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1406-'));
t.after(() => cleanup(dir));
try { setGsdConfig(dir, '__proto__.polluted', true); } catch (_) { /* expected */ }
try { setGsdConfig(dir, 'constructor.prototype.polluted', true); } catch (_) { /* expected */ }
try { setGsdConfig(dir, 'a.__proto__.polluted', true); } catch (_) { /* expected */ }
assert.strictEqual(({}).polluted, undefined);
assert.strictEqual(Object.prototype.polluted, undefined);
});
test('still writes a normal nested key', (t) => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1406-'));
t.after(() => cleanup(dir));
setGsdConfig(dir, 'git.base_branch', 'develop');
const cfgPath = path.join(dir, '.planning', 'config.json');
const cfg = JSON.parse(fs.readFileSync(cfgPath, 'utf8'));
assert.strictEqual(cfg.git.base_branch, 'develop');
});
});
// ────────────────────────────────────────────────────────────────────────
// Folded from tests/bug-2004-pr-branch-milestone.test.cjs — consolidation epic #1969 (B4 #1973)
// ────────────────────────────────────────────────────────────────────────
{
const { describe: __foldDescribe } = require('node:test');
__foldDescribe("folded:bug-2004-pr-branch-milestone (consolidation epic #1969 B4 #1973)", () => {
/**
* Regression tests for bug #2004
*
* /gsd-pr-branch must not exclude milestone archive and structural planning
* commits. The previous implementation filtered ALL .planning/-only commits,
* including STATE.md, ROADMAP.md, MILESTONES.md, and milestones/** updates
* that are needed to preserve repository planning state after a merge.
*
* Fixed: pr-branch.md now distinguishes:
* - Transient planning commits (phase plans, summaries, research, context) → EXCLUDE
* - Structural planning commits (STATE.md, ROADMAP.md, MILESTONES.md,
* PROJECT.md, milestones/**) → INCLUDE
* - Code commits (any non-.planning/ file) → INCLUDE
* - Mixed commits (code + planning) → INCLUDE
*/
'use strict';
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const workflowPath = path.resolve(
__dirname, '..', 'gsd-core', 'workflows', 'pr-branch.md'
);
describe('bug #2004: pr-branch preserves structural planning commits', () => {
let content;
test('setup: pr-branch workflow is readable', () => {
content = fs.readFileSync(workflowPath, 'utf-8');
assert.ok(content.length > 0, 'pr-branch.md must not be empty');
});
test('workflow distinguishes structural vs transient planning commits', () => {
content = content || fs.readFileSync(workflowPath, 'utf-8');
// Must contain language distinguishing structural from transient/phase planning files
assert.ok(
/structural|milestone.*archive|STATE\.md.*INCLUDE|preserve.*milestone|milestone.*preserve/i.test(content),
'pr-branch.md must distinguish structural planning commits from transient ones'
);
});
test('workflow lists STATE.md and ROADMAP.md as structural files to preserve', () => {
content = content || fs.readFileSync(workflowPath, 'utf-8');
assert.ok(
content.includes('STATE.md'),
'pr-branch.md must reference STATE.md as a structural file to preserve'
);
assert.ok(
content.includes('ROADMAP.md'),
'pr-branch.md must reference ROADMAP.md as a structural file to preserve'
);
});
test('workflow lists MILESTONES.md or milestones/ as structural files to preserve', () => {
content = content || fs.readFileSync(workflowPath, 'utf-8');
assert.ok(
content.includes('MILESTONES.md') || content.includes('milestones/'),
'pr-branch.md must reference MILESTONES.md or milestones/ as structural files to preserve'
);
});
test('workflow has four commit categories (code, planning-only, mixed, structural)', () => {
content = content || fs.readFileSync(workflowPath, 'utf-8');
// Must have at least a "structural" or "milestone" category beyond the original three
assert.ok(
/structural.*commit|milestone.*commit|commit.*structural|commit.*milestone/i.test(content) ||
/INCLUDE.*STATE\.md|STATE\.md.*INCLUDE/i.test(content),
'pr-branch.md must classify structural planning commits as INCLUDE'
);
});
test('create_pr_branch step does not rm -r --cached all of .planning/', () => {
content = content || fs.readFileSync(workflowPath, 'utf-8');
// The original bug: `git rm -r --cached .planning/` nuked structural files.
// The fix must either remove this wholesale rm or scope it to transient dirs.
// Acceptable: narrowed rm targeting only phase/, quick/, research/, etc.
// Not acceptable: `git rm -r --cached .planning/` with no scoping.
const hasUnscoped = /git rm -r --cached \.planning\/(?!\*)?(?!phases|quick|research|threads|todos|debug|seeds|ui-reviews|codebase)/
.test(content);
assert.ok(
!hasUnscoped,
'create_pr_branch must not use unscoped "git rm -r --cached .planning/" — scope to transient subdirectories only'
);
});
});
});
}
// ────────────────────────────────────────────────────────────────────────
// Folded from tests/bug-2916-handle-branching-default-base.test.cjs — consolidation epic #1969 (B6 #1975)
// ────────────────────────────────────────────────────────────────────────
{
const { describe: __foldDescribe } = require('node:test');
__foldDescribe("folded:bug-2916-handle-branching-default-base (consolidation epic #1969 B6 #1975)", () => {
/**
* Regression test for #2916: execute-phase `handle_branching` step creates the
* per-phase branch off whatever HEAD is currently checked out (typically the
* previous phase's unmerged branch) instead of off `origin/HEAD`.
*
* The bug compounded phases on top of each other and stranded them unpushed
* for weeks. The fix:
* 1. Detect the default branch via `git symbolic-ref refs/remotes/origin/HEAD`.
* 2. If $BRANCH_NAME exists, switch to it (preserve existing behavior).
* 3. Otherwise, ff-update the default branch from origin and create the new
* phase branch off the default-branch tip.
* 4. Refuse-or-warn on dirty working tree.
* 5. Post-creation, assert `git rev-list --count $DEFAULT_BRANCH..HEAD == 0`.
*
* This test extracts the bash payload from the <step name="handle_branching">
* block in execute-phase.md (parsed structurally — no regex on prose), executes
* it inside a fixture git repo where HEAD sits on a previous-phase branch with
* extra commits, and asserts that the new phase branch's tip equals
* `origin/main` (no commits inherited from the previous phase).
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const { execFileSync } = require('node:child_process');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { cleanup } = require('./helpers.cjs');
const EXECUTE_PHASE_PATH = path.join(
__dirname,
'..',
'gsd-core',
'workflows',
'execute-phase.md'
);
const GIT_ENV = Object.freeze({
...process.env,
GIT_AUTHOR_NAME: 'Test',
GIT_AUTHOR_EMAIL: 'test@test.com',
GIT_COMMITTER_NAME: 'Test',
GIT_COMMITTER_EMAIL: 'test@test.com',
});
function git(cwd, ...args) {
return execFileSync('git', args, {
cwd,
env: GIT_ENV,
stdio: ['pipe', 'pipe', 'pipe'],
})
.toString()
.trim();
}
/**
* Structurally extract the bash code that the handle_branching step instructs
* the agent to run. We:
* 1. Locate the <step name="handle_branching"> ... </step> block.
* 2. Walk its body looking for fenced ```bash blocks.
* 3. Concatenate every bash block in the step (the fix may use more than one).
*
* No `.includes()` content checks — we parse fence-delimited code blocks the
* same way a markdown parser would.
*/
function extractHandleBranchingBash() {
const content = readFileNormalized(EXECUTE_PHASE_PATH);
const lines = content.split('\n');
let start = -1;
let end = -1;
for (let i = 0; i < lines.length; i += 1) {
if (start === -1 && /^<step\s+name="handle_branching">\s*$/.test(lines[i])) {
start = i + 1;
} else if (start !== -1 && /^<\/step>\s*$/.test(lines[i])) {
end = i;
break;
}
}
if (start === -1 || end === -1) {
throw new Error(
'execute-phase.md does not contain a <step name="handle_branching"> ... </step> block'
);
}
const bashBlocks = [];
let inBash = false;
let buffer = [];
for (let i = start; i < end; i += 1) {
const line = lines[i];
if (!inBash && /^```bash\s*$/.test(line)) {
inBash = true;
buffer = [];
continue;
}
if (inBash && /^```\s*$/.test(line)) {
bashBlocks.push(buffer.join('\n'));
inBash = false;
continue;
}
if (inBash) buffer.push(line);
}
if (bashBlocks.length === 0) {
throw new Error(
'handle_branching step contains no ```bash code blocks to execute'
);
}
return bashBlocks.join('\n');
}
/**
* Build a fixture: a bare "origin" repo with the named default branch (one
* commit), a clone with `origin/HEAD` pointed at it, and a checked-out
* previous-phase branch carrying its own unmerged commit.
*
* `defaultBranch` is parameterized so callers can lock in that the workflow
* honors `git symbolic-ref refs/remotes/origin/HEAD` rather than silently
* defaulting to `main` (#2921 CR feedback — quick-branching.test.cjs got the
* same treatment in 80f14cac; this test deserves the same coverage).
*/
function setupFixture(defaultBranch = 'main') {
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2916-'));
const seedPath = path.join(root, 'seed');
const originPath = path.join(root, 'origin.git');
const clonePath = path.join(root, 'clone');
fs.mkdirSync(seedPath);
git(seedPath, 'init', '-b', defaultBranch);
git(seedPath, 'config', 'commit.gpgsign', 'false');
fs.writeFileSync(path.join(seedPath, 'README.md'), '# seed\n');
git(seedPath, 'add', 'README.md');
git(seedPath, 'commit', '-m', 'initial');
git(root, 'clone', '--bare', seedPath, originPath);
git(originPath, 'symbolic-ref', 'HEAD', `refs/heads/${defaultBranch}`);
git(root, 'clone', originPath, clonePath);
git(clonePath, 'config', 'commit.gpgsign', 'false');
git(clonePath, 'config', 'user.email', 'test@test.com');
git(clonePath, 'config', 'user.name', 'Test');
// Simulate finishing a previous phase: branch off the default branch, add
// a commit, and *stay* on it (the failure scenario described in the bug).
git(clonePath, 'checkout', '-b', 'feature/phase-01-foundation');
fs.writeFileSync(path.join(clonePath, 'phase01.txt'), 'phase 1 work\n');
git(clonePath, 'add', 'phase01.txt');
git(clonePath, 'commit', '-m', 'phase 01 work');
return { root, clonePath, defaultBranch };
}
function runHandleBranchingStep(bash, cwd, branchName) {
// Write the script to a sibling tempdir, not inside the repo — putting it in
// `cwd` would create an untracked file that trips `git status --porcelain`
// and steers the step into its dirty-tree fallback path.
const scriptDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2916-step-'));
const scriptPath = path.join(scriptDir, 'handle-branching.sh');
const script = `#!/usr/bin/env bash\nset -uo pipefail\nBRANCH_NAME="${branchName}"\n${bash}\n`;
fs.writeFileSync(scriptPath, script, { mode: 0o755 });
try {
return execFileSync('bash', [scriptPath], {
cwd,
env: GIT_ENV,
stdio: ['pipe', 'pipe', 'pipe'],
}).toString();
} finally {
cleanup(scriptDir);
}
}
describe('handle_branching branches off origin/HEAD, not current HEAD (#2916)', () => {
// Run against `main` (conventional default) and `trunk` (non-main default
// exercising the symbolic-ref code path) so a regression that hard-codes
// `main` instead of consulting origin/HEAD will fail the trunk variant.
for (const defaultBranch of ['main', 'trunk']) {
test(`new phase branch branches off origin/${defaultBranch} with 0 inherited commits`, () => {
const bash = extractHandleBranchingBash();
const { root, clonePath } = setupFixture(defaultBranch);
try {
const upstream = `origin/${defaultBranch}`;
assert.equal(
git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'),
'feature/phase-01-foundation'
);
assert.equal(
git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`),
'1',
`fixture should be 1 commit ahead of ${upstream}`
);
runHandleBranchingStep(bash, clonePath, 'feature/phase-02-content-sync');
assert.equal(
git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'),
'feature/phase-02-content-sync',
'handle_branching should switch to the new phase branch'
);
const inherited = git(clonePath, 'rev-list', '--count', `${upstream}..HEAD`);
assert.equal(
inherited,
'0',
`new phase branch must branch off ${upstream}, but inherited ${inherited} commit(s) from previous-phase HEAD`
);
assert.equal(
git(clonePath, 'rev-parse', 'HEAD'),
git(clonePath, 'rev-parse', upstream),
`new phase branch tip must equal ${upstream} tip`
);
} finally {
cleanup(root);
}
});
}
test('handle_branching reuses an existing branch instead of forking again', () => {
const bash = extractHandleBranchingBash();
const { root, clonePath } = setupFixture();
try {
// Pre-create the target branch off origin/main with its own commit, then
// walk away to a different branch — the step must switch back to it.
git(clonePath, 'checkout', '-B', 'feature/phase-02-content-sync', 'origin/main');
fs.writeFileSync(path.join(clonePath, 'phase02.txt'), 'phase 2 work\n');
git(clonePath, 'add', 'phase02.txt');
git(clonePath, 'commit', '-m', 'phase 02 wip');
const phase02Sha = git(clonePath, 'rev-parse', 'HEAD');
git(clonePath, 'checkout', 'feature/phase-01-foundation');
runHandleBranchingStep(bash, clonePath, 'feature/phase-02-content-sync');
assert.equal(
git(clonePath, 'rev-parse', '--abbrev-ref', 'HEAD'),
'feature/phase-02-content-sync'
);
assert.equal(
git(clonePath, 'rev-parse', 'HEAD'),
phase02Sha,
'existing-branch tip must be preserved (no rebase/reset)'
);
} finally {
cleanup(root);
}
});
});
});
}