fix: review fixes — dead code, zero-test gate, safeBranch regex, worktree collision guard
- Remove dead worktreeExists() and unused WorktreeSpec import (#1, #2) - verify-gate: require counts.total > 0 — silent runner no longer passes (#4) - worktree-isolation: add /g flag to safeBranch regex (#5) - execution-pipeline: try/catch around wm.create with prune-and-retry (#7) - Tests updated to output parseable '1 passed' for mock test commands 69 files, 295 tests.
This commit is contained in:
parent
61bc109982
commit
e59700325a
|
|
@ -1,4 +1,4 @@
|
||||||
import { describe, it, expect, vi } from "vitest";
|
import { describe, it, expect, vi, beforeAll, afterAll } from "vitest";
|
||||||
import * as path from "node:path";
|
import * as path from "node:path";
|
||||||
import * as os from "node:os";
|
import * as os from "node:os";
|
||||||
import * as fs from "node:fs";
|
import * as fs from "node:fs";
|
||||||
|
|
@ -44,7 +44,7 @@ describe("execution pipeline", () => {
|
||||||
agentPrompt: "do the thing",
|
agentPrompt: "do the thing",
|
||||||
repoRoot: testRoot,
|
repoRoot: testRoot,
|
||||||
buildCmd: [process.execPath, "-e", "process.exit(0)"],
|
buildCmd: [process.execPath, "-e", "process.exit(0)"],
|
||||||
testCmd: [process.execPath, "-e", "process.exit(0)"],
|
testCmd: [process.execPath, "-e", "console.log('1 passed'); process.exit(0)"],
|
||||||
runAgent: makeMockAgent("done"),
|
runAgent: makeMockAgent("done"),
|
||||||
interruptOpts: { defaultValue: "allow" },
|
interruptOpts: { defaultValue: "allow" },
|
||||||
};
|
};
|
||||||
|
|
@ -83,7 +83,7 @@ describe("execution pipeline", () => {
|
||||||
agentPrompt: "do the thing",
|
agentPrompt: "do the thing",
|
||||||
repoRoot: testRoot,
|
repoRoot: testRoot,
|
||||||
buildCmd: [process.execPath, "-e", "process.exit(0)"],
|
buildCmd: [process.execPath, "-e", "process.exit(0)"],
|
||||||
testCmd: [process.execPath, "-e", "process.exit(0)"],
|
testCmd: [process.execPath, "-e", "console.log('1 passed'); process.exit(0)"],
|
||||||
runAgent: makeMockAgent("done"),
|
runAgent: makeMockAgent("done"),
|
||||||
interruptOpts: { defaultValue: "deny" },
|
interruptOpts: { defaultValue: "deny" },
|
||||||
};
|
};
|
||||||
|
|
@ -108,7 +108,7 @@ describe("execution pipeline", () => {
|
||||||
agentPrompt: "original prompt",
|
agentPrompt: "original prompt",
|
||||||
repoRoot: testRoot,
|
repoRoot: testRoot,
|
||||||
buildCmd: [process.execPath, "-e", "process.exit(0)"],
|
buildCmd: [process.execPath, "-e", "process.exit(0)"],
|
||||||
testCmd: [process.execPath, "-e", "process.exit(0)"],
|
testCmd: [process.execPath, "-e", "console.log('1 passed'); process.exit(0)"],
|
||||||
runAgent: capturingAgent,
|
runAgent: capturingAgent,
|
||||||
interruptOpts: { defaultValue: "allow" },
|
interruptOpts: { defaultValue: "allow" },
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
import { WorktreeSpec, WorktreeManager } from "./worktree-isolation.js";
|
import { WorktreeManager } from "./worktree-isolation.js";
|
||||||
import { VerifyGateSpec, VerifyGateResult, runVerifyGate } from "./verify-gate.js";
|
import { VerifyGateSpec, VerifyGateResult, runVerifyGate } from "./verify-gate.js";
|
||||||
import { humanInterruptGate, InterruptGateOptions } from "./interrupt-gate.js";
|
import { humanInterruptGate, InterruptGateOptions } from "./interrupt-gate.js";
|
||||||
import { FlatLedger, LedgerEntry } from "./flat-ledger.js";
|
import { FlatLedger, LedgerEntry } from "./flat-ledger.js";
|
||||||
|
|
@ -26,16 +26,6 @@ export interface PipelineResult {
|
||||||
durationMs: number;
|
durationMs: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
function worktreeExists(wm: WorktreeManager, name: string): boolean {
|
|
||||||
try {
|
|
||||||
const path = (wm as any).baseDir;
|
|
||||||
if (!path) return false;
|
|
||||||
return require("node:fs").existsSync(require("node:path").join(path, name));
|
|
||||||
} catch {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
export async function runPipeline(spec: PipelineSpec): Promise<PipelineResult> {
|
export async function runPipeline(spec: PipelineSpec): Promise<PipelineResult> {
|
||||||
const start = Date.now();
|
const start = Date.now();
|
||||||
const session = FlatLedger.sessionNameFromTask(spec.taskName);
|
const session = FlatLedger.sessionNameFromTask(spec.taskName);
|
||||||
|
|
@ -45,11 +35,22 @@ export async function runPipeline(spec: PipelineSpec): Promise<PipelineResult> {
|
||||||
|
|
||||||
// 1. Create worktree
|
// 1. Create worktree
|
||||||
wm.initDir();
|
wm.initDir();
|
||||||
const worktreePath = wm.create({
|
let worktreePath: string;
|
||||||
name: worktreeName,
|
try {
|
||||||
branch: spec.worktreeBranch ?? `pipeline/${session}`,
|
worktreePath = wm.create({
|
||||||
targetDir: spec.repoRoot,
|
name: worktreeName,
|
||||||
});
|
branch: spec.worktreeBranch ?? `pipeline/${session}`,
|
||||||
|
targetDir: spec.repoRoot,
|
||||||
|
});
|
||||||
|
} catch (e: any) {
|
||||||
|
// Worktree or branch already exists from a prior run — prune and retry once
|
||||||
|
wm.prune();
|
||||||
|
worktreePath = wm.create({
|
||||||
|
name: worktreeName,
|
||||||
|
branch: `${spec.worktreeBranch ?? `pipeline/${session}`}-${Date.now()}`,
|
||||||
|
targetDir: spec.repoRoot,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// 2. Inject repo map into prompt
|
// 2. Inject repo map into prompt
|
||||||
const repoMap = buildRepoMap(spec.repoRoot, 3);
|
const repoMap = buildRepoMap(spec.repoRoot, 3);
|
||||||
|
|
|
||||||
|
|
@ -44,6 +44,7 @@ function parseTestOutput(output: string): { passed: number; failed: number; tota
|
||||||
|
|
||||||
export function runVerifyGate(spec: VerifyGateSpec): VerifyGateResult {
|
export function runVerifyGate(spec: VerifyGateSpec): VerifyGateResult {
|
||||||
const start = Date.now();
|
const start = Date.now();
|
||||||
|
// ponytail: worktrees have a .git *file* (pointer), not a dir — fs.existsSync covers both
|
||||||
if (!fs.existsSync(path.join(spec.cwd, ".git"))) {
|
if (!fs.existsSync(path.join(spec.cwd, ".git"))) {
|
||||||
return {
|
return {
|
||||||
build: { pass: false, exitCode: -1, output: "" },
|
build: { pass: false, exitCode: -1, output: "" },
|
||||||
|
|
@ -69,7 +70,7 @@ export function runVerifyGate(spec: VerifyGateSpec): VerifyGateResult {
|
||||||
}
|
}
|
||||||
const testResult = run(spec.testCmd, spec.cwd);
|
const testResult = run(spec.testCmd, spec.cwd);
|
||||||
const counts = parseTestOutput(testResult.output);
|
const counts = parseTestOutput(testResult.output);
|
||||||
const testPass = testResult.exitCode === 0 && counts.failed === 0;
|
const testPass = testResult.exitCode === 0 && counts.failed === 0 && counts.total > 0;
|
||||||
const test = { pass: testPass, exitCode: testResult.exitCode, output: testResult.output, ...counts };
|
const test = { pass: testPass, exitCode: testResult.exitCode, output: testResult.output, ...counts };
|
||||||
return {
|
return {
|
||||||
build,
|
build,
|
||||||
|
|
|
||||||
|
|
@ -165,6 +165,6 @@ export class WorktreeManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
private safeBranch(value: string): string {
|
private safeBranch(value: string): string {
|
||||||
return this.safeName(value).replace(/\/+/, "-");
|
return this.safeName(value).replace(/\/+/g, "-");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue