fix(FN-834): fix branch prefix drift, add merger branch guard, and fix test OOM
- Fix resolveBaseBranch to use stored branch name and consistent fusion/ prefix for both explicit deps and blockedBy paths (was using kb/ for blockedBy) - Add main branch checkout verification in merger before squash merge to prevent feature code from landing on wrong branch lineage - Align all branch prefix references from stale kb/ to fusion/ across executor, merger, store, and routes - Fix executor test OOM by mocking merger fully, adding fake timers to retry tests, and switching vitest pool to vmThreads - Update all test assertions to use fusion/ branch prefix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -10,7 +10,7 @@
|
||||
* - onTerminated: Called when an unresponsive agent is terminated
|
||||
*/
|
||||
|
||||
import type { AgentStore, Agent, AgentState } from "@fusion/core";
|
||||
import type { AgentStore, AgentHeartbeatRun, HeartbeatInvocationSource } from "@fusion/core";
|
||||
|
||||
/** Options for HeartbeatMonitor constructor */
|
||||
export interface HeartbeatMonitorOptions {
|
||||
@@ -20,12 +20,28 @@ export interface HeartbeatMonitorOptions {
|
||||
pollIntervalMs?: number;
|
||||
/** Heartbeat timeout in milliseconds (default: 60000) */
|
||||
heartbeatTimeoutMs?: number;
|
||||
/** Max concurrent runs per agent (default: 1) */
|
||||
maxConcurrentRuns?: number;
|
||||
/** Callback when an agent misses its heartbeat */
|
||||
onMissed?: (agentId: string) => void;
|
||||
/** Callback when an agent recovers after a missed heartbeat */
|
||||
onRecovered?: (agentId: string) => void;
|
||||
/** Callback when an unresponsive agent is terminated */
|
||||
onTerminated?: (agentId: string) => void;
|
||||
/** Callback when a run starts */
|
||||
onRunStarted?: (agentId: string, run: AgentHeartbeatRun) => void;
|
||||
/** Callback when a run completes */
|
||||
onRunCompleted?: (agentId: string, run: AgentHeartbeatRun) => void;
|
||||
}
|
||||
|
||||
/** Options for waking up an agent */
|
||||
export interface WakeupOptions {
|
||||
/** What triggered the wakeup */
|
||||
source: HeartbeatInvocationSource;
|
||||
/** Detail about the trigger (manual, ping, scheduler, system) */
|
||||
triggerDetail?: string;
|
||||
/** Context snapshot for the run */
|
||||
contextSnapshot?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
/** Session interface for disposing agent resources */
|
||||
@@ -41,6 +57,8 @@ interface TrackedAgent {
|
||||
runId: string;
|
||||
lastSeen: number; // timestamp from Date.now()
|
||||
missedHeartbeatReported: boolean;
|
||||
/** Session ID before this execution started */
|
||||
sessionIdBefore?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -51,11 +69,15 @@ export class HeartbeatMonitor {
|
||||
private store: AgentStore;
|
||||
private pollIntervalMs: number;
|
||||
private heartbeatTimeoutMs: number;
|
||||
private maxConcurrentRuns: number;
|
||||
private onMissed?: (agentId: string) => void;
|
||||
private onRecovered?: (agentId: string) => void;
|
||||
private onTerminated?: (agentId: string) => void;
|
||||
private onRunStarted?: (agentId: string, run: AgentHeartbeatRun) => void;
|
||||
private onRunCompleted?: (agentId: string, run: AgentHeartbeatRun) => void;
|
||||
|
||||
private trackedAgents: Map<string, TrackedAgent> = new Map();
|
||||
private agentStartLocks: Map<string, Promise<unknown>> = new Map();
|
||||
private pollInterval: NodeJS.Timeout | null = null;
|
||||
private isRunning = false;
|
||||
|
||||
@@ -63,9 +85,12 @@ export class HeartbeatMonitor {
|
||||
this.store = options.store;
|
||||
this.pollIntervalMs = options.pollIntervalMs ?? 30000;
|
||||
this.heartbeatTimeoutMs = options.heartbeatTimeoutMs ?? 60000;
|
||||
this.maxConcurrentRuns = options.maxConcurrentRuns ?? 1;
|
||||
this.onMissed = options.onMissed;
|
||||
this.onRecovered = options.onRecovered;
|
||||
this.onTerminated = options.onTerminated;
|
||||
this.onRunStarted = options.onRunStarted;
|
||||
this.onRunCompleted = options.onRunCompleted;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -103,18 +128,20 @@ export class HeartbeatMonitor {
|
||||
}
|
||||
|
||||
/**
|
||||
* Register an agent for monitoring.
|
||||
* Register an agent for monitoring with optional session context.
|
||||
* @param agentId - The agent ID
|
||||
* @param session - Session with dispose() for cleanup
|
||||
* @param runId - The heartbeat run ID
|
||||
* @param sessionIdBefore - Optional session ID from before execution
|
||||
*/
|
||||
trackAgent(agentId: string, session: AgentSession, runId: string): void {
|
||||
trackAgent(agentId: string, session: AgentSession, runId: string, sessionIdBefore?: string): void {
|
||||
const tracked: TrackedAgent = {
|
||||
agentId,
|
||||
session,
|
||||
runId,
|
||||
lastSeen: Date.now(),
|
||||
missedHeartbeatReported: false,
|
||||
sessionIdBefore,
|
||||
};
|
||||
|
||||
this.trackedAgents.set(agentId, tracked);
|
||||
@@ -123,6 +150,126 @@ export class HeartbeatMonitor {
|
||||
void this.store.recordHeartbeat(agentId, "ok", runId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize run starts per agent to prevent concurrent execution.
|
||||
* @param agentId - The agent ID
|
||||
* @param fn - Function to execute with the lock
|
||||
*/
|
||||
async withAgentStartLock<T>(agentId: string, fn: () => Promise<T>): Promise<T> {
|
||||
const existing = this.agentStartLocks.get(agentId) ?? Promise.resolve();
|
||||
const operation = existing.then(fn, fn);
|
||||
this.agentStartLocks.set(agentId, operation);
|
||||
return operation as Promise<T>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Start a rich heartbeat run with full context capture.
|
||||
* Creates a structured run record and saves it to the run store.
|
||||
* @param agentId - The agent ID
|
||||
* @param options - Wakeup options with trigger context
|
||||
* @returns The created run
|
||||
*/
|
||||
async startRun(agentId: string, options?: WakeupOptions): Promise<AgentHeartbeatRun> {
|
||||
const run = await this.store.startHeartbeatRun(agentId);
|
||||
|
||||
// Enrich with execution context
|
||||
const enrichedRun: AgentHeartbeatRun = {
|
||||
...run,
|
||||
invocationSource: options?.source ?? "on_demand",
|
||||
triggerDetail: options?.triggerDetail ?? "manual",
|
||||
contextSnapshot: options?.contextSnapshot,
|
||||
processPid: process.pid,
|
||||
};
|
||||
|
||||
// Save rich run data
|
||||
await this.store.saveRun(enrichedRun);
|
||||
|
||||
// Transition agent to running state
|
||||
try {
|
||||
await this.store.updateAgentState(agentId, "running");
|
||||
} catch {
|
||||
// May fail if already in running state - that's ok
|
||||
}
|
||||
|
||||
this.onRunStarted?.(agentId, enrichedRun);
|
||||
return enrichedRun;
|
||||
}
|
||||
|
||||
/**
|
||||
* Complete a heartbeat run with results.
|
||||
* @param agentId - The agent ID
|
||||
* @param runId - The run ID to complete
|
||||
* @param result - Execution results
|
||||
*/
|
||||
async completeRun(
|
||||
agentId: string,
|
||||
runId: string,
|
||||
result: {
|
||||
status: "completed" | "failed" | "terminated";
|
||||
exitCode?: number;
|
||||
sessionIdAfter?: string;
|
||||
usageJson?: { inputTokens: number; outputTokens: number; cachedTokens: number };
|
||||
resultJson?: Record<string, unknown>;
|
||||
stdoutExcerpt?: string;
|
||||
stderrExcerpt?: string;
|
||||
}
|
||||
): Promise<void> {
|
||||
// Load and update the run
|
||||
const run = await this.store.getRunDetail(agentId, runId);
|
||||
if (!run) return;
|
||||
|
||||
const tracked = this.trackedAgents.get(agentId);
|
||||
const completedRun: AgentHeartbeatRun = {
|
||||
...run,
|
||||
endedAt: new Date().toISOString(),
|
||||
status: result.status,
|
||||
exitCode: result.exitCode,
|
||||
sessionIdBefore: tracked?.sessionIdBefore,
|
||||
sessionIdAfter: result.sessionIdAfter,
|
||||
usageJson: result.usageJson,
|
||||
resultJson: result.resultJson,
|
||||
stdoutExcerpt: result.stdoutExcerpt,
|
||||
stderrExcerpt: result.stderrExcerpt,
|
||||
};
|
||||
|
||||
await this.store.saveRun(completedRun);
|
||||
|
||||
// Update cumulative usage on agent
|
||||
if (result.usageJson) {
|
||||
try {
|
||||
const agent = await this.store.getAgent(agentId);
|
||||
if (agent) {
|
||||
await this.store.updateAgent(agentId, {
|
||||
totalInputTokens: (agent.totalInputTokens ?? 0) + result.usageJson.inputTokens,
|
||||
totalOutputTokens: (agent.totalOutputTokens ?? 0) + result.usageJson.outputTokens,
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
// Non-critical, skip
|
||||
}
|
||||
}
|
||||
|
||||
// Transition agent state based on result
|
||||
try {
|
||||
if (result.status === "failed") {
|
||||
await this.store.updateAgentState(agentId, "error");
|
||||
await this.store.updateAgent(agentId, { lastError: result.stderrExcerpt ?? "Run failed" });
|
||||
} else if (result.status === "terminated") {
|
||||
await this.store.updateAgentState(agentId, "terminated");
|
||||
} else {
|
||||
// Completed successfully - back to active
|
||||
await this.store.updateAgentState(agentId, "active");
|
||||
}
|
||||
} catch {
|
||||
// State transition may fail if already in target state
|
||||
}
|
||||
|
||||
// End the heartbeat run tracking
|
||||
await this.store.endHeartbeatRun(runId, result.status === "completed" ? "completed" : "terminated");
|
||||
|
||||
this.onRunCompleted?.(agentId, completedRun);
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove an agent from monitoring.
|
||||
* Does NOT end the heartbeat run - caller's responsibility.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { AgentSemaphore } from "./concurrency.js";
|
||||
|
||||
// Mock external dependencies
|
||||
@@ -37,13 +37,10 @@ vi.mock("./logger.js", () => {
|
||||
hybridExecutorLog: createMockLogger(),
|
||||
};
|
||||
});
|
||||
vi.mock("./merger.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("./merger.js")>();
|
||||
return {
|
||||
...actual,
|
||||
findWorktreeUser: vi.fn().mockResolvedValue(null),
|
||||
};
|
||||
});
|
||||
vi.mock("./merger.js", () => ({
|
||||
aiMergeTask: vi.fn(),
|
||||
findWorktreeUser: vi.fn().mockResolvedValue(null),
|
||||
}));
|
||||
vi.mock("./worktree-names.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("./worktree-names.js")>("./worktree-names.js");
|
||||
return {
|
||||
@@ -62,6 +59,16 @@ vi.mock("node:fs", () => ({
|
||||
vi.mock("./rate-limit-retry.js", () => ({
|
||||
withRateLimitRetry: (fn: () => Promise<any>) => fn(),
|
||||
}));
|
||||
vi.mock("@mariozechner/pi-coding-agent", () => {
|
||||
const mockSessionManager = {};
|
||||
return {
|
||||
SessionManager: {
|
||||
create: vi.fn().mockReturnValue(mockSessionManager),
|
||||
open: vi.fn().mockReturnValue(mockSessionManager),
|
||||
inMemory: vi.fn().mockReturnValue(mockSessionManager),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import { TaskExecutor, buildExecutionPrompt } from "./executor.js";
|
||||
import { createKbAgent } from "./pi.js";
|
||||
@@ -71,8 +78,10 @@ import { findWorktreeUser, aiMergeTask } from "./merger.js";
|
||||
import { WorktreePool } from "./worktree-pool.js";
|
||||
import { generateWorktreeName, slugify } from "./worktree-names.js";
|
||||
import type { Column, Task, TaskDetail } from "@fusion/core";
|
||||
import { SessionManager } from "@mariozechner/pi-coding-agent";
|
||||
|
||||
const mockedCreateHaiAgent = vi.mocked(createKbAgent);
|
||||
const mockedSessionManager = vi.mocked(SessionManager);
|
||||
const mockedGenerateWorktreeName = vi.mocked(generateWorktreeName);
|
||||
const mockedFindWorktreeUser = vi.mocked(findWorktreeUser);
|
||||
|
||||
@@ -438,7 +447,7 @@ describe("TaskExecutor worktree naming", () => {
|
||||
// The worktree path stored should use the generated name, not the task ID
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-030", {
|
||||
worktree: "/tmp/test/.worktrees/swift-falcon",
|
||||
branch: "kb/fn-030",
|
||||
branch: "fusion/fn-030",
|
||||
});
|
||||
expect(mockedGenerateWorktreeName).toHaveBeenCalledWith("/tmp/test");
|
||||
});
|
||||
@@ -490,7 +499,7 @@ describe("TaskExecutor worktree naming", () => {
|
||||
// Should use task ID (lowercase) as worktree name
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-042", {
|
||||
worktree: "/tmp/test/.worktrees/fn-042",
|
||||
branch: "kb/fn-042",
|
||||
branch: "fusion/fn-042",
|
||||
});
|
||||
// Should NOT call generateWorktreeName when using task-id
|
||||
expect(mockedGenerateWorktreeName).not.toHaveBeenCalled();
|
||||
@@ -517,7 +526,7 @@ describe("TaskExecutor worktree naming", () => {
|
||||
const expectedSlug = slugify("Fix login bug with OAuth");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-043", {
|
||||
worktree: `/tmp/test/.worktrees/${expectedSlug}`,
|
||||
branch: "kb/fn-043",
|
||||
branch: "fusion/fn-043",
|
||||
});
|
||||
expect(mockedGenerateWorktreeName).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -545,7 +554,7 @@ describe("TaskExecutor worktree naming", () => {
|
||||
const expectedSlug = slugify(taskDescription.slice(0, 60));
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-044", {
|
||||
worktree: `/tmp/test/.worktrees/${expectedSlug}`,
|
||||
branch: "kb/fn-044",
|
||||
branch: "fusion/fn-044",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -566,7 +575,7 @@ describe("TaskExecutor worktree naming", () => {
|
||||
// Should use generateWorktreeName for random mode
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-045", {
|
||||
worktree: "/tmp/test/.worktrees/swift-falcon",
|
||||
branch: "kb/fn-045",
|
||||
branch: "fusion/fn-045",
|
||||
});
|
||||
expect(mockedGenerateWorktreeName).toHaveBeenCalledWith("/tmp/test");
|
||||
});
|
||||
@@ -588,7 +597,7 @@ describe("TaskExecutor worktree naming", () => {
|
||||
// Should default to random naming
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-046", {
|
||||
worktree: "/tmp/test/.worktrees/swift-falcon",
|
||||
branch: "kb/fn-046",
|
||||
branch: "fusion/fn-046",
|
||||
});
|
||||
expect(mockedGenerateWorktreeName).toHaveBeenCalledWith("/tmp/test");
|
||||
});
|
||||
@@ -618,7 +627,7 @@ describe("TaskExecutor worktree naming", () => {
|
||||
// Should acquire from pool, ignoring the task-id naming preference
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-047", {
|
||||
worktree: "/tmp/test/.worktrees/pooled-warm-wt",
|
||||
branch: "kb/fn-047",
|
||||
branch: "fusion/fn-047",
|
||||
});
|
||||
// Should NOT call generateWorktreeName when using pooled worktree
|
||||
expect(mockedGenerateWorktreeName).not.toHaveBeenCalled();
|
||||
@@ -646,6 +655,7 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
mockedGenerateWorktreeName.mockReturnValue("swift-falcon");
|
||||
@@ -657,6 +667,10 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
} as any);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("creates worktree successfully on first attempt", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
@@ -735,7 +749,10 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
const onError = vi.fn();
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { onError });
|
||||
|
||||
await executor.execute(makeTask());
|
||||
const executePromise = executor.execute(makeTask());
|
||||
// Advance past all retry delays (100 + 500 + 1000ms)
|
||||
await vi.advanceTimersByTimeAsync(2000);
|
||||
await executePromise;
|
||||
|
||||
// Should log final failure
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
@@ -764,19 +781,19 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
if (command.includes("-b")) {
|
||||
// First attempt: createWithBranch fails with branch already exists
|
||||
const error: any = new Error(
|
||||
"fatal: A branch named 'kb/fn-050' already exists.",
|
||||
"fatal: A branch named 'fusion/fn-050' already exists.",
|
||||
);
|
||||
error.stderr = Buffer.from(
|
||||
"fatal: A branch named 'kb/fn-050' already exists.",
|
||||
"fatal: A branch named 'fusion/fn-050' already exists.",
|
||||
);
|
||||
throw error;
|
||||
} else {
|
||||
// Fallback createFromExistingBranch fails with already used
|
||||
const error: any = new Error(
|
||||
"fatal: 'kb/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
"fatal: 'fusion/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
);
|
||||
error.stderr = Buffer.from(
|
||||
"fatal: 'kb/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
"fatal: 'fusion/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
);
|
||||
throw error;
|
||||
}
|
||||
@@ -803,18 +820,18 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
}
|
||||
if (command.includes("-b")) {
|
||||
const error: any = new Error(
|
||||
"fatal: A branch named 'kb/fn-050' already exists.",
|
||||
"fatal: A branch named 'fusion/fn-050' already exists.",
|
||||
);
|
||||
error.stderr = Buffer.from(
|
||||
"fatal: A branch named 'kb/fn-050' already exists.",
|
||||
"fatal: A branch named 'fusion/fn-050' already exists.",
|
||||
);
|
||||
throw error;
|
||||
} else {
|
||||
const error: any = new Error(
|
||||
"fatal: 'kb/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
"fatal: 'fusion/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
);
|
||||
error.stderr = Buffer.from(
|
||||
"fatal: 'kb/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
"fatal: 'fusion/fn-050' is already used by worktree at '/tmp/test/.worktrees/green-sage'",
|
||||
);
|
||||
throw error;
|
||||
}
|
||||
@@ -961,7 +978,7 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
expect.stringContaining("Pruned stale worktree metadata"),
|
||||
"kb/fn-050",
|
||||
"fusion/fn-050",
|
||||
);
|
||||
// Should also call branch -D after prune
|
||||
expect(mockedExecSync).toHaveBeenCalledWith(
|
||||
@@ -1063,7 +1080,9 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
|
||||
const onError = vi.fn();
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { onError });
|
||||
await executor.execute(makeTask());
|
||||
const executePromise = executor.execute(makeTask());
|
||||
await vi.advanceTimersByTimeAsync(2000);
|
||||
await executePromise;
|
||||
|
||||
// Should have logged terminal failure for the stale reference
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
@@ -1278,8 +1297,10 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
mockedFindWorktreeUser.mockResolvedValue(null);
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -1294,7 +1315,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
|
||||
await executor.execute(makeTask({
|
||||
id: "FN-060",
|
||||
baseBranch: "kb/fn-059",
|
||||
baseBranch: "fusion/fn-059",
|
||||
}));
|
||||
|
||||
// The git worktree add command should include the startPoint
|
||||
@@ -1302,7 +1323,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
(c) => typeof c[0] === "string" && (c[0] as string).includes("worktree add"),
|
||||
);
|
||||
expect(worktreeAddCalls.length).toBeGreaterThan(0);
|
||||
expect(worktreeAddCalls[0][0]).toContain("kb/fn-059");
|
||||
expect(worktreeAddCalls[0][0]).toContain("fusion/fn-059");
|
||||
});
|
||||
|
||||
it("creates worktree from HEAD when baseBranch is not set", async () => {
|
||||
@@ -1332,12 +1353,12 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
|
||||
await executor.execute(makeTask({
|
||||
id: "FN-062",
|
||||
baseBranch: "kb/fn-061",
|
||||
baseBranch: "fusion/fn-061",
|
||||
}));
|
||||
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-062",
|
||||
expect.stringContaining("based on kb/fn-061"),
|
||||
expect.stringContaining("based on fusion/fn-061"),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1367,10 +1388,10 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
if (typeof cmd === "string" && cmd.includes("git worktree add") && cmd.includes("-b") && firstAttempt) {
|
||||
firstAttempt = false;
|
||||
const err: any = new Error(
|
||||
`fatal: 'kb/fn-064' is already used by worktree at '${conflictingPath}'`,
|
||||
`fatal: 'fusion/fn-064' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
err.stderr = Buffer.from(
|
||||
`fatal: 'kb/fn-064' is already used by worktree at '${conflictingPath}'`,
|
||||
`fatal: 'fusion/fn-064' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
@@ -1384,7 +1405,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
expect.objectContaining({ cwd: "/tmp/test", stdio: "pipe" }),
|
||||
);
|
||||
expect(mockedExecSync).toHaveBeenCalledWith(
|
||||
'git branch -D "kb/fn-064"',
|
||||
'git branch -D "fusion/fn-064"',
|
||||
expect.objectContaining({ cwd: "/tmp/test", stdio: "pipe" }),
|
||||
);
|
||||
|
||||
@@ -1399,6 +1420,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
});
|
||||
|
||||
it("throws original error if cleanup also fails", async () => {
|
||||
vi.useFakeTimers();
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const conflictingPath = "/tmp/test/.worktrees/sharp-stone";
|
||||
@@ -1406,10 +1428,10 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
if (typeof cmd === "string" && cmd.includes("git worktree add") && cmd.includes("-b")) {
|
||||
const err: any = new Error(
|
||||
`fatal: 'kb/fn-065' is already used by worktree at '${conflictingPath}'`,
|
||||
`fatal: 'fusion/fn-065' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
err.stderr = Buffer.from(
|
||||
`fatal: 'kb/fn-065' is already used by worktree at '${conflictingPath}'`,
|
||||
`fatal: 'fusion/fn-065' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
@@ -1419,7 +1441,10 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
await executor.execute(makeTask({ id: "FN-065" }));
|
||||
const executePromise = executor.execute(makeTask({ id: "FN-065" }));
|
||||
await vi.advanceTimersByTimeAsync(2000);
|
||||
await executePromise;
|
||||
vi.useRealTimers();
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-065", {
|
||||
status: "failed",
|
||||
@@ -1434,7 +1459,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
(p) => p === "/tmp/test/.worktrees/idle-wt",
|
||||
);
|
||||
|
||||
const prepareSpy = vi.spyOn(pool, "prepareForTask").mockReturnValue("kb/fn-064");
|
||||
const prepareSpy = vi.spyOn(pool, "prepareForTask").mockReturnValue("fusion/fn-064");
|
||||
|
||||
const store = createMockStore();
|
||||
store.getSettings.mockResolvedValue({
|
||||
@@ -1450,13 +1475,13 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
|
||||
await executor.execute(makeTask({
|
||||
id: "FN-064",
|
||||
baseBranch: "kb/fn-063",
|
||||
baseBranch: "fusion/fn-063",
|
||||
}));
|
||||
|
||||
expect(prepareSpy).toHaveBeenCalledWith(
|
||||
"/tmp/test/.worktrees/idle-wt",
|
||||
"kb/fn-064",
|
||||
"kb/fn-063",
|
||||
"fusion/fn-064",
|
||||
"fusion/fn-063",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1467,7 +1492,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
(p) => p === "/tmp/test/.worktrees/idle-wt",
|
||||
);
|
||||
|
||||
const prepareSpy = vi.spyOn(pool, "prepareForTask").mockReturnValue("kb/fn-065");
|
||||
const prepareSpy = vi.spyOn(pool, "prepareForTask").mockReturnValue("fusion/fn-065");
|
||||
|
||||
const store = createMockStore();
|
||||
store.getSettings.mockResolvedValue({
|
||||
@@ -1487,7 +1512,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
|
||||
expect(prepareSpy).toHaveBeenCalledWith(
|
||||
"/tmp/test/.worktrees/idle-wt",
|
||||
"kb/fn-065",
|
||||
"fusion/fn-065",
|
||||
undefined,
|
||||
);
|
||||
});
|
||||
@@ -1500,7 +1525,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
);
|
||||
|
||||
// Pool returns a suffixed branch name due to conflict
|
||||
vi.spyOn(pool, "prepareForTask").mockReturnValue("kb/fn-066-2");
|
||||
vi.spyOn(pool, "prepareForTask").mockReturnValue("fusion/fn-066-2");
|
||||
|
||||
const store = createMockStore();
|
||||
store.getSettings.mockResolvedValue({
|
||||
@@ -1521,7 +1546,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
// Should store the suffixed branch name
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-066", {
|
||||
worktree: "/tmp/test/.worktrees/idle-wt",
|
||||
branch: "kb/fn-066-2",
|
||||
branch: "fusion/fn-066-2",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1770,117 +1795,29 @@ describe("Merger worktree pool integration", () => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
function createMergerMockStore(overrides: Record<string, any> = {}) {
|
||||
const listeners = new Map<string, Function[]>();
|
||||
return {
|
||||
on: vi.fn((event: string, fn: Function) => {
|
||||
const existing = listeners.get(event) || [];
|
||||
existing.push(fn);
|
||||
listeners.set(event, existing);
|
||||
}),
|
||||
emit: vi.fn(),
|
||||
getTask: vi.fn().mockResolvedValue({
|
||||
id: "FN-050",
|
||||
title: "Test merge",
|
||||
description: "Test",
|
||||
column: "in-review",
|
||||
dependencies: [],
|
||||
worktree: "/tmp/test/.worktrees/KB-050",
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
}),
|
||||
updateTask: vi.fn().mockResolvedValue({}),
|
||||
moveTask: vi.fn().mockResolvedValue({
|
||||
id: "FN-050",
|
||||
column: "done",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
}),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
logEntry: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: false,
|
||||
recycleWorktrees: false,
|
||||
...overrides,
|
||||
}),
|
||||
} as any;
|
||||
}
|
||||
|
||||
function mockMergerExecSync(cmd: any, opts?: any): any {
|
||||
const s = typeof cmd === "string" ? cmd : "";
|
||||
const isString = opts?.encoding === "utf-8";
|
||||
if (s.includes("rev-parse --verify")) return isString ? "abc123" : Buffer.from("abc123");
|
||||
if (s.includes("git log")) return isString ? "- test commit" : Buffer.from("- test commit");
|
||||
if (s.includes("git diff") && s.includes("--stat")) return isString ? "file.ts | 5 +++++" : Buffer.from("file.ts | 5 +++++");
|
||||
if (s.includes("diff --cached --quiet")) return isString ? "0" : Buffer.from("0");
|
||||
if (s.includes("diff --name-only --diff-filter=U")) return isString ? "" : Buffer.from("");
|
||||
return isString ? "" : Buffer.from("");
|
||||
}
|
||||
|
||||
it("releases worktree to pool instead of removing when recycleWorktrees is true", async () => {
|
||||
it("passes pool option through to aiMergeTask", async () => {
|
||||
const pool = new WorktreePool();
|
||||
const store = createMergerMockStore({ recycleWorktrees: true });
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
const mockedAiMergeTask = vi.mocked(aiMergeTask);
|
||||
mockedAiMergeTask.mockResolvedValue({
|
||||
task: { id: "FN-050" } as any,
|
||||
branch: "fusion/fn-050",
|
||||
merged: true,
|
||||
worktreeRemoved: false,
|
||||
branchDeleted: true,
|
||||
});
|
||||
|
||||
mockedExecSync.mockImplementation(mockMergerExecSync);
|
||||
await aiMergeTask({} as any, "/tmp/test", "FN-050", { pool });
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/test", "FN-050", { pool });
|
||||
|
||||
// Worktree should be in the pool, NOT removed
|
||||
expect(pool.has("/tmp/test/.worktrees/KB-050")).toBe(true);
|
||||
expect(result.worktreeRemoved).toBe(false);
|
||||
|
||||
// git worktree remove should NOT have been called
|
||||
const removeCalls = mockedExecSync.mock.calls.filter(
|
||||
(c) => typeof c[0] === "string" && (c[0] as string).includes("worktree remove"),
|
||||
expect(mockedAiMergeTask).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
"/tmp/test",
|
||||
"FN-050",
|
||||
expect.objectContaining({ pool }),
|
||||
);
|
||||
expect(removeCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("removes worktree normally when recycleWorktrees is false", async () => {
|
||||
const pool = new WorktreePool();
|
||||
const store = createMergerMockStore({ recycleWorktrees: false });
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
|
||||
mockedExecSync.mockImplementation(mockMergerExecSync);
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/test", "FN-050", { pool });
|
||||
|
||||
// Worktree should NOT be in the pool
|
||||
expect(pool.size).toBe(0);
|
||||
expect(result.worktreeRemoved).toBe(true);
|
||||
|
||||
// git worktree remove should have been called
|
||||
const removeCalls = mockedExecSync.mock.calls.filter(
|
||||
(c) => typeof c[0] === "string" && (c[0] as string).includes("worktree remove"),
|
||||
);
|
||||
expect(removeCalls.length).toBeGreaterThan(0);
|
||||
});
|
||||
// Full merger worktree pool integration tests are in merger.test.ts
|
||||
// which tests aiMergeTask with real implementation
|
||||
});
|
||||
|
||||
function createMockTaskDetail(overrides: Partial<TaskDetail> = {}): TaskDetail {
|
||||
@@ -2569,6 +2506,163 @@ describe("TaskExecutor pause behavior", () => {
|
||||
// Only one agent session created — the unpause during active session was a no-op
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("uses SessionManager.create for fresh execution and persists sessionFile", async () => {
|
||||
const store = createMockStore();
|
||||
const sessionFilePath = "/tmp/sessions/session_123.jsonl";
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
sessionFile: sessionFilePath,
|
||||
} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Fresh task",
|
||||
description: "Test fresh session",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Should use SessionManager.create for fresh execution
|
||||
expect(mockedSessionManager.create).toHaveBeenCalledWith(
|
||||
expect.stringContaining(".worktrees"),
|
||||
);
|
||||
expect(mockedSessionManager.open).not.toHaveBeenCalled();
|
||||
|
||||
// Should persist the session file path on the task
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { sessionFile: sessionFilePath });
|
||||
});
|
||||
|
||||
it("uses SessionManager.open to resume session when task has sessionFile", async () => {
|
||||
const store = createMockStore();
|
||||
const sessionFilePath = "/tmp/sessions/session_123.jsonl";
|
||||
const resumePromptFn = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
// existsSync must return true for the session file
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: resumePromptFn,
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
sessionFile: sessionFilePath,
|
||||
} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Resumed task",
|
||||
description: "Test session resume",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
sessionFile: sessionFilePath,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Should use SessionManager.open for the initial resumed execution
|
||||
expect(mockedSessionManager.open).toHaveBeenCalledWith(sessionFilePath);
|
||||
|
||||
// The first createKbAgent call should use the opened session manager
|
||||
const firstCall = mockedCreateHaiAgent.mock.calls[0][0] as any;
|
||||
expect(firstCall.sessionManager).toBeDefined();
|
||||
|
||||
// The log should indicate resume
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.stringContaining("Resumed agent session after unpause"),
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves sessionFile when task is paused (graceful exit)", async () => {
|
||||
const store = createMockStore();
|
||||
const sessionFilePath = "/tmp/sessions/session_456.jsonl";
|
||||
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
// Simulate pause — session ends gracefully
|
||||
store._trigger("task:updated", { id: "FN-001", paused: true, column: "in-progress" });
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
sessionFile: sessionFilePath,
|
||||
}) as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Pauseable task",
|
||||
description: "Test session file preserved on pause",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Session file should NOT be cleared when paused
|
||||
const clearCalls = store.updateTask.mock.calls.filter(
|
||||
(call: any[]) => call[0] === "FN-001" && call[1]?.sessionFile === null,
|
||||
);
|
||||
expect(clearCalls.length).toBe(0);
|
||||
|
||||
// Task should be moved to todo (ready for resume)
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo");
|
||||
});
|
||||
|
||||
it("falls back to fresh session when sessionFile no longer exists on disk", async () => {
|
||||
const store = createMockStore();
|
||||
const staleSessionFile = "/tmp/sessions/deleted_session.jsonl";
|
||||
|
||||
// Session file does NOT exist on disk
|
||||
mockedExistsSync.mockImplementation(
|
||||
(p) => p !== staleSessionFile,
|
||||
);
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
sessionFile: "/tmp/sessions/new_session.jsonl",
|
||||
} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Stale session",
|
||||
description: "Test stale session file fallback",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
sessionFile: staleSessionFile,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Should fall back to SessionManager.create (not open)
|
||||
expect(mockedSessionManager.create).toHaveBeenCalled();
|
||||
expect(mockedSessionManager.open).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskExecutor global pause behavior", () => {
|
||||
@@ -4206,7 +4300,7 @@ describe("task_add_dep tool", () => {
|
||||
|
||||
// Branch deletion should have been attempted
|
||||
const branchDeleteCalls = mockedExecSync.mock.calls.filter(
|
||||
(c) => typeof c[0] === "string" && (c[0] as string).includes("branch -D") && (c[0] as string).includes("kb/fn-dep"),
|
||||
(c) => typeof c[0] === "string" && (c[0] as string).includes("branch -D") && (c[0] as string).includes("fusion/fn-dep"),
|
||||
);
|
||||
expect(branchDeleteCalls.length).toBeGreaterThan(0);
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ import { generateWorktreeName, slugify } from "./worktree-names.js";
|
||||
import { Type, type Static } from "@mariozechner/pi-ai";
|
||||
import { createKbAgent, describeModel, promptWithFallback } from "./pi.js";
|
||||
import { reviewStep, type ReviewVerdict } from "./reviewer.js";
|
||||
import type { ToolDefinition, AgentSession, SessionManager } from "@mariozechner/pi-coding-agent";
|
||||
import { SessionManager, type ToolDefinition, type AgentSession } from "@mariozechner/pi-coding-agent";
|
||||
import { PRIORITY_EXECUTE, type AgentSemaphore } from "./concurrency.js";
|
||||
import type { WorktreePool } from "./worktree-pool.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
@@ -431,7 +431,7 @@ export class TaskExecutor {
|
||||
}
|
||||
|
||||
// Create or reuse worktree — try pool first when recycling is enabled
|
||||
const branchName = `kb/${task.id.toLowerCase()}`;
|
||||
const branchName = `fusion/${task.id.toLowerCase()}`;
|
||||
// Use generateWorktreeName for human-friendly directory names (adjective-noun pattern)
|
||||
// instead of task.id, so worktrees are named like ".worktrees/swift-falcon"
|
||||
let isResume = existsSync(worktreePath);
|
||||
@@ -563,6 +563,7 @@ export class TaskExecutor {
|
||||
const codeReviewVerdicts = new Map<number, ReviewVerdict>();
|
||||
|
||||
let taskDone = false;
|
||||
let wasPaused = false;
|
||||
// Mutable ref — populated after createKbAgent, tools access lazily via closure
|
||||
const sessionRef: { current: AgentSession | null } = { current: null };
|
||||
const stepCheckpoints = new Map<number, string>();
|
||||
@@ -604,7 +605,15 @@ export class TaskExecutor {
|
||||
const executorFallbackProvider = settings.fallbackProvider;
|
||||
const executorFallbackModelId = settings.fallbackModelId;
|
||||
|
||||
let { session } = await createKbAgent({
|
||||
// Determine whether we're resuming a previous session (pause/resume)
|
||||
// or starting fresh. Use file-based sessions so conversation state
|
||||
// persists across pause/unpause cycles.
|
||||
const isResuming = !!task.sessionFile && existsSync(task.sessionFile);
|
||||
const sessionManager = isResuming
|
||||
? SessionManager.open(task.sessionFile!)
|
||||
: SessionManager.create(worktreePath);
|
||||
|
||||
let { session, sessionFile } = await createKbAgent({
|
||||
cwd: worktreePath,
|
||||
systemPrompt: EXECUTOR_SYSTEM_PROMPT,
|
||||
tools: "coding",
|
||||
@@ -618,10 +627,20 @@ export class TaskExecutor {
|
||||
fallbackProvider: executorFallbackProvider,
|
||||
fallbackModelId: executorFallbackModelId,
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
sessionManager,
|
||||
});
|
||||
|
||||
executorLog.log(`${task.id}: using model ${describeModel(session)}`);
|
||||
await this.store.logEntry(task.id, `Executor using model: ${describeModel(session)}`);
|
||||
if (isResuming) {
|
||||
executorLog.log(`${task.id}: resumed session from ${task.sessionFile}`);
|
||||
await this.store.logEntry(task.id, `Resumed agent session after unpause (model: ${describeModel(session)})`);
|
||||
} else {
|
||||
executorLog.log(`${task.id}: using model ${describeModel(session)}`);
|
||||
await this.store.logEntry(task.id, `Executor using model: ${describeModel(session)}`);
|
||||
// Persist session file path so pause/resume can reopen it
|
||||
if (sessionFile) {
|
||||
await this.store.updateTask(task.id, { sessionFile });
|
||||
}
|
||||
}
|
||||
|
||||
// Make session available to custom tools (task_update checkpoint capture, review_step rewind)
|
||||
sessionRef.current = session;
|
||||
@@ -640,10 +659,21 @@ export class TaskExecutor {
|
||||
stuckDetector?.trackTask(task.id, session);
|
||||
|
||||
try {
|
||||
const agentPrompt = buildExecutionPrompt(detail, this.rootDir, settings);
|
||||
// Record activity on prompt start (heartbeat for stuck detection)
|
||||
stuckDetector?.recordActivity(task.id);
|
||||
await promptWithFallback(session, agentPrompt);
|
||||
|
||||
if (isResuming) {
|
||||
// Session already has full conversation history — just tell the
|
||||
// agent it was paused and should pick up where it left off.
|
||||
await promptWithFallback(session, [
|
||||
"Your session was paused and has now been resumed.",
|
||||
"Continue working on the task from where you left off.",
|
||||
"Review the current state of your worktree and proceed with the next pending step.",
|
||||
].join("\n"));
|
||||
} else {
|
||||
const agentPrompt = buildExecutionPrompt(detail, this.rootDir, settings);
|
||||
await promptWithFallback(session, agentPrompt);
|
||||
}
|
||||
|
||||
// Re-raise errors that pi-coding-agent swallowed after exhausting retries.
|
||||
// session.prompt() resolves normally even when retries are exhausted —
|
||||
@@ -662,8 +692,9 @@ export class TaskExecutor {
|
||||
// prompt to resolve gracefully instead of throwing.
|
||||
if (this.pausedAborted.has(task.id)) {
|
||||
this.pausedAborted.delete(task.id);
|
||||
wasPaused = true;
|
||||
executorLog.log(`${task.id} paused (graceful session exit) — moving to todo`);
|
||||
await this.store.logEntry(task.id, "Execution paused — agent terminated, moved to todo");
|
||||
await this.store.logEntry(task.id, "Execution paused — session preserved for resume, moved to todo");
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
return;
|
||||
}
|
||||
@@ -708,7 +739,7 @@ export class TaskExecutor {
|
||||
this.activeSessions.delete(task.id);
|
||||
session.dispose();
|
||||
|
||||
const { session: retrySession } = await createKbAgent({
|
||||
const { session: retrySession, sessionFile: retrySessionFile } = await createKbAgent({
|
||||
cwd: worktreePath,
|
||||
systemPrompt: EXECUTOR_SYSTEM_PROMPT,
|
||||
tools: "coding",
|
||||
@@ -722,7 +753,12 @@ export class TaskExecutor {
|
||||
fallbackProvider: executorFallbackProvider,
|
||||
fallbackModelId: executorFallbackModelId,
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
sessionManager: SessionManager.create(worktreePath),
|
||||
});
|
||||
// Update session file for the retry session (so pause/resume works)
|
||||
if (retrySessionFile) {
|
||||
this.store.updateTask(task.id, { sessionFile: retrySessionFile }).catch(() => {});
|
||||
}
|
||||
|
||||
// Reassign so finally{} disposes the correct session
|
||||
session = retrySession;
|
||||
@@ -778,6 +814,13 @@ export class TaskExecutor {
|
||||
stuckDetector?.untrackTask(task.id);
|
||||
await agentLogger.flush();
|
||||
session.dispose();
|
||||
// Clear session file when task completes or fails (not when paused —
|
||||
// the file is preserved so unpause can resume the conversation).
|
||||
// Check both the local flag (graceful exit) and the instance set
|
||||
// (error path where dispose caused prompt to throw).
|
||||
if (!wasPaused && !this.pausedAborted.has(task.id)) {
|
||||
this.store.updateTask(task.id, { sessionFile: null }).catch(() => {});
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1282,7 +1325,7 @@ export class TaskExecutor {
|
||||
|
||||
// Delete the branch — use stored branch name if available, fall back to convention
|
||||
const task = await this.store.getTask(taskId);
|
||||
const branch = task.branch || `kb/${taskId.toLowerCase()}`;
|
||||
const branch = task.branch || `fusion/${taskId.toLowerCase()}`;
|
||||
try {
|
||||
execSync(`git branch -D "${branch}"`, { cwd: this.rootDir, stdio: "pipe" });
|
||||
} catch {
|
||||
|
||||
@@ -237,23 +237,23 @@ describe("aiMergeTask — task.branch field", () => {
|
||||
|
||||
it("uses task.branch when set instead of deriving from task ID", async () => {
|
||||
const store = createMockStore(
|
||||
{ id: "FN-050", branch: "kb/fn-050-2", worktree: "/tmp/root/.worktrees/KB-050" },
|
||||
{ id: "FN-050", branch: "fusion/fn-050-2", worktree: "/tmp/root/.worktrees/KB-050" },
|
||||
[{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
|
||||
);
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
|
||||
// Should use kb/fn-050-2, not kb/fn-050
|
||||
expect(result.branch).toBe("kb/fn-050-2");
|
||||
expect(result.branch).toBe("fusion/fn-050-2");
|
||||
|
||||
// Verify the suffixed branch was verified and deleted
|
||||
const revParseCall = mockedExecSync.mock.calls.find(
|
||||
(call) => String(call[0]).includes("rev-parse --verify") && String(call[0]).includes("kb/fn-050-2"),
|
||||
(call) => String(call[0]).includes("rev-parse --verify") && String(call[0]).includes("fusion/fn-050-2"),
|
||||
);
|
||||
expect(revParseCall).toBeDefined();
|
||||
|
||||
const branchDeleteCall = mockedExecSync.mock.calls.find(
|
||||
(call) => String(call[0]).includes("branch -d") && String(call[0]).includes("kb/fn-050-2"),
|
||||
(call) => String(call[0]).includes("branch -d") && String(call[0]).includes("fusion/fn-050-2"),
|
||||
);
|
||||
expect(branchDeleteCall).toBeDefined();
|
||||
});
|
||||
@@ -266,7 +266,7 @@ describe("aiMergeTask — task.branch field", () => {
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
|
||||
expect(result.branch).toBe("kb/fn-050");
|
||||
expect(result.branch).toBe("fusion/fn-050");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -590,7 +590,7 @@ export async function aiMergeTask(
|
||||
throw new Error(`Cannot merge ${taskId}: ${mergeBlocker}`);
|
||||
}
|
||||
|
||||
const branch = task.branch || `kb/${taskId.toLowerCase()}`;
|
||||
const branch = task.branch || `fusion/${taskId.toLowerCase()}`;
|
||||
const worktreePath = task.worktree;
|
||||
const result: MergeResult = {
|
||||
task,
|
||||
@@ -622,6 +622,36 @@ export async function aiMergeTask(
|
||||
return result;
|
||||
}
|
||||
|
||||
// 3b. Ensure rootDir is on the main branch before merging.
|
||||
// Without this, a merge could land on whatever branch was last checked out,
|
||||
// causing feature code to be committed to the wrong lineage.
|
||||
try {
|
||||
const currentBranch = execSync("git symbolic-ref --short HEAD", {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
}).trim();
|
||||
const mainBranch = execSync("git rev-parse --abbrev-ref origin/HEAD", {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
}).trim().replace(/^origin\//, "");
|
||||
if (currentBranch !== mainBranch) {
|
||||
mergerLog.log(`${taskId}: rootDir on '${currentBranch}', checking out '${mainBranch}' before merge`);
|
||||
execSync(`git checkout "${mainBranch}"`, {
|
||||
cwd: rootDir,
|
||||
stdio: "pipe",
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
// Fallback: try checking out main directly
|
||||
try {
|
||||
execSync("git checkout main", { cwd: rootDir, stdio: "pipe" });
|
||||
} catch {
|
||||
mergerLog.warn(`${taskId}: unable to verify/checkout main branch — proceeding on current HEAD`);
|
||||
}
|
||||
}
|
||||
|
||||
// 4. Gather context for the agent (used in all attempts)
|
||||
let commitLog = "";
|
||||
let diffStat = "";
|
||||
|
||||
@@ -26,6 +26,8 @@ import {
|
||||
|
||||
export interface AgentResult {
|
||||
session: AgentSession;
|
||||
/** Path to the persisted session file (undefined for in-memory sessions). */
|
||||
sessionFile?: string;
|
||||
}
|
||||
|
||||
export interface PromptableSession extends AgentSession {
|
||||
@@ -77,6 +79,10 @@ export interface AgentOptions {
|
||||
fallbackModelId?: string;
|
||||
/** Default thinking effort level (e.g. "medium", "high"). When provided, sets the session's thinking level after creation. */
|
||||
defaultThinkingLevel?: string;
|
||||
/** Optional pre-configured SessionManager. When provided, the agent session
|
||||
* uses this instead of creating an in-memory session. Pass a file-based
|
||||
* SessionManager to enable session persistence and pause/resume. */
|
||||
sessionManager?: SessionManager;
|
||||
}
|
||||
|
||||
function resolveConfiguredModel(
|
||||
@@ -228,6 +234,8 @@ export async function createKbAgent(options: AgentOptions): Promise<AgentResult>
|
||||
});
|
||||
await resourceLoader.reload();
|
||||
|
||||
const sessionManager = options.sessionManager ?? SessionManager.inMemory();
|
||||
|
||||
const createSessionWithModel = async (modelOverride?: typeof selectedModel) => {
|
||||
return createAgentSession({
|
||||
cwd: options.cwd,
|
||||
@@ -236,7 +244,7 @@ export async function createKbAgent(options: AgentOptions): Promise<AgentResult>
|
||||
resourceLoader,
|
||||
tools,
|
||||
customTools: options.customTools,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
sessionManager,
|
||||
settingsManager,
|
||||
...(modelOverride ? { model: modelOverride } : {}),
|
||||
});
|
||||
@@ -336,5 +344,5 @@ export async function createKbAgent(options: AgentOptions): Promise<AgentResult>
|
||||
}
|
||||
});
|
||||
|
||||
return { session: promptableSession };
|
||||
return { session: promptableSession, sessionFile: promptableSession.sessionFile };
|
||||
}
|
||||
|
||||
@@ -38,6 +38,16 @@ vi.mock("node:fs", () => ({
|
||||
vi.mock("node:fs/promises", () => ({
|
||||
readFile: vi.fn().mockResolvedValue("# Task prompt content"),
|
||||
}));
|
||||
vi.mock("@mariozechner/pi-coding-agent", () => {
|
||||
const mockSessionManager = {};
|
||||
return {
|
||||
SessionManager: {
|
||||
create: vi.fn().mockReturnValue(mockSessionManager),
|
||||
open: vi.fn().mockReturnValue(mockSessionManager),
|
||||
inMemory: vi.fn().mockReturnValue(mockSessionManager),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import { TaskExecutor } from "./executor.js";
|
||||
import { TriageProcessor } from "./triage.js";
|
||||
|
||||
@@ -326,7 +326,7 @@ export class Scheduler {
|
||||
for (const depId of task.dependencies) {
|
||||
const dep = allTasks.find((t) => t.id === depId);
|
||||
if (dep && dep.column === "in-review" && dep.worktree) {
|
||||
return `fusion/${dep.id.toLowerCase()}`;
|
||||
return dep.branch || `fusion/${dep.id.toLowerCase()}`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -334,7 +334,7 @@ export class Scheduler {
|
||||
if (task.blockedBy) {
|
||||
const blocker = allTasks.find((t) => t.id === task.blockedBy);
|
||||
if (blocker && blocker.column === "in-review" && blocker.worktree) {
|
||||
return `kb/${blocker.id.toLowerCase()}`;
|
||||
return blocker.branch || `fusion/${blocker.id.toLowerCase()}`;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ export default defineConfig({
|
||||
include: ["src/**/*.test.ts"],
|
||||
maxWorkers,
|
||||
fileParallelism: true,
|
||||
pool: "vmThreads",
|
||||
coverage: {
|
||||
enabled: false,
|
||||
reporter: ["text", "html", "json"],
|
||||
|
||||
Reference in New Issue
Block a user