Skip to content

Commit 7f30b80

Browse files
nigel-devnigel-dev
committed
fix(merge-train): run setup commands before test gating
1 parent badeadf commit 7f30b80

3 files changed

Lines changed: 272 additions & 4 deletions

File tree

‎src/lib/merge-train.ts‎

Lines changed: 158 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { join } from 'path';
2+
import { existsSync, lstatSync, rmSync } from 'fs';
23
import type { JobSpec } from './plan-types';
34
import { gitCommand } from './git';
45
import { getIntegrationWorktree } from './integration';
@@ -17,10 +18,20 @@ type MergeTrainConfig = {
1718
testCommand?: string;
1819
testTimeout?: number;
1920
mergeStrategy?: 'squash' | 'ff-only' | 'merge';
21+
setupCommands?: string[];
2022
};
2123

2224
const DEFAULT_TEST_TIMEOUT_MS = 600000;
2325

26+
const INSTALL_COMMAND_BY_LOCKFILE = [
27+
{ file: 'bun.lockb', command: 'bun install --frozen-lockfile' },
28+
{ file: 'bun.lock', command: 'bun install --frozen-lockfile' },
29+
{ file: 'pnpm-lock.yaml', command: 'pnpm install --frozen-lockfile' },
30+
{ file: 'yarn.lock', command: 'yarn install --frozen-lockfile' },
31+
{ file: 'package-lock.json', command: 'npm ci' },
32+
{ file: 'npm-shrinkwrap.json', command: 'npm ci' },
33+
] as const;
34+
2435

2536

2637
async function rollbackMerge(worktreePath: string): Promise<void> {
@@ -61,6 +72,40 @@ export async function detectTestCommand(worktreePath: string): Promise<string |
6172
}
6273
}
6374

75+
export async function detectInstallCommand(worktreePath: string): Promise<string | null> {
76+
try {
77+
for (const entry of INSTALL_COMMAND_BY_LOCKFILE) {
78+
const lockfile = Bun.file(join(worktreePath, entry.file));
79+
if (await lockfile.exists()) {
80+
return entry.command;
81+
}
82+
}
83+
84+
const packageJsonFile = Bun.file(join(worktreePath, 'package.json'));
85+
if (!(await packageJsonFile.exists())) {
86+
return null;
87+
}
88+
89+
const packageJson = JSON.parse(await packageJsonFile.text()) as {
90+
packageManager?: string;
91+
};
92+
const packageManager = packageJson.packageManager?.toLowerCase() ?? '';
93+
if (packageManager.startsWith('bun@')) {
94+
return 'bun install';
95+
}
96+
if (packageManager.startsWith('pnpm@')) {
97+
return 'pnpm install';
98+
}
99+
if (packageManager.startsWith('yarn@')) {
100+
return 'yarn install';
101+
}
102+
103+
return null;
104+
} catch {
105+
return null;
106+
}
107+
}
108+
64109
export async function runTestCommand(
65110
worktreePath: string,
66111
command: string,
@@ -94,6 +139,103 @@ export async function runTestCommand(
94139
};
95140
}
96141

