feat(FN-2089): merge fusion/fn-2089
This commit is contained in:
@@ -1337,7 +1337,12 @@ export class HeartbeatMonitor {
|
||||
try { this.untrackAgent(agentId); } catch (untrackErr) {
|
||||
heartbeatLog.warn(`untrackAgent failed for ${agentId}: ${untrackErr instanceof Error ? untrackErr.message : String(untrackErr)}`);
|
||||
}
|
||||
try { session.dispose(); } catch { /* ignore */ }
|
||||
try {
|
||||
session.dispose();
|
||||
} catch (disposeErr: unknown) {
|
||||
const errorMessage = disposeErr instanceof Error ? disposeErr.message : String(disposeErr);
|
||||
heartbeatLog.warn(`session.dispose() failed for ${agentId}: ${errorMessage}`);
|
||||
}
|
||||
}
|
||||
|
||||
return (await this.store.getRunDetail(agentId, run.id))!;
|
||||
|
||||
@@ -36,6 +36,14 @@ vi.mock("./pi.js", () => {
|
||||
return { createKbAgent, promptWithFallback };
|
||||
});
|
||||
|
||||
vi.mock("./logger.js", () => ({
|
||||
createLogger: vi.fn((_name: string) => ({
|
||||
log: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
})),
|
||||
}));
|
||||
|
||||
// Helper to reset mock session state
|
||||
function resetMockSession() {
|
||||
mockSessionHolder.session.state.messages = [];
|
||||
@@ -43,7 +51,7 @@ function resetMockSession() {
|
||||
}
|
||||
|
||||
// Import AFTER vi.mock so the mock is applied
|
||||
import { MissionExecutionLoop } from "./mission-execution-loop.js";
|
||||
import { MissionExecutionLoop, loopLog } from "./mission-execution-loop.js";
|
||||
|
||||
// ── Mock Factories ──────────────────────────────────────────────────────────
|
||||
|
||||
@@ -594,6 +602,31 @@ describe("MissionExecutionLoop", () => {
|
||||
await expect(loop.recoverActiveMissions()).resolves.not.toThrow();
|
||||
});
|
||||
|
||||
it("logs warn when mission hierarchy lookup throws during recovery", async () => {
|
||||
const mission = createMockMission({ id: "M-LOOKUP", status: "active" });
|
||||
missionStore._setMission(mission);
|
||||
missionStore.getMissionWithHierarchy = vi.fn().mockImplementation(() => {
|
||||
throw new Error("Database error");
|
||||
});
|
||||
|
||||
vi.mocked(loopLog.warn).mockClear();
|
||||
|
||||
loop = new MissionExecutionLoop({
|
||||
taskStore: taskStore as any,
|
||||
missionStore: missionStore as any,
|
||||
rootDir: "/tmp",
|
||||
});
|
||||
loop.start();
|
||||
|
||||
await loop.recoverActiveMissions();
|
||||
|
||||
expect(loopLog.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
"getMissionWithHierarchy failed for mission M-LOOKUP: Database error",
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it("should handle empty hierarchy gracefully", async () => {
|
||||
const mission = createMockMission({ status: "active" });
|
||||
missionStore._setMission(mission);
|
||||
|
||||
@@ -139,7 +139,9 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
let hierarchy;
|
||||
try {
|
||||
hierarchy = this.missionStore.getMissionWithHierarchy(mission.id);
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
loopLog.warn(`getMissionWithHierarchy failed for mission ${mission.id}: ${errorMessage} — skipping`);
|
||||
// Database error, skip this mission
|
||||
continue;
|
||||
}
|
||||
@@ -424,7 +426,7 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
try {
|
||||
parsed = JSON.parse(jsonCandidate);
|
||||
} catch {
|
||||
// Try to repair common JSON issues
|
||||
// Intentional fallback: initial parse can fail on malformed JSON; try repairJson() next.
|
||||
const repaired = this.repairJson(jsonCandidate);
|
||||
try {
|
||||
parsed = JSON.parse(repaired);
|
||||
@@ -493,7 +495,9 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
}
|
||||
|
||||
return undefined;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
loopLog.warn(`AI response JSON extraction failed: ${errorMessage}`);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import { AgentSemaphore } from "./concurrency.js";
|
||||
import type { TaskStore, Task, TaskDetail } from "@fusion/core";
|
||||
import { existsSync } from "node:fs";
|
||||
import { readFile } from "node:fs/promises";
|
||||
import { schedulerLog } from "./logger.js";
|
||||
|
||||
// Mock fs modules
|
||||
vi.mock("node:fs", async (importOriginal) => {
|
||||
@@ -23,6 +24,14 @@ vi.mock("node:fs/promises", async (importOriginal) => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("./logger.js", () => ({
|
||||
schedulerLog: {
|
||||
log: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
// Helper to create mock tasks
|
||||
function createMockTask(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
@@ -834,6 +843,26 @@ describe("Scheduler", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("logs warn when PROMPT.md read throws during validation", async () => {
|
||||
const store = createMockStore({
|
||||
getTasksDir: vi.fn().mockReturnValue("/test/project/.fusion/tasks"),
|
||||
});
|
||||
const scheduler = new Scheduler(store);
|
||||
|
||||
vi.mocked(schedulerLog.warn).mockClear();
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockRejectedValue(new Error("EACCES"));
|
||||
|
||||
const validation = await (scheduler as any).validateTaskFilesystem("FN-READ");
|
||||
|
||||
expect(validation).toEqual({ valid: false, reason: "missing or empty PROMPT.md" });
|
||||
expect(schedulerLog.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
"PROMPT.md read failed for task dispatch validation (FN-READ): EACCES",
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
it("proceeds with scheduling when filesystem is valid", async () => {
|
||||
const tasks = [
|
||||
createMockTask({ id: "FN-004", column: "todo", dependencies: [] }),
|
||||
|
||||
@@ -295,7 +295,9 @@ export class Scheduler {
|
||||
if (!content || content.trim().length === 0) {
|
||||
return { valid: false, reason: "missing or empty PROMPT.md" };
|
||||
}
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
schedulerLog.warn(`PROMPT.md read failed for task dispatch validation (${id}): ${errorMessage}`);
|
||||
return { valid: false, reason: "missing or empty PROMPT.md" };
|
||||
}
|
||||
|
||||
@@ -545,7 +547,11 @@ export class Scheduler {
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
schedulerLog.warn(
|
||||
`Mission/slice lookup failed during scheduling (task ${t.id}): ${errorMessage} — proceeding without blocked-slice check`,
|
||||
);
|
||||
// If lookup fails, don't block the task
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,16 +56,40 @@ vi.mock("./worktree-pool.js", () => ({
|
||||
scanOrphanedBranches: vi.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
vi.mock("./logger.js", () => ({
|
||||
createLogger: vi.fn((_name: string) => ({
|
||||
log: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
})),
|
||||
}));
|
||||
|
||||
import { SelfHealingManager } from "./self-healing.js";
|
||||
import type { TaskStore, Settings, Task } from "@fusion/core";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { execSync } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { scanOrphanedBranches } from "./worktree-pool.js";
|
||||
import { createLogger } from "./logger.js";
|
||||
|
||||
const mockedExecSync = vi.mocked(execSync);
|
||||
const mockedExistsSync = vi.mocked(existsSync);
|
||||
const mockedScanOrphanedBranches = vi.mocked(scanOrphanedBranches);
|
||||
const mockedCreateLogger = vi.mocked(createLogger);
|
||||
|
||||
type MockLogger = {
|
||||
log: ReturnType<typeof vi.fn>;
|
||||
warn: ReturnType<typeof vi.fn>;
|
||||
error: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
function getSelfHealingLogger(): MockLogger {
|
||||
const idx = mockedCreateLogger.mock.calls.findIndex(([name]) => name === "self-healing");
|
||||
if (idx === -1) {
|
||||
throw new Error("self-healing logger was not created");
|
||||
}
|
||||
return mockedCreateLogger.mock.results[idx]?.value as MockLogger;
|
||||
}
|
||||
|
||||
// ── Mock helpers ────────────────────────────────────────────────────
|
||||
|
||||
@@ -485,6 +509,64 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("silent catch logging", () => {
|
||||
it("logs warn when interrupted-merge worktree removal fails", async () => {
|
||||
const warn = getSelfHealingLogger().warn;
|
||||
warn.mockClear();
|
||||
|
||||
const task = {
|
||||
id: "FN-123",
|
||||
worktree: "/tmp/test-project/.worktrees/fn-123",
|
||||
branch: "fusion/fn-123",
|
||||
} as Task;
|
||||
|
||||
mockedExistsSync.mockReset();
|
||||
mockedExistsSync.mockReturnValueOnce(true);
|
||||
mockedExecSync.mockReset();
|
||||
mockedExecSync.mockImplementationOnce(() => {
|
||||
throw new Error("cannot remove worktree");
|
||||
});
|
||||
|
||||
await (manager as any).cleanupInterruptedMergeArtifacts(task);
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
`Failed to remove interrupted-merge worktree ${task.worktree} for ${task.id}: cannot remove worktree`,
|
||||
),
|
||||
);
|
||||
|
||||
mockedExecSync.mockClear();
|
||||
mockedExistsSync.mockReset();
|
||||
});
|
||||
|
||||
it("logs warn when interrupted-merge branch deletion fails", async () => {
|
||||
const warn = getSelfHealingLogger().warn;
|
||||
warn.mockClear();
|
||||
|
||||
const task = {
|
||||
id: "FN-124",
|
||||
branch: "fusion/fn-124",
|
||||
} as Task;
|
||||
|
||||
mockedExistsSync.mockReset();
|
||||
mockedExecSync.mockReset();
|
||||
mockedExecSync.mockImplementationOnce(() => {
|
||||
throw new Error("cannot delete branch");
|
||||
});
|
||||
|
||||
await (manager as any).cleanupInterruptedMergeArtifacts(task);
|
||||
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
`Failed to delete interrupted-merge branch fusion/fn-124 for FN-124: cannot delete branch`,
|
||||
),
|
||||
);
|
||||
|
||||
mockedExecSync.mockClear();
|
||||
mockedExistsSync.mockReset();
|
||||
});
|
||||
});
|
||||
|
||||
// ── cleanupOrphanedBranches ────────────────────────────────────────
|
||||
|
||||
describe("cleanupOrphanedBranches", () => {
|
||||
|
||||
@@ -177,8 +177,10 @@ export class SelfHealingManager {
|
||||
if (this.settingsListener) {
|
||||
try {
|
||||
this.store.removeListener("settings:updated", this.settingsListener);
|
||||
} catch {
|
||||
// Store may not support removeListener (e.g., test mocks)
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
// Store may not support removeListener (e.g., test mocks) — non-fatal.
|
||||
log.warn(`Failed to remove settings:updated listener during stop(): ${errorMessage}`);
|
||||
}
|
||||
this.settingsListener = null;
|
||||
}
|
||||
@@ -372,8 +374,11 @@ export class SelfHealingManager {
|
||||
`Reset ${completedSteps.length} step(s) to pending — branch had no commits (uncommitted work lost with worktree)`,
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
// Branch may not exist or git commands may fail — non-fatal
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to reset steps for ${task.id} after branch/worktree loss (${branchName}): ${errorMessage} — non-fatal`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -421,7 +426,11 @@ export class SelfHealingManager {
|
||||
try {
|
||||
const result = await readLog(task.baseCommitSha ? `${task.baseCommitSha}..HEAD` : "HEAD");
|
||||
stdout = result.stdout;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to read git log for landed commit lookup (${task.id}): ${errorMessage} — retrying with HEAD range`,
|
||||
);
|
||||
if (!task.baseCommitSha) return null;
|
||||
const result = await readLog("HEAD");
|
||||
stdout = result.stdout;
|
||||
@@ -440,7 +449,11 @@ export class SelfHealingManager {
|
||||
maxBuffer: 1024 * 1024,
|
||||
});
|
||||
Object.assign(commit, parseShortstat(stats.stdout));
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to read shortstat for landed commit ${sha} (${task.id}): ${errorMessage} — continuing without stats`,
|
||||
);
|
||||
// Stats are useful for the task detail view but not required for recovery.
|
||||
}
|
||||
|
||||
@@ -454,8 +467,11 @@ export class SelfHealingManager {
|
||||
cwd: this.options.rootDir,
|
||||
timeout: 120_000,
|
||||
});
|
||||
} catch {
|
||||
// Non-fatal; existing orphan/worktree cleanup can retry later.
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to remove interrupted-merge worktree ${task.worktree} for ${task.id}: ${errorMessage} — non-fatal, cleanup can retry later`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -465,7 +481,11 @@ export class SelfHealingManager {
|
||||
cwd: this.options.rootDir,
|
||||
timeout: 120_000,
|
||||
});
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to delete interrupted-merge branch ${branch} for ${task.id}: ${errorMessage} — non-fatal`,
|
||||
);
|
||||
// Non-fatal; branch may be gone or still checked out.
|
||||
}
|
||||
}
|
||||
@@ -1181,7 +1201,11 @@ export class SelfHealingManager {
|
||||
timeout: 30_000,
|
||||
});
|
||||
if (status.trim().length > 0) return true;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to inspect worktree status for ${task.id} at ${task.worktree}: ${errorMessage} — preserving worktree`,
|
||||
);
|
||||
// If we cannot inspect an existing worktree, preserve it.
|
||||
return true;
|
||||
}
|
||||
@@ -1194,6 +1218,7 @@ export class SelfHealingManager {
|
||||
timeout: 30_000,
|
||||
});
|
||||
} catch {
|
||||
// Intentional negative test: rev-parse exits non-zero when branch does not exist.
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -1203,7 +1228,11 @@ export class SelfHealingManager {
|
||||
{ cwd: this.options.rootDir, timeout: 30_000 },
|
||||
);
|
||||
return Number.parseInt(uniqueCommits.trim(), 10) > 0;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Failed to compare branch ${branchName} against HEAD for ${task.id}: ${errorMessage} — preserving branch`,
|
||||
);
|
||||
// If the branch exists but cannot be compared, preserve it.
|
||||
return true;
|
||||
}
|
||||
@@ -1354,7 +1383,9 @@ export class SelfHealingManager {
|
||||
timeout: 30_000,
|
||||
});
|
||||
cleaned++;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(`Failed to remove orphaned worktree ${worktreePath}: ${errorMessage} — non-fatal`);
|
||||
// Individual failure is non-fatal
|
||||
}
|
||||
}
|
||||
@@ -1397,7 +1428,11 @@ export class SelfHealingManager {
|
||||
});
|
||||
log.log(`Deleted branch: ${branch}`);
|
||||
cleaned++;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(
|
||||
`Safe delete failed for orphaned branch ${branch}: ${errorMessage} — attempting force delete`,
|
||||
);
|
||||
// Safe delete failed (not merged) — force delete
|
||||
try {
|
||||
await execAsync(`git branch -D "${branch}"`, {
|
||||
@@ -1406,7 +1441,9 @@ export class SelfHealingManager {
|
||||
});
|
||||
log.log(`Force-deleted branch: ${branch}`);
|
||||
cleaned++;
|
||||
} catch {
|
||||
} catch (forceErr: unknown) {
|
||||
const forceErrorMessage = forceErr instanceof Error ? forceErr.message : String(forceErr);
|
||||
log.warn(`Failed to force-delete orphaned branch ${branch}: ${forceErrorMessage} — non-fatal`);
|
||||
// Individual failure is non-fatal
|
||||
}
|
||||
}
|
||||
@@ -1457,7 +1494,9 @@ export class SelfHealingManager {
|
||||
const withMtime = idle.map((p) => {
|
||||
try {
|
||||
return { path: p, mtime: statSync(p).mtimeMs };
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(`Failed to read mtime for worktree ${p}: ${errorMessage} — defaulting mtime to 0`);
|
||||
return { path: p, mtime: 0 };
|
||||
}
|
||||
});
|
||||
@@ -1474,7 +1513,9 @@ export class SelfHealingManager {
|
||||
timeout: 30_000,
|
||||
});
|
||||
removed++;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.warn(`Failed to remove idle worktree ${worktreePath} during cap enforcement: ${errorMessage} — non-fatal`);
|
||||
// Individual failure is non-fatal
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user