Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/__tests__/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,50 @@ describe("Store", () => {
});
});

// ── review persistence ─────────────────────────────────────────────────────

describe("review persistence", () => {
it("save() and get() round-trip review field", () => {
const { store, cleanup } = makeTempStore();
try {
const review = { approve: true, score: 85, issues: [], suggestions: ["clean"] };
const task = makeTask({ id: "rev-1", review });
store.save(task);
const got = store.get("rev-1");
assert.ok(got !== null);
assert.deepStrictEqual(got.review, review);
} finally {
cleanup();
}
});

it("task without review returns undefined after load", () => {
const { store, cleanup } = makeTempStore();
try {
const task = makeTask({ id: "rev-2" });
store.save(task);
const got = store.get("rev-2");
assert.ok(got !== null);
assert.strictEqual(got.review, undefined);
} finally {
cleanup();
}
});

it("update() can set review field", () => {
const { store, cleanup } = makeTempStore();
try {
store.save(makeTask({ id: "rev-3" }));
const review = { approve: false, score: 40, issues: ["bug"], suggestions: [] };
const updated = store.update("rev-3", { review });
assert.ok(updated !== null);
assert.deepStrictEqual(updated.review, review);
} finally {
cleanup();
}
});
});

// ── close ───────────────────────────────────────────────────────────────────

describe("close", () => {
Expand Down
95 changes: 95 additions & 0 deletions src/__tests__/worktree-pool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,101 @@ describe("WorktreePool lookup", () => {
});
});

// ---------------------------------------------------------------------------
// Squash merge
// ---------------------------------------------------------------------------

describe("WorktreePool squash merge", () => {
it("release(name, true, taskId) squash-merges to single commit", async () => {
const { repoPath, cleanup } = await makeTempRepo();
try {
const pool = new WorktreePool(repoPath, 1);
await pool.init();

const worker = await pool.acquire();
assert.ok(worker !== null, "should acquire a worker");

const git = (...args: string[]) => execFileAsync("git", args, { cwd: worker.path });

// Count commits on main before
const { stdout: mainLogBefore } = await execFileAsync("git", ["log", "--oneline", "main"], { cwd: repoPath });
const mainCommitsBefore = mainLogBefore.trim().split("\n").length;

// Make 3 commits in the worktree (simulating agent work)
fs.writeFileSync(path.join(worker.path, "file1.txt"), "hello\n");
await git("add", "file1.txt");
await git("commit", "-m", "agent commit 1");

fs.writeFileSync(path.join(worker.path, "file2.txt"), "world\n");
await git("add", "file2.txt");
await git("commit", "-m", "agent commit 2");

fs.writeFileSync(path.join(worker.path, "file3.txt"), "!\n");
await git("add", "file3.txt");
await git("commit", "-m", "agent commit 3");

// Release with squash merge
const result = await pool.release(worker.name, true, "test123");
assert.strictEqual(result.merged, true, "merge should succeed");

// Assert: main has exactly 1 new commit (squashed)
const { stdout: mainLogAfter } = await execFileAsync("git", ["log", "--oneline", "main"], { cwd: repoPath });
const mainCommitsAfter = mainLogAfter.trim().split("\n").length;
assert.strictEqual(mainCommitsAfter, mainCommitsBefore + 1, "main should have exactly 1 new commit");

// Assert: the squash commit message contains the taskId
const { stdout: lastCommit } = await execFileAsync("git", ["log", "-1", "--format=%s", "main"], { cwd: repoPath });
assert.ok(lastCommit.includes("task(test123)"), `commit msg should contain task(test123), got: ${lastCommit.trim()}`);

// Assert: refs/tasks/test123 exists
const { stdout: refOut } = await execFileAsync("git", ["show-ref", "refs/tasks/test123"], { cwd: repoPath });
assert.ok(refOut.trim().length > 0, "refs/tasks/test123 should exist");

// Assert: the ref preserves all 3 original commits
const { stdout: refLog } = await execFileAsync("git", ["log", "--oneline", "refs/tasks/test123"], { cwd: repoPath });
const refCommits = refLog.trim().split("\n");
// 3 agent commits + 1 initial commit = 4 total
assert.strictEqual(refCommits.length, 4, "ref should show all 3 agent commits + initial");
} finally {
cleanup();
}
});

it("release(name, true) without taskId falls back to legacy merge", async () => {
const { repoPath, cleanup } = await makeTempRepo();
try {
const pool = new WorktreePool(repoPath, 1);
await pool.init();

const worker = await pool.acquire();
assert.ok(worker !== null);

const git = (...args: string[]) => execFileAsync("git", args, { cwd: worker.path });

// Make 2 commits
fs.writeFileSync(path.join(worker.path, "a.txt"), "a\n");
await git("add", "a.txt");
await git("commit", "-m", "commit a");

fs.writeFileSync(path.join(worker.path, "b.txt"), "b\n");
await git("add", "b.txt");
await git("commit", "-m", "commit b");

// Release WITHOUT taskId — should use legacy ff/merge
const result = await pool.release(worker.name, true);
assert.strictEqual(result.merged, true, "merge should succeed");

// Main should have all individual commits (not squashed)
const { stdout: mainLog } = await execFileAsync("git", ["log", "--oneline", "main"], { cwd: repoPath });
const commits = mainLog.trim().split("\n");
// initial + 2 agent commits = 3 (ff merge preserves all)
assert.ok(commits.length >= 3, `expected >= 3 commits on main, got ${commits.length}`);
} finally {
cleanup();
}
});
});

