Skip to content

Commit ded33ec

Browse files
feat(pool): squash merge + persist review to SQLite (#40)
Squash merge: release(name, true, taskId) now squash-merges agent commits into a single result commit on main. Full agent history preserved at refs/tasks/{taskId}. Without taskId, falls back to legacy ff/merge for backward compat. Uses temp branch + update-ref pattern since worktrees can't checkout main directly. Review persistence: task.review (ReviewResult) round-trips through SQLite via new review TEXT column with JSON serialization. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0b8b027 commit ded33ec

6 files changed

Lines changed: 244 additions & 32 deletions

File tree

src/__tests__/store.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,50 @@ describe("Store", () => {
579579
});
580580
});
581581

582+
// ── review persistence ─────────────────────────────────────────────────────
583+
584+
describe("review persistence", () => {
585+
it("save() and get() round-trip review field", () => {
586+
const { store, cleanup } = makeTempStore();
587+
try {
588+
const review = { approve: true, score: 85, issues: [], suggestions: ["clean"] };
589+
const task = makeTask({ id: "rev-1", review });
590+
store.save(task);
591+
const got = store.get("rev-1");
592+
assert.ok(got !== null);
593+
assert.deepStrictEqual(got.review, review);
594+
} finally {
595+
cleanup();
596+
}
597+
});
598+
599+
it("task without review returns undefined after load", () => {
600+
const { store, cleanup } = makeTempStore();
601+
try {
602+
const task = makeTask({ id: "rev-2" });
603+
store.save(task);
604+
const got = store.get("rev-2");
605+
assert.ok(got !== null);
606+
assert.strictEqual(got.review, undefined);
607+
} finally {
608+
cleanup();
609+
}
610+
});
611+
612+
it("update() can set review field", () => {
613+
const { store, cleanup } = makeTempStore();
614+
try {
615+
store.save(makeTask({ id: "rev-3" }));
616+
const review = { approve: false, score: 40, issues: ["bug"], suggestions: [] };
617+
const updated = store.update("rev-3", { review });
618+
assert.ok(updated !== null);
619+
assert.deepStrictEqual(updated.review, review);
620+
} finally {
621+
cleanup();
622+
}
623+
});
624+
});
625+
582626
// ── close ───────────────────────────────────────────────────────────────────
583627

584628
describe("close", () => {

src/__tests__/worktree-pool.test.ts

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,101 @@ describe("WorktreePool lookup", () => {
348348
});
349349
});
350350

