fix(engine): cap verification-failure bounces, reap unregistered worktrees, dedupe activity log
Three fixes for the worktree-overflow / stuck-task incident: 1. Cap deterministic-verification-failure bounces (fix #2) Auto-merge previously bounced an in-review task back to in-progress on every verification failure with no upper bound. A single flaky test could keep a task ping-ponging in-review→in-progress forever, holding its worktree and consuming agent slots. Adds verificationFailureCount on Task (DB migration v48), increments on each bounce, and after 3 failures marks the task failed and creates a follow-up triage task so a fresh agent can investigate the underlying flake instead of re-running the same fix loop. 2. Reap unregistered orphan worktree dirs even when recycle is on (fix #3) cleanupOrphans previously bailed out entirely when recycleWorktrees was true, leaving stale dirs (clear-hawk-broken, *-bak, leftover crash debris) on disk forever. New reapUnregisteredOrphans pass removes only directories that aren't registered git worktrees, so the recycle pool keeps its warm worktrees but the trash gets cleared. 3. Idempotence guard on activity-log listener wiring (fix #6) setupActivityLogListeners() was registering handlers on every call. When init() ran twice, every task:created / task:moved event wrote N rows to activityLog, producing the duplicate entries visible in the DB. Added activityListenersWired flag so repeated calls no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -131,7 +131,7 @@ describe("Database", () => {
|
||||
});
|
||||
|
||||
it("seeds schema version", () => {
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
});
|
||||
|
||||
it("seeds lastModified", () => {
|
||||
@@ -154,7 +154,7 @@ describe("Database", () => {
|
||||
|
||||
it("is idempotent - calling init() twice does not fail", () => {
|
||||
expect(() => db.init()).not.toThrow();
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
});
|
||||
|
||||
it("does not overwrite existing config on re-init", () => {
|
||||
@@ -761,7 +761,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29 (includes v1→v2 through v26→v29)
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -786,11 +786,11 @@ describe("schema migrations", () => {
|
||||
const db = new Database(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
// Re-init should not fail
|
||||
db.init();
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -825,7 +825,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("priority");
|
||||
@@ -866,7 +866,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -935,7 +935,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -976,7 +976,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "agentRatings" }]);
|
||||
@@ -1000,7 +1000,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "mission_events" }]);
|
||||
@@ -1104,7 +1104,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -1473,7 +1473,7 @@ describe("createDatabase factory", () => {
|
||||
const db = createDatabase(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
expect(db.getLastModified()).toBeGreaterThan(0);
|
||||
|
||||
db.close();
|
||||
|
||||
@@ -779,7 +779,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
// Step 1: Create a fresh database at v33 (runs all migrations up to 33)
|
||||
const db1 = createDatabase(legacyDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(47);
|
||||
expect(db1.getSchemaVersion()).toBe(48);
|
||||
db1.close();
|
||||
|
||||
// Step 2: Manually downgrade to version 32 and drop insight tables
|
||||
@@ -814,7 +814,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
expect(tableNamesBefore).not.toContain("project_insight_runs");
|
||||
// Now run init — this triggers the v32→v33 migration
|
||||
db3.init();
|
||||
expect(db3.getSchemaVersion()).toBe(47);
|
||||
expect(db3.getSchemaVersion()).toBe(48);
|
||||
|
||||
// Step 4: Verify insight tables exist after migration
|
||||
const tablesAfter = db3.prepare(
|
||||
@@ -845,12 +845,12 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
try {
|
||||
const db1 = createDatabase(testDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(47);
|
||||
expect(db1.getSchemaVersion()).toBe(48);
|
||||
db1.close();
|
||||
|
||||
const db2 = createDatabase(testDir);
|
||||
expect(() => db2.init()).not.toThrow();
|
||||
expect(db2.getSchemaVersion()).toBe(47);
|
||||
expect(db2.getSchemaVersion()).toBe(48);
|
||||
db2.close();
|
||||
} finally {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
|
||||
@@ -2629,7 +2629,7 @@ describe("MissionStore", () => {
|
||||
|
||||
describe("Loop State & Validator Run Schema (v31)", () => {
|
||||
it("schema version is 40 after migration", () => {
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
});
|
||||
|
||||
it("mission_features table has loop state columns", () => {
|
||||
|
||||
@@ -742,7 +742,7 @@ describe("RoadmapStore", () => {
|
||||
|
||||
describe("schema version", () => {
|
||||
it("schema version is 40 after init", () => {
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -465,7 +465,7 @@ describe("Run Audit", () => {
|
||||
});
|
||||
|
||||
it("schema version is bumped to 40", () => {
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -51,7 +51,7 @@ describe("TaskStore task documents", () => {
|
||||
|
||||
expect(tableNames.has("task_documents")).toBe(true);
|
||||
expect(tableNames.has("task_document_revisions")).toBe(true);
|
||||
expect(db.getSchemaVersion()).toBe(47);
|
||||
expect(db.getSchemaVersion()).toBe(48);
|
||||
|
||||
const index = db
|
||||
.prepare(
|
||||
|
||||
@@ -1836,6 +1836,15 @@ export class Database {
|
||||
});
|
||||
}
|
||||
|
||||
// Outer verification-failure bounce counter — counts in-review→in-progress
|
||||
// returns triggered by VerificationError. Capped to prevent infinite
|
||||
// re-merge loops on flaky tests (see project-engine.ts auto-merge handler).
|
||||
if (version < 48) {
|
||||
this.applyMigration(48, () => {
|
||||
this.addColumnIfMissing("tasks", "verificationFailureCount", "INTEGER DEFAULT 0");
|
||||
});
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -52,6 +52,7 @@ interface TaskRow {
|
||||
postReviewFixCount: number | null;
|
||||
recoveryRetryCount: number | null;
|
||||
taskDoneRetryCount: number | null;
|
||||
verificationFailureCount: number | null;
|
||||
nextRecoveryAt: string | null;
|
||||
error: string | null;
|
||||
summary: string | null;
|
||||
@@ -350,6 +351,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
private configPath: string;
|
||||
/** SQLite database for structured data storage */
|
||||
private _db: Database | null = null;
|
||||
private activityListenersWired = false;
|
||||
/** Separate SQLite database for compact archived task snapshots. */
|
||||
private _archiveDb: ArchiveDatabase | null = null;
|
||||
|
||||
@@ -530,6 +532,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
postReviewFixCount: row.postReviewFixCount ?? undefined,
|
||||
recoveryRetryCount: row.recoveryRetryCount ?? undefined,
|
||||
taskDoneRetryCount: row.taskDoneRetryCount ?? undefined,
|
||||
verificationFailureCount: row.verificationFailureCount ?? undefined,
|
||||
nextRecoveryAt: row.nextRecoveryAt || undefined,
|
||||
error: row.error || undefined,
|
||||
summary: row.summary || undefined,
|
||||
@@ -823,7 +826,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"modelPresetId", "modelProvider", "modelId",
|
||||
"validatorModelProvider", "validatorModelId",
|
||||
"planningModelProvider", "planningModelId",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "nextRecoveryAt",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "verificationFailureCount", "nextRecoveryAt",
|
||||
"error", "summary", "thinkingLevel", "executionMode",
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt",
|
||||
"createdAt", "updatedAt", "columnMovedAt",
|
||||
@@ -842,7 +845,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"modelPresetId", "modelProvider", "modelId",
|
||||
"validatorModelProvider", "validatorModelId",
|
||||
"planningModelProvider", "planningModelId",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "nextRecoveryAt",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "verificationFailureCount", "nextRecoveryAt",
|
||||
"error", "summary", "thinkingLevel", "executionMode",
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt",
|
||||
"createdAt", "updatedAt", "columnMovedAt",
|
||||
@@ -884,7 +887,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
id, title, description, priority, "column", status, size, reviewLevel, currentStep,
|
||||
worktree, blockedBy, paused, baseBranch, branch, baseCommitSha, modelPresetId, modelProvider,
|
||||
modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, mergeRetries,
|
||||
workflowStepRetries, stuckKillCount, postReviewFixCount, recoveryRetryCount, taskDoneRetryCount, nextRecoveryAt, error,
|
||||
workflowStepRetries, stuckKillCount, postReviewFixCount, recoveryRetryCount, taskDoneRetryCount, verificationFailureCount, nextRecoveryAt, error,
|
||||
summary, thinkingLevel, executionMode, tokenUsageInputTokens, tokenUsageOutputTokens, tokenUsageCachedTokens,
|
||||
tokenUsageTotalTokens, tokenUsageFirstUsedAt, tokenUsageLastUsedAt, createdAt, updatedAt, columnMovedAt,
|
||||
dependencies, steps, log, attachments, steeringComments,
|
||||
@@ -892,7 +895,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
sourceIssueProvider, sourceIssueRepository, sourceIssueExternalIssueId, sourceIssueNumber, sourceIssueUrl,
|
||||
mergeDetails, breakIntoSubtasks, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, assignedAgentId, assigneeUserId, checkedOutBy, checkedOutAt
|
||||
) VALUES (
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
)
|
||||
ON CONFLICT(id) DO UPDATE SET
|
||||
title = excluded.title,
|
||||
@@ -922,6 +925,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
postReviewFixCount = excluded.postReviewFixCount,
|
||||
recoveryRetryCount = excluded.recoveryRetryCount,
|
||||
taskDoneRetryCount = excluded.taskDoneRetryCount,
|
||||
verificationFailureCount = excluded.verificationFailureCount,
|
||||
nextRecoveryAt = excluded.nextRecoveryAt,
|
||||
error = excluded.error,
|
||||
summary = excluded.summary,
|
||||
@@ -989,6 +993,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
task.postReviewFixCount ?? 0,
|
||||
task.recoveryRetryCount ?? null,
|
||||
task.taskDoneRetryCount ?? 0,
|
||||
task.verificationFailureCount ?? 0,
|
||||
task.nextRecoveryAt ?? null,
|
||||
task.error ?? null,
|
||||
task.summary ?? null,
|
||||
@@ -1073,8 +1078,15 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
/**
|
||||
* Set up event listeners for activity logging.
|
||||
* Call after init() to record task lifecycle events.
|
||||
*
|
||||
* Idempotent — repeated calls are no-ops. Without this guard, each duplicate
|
||||
* call double-registers handlers, causing the activity log to record every
|
||||
* `task:created` / `task:moved` event N times where N = number of init() calls.
|
||||
*/
|
||||
private setupActivityLogListeners(): void {
|
||||
if (this.activityListenersWired) return;
|
||||
this.activityListenersWired = true;
|
||||
|
||||
// Task created
|
||||
this.on("task:created", (task) => {
|
||||
this.recordActivityFromListener(
|
||||
@@ -2553,7 +2565,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
async updateTask(
|
||||
id: string,
|
||||
updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; currentStep?: number; blockedBy?: string | null; assignedAgentId?: string | null; assigneeUserId?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
|
||||
updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; currentStep?: number; blockedBy?: string | null; assignedAgentId?: string | null; assigneeUserId?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; verificationFailureCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
|
||||
runContext?: RunMutationContext,
|
||||
): Promise<Task> {
|
||||
return this.withTaskLock(id, async () => {
|
||||
@@ -2674,6 +2686,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
} else if (updates.taskDoneRetryCount !== undefined) {
|
||||
task.taskDoneRetryCount = updates.taskDoneRetryCount;
|
||||
}
|
||||
if (updates.verificationFailureCount === null) {
|
||||
task.verificationFailureCount = undefined;
|
||||
} else if (updates.verificationFailureCount !== undefined) {
|
||||
task.verificationFailureCount = updates.verificationFailureCount;
|
||||
}
|
||||
if (updates.nextRecoveryAt === null) {
|
||||
task.nextRecoveryAt = undefined;
|
||||
} else if (updates.nextRecoveryAt !== undefined) {
|
||||
|
||||
@@ -815,6 +815,13 @@ export interface Task {
|
||||
* failures. Capped by `MAX_TASK_DONE_RETRIES`; when exhausted the task stays
|
||||
* in `in-review` for human inspection. Cleared on successful completion. */
|
||||
taskDoneRetryCount?: number;
|
||||
/** Number of times this task has bounced from `in-review` back to `in-progress`
|
||||
* due to a deterministic verification failure during auto-merge. Incremented
|
||||
* by the auto-merge error handler (project-engine.ts). When this reaches
|
||||
* `MAX_VERIFICATION_FAILURE_BOUNCES`, the task is marked failed and a
|
||||
* follow-up triage task is created so a human / fresh agent can investigate
|
||||
* rather than endlessly re-attempting the same fix. */
|
||||
verificationFailureCount?: number;
|
||||
/** ISO-8601 timestamp indicating when the task becomes eligible for the next
|
||||
* recovery retry. Scheduler and triage processor skip tasks whose
|
||||
* `nextRecoveryAt` is still in the future. Cleared alongside `recoveryRetryCount`. */
|
||||
|
||||
Reference in New Issue
Block a user