// ---------------------------------------------------------------------------
// Worker stats
// ---------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ export class Scheduler {
await this.runner.run(task, workerPath, this.onEvent);

const shouldMerge = task.status === "success";
const mergeResult = await this.pool.release(workerName, shouldMerge);
const mergeResult = await this.pool.release(workerName, shouldMerge, task.id);

if (shouldMerge && !mergeResult.merged) {
const fileList = mergeResult.conflictFiles?.length
Expand Down
28 changes: 19 additions & 9 deletions src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ export class Store {
} catch {
// Column already exists — safe to ignore
}
// Add review column to persist cross-agent review results
try {
this.db.exec("ALTER TABLE tasks ADD COLUMN review TEXT");
} catch {
// Column already exists — safe to ignore
}
// Indexes for common query patterns
this.db.exec(
"CREATE INDEX IF NOT EXISTS idx_tasks_status ON tasks(status)"
Expand Down Expand Up @@ -115,6 +121,7 @@ export class Store {
JSON.stringify(task.tags ?? []),
task.dependsOn ?? null, task.webhookUrl ?? null, task.summary ?? null,
task.agent ?? "claude",
JSON.stringify(task.review ?? null),
];
}

Expand All @@ -126,8 +133,8 @@ export class Store {
(id, prompt, status, worktree, output, error, events, created_at,
started_at, completed_at, timeout, max_budget, cost_usd,
token_input, token_output, duration_ms, retry_count, max_retries, priority, tags,
depends_on, webhook_url, summary, agent)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
depends_on, webhook_url, summary, agent, review)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`).run(...params);
if (insertResult.changes === 0) {
// Row already exists — update it (params[0] is id, rest are fields; append id at end for WHERE)
Expand All @@ -136,7 +143,7 @@ export class Store {
prompt=?, status=?, worktree=?, output=?, error=?, events=?, created_at=?,
started_at=?, completed_at=?, timeout=?, max_budget=?, cost_usd=?,
token_input=?, token_output=?, duration_ms=?, retry_count=?, max_retries=?,
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?, review=?
WHERE id=?
`).run(...params.slice(1), task.id);
}
Expand All @@ -153,15 +160,15 @@ export class Store {
(id, prompt, status, worktree, output, error, events, created_at,
started_at, completed_at, timeout, max_budget, cost_usd,
token_input, token_output, duration_ms, retry_count, max_retries, priority, tags,
depends_on, webhook_url, summary, agent)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
depends_on, webhook_url, summary, agent, review)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`);
const updateStmt = this.db.prepare(`
UPDATE tasks SET
prompt=?, status=?, worktree=?, output=?, error=?, events=?, created_at=?,
started_at=?, completed_at=?, timeout=?, max_budget=?, cost_usd=?,
token_input=?, token_output=?, duration_ms=?, retry_count=?, max_retries=?,
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?, review=?
WHERE id=?
`);
const runAll = this.db.transaction((batch: Task[]) => {
Expand Down Expand Up @@ -191,15 +198,15 @@ export class Store {
(id, prompt, status, worktree, output, error, events, created_at,
started_at, completed_at, timeout, max_budget, cost_usd,
token_input, token_output, duration_ms, retry_count, max_retries, priority, tags,
depends_on, webhook_url, summary, agent)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
depends_on, webhook_url, summary, agent, review)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`);
const updateStmt = this.db.prepare(`
UPDATE tasks SET
prompt=?, status=?, worktree=?, output=?, error=?, events=?, created_at=?,
started_at=?, completed_at=?, timeout=?, max_budget=?, cost_usd=?,
token_input=?, token_output=?, duration_ms=?, retry_count=?, max_retries=?,
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?, review=?
WHERE id=?
`);
this.transaction(() => {
Expand Down Expand Up @@ -243,6 +250,7 @@ export class Store {
webhookUrl: { col: "webhook_url" },
summary: { col: "summary" },
agent: { col: "agent" },
review: { col: "review", serialize: (v) => JSON.stringify(v) },
};

const setClauses: string[] = [];
Expand Down Expand Up @@ -400,6 +408,8 @@ export class Store {
webhookUrl: row.webhook_url ?? undefined,
summary: row.summary ?? undefined,
agent: row.agent ?? "claude",
// ?? undefined converts null (from JSON.parse("null")) back to undefined
review: this.safeJsonParse(row.review, undefined) ?? undefined,
};
}

Expand Down
9 changes: 9 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ export type TaskPriority = "urgent" | "high" | "normal" | "low";
/** Built-in agent types with known output parsing. Any string is accepted for generic CLI agents. */
export type AgentType = "claude" | "claude-sdk" | "codex";

export interface ReviewResult {
approve: boolean;
score: number;
issues: string[];
suggestions: string[];
reviewAgent?: string;
}

export interface Task {
id: string;
prompt: string;
Expand All @@ -28,6 +36,7 @@ export interface Task {
webhookUrl?: string;
summary?: string;
agent?: string;
review?: ReviewResult;
}

export interface TaskEvent {
Expand Down
Loading