351+
// ---------------------------------------------------------------------------
352+
// Squash merge
353+
// ---------------------------------------------------------------------------
354+
355+
describe("WorktreePool squash merge", () => {
356+
it("release(name, true, taskId) squash-merges to single commit", async () => {
357+
const { repoPath, cleanup } = await makeTempRepo();
358+
try {
359+
const pool = new WorktreePool(repoPath, 1);
360+
await pool.init();
361+
362+
const worker = await pool.acquire();
363+
assert.ok(worker !== null, "should acquire a worker");
364+
365+
const git = (...args: string[]) => execFileAsync("git", args, { cwd: worker.path });
366+
367+
// Count commits on main before
368+
const { stdout: mainLogBefore } = await execFileAsync("git", ["log", "--oneline", "main"], { cwd: repoPath });
369+
const mainCommitsBefore = mainLogBefore.trim().split("\n").length;
370+
371+
// Make 3 commits in the worktree (simulating agent work)
372+
fs.writeFileSync(path.join(worker.path, "file1.txt"), "hello\n");
373+
await git("add", "file1.txt");
374+
await git("commit", "-m", "agent commit 1");
375+
376+
fs.writeFileSync(path.join(worker.path, "file2.txt"), "world\n");
377+
await git("add", "file2.txt");
378+
await git("commit", "-m", "agent commit 2");
379+
380+
fs.writeFileSync(path.join(worker.path, "file3.txt"), "!\n");
381+
await git("add", "file3.txt");
382+
await git("commit", "-m", "agent commit 3");
383+
384+
// Release with squash merge
385+
const result = await pool.release(worker.name, true, "test123");
386+
assert.strictEqual(result.merged, true, "merge should succeed");
387+
388+
// Assert: main has exactly 1 new commit (squashed)
389+
const { stdout: mainLogAfter } = await execFileAsync("git", ["log", "--oneline", "main"], { cwd: repoPath });
390+
const mainCommitsAfter = mainLogAfter.trim().split("\n").length;
391+
assert.strictEqual(mainCommitsAfter, mainCommitsBefore + 1, "main should have exactly 1 new commit");
392+
393+
// Assert: the squash commit message contains the taskId
394+
const { stdout: lastCommit } = await execFileAsync("git", ["log", "-1", "--format=%s", "main"], { cwd: repoPath });
395+
assert.ok(lastCommit.includes("task(test123)"), `commit msg should contain task(test123), got: ${lastCommit.trim()}`);
396+
397+
// Assert: refs/tasks/test123 exists
398+
const { stdout: refOut } = await execFileAsync("git", ["show-ref", "refs/tasks/test123"], { cwd: repoPath });
399+
assert.ok(refOut.trim().length > 0, "refs/tasks/test123 should exist");
400+
401+
// Assert: the ref preserves all 3 original commits
402+
const { stdout: refLog } = await execFileAsync("git", ["log", "--oneline", "refs/tasks/test123"], { cwd: repoPath });
403+
const refCommits = refLog.trim().split("\n");
404+
// 3 agent commits + 1 initial commit = 4 total
405+
assert.strictEqual(refCommits.length, 4, "ref should show all 3 agent commits + initial");
406+
} finally {
407+
cleanup();
408+
}
409+
});
410+
411+
it("release(name, true) without taskId falls back to legacy merge", async () => {
412+
const { repoPath, cleanup } = await makeTempRepo();
413+
try {
414+
const pool = new WorktreePool(repoPath, 1);
415+
await pool.init();
416+
417+
const worker = await pool.acquire();
418+
assert.ok(worker !== null);
419+
420+
const git = (...args: string[]) => execFileAsync("git", args, { cwd: worker.path });
421+
422+
// Make 2 commits
423+
fs.writeFileSync(path.join(worker.path, "a.txt"), "a\n");
424+
await git("add", "a.txt");
425+
await git("commit", "-m", "commit a");
426+
427+
fs.writeFileSync(path.join(worker.path, "b.txt"), "b\n");
428+
await git("add", "b.txt");
429+
await git("commit", "-m", "commit b");
430+
431+
// Release WITHOUT taskId — should use legacy ff/merge
432+
const result = await pool.release(worker.name, true);
433+
assert.strictEqual(result.merged, true, "merge should succeed");
434+
435+
// Main should have all individual commits (not squashed)
436+
const { stdout: mainLog } = await execFileAsync("git", ["log", "--oneline", "main"], { cwd: repoPath });
437+
const commits = mainLog.trim().split("\n");
438+
// initial + 2 agent commits = 3 (ff merge preserves all)
439+
assert.ok(commits.length >= 3, `expected >= 3 commits on main, got ${commits.length}`);
440+
} finally {
441+
cleanup();
442+
}
443+
});
444+
});
445+
351446
// ---------------------------------------------------------------------------
352447
// Worker stats
353448
// ---------------------------------------------------------------------------

