feat(sdk): add --respect-staged opt-in to gsd-sdk query commit --files (#3522)
Closes #3522. When --respect-staged is passed the git add loop is skipped entirely so per-hunk staging from git add -p is preserved. Default behavior (full re-stage) is unchanged. The #3061 pathspec invariant holds under both modes. Nothing-staged within scope returns { committed: false, reason: 'nothing staged' } without error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
5
.changeset/kind-foxes-click.md
Normal file
5
.changeset/kind-foxes-click.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Added
|
||||
pr: 3522
|
||||
---
|
||||
**`gsd-sdk query commit --files` gains `--respect-staged` opt-in** — pass `--respect-staged` to skip the automatic `git add` step and commit only what is already staged within the requested pathspec. Callers using `git add -p` to stage individual hunks no longer have their partial staging silently overwritten. Without the flag, default behavior is unchanged (#3061 invariant preserved under both modes).
|
||||
@@ -422,11 +422,13 @@ node gsd-tools.cjs audit-open [--json]
|
||||
node gsd-tools.cjs from-gsd2 [--path <dir>] [--force] [--dry-run]
|
||||
|
||||
# Git commit with config checks
|
||||
node gsd-tools.cjs commit <message> [--files f1 f2] [--amend] [--no-verify]
|
||||
node gsd-tools.cjs commit <message> [--files f1 f2] [--amend] [--no-verify] [--respect-staged]
|
||||
```
|
||||
|
||||
> `--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)
|
||||
node gsd-tools.cjs websearch <query> [--limit N] [--freshness day|week|month]
|
||||
```
|
||||
|
||||
@@ -298,7 +298,7 @@ Disposition: **Registered** = handled in `createRegistry()` under the listed SDK
|
||||
| `state` (subcommands) | `state.load`, `state.json`, `state.get`, `state.update`, `state.patch`, … | Registered | Dotted and `state …` space aliases in `index.ts`. |
|
||||
| `resolve-model` | `resolve-model` | Registered | |
|
||||
| `find-phase` | `find-phase` | Registered | Golden: subset parity (see above). |
|
||||
| `commit`, `check-commit`, `commit-to-subrepo` | `commit`, `check-commit`, `commit-to-subrepo` | Registered | |
|
||||
| `commit`, `check-commit`, `commit-to-subrepo` | `commit`, `check-commit`, `commit-to-subrepo` | Registered | `commit`: `--files` re-stages full files by default; use `--respect-staged` to skip `git add` and commit only what is already staged within the pathspec (#3522). |
|
||||
| `verify-summary` | `verify-summary`, `verify.summary`, `verify summary` | Registered | |
|
||||
| `template` | `template.fill`, `template.select`, … | Registered | |
|
||||
| `frontmatter` | `frontmatter.get`, `frontmatter.set`, … | Registered | |
|
||||
|
||||
@@ -339,3 +339,124 @@ describe('commit input validation and option safety (#3061)', () => {
|
||||
expect(committed).toContain(dashName);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── --respect-staged flag (#3522) ────────────────────────────────────────
|
||||
//
|
||||
// When --respect-staged is passed:
|
||||
// 1. The git add loop is skipped entirely — partial (per-hunk) staging is
|
||||
// preserved rather than overwritten by a full `git add -- <file>`.
|
||||
// 2. If nothing is staged within the requested pathspec, the handler
|
||||
// returns { committed: false, reason: 'nothing staged' } without error.
|
||||
// 3. The #3061 invariant still holds: files staged *outside* --files <paths>
|
||||
// do not appear in the commit (trailing -- pathspec still applied).
|
||||
// 4. Without --respect-staged, --files re-stages the full file as before
|
||||
// (back-compat: partial staging is overwritten).
|
||||
|
||||
describe('commit --respect-staged (#3522)', () => {
|
||||
// Each test needs an existing HEAD so we can create tracked files and
|
||||
// simulate `git add -p` style partial staging.
|
||||
beforeEach(async () => {
|
||||
await writeFile(join(tmpDir, 'README.md'), 'init\n');
|
||||
execSync('git add README.md', { cwd: tmpDir, stdio: 'pipe' });
|
||||
execSync('git commit -m "init"', { cwd: tmpDir, stdio: 'pipe' });
|
||||
await writeFile(
|
||||
join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify({ commit_docs: true }),
|
||||
);
|
||||
});
|
||||
|
||||
it('commits only the pre-staged file when --respect-staged is passed (simulated git add -p)', async () => {
|
||||
const { commit } = await import('./commit.js');
|
||||
|
||||
// Simulate a `git add -p` scenario using two files:
|
||||
// - staged-hunk.ts: the hunk the operator chose to stage (staged beforehand)
|
||||
// - unstaged-hunk.ts: the hunk the operator left out of the index
|
||||
// Both paths are listed in --files. Without --respect-staged the handler
|
||||
// would git add both, committing all working-tree changes. With
|
||||
// --respect-staged it must commit only what is already in the index.
|
||||
await writeFile(join(tmpDir, 'staged-hunk.ts'), '// selected hunk\nconst a = 1;\n');
|
||||
await writeFile(join(tmpDir, 'unstaged-hunk.ts'), '// skipped hunk\nconst b = 2;\n');
|
||||
|
||||
// Stage only the selected hunk — the skipped hunk stays in the work tree.
|
||||
execSync('git add staged-hunk.ts', { cwd: tmpDir, stdio: 'pipe' });
|
||||
|
||||
const result = await commit(
|
||||
['feat: partial hunk commit', '--files', 'staged-hunk.ts', 'unstaged-hunk.ts', '--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);
|
||||
// Only the pre-staged hunk must appear in the commit.
|
||||
expect(committedFiles).toContain('staged-hunk.ts');
|
||||
expect(committedFiles).not.toContain('unstaged-hunk.ts');
|
||||
});
|
||||
|
||||
it('returns { committed: false, reason: "nothing staged" } when --respect-staged has nothing staged in pathspec', async () => {
|
||||
const { commit } = await import('./commit.js');
|
||||
|
||||
// Create a file but do NOT stage it.
|
||||
await writeFile(join(tmpDir, 'unstaged.ts'), '// not staged\n');
|
||||
|
||||
const result = await commit(
|
||||
['feat: should not commit', '--files', 'unstaged.ts', '--respect-staged'],
|
||||
tmpDir,
|
||||
);
|
||||
expect((result.data as { committed: boolean }).committed).toBe(false);
|
||||
expect((result.data as { reason: string }).reason).toBe('nothing staged');
|
||||
});
|
||||
|
||||
it('files staged outside --files pathspec are excluded under --respect-staged (#3061 invariant)', async () => {
|
||||
const { commit } = await import('./commit.js');
|
||||
|
||||
// Stage an "in-scope" file and an "out-of-scope" file.
|
||||
await writeFile(join(tmpDir, 'in-scope.ts'), '// in scope\n');
|
||||
await writeFile(join(tmpDir, 'out-of-scope.ts'), '// out of scope\n');
|
||||
execSync('git add in-scope.ts out-of-scope.ts', { cwd: tmpDir, stdio: 'pipe' });
|
||||
|
||||
// --files only names in-scope.ts. --respect-staged skips git add.
|
||||
const result = await commit(
|
||||
['feat: scoped commit', '--files', 'in-scope.ts', '--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('in-scope.ts');
|
||||
expect(committedFiles).not.toContain('out-of-scope.ts');
|
||||
|
||||
// out-of-scope.ts must remain staged (not committed, not lost).
|
||||
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');
|
||||
|
||||
// Write a file, land it, then modify it in working tree without staging.
|
||||
await writeFile(join(tmpDir, 'full-file.ts'), '// original\n');
|
||||
execSync('git add full-file.ts', { cwd: tmpDir, stdio: 'pipe' });
|
||||
execSync('git commit -m "land file"', { cwd: tmpDir, stdio: 'pipe' });
|
||||
|
||||
// Unstaged modification in the working tree.
|
||||
await writeFile(join(tmpDir, 'full-file.ts'), '// modified in working tree\n');
|
||||
|
||||
// Without --respect-staged, --files full-file.ts should run git add and
|
||||
// stage the full working-tree content before committing.
|
||||
const result = await commit(
|
||||
['feat: full restage', '--files', 'full-file.ts'],
|
||||
tmpDir,
|
||||
);
|
||||
expect((result.data as { committed: boolean }).committed).toBe(true);
|
||||
|
||||
// The full working-tree change must be in the commit.
|
||||
const diff = execSync('git show HEAD -- full-file.ts', { cwd: tmpDir, encoding: 'utf-8' });
|
||||
expect(diff).toContain('// modified in working tree');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -91,7 +91,19 @@ export function sanitizeCommitMessage(text: string): string {
|
||||
* Checks commit_docs config (unless --force), sanitizes message,
|
||||
* stages specified files (or all .planning/), and commits.
|
||||
*
|
||||
* @param args - args[0]=message, remaining=file paths or flags (--force, --amend, --no-verify)
|
||||
* By default, `--files <paths>` runs `git add -- <path>` for each named file
|
||||
* before committing. This means any per-hunk staging set up via `git add -p`
|
||||
* is **overwritten** by a full re-stage of the file's working-tree content.
|
||||
*
|
||||
* Pass `--respect-staged` to skip the `git add` step entirely. The handler
|
||||
* will commit only what is already staged within the requested pathspec. If
|
||||
* nothing is staged within that scope, the handler returns
|
||||
* `{ committed: false, reason: 'nothing staged' }` without error. The #3061
|
||||
* leak-prevention invariant still holds: the trailing `-- <paths>` pathspec
|
||||
* on the commit ensures files staged outside `--files <paths>` are excluded.
|
||||
*
|
||||
* @param args - args[0]=message, remaining=file paths or flags
|
||||
* (--force, --amend, --no-verify, --respect-staged)
|
||||
* @param projectDir - Project root directory
|
||||
* @returns QueryResult with commit result
|
||||
*/
|
||||
@@ -102,10 +114,11 @@ export const commit: QueryHandler = async (args, projectDir, workstream) => {
|
||||
const hasForce = allArgs.includes('--force');
|
||||
const hasAmend = allArgs.includes('--amend');
|
||||
const hasNoVerify = allArgs.includes('--no-verify');
|
||||
const hasRespectStaged = allArgs.includes('--respect-staged');
|
||||
const filesIndex = allArgs.indexOf('--files');
|
||||
const endIndex = filesIndex !== -1 ? filesIndex : allArgs.length;
|
||||
// CodeRabbit #6: don't strip arbitrary `--foo` tokens from commit messages
|
||||
const knownFlags = new Set(['--force', '--amend', '--no-verify']);
|
||||
const knownFlags = new Set(['--force', '--amend', '--no-verify', '--respect-staged']);
|
||||
const messageArgs = allArgs.slice(0, endIndex).filter(a => !knownFlags.has(a));
|
||||
const message = messageArgs.join(' ') || undefined;
|
||||
const filePaths =
|
||||
@@ -142,30 +155,60 @@ export const commit: QueryHandler = async (args, projectDir, workstream) => {
|
||||
// Compute pathspec once: the handler commits exactly the paths it staged,
|
||||
// never anything that was pre-staged externally (#3061).
|
||||
const pathsToCommit = filePaths.length > 0 ? filePaths : ['.planning/'];
|
||||
for (const file of pathsToCommit) {
|
||||
// The `--` separator keeps any path that starts with `-` from being
|
||||
// interpreted as a git option (e.g. a file literally named `-A`).
|
||||
const addResult = execGit(projectDir, ['add', '--', file]);
|
||||
if (addResult.exitCode !== 0) {
|
||||
return { data: { committed: false, reason: addResult.stderr || `failed to stage ${file}`, exitCode: addResult.exitCode } };
|
||||
|
||||
// When --respect-staged is set, skip re-staging so that per-hunk staging
|
||||
// from `git add -p` is preserved. Without the flag, run git add for each
|
||||
// path (default behavior, back-compat).
|
||||
if (!hasRespectStaged) {
|
||||
for (const file of pathsToCommit) {
|
||||
// The `--` separator keeps any path that starts with `-` from being
|
||||
// interpreted as a git option (e.g. a file literally named `-A`).
|
||||
const addResult = execGit(projectDir, ['add', '--', file]);
|
||||
if (addResult.exitCode !== 0) {
|
||||
return { data: { committed: false, reason: addResult.stderr || `failed to stage ${file}`, exitCode: addResult.exitCode } };
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Check if anything is staged within the pathspec we're about to commit.
|
||||
const diffResult = execGit(projectDir, ['diff', '--cached', '--name-only', '--', ...pathsToCommit]);
|
||||
const stagedFiles = diffResult.stdout ? diffResult.stdout.split('\n').filter(Boolean) : [];
|
||||
// When --respect-staged is set the caller may list paths that git doesn't
|
||||
// know yet (e.g. untracked files the operator intentionally skipped via
|
||||
// `git add -p`). Passing those unknown paths directly as a git pathspec
|
||||
// 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[] = (() => {
|
||||
if (hasRespectStaged) {
|
||||
const allStaged = execGit(projectDir, ['diff', '--cached', '--name-only']);
|
||||
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 diffResult = execGit(projectDir, ['diff', '--cached', '--name-only', '--', ...pathsToCommit]);
|
||||
return diffResult.stdout ? diffResult.stdout.split('\n').filter(Boolean) : [];
|
||||
})();
|
||||
if (stagedFiles.length === 0) {
|
||||
return { data: { committed: false, reason: 'nothing staged' } };
|
||||
}
|
||||
|
||||
// Build commit command. The trailing `-- pathsToCommit` ensures the commit
|
||||
// captures only files within the requested scope, even when the caller's
|
||||
// index already had unrelated entries staged before this handler ran.
|
||||
// Build commit command. The trailing pathspec ensures the commit captures
|
||||
// only files within the requested scope (#3061 invariant).
|
||||
//
|
||||
// For the default path: use `pathsToCommit` (the caller's requested scope).
|
||||
// For --respect-staged: use `stagedFiles` (the exact set already in the
|
||||
// index within scope). This avoids git rejecting the commit when the caller
|
||||
// listed paths that are not yet known to git (e.g. skipped hunks from
|
||||
// `git add -p` that were intentionally left unstaged).
|
||||
const commitPathspec = hasRespectStaged ? stagedFiles : pathsToCommit;
|
||||
const commitArgs: string[] = hasAmend
|
||||
? ['commit', '--amend', '--no-edit']
|
||||
: ['commit', '-m', sanitized ?? ''];
|
||||
if (hasNoVerify) commitArgs.push('--no-verify');
|
||||
commitArgs.push('--', ...pathsToCommit);
|
||||
commitArgs.push('--', ...commitPathspec);
|
||||
|
||||
const commitResult = execGit(projectDir, commitArgs);
|
||||
if (commitResult.exitCode !== 0) {
|
||||
|
||||
Reference in New Issue
Block a user