feat(KB-206): add stuck task detection and auto-recovery
- Add taskStuckTimeoutMs setting to detect tasks with no activity - Create StuckTaskDetector to poll in-progress tasks every 30s - Integrate heartbeat tracking into executor via step callbacks - Implement recovery flow: abort stuck sessions, retry with preserved progress - Export detector and wire into dashboard for real-time monitoring - Add comprehensive tests and documentation to AGENTS.md
This commit is contained in:
@@ -4,7 +4,7 @@ import { createInterface } from "node:readline";
|
||||
import { TaskStore, AutomationStore } from "@kb/core";
|
||||
import type { Settings, TaskDetail, PrInfo } from "@kb/core";
|
||||
import { createServer, GitHubClient } from "@kb/dashboard";
|
||||
import { TriageProcessor, TaskExecutor, Scheduler, AgentSemaphore, WorktreePool, aiMergeTask, UsageLimitPauser, PRIORITY_MERGE, scanIdleWorktrees, cleanupOrphanedWorktrees, NtfyNotifier, PrMonitor, PrCommentHandler, CronRunner } from "@kb/engine";
|
||||
import { TriageProcessor, TaskExecutor, Scheduler, AgentSemaphore, WorktreePool, aiMergeTask, UsageLimitPauser, PRIORITY_MERGE, scanIdleWorktrees, cleanupOrphanedWorktrees, NtfyNotifier, PrMonitor, PrCommentHandler, CronRunner, StuckTaskDetector } from "@kb/engine";
|
||||
import { AuthStorage, ModelRegistry } from "@mariozechner/pi-coding-agent";
|
||||
|
||||
/**
|
||||
@@ -466,14 +466,28 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
onSpecifyError: (t, e) => console.log(`[engine] ✗ ${t.id}: ${e.message}`),
|
||||
});
|
||||
|
||||
// ── Stuck task detector: monitors agent sessions for stagnation ────
|
||||
// Created before the executor so it can be passed in options.
|
||||
// The onStuck callback is wired to executor.markStuckAborted after
|
||||
// executor creation (late-binding via closure on executorRef).
|
||||
const executorRef: { current: TaskExecutor | null } = { current: null };
|
||||
const stuckTaskDetector = new StuckTaskDetector(store, {
|
||||
onStuck: (taskId) => {
|
||||
executorRef.current?.markStuckAborted(taskId);
|
||||
console.log(`[engine] ⚠ ${taskId} stuck — terminated, will retry`);
|
||||
},
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, cwd, {
|
||||
semaphore,
|
||||
pool,
|
||||
usageLimitPauser,
|
||||
stuckTaskDetector,
|
||||
onStart: (t, p) => console.log(`[engine] Executing ${t.id} in ${p}`),
|
||||
onComplete: (t) => console.log(`[engine] ✓ ${t.id} → in-review`),
|
||||
onError: (t, e) => console.log(`[engine] ✗ ${t.id}: ${e.message}`),
|
||||
});
|
||||
executorRef.current = executor;
|
||||
|
||||
const settings = await store.getSettings();
|
||||
const prMonitor = new PrMonitor();
|
||||
@@ -495,6 +509,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
|
||||
triage.start();
|
||||
scheduler.start();
|
||||
stuckTaskDetector.start();
|
||||
|
||||
// ── Startup sweep: resume orphaned in-progress tasks ──────────────
|
||||
executor.resumeOrphaned().catch((err) =>
|
||||
@@ -595,6 +610,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
scheduleMergeRetry();
|
||||
|
||||
process.on("SIGINT", () => {
|
||||
stuckTaskDetector.stop();
|
||||
triage.stop();
|
||||
scheduler.stop();
|
||||
cronRunner.stop();
|
||||
|
||||
@@ -301,6 +301,11 @@ export interface Settings {
|
||||
/** When true, enables ntfy.sh push notifications for task completion and failures.
|
||||
* Requires ntfyTopic to be set. Default: false. */
|
||||
ntfyEnabled?: boolean;
|
||||
/** Timeout in milliseconds for detecting stuck tasks. When a task's agent session
|
||||
* shows no activity (no text deltas, tool calls, or progress updates) for longer
|
||||
* than this duration, the task is considered stuck and will be terminated and retried.
|
||||
* Default: undefined (disabled). Suggested value: 600000 (10 minutes). */
|
||||
taskStuckTimeoutMs?: number;
|
||||
/** Theme mode preference: dark, light, or system (follows OS). Default: "dark". */
|
||||
themeMode?: ThemeMode;
|
||||
/** Color theme preference for accent colors and styling. Default: "default". */
|
||||
@@ -329,6 +334,7 @@ export const DEFAULT_SETTINGS: Settings = {
|
||||
requirePlanApproval: false,
|
||||
ntfyEnabled: false,
|
||||
ntfyTopic: undefined,
|
||||
taskStuckTimeoutMs: undefined,
|
||||
themeMode: "dark",
|
||||
colorTheme: "default",
|
||||
};
|
||||
|
||||
@@ -13,6 +13,7 @@ import type { WorktreePool } from "./worktree-pool.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
import { executorLog, reviewerLog } from "./logger.js";
|
||||
import { isUsageLimitError, checkSessionError, type UsageLimitPauser } from "./usage-limit-detector.js";
|
||||
import type { StuckTaskDetector } from "./stuck-task-detector.js";
|
||||
|
||||
// Re-export for backward compatibility (tests import from executor.ts)
|
||||
export { summarizeToolArgs } from "./agent-logger.js";
|
||||
@@ -147,6 +148,8 @@ export interface TaskExecutorOptions {
|
||||
pool?: WorktreePool;
|
||||
/** Usage limit pauser — triggers global pause when API limits are detected. */
|
||||
usageLimitPauser?: UsageLimitPauser;
|
||||
/** Stuck task detector — monitors agent sessions for stagnation and triggers recovery. */
|
||||
stuckTaskDetector?: StuckTaskDetector;
|
||||
onStart?: (task: Task, worktreePath: string) => void;
|
||||
onComplete?: (task: Task) => void;
|
||||
onError?: (task: Task, error: Error) => void;
|
||||
@@ -163,6 +166,8 @@ export class TaskExecutor {
|
||||
private pausedAborted = new Set<string>();
|
||||
/** Tasks that had a dependency added mid-execution (abort + discard worktree). */
|
||||
private depAborted = new Set<string>();
|
||||
/** Tasks that were killed by stuck task detector (to avoid marking them as "failed"). */
|
||||
private stuckAborted = new Set<string>();
|
||||
|
||||
/**
|
||||
* @param store — Task store instance (also used to listen for events)
|
||||
@@ -194,6 +199,7 @@ export class TaskExecutor {
|
||||
if (task.paused && this.activeSessions.has(task.id)) {
|
||||
executorLog.log(`Pausing ${task.id} — terminating agent session`);
|
||||
this.pausedAborted.add(task.id);
|
||||
this.options.stuckTaskDetector?.untrackTask(task.id);
|
||||
const session = this.activeSessions.get(task.id);
|
||||
session?.dispose();
|
||||
}
|
||||
@@ -205,6 +211,7 @@ export class TaskExecutor {
|
||||
for (const [taskId, session] of this.activeSessions) {
|
||||
executorLog.log(`Global pause — terminating agent session for ${taskId}`);
|
||||
this.pausedAborted.add(taskId);
|
||||
this.options.stuckTaskDetector?.untrackTask(taskId);
|
||||
session.dispose();
|
||||
}
|
||||
}
|
||||
@@ -417,8 +424,10 @@ export class TaskExecutor {
|
||||
const sessionRef: { current: AgentSession | null } = { current: null };
|
||||
const stepCheckpoints = new Map<number, string>();
|
||||
|
||||
const stuckDetector = this.options.stuckTaskDetector;
|
||||
|
||||
const customTools = [
|
||||
this.createTaskUpdateTool(task.id, codeReviewVerdicts, sessionRef, stepCheckpoints),
|
||||
this.createTaskUpdateTool(task.id, codeReviewVerdicts, sessionRef, stepCheckpoints, stuckDetector),
|
||||
this.createTaskLogTool(task.id),
|
||||
this.createTaskCreateTool(),
|
||||
this.createTaskAddDepTool(task.id),
|
||||
@@ -430,8 +439,14 @@ export class TaskExecutor {
|
||||
store: this.store,
|
||||
taskId: task.id,
|
||||
agent: "executor",
|
||||
onAgentText: this.options.onAgentText,
|
||||
onAgentTool: this.options.onAgentTool,
|
||||
onAgentText: (taskId, delta) => {
|
||||
stuckDetector?.recordActivity(taskId);
|
||||
this.options.onAgentText?.(taskId, delta);
|
||||
},
|
||||
onAgentTool: (taskId, toolName) => {
|
||||
stuckDetector?.recordActivity(taskId);
|
||||
this.options.onAgentTool?.(taskId, toolName);
|
||||
},
|
||||
});
|
||||
|
||||
const agentWork = async () => {
|
||||
@@ -464,8 +479,13 @@ export class TaskExecutor {
|
||||
// Register session so the pause listener can terminate it
|
||||
this.activeSessions.set(task.id, session);
|
||||
|
||||
// Register with stuck task detector for heartbeat monitoring
|
||||
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 session.prompt(agentPrompt);
|
||||
|
||||
// Re-raise errors that pi-coding-agent swallowed after exhausting retries.
|
||||
@@ -498,6 +518,7 @@ export class TaskExecutor {
|
||||
}
|
||||
} finally {
|
||||
this.activeSessions.delete(task.id);
|
||||
stuckDetector?.untrackTask(task.id);
|
||||
await agentLogger.flush();
|
||||
session.dispose();
|
||||
}
|
||||
@@ -530,6 +551,11 @@ export class TaskExecutor {
|
||||
this.pausedAborted.delete(task.id);
|
||||
await this.store.logEntry(task.id, "Execution paused — agent terminated, moved to todo");
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
} else if (this.stuckAborted.has(task.id)) {
|
||||
// Task was killed by stuck task detector — already moved to todo by killAndRetry.
|
||||
// Don't mark as failed; the scheduler will retry it naturally.
|
||||
executorLog.log(`${task.id} terminated by stuck task detector — will retry`);
|
||||
this.stuckAborted.delete(task.id);
|
||||
} else {
|
||||
// Check if the error is a usage-limit error and trigger global pause
|
||||
if (this.options.usageLimitPauser && isUsageLimitError(err.message)) {
|
||||
@@ -552,6 +578,7 @@ export class TaskExecutor {
|
||||
codeReviewVerdicts: Map<number, ReviewVerdict>,
|
||||
sessionRef: { current: AgentSession | null },
|
||||
stepCheckpoints: Map<number, string>,
|
||||
stuckDetector?: StuckTaskDetector,
|
||||
): ToolDefinition {
|
||||
const store = this.store;
|
||||
return {
|
||||
@@ -565,6 +592,9 @@ export class TaskExecutor {
|
||||
execute: async (_id: string, params: Static<typeof taskUpdateParams>) => {
|
||||
const { step, status } = params;
|
||||
|
||||
// Record heartbeat for stuck task detection
|
||||
stuckDetector?.recordActivity(taskId);
|
||||
|
||||
// Enforce code review REVISE: block advancing to "done" when the last
|
||||
// code review for this step returned REVISE. The agent must fix the
|
||||
// issues and call review_step(type="code") again before proceeding.
|
||||
@@ -1043,6 +1073,15 @@ export class TaskExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark a task as stuck-aborted so the executor's error handling
|
||||
* knows not to treat the disposed session as a genuine failure.
|
||||
* Called by the stuck task detector's onStuck callback.
|
||||
*/
|
||||
markStuckAborted(taskId: string): void {
|
||||
this.stuckAborted.add(taskId);
|
||||
}
|
||||
|
||||
getWorktreePath(taskId: string): string | undefined {
|
||||
return this.activeWorktrees.get(taskId);
|
||||
}
|
||||
|
||||
@@ -13,3 +13,4 @@ export { PrMonitor, type PrComment, type TrackedPr, type OnNewCommentsCallback }
|
||||
export { PrCommentHandler } from "./pr-comment-handler.js";
|
||||
export { NtfyNotifier, type NtfyNotifierOptions } from "./notifier.js";
|
||||
export { CronRunner, type CronRunnerOptions } from "./cron-runner.js";
|
||||
export { StuckTaskDetector, type StuckTaskDetectorOptions, type DisposableSession } from "./stuck-task-detector.js";
|
||||
|
||||
442
packages/engine/src/stuck-task-detector.test.ts
Normal file
442
packages/engine/src/stuck-task-detector.test.ts
Normal file
@@ -0,0 +1,442 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { StuckTaskDetector, type DisposableSession } from "./stuck-task-detector.js";
|
||||
import type { Settings } from "@kb/core";
|
||||
|
||||
// Mock the logger
|
||||
vi.mock("./logger.js", () => ({
|
||||
createLogger: () => ({ log: vi.fn(), warn: vi.fn(), error: vi.fn() }),
|
||||
}));
|
||||
|
||||
/** Minimal mock store that satisfies StuckTaskDetector's needs. */
|
||||
function createMockStore(settings: Partial<Settings> = {}) {
|
||||
const defaultSettings: Settings = {
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
taskStuckTimeoutMs: undefined,
|
||||
...settings,
|
||||
};
|
||||
|
||||
const store: any = {
|
||||
_settings: { ...defaultSettings },
|
||||
getSettings: vi.fn(async () => ({ ...store._settings })),
|
||||
logEntry: vi.fn(async () => {}),
|
||||
updateTask: vi.fn(async () => ({})),
|
||||
moveTask: vi.fn(async () => ({})),
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
emit: vi.fn(),
|
||||
};
|
||||
return store;
|
||||
}
|
||||
|
||||
function createMockSession(): DisposableSession & { dispose: ReturnType<typeof vi.fn> } {
|
||||
return { dispose: vi.fn() };
|
||||
}
|
||||
|
||||
describe("StuckTaskDetector", () => {
|
||||
let detector: StuckTaskDetector;
|
||||
|
||||
afterEach(() => {
|
||||
detector?.stop();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("trackTask / untrackTask", () => {
|
||||
it("records initial activity timestamp when tracking a task", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
const before = Date.now();
|
||||
detector.trackTask("KB-001", session);
|
||||
const after = Date.now();
|
||||
|
||||
const lastActivity = detector.getLastActivity("KB-001");
|
||||
expect(lastActivity).toBeDefined();
|
||||
expect(lastActivity).toBeGreaterThanOrEqual(before);
|
||||
expect(lastActivity).toBeLessThanOrEqual(after);
|
||||
});
|
||||
|
||||
it("returns undefined for untracked tasks", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
expect(detector.getLastActivity("KB-999")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("removes task from monitoring on untrack", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
expect(detector.trackedCount).toBe(1);
|
||||
|
||||
detector.untrackTask("KB-001");
|
||||
expect(detector.trackedCount).toBe(0);
|
||||
expect(detector.getLastActivity("KB-001")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("is a no-op when untracking a non-existent task", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
// Should not throw
|
||||
detector.untrackTask("KB-999");
|
||||
expect(detector.trackedCount).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("recordActivity", () => {
|
||||
it("updates the activity timestamp", async () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
const initial = detector.getLastActivity("KB-001")!;
|
||||
|
||||
// Wait a small amount to ensure timestamp differs
|
||||
await new Promise((resolve) => setTimeout(resolve, 5));
|
||||
|
||||
detector.recordActivity("KB-001");
|
||||
const updated = detector.getLastActivity("KB-001")!;
|
||||
|
||||
expect(updated).toBeGreaterThanOrEqual(initial);
|
||||
});
|
||||
|
||||
it("is a no-op for untracked tasks", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
// Should not throw
|
||||
detector.recordActivity("KB-999");
|
||||
expect(detector.getLastActivity("KB-999")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("isStuck", () => {
|
||||
it("returns true when elapsed time exceeds timeout", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Manually backdate the activity to simulate stagnation
|
||||
const entry = (detector as any).tracked.get("KB-001");
|
||||
entry.lastActivity = Date.now() - 600_001; // 10 minutes + 1ms
|
||||
|
||||
expect(detector.isStuck("KB-001", 600_000)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false when elapsed time is within timeout", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Just tracked — should not be stuck
|
||||
expect(detector.isStuck("KB-001", 600_000)).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for untracked tasks", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
expect(detector.isStuck("KB-999", 600_000)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("killAndRetry", () => {
|
||||
it("disposes the session and moves task to todo", async () => {
|
||||
const store = createMockStore();
|
||||
const onStuck = vi.fn();
|
||||
detector = new StuckTaskDetector(store, { onStuck });
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
await detector.killAndRetry("KB-001", 600_000);
|
||||
|
||||
// Session disposed
|
||||
expect(session.dispose).toHaveBeenCalledOnce();
|
||||
|
||||
// Task logged
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"KB-001",
|
||||
expect.stringContaining("stuck agent session"),
|
||||
);
|
||||
|
||||
// Task set to transient "stuck-killed" status then moved to todo
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: "stuck-killed" });
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
|
||||
// Callback invoked
|
||||
expect(onStuck).toHaveBeenCalledWith("KB-001");
|
||||
|
||||
// Task untracked
|
||||
expect(detector.trackedCount).toBe(0);
|
||||
});
|
||||
|
||||
it("is a no-op for untracked tasks", async () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
await detector.killAndRetry("KB-999", 600_000);
|
||||
|
||||
expect(store.logEntry).not.toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("sets transient stuck-killed status then moves to todo", async () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
await detector.killAndRetry("KB-001", 600_000);
|
||||
|
||||
// First updateTask sets "stuck-killed" (transient status)
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-001", { status: "stuck-killed" });
|
||||
// Then moveTask moves to "todo" — moveTask automatically clears status
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
// Only one updateTask call (the stuck-killed one) — no explicit clear needed
|
||||
expect(store.updateTask).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("preserves step progress — does not reset currentStep", async () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
await detector.killAndRetry("KB-001", 600_000);
|
||||
|
||||
// updateTask should NOT set currentStep or steps — they're preserved
|
||||
for (const call of store.updateTask.mock.calls) {
|
||||
const [, update] = call;
|
||||
expect(update).not.toHaveProperty("currentStep");
|
||||
expect(update).not.toHaveProperty("steps");
|
||||
}
|
||||
});
|
||||
|
||||
it("handles session dispose errors gracefully", async () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
session.dispose.mockImplementation(() => {
|
||||
throw new Error("Session already disposed");
|
||||
});
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Should not throw
|
||||
await detector.killAndRetry("KB-001", 600_000);
|
||||
|
||||
// Should still proceed to move task
|
||||
expect(store.moveTask).toHaveBeenCalledWith("KB-001", "todo");
|
||||
});
|
||||
});
|
||||
|
||||
describe("checkStuckTasks (via polling)", () => {
|
||||
it("kills only tasks exceeding the timeout", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: 60_000 });
|
||||
const onStuck = vi.fn();
|
||||
detector = new StuckTaskDetector(store, { onStuck });
|
||||
|
||||
const session1 = createMockSession();
|
||||
const session2 = createMockSession();
|
||||
|
||||
detector.trackTask("KB-001", session1);
|
||||
detector.trackTask("KB-002", session2);
|
||||
|
||||
// Backdate KB-001 to be stuck
|
||||
const entry1 = (detector as any).tracked.get("KB-001");
|
||||
entry1.lastActivity = Date.now() - 120_000; // 2 minutes ago (> 60s timeout)
|
||||
|
||||
// KB-002 is still recent (just tracked)
|
||||
|
||||
// Trigger check manually
|
||||
await (detector as any).checkStuckTasks();
|
||||
|
||||
// Only KB-001 should be killed
|
||||
expect(session1.dispose).toHaveBeenCalledOnce();
|
||||
expect(session2.dispose).not.toHaveBeenCalled();
|
||||
expect(onStuck).toHaveBeenCalledWith("KB-001");
|
||||
expect(onStuck).not.toHaveBeenCalledWith("KB-002");
|
||||
});
|
||||
|
||||
it("does nothing when taskStuckTimeoutMs is undefined (disabled)", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: undefined });
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Backdate to be "stuck"
|
||||
const entry = (detector as any).tracked.get("KB-001");
|
||||
entry.lastActivity = Date.now() - 9999_000;
|
||||
|
||||
await (detector as any).checkStuckTasks();
|
||||
|
||||
// Should not kill anything since feature is disabled
|
||||
expect(session.dispose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does nothing when taskStuckTimeoutMs is 0", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: 0 });
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
const entry = (detector as any).tracked.get("KB-001");
|
||||
entry.lastActivity = Date.now() - 9999_000;
|
||||
|
||||
await (detector as any).checkStuckTasks();
|
||||
|
||||
expect(session.dispose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does nothing when no tasks are tracked", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: 60_000 });
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
// Should not throw or call settings
|
||||
await (detector as any).checkStuckTasks();
|
||||
|
||||
expect(store.getSettings).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("respects dynamically changed timeout values", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: 300_000 }); // 5 minutes
|
||||
const onStuck = vi.fn();
|
||||
detector = new StuckTaskDetector(store, { onStuck });
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Backdate to 2 minutes ago
|
||||
const entry = (detector as any).tracked.get("KB-001");
|
||||
entry.lastActivity = Date.now() - 120_000;
|
||||
|
||||
// With 5-minute timeout, should not be stuck
|
||||
await (detector as any).checkStuckTasks();
|
||||
expect(onStuck).not.toHaveBeenCalled();
|
||||
|
||||
// Change settings to 1-minute timeout
|
||||
store._settings.taskStuckTimeoutMs = 60_000;
|
||||
|
||||
// Now it should be detected as stuck (120s > 60s)
|
||||
await (detector as any).checkStuckTasks();
|
||||
expect(onStuck).toHaveBeenCalledWith("KB-001");
|
||||
});
|
||||
});
|
||||
|
||||
describe("start / stop", () => {
|
||||
it("starts polling and can be stopped", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: 100 });
|
||||
detector = new StuckTaskDetector(store, { pollIntervalMs: 50 });
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Backdate to trigger stuck detection
|
||||
const entry = (detector as any).tracked.get("KB-001");
|
||||
entry.lastActivity = Date.now() - 200;
|
||||
|
||||
detector.start();
|
||||
|
||||
// Wait for at least one poll cycle
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
// Should have detected and killed the stuck task
|
||||
expect(session.dispose).toHaveBeenCalled();
|
||||
|
||||
detector.stop();
|
||||
});
|
||||
|
||||
it("stop prevents further checks", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: 100 });
|
||||
const onStuck = vi.fn();
|
||||
detector = new StuckTaskDetector(store, { pollIntervalMs: 30, onStuck });
|
||||
|
||||
detector.start();
|
||||
detector.stop();
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
const entry = (detector as any).tracked.get("KB-001");
|
||||
entry.lastActivity = Date.now() - 200;
|
||||
|
||||
// Wait to confirm no polling happens
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
expect(onStuck).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("start is idempotent", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store, { pollIntervalMs: 10_000 });
|
||||
|
||||
detector.start();
|
||||
detector.start(); // Should not create a second interval
|
||||
|
||||
// Just verify it doesn't throw
|
||||
detector.stop();
|
||||
});
|
||||
|
||||
it("stop is idempotent", () => {
|
||||
const store = createMockStore();
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
detector.stop(); // Not started — should be a no-op
|
||||
detector.start();
|
||||
detector.stop();
|
||||
detector.stop(); // Already stopped — should be a no-op
|
||||
});
|
||||
});
|
||||
|
||||
describe("edge cases", () => {
|
||||
it("handles getSettings failure gracefully during check", async () => {
|
||||
const store = createMockStore({ taskStuckTimeoutMs: 60_000 });
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Make getSettings throw
|
||||
store.getSettings.mockRejectedValueOnce(new Error("Store unavailable"));
|
||||
|
||||
// Should not throw
|
||||
await (detector as any).checkStuckTasks();
|
||||
|
||||
// Session should not be killed
|
||||
expect(session.dispose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("handles moveTask failure gracefully during killAndRetry", async () => {
|
||||
const store = createMockStore();
|
||||
store.moveTask.mockRejectedValueOnce(new Error("Invalid transition"));
|
||||
detector = new StuckTaskDetector(store);
|
||||
|
||||
const session = createMockSession();
|
||||
detector.trackTask("KB-001", session);
|
||||
|
||||
// Should not throw
|
||||
await detector.killAndRetry("KB-001", 600_000);
|
||||
|
||||
// Session should still be disposed
|
||||
expect(session.dispose).toHaveBeenCalled();
|
||||
// Task should be untracked even on error
|
||||
expect(detector.trackedCount).toBe(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
212
packages/engine/src/stuck-task-detector.ts
Normal file
212
packages/engine/src/stuck-task-detector.ts
Normal file
@@ -0,0 +1,212 @@
|
||||
/**
|
||||
* Stuck Task Detector — monitors in-progress tasks for agent session stagnation.
|
||||
*
|
||||
* When a task's agent session shows no activity (no text deltas, tool calls, or
|
||||
* progress updates) for longer than the configured timeout, the detector
|
||||
* terminates the stuck session and triggers recovery (moving the task back to
|
||||
* "todo" for the scheduler to retry).
|
||||
*
|
||||
* Activity is tracked via `recordActivity(taskId)` calls from the executor's
|
||||
* agent event handlers. The detector polls at a configurable interval and
|
||||
* compares the last activity timestamp against `taskStuckTimeoutMs` from settings.
|
||||
*/
|
||||
|
||||
import type { TaskStore, Settings } from "@kb/core";
|
||||
import { createLogger } from "./logger.js";
|
||||
|
||||
const stuckLog = createLogger("stuck-detector");
|
||||
|
||||
/** Minimal session interface — matches what TaskExecutor stores. */
|
||||
export interface DisposableSession {
|
||||
dispose: () => void;
|
||||
}
|
||||
|
||||
/** Tracked entry for a single in-progress task. */
|
||||
interface TrackedTask {
|
||||
session: DisposableSession;
|
||||
lastActivity: number;
|
||||
}
|
||||
|
||||
export interface StuckTaskDetectorOptions {
|
||||
/** Polling interval in milliseconds. Default: 30000 (30 seconds). */
|
||||
pollIntervalMs?: number;
|
||||
/** Callback invoked when a stuck task is detected and killed.
|
||||
* The task will be moved to "todo" for retry by the detector. */
|
||||
onStuck?: (taskId: string) => void;
|
||||
}
|
||||
|
||||
export class StuckTaskDetector {
|
||||
private tracked = new Map<string, TrackedTask>();
|
||||
private interval: ReturnType<typeof setInterval> | null = null;
|
||||
private pollIntervalMs: number;
|
||||
private onStuck?: (taskId: string) => void;
|
||||
|
||||
constructor(
|
||||
private store: TaskStore,
|
||||
options: StuckTaskDetectorOptions = {},
|
||||
) {
|
||||
this.pollIntervalMs = options.pollIntervalMs ?? 30_000;
|
||||
this.onStuck = options.onStuck;
|
||||
}
|
||||
|
||||
/**
|
||||
* Start the polling loop that checks for stuck tasks.
|
||||
* Safe to call multiple times (no-ops if already running).
|
||||
*/
|
||||
start(): void {
|
||||
if (this.interval) return;
|
||||
this.interval = setInterval(() => {
|
||||
this.checkStuckTasks().catch((err) => {
|
||||
stuckLog.error("Error checking stuck tasks:", err);
|
||||
});
|
||||
}, this.pollIntervalMs);
|
||||
stuckLog.log(`Started (poll interval: ${this.pollIntervalMs}ms)`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop the polling loop.
|
||||
* Does not untrack any tasks — just stops checking.
|
||||
*/
|
||||
stop(): void {
|
||||
if (this.interval) {
|
||||
clearInterval(this.interval);
|
||||
this.interval = null;
|
||||
stuckLog.log("Stopped");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Register an active agent session for monitoring.
|
||||
* Sets the initial activity timestamp to now.
|
||||
*/
|
||||
trackTask(taskId: string, session: DisposableSession): void {
|
||||
this.tracked.set(taskId, {
|
||||
session,
|
||||
lastActivity: Date.now(),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a task from monitoring.
|
||||
* Called when a task finishes (success, failure, or pause).
|
||||
*/
|
||||
untrackTask(taskId: string): void {
|
||||
this.tracked.delete(taskId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Record a heartbeat for a task's agent session.
|
||||
* Called on text deltas, tool calls, and progress updates.
|
||||
*/
|
||||
recordActivity(taskId: string): void {
|
||||
const entry = this.tracked.get(taskId);
|
||||
if (entry) {
|
||||
entry.lastActivity = Date.now();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the last activity timestamp for a tracked task.
|
||||
* Returns undefined if the task is not tracked.
|
||||
*/
|
||||
getLastActivity(taskId: string): number | undefined {
|
||||
return this.tracked.get(taskId)?.lastActivity;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a task is stuck (no activity for longer than timeout).
|
||||
*/
|
||||
isStuck(taskId: string, timeoutMs: number): boolean {
|
||||
const entry = this.tracked.get(taskId);
|
||||
if (!entry) return false;
|
||||
return (Date.now() - entry.lastActivity) > timeoutMs;
|
||||
}
|
||||
|
||||
/**
|
||||
* Terminate a stuck task's agent session and trigger recovery.
|
||||
* - Disposes the agent session
|
||||
* - Logs the stuck event to the task log
|
||||
* - Moves the task back to "todo" (preserving step progress)
|
||||
* - Invokes the onStuck callback
|
||||
*/
|
||||
async killAndRetry(taskId: string, timeoutMs: number): Promise<void> {
|
||||
const entry = this.tracked.get(taskId);
|
||||
if (!entry) return;
|
||||
|
||||
const elapsedMin = Math.round((Date.now() - entry.lastActivity) / 60_000);
|
||||
|
||||
stuckLog.log(`Killing stuck task ${taskId} (no activity for ~${elapsedMin} minutes)`);
|
||||
|
||||
// Dispose the agent session first
|
||||
try {
|
||||
entry.session.dispose();
|
||||
} catch (err) {
|
||||
stuckLog.error(`Failed to dispose session for ${taskId}:`, err);
|
||||
}
|
||||
|
||||
// Remove from tracking
|
||||
this.tracked.delete(taskId);
|
||||
|
||||
// Log the event to the task log
|
||||
try {
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
`Task terminated due to stuck agent session (no activity for ~${elapsedMin} minutes)`,
|
||||
);
|
||||
} catch (err) {
|
||||
stuckLog.error(`Failed to log stuck event for ${taskId}:`, err);
|
||||
}
|
||||
|
||||
// Set transient "stuck-killed" status, then move to "todo" for retry.
|
||||
// moveTask from "in-progress" to "todo" automatically clears status,
|
||||
// so no explicit status clear is needed after the move.
|
||||
// currentStep and step statuses are preserved so execution resumes where it left off.
|
||||
try {
|
||||
await this.store.updateTask(taskId, { status: "stuck-killed" });
|
||||
await this.store.moveTask(taskId, "todo");
|
||||
stuckLog.log(`${taskId} moved to todo for retry`);
|
||||
} catch (err) {
|
||||
stuckLog.error(`Failed to move ${taskId} to todo:`, err);
|
||||
}
|
||||
|
||||
// Notify listeners
|
||||
this.onStuck?.(taskId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Poll all tracked tasks and kill any that have exceeded the timeout.
|
||||
* Reads `taskStuckTimeoutMs` from settings on each check so changes
|
||||
* take effect on the next poll cycle.
|
||||
*/
|
||||
private async checkStuckTasks(): Promise<void> {
|
||||
if (this.tracked.size === 0) return;
|
||||
|
||||
let settings: Settings;
|
||||
try {
|
||||
settings = await this.store.getSettings();
|
||||
} catch {
|
||||
return; // Can't read settings — skip this cycle
|
||||
}
|
||||
|
||||
const timeoutMs = settings.taskStuckTimeoutMs;
|
||||
if (!timeoutMs || timeoutMs <= 0) return; // Disabled
|
||||
|
||||
const now = Date.now();
|
||||
const stuckTasks: string[] = [];
|
||||
|
||||
for (const [taskId, entry] of this.tracked) {
|
||||
if ((now - entry.lastActivity) > timeoutMs) {
|
||||
stuckTasks.push(taskId);
|
||||
}
|
||||
}
|
||||
|
||||
for (const taskId of stuckTasks) {
|
||||
await this.killAndRetry(taskId, timeoutMs);
|
||||
}
|
||||
}
|
||||
|
||||
/** Number of currently tracked tasks (for testing). */
|
||||
get trackedCount(): number {
|
||||
return this.tracked.size;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user