fix: harden respect-staged pathspec handling
This commit is contained in:
@@ -426,7 +426,6 @@ node gsd-tools.cjs commit <message> [--files f1 f2] [--amend] [--no-verify] [--r
|
||||
```
|
||||
|
||||
> `--no-verify`: Skips pre-commit hooks. Used by parallel executor agents during wave-based execution to avoid build lock contention (e.g., cargo lock fights in Rust projects). The orchestrator runs hooks once after each wave completes. Do not use `--no-verify` during sequential execution — let hooks run normally.
|
||||
|
||||
> `--files <paths>` **staging behaviour**: by default, `--files` runs `git add -- <path>` for each named file before committing. This overwrites any per-hunk staging set up via `git add -p`. Pass `--respect-staged` to skip the `git add` step and commit only what is already in the index within the requested pathspec. If nothing is staged within that scope, the command returns `{ committed: false, reason: 'nothing staged' }` without error. The trailing `-- <paths>` pathspec on the commit is applied under both modes, so files staged outside the `--files` scope are never included (#3061 invariant).
|
||||
|
||||
# Web search (requires Brave API key)
|
||||
|
||||
@@ -436,6 +436,29 @@ describe('commit --respect-staged (#3522)', () => {
|
||||
expect(staged).toContain('out-of-scope.ts');
|
||||
});
|
||||
|
||||
it('treats directory pathspecs without trailing slash as directories under --respect-staged', async () => {
|
||||
const { commit } = await import('./commit.js');
|
||||
|
||||
await writeFile(join(tmpDir, 'out-of-scope.ts'), '// out of scope\n');
|
||||
execSync('git add .planning/config.json out-of-scope.ts', { cwd: tmpDir, stdio: 'pipe' });
|
||||
|
||||
const result = await commit(
|
||||
['feat: dir scope', '--files', '.planning', '--respect-staged'],
|
||||
tmpDir,
|
||||
);
|
||||
expect((result.data as { committed: boolean }).committed).toBe(true);
|
||||
|
||||
const committedFiles = execSync('git show --name-only --format= HEAD', { cwd: tmpDir, encoding: 'utf-8' })
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter(Boolean);
|
||||
expect(committedFiles).toContain('.planning/config.json');
|
||||
expect(committedFiles).not.toContain('out-of-scope.ts');
|
||||
|
||||
const staged = execSync('git diff --cached --name-only', { cwd: tmpDir, encoding: 'utf-8' }).trim();
|
||||
expect(staged).toContain('out-of-scope.ts');
|
||||
});
|
||||
|
||||
it('without --respect-staged, --files re-stages the full file even when partially pre-staged (back-compat)', async () => {
|
||||
const { commit } = await import('./commit.js');
|
||||
|
||||
|
||||
@@ -177,20 +177,42 @@ export const commit: QueryHandler = async (args, projectDir, workstream) => {
|
||||
// causes `git diff --cached` to exit non-zero with "pathspec did not match".
|
||||
// To avoid that, get all staged files and filter by the requested paths in
|
||||
// TypeScript instead.
|
||||
const stagedFiles: string[] = (() => {
|
||||
const stagedFilesResult: { files: string[] } | { error: { reason: string; exitCode: number } } = (() => {
|
||||
if (hasRespectStaged) {
|
||||
const allStaged = execGit(projectDir, ['diff', '--cached', '--name-only']);
|
||||
if (allStaged.exitCode !== 0) {
|
||||
return {
|
||||
error: {
|
||||
reason: allStaged.stderr || allStaged.stdout || 'failed to inspect staged files',
|
||||
exitCode: allStaged.exitCode,
|
||||
},
|
||||
};
|
||||
}
|
||||
const allStagedFiles = allStaged.stdout ? allStaged.stdout.split('\n').filter(Boolean) : [];
|
||||
// Build a Set of the requested paths for O(1) lookup.
|
||||
const pathSet = new Set(pathsToCommit);
|
||||
// For directory entries (e.g. '.planning/'), match by prefix.
|
||||
return allStagedFiles.filter(f =>
|
||||
pathSet.has(f) || pathsToCommit.some(p => p.endsWith('/') && f.startsWith(p)),
|
||||
);
|
||||
const normalizePathspec = (p: string) => p.replace(/\\/g, '/').replace(/\/+$/, '');
|
||||
const normalizedSpecs = pathsToCommit.map(normalizePathspec);
|
||||
return {
|
||||
files: allStagedFiles.filter(file => {
|
||||
const normalizedFile = normalizePathspec(file);
|
||||
return normalizedSpecs.some(spec => normalizedFile === spec || normalizedFile.startsWith(`${spec}/`));
|
||||
}),
|
||||
};
|
||||
}
|
||||
const diffResult = execGit(projectDir, ['diff', '--cached', '--name-only', '--', ...pathsToCommit]);
|
||||
return diffResult.stdout ? diffResult.stdout.split('\n').filter(Boolean) : [];
|
||||
if (diffResult.exitCode !== 0) {
|
||||
return {
|
||||
error: {
|
||||
reason: diffResult.stderr || diffResult.stdout || 'failed to inspect staged files',
|
||||
exitCode: diffResult.exitCode,
|
||||
},
|
||||
};
|
||||
}
|
||||
return { files: diffResult.stdout ? diffResult.stdout.split('\n').filter(Boolean) : [] };
|
||||
})();
|
||||
if ('error' in stagedFilesResult) {
|
||||
return { data: { committed: false, ...stagedFilesResult.error } };
|
||||
}
|
||||
const stagedFiles = stagedFilesResult.files;
|
||||
if (stagedFiles.length === 0) {
|
||||
return { data: { committed: false, reason: 'nothing staged' } };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user