fix(git): scope the dirty-main squash guard to staged changes only

The previous guard rejected any uncommitted tracked change (git status
--untracked-files=no), which also caught unstaged edits like a ktx.yaml that
setup writes during the flow and commits only after the context build — so the
guard wrongly blocked setup context builds and ingest (8 local-bundle-ingest
cases failed with 'uncommitted changes (ktx.yaml)').

The actual hazard is the index, not the working tree: 'git commit' captures only
staged changes, and the auto_commit:false residue is staged by 'git merge
--squash'. Narrow the check to 'git diff --cached' so only pre-existing staged
changes are refused; unstaged working-tree edits proceed untouched (never
committed by the squash). Adds a regression test that an unstaged tracked change
does not block the merge and is neither committed nor discarded.
This commit is contained in:
Andrey Avtomonov 2026-06-09 14:55:08 +02:00
parent 9ac37166f5
commit 852ac8a836
4 changed files with 45 additions and 13 deletions

View file

@ -744,19 +744,21 @@ export class GitService {
| { ok: false; conflict: true; conflictPaths: string[] }
| { ok: false; dirty: true; dirtyPaths: string[] }
> {
// The squash flow commits the whole index and, on conflict, `reset --hard`s the tree, so
// it must start from a clean target. Refuse if the target has uncommitted tracked changes
// (e.g. residue from a prior `auto_commit: false` run) rather than committing them under
// this run's message or discarding them. Untracked files (gitignored .ktx state, stray
// files) are not committed by the squash and are intentionally ignored here.
const dirtyPaths = (await this.git.raw(['status', '--porcelain', '--untracked-files=no']))
// `git commit` after the squash commits the WHOLE index, and conflict cleanup `reset
// --hard`s the tree. So pre-existing *staged* changes are the hazard: they would be
// committed under this run's message, or discarded on conflict. The residue from a prior
// `auto_commit: false` run is exactly that — `git merge --squash` stages without
// committing. Refuse when the index already differs from HEAD. Unstaged working-tree edits
// (e.g. a `ktx.yaml` written by setup and committed later, or user edits) are NOT captured
// by `git commit`, so they must not block the squash; untracked files are likewise ignored.
const dirtyPaths = (await this.git.raw(['diff', '--cached', '--name-only']))
.split('\n')
.map((line) => line.slice(3).trim())
.map((line) => line.trim())
.filter(Boolean);
if (dirtyPaths.length > 0) {
this.logger.warn(
`applySquashToIndex: target worktree has ${dirtyPaths.length} uncommitted change(s) ` +
`(${dirtyPaths.slice(0, 5).join(', ')}); refusing to squash ${branch} to avoid contaminating or discarding them`,
`applySquashToIndex: target index has ${dirtyPaths.length} staged change(s) ` +
`(${dirtyPaths.slice(0, 5).join(', ')}); refusing to squash ${branch} to avoid committing or discarding them`,
);
return { ok: false, dirty: true, dirtyPaths };
}

View file

@ -2700,8 +2700,8 @@ export class IngestBundleRunner {
await this.deps.runs.markFailed(runRow.id);
if ('dirty' in squashResult.merge) {
throw new Error(
'The project working tree has uncommitted changes ' +
`(${squashResult.merge.dirtyPaths.slice(0, 5).join(', ')}); commit or discard them before ingesting ` +
'The project has staged but uncommitted changes ' +
`(${squashResult.merge.dirtyPaths.slice(0, 5).join(', ')}); commit or unstage them before ingesting ` +
'(this typically means a previous run with storage.git.auto_commit: false was left staged).',
);
}

View file

@ -276,8 +276,8 @@ export class MemoryAgentService {
sessionOutcome = 'conflict';
if ('dirty' in mergeResult) {
this.logger.warn(
`[memory-agent] chat=${chatId} not landed: project working tree has uncommitted changes ` +
`(${mergeResult.dirtyPaths.slice(0, 5).join(', ')}); commit or discard them and re-run ` +
`[memory-agent] chat=${chatId} not landed: project has staged but uncommitted changes ` +
`(${mergeResult.dirtyPaths.slice(0, 5).join(', ')}); commit or unstage them and re-run ` +
'(a previous run with auto_commit disabled may have been left staged).',
);
} else {

View file

@ -496,6 +496,36 @@ describe('GitService', () => {
await rm(wtDir, { recursive: true, force: true }).catch(() => undefined);
});
it('allows the squash when main has only unstaged (not staged) changes', async () => {
// e.g. setup writes ktx.yaml during the flow and commits it only after the context
// build, leaving it modified-but-unstaged. `git commit` never captures unstaged changes,
// so the squash must proceed and leave them untouched.
const { commitHash: baseSha } = await writeAndCommit('config.yaml', 'a: 1\n');
const parent = await realpath(join(tempDir, '..'));
const wtDir = join(parent, `wt-${Date.now()}-unstaged`);
await service.addWorktree(wtDir, 'session/unstaged', baseSha);
const scoped = service.forWorktree(wtDir);
await writeFile(join(wtDir, 'from-branch.yaml'), 'b: 1\n', 'utf-8');
await scoped.commitFile('from-branch.yaml', 'wip', 'System User', 'system@example.com');
// Modify a tracked file on main WITHOUT staging it.
await writeFile(join(tempDir, 'config.yaml'), 'a: 2\n', 'utf-8');
const result = await service.squashMergeIntoMain('session/unstaged', 'System User', 'system@example.com', 'msg');
expect(result.ok).toBe(true);
if (!result.ok || !('squashSha' in result)) {
throw new Error('expected the squash to land');
}
expect(result.touchedPaths).toEqual(['from-branch.yaml']);
// The branch landed, and the unstaged edit was neither committed nor discarded.
await expect(readFile(join(tempDir, 'from-branch.yaml'), 'utf-8')).resolves.toBe('b: 1\n');
await expect(readFile(join(tempDir, 'config.yaml'), 'utf-8')).resolves.toBe('a: 2\n');
await service.removeWorktree(wtDir).catch(() => undefined);
await rm(wtDir, { recursive: true, force: true }).catch(() => undefined);
});
it('stageSquashMergeIntoMain also refuses on a dirty main', async () => {
const { commitHash: baseSha } = await writeAndCommit('seed.md', 'seed');
const parent = await realpath(join(tempDir, '..'));