src/scheduler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ export class Scheduler {
472472
await this.runner.run(task, workerPath, this.onEvent);
473473

474474
const shouldMerge = task.status === "success";
475-
const mergeResult = await this.pool.release(workerName, shouldMerge);
475+
const mergeResult = await this.pool.release(workerName, shouldMerge, task.id);
476476

477477
if (shouldMerge && !mergeResult.merged) {
478478
const fileList = mergeResult.conflictFiles?.length

src/store.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ export class Store {
8282
} catch {
8383
// Column already exists — safe to ignore
8484
}
85+
// Add review column to persist cross-agent review results
86+
try {
87+
this.db.exec("ALTER TABLE tasks ADD COLUMN review TEXT");
88+
} catch {
89+
// Column already exists — safe to ignore
90+
}
8591
// Indexes for common query patterns
8692
this.db.exec(
8793
"CREATE INDEX IF NOT EXISTS idx_tasks_status ON tasks(status)"
@@ -115,6 +121,7 @@ export class Store {
115121
JSON.stringify(task.tags ?? []),
116122
task.dependsOn ?? null, task.webhookUrl ?? null, task.summary ?? null,
117123
task.agent ?? "claude",
124+
JSON.stringify(task.review ?? null),
118125
];
119126
}
120127

@@ -126,8 +133,8 @@ export class Store {
126133
(id, prompt, status, worktree, output, error, events, created_at,
127134
started_at, completed_at, timeout, max_budget, cost_usd,
128135
token_input, token_output, duration_ms, retry_count, max_retries, priority, tags,
129-
depends_on, webhook_url, summary, agent)
130-
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
136+
depends_on, webhook_url, summary, agent, review)
137+
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
131138
`).run(...params);
132139
if (insertResult.changes === 0) {
133140
// Row already exists — update it (params[0] is id, rest are fields; append id at end for WHERE)
@@ -136,7 +143,7 @@ export class Store {
136143
prompt=?, status=?, worktree=?, output=?, error=?, events=?, created_at=?,
137144
started_at=?, completed_at=?, timeout=?, max_budget=?, cost_usd=?,
138145
token_input=?, token_output=?, duration_ms=?, retry_count=?, max_retries=?,
139-
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?
146+
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?, review=?
140147
WHERE id=?
141148
`).run(...params.slice(1), task.id);
142149
}
@@ -153,15 +160,15 @@ export class Store {
153160
(id, prompt, status, worktree, output, error, events, created_at,
154161
started_at, completed_at, timeout, max_budget, cost_usd,
155162
token_input, token_output, duration_ms, retry_count, max_retries, priority, tags,
156-
depends_on, webhook_url, summary, agent)
157-
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
163+
depends_on, webhook_url, summary, agent, review)
164+
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
158165
`);
159166
const updateStmt = this.db.prepare(`
160167
UPDATE tasks SET
161168
prompt=?, status=?, worktree=?, output=?, error=?, events=?, created_at=?,
162169
started_at=?, completed_at=?, timeout=?, max_budget=?, cost_usd=?,
163170
token_input=?, token_output=?, duration_ms=?, retry_count=?, max_retries=?,
164-
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?
171+
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?, review=?
165172
WHERE id=?
166173
`);
167174
const runAll = this.db.transaction((batch: Task[]) => {
@@ -191,15 +198,15 @@ export class Store {
191198
(id, prompt, status, worktree, output, error, events, created_at,
192199
started_at, completed_at, timeout, max_budget, cost_usd,
193200
token_input, token_output, duration_ms, retry_count, max_retries, priority, tags,
194-
depends_on, webhook_url, summary, agent)
195-
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
201+
depends_on, webhook_url, summary, agent, review)
202+
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
196203
`);
197204
const updateStmt = this.db.prepare(`
198205
UPDATE tasks SET
199206
prompt=?, status=?, worktree=?, output=?, error=?, events=?, created_at=?,
200207
started_at=?, completed_at=?, timeout=?, max_budget=?, cost_usd=?,
201208
token_input=?, token_output=?, duration_ms=?, retry_count=?, max_retries=?,
202-
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?
209+
priority=?, tags=?, depends_on=?, webhook_url=?, summary=?, agent=?, review=?
203210
WHERE id=?
204211
`);
205212
this.transaction(() => {
@@ -243,6 +250,7 @@ export class Store {
243250
webhookUrl: { col: "webhook_url" },
244251
summary: { col: "summary" },
245252
agent: { col: "agent" },
253+
review: { col: "review", serialize: (v) => JSON.stringify(v) },
246254
};
247255

248256
const setClauses: string[] = [];
@@ -400,6 +408,8 @@ export class Store {
400408
webhookUrl: row.webhook_url ?? undefined,
401409
summary: row.summary ?? undefined,
402410
agent: row.agent ?? "claude",
411+
// ?? undefined converts null (from JSON.parse("null")) back to undefined
412+
review: this.safeJsonParse(row.review, undefined) ?? undefined,
403413
};
404414
}
405415

src/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@ export type TaskPriority = "urgent" | "high" | "normal" | "low";
33
/** Built-in agent types with known output parsing. Any string is accepted for generic CLI agents. */
44
export type AgentType = "claude" | "claude-sdk" | "codex";
55

6+
export interface ReviewResult {
7+
approve: boolean;
8+
score: number;
9+
issues: string[];
10+
suggestions: string[];
11+
reviewAgent?: string;
12+
}
13+
614
export interface Task {
715
id: string;
816
prompt: string;
@@ -28,6 +36,7 @@ export interface Task {
2836
webhookUrl?: string;
2937
summary?: string;
3038
agent?: string;
39+
review?: ReviewResult;
3140
}
3241

3342
export interface TaskEvent {

0 commit comments

Comments
 (0)