Files
msd-core/tests/commit-files-pathspec.test.cjs
Tom Boucher 1d208e5af6 test(#3144): bound the git/worktree cluster onto the process seam (#3152)
* test(#3144): bound the git/worktree cluster onto the process seam

Migrates 180 unbounded sync spawn sites across 19 files. Every previously
unbounded call now carries an explicit timeout with a comment giving the
number and why.

The migration is not a callee swap. execSync and execFileSync throw on a
non-zero exit and the seam never does, so each site was classified first:
sites that rely on the throw route to gitOrThrow, and sites that already read
.status to detect an EXPECTED non-zero -- an intended cherry-pick conflict, a
rev-parse outside a repo driving a skip -- route to the never-throwing runGit
instead, which would otherwise throw on exactly the exit being probed for.

Two same-named git() helpers in worktree-cleanup.test.cjs have different
return contracts, one trimmed and one raw; both are preserved rather than
unified.

Collapses five hand-rolled throw wrappers onto one throwIfFailed in
git-fixture.cjs, which gitOrThrow now also uses so the shape cannot drift.

Allowlist drops 139 to 120; BASELINE lowered to match.

* test(#3144): fix pre-PR review findings

Documents throwIfFailed in the CONTEXT.md glossary and CONTRIBUTING.md --
it became the shared throw mechanism without either doc naming it.

Routes the sixth and seventh hand-rolled copies of the throw shape through
throwIfFailed (worktree-baseref-install, worktree-safety-reap); the first
consolidation missed both.

Converts ci-rebase-check's 8 fixture-setup calls from unchecked runGit to
gitOrThrow so a failed setup step aborts where it fails rather than
surfacing later as a confusing failure against the wrong subject.

Adds 12 direct unit tests for throwIfFailed, which until now was only
exercised transitively.

Splits verify.test.cjs's non-git grep/sed bound off GIT_TIMEOUT_MS.

---------

Co-authored-by: sim <sim@local>
2026-08-07 10:58:34 -04:00

256 lines
11 KiB
JavaScript

/**
* Regression test for #2112: gsd-tools commit --files commits the entire
* index, not the declared paths.
*
* `cmdCommit` staged exactly the files named in --files but then ran a bare
* `git commit` with no pathspec, absorbing anything else that happened to be
* staged into a commit whose message described only the named files.
*
* The fix adds `'--', ...stagedPaths` to the commit args **only when** the
* caller declared a scope (explicitFiles), and only for paths that were
* actually staged (skipped missing files are excluded to avoid #2014).
*/
const { describe, test, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const { createTempGitProject, cleanup, runGsdTools } = require('./helpers.cjs');
const { gitOrThrow } = require('./helpers/git-fixture.cjs');
// 15000ms: git plumbing (add/commit/diff/status/rev-list) on a small mkdtemp
// fixture repo — far over any observed duration for that class of call.
const GIT_TIMEOUT_MS = 15000;
describe('commit --files: pathspec honors declared scope (#2112)', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempGitProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('commit --files does not absorb unrelated staged files', () => {
// Developer stages a WIP file via git add (not via --files).
fs.writeFileSync(path.join(tmpDir, 'src-wip.txt'), 'work in progress\n');
gitOrThrow(['add', 'src-wip.txt'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
// GSD writes and commits a planning artifact, naming ONLY that file.
fs.writeFileSync(path.join(tmpDir, '.planning', 'PLAN.md'), '# Plan\n');
runGsdTools(
['commit', 'docs(01): add PLAN.md', '--files', '.planning/PLAN.md'],
tmpDir,
);
// The commit must contain ONLY .planning/PLAN.md.
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], {
cwd: tmpDir,
timeoutMs: GIT_TIMEOUT_MS,
}).trim();
assert.strictEqual(
diffOutput,
'.planning/PLAN.md',
'commit --files must contain only the named files, got:\n' + diffOutput,
);
// The WIP file must still be staged, not committed.
const statusOutput = gitOrThrow(['status', '--porcelain'], {
cwd: tmpDir,
timeoutMs: GIT_TIMEOUT_MS,
}).trim();
assert.ok(
statusOutput.includes('A src-wip.txt') || statusOutput.includes('A\tsrc-wip.txt'),
'src-wip.txt should remain staged, not committed. Status:\n' + statusOutput,
);
});
test('commit --files with two files commits exactly those two', () => {
fs.writeFileSync(path.join(tmpDir, '.planning', 'PLAN.md'), '# Plan\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'RESEARCH.md'), '# Research\n');
runGsdTools(
['commit', 'docs: artifacts', '--files', '.planning/PLAN.md', '.planning/RESEARCH.md'],
tmpDir,
);
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], {
cwd: tmpDir,
timeoutMs: GIT_TIMEOUT_MS,
});
const files = diffOutput.trim().split('\n').sort();
assert.deepEqual(
files,
['.planning/PLAN.md', '.planning/RESEARCH.md'],
'commit should contain exactly the two named files',
);
});
test('commit without --files still commits the entire .planning/ index (default path)', () => {
// Write a planning artifact and stage it.
fs.writeFileSync(path.join(tmpDir, '.planning', 'PLAN.md'), '# Plan\n');
gitOrThrow(['add', '.planning/PLAN.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
// Also stage an unrelated file.
fs.writeFileSync(path.join(tmpDir, 'extra.txt'), 'extra\n');
gitOrThrow(['add', 'extra.txt'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
runGsdTools(['commit', 'docs: default commit'], tmpDir);
// Default path (no --files) commits everything staged.
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], {
cwd: tmpDir,
timeoutMs: GIT_TIMEOUT_MS,
});
const files = diffOutput.trim().split('\n').sort();
assert.ok(
files.includes('.planning/PLAN.md') && files.includes('extra.txt'),
'default commit (no --files) should commit everything staged, got:\n' + files,
);
});
test('missing tracked file in --files is still not committed as deletion (#2014 guard)', () => {
// Create and commit STATE.md, then remove it from disk.
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n');
gitOrThrow(['add', '.planning/STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
gitOrThrow(['commit', '-m', 'add STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
fs.unlinkSync(path.join(tmpDir, '.planning', 'STATE.md'));
// Also create a valid file to commit.
fs.writeFileSync(path.join(tmpDir, '.planning', 'PLAN.md'), '# Plan\n');
runGsdTools(
['commit', 'docs: add plan', '--files', '.planning/PLAN.md', '.planning/STATE.md'],
tmpDir,
);
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-status'], {
cwd: tmpDir,
timeoutMs: GIT_TIMEOUT_MS,
});
assert.ok(
!diffOutput.includes('D\t.planning/STATE.md'),
'missing tracked file must not appear as a deletion, diff was:\n' + diffOutput,
);
assert.ok(
diffOutput.includes('.planning/PLAN.md'),
'PLAN.md should be committed',
);
});
test('commit --files with only missing files returns nothing_to_commit', () => {
// Create and commit STATE.md, then remove it from disk.
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n');
gitOrThrow(['add', '.planning/STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
gitOrThrow(['commit', '-m', 'add STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
fs.unlinkSync(path.join(tmpDir, '.planning', 'STATE.md'));
// Stage an unrelated file so the index is non-empty.
fs.writeFileSync(path.join(tmpDir, 'extra.txt'), 'extra\n');
gitOrThrow(['add', 'extra.txt'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
const result = runGsdTools(
['commit', 'docs: try', '--files', '.planning/STATE.md'],
tmpDir,
);
const parsed = JSON.parse(result.output);
assert.strictEqual(
parsed.committed, false,
'should not commit when all --files are missing',
);
assert.strictEqual(
parsed.reason, 'nothing_to_commit',
'should report nothing_to_commit, not absorb the index',
);
// The unrelated staged file must still be staged, not committed.
const statusOutput = gitOrThrow(['status', '--porcelain'], {
cwd: tmpDir,
timeoutMs: GIT_TIMEOUT_MS,
}).trim();
assert.ok(
statusOutput.includes('extra.txt'),
'extra.txt should remain staged, not absorbed into a commit',
);
});
test('#2523: absolute --files path inside the repo is committed, not silently dropped', () => {
// init phase-op emits phase_dir as an ABSOLUTE path (#2428); cmdCommit must
// accept it. The bug was path.join(cwd, absPath) → cwd+absPath (non-existent)
// → silently skipped as nothing_to_commit (#2523).
fs.writeFileSync(path.join(tmpDir, '.planning', 'A.md'), 'a\n');
const absPath = path.join(tmpDir, '.planning', 'A.md');
const res = runGsdTools(['commit', 'docs: abs path', '--files', absPath], tmpDir);
const parsed = JSON.parse(res.output);
assert.strictEqual(parsed.committed, true, `absolute path must commit, not nothing_to_commit: ${res.output}`);
// The absolute path must land in the commit, normalized to repo-relative.
const diff = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS }).trim();
assert.strictEqual(diff, '.planning/A.md', `absolute --files path must be committed (normalized to relative); got: ${diff}`);
});
test('#2523: mixed relative+absolute --files list commits BOTH (no silent partial commit)', () => {
// The sharpest symptom: a mixed list committed the relative entry, dropped the
// absolute one, and reported committed:true (#2523). Both must land.
fs.writeFileSync(path.join(tmpDir, '.planning', 'REL.md'), 'r\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'ABS.md'), 'a\n');
const absPath = path.join(tmpDir, '.planning', 'ABS.md');
const res = runGsdTools(
['commit', 'docs: mixed', '--files', '.planning/REL.md', absPath],
tmpDir,
);
const parsed = JSON.parse(res.output);
assert.strictEqual(parsed.committed, true, `mixed list must commit: ${res.output}`);
const diff = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS })
.trim().split('\n').sort();
assert.deepStrictEqual(
diff,
['.planning/ABS.md', '.planning/REL.md'],
`mixed relative+absolute list must commit BOTH entries (the bug dropped the absolute one); got: ${diff.join(',')}`,
);
});
test('#2523: out-of-repo --files path is rejected by git (staging_failed), no index pollution', (t) => {
// An absolute path resolving OUTSIDE the project root: git add rejects it. No
// index pollution (#2523). Not "path_outside_repo" (that guard was removed for
// macOS symlink compatibility — git's own rejection suffices).
//
// #2608 changed the REASON this reports, deliberately. It used to be
// `nothing_to_commit`, because a failed `git add` was skipped and the empty
// stagedPaths list fell through to the empty-changeset branch. But "nothing to
// commit" is not what happened — the caller named a file and git refused it —
// and that misreport is the very class of defect #2608 closes. The result now
// carries `staging_failed` plus the offending path and git's own message
// ("… is outside repository at …"), which is strictly more actionable.
//
// #2523's two substantive invariants are unchanged and still asserted below:
// no commit is created, and the index is left clean.
const outsideDir = path.join(tmpDir, '..', `gsd-2523-outside-${process.pid}-${Date.now()}`);
fs.mkdirSync(outsideDir, { recursive: true });
t.after(() => cleanup(outsideDir));
const outsideFile = path.join(outsideDir, 'secret.md');
fs.writeFileSync(outsideFile, 's\n');
const res = runGsdTools(
['commit', 'docs: outside', '--files', path.resolve(outsideFile)],
tmpDir,
);
const parsed = JSON.parse(res.output);
assert.strictEqual(parsed.committed, false, 'out-of-repo path must not commit');
assert.strictEqual(parsed.reason, 'staging_failed', `out-of-repo: git rejects → staging_failed (#2608): ${res.output}`);
assert.strictEqual(parsed.file, path.resolve(outsideFile), 'the rejected path must be named');
assert.match(parsed.error, /outside repository/, "git's own rejection message must be preserved (#2608)");
// No new commit created (still at the single initial commit).
const logCount = gitOrThrow(['rev-list', '--count', 'HEAD'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS }).trim();
assert.strictEqual(logCount, '1', 'no new commit must be created for an out-of-repo path');
// Index stays clean (git add failed → nothing staged).
const status = gitOrThrow(['status', '--porcelain'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS }).trim();
assert.strictEqual(status, '', `index must be clean (no pollution): ${status}`);
});
});