Skip to content

Commit 9a15d24

Browse files
committed
fix(merge): refuse rollback when main worktree is dirty
1 parent 7f30b80 commit 9a15d24

2 files changed

Lines changed: 98 additions & 6 deletions

File tree

src/tools/merge.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,21 @@ async function getBaseBranch(cwd: string): Promise<string> {
1919
throw new Error('Could not find main or master branch');
2020
}
2121

22+
async function assertCleanWorktree(cwd: string): Promise<void> {
23+
const status = await gitCommand(['status', '--porcelain'], { cwd });
24+
if (status.exitCode !== 0) {
25+
throw new Error(
26+
`Failed to check main worktree status: ${status.stderr || status.stdout}`,
27+
);
28+
}
29+
30+
if (status.stdout.trim() !== '') {
31+
throw new Error(
32+
'Main worktree has uncommitted changes. Refusing merge because automatic rollback may discard local changes. Commit, stash, or clean the worktree and retry.',
33+
);
34+
}
35+
}
36+
2237
export const mc_merge: ToolDefinition = tool({
2338
description: 'Merge a job\'s branch back to main (for non-PR workflows)',
2439
args: {
@@ -60,7 +75,8 @@ export const mc_merge: ToolDefinition = tool({
6075
);
6176
}
6277

63-
// 7. Execute merge based on strategy
78+
await assertCleanWorktree(mainWorktreePath);
79+
6480
if (mergeStrategy === 'squash') {
6581
// Squash: merge --squash then commit
6682
const squashResult = await gitCommand(['merge', '--squash', job.branch], {
@@ -146,7 +162,6 @@ export const mc_merge: ToolDefinition = tool({
146162
}
147163
}
148164

149-
// 8. Check if job belongs to active plan
150165
let planWarning = '';
151166
if (job.planId) {
152167
const activePlan = await loadPlan();
@@ -156,7 +171,6 @@ export const mc_merge: ToolDefinition = tool({
156171
}
157172
}
158173

159-
// 9. Return success message
160174
const lines: string[] = [
161175
`${planWarning}Successfully merged '${job.branch}' into '${baseBranch}'`,
162176
'',

tests/tools/merge.test.ts

Lines changed: 81 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, beforeEach, afterEach, vi, type Mock } from 'vitest';
1+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
22
import { join } from 'path';
33
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'fs';
44
import { tmpdir } from 'os';
@@ -7,10 +7,11 @@ import * as jobState from '../../src/lib/job-state';
77
import * as worktree from '../../src/lib/worktree';
88
import * as config from '../../src/lib/config';
99
import * as planState from '../../src/lib/plan-state';
10+
import * as git from '../../src/lib/git';
1011

1112
const { mc_merge } = await import('../../src/tools/merge');
1213

13-
let mockGetJobByName: Mock;
14+
let mockGetJobByName: any;
1415

1516
const mockContext = {
1617
sessionID: 'test-session',
@@ -25,8 +26,42 @@ const mockContext = {
2526

2627
describe('mc_merge', () => {
2728
beforeEach(() => {
28-
vi.clearAllMocks();
29+
vi.restoreAllMocks();
2930
mockGetJobByName = vi.spyOn(jobState, 'getJobByName').mockImplementation(() => undefined as any);
31+
vi.spyOn(worktree, 'getMainWorktree').mockResolvedValue('/tmp/mc-merge-mock-main');
32+
vi.spyOn(config, 'loadConfig').mockResolvedValue({ mergeStrategy: 'squash' } as any);
33+
vi.spyOn(planState, 'loadPlan').mockResolvedValue(null);
34+
vi.spyOn(git, 'gitCommand').mockImplementation(async (args: string[]) => {
35+
if (
36+
args[0] === 'rev-parse' &&
37+
args.includes('--verify') &&
38+
args[args.length - 1] === 'main'
39+
) {
40+
return { stdout: 'main', stderr: '', exitCode: 0 };
41+
}
42+
43+
if (
44+
args[0] === 'rev-parse' &&
45+
args.includes('--verify') &&
46+
args[args.length - 1] === 'master'
47+
) {
48+
return { stdout: '', stderr: '', exitCode: 1 };
49+
}
50+
51+
if (args[0] === 'rev-parse' && args.includes('--abbrev-ref')) {
52+
return { stdout: 'main', stderr: '', exitCode: 0 };
53+
}
54+
55+
if (args[0] === 'merge' && args.includes('--squash')) {
56+
return { stdout: '', stderr: 'mock squash merge failure', exitCode: 1 };
57+
}
58+
59+
return { stdout: '', stderr: '', exitCode: 0 };
60+
});
61+
});
62+
63+
afterEach(() => {
64+
vi.restoreAllMocks();
3065
});
3166

3267
describe('tool definition', () => {
@@ -59,6 +94,49 @@ describe('mc_merge', () => {
5994
});
6095
});
6196

97+
describe('safety guard', () => {
98+
it('should refuse merge when main worktree has uncommitted changes', async () => {
99+
const job: Job = {
100+
id: 'job-dirty',
101+
name: 'dirty-merge',
102+
worktreePath: '/tmp/mc-worktrees/dirty-merge',
103+
branch: 'mc/dirty-merge',
104+
tmuxTarget: 'mc-dirty-merge',
105+
placement: 'session',
106+
status: 'running',
107+
prompt: 'Dirty merge test',
108+
mode: 'vanilla',
109+
createdAt: new Date().toISOString(),
110+
};
111+
112+
mockGetJobByName.mockResolvedValue(job);
113+
114+
(git.gitCommand as any).mockImplementation(async (args: string[]) => {
115+
if (
116+
args[0] === 'rev-parse' &&
117+
args.includes('--verify') &&
118+
args[args.length - 1] === 'main'
119+
) {
120+
return { stdout: 'main', stderr: '', exitCode: 0 };
121+
}
122+
123+
if (args[0] === 'rev-parse' && args.includes('--abbrev-ref')) {
124+
return { stdout: 'main', stderr: '', exitCode: 0 };
125+
}
126+
127+
if (args[0] === 'status' && args.includes('--porcelain')) {
128+
return { stdout: ' M README.md', stderr: '', exitCode: 0 };
129+
}
130+
131+
return { stdout: '', stderr: '', exitCode: 0 };
132+
});
133+
134+
await expect(
135+
mc_merge.execute({ name: 'dirty-merge' }, mockContext),
136+
).rejects.toThrow('Main worktree has uncommitted changes');
137+
});
138+
});
139+
62140
describe('tool args validation', () => {
63141
it('should have name as required arg', () => {
64142
const nameArg = mc_merge.args.name;

0 commit comments

Comments
 (0)