fix(FN-4811): persist done-task integrity warnings across engine restarts
The periodic self-healing sweep at
SelfHealingManager.reconcileDoneTaskIntegrity() emits a single
'Integrity warning: done-task finalize evidence is unproven (<reason>)' log
entry per task when the task is in 'done' but has no provable on-main
evidence. Dedup was via an in-memory finalizeUnprovenWarned Set per manager
instance, so every engine restart resurfaced the same warning on the next
sweep — significant noise on done tasks that legitimately lack evidence,
typically residue of FN-4811 contamination (FN-4771/FN-4778 in production).
Adds an optional MergeDetails.integrityWarning = { warnedAt, reason } field
and persists it on the first warning. Both warning sites in
reconcileDoneTaskIntegrity() (the unproven-and-still-mergeable branch and
the unproven-final branch) now consult the persisted record:
- Same reason as persisted → skip re-emitting, just rehydrate the in-memory
Set for in-process consistency.
- Different reason → re-warn (so a *new* classification problem still
surfaces) and update the persisted record.
Tests added under
packages/engine/src/__tests__/reliability-interactions/integrity-warning-persisted-dedup.test.ts
(real-git, 4 cases):
- First sweep: emits warning + persists record.
- Second sweep, same instance: in-memory Set dedupes (existing contract).
- Fresh manager (simulated engine restart) + pre-persisted record:
persisted dedup suppresses re-emission.
- Fresh manager + persisted record with different reason: must re-warn
and overwrite the persisted reason.
Full engine suite: 308 files, 5041 tests pass, 1 skipped. Lint clean.
Fusion-Task-Id: FN-4811
This commit is contained in:
@@ -0,0 +1,223 @@
|
||||
/**
|
||||
* FN-4811 follow-up: persisted dedup of done-task finalize-integrity warnings.
|
||||
*
|
||||
* Before this change, `SelfHealingManager.reconcileDoneTaskIntegrity()` deduped
|
||||
* warnings via an in-memory `finalizeUnprovenWarned: Set<string>` per instance.
|
||||
* Every engine restart created a fresh instance, so the periodic sweep re-emitted
|
||||
* the same "Integrity warning: done-task finalize evidence is unproven" log entry
|
||||
* for the same task every cycle — significant log noise on done tasks that
|
||||
* legitimately had no on-main evidence (often residue of FN-4811 contamination).
|
||||
*
|
||||
* The fix persists the first-warning state on `task.mergeDetails.integrityWarning`
|
||||
* (warnedAt + reason) and the sweep checks that field before emitting again.
|
||||
*
|
||||
* This is the canonical real-git regression backstop for the persisted dedup contract.
|
||||
*/
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { execSync } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { Settings, Task, TaskStore } from "@fusion/core";
|
||||
import { SelfHealingManager } from "../../self-healing.js";
|
||||
|
||||
function git(dir: string, cmd: string): string {
|
||||
return execSync(cmd, { cwd: dir, stdio: "pipe" }).toString().trim();
|
||||
}
|
||||
|
||||
function makeStore(task: Task, events: unknown[] = []): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter();
|
||||
const mergedSettings = {
|
||||
autoMerge: true,
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
mergeStrategy: "direct",
|
||||
directMergeCommitStrategy: "auto",
|
||||
includeTaskIdInCommit: false,
|
||||
commitAuthorEnabled: false,
|
||||
useAiMergeCommitSummary: false,
|
||||
baseBranch: "main",
|
||||
} as Settings;
|
||||
return Object.assign(emitter, {
|
||||
getSettings: vi.fn(async () => mergedSettings),
|
||||
getTask: vi.fn(async () => task),
|
||||
listTasks: vi.fn(async ({ column }: { column?: string } = {}) =>
|
||||
column ? [task].filter((t) => t.column === column) : [task],
|
||||
),
|
||||
updateTask: vi.fn(async (_id: string, updates: Partial<Task>) => Object.assign(task, updates)),
|
||||
moveTask: vi.fn(async (_id: string, column: Task["column"]) => {
|
||||
task.column = column;
|
||||
return task;
|
||||
}),
|
||||
logEntry: vi.fn(async () => undefined),
|
||||
appendAgentLog: vi.fn(async () => undefined),
|
||||
updateSettings: vi.fn(async () => mergedSettings),
|
||||
clearStaleExecutionStartBranchReferences: vi.fn(() => []),
|
||||
recordRunAuditEvent: vi.fn(async (event: unknown) => {
|
||||
events.push(event);
|
||||
}),
|
||||
walCheckpoint: vi.fn(() => ({ busy: 0, log: 0, checkpointed: 0 })),
|
||||
archiveTaskAndCleanup: vi.fn(async () => ({})),
|
||||
mergeTask: vi.fn(async () => undefined),
|
||||
getRootDir: vi.fn(() => ""),
|
||||
}) as unknown as TaskStore & EventEmitter;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set up a real git repo where the task branch has only foreign-attributed commits,
|
||||
* so `classifyOwnedLandedEvidence` returns `kind: "unproven"` with reason
|
||||
* `"no-owned-commit-foreign-deltas"` — the canonical FN-4771/FN-4778 production shape.
|
||||
*/
|
||||
function setupForeignOnlyRepo(dir: string, taskId: string): void {
|
||||
git(dir, "git init -b main");
|
||||
git(dir, 'git config user.email "test@example.com"');
|
||||
git(dir, 'git config user.name "Test"');
|
||||
git(dir, "git commit --allow-empty -m init");
|
||||
git(dir, `git checkout -b fusion/${taskId.toLowerCase()}`);
|
||||
writeFileSync(join(dir, "foreign.txt"), "from foreign\n");
|
||||
git(dir, "git add foreign.txt");
|
||||
git(dir, "git commit -m 'feat(FN-OTHER): foreign' -m 'Fusion-Task-Id: FN-OTHER'");
|
||||
git(dir, "git checkout main");
|
||||
}
|
||||
|
||||
function makeUnprovenDoneTask(taskId: string): Task {
|
||||
return {
|
||||
id: taskId,
|
||||
title: "test",
|
||||
description: "test",
|
||||
column: "done",
|
||||
branch: `fusion/${taskId.toLowerCase()}`,
|
||||
baseBranch: "main",
|
||||
modifiedFiles: ["some/file.ts"],
|
||||
mergeDetails: {},
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as Task;
|
||||
}
|
||||
|
||||
describe("FN-4811 follow-up: integrity warning dedup persists across restarts", () => {
|
||||
it("emits the unproven warning once, persists it on mergeDetails.integrityWarning", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "fn-4811-dedup-first-"));
|
||||
try {
|
||||
const taskId = "FN-DEDUP-1";
|
||||
setupForeignOnlyRepo(dir, taskId);
|
||||
const task = makeUnprovenDoneTask(taskId);
|
||||
|
||||
const store = makeStore(task);
|
||||
const manager = new SelfHealingManager(store, { rootDir: dir });
|
||||
|
||||
await manager.reconcileDoneTaskIntegrity();
|
||||
|
||||
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const warningEmitted = logCalls.some((c: any[]) =>
|
||||
/Integrity warning: done-task finalize evidence is unproven/.test(String(c[1] ?? "")),
|
||||
);
|
||||
expect(warningEmitted).toBe(true);
|
||||
|
||||
// Persisted record must be present.
|
||||
expect(task.mergeDetails?.integrityWarning).toBeDefined();
|
||||
expect(task.mergeDetails?.integrityWarning?.reason).toBe("no-owned-commit-foreign-deltas");
|
||||
expect(typeof task.mergeDetails?.integrityWarning?.warnedAt).toBe("string");
|
||||
|
||||
manager.stop();
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does NOT re-emit the warning on a second sweep within the same instance", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "fn-4811-dedup-same-"));
|
||||
try {
|
||||
const taskId = "FN-DEDUP-2";
|
||||
setupForeignOnlyRepo(dir, taskId);
|
||||
const task = makeUnprovenDoneTask(taskId);
|
||||
|
||||
const store = makeStore(task);
|
||||
const manager = new SelfHealingManager(store, { rootDir: dir });
|
||||
|
||||
await manager.reconcileDoneTaskIntegrity();
|
||||
(store.logEntry as ReturnType<typeof vi.fn>).mockClear();
|
||||
await manager.reconcileDoneTaskIntegrity();
|
||||
|
||||
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const reWarned = logCalls.some((c: any[]) =>
|
||||
/Integrity warning: done-task finalize evidence is unproven/.test(String(c[1] ?? "")),
|
||||
);
|
||||
expect(reWarned).toBe(false);
|
||||
|
||||
manager.stop();
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does NOT re-emit the warning across a simulated engine restart (persisted dedup)", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "fn-4811-dedup-restart-"));
|
||||
try {
|
||||
const taskId = "FN-DEDUP-3";
|
||||
setupForeignOnlyRepo(dir, taskId);
|
||||
// Seed task with the warning persisted from a prior session.
|
||||
const task = makeUnprovenDoneTask(taskId);
|
||||
task.mergeDetails = {
|
||||
integrityWarning: {
|
||||
warnedAt: "2026-05-16T22:00:00.000Z",
|
||||
reason: "no-owned-commit-foreign-deltas",
|
||||
},
|
||||
};
|
||||
|
||||
const store = makeStore(task);
|
||||
// Fresh manager == fresh in-memory dedup Set, simulating restart.
|
||||
const manager = new SelfHealingManager(store, { rootDir: dir });
|
||||
|
||||
await manager.reconcileDoneTaskIntegrity();
|
||||
|
||||
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const warned = logCalls.some((c: any[]) =>
|
||||
/Integrity warning: done-task finalize evidence is unproven/.test(String(c[1] ?? "")),
|
||||
);
|
||||
expect(warned).toBe(false);
|
||||
|
||||
manager.stop();
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("DOES re-emit the warning if the classification reason changes (different problem)", async () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "fn-4811-dedup-reason-"));
|
||||
try {
|
||||
const taskId = "FN-DEDUP-4";
|
||||
setupForeignOnlyRepo(dir, taskId);
|
||||
const task = makeUnprovenDoneTask(taskId);
|
||||
// Seed a persisted warning with a DIFFERENT reason than the current classification.
|
||||
task.mergeDetails = {
|
||||
integrityWarning: {
|
||||
warnedAt: "2026-05-16T22:00:00.000Z",
|
||||
reason: "missing-evidence",
|
||||
},
|
||||
};
|
||||
|
||||
const store = makeStore(task);
|
||||
const manager = new SelfHealingManager(store, { rootDir: dir });
|
||||
|
||||
await manager.reconcileDoneTaskIntegrity();
|
||||
|
||||
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const warned = logCalls.some((c: any[]) =>
|
||||
/Integrity warning: done-task finalize evidence is unproven/.test(String(c[1] ?? "")),
|
||||
);
|
||||
expect(warned).toBe(true);
|
||||
// Persisted record updated to the new reason.
|
||||
expect(task.mergeDetails?.integrityWarning?.reason).toBe("no-owned-commit-foreign-deltas");
|
||||
|
||||
manager.stop();
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -2241,13 +2241,33 @@ export class SelfHealingManager {
|
||||
const classification = await classifyOwnedLandedEvidenceForSelfHealing(this.options.rootDir, task, ahead.baseRef);
|
||||
|
||||
if (classification.kind === "unproven") {
|
||||
if (!this.finalizeUnprovenWarned.has(task.id)) {
|
||||
// FN-4811 follow-up: dedupe across engine restarts. The in-memory Set only
|
||||
// dedupes within one process; persist the first-warning state on
|
||||
// mergeDetails.integrityWarning so subsequent sweep runs (after restart) skip
|
||||
// re-emitting an identical log entry.
|
||||
const alreadyWarned =
|
||||
this.finalizeUnprovenWarned.has(task.id) ||
|
||||
(task.mergeDetails?.integrityWarning?.reason === classification.reason);
|
||||
if (!alreadyWarned) {
|
||||
this.finalizeUnprovenWarned.add(task.id);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Finalize blocked: unproven ownership evidence (${classification.reason}); no owned landed commit was found — auto-retrying via todo requeue`,
|
||||
JSON.stringify(classification.details, null, 2),
|
||||
);
|
||||
await this.store.updateTask(task.id, {
|
||||
mergeDetails: {
|
||||
...(task.mergeDetails || {}),
|
||||
integrityWarning: {
|
||||
warnedAt: new Date().toISOString(),
|
||||
reason: classification.reason,
|
||||
},
|
||||
},
|
||||
});
|
||||
} else {
|
||||
// Hydrate the in-memory dedup Set from the persisted record so subsequent
|
||||
// checks in this process don't have to re-query the task.
|
||||
this.finalizeUnprovenWarned.add(task.id);
|
||||
}
|
||||
await this.recordIntegrityAudit(task.id, "task:finalize-unproven-blocked", {
|
||||
reason: classification.reason,
|
||||
@@ -2388,13 +2408,28 @@ export class SelfHealingManager {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!this.finalizeUnprovenWarned.has(task.id)) {
|
||||
// FN-4811 follow-up: dedupe across engine restarts (see matching block above).
|
||||
const alreadyWarned =
|
||||
this.finalizeUnprovenWarned.has(task.id) ||
|
||||
(task.mergeDetails?.integrityWarning?.reason === classification.reason);
|
||||
if (!alreadyWarned) {
|
||||
this.finalizeUnprovenWarned.add(task.id);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Integrity warning: done-task finalize evidence is unproven (${classification.reason})`,
|
||||
JSON.stringify(classification.details, null, 2),
|
||||
);
|
||||
await this.store.updateTask(task.id, {
|
||||
mergeDetails: {
|
||||
...(task.mergeDetails || {}),
|
||||
integrityWarning: {
|
||||
warnedAt: new Date().toISOString(),
|
||||
reason: classification.reason,
|
||||
},
|
||||
},
|
||||
});
|
||||
} else {
|
||||
this.finalizeUnprovenWarned.add(task.id);
|
||||
}
|
||||
await this.recordIntegrityAudit(task.id, "task:integrity-warning", {
|
||||
reason: classification.reason,
|
||||
|
||||
Reference in New Issue
Block a user