diff --git a/tests/git-base-branch.test.cjs b/tests/git-base-branch.test.cjs index d67ff38bc..7a0936407 100644 --- a/tests/git-base-branch.test.cjs +++ b/tests/git-base-branch.test.cjs @@ -283,9 +283,14 @@ describe('#1268 gitWorktreeInfoInternal: relocation to git-base-branch', () => { const dir = createTempGitProject('gsd-wt-info-'); t.after(() => cleanup(dir)); const result = gitBaseBranch.gitWorktreeInfoInternal(dir); + // `git rev-parse --show-toplevel` reports the resolved (symlink-free) path, + // which on macOS differs from the mkdtemp path (/var → /private/var). Pin the + // exact value rather than "a non-empty string": a resolver that returned the + // .git dir, the cwd, or any other plausible-looking path would pass the weaker + // shape check while being wrong. 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)}`); + assert.strictEqual(result.worktreeRoot, fs.realpathSync.native(dir), + 'worktreeRoot must be the resolved worktree root path'); }); test('gitWorktreeInfoInternal(createTempDir()) returns {inside:false, worktreeRoot:null} for a non-git dir', (t) => { @@ -296,11 +301,12 @@ describe('#1268 gitWorktreeInfoInternal: relocation to git-base-branch', () => { 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)); - }); + // NOTE: the former "never throws" liveness test that sat here was replaced + // (#3057 W3). "It did not throw" is satisfied by a function that returns + // undefined, the wrong branch, or nothing useful at all. The + // `execGit throws → {inside:false, worktreeRoot:null}` test below asserts the + // exact value the catch arm is contracted to produce, which is what the old + // test was gesturing at. }); // ─── #3057 B4: last-resort "main" — verified vs unverified ─────────────────── @@ -365,7 +371,11 @@ describe('#3057 B4: resolveBaseBranchDiagnostics — verified vs unverified last writeDiagnostic: (s) => { stderrText += s; }, }); assert.strictEqual(stdoutText, 'main\n'); - assert.ok(stderrText.length > 0, 'the unverified fallback must write a stderr diagnostic'); + assert.strictEqual( + stderrText, + `⚠ git-base-branch: defaulted to 'main' WITHOUT verifying against this repository — ` + + `a git query timed out or could not run. See #3057.\n`, + ); stdoutText = ''; stderrText = ''; @@ -379,6 +389,491 @@ describe('#3057 B4: resolveBaseBranchDiagnostics — verified vs unverified last }); }); +// ─── #3057 W3: negative-space coverage for the resolver's failure arms ─────── +// +// Everything below drives the *unhappy* halves of git-base-branch: malformed +// config, git output that parses but says nothing useful, git that cannot run +// at all, and a repository with no work tree. Each test asserts the exact value +// the arm is contracted to produce — never "it did not throw", never a shape +// check — because an arm that silently returns `undefined` instead of `null` +// changes the precedence ladder's behaviour while passing any weaker assertion. + +/** + * Build a result object shaped exactly like `execGit`'s (see `_spawnResult` in + * shell-command-projection). Defaults are a benign, completed, zero-exit call. + */ +function gitResult(overrides) { + return { + exitCode: 0, + stdout: '', + stderr: '', + signal: null, + error: null, + timedOut: false, + ...overrides, + }; +} + +/** An `execGit` stand-in that always returns the same shaped result. */ +function constGit(overrides) { + return () => gitResult(overrides); +} + +/** + * An `execGit` stand-in that throws. `makeFaultyGit` deliberately never throws + * (it returns a shaped failure result), so the resolver's `catch` arms need + * this instead. + */ +function throwingGit(message) { + return () => { throw new Error(message); }; +} + +describe('#3057 W3: readConfigBaseBranch — config present but unusable', () => { + const PLANNING_DIR = path.join(path.sep, 'gsd-3057-w3', '.planning'); + + /** Read a config whose raw text is `raw`, recording the paths requested. */ + function readWith(raw, seenPaths) { + return gitBaseBranch.readConfigBaseBranch(PLANNING_DIR, { + readFile: (p) => { if (seenPaths) seenPaths.push(p); return raw; }, + }); + } + + test('config.json exists but is not valid JSON → null (parse failure swallowed)', () => { + const seen = []; + assert.strictEqual(readWith('{ not json', seen), null); + assert.deepStrictEqual(seen, [path.join(PLANNING_DIR, 'config.json')], + 'the resolver must look for config.json inside the planning dir it was given'); + }); + + test('config.json parses to a non-object → null for null / string / number / array', () => { + assert.strictEqual(readWith('null'), null, 'JSON null must not be treated as a config'); + assert.strictEqual(readWith('"master"'), null, 'a bare JSON string must not be treated as a config'); + assert.strictEqual(readWith('42'), null, 'a bare JSON number must not be treated as a config'); + assert.strictEqual(readWith('[]'), null, 'a JSON array must not be treated as a config'); + }); + + test('"git" section present but base_branch missing / non-string / blank → null', () => { + assert.strictEqual(readWith('{"git":{}}'), null); + assert.strictEqual(readWith('{"git":{"base_branch":42}}'), null); + assert.strictEqual(readWith('{"git":{"base_branch":null}}'), null); + assert.strictEqual(readWith('{"git":{"base_branch":""}}'), null); + assert.strictEqual(readWith('{"git":{"base_branch":" "}}'), null, + 'a whitespace-only override must not win the precedence ladder'); + }); + + test('"git" key present but not an object → skipped, flat legacy key still consulted', () => { + assert.strictEqual(readWith('{"git":"main","base_branch":"release"}'), 'release'); + assert.strictEqual(readWith('{"git":[],"base_branch":"release"}'), 'release'); + assert.strictEqual(readWith('{"git":null,"base_branch":"release"}'), 'release'); + }); + + test('flat base_branch present but non-string / blank → null', () => { + assert.strictEqual(readWith('{"base_branch":true}'), null); + assert.strictEqual(readWith('{"base_branch":["main"]}'), null); + assert.strictEqual(readWith('{"base_branch":""}'), null); + assert.strictEqual(readWith('{"base_branch":" "}'), null); + }); + + test('config parses cleanly but carries neither key → null (distinct from an absent file)', () => { + // The absent-file path returns null after reading an empty string and never + // reaches JSON.parse. This one parses a real object and falls all the way + // through both key lookups to the final return. + assert.strictEqual(readWith('{"other":1}'), null); + assert.strictEqual(readWith('{}'), null); + assert.strictEqual(readWith(''), null, 'absent file (empty read) also yields null'); + }); + + test('positive controls: values are trimmed, and the nested key outranks the flat one', () => { + assert.strictEqual(readWith('{"git":{"base_branch":" develop "}}'), 'develop'); + assert.strictEqual(readWith('{"base_branch":" release\\n"}'), 'release'); + assert.strictEqual(readWith('{"git":{"base_branch":"nested"},"base_branch":"flat"}'), 'nested'); + }); +}); + +describe('#3057 W3: trySymbolicRef — tier-2 output that resolves to nothing', () => { + test('stdout is exactly "origin/" → null (prefix strip leaves an empty name)', () => { + assert.strictEqual(gitBaseBranch.trySymbolicRef('/x', constGit({ stdout: 'origin/' })), null); + assert.strictEqual(gitBaseBranch.trySymbolicRef('/x', constGit({ stdout: 'origin/\n' })), null); + }); + + test('only ONE leading "origin/" is stripped — slashes inside the name survive', () => { + assert.strictEqual( + gitBaseBranch.trySymbolicRef('/x', constGit({ stdout: 'origin/feature/long-name\n' })), + 'feature/long-name'); + assert.strictEqual( + gitBaseBranch.trySymbolicRef('/x', constGit({ stdout: 'origin/origin/main\n' })), + 'origin/main'); + }); + + test('execGit THROWS → null (catch arm; makeFaultyGit cannot reach this)', () => { + assert.strictEqual(gitBaseBranch.trySymbolicRef('/x', throwingGit('symbolic-ref exploded')), null); + }); + + test('the tier-2 subprocess is bounded (argv + timeout are pinned)', () => { + const seen = []; + gitBaseBranch.trySymbolicRef('/some/cwd', (args, opts) => { + seen.push({ args, opts }); + return gitResult({ stdout: 'origin/main\n' }); + }); + assert.deepStrictEqual(seen, [{ + args: ['symbolic-ref', '--quiet', '--short', 'refs/remotes/origin/HEAD'], + opts: { cwd: '/some/cwd', timeout: 5_000 }, + }]); + }); +}); + +describe('#3057 W3: tryRemoteShow — tier-3 output that is present but not authoritative', () => { + const REMOTE_SHOW_NO_HEAD = [ + '* remote origin', + ' Fetch URL: /tmp/origin.git', + ' Push URL: /tmp/origin.git', + ' Remote branch:', + ' main tracked', + '', + ].join('\n'); + + test('stdout has no "HEAD branch:" line → null', () => { + assert.strictEqual( + gitBaseBranch.tryRemoteShow('/x', constGit({ stdout: REMOTE_SHOW_NO_HEAD })), null); + }); + + test('"HEAD branch:" with no value on the line → null (no capture, no guess)', () => { + assert.strictEqual( + gitBaseBranch.tryRemoteShow('/x', constGit({ stdout: ' HEAD branch: \n' })), null); + }); + + test('"HEAD branch: (unknown)" → null — the documented offline-remote case', () => { + // git prints "(unknown)" when it could not reach the remote. Returning it + // verbatim would hand a literal branch named "(unknown)" to five workflows; + // returning null lets tier 4 answer instead. + assert.strictEqual( + gitBaseBranch.tryRemoteShow('/x', constGit({ stdout: ' HEAD branch: (unknown)\n' })), null); + }); + + test('execGit THROWS → null (catch arm)', () => { + assert.strictEqual(gitBaseBranch.tryRemoteShow('/x', throwingGit('remote show exploded')), null); + }); + + test('positive control: the HEAD branch line is found mid-output and returned verbatim', () => { + const stdout = [ + '* remote origin', + ' Fetch URL: /tmp/origin.git', + ' HEAD branch: master', + ' Remote branch:', + ' master tracked', + '', + ].join('\n'); + assert.strictEqual(gitBaseBranch.tryRemoteShow('/x', constGit({ stdout })), 'master'); + }); + + test('the tier-3 subprocess is bounded (argv + timeout are pinned)', () => { + const seen = []; + gitBaseBranch.tryRemoteShow('/some/cwd', (args, opts) => { + seen.push({ args, opts }); + return gitResult({ stdout: ' HEAD branch: main\n' }); + }); + assert.deepStrictEqual(seen, [{ + args: ['remote', 'show', 'origin'], + opts: { cwd: '/some/cwd', timeout: 15_000 }, + }]); + }); +}); + +describe('#3057 W3: tryLocalBranch — non-empty stdout that names neither main nor master', () => { + /** + * An `execGit` stand-in whose result counts how many times `stdout` is read. + * `tryLocalBranch` reads it once in the `!r.stdout` guard and a second time to + * split it into lines. The read count therefore identifies WHICH null the + * function returned: 1 read = the early guard fired; 2 reads = the guard was + * passed and the final `return null` after the main/master checks ran. + */ + function countingGit(stdoutValue) { + const state = { reads: 0 }; + const git = () => ({ + exitCode: 0, + stderr: '', + signal: null, + error: null, + timedOut: false, + get stdout() { state.reads += 1; return stdoutValue; }, + }); + git.state = state; + return git; + } + + test('stdout is exactly "\\n" → null, reached PAST the empty-stdout guard', () => { + // This is the branch that was once deleted as "unreachable". The guard is + // `if (r.exitCode !== 0 || !r.stdout) return null` — `"\n"` is a truthy + // string, so the guard does NOT fire; `split('\n')` yields ["", ""], both + // main/master checks are false, and the FINAL `return null` executes. + // Deleting that line makes this function return `undefined`, which + // strictEqual(null) catches. The read count proves which null we got. + const git = countingGit('\n'); + assert.strictEqual(gitBaseBranch.tryLocalBranch('/x', git), null); + assert.strictEqual(git.state.reads, 2, + 'stdout must be read twice: once by the guard (which passes) and once to split into lines'); + }); + + test('stdout is exactly "" → null via the EARLY guard (a different arm)', () => { + const git = countingGit(''); + assert.strictEqual(gitBaseBranch.tryLocalBranch('/x', git), null); + assert.strictEqual(git.state.reads, 1, + 'an empty stdout must short-circuit in the guard, never reaching the line split'); + }); + + // Boundary trio over the number of branch lines `git branch --list main master` + // can emit: 0 (below the smallest useful listing), 1, and 2 (the maximum this + // argv can produce). + test('0 branch lines → null', () => { + assert.strictEqual(gitBaseBranch.tryLocalBranch('/x', constGit({ stdout: '\n' })), null); + }); + + test('1 branch line → that branch', () => { + assert.strictEqual(gitBaseBranch.tryLocalBranch('/x', constGit({ stdout: ' main\n' })), 'main'); + assert.strictEqual(gitBaseBranch.tryLocalBranch('/x', constGit({ stdout: ' master\n' })), 'master'); + assert.strictEqual(gitBaseBranch.tryLocalBranch('/x', constGit({ stdout: '* master\n' })), 'master', + 'the checked-out marker "* " must be stripped before matching'); + }); + + test('2 branch lines → "main" wins the tie-break', () => { + assert.strictEqual( + gitBaseBranch.tryLocalBranch('/x', constGit({ stdout: ' main\n master\n' })), 'main'); + assert.strictEqual( + gitBaseBranch.tryLocalBranch('/x', constGit({ stdout: '* master\n main\n' })), 'main'); + }); + + test('execGit THROWS → null (catch arm)', () => { + assert.strictEqual(gitBaseBranch.tryLocalBranch('/x', throwingGit('branch --list exploded')), null); + }); + + test('the tier-4 subprocess is bounded (argv + timeout are pinned)', () => { + const seen = []; + gitBaseBranch.tryLocalBranch('/some/cwd', (args, opts) => { + seen.push({ args, opts }); + return gitResult({ stdout: ' main\n' }); + }); + assert.deepStrictEqual(seen, [{ + args: ['branch', '--list', 'main', 'master'], + opts: { cwd: '/some/cwd', timeout: 5_000 }, + }]); + }); +}); + +describe('#3057 W3: resolveBaseBranchDiagnostics — which tier actually answered', () => { + // Every tier can produce a plausible-looking branch name, so asserting the + // returned string alone cannot tell a tier-2 answer from a tier-3 or tier-4 + // one. Each test below rigs the LOWER tiers to answer with a DIFFERENT branch + // than the tier under test, so a resolver that consulted them in the wrong + // order returns the wrong string, and additionally pins the recorded argv so + // an early return is provably an early return. + + /** A passthrough answering each tier with a distinct, recognisable branch. */ + function tieredPassthrough({ symref, remote, local }) { + return (args) => { + if (args[0] === 'symbolic-ref') { + return symref === null ? gitResult({ exitCode: 1 }) : gitResult({ stdout: `origin/${symref}\n` }); + } + if (args[0] === 'remote') { + return remote === null ? gitResult({ exitCode: 128 }) : gitResult({ stdout: ` HEAD branch: ${remote}\n` }); + } + if (args[0] === 'branch') { + return local === null ? gitResult({ stdout: '\n' }) : gitResult({ stdout: ` ${local}\n` }); + } + return gitResult({}); + }; + } + + const NO_CONFIG = { readFile: () => null }; + + test('tier 2 answers → tiers 3 and 4 are never consulted', () => { + const git = makeFaultyGit({ + passthrough: tieredPassthrough({ symref: 'from-symref', remote: 'from-remote', local: 'master' }), + }); + const result = gitBaseBranch.resolveBaseBranchDiagnostics('/x', { ...NO_CONFIG, execGit: git }); + assert.deepStrictEqual(result, { branch: 'from-symref', verified: true }); + assert.deepStrictEqual(git.calls.map((c) => c.args[0]), ['symbolic-ref'], + 'a tier-2 hit must stop the ladder before `remote show` and `branch --list`'); + }); + + test('tier 3 answers → tier 4 is never consulted, even though it WOULD answer "master"', () => { + const git = makeFaultyGit({ + passthrough: tieredPassthrough({ symref: null, remote: 'from-remote', local: 'master' }), + }); + const result = gitBaseBranch.resolveBaseBranchDiagnostics('/x', { ...NO_CONFIG, execGit: git }); + assert.deepStrictEqual(result, { branch: 'from-remote', verified: true }); + assert.deepStrictEqual(git.calls.map((c) => c.args[0]), ['symbolic-ref', 'remote'], + 'a tier-3 hit must stop the ladder before `branch --list`'); + }); + + test('tier 4 answers only after tiers 2 and 3 both decline', () => { + const git = makeFaultyGit({ + passthrough: tieredPassthrough({ symref: null, remote: null, local: 'master' }), + }); + const result = gitBaseBranch.resolveBaseBranchDiagnostics('/x', { ...NO_CONFIG, execGit: git }); + assert.deepStrictEqual(result, { branch: 'master', verified: true }); + assert.deepStrictEqual(git.calls.map((c) => c.args[0]), ['symbolic-ref', 'remote', 'branch']); + }); + + test('a config override answers before ANY git subprocess runs', () => { + const git = makeFaultyGit({ + passthrough: tieredPassthrough({ symref: 'from-symref', remote: 'from-remote', local: 'master' }), + }); + const result = gitBaseBranch.resolveBaseBranchDiagnostics('/x', { + readFile: () => '{"git":{"base_branch":"from-config"}}', + execGit: git, + }); + assert.deepStrictEqual(result, { branch: 'from-config', verified: true }); + assert.deepStrictEqual(git.calls, [], 'tier 1 must not spawn git at all'); + }); + + test('git cannot be SPAWNED at all (exit 127 + error) → "main", verified:false', () => { + // Distinct from the timeout case already covered by #3057 B4: here every + // call returns exitCode 127 with `error` set and `timedOut:false`, which is + // the `r.error` disjunct of the failure detector rather than `r.timedOut`. + const git = makeFaultyGit({ faults: [{ kind: 'spawnFail' }] }); + const result = gitBaseBranch.resolveBaseBranchDiagnostics('/x', { ...NO_CONFIG, execGit: git }); + assert.deepStrictEqual(result, { branch: 'main', verified: false }); + assert.deepStrictEqual(git.calls.map((c) => c.args[0]), ['symbolic-ref', 'remote', 'branch'], + 'all three tiers must still be attempted before the unverified default'); + }); + + test('a spawn failure on ONE tier alone is enough to mark the default unverified', () => { + // Tiers 2 and 3 complete cleanly with "no answer"; only tier 4 fails to run. + const git = makeFaultyGit({ + faults: [{ kind: 'spawnFail', when: ['branch', '--list'] }], + passthrough: tieredPassthrough({ symref: null, remote: null, local: null }), + }); + const result = gitBaseBranch.resolveBaseBranchDiagnostics('/x', { ...NO_CONFIG, execGit: git }); + assert.deepStrictEqual(result, { branch: 'main', verified: false }); + }); + + test('tier-4 stdout of "\\n" (no branches) is a VERIFIED "no candidate", not a failure', () => { + // The counterpart to the tryLocalBranch "\n" test, one level up: git ran, + // answered, and the answer was "neither branch exists". That must still be + // verified:true — collapsing it into verified:false would re-fail-open the + // exact distinction #3057 B4 introduced. + const git = makeFaultyGit({ + passthrough: tieredPassthrough({ symref: null, remote: null, local: null }), + }); + const result = gitBaseBranch.resolveBaseBranchDiagnostics('/x', { ...NO_CONFIG, execGit: git }); + assert.deepStrictEqual(result, { branch: 'main', verified: true }); + }); +}); + +describe('#3057 W3: gitWorktreeInfoInternal — no work tree, and git failing mid-sequence', () => { + test('a REAL bare repository reports {inside:false, worktreeRoot:null}', (t) => { + // `git rev-parse --is-inside-work-tree` exits 0 in a bare repo and prints + // "false" — the exitCode guard does NOT fire, so this is the stdout check, + // and it is reachable without any injection. + const dir = createTempDir('gsd-3057-w3-bare-'); + t.after(() => cleanup(dir)); + execSync('git init --bare', { cwd: dir, stdio: 'pipe' }); + + assert.deepStrictEqual( + gitBaseBranch.gitWorktreeInfoInternal(dir), + { inside: false, worktreeRoot: null }); + }); + + test('is-inside-work-tree prints "false" with exit 0 → no second git call is made', () => { + const git = makeFaultyGit({ passthrough: () => gitResult({ stdout: 'false\n' }) }); + assert.deepStrictEqual( + gitBaseBranch.gitWorktreeInfoInternal('/x', { execGit: git }), + { inside: false, worktreeRoot: null }); + assert.deepStrictEqual(git.calls.map((c) => c.args), [['rev-parse', '--is-inside-work-tree']], + '--show-toplevel must not be queried once we know there is no work tree'); + }); + + test('inside a work tree but --show-toplevel FAILS → {inside:true, worktreeRoot:null}', () => { + // inside is still reported truthfully; only the root is unknown. Reporting + // inside:false here would be a lie about a repository we just confirmed. + const git = makeFaultyGit({ + faults: [{ + kind: 'exit', + exitCode: 128, + stderr: 'fatal: no work tree', + when: ['rev-parse', '--show-toplevel'], + }], + passthrough: () => gitResult({ stdout: 'true\n' }), + }); + assert.deepStrictEqual( + gitBaseBranch.gitWorktreeInfoInternal('/x', { execGit: git }), + { inside: true, worktreeRoot: null }); + assert.deepStrictEqual(git.calls.map((c) => c.args[1]), + ['--is-inside-work-tree', '--show-toplevel']); + }); + + test('--show-toplevel succeeds with blank stdout → {inside:true, worktreeRoot:null}', () => { + const git = makeFaultyGit({ + passthrough: (args) => gitResult({ stdout: args[1] === '--show-toplevel' ? ' \n' : 'true\n' }), + }); + assert.deepStrictEqual( + gitBaseBranch.gitWorktreeInfoInternal('/x', { execGit: git }), + { inside: true, worktreeRoot: null }); + }); + + test('--show-toplevel succeeds → the trimmed path is returned', () => { + const git = makeFaultyGit({ + passthrough: (args) => gitResult({ stdout: args[1] === '--show-toplevel' ? ' /repo/root \n' : 'true\n' }), + }); + assert.deepStrictEqual( + gitBaseBranch.gitWorktreeInfoInternal('/x', { execGit: git }), + { inside: true, worktreeRoot: '/repo/root' }); + }); + + test('execGit THROWS → {inside:false, worktreeRoot:null} (catch arm)', () => { + assert.deepStrictEqual( + gitBaseBranch.gitWorktreeInfoInternal('/x', { execGit: throwingGit('git is gone') }), + { inside: false, worktreeRoot: null }); + }); + + test('both worktree probes are bounded and receive the caller cwd', () => { + const git = makeFaultyGit({ + passthrough: (args) => gitResult({ stdout: args[1] === '--show-toplevel' ? '/repo/root\n' : 'true\n' }), + }); + gitBaseBranch.gitWorktreeInfoInternal('/some/cwd', { execGit: git }); + assert.deepStrictEqual(git.calls, [ + { args: ['rev-parse', '--is-inside-work-tree'], opts: { cwd: '/some/cwd', timeout: 5000 } }, + { args: ['rev-parse', '--show-toplevel'], opts: { cwd: '/some/cwd', timeout: 5000 } }, + ]); + }); +}); + +describe('#3057 W3: cmdGitBaseBranch — the DEFAULT diagnostic sink', () => { + test('with no writeDiagnostic injected, the unverified warning goes to process.stderr', (t) => { + const written = []; + t.mock.method(process.stderr, 'write', (chunk) => { written.push(String(chunk)); return true; }); + + const stdout = []; + const branch = gitBaseBranch.cmdGitBaseBranch('/x', [], { + readFile: () => null, + execGit: makeFaultyGit({ faults: [{ kind: 'timeout' }] }), + write: (s) => { stdout.push(s); }, + // writeDiagnostic deliberately omitted → the process.stderr default arm. + }); + + assert.strictEqual(branch, 'main'); + assert.deepStrictEqual(stdout, ['main\n'], 'the stdout contract five workflows parse is unchanged'); + assert.strictEqual(written.length, 1, 'exactly one diagnostic must reach the default stderr sink'); + assert.match(written[0], /WITHOUT verifying/); + }); + + test('with no writeDiagnostic injected and a VERIFIED answer, process.stderr is untouched', (t) => { + const written = []; + t.mock.method(process.stderr, 'write', (chunk) => { written.push(String(chunk)); return true; }); + + const stdout = []; + const branch = gitBaseBranch.cmdGitBaseBranch('/x', [], { + readFile: () => '{"git":{"base_branch":"develop"}}', + execGit: makeFaultyGit(), + write: (s) => { stdout.push(s); }, + }); + + assert.strictEqual(branch, 'develop'); + assert.deepStrictEqual(stdout, ['develop\n']); + assert.deepStrictEqual(written, [], 'a verified answer must write nothing to the default stderr sink'); + }); +}); + // ─── setGsdConfig prototype-pollution guard (#1406) ─────────────────────────── describe('#1406: setGsdConfig prototype-pollution guard', () => { @@ -697,76 +1192,72 @@ describe('handle_branching branches off origin/HEAD, not current HEAD (#2916)', // 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`, () => { + test(`new phase branch branches off origin/${defaultBranch} with 0 inherited commits`, (t) => { const bash = extractHandleBranchingBash(); const { root, clonePath } = setupFixture(defaultBranch); + // Teardown via t.after, not try/finally — CONTRIBUTING.md "Setup and + // Cleanup" reserves try/finally for context-free helper functions. + t.after(() => cleanup(root)); - try { - const upstream = `origin/${defaultBranch}`; + 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'); + 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' + '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'), - phase02Sha, - 'existing-branch tip must be preserved (no rebase/reset)' + 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', (t) => { + const bash = extractHandleBranchingBash(); + const { root, clonePath } = setupFixture(); + t.after(() => cleanup(root)); + + // 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)' + ); }); }); });