feat(KB-176): make enginePaused a soft pause that lets running agents finish
- Remove agent termination listeners for enginePaused in executor and triage - Update Settings.enginePaused JSDoc to document soft-pause semantics - Update executor/triage/integration tests to assert sessions are NOT disposed on enginePaused - Remove pause-abort tracking for enginePaused (only globalPause hard-stops agents) - Add changeset for the behavioral change
This commit is contained in:
@@ -101,17 +101,15 @@ export interface Settings {
|
||||
* global emergency stop for the entire AI engine.
|
||||
* Individual per-task pause flags are unaffected. */
|
||||
globalPause?: boolean;
|
||||
/** Engine pause: when true, the scheduler and triage processor stop
|
||||
* dispatching **new** work (scheduling, triage specification, and
|
||||
* auto-merge), and all active agent sessions (executor and triage) are
|
||||
* terminated — matching the {@link globalPause} behavior for agent
|
||||
* lifecycle. Terminated tasks are moved back to `todo` (executor) or
|
||||
* have their `specifying` status cleared (triage) so they can resume
|
||||
* cleanly when the engine is unpaused. Remains independent from
|
||||
* `globalPause` for scheduling control: `globalPause` is the emergency
|
||||
* stop, while `enginePaused` is the normal on/off toggle for the AI
|
||||
* engine. Has no additional effect when {@link globalPause} is also
|
||||
* true (hard stop already covers agent termination). */
|
||||
/** Engine pause (soft pause): when true, the scheduler and triage
|
||||
* processor stop dispatching **new** work (scheduling, triage
|
||||
* specification, and auto-merge), but currently running agent sessions
|
||||
* are allowed to finish naturally — no sessions are terminated.
|
||||
* This is the normal on/off toggle for the AI engine.
|
||||
* Contrast with {@link globalPause}, which is a hard stop that
|
||||
* immediately terminates all active agent sessions. Has no additional
|
||||
* effect when {@link globalPause} is also true (hard stop already
|
||||
* covers everything). */
|
||||
enginePaused?: boolean;
|
||||
/** Maximum number of concurrent AI agents across all activity types
|
||||
* (triage specification, task execution, and merge operations). */
|
||||
|
||||
@@ -1316,61 +1316,47 @@ describe("TaskExecutor global pause behavior", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskExecutor enginePaused agent termination", () => {
|
||||
describe("TaskExecutor enginePaused soft pause (no agent termination)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
});
|
||||
|
||||
it("disposes all active sessions when enginePaused transitions false→true", async () => {
|
||||
it("does NOT dispose active sessions when enginePaused transitions false→true", async () => {
|
||||
const store = createMockStore();
|
||||
const disposeFn1 = vi.fn();
|
||||
const disposeFn2 = vi.fn();
|
||||
let callCount = 0;
|
||||
const disposeFn = vi.fn();
|
||||
|
||||
mockedCreateHaiAgent.mockImplementation(async () => {
|
||||
callCount++;
|
||||
const dispose = callCount === 1 ? disposeFn1 : disposeFn2;
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
// Fire engine pause when the second task starts
|
||||
if (callCount === 2) {
|
||||
store._trigger("settings:updated", {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
}
|
||||
throw new Error("Session terminated");
|
||||
}),
|
||||
dispose,
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
// Trigger engine pause while the session is active
|
||||
store._trigger("settings:updated", {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
// Session continues normally — no error thrown
|
||||
}),
|
||||
dispose: disposeFn,
|
||||
},
|
||||
} as any));
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute({
|
||||
id: "KB-001", title: "Test", description: "T", column: "in-progress",
|
||||
dependencies: [], steps: [], currentStep: 0, log: [],
|
||||
createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
await Promise.all([
|
||||
executor.execute({
|
||||
id: "KB-001", title: "T1", description: "T", column: "in-progress",
|
||||
dependencies: [], steps: [], currentStep: 0, log: [],
|
||||
createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(),
|
||||
}),
|
||||
executor.execute({
|
||||
id: "KB-002", title: "T2", description: "T", column: "in-progress",
|
||||
dependencies: [], steps: [], currentStep: 0, log: [],
|
||||
createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(),
|
||||
}),
|
||||
]);
|
||||
|
||||
// Both tasks should be moved to todo (not marked as failed)
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-002", "todo");
|
||||
// dispose should only be called once in the finally block (normal cleanup),
|
||||
// NOT by an engine pause listener
|
||||
expect(disposeFn).toHaveBeenCalledTimes(1);
|
||||
// Task should complete normally and move to in-review, not todo
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "in-review");
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("KB-001", "todo");
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("KB-001", { status: "failed" });
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("KB-002", { status: "failed" });
|
||||
});
|
||||
|
||||
it("moves terminated tasks to todo (not failed)", async () => {
|
||||
it("does NOT move tasks to todo when enginePaused transitions false→true", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
@@ -1380,7 +1366,7 @@ describe("TaskExecutor enginePaused agent termination", () => {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
throw new Error("Session terminated");
|
||||
// Session continues normally
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
@@ -1393,8 +1379,9 @@ describe("TaskExecutor enginePaused agent termination", () => {
|
||||
createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("KB-001", { status: "failed" });
|
||||
// Task should complete normally (in-review), not be moved to todo
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "in-review");
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("KB-001", "todo");
|
||||
});
|
||||
|
||||
it("takes no action when enginePaused stays false (false→false)", async () => {
|
||||
|
||||
@@ -172,7 +172,8 @@ export class TaskExecutor {
|
||||
* Listens for `task:moved` to auto-execute tasks moved to `in-progress`,
|
||||
* `task:updated` to terminate agent sessions when individual tasks are paused,
|
||||
* and `settings:updated` to terminate **all** active agent sessions when
|
||||
* `globalPause` or `enginePaused` transitions from `false` to `true`.
|
||||
* `globalPause` transitions from `false` to `true`. `enginePaused` only
|
||||
* prevents new work dispatch — running sessions continue to completion.
|
||||
* Paused tasks are moved back to `todo` rather than marked as `failed`.
|
||||
*/
|
||||
constructor(
|
||||
@@ -209,17 +210,6 @@ export class TaskExecutor {
|
||||
}
|
||||
});
|
||||
|
||||
// When enginePaused transitions from false → true, terminate all active agent sessions.
|
||||
// Same pattern as globalPause: agents are killed, tasks moved to todo (not failed).
|
||||
store.on("settings:updated", ({ settings, previous }) => {
|
||||
if (settings.enginePaused && !previous.enginePaused) {
|
||||
for (const [taskId, session] of this.activeSessions) {
|
||||
executorLog.log(`Engine pause — terminating agent session for ${taskId}`);
|
||||
this.pausedAborted.add(taskId);
|
||||
session.dispose();
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -923,21 +923,21 @@ describe("Edge case: worktree deleted between scan and acquire", () => {
|
||||
// ── Engine pause/unpause cycle integration tests ──────────────────────────
|
||||
|
||||
describe("Engine pause/unpause cycle", () => {
|
||||
it("executor: agents terminated on pause, tasks moved to todo, resume picks them up", async () => {
|
||||
it("executor: agents continue running on enginePaused (soft pause), complete normally", async () => {
|
||||
const store = createMockStore();
|
||||
const task = makeTask("KB-EP1", "in-progress");
|
||||
store.getTask.mockResolvedValue(makeTaskDetail("KB-EP1", "in-progress"));
|
||||
|
||||
// Agent waits for engine pause, then throws (simulating session termination)
|
||||
// Agent triggers engine pause mid-flight but continues normally (soft pause)
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
// Trigger engine pause
|
||||
// Trigger engine pause — session should NOT be terminated
|
||||
store._trigger("settings:updated", {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
throw new Error("Session terminated");
|
||||
// Session continues normally
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
@@ -946,36 +946,18 @@ describe("Engine pause/unpause cycle", () => {
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(task);
|
||||
|
||||
// Task should be moved to todo (not failed)
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-EP1", "todo");
|
||||
// Task should complete normally (in-review), NOT moved to todo
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-EP1", "in-review");
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("KB-EP1", "todo");
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("KB-EP1", { status: "failed" });
|
||||
|
||||
// Now simulate unpause: resumeOrphaned picks up in-progress tasks
|
||||
store.moveTask.mockClear();
|
||||
mockedCreateHaiAgent.mockClear();
|
||||
|
||||
// After unpause, the task would be in todo; scheduler would move it to in-progress
|
||||
// Here we test that resumeOrphaned can pick up orphaned in-progress tasks
|
||||
const resumeTask = makeTask("KB-EP1", "in-progress");
|
||||
store.listTasks.mockResolvedValue([resumeTask]);
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
await executor.resumeOrphaned();
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// Agent should be created again for the resumed task
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("triage: agents terminated on pause, status cleared, resume picks up tasks", async () => {
|
||||
it("triage: agents NOT terminated on enginePaused (soft pause), session continues", async () => {
|
||||
const store = createMockStore();
|
||||
const disposeFn = vi.fn();
|
||||
let sessionContinued = false;
|
||||
|
||||
// Agent waits for engine pause, then throws
|
||||
// Agent triggers engine pause mid-flight; session should NOT be disposed
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
@@ -983,41 +965,24 @@ describe("Engine pause/unpause cycle", () => {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
throw new Error("Session terminated");
|
||||
// If the session was disposed by the listener, we'd get an error.
|
||||
// Instead, the session continues normally (soft pause).
|
||||
sessionContinued = true;
|
||||
// Throw to exit without needing file system (PROMPT.md read).
|
||||
// The key assertion is that dispose was NOT called by the listener.
|
||||
throw new Error("simulated completion");
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
dispose: disposeFn,
|
||||
},
|
||||
} as any));
|
||||
|
||||
const triage = new TriageProcessor(store, "/tmp/test");
|
||||
|
||||
await triage.specifyTask(makeTask("KB-EP2", "triage"));
|
||||
|
||||
// Status should be cleared (not reported as error)
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-EP2", { status: null });
|
||||
|
||||
// Now simulate unpause: triage poll picks up unspecified tasks
|
||||
store.updateTask.mockClear();
|
||||
mockedCreateHaiAgent.mockClear();
|
||||
|
||||
const triageTask = makeTask("KB-EP2", "triage");
|
||||
store.listTasks.mockResolvedValue([triageTask]);
|
||||
store.getSettings.mockResolvedValue({ ...DEFAULT_SETTINGS, enginePaused: false });
|
||||
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
// Trigger a poll — triage should process the task again
|
||||
(triage as any).running = true;
|
||||
await (triage as any).poll();
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// Task should be picked up for specification
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-EP2", { status: "specifying" });
|
||||
// Session should have continued past the enginePaused event
|
||||
expect(sessionContinued).toBe(true);
|
||||
// dispose should only be called once in the finally block, not by the engine pause listener
|
||||
expect(disposeFn).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("scheduler resumes on unpause: schedule() runs when enginePaused goes true→false", async () => {
|
||||
@@ -1065,7 +1030,7 @@ describe("Engine pause/unpause cycle", () => {
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-EP4", "in-progress");
|
||||
});
|
||||
|
||||
it("concurrency slots freed on pause: semaphore released when agents are terminated", async () => {
|
||||
it("concurrency slots freed after agent completes during enginePaused (soft pause)", async () => {
|
||||
const sem = new AgentSemaphore(1); // Only 1 concurrent slot
|
||||
const store = createMockStore();
|
||||
store.getTask.mockResolvedValue(makeTaskDetail("KB-EP5", "in-progress"));
|
||||
@@ -1078,7 +1043,7 @@ describe("Engine pause/unpause cycle", () => {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
throw new Error("Session terminated");
|
||||
// Agent continues and finishes normally (soft pause)
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
@@ -1086,10 +1051,10 @@ describe("Engine pause/unpause cycle", () => {
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { semaphore: sem });
|
||||
|
||||
// Execute — this acquires the semaphore slot, runs the agent, then releases on termination
|
||||
// Execute — agent runs to completion despite engine pause
|
||||
await executor.execute(makeTask("KB-EP5", "in-progress"));
|
||||
|
||||
// After termination, the semaphore slot should be freed.
|
||||
// After completion, the semaphore slot should be freed.
|
||||
// Verify by running a new task through the semaphore — it should not block.
|
||||
let secondSlotAcquired = false;
|
||||
const runPromise = sem.run(async () => {
|
||||
|
||||
@@ -1821,12 +1821,12 @@ describe("TriageProcessor global pause agent kill", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("TriageProcessor enginePaused agent termination", () => {
|
||||
describe("TriageProcessor enginePaused soft pause (no agent termination)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("terminates active triage sessions when enginePaused transitions false→true", async () => {
|
||||
it("does NOT terminate active triage sessions when enginePaused transitions false→true", async () => {
|
||||
const store = createMockStore();
|
||||
const disposeFn = vi.fn();
|
||||
|
||||
@@ -1838,7 +1838,7 @@ describe("TriageProcessor enginePaused agent termination", () => {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
throw new Error("Session terminated");
|
||||
// Session continues normally — no error thrown
|
||||
}),
|
||||
dispose: disposeFn,
|
||||
},
|
||||
@@ -1859,11 +1859,14 @@ describe("TriageProcessor enginePaused agent termination", () => {
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// dispose is called by the engine pause listener and again in finally
|
||||
expect(disposeFn).toHaveBeenCalled();
|
||||
// dispose should only be called once in the finally block (normal cleanup),
|
||||
// NOT by an engine pause listener
|
||||
expect(disposeFn).toHaveBeenCalledTimes(1);
|
||||
// Task should proceed to todo (normal completion), not be aborted
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
});
|
||||
|
||||
it("clears specifying status on terminated tasks", async () => {
|
||||
it("does NOT clear specifying status when enginePaused transitions false→true", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
@@ -1873,7 +1876,7 @@ describe("TriageProcessor enginePaused agent termination", () => {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
throw new Error("Session terminated");
|
||||
// Session continues normally
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
@@ -1894,46 +1897,8 @@ describe("TriageProcessor enginePaused agent termination", () => {
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Status should be cleared (not reported as error)
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: null });
|
||||
});
|
||||
|
||||
it("does not report errors for engine-pause-aborted tasks", async () => {
|
||||
const store = createMockStore();
|
||||
const onError = vi.fn();
|
||||
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
store._trigger("settings:updated", {
|
||||
settings: { enginePaused: true },
|
||||
previous: { enginePaused: false },
|
||||
});
|
||||
throw new Error("Session terminated");
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any));
|
||||
|
||||
const triage = new TriageProcessor(store, "/tmp/test", { onSpecifyError: onError });
|
||||
|
||||
await triage.specifyTask({
|
||||
id: "KB-001",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// onSpecifyError should NOT be called for engine-pause aborted tasks
|
||||
expect(onError).not.toHaveBeenCalled();
|
||||
// Status should be cleared
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: null });
|
||||
// Task should complete normally and move to todo
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -215,7 +215,7 @@ export class TriageProcessor {
|
||||
private wasEnginePaused = false;
|
||||
/** Active agent sessions per task, used to terminate on pause. */
|
||||
private activeSessions = new Map<string, { dispose: () => void }>();
|
||||
/** Tasks aborted due to globalPause or enginePaused (to avoid reporting as errors). */
|
||||
/** Tasks aborted due to globalPause (to avoid reporting as errors). */
|
||||
private pauseAborted = new Set<string>();
|
||||
|
||||
/**
|
||||
@@ -223,9 +223,10 @@ export class TriageProcessor {
|
||||
* @param rootDir — Project root directory
|
||||
* @param options — Processor configuration
|
||||
*
|
||||
* Listens for `settings:updated` events: when `globalPause` or `enginePaused`
|
||||
* transitions from `false` to `true`, all active triage specification sessions
|
||||
* are immediately terminated so the engine stops all AI activity.
|
||||
* Listens for `settings:updated` events: when `globalPause` transitions from
|
||||
* `false` to `true`, all active triage specification sessions are immediately
|
||||
* terminated. When `enginePaused` transitions, only new work dispatch is
|
||||
* affected — running sessions continue to completion.
|
||||
*/
|
||||
constructor(
|
||||
private store: TaskStore,
|
||||
@@ -245,20 +246,6 @@ export class TriageProcessor {
|
||||
}
|
||||
});
|
||||
|
||||
// When enginePaused transitions from false → true, terminate all active triage sessions.
|
||||
// Same pattern as globalPause: agents are killed, status cleared (not reported as error).
|
||||
store.on("settings:updated", ({ settings, previous }) => {
|
||||
if (settings.enginePaused && !previous.enginePaused) {
|
||||
for (const [taskId, session] of this.activeSessions) {
|
||||
triageLog.log(
|
||||
`Engine pause — terminating triage session for ${taskId}`,
|
||||
);
|
||||
this.pauseAborted.add(taskId);
|
||||
session.dispose();
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Immediate unpause resume: when `globalPause` transitions from `true`
|
||||
* to `false`, trigger a triage poll right away instead of waiting for
|
||||
|
||||
Reference in New Issue
Block a user