142+
function normalizeCommands(commands?: string[]): string[] {
143+
if (!commands) {
144+
return [];
145+
}
146+
return [...new Set(commands.map((command) => command.trim()).filter(Boolean))];
147+
}
148+
149+
function getNodeModulesStatus(worktreePath: string): 'present' | 'missing' | 'dangling_symlink' {
150+
const nodeModulesPath = join(worktreePath, 'node_modules');
151+
if (existsSync(nodeModulesPath)) {
152+
return 'present';
153+
}
154+
155+
try {
156+
const stat = lstatSync(nodeModulesPath);
157+
if (stat.isSymbolicLink()) {
158+
return 'dangling_symlink';
159+
}
160+
} catch {}
161+
162+
return 'missing';
163+
}
164+
165+
async function ensureTestDependencies(
166+
worktreePath: string,
167+
timeoutMs: number,
168+
setupCommands?: string[],
169+
): Promise<{ success: boolean; output: string; timedOut: boolean }> {
170+
const configuredSetupCommands = normalizeCommands(setupCommands);
171+
if (configuredSetupCommands.length > 0) {
172+
for (const command of configuredSetupCommands) {
173+
const setupResult = await runTestCommand(worktreePath, command, timeoutMs);
174+
if (setupResult.success) {
175+
continue;
176+
}
177+
178+
const prefix = setupResult.timedOut
179+
? `Dependency setup command timed out after ${timeoutMs}ms`
180+
: 'Dependency setup command failed';
181+
182+
return {
183+
...setupResult,
184+
output: setupResult.output
185+
? `${prefix} (${command})\n${setupResult.output}`
186+
: `${prefix} (${command})`,
187+
};
188+
}
189+
190+
return {
191+
success: true,
192+
output: '',
193+
timedOut: false,
194+
};
195+
}
196+
197+
const installCommand = await detectInstallCommand(worktreePath);
198+
if (!installCommand) {
199+
return {
200+
success: true,
201+
output: '',
202+
timedOut: false,
203+
};
204+
}
205+
206+
const nodeModulesStatus = getNodeModulesStatus(worktreePath);
207+
if (nodeModulesStatus === 'present') {
208+
return {
209+
success: true,
210+
output: '',
211+
timedOut: false,
212+
};
213+
}
214+
215+
if (nodeModulesStatus === 'dangling_symlink') {
216+
rmSync(join(worktreePath, 'node_modules'), {
217+
force: true,
218+
recursive: true,
219+
});
220+
}
221+
222+
const installResult = await runTestCommand(worktreePath, installCommand, timeoutMs);
223+
if (installResult.success) {
224+
return installResult;
225+
}
226+
227+
const prefix = installResult.timedOut
228+
? `Dependency install timed out after ${timeoutMs}ms`
229+
: 'Dependency install failed';
230+
231+
return {
232+
...installResult,
233+
output: installResult.output
234+
? `${prefix} (${installCommand})\n${installResult.output}`
235+
: `${prefix} (${installCommand})`,
236+
};
237+
}
238+
97239
export class MergeTrain {
98240
private queue: JobSpec[] = [];
99241

@@ -253,7 +395,7 @@ export class MergeTrain {
253395

254396
if (!testCommand) {
255397
console.warn(
256-
`No test command found in ${this.integrationWorktree}/package.json. Skipping test gating.`,
398+
`No test command configured or detected in ${this.integrationWorktree}. Skipping test gating.`,
257399
);
258400
return {
259401
success: true,
@@ -262,6 +404,21 @@ export class MergeTrain {
262404
}
263405

264406
const timeoutMs = this.config?.testTimeout ?? DEFAULT_TEST_TIMEOUT_MS;
407+
const dependencySetupResult = await ensureTestDependencies(
408+
this.integrationWorktree,
409+
timeoutMs,
410+
this.config?.setupCommands,
411+
);
412+
413+
if (!dependencySetupResult.success) {
414+
await rollbackMergeToHead(this.integrationWorktree, headBeforeStr);
415+
return {
416+
success: false,
417+
type: 'test_failure',
418+
output: dependencySetupResult.output,
419+
};
420+
}
421+
265422
const testResult = await runTestCommand(this.integrationWorktree, testCommand, timeoutMs);
266423

267424
if (testResult.timedOut) {

‎src/lib/orchestrator.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,16 +151,25 @@ export class Orchestrator {
151151
private jobsLaunchedCount = 0;
152152
private firstJobCompleted = false;
153153

154-
private getMergeTrainConfig(): { testCommand?: string; testTimeout?: number; mergeStrategy?: 'squash' | 'ff-only' | 'merge' } {
154+
private getMergeTrainConfig(): {
155+
testCommand?: string;
156+
testTimeout?: number;
157+
mergeStrategy?: 'squash' | 'ff-only' | 'merge';
158+
setupCommands?: string[];
159+
} {
155160
const config = this.config as MCConfig & {
156161
testCommand?: string;
157162
testTimeout?: number;
158163
mergeStrategy?: 'squash' | 'ff-only' | 'merge';
164+
worktreeSetup?: {
165+
commands?: string[];
166+
};
159167
};
160168
return {
161169
testCommand: config.testCommand,
162170
testTimeout: config.testTimeout,
163171
mergeStrategy: config.mergeStrategy,
172+
setupCommands: config.worktreeSetup?.commands,
164173
};
165174
}
166175

‎tests/lib/merge-train.test.ts‎

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
22
import { join } from 'path';
3-
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'fs';
3+
import { existsSync, mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'fs';
44
import { tmpdir } from 'os';
55
import type { JobSpec } from '../../src/lib/plan-types';
6-
import { MergeTrain, detectTestCommand } from '../../src/lib/merge-train';
6+
import { MergeTrain, detectInstallCommand, detectTestCommand } from '../../src/lib/merge-train';
77

88
type TestRepo = {
99
rootDir: string;
@@ -66,6 +66,7 @@ async function setupRepo(): Promise<TestRepo> {
6666
join(repoDir, 'package.json'),
6767
JSON.stringify({ scripts: { test: 'true' } }, null, 2),
6868
);
69+
writeFileSync(join(repoDir, '.gitignore'), 'node_modules\n');
6970
writeFileSync(join(repoDir, 'base.txt'), 'base\n');
7071

7172
await mustExec(['git', 'add', '.'], repoDir);
@@ -74,6 +75,7 @@ async function setupRepo(): Promise<TestRepo> {
7475

7576
await mustExec(['git', 'branch', 'integration'], repoDir);
7677
await mustExec(['git', 'worktree', 'add', integrationWorktree, 'integration'], repoDir);
78+
mkdirSync(join(integrationWorktree, 'node_modules'), { recursive: true });
7779

7880
return { rootDir, repoDir, integrationWorktree };
7981
}
@@ -190,6 +192,106 @@ describe('MergeTrain', () => {
190192
expect(command).toBe('bun test tests/smoke.test.ts');
191193
});
192194

195+
it('detects install command from lockfile', async () => {
196+
writeFileSync(
197+
join(testRepo.integrationWorktree, 'package-lock.json'),
198+
'{"name":"repo","lockfileVersion":3}',
199+
);
200+
201+
const command = await detectInstallCommand(testRepo.integrationWorktree);
202+
expect(command).toBe('npm ci');
203+
});
204+
205+
it('installs dependencies when node_modules is missing before tests', async () => {
206+
await createBranchCommit(testRepo.repoDir, 'feature-install', 'install.txt', 'install\n');
207+
208+
rmSync(join(testRepo.integrationWorktree, 'node_modules'), {
209+
recursive: true,
210+
force: true,
211+
});
212+
213+
writeFileSync(
214+
join(testRepo.integrationWorktree, 'package.json'),
215+
JSON.stringify(
216+
{
217+
name: 'repo',
218+
version: '1.0.0',
219+
packageManager: '[email protected]',
220+
scripts: { test: 'test -d node_modules' },
221+
},
222+
null,
223+
2,
224+
),
225+
);
226+
227+
const train = new MergeTrain(testRepo.integrationWorktree, {
228+
testCommand: 'test -d node_modules',
229+
testTimeout: 60000,
230+
});
231+
train.enqueue(makeJob('feature-install'));
232+
233+
const result = await train.processNext();
234+
235+
expect(result.success).toBe(true);
236+
expect(existsSync(join(testRepo.integrationWorktree, 'node_modules'))).toBe(true);
237+
});
238+
239+
it('runs configured setup commands before tests', async () => {
240+
await createBranchCommit(testRepo.repoDir, 'feature-setup', 'setup.txt', 'setup\n');
241+
242+
rmSync(join(testRepo.integrationWorktree, '.deps-ready'), {
243+
recursive: true,
244+
force: true,
245+
});
246+
247+
const train = new MergeTrain(testRepo.integrationWorktree, {
248+
setupCommands: ['touch .deps-ready'],
249+
testCommand: 'test -f .deps-ready',
250+
testTimeout: 60000,
251+
});
252+
train.enqueue(makeJob('feature-setup'));
253+
254+
const result = await train.processNext();
255+
256+
expect(result.success).toBe(true);
257+
expect(existsSync(join(testRepo.integrationWorktree, '.deps-ready'))).toBe(true);
258+
});
259+
260+
it('rolls back merge when setup command fails', async () => {
261+
await createBranchCommit(
262+
testRepo.repoDir,
263+
'feature-setup-fail',
264+
'setup-fail.txt',
265+
'setup-fail\n',
266+
);
267+
268+
const headBefore = await mustExec(
269+
['git', 'rev-parse', 'HEAD'],
270+
testRepo.integrationWorktree,
271+
);
272+
273+
const train = new MergeTrain(testRepo.integrationWorktree, {
274+
setupCommands: ['false'],
275+
testCommand: 'true',
276+
testTimeout: 60000,
277+
});
278+
train.enqueue(makeJob('feature-setup-fail'));
279+
280+
const result = await train.processNext();
281+
282+
expect(result.success).toBe(false);
283+
if (!result.success) {
284+
expect(result.type).toBe('test_failure');
285+
expect(result.output).toContain('Dependency setup command failed');
286+
}
287+
288+
const headAfter = await mustExec(
289+
['git', 'rev-parse', 'HEAD'],
290+
testRepo.integrationWorktree,
291+
);
292+
expect(headAfter).toBe(headBefore);
293+
});
294+
193295
it('rolls back merge when tests fail', async () => {
194296
await createBranchCommit(testRepo.repoDir, 'feature-fail', 'fail.txt', 'fail\n');
195297

0 commit comments

Comments
 (0)