Merge main: adopt FN-5902 lazy assertion linkage in shared runFeatureValidation
- runFeatureValidation now lazy-ensures a linked assertion (FN-5902) instead of the removed zero-assertion auto-pass, for both task-completion and the stranded-feature recovery path - CONCEPTS.md: union of main's Merge-lifecycle cluster and this branch's Missions clusters; Contract Assertion entry updated for FN-5902 semantics - AGENTS.md: take main's docs/solutions + CONCEPTS.md pointer wording Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
@@ -1,4 +1,7 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import * as core from "@fusion/core";
|
||||
import { collectTaskEvaluationEvidence } from "../evaluator-evidence.js";
|
||||
|
||||
@@ -30,6 +33,23 @@ function makeStore(overrides: Partial<core.TaskStore> = {}): core.TaskStore {
|
||||
}
|
||||
|
||||
describe("collectTaskEvaluationEvidence", () => {
|
||||
let integrationRootDir: string | null = null;
|
||||
let integrationStore: core.TaskStore | null = null;
|
||||
|
||||
beforeEach(() => {
|
||||
integrationRootDir = null;
|
||||
integrationStore = null;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
integrationStore?.close();
|
||||
integrationStore = null;
|
||||
if (integrationRootDir) {
|
||||
rmSync(integrationRootDir, { recursive: true, force: true });
|
||||
integrationRootDir = null;
|
||||
}
|
||||
});
|
||||
|
||||
it("collects fixed source groups with bounded excerpts", async () => {
|
||||
const store = makeStore({
|
||||
getTaskDocuments: vi.fn().mockResolvedValue([{ key: "plan", content: "x".repeat(900), revision: 1, author: "agent", updatedAt: "2026-01-01T00:01:00.000Z" }]),
|
||||
@@ -123,6 +143,31 @@ describe("collectTaskEvaluationEvidence", () => {
|
||||
expect(evidence.agentLogs.at(-1)?.excerpt).toContain("entry-29");
|
||||
});
|
||||
|
||||
it("reads file-backed agent logs through the TaskStore evidence seam", async () => {
|
||||
integrationRootDir = mkdtempSync(join(tmpdir(), "fusion-evaluator-evidence-"));
|
||||
const globalDir = join(integrationRootDir, ".fusion-global-settings");
|
||||
integrationStore = new core.TaskStore(integrationRootDir, globalDir, { inMemoryDb: true });
|
||||
await integrationStore.init();
|
||||
|
||||
const task = await integrationStore.createTask({ description: "Collect evaluator evidence from file-backed logs" });
|
||||
await integrationStore.appendAgentLog(task.id, "first line", "text", undefined, "executor");
|
||||
await integrationStore.appendAgentLog(task.id, "tool finished", "tool_result", "ok", "executor");
|
||||
|
||||
const detail = await integrationStore.getTask(task.id);
|
||||
const evidence = await collectTaskEvaluationEvidence({
|
||||
store: integrationStore,
|
||||
task: detail,
|
||||
runId: "ER-file-backed",
|
||||
cwd: integrationRootDir,
|
||||
});
|
||||
|
||||
expect(evidence.agentLogs).toHaveLength(2);
|
||||
expect(evidence.agentLogs.map((entry) => entry.label)).toEqual(["text", "tool_result"]);
|
||||
expect(evidence.agentLogs.map((entry) => entry.excerpt)).toEqual(["first line", "tool finished — ok"]);
|
||||
expect(evidence.agentLogs.map((entry) => entry.agentId)).toEqual(["executor", "executor"]);
|
||||
|
||||
});
|
||||
|
||||
it("truncates task metadata summary when oversized", async () => {
|
||||
const evidence = await collectTaskEvaluationEvidence({
|
||||
store: makeStore(),
|
||||
|
||||
@@ -573,7 +573,17 @@ describe("aiMergeTask — build verification", () => {
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
|
||||
});
|
||||
|
||||
it("syncs dependencies before build verification when install state is missing", async () => {
|
||||
function setupDependencySyncVerificationScenario({
|
||||
taskId = "FN-050",
|
||||
installStatePresent,
|
||||
stagedFiles,
|
||||
settingsOverrides,
|
||||
}: {
|
||||
taskId?: string;
|
||||
installStatePresent: boolean;
|
||||
stagedFiles: string[];
|
||||
settingsOverrides: Partial<typeof DEFAULT_SETTINGS>;
|
||||
}) {
|
||||
mockedCreateFnAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -583,7 +593,7 @@ describe("aiMergeTask — build verification", () => {
|
||||
|
||||
mockedExistsSync.mockImplementation((path: any) => {
|
||||
const pathStr = String(path);
|
||||
if (pathStr.includes("node_modules") || pathStr.endsWith(".pnp.cjs")) return false;
|
||||
if (pathStr.includes("node_modules") || pathStr.endsWith(".pnp.cjs")) return installStatePresent;
|
||||
return true;
|
||||
});
|
||||
|
||||
@@ -598,9 +608,16 @@ describe("aiMergeTask — build verification", () => {
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --name-only --diff-filter=U")) return "" as any;
|
||||
if (cmdStr.includes("git diff --cached --name-only")) {
|
||||
return "package.json\npackages/desktop/package.json" as any;
|
||||
return stagedFiles.join("\n") as any;
|
||||
}
|
||||
if (
|
||||
cmdStr.includes("pnpm install --frozen-lockfile") ||
|
||||
cmdStr.includes("pnpm run setup:merge") ||
|
||||
cmdStr.includes("pnpm test") ||
|
||||
cmdStr.includes("pnpm build")
|
||||
) {
|
||||
return Buffer.from("");
|
||||
}
|
||||
if (cmdStr.includes("pnpm install --frozen-lockfile")) return "Lockfile is up to date" as any;
|
||||
if (cmdStr.includes("diff --cached --quiet")) {
|
||||
cachedQuietChecks += 1;
|
||||
return cachedQuietChecks === 1 ? "1" as any : "0" as any;
|
||||
@@ -612,13 +629,23 @@ describe("aiMergeTask — build verification", () => {
|
||||
});
|
||||
|
||||
const store = createMockStore(
|
||||
{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
|
||||
[{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
|
||||
{ id: taskId, worktree: `/tmp/root/.worktrees/${taskId}` },
|
||||
[{ id: taskId, worktree: `/tmp/root/.worktrees/${taskId}`, column: "in-review" } as Task],
|
||||
);
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...DEFAULT_SETTINGS,
|
||||
mergeIntegrationWorktree: "cwd-main" as const,
|
||||
buildCommand: "pnpm build",
|
||||
...settingsOverrides,
|
||||
});
|
||||
|
||||
return { store };
|
||||
}
|
||||
|
||||
it("syncs dependencies before build verification when install state is missing", async () => {
|
||||
const { store } = setupDependencySyncVerificationScenario({
|
||||
installStatePresent: false,
|
||||
stagedFiles: ["package.json", "packages/desktop/package.json"],
|
||||
settingsOverrides: { buildCommand: "pnpm build" },
|
||||
});
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
@@ -635,51 +662,11 @@ describe("aiMergeTask — build verification", () => {
|
||||
});
|
||||
|
||||
it("syncs dependencies before test verification when install state is missing", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
mockedExistsSync.mockImplementation((path: any) => {
|
||||
const pathStr = String(path);
|
||||
if (pathStr.includes("node_modules") || pathStr.endsWith(".pnp.cjs")) return false;
|
||||
return true;
|
||||
});
|
||||
|
||||
let cachedQuietChecks = 0;
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "2 files changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --name-only --diff-filter=U")) return "" as any;
|
||||
if (cmdStr.includes("git diff --cached --name-only")) {
|
||||
return "package.json\npackages/desktop/package.json" as any;
|
||||
}
|
||||
if (cmdStr.includes("pnpm install --frozen-lockfile")) return "Lockfile is up to date" as any;
|
||||
if (cmdStr.includes("diff --cached --quiet")) {
|
||||
cachedQuietChecks += 1;
|
||||
return cachedQuietChecks === 1 ? "1" as any : "0" as any;
|
||||
}
|
||||
if (cmdStr.includes("show --shortstat")) return "3 files changed, 10 insertions(+), 2 deletions(-)" as any;
|
||||
if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from("");
|
||||
if (cmdStr.includes("worktree remove")) return Buffer.from("");
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
const store = createMockStore(
|
||||
{ id: "FN-051", worktree: "/tmp/root/.worktrees/KB-051" },
|
||||
[{ id: "FN-051", worktree: "/tmp/root/.worktrees/KB-051", column: "in-review" } as Task],
|
||||
);
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...DEFAULT_SETTINGS,
|
||||
mergeIntegrationWorktree: "cwd-main" as const,
|
||||
testCommand: "pnpm test",
|
||||
const { store } = setupDependencySyncVerificationScenario({
|
||||
taskId: "FN-051",
|
||||
installStatePresent: false,
|
||||
stagedFiles: ["package.json", "packages/desktop/package.json"],
|
||||
settingsOverrides: { testCommand: "pnpm test" },
|
||||
});
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-051");
|
||||
@@ -689,6 +676,84 @@ describe("aiMergeTask — build verification", () => {
|
||||
mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm install --frozen-lockfile")),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("runs the configured worktree init command before verification when install state is warm and no dependency files are staged", async () => {
|
||||
const { store } = setupDependencySyncVerificationScenario({
|
||||
installStatePresent: true,
|
||||
stagedFiles: ["packages/engine/src/merger.ts"],
|
||||
settingsOverrides: {
|
||||
testCommand: "pnpm test",
|
||||
worktreeInitCommand: "pnpm run setup:merge",
|
||||
},
|
||||
});
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
|
||||
expect(result.merged).toBe(true);
|
||||
expect(mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm run setup:merge"))).toBe(true);
|
||||
expect(
|
||||
mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm install --frozen-lockfile")),
|
||||
).toBe(false);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
"Syncing dependencies before merge verification: pnpm run setup:merge",
|
||||
);
|
||||
});
|
||||
|
||||
it("runs the configured worktree init command before verification when install state is missing", async () => {
|
||||
const { store } = setupDependencySyncVerificationScenario({
|
||||
installStatePresent: false,
|
||||
stagedFiles: ["package.json"],
|
||||
settingsOverrides: {
|
||||
buildCommand: "pnpm build",
|
||||
worktreeInitCommand: "pnpm run setup:merge",
|
||||
},
|
||||
});
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
|
||||
expect(result.merged).toBe(true);
|
||||
expect(mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm run setup:merge"))).toBe(true);
|
||||
expect(
|
||||
mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm install --frozen-lockfile")),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves inferred install behavior when no worktree init command is configured", async () => {
|
||||
expect(shouldSyncDependenciesForMerge(["packages/engine/src/merger.ts"], true, false)).toBe(false);
|
||||
|
||||
const { store } = setupDependencySyncVerificationScenario({
|
||||
installStatePresent: true,
|
||||
stagedFiles: ["packages/engine/src/merger.ts"],
|
||||
settingsOverrides: { testCommand: "pnpm test" },
|
||||
});
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
|
||||
expect(result.merged).toBe(true);
|
||||
expect(
|
||||
mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm install --frozen-lockfile")),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("treats whitespace-only worktree init commands as unset and falls back to inferred install behavior", async () => {
|
||||
const { store } = setupDependencySyncVerificationScenario({
|
||||
installStatePresent: false,
|
||||
stagedFiles: ["package.json"],
|
||||
settingsOverrides: {
|
||||
testCommand: "pnpm test",
|
||||
worktreeInitCommand: " ",
|
||||
},
|
||||
});
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
|
||||
expect(result.merged).toBe(true);
|
||||
expect(mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm run setup:merge"))).toBe(false);
|
||||
expect(
|
||||
mockedExecSync.mock.calls.some((call) => String(call[0]).includes("pnpm install --frozen-lockfile")),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Deterministic Merge Verification Tests ──────────────────────────────
|
||||
|
||||
@@ -204,6 +204,16 @@ function createMockMissionStore() {
|
||||
return updated;
|
||||
}),
|
||||
listAssertionsForFeature: vi.fn((featureId: string) => assertionsByFeature.get(featureId) ?? []),
|
||||
ensureFeatureAssertionLinked: vi.fn((featureId: string) => {
|
||||
const feature = features.get(featureId);
|
||||
if (!feature) {
|
||||
throw new Error(`Feature ${featureId} not found`);
|
||||
}
|
||||
if ((assertionsByFeature.get(featureId) ?? []).length === 0) {
|
||||
store._addFeatureWithManagedAssertion(feature);
|
||||
}
|
||||
return assertionsByFeature.get(featureId) ?? [];
|
||||
}),
|
||||
getAssertionsForFeature: vi.fn((featureId: string) => assertionsByFeature.get(featureId) ?? []),
|
||||
getSlice: vi.fn((id: string) => {
|
||||
// Return a mock slice with milestoneId for the hierarchy
|
||||
@@ -845,12 +855,21 @@ describe("MissionExecutionLoop", () => {
|
||||
expect(missionStore.completeValidatorRun).toHaveBeenCalledWith(expect.any(String), "passed", "Recovered validation passed");
|
||||
});
|
||||
|
||||
it("should auto-pass if feature has no linked assertions", async () => {
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001" });
|
||||
it("lazy-ensures a managed assertion and routes zero-assertion features through validation", async () => {
|
||||
const feature = createMockFeature({
|
||||
id: "F-001",
|
||||
loopState: "implementing",
|
||||
taskId: "FN-001",
|
||||
title: "Feature from prose",
|
||||
acceptanceCriteria: "Feature must validate through AI",
|
||||
});
|
||||
missionStore._setFeature(feature);
|
||||
taskStore._setTask({ id: "FN-001", title: "Test", description: "Test task", log: [] });
|
||||
missionStore.getFeatureByTaskId = vi.fn().mockReturnValue(feature);
|
||||
missionStore.listAssertionsForFeature = vi.fn().mockReturnValue([]);
|
||||
missionStore.listAssertionsForFeature = vi
|
||||
.fn()
|
||||
.mockReturnValueOnce([])
|
||||
.mockImplementation((featureId: string) => (missionStore as any).getAssertionsForFeature(featureId));
|
||||
|
||||
loop = new MissionExecutionLoop({
|
||||
taskStore: taskStore as any,
|
||||
@@ -858,41 +877,39 @@ describe("MissionExecutionLoop", () => {
|
||||
rootDir: "/tmp",
|
||||
});
|
||||
const emitSpy = vi.spyOn(loop, "emit");
|
||||
vi.spyOn(loop as any, "runValidation").mockResolvedValue({ status: "pass", summary: "ok" });
|
||||
loop.start();
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
// When there are no assertions, we skip starting a validator run
|
||||
expect(missionStore.startValidatorRun).not.toHaveBeenCalled();
|
||||
// But the passed event should be emitted
|
||||
expect(missionStore.ensureFeatureAssertionLinked).toHaveBeenCalledWith("F-001");
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion");
|
||||
expect(emitSpy).toHaveBeenCalledWith(
|
||||
"validation:passed",
|
||||
expect.objectContaining({ featureId: "F-001" }),
|
||||
);
|
||||
expect(missionStore.updateFeature).toHaveBeenCalledWith(
|
||||
"F-001",
|
||||
expect.objectContaining({ loopState: "passed", lastValidatorStatus: "passed" }),
|
||||
);
|
||||
expect(missionStore.logMissionEvent).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
"warning",
|
||||
expect.stringContaining("auto-passed"),
|
||||
expect.objectContaining({
|
||||
code: "validation_auto_passed_no_assertions",
|
||||
featureId: "F-001",
|
||||
reason: "No assertions linked",
|
||||
taskId: "FN-001",
|
||||
}),
|
||||
const noAssertionEvents = missionStore.logMissionEvent.mock.calls.filter(
|
||||
([, , , payload]) => payload?.code === "validation_auto_passed_no_assertions",
|
||||
);
|
||||
expect(noAssertionEvents).toHaveLength(0);
|
||||
expectNoValidationBoardTaskMutation(taskStore);
|
||||
});
|
||||
|
||||
it("emits no-assertions auto-pass event exactly once across re-entry", async () => {
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001" });
|
||||
it("does not emit auto-pass evidence across re-entry after lazy assertion ensure", async () => {
|
||||
const feature = createMockFeature({
|
||||
id: "F-001",
|
||||
loopState: "implementing",
|
||||
taskId: "FN-001",
|
||||
title: "Feature from prose",
|
||||
acceptanceCriteria: "Feature must validate through AI",
|
||||
});
|
||||
missionStore._setFeature(feature);
|
||||
taskStore._setTask({ id: "FN-001", title: "Test", description: "Test task", log: [] });
|
||||
missionStore.getFeatureByTaskId = vi.fn().mockReturnValue(feature);
|
||||
missionStore.listAssertionsForFeature = vi.fn().mockReturnValue([]);
|
||||
missionStore.listAssertionsForFeature = vi
|
||||
.fn()
|
||||
.mockReturnValueOnce([])
|
||||
.mockImplementation((featureId: string) => (missionStore as any).getAssertionsForFeature(featureId));
|
||||
|
||||
loop = new MissionExecutionLoop({
|
||||
taskStore: taskStore as any,
|
||||
@@ -904,10 +921,11 @@ describe("MissionExecutionLoop", () => {
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
expect(missionStore.ensureFeatureAssertionLinked).toHaveBeenCalledTimes(1);
|
||||
const noAssertionEvents = missionStore.logMissionEvent.mock.calls.filter(
|
||||
([, , , payload]) => payload?.code === "validation_auto_passed_no_assertions",
|
||||
);
|
||||
expect(noAssertionEvents).toHaveLength(1);
|
||||
expect(noAssertionEvents).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("uses validator path for later-added feature with managed assertion", async () => {
|
||||
@@ -935,6 +953,44 @@ describe("MissionExecutionLoop", () => {
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-LATER", "task_completion");
|
||||
});
|
||||
|
||||
it("threads milestone acceptance criteria into validator prompts", () => {
|
||||
const feature = createMockFeature({
|
||||
id: "F-MILESTONE",
|
||||
title: "Feature under milestone",
|
||||
acceptanceCriteria: "Feature criteria",
|
||||
});
|
||||
const milestone = createMockMilestone({
|
||||
id: "MS-MILESTONE",
|
||||
acceptanceCriteria: "Milestone pass bar text",
|
||||
});
|
||||
const assertions = [
|
||||
{
|
||||
id: "CA-1",
|
||||
milestoneId: milestone.id,
|
||||
title: "Managed assertion",
|
||||
assertion: "Feature criteria",
|
||||
status: "pending" as const,
|
||||
orderIndex: 0,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
},
|
||||
];
|
||||
|
||||
loop = new MissionExecutionLoop({
|
||||
taskStore: taskStore as any,
|
||||
missionStore: missionStore as any,
|
||||
rootDir: "/tmp",
|
||||
});
|
||||
|
||||
const prompt = (loop as any).buildValidationPrompt(feature, assertions, milestone);
|
||||
const systemPrompt = (loop as any).buildValidationSystemPrompt(feature, assertions, "Task context", milestone);
|
||||
|
||||
expect(prompt).toContain("Milestone pass bar text");
|
||||
expect(prompt).toContain("must also be satisfied for this feature to pass");
|
||||
expect(systemPrompt).toContain("Milestone pass bar text");
|
||||
expect(systemPrompt).toContain("validator-executed requirements");
|
||||
});
|
||||
|
||||
it("does NOT create a board task for single-feature validation", async () => {
|
||||
const feature = createMockFeature({ loopState: "implementing", taskId: "FN-001", sliceId: "SL-001" });
|
||||
missionStore._setFeature(feature);
|
||||
@@ -1545,7 +1601,7 @@ describe("MissionExecutionLoop", () => {
|
||||
});
|
||||
missionStore._setFeature(feature);
|
||||
missionStore.getFeatureByTaskId = vi.fn().mockReturnValue(feature);
|
||||
missionStore.listAssertionsForFeature = vi.fn().mockReturnValue([]); // No assertions = auto-pass
|
||||
missionStore.listAssertionsForFeature = vi.fn().mockReturnValue([]);
|
||||
taskStore._setTask({ id: "FN-001", title: "Test", description: "Test", log: [] });
|
||||
|
||||
const notifySpy = vi.fn();
|
||||
@@ -1558,12 +1614,13 @@ describe("MissionExecutionLoop", () => {
|
||||
},
|
||||
});
|
||||
const emitSpy = vi.spyOn(loop, "emit");
|
||||
vi.spyOn(loop as any, "runValidation").mockResolvedValue({ status: "pass", summary: "ok" });
|
||||
loop.start();
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
// No validator run started (no assertions)
|
||||
expect(missionStore.startValidatorRun).not.toHaveBeenCalled();
|
||||
expect(missionStore.ensureFeatureAssertionLinked).toHaveBeenCalledWith("F-001");
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion");
|
||||
|
||||
// validation:passed event emitted
|
||||
expect(emitSpy).toHaveBeenCalledWith(
|
||||
|
||||
@@ -26,11 +26,14 @@ vi.mock("../pr-monitor.js", () => ({ PrMonitor: vi.fn().mockImplementation(() =>
|
||||
vi.mock("../pr-comment-handler.js", () => ({ PrCommentHandler: vi.fn().mockImplementation(() => ({ handleNewComments: vi.fn() })) }));
|
||||
vi.mock("../auth-storage.js", () => ({
|
||||
createFusionAuthStorage: vi.fn(() => ({ reload: vi.fn(), getOAuthProviders: vi.fn(() => []), get: vi.fn(() => undefined) })),
|
||||
getFusionOAuthAlertStatePath: vi.fn(() => "/tmp/oauth-alert-state.json"),
|
||||
}));
|
||||
vi.mock("../notifier.js", () => ({ NtfyNotifier: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })) }));
|
||||
vi.mock("../notification/index.js", () => ({
|
||||
NotificationService: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
OAuthAlertStateStore: vi.fn().mockImplementation(() => ({})),
|
||||
OAuthExpiryMonitor: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
OAuthValidityLogger: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
}));
|
||||
vi.mock("../cron-runner.js", () => ({
|
||||
CronRunner: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
|
||||
@@ -4,7 +4,7 @@ import { ProjectEngine } from "../project-engine.js";
|
||||
import { runtimeLog } from "../logger.js";
|
||||
import { TunnelProcessManager } from "../remote-access/tunnel-process-manager.js";
|
||||
import { NtfyNotifier } from "../notifier.js";
|
||||
import { NotificationService, OAuthExpiryMonitor, OAuthValidityLogger } from "../notification/index.js";
|
||||
import { NotificationService, OAuthAlertStateStore, OAuthExpiryMonitor, OAuthValidityLogger } from "../notification/index.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
syncInsightExtractionAutomation: vi.fn(),
|
||||
@@ -97,6 +97,7 @@ vi.mock("../notification/index.js", () => ({
|
||||
start: mocks.notificationServiceStart,
|
||||
stop: mocks.notificationServiceStop,
|
||||
})),
|
||||
OAuthAlertStateStore: vi.fn().mockImplementation(() => ({})),
|
||||
OAuthExpiryMonitor: vi.fn().mockImplementation(() => ({
|
||||
start: mocks.oauthExpiryMonitorStart,
|
||||
stop: mocks.oauthExpiryMonitorStop,
|
||||
@@ -113,6 +114,7 @@ vi.mock("../auth-storage.js", () => ({
|
||||
getOAuthProviders: vi.fn(() => []),
|
||||
get: vi.fn(() => undefined),
|
||||
})),
|
||||
getFusionOAuthAlertStatePath: vi.fn(() => "/tmp/oauth-alert-state.json"),
|
||||
}));
|
||||
|
||||
vi.mock("../runtimes/in-process-runtime.js", () => ({
|
||||
@@ -305,10 +307,19 @@ describe("ProjectEngine notification ownership wiring", () => {
|
||||
await engine.start();
|
||||
|
||||
expect(NotificationService).toHaveBeenCalledTimes(1);
|
||||
expect(OAuthAlertStateStore).toHaveBeenCalledTimes(1);
|
||||
expect(OAuthExpiryMonitor).toHaveBeenCalledTimes(1);
|
||||
expect(OAuthValidityLogger).toHaveBeenCalledTimes(1);
|
||||
expect(NtfyNotifier).toHaveBeenCalledTimes(1);
|
||||
const notifierCtorArgs = vi.mocked(NtfyNotifier).mock.calls[0];
|
||||
expect(notifierCtorArgs?.[2]).toBe(vi.mocked(NotificationService).mock.results[0]?.value);
|
||||
const alertStateInstance = vi.mocked(OAuthAlertStateStore).mock.results[0]?.value;
|
||||
expect(vi.mocked(OAuthExpiryMonitor).mock.calls[0]?.[0]).toEqual(
|
||||
expect.objectContaining({ alertState: alertStateInstance }),
|
||||
);
|
||||
expect(vi.mocked(OAuthValidityLogger).mock.calls[0]?.[0]).toEqual(
|
||||
expect.objectContaining({ alertState: alertStateInstance }),
|
||||
);
|
||||
|
||||
expect(mocks.notificationServiceStart).toHaveBeenCalledTimes(1);
|
||||
expect(mocks.oauthExpiryMonitorStart).toHaveBeenCalledTimes(1);
|
||||
@@ -329,7 +340,9 @@ describe("ProjectEngine notification ownership wiring", () => {
|
||||
// Root cause guard: if ProjectEngine.start is called more than once, it should not
|
||||
// wire a second NotificationService/NtfyNotifier pair for the same store.
|
||||
expect(NotificationService).toHaveBeenCalledTimes(1);
|
||||
expect(OAuthAlertStateStore).toHaveBeenCalledTimes(1);
|
||||
expect(OAuthExpiryMonitor).toHaveBeenCalledTimes(1);
|
||||
expect(OAuthValidityLogger).toHaveBeenCalledTimes(1);
|
||||
expect(NtfyNotifier).toHaveBeenCalledTimes(1);
|
||||
expect(mocks.notificationServiceStart).toHaveBeenCalledTimes(1);
|
||||
expect(mocks.oauthExpiryMonitorStart).toHaveBeenCalledTimes(1);
|
||||
@@ -344,7 +357,9 @@ describe("ProjectEngine notification ownership wiring", () => {
|
||||
await engine.start();
|
||||
|
||||
expect(NotificationService).not.toHaveBeenCalled();
|
||||
expect(OAuthAlertStateStore).not.toHaveBeenCalled();
|
||||
expect(OAuthExpiryMonitor).not.toHaveBeenCalled();
|
||||
expect(OAuthValidityLogger).not.toHaveBeenCalled();
|
||||
expect(NtfyNotifier).not.toHaveBeenCalled();
|
||||
|
||||
await engine.stop();
|
||||
@@ -2655,3 +2670,97 @@ describe("ProjectEngine stale mergeActive rescue (FN-3900)", () => {
|
||||
await engine.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("allowInReviewMergeProcessing per-task autoMerge override", () => {
|
||||
const gate = (task: Partial<Task>, settings: { autoMerge: boolean }) =>
|
||||
(createEngine() as any).allowInReviewMergeProcessing(task, settings) as boolean;
|
||||
|
||||
it("lets an explicit per-task autoMerge:true through when the global setting is off", () => {
|
||||
expect(gate({ autoMerge: true }, { autoMerge: false })).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks tasks without a per-task override when the global setting is off", () => {
|
||||
expect(gate({}, { autoMerge: false })).toBe(false);
|
||||
expect(gate({ autoMerge: false }, { autoMerge: false })).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps everything flowing when the global setting is on — explicit autoMerge:false is parked manual-required downstream", () => {
|
||||
expect(gate({}, { autoMerge: true })).toBe(true);
|
||||
expect(gate({ autoMerge: false }, { autoMerge: true })).toBe(true);
|
||||
});
|
||||
|
||||
it("still exempts shared-branch-group member integration when the global setting is off", () => {
|
||||
expect(gate(
|
||||
{ branchContext: { assignmentMode: "shared", groupId: "grp-1" } as Task["branchContext"] },
|
||||
{ autoMerge: false },
|
||||
)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ## Surface Enumeration
|
||||
//
|
||||
// Known in-review merge entry surfaces in ProjectEngine, and how each enforces
|
||||
// the per-task `autoMerge` override invariant (a task with `autoMerge:true` must
|
||||
// still be enqueued for merge even when the global `autoMerge` setting is off):
|
||||
//
|
||||
// 1. Startup merge sweep (project-engine.ts ~:2857) ─┐
|
||||
// 2. Periodic merge retry sweep (project-engine.ts ~:2916) ─┼─ all call
|
||||
// 3. Resume-after-unpause sweep (project-engine.ts ~:2977) ─┘ enqueueEligibleInReviewTasks(...)
|
||||
// 4. task:moved fast path (project-engine.ts ~:1506) ─── inline allowInReviewMergeProcessing(...)
|
||||
//
|
||||
// Surfaces 1–3 funnel through `enqueueEligibleInReviewTasks`, whose filter is
|
||||
// `!t.paused && canMergeTask(t) && allowInReviewMergeProcessing(t, settings)`.
|
||||
// The behavior tests below exercise that shared funnel directly on a real engine
|
||||
// instance (with `internalEnqueueMerge` stubbed), so a regression in any of the
|
||||
// three sweep wrappers (wireAutoMerge / startupMergeSweep / scheduleMergeRetry /
|
||||
// resumeAfterUnpauseAndSweepInReview) that still routes through the funnel is
|
||||
// caught. Surface 4 (the task:moved fast path) shares the same
|
||||
// `allowInReviewMergeProcessing` gate, which is covered by the direct helper
|
||||
// tests above.
|
||||
|
||||
describe("enqueueEligibleInReviewTasks honors per-task autoMerge override (shared sweep funnel)", () => {
|
||||
const inReview = (id: string, overrides: Partial<Task> = {}): Task =>
|
||||
({
|
||||
id,
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
mergeRetries: 0,
|
||||
status: null,
|
||||
...overrides,
|
||||
}) as unknown as Task;
|
||||
|
||||
const setup = () => {
|
||||
const engine = createEngine() as any;
|
||||
const enqueueSpy = vi
|
||||
.spyOn(engine, "internalEnqueueMerge")
|
||||
.mockImplementation(() => true);
|
||||
const run = (tasks: Task[], settings: { autoMerge: boolean }): number =>
|
||||
engine.enqueueEligibleInReviewTasks(tasks, settings) as number;
|
||||
return { engine, enqueueSpy, run };
|
||||
};
|
||||
|
||||
it("enqueues an in-review task with autoMerge:true even when the global setting is off", () => {
|
||||
const { enqueueSpy, run } = setup();
|
||||
const count = run([inReview("FN-override", { autoMerge: true })], { autoMerge: false });
|
||||
expect(count).toBe(1);
|
||||
expect(enqueueSpy).toHaveBeenCalledWith("FN-override");
|
||||
});
|
||||
|
||||
it("does not enqueue a sibling task without an override in the same sweep when the global setting is off", () => {
|
||||
const { enqueueSpy, run } = setup();
|
||||
const count = run(
|
||||
[inReview("FN-override", { autoMerge: true }), inReview("FN-plain")],
|
||||
{ autoMerge: false },
|
||||
);
|
||||
expect(count).toBe(1);
|
||||
expect(enqueueSpy).toHaveBeenCalledWith("FN-override");
|
||||
expect(enqueueSpy).not.toHaveBeenCalledWith("FN-plain");
|
||||
});
|
||||
|
||||
it("still enqueues a task with autoMerge:false when the global setting is on (parked manual-required downstream)", () => {
|
||||
const { enqueueSpy, run } = setup();
|
||||
const count = run([inReview("FN-explicit-false", { autoMerge: false })], { autoMerge: true });
|
||||
expect(count).toBe(1);
|
||||
expect(enqueueSpy).toHaveBeenCalledWith("FN-explicit-false");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -186,9 +186,10 @@ describe("FN-5715 reliability: mission validation trigger gap", () => {
|
||||
loop.stop();
|
||||
});
|
||||
|
||||
it("periodic recovery pass replays implementing done tasks with zero assertions and advances loop state", async () => {
|
||||
const feature = makeFeature({ status: "done", lastValidatorStatus: undefined, loopState: "implementing" });
|
||||
it("periodic recovery lazily ensures assertions and AI-validates zero-link legacy features", async () => {
|
||||
const feature = makeFeature({ status: "done", lastValidatorStatus: undefined, loopState: "implementing", acceptanceCriteria: "must pass" });
|
||||
const currentFeature = { ...feature };
|
||||
const linkedAssertions: Array<{ id: string }> = [];
|
||||
const missionStore = {
|
||||
listMissions: vi.fn(() => [{ id: "M-001", status: "active" }]),
|
||||
getMissionWithHierarchy: vi.fn(() => ({
|
||||
@@ -203,11 +204,21 @@ describe("FN-5715 reliability: mission validation trigger gap", () => {
|
||||
Object.assign(currentFeature, patch);
|
||||
return { ...currentFeature };
|
||||
}),
|
||||
listAssertionsForFeature: vi.fn(() => []),
|
||||
listAssertionsForFeature: vi.fn(() => linkedAssertions),
|
||||
ensureFeatureAssertionLinked: vi.fn(() => {
|
||||
if (linkedAssertions.length === 0) {
|
||||
linkedAssertions.push({ id: "CA-ENSURED" });
|
||||
}
|
||||
return linkedAssertions;
|
||||
}),
|
||||
startValidatorRun: vi.fn(() => ({ id: "VR-001", featureId: "F-001" })),
|
||||
completeValidatorRun: vi.fn(),
|
||||
getSlice: vi.fn(() => ({ id: "SL-001", milestoneId: "MS-001", status: "active" })),
|
||||
getMilestone: vi.fn(() => ({ id: "MS-001", missionId: "M-001" })),
|
||||
logMissionEvent: vi.fn(),
|
||||
transitionLoopState: vi.fn(),
|
||||
setFeatureCurrentTaskRunId: vi.fn(),
|
||||
getFailuresForRun: vi.fn(() => []),
|
||||
};
|
||||
const taskStore = {
|
||||
getTask: vi.fn(async () => ({ id: "FN-001", column: "done", status: "done" })),
|
||||
@@ -220,25 +231,23 @@ describe("FN-5715 reliability: mission validation trigger gap", () => {
|
||||
taskStore: taskStore as any,
|
||||
rootDir: process.cwd(),
|
||||
});
|
||||
vi.spyOn(loop as any, "runValidation").mockResolvedValue({ status: "pass", summary: "ok" });
|
||||
loop.start();
|
||||
|
||||
const periodicMaintenancePass = async () => loop.recoverActiveMissions();
|
||||
await periodicMaintenancePass();
|
||||
await periodicMaintenancePass();
|
||||
|
||||
expect(missionStore.updateFeature).toHaveBeenCalledTimes(1);
|
||||
expect(missionStore.updateFeature).toHaveBeenCalledWith(
|
||||
"F-001",
|
||||
expect.objectContaining({ loopState: "passed", lastValidatorStatus: "passed" }),
|
||||
);
|
||||
expect(missionStore.ensureFeatureAssertionLinked).toHaveBeenCalledWith("F-001");
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion");
|
||||
const noAssertionEvents = missionStore.logMissionEvent.mock.calls.filter(
|
||||
([, type, , payload]) => type === "warning" && payload?.code === "validation_auto_passed_no_assertions",
|
||||
);
|
||||
expect(noAssertionEvents).toHaveLength(1);
|
||||
expect(noAssertionEvents).toHaveLength(0);
|
||||
loop.stop();
|
||||
});
|
||||
|
||||
it("routes through validator after assertion backfill instead of no-assertion auto-pass", async () => {
|
||||
it("keeps backfill optional because runtime lazy-ensure routes through validator", async () => {
|
||||
const feature = makeFeature({ status: "done", acceptanceCriteria: "must pass", loopState: "implementing" });
|
||||
const currentFeature = { ...feature };
|
||||
const linkedAssertions: Array<{ id: string }> = [];
|
||||
@@ -258,6 +267,12 @@ describe("FN-5715 reliability: mission validation trigger gap", () => {
|
||||
return { ...currentFeature };
|
||||
}),
|
||||
listAssertionsForFeature: vi.fn(() => linkedAssertions),
|
||||
ensureFeatureAssertionLinked: vi.fn(() => {
|
||||
if (linkedAssertions.length === 0) {
|
||||
linkedAssertions.push({ id: "CA-001" });
|
||||
}
|
||||
return linkedAssertions;
|
||||
}),
|
||||
startValidatorRun: vi.fn(() => ({ id: "VR-001", featureId: "F-001" })),
|
||||
completeValidatorRun: vi.fn(),
|
||||
getSlice: vi.fn(() => ({ id: "SL-001", milestoneId: "MS-001", status: "active" })),
|
||||
@@ -279,27 +294,13 @@ describe("FN-5715 reliability: mission validation trigger gap", () => {
|
||||
|
||||
await loop.recoverActiveMissions();
|
||||
|
||||
const noAssertionEventsBefore = missionStore.logMissionEvent.mock.calls.filter(
|
||||
([, type, , payload]) => type === "warning" && payload?.code === "validation_auto_passed_no_assertions",
|
||||
);
|
||||
expect(noAssertionEventsBefore).toHaveLength(1);
|
||||
expect(missionStore.startValidatorRun).not.toHaveBeenCalled();
|
||||
|
||||
linkedAssertions.push({ id: "CA-001" });
|
||||
currentFeature.loopState = "implementing";
|
||||
currentFeature.lastValidatorStatus = undefined;
|
||||
|
||||
await loop.processTaskOutcome("FN-001");
|
||||
|
||||
expect(missionStore.ensureFeatureAssertionLinked).toHaveBeenCalledWith("F-001");
|
||||
expect(missionStore.startValidatorRun).toHaveBeenCalledWith("F-001", "task_completion");
|
||||
const noAssertionEventsAfter = missionStore.logMissionEvent.mock.calls.filter(
|
||||
const noAssertionEvents = missionStore.logMissionEvent.mock.calls.filter(
|
||||
([, type, , payload]) => type === "warning" && payload?.code === "validation_auto_passed_no_assertions",
|
||||
);
|
||||
expect(noAssertionEventsAfter).toHaveLength(1);
|
||||
expect(missionStore.updateFeature).toHaveBeenCalledWith(
|
||||
"F-001",
|
||||
expect.objectContaining({ loopState: "passed", lastValidatorStatus: "passed" }),
|
||||
);
|
||||
expect(noAssertionEvents).toHaveLength(0);
|
||||
expect(missionStore.completeValidatorRun).toHaveBeenCalledWith("VR-001", "passed", "ok");
|
||||
|
||||
loop.stop();
|
||||
});
|
||||
|
||||
@@ -29,11 +29,14 @@ vi.mock("../../pr-monitor.js", () => ({ PrMonitor: vi.fn().mockImplementation(()
|
||||
vi.mock("../../pr-comment-handler.js", () => ({ PrCommentHandler: vi.fn().mockImplementation(() => ({ handleNewComments: vi.fn() })) }));
|
||||
vi.mock("../../auth-storage.js", () => ({
|
||||
createFusionAuthStorage: vi.fn(() => ({ reload: vi.fn(), getOAuthProviders: vi.fn(() => []), get: vi.fn(() => undefined) })),
|
||||
getFusionOAuthAlertStatePath: vi.fn(() => "/tmp/oauth-alert-state.json"),
|
||||
}));
|
||||
vi.mock("../../notifier.js", () => ({ NtfyNotifier: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })) }));
|
||||
vi.mock("../../notification/index.js", () => ({
|
||||
NotificationService: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
OAuthAlertStateStore: vi.fn().mockImplementation(() => ({})),
|
||||
OAuthExpiryMonitor: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
OAuthValidityLogger: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
}));
|
||||
vi.mock("../../cron-runner.js", () => ({
|
||||
CronRunner: vi.fn().mockImplementation(() => ({ start: vi.fn(), stop: vi.fn() })),
|
||||
|
||||
@@ -37,13 +37,12 @@ function buildParentScript(scenario: Scenario): string {
|
||||
const child = superviseSpawn(process.execPath, [${JSON.stringify(fixturePath)}, "keepalive"], {
|
||||
stdio: "ignore",
|
||||
killGraceMs: 50,
|
||||
maxLifetimeMs: 5_000,
|
||||
maxLifetimeMs: 500,
|
||||
});
|
||||
console.log(String(child.pid));
|
||||
await new Promise((resolve) => process.stdout.write(String(child.pid) + "\\n", resolve));
|
||||
if (${JSON.stringify(scenario)} === "clean-exit") {
|
||||
process.exit(0);
|
||||
}
|
||||
if (${JSON.stringify(scenario)} === "sigterm") {
|
||||
} else if (${JSON.stringify(scenario)} === "sigterm") {
|
||||
process.on("SIGTERM", () => process.exit(0));
|
||||
setInterval(() => {}, 1_000);
|
||||
} else {
|
||||
@@ -69,7 +68,7 @@ async function spawnParent(scenario: Scenario): Promise<{ parent: ReturnType<typ
|
||||
const childPid = await new Promise<number>((resolve, reject) => {
|
||||
const timeout = setTimeout(() => {
|
||||
reject(new Error(`Timed out waiting for supervised child pid from scenario ${scenario}`));
|
||||
}, 5_000);
|
||||
}, 15_000);
|
||||
|
||||
const onData = (chunk: Buffer | string) => {
|
||||
stdout += chunk.toString();
|
||||
@@ -101,14 +100,18 @@ async function spawnParent(scenario: Scenario): Promise<{ parent: ReturnType<typ
|
||||
describe("reliability interactions: FN-5189 verification spawn supervision", () => {
|
||||
const spawnedParents = new Set<ReturnType<typeof spawn>>();
|
||||
|
||||
afterEach(() => {
|
||||
afterEach(async () => {
|
||||
for (const parent of spawnedParents) {
|
||||
if (parent.exitCode === null && parent.signalCode === null) {
|
||||
// Register the exit listener BEFORE kill so we don't miss the
|
||||
// event and deadlock.
|
||||
const exited = once(parent, "exit");
|
||||
try {
|
||||
parent.kill("SIGKILL");
|
||||
} catch {
|
||||
// ignore cleanup failures
|
||||
}
|
||||
await exited.catch(() => {});
|
||||
}
|
||||
}
|
||||
spawnedParents.clear();
|
||||
@@ -117,6 +120,8 @@ describe("reliability interactions: FN-5189 verification spawn supervision", ()
|
||||
const caseIt = process.platform === "win32" ? it.skip : it;
|
||||
|
||||
caseIt.each([
|
||||
// Cover all parent teardown surfaces from FN-5893: normal exit, signal-driven exit,
|
||||
// and crash exit should all reap the supervised keepalive child within the guard window.
|
||||
["clean-exit"],
|
||||
["sigterm"],
|
||||
["uncaught-exception"],
|
||||
|
||||
@@ -1298,9 +1298,35 @@ describe("Crash scenario edge cases", () => {
|
||||
|
||||
it("concurrent resumeOrphaned() calls don't double-execute the same task", async () => {
|
||||
const store = createMockStore();
|
||||
const task = makeTask("FN-092", "in-progress");
|
||||
const rootDir = "/private/tmp/test";
|
||||
const worktreePath = `${rootDir}/.worktrees/swift-falcon`;
|
||||
const task = makeTask("FN-092", "in-progress", {
|
||||
worktree: worktreePath,
|
||||
branch: "fusion/fn-092",
|
||||
});
|
||||
store.listTasks.mockResolvedValue([task]);
|
||||
store.getTask.mockResolvedValue(makeTaskDetail("FN-092", "in-progress"));
|
||||
store.getTask.mockResolvedValue(makeTaskDetail("FN-092", "in-progress", {
|
||||
worktree: worktreePath,
|
||||
branch: "fusion/fn-092",
|
||||
}));
|
||||
mockedExecSync.mockImplementation(((cmd: unknown) => {
|
||||
if (String(cmd) === "git rev-parse --is-inside-work-tree") {
|
||||
return "true\n" as any;
|
||||
}
|
||||
if (String(cmd) === "git worktree list --porcelain") {
|
||||
return [
|
||||
`worktree ${rootDir}`,
|
||||
"HEAD abc123",
|
||||
"branch refs/heads/main",
|
||||
"",
|
||||
`worktree ${worktreePath}`,
|
||||
"HEAD def456",
|
||||
"branch refs/heads/fusion/fn-092",
|
||||
"",
|
||||
].join("\n") as any;
|
||||
}
|
||||
return Buffer.from("");
|
||||
}) as any);
|
||||
|
||||
let resolvePrompt: (() => void) | undefined;
|
||||
mockedCreateFnAgent.mockResolvedValue({
|
||||
@@ -1310,7 +1336,7 @@ describe("Crash scenario edge cases", () => {
|
||||
},
|
||||
} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const executor = new TaskExecutor(store, rootDir);
|
||||
|
||||
// First call starts execution
|
||||
const first = executor.resumeOrphaned();
|
||||
|
||||
@@ -291,6 +291,23 @@ describe("reviewStep — spec review type", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("FN-5928 surface-enumeration review-gate wording", () => {
|
||||
it("requires spec reviews to block missing or incomplete surface enumeration for bug-fix specs", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("**Surface enumeration:**");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Missing or incomplete coverage is a blocking REVISE");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("desktop + mobile breakpoints/platforms");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("shared hooks/components/modules/helpers");
|
||||
});
|
||||
|
||||
it("requires code reviews to reject repro-only regression tests for bug fixes", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("repro-only regression test");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("spanning the `## Surface Enumeration` checklist");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5787/FN-5789/FN-5803");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5797/FN-5875/FN-5919");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5751");
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — context-limit retry", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -97,7 +97,7 @@ vi.mock("../merger.js", () => ({
|
||||
classifyOwnedLandedEvidence: vi.fn(),
|
||||
}));
|
||||
|
||||
import { SelfHealingManager, isBranchAheadOfBase } from "../self-healing.js";
|
||||
import { SelfHealingManager, isBranchAheadOfBase, MAX_AUTO_MERGE_RETRIES } from "../self-healing.js";
|
||||
import type { TaskStore, Settings, Task, AgentStore, Agent, NotificationProvider } from "@fusion/core";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { execSync } from "node:child_process";
|
||||
@@ -3365,7 +3365,8 @@ describe("SelfHealingManager", () => {
|
||||
const result = await managerWithRecovery.recoverMergeableReviewTasks();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.listTasks).not.toHaveBeenCalled();
|
||||
// The sweep may list tasks to discover per-task autoMerge overrides,
|
||||
// but must not merge or enqueue anything without one.
|
||||
expect(store.mergeTask).not.toHaveBeenCalled();
|
||||
expect(enqueueMerge).not.toHaveBeenCalled();
|
||||
|
||||
@@ -3747,7 +3748,10 @@ describe("SelfHealingManager", () => {
|
||||
const result = await managerWithRecovery.finalizeNoOpReviewTasks();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.listTasks).not.toHaveBeenCalled();
|
||||
// The sweep may list tasks to discover per-task autoMerge overrides,
|
||||
// but must not finalize anything without one.
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
@@ -8208,6 +8212,153 @@ describe("autoMerge gating for mutating in-review sweeps (FN-5147)", () => {
|
||||
taskStuckTimeoutMs: 1_000,
|
||||
maxPostReviewFixes: 1,
|
||||
});
|
||||
|
||||
// Seed real, stale in-review sweep candidates with NO per-task autoMerge
|
||||
// override. Each fixture matches a distinct covered sweep's candidate shape
|
||||
// and would be mutated if the per-task gate (allowsAutoMergeProcessing) were
|
||||
// ignored. Because the global setting is autoMerge:false and none of these
|
||||
// carry autoMerge:true, every sweep must enumerate them and skip them solely
|
||||
// due to the gate — which is the regression under test. The gate is the
|
||||
// first/early filter in each sweep, so candidates are dropped before any
|
||||
// store.getTask / git helper is reached.
|
||||
const stale = new Date(Date.now() - 600_000).toISOString();
|
||||
const seededInReviewCandidates = [
|
||||
// recoverStaleIncompleteReviewTasks + recoverGhostReviewTasks:
|
||||
// idle in-review with incomplete steps, stale.
|
||||
{
|
||||
id: "FN-GATE-INCOMPLETE",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
steps: [{ status: "pending" }],
|
||||
log: [],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverInterruptedMergingTasks: stale `merging` status.
|
||||
{
|
||||
id: "FN-GATE-MERGING",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "merging",
|
||||
steps: [],
|
||||
log: [],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverMergedReviewTasks + recoverGhostReviewTasks(skip merge-confirmed):
|
||||
// mergeConfirmed:true stuck in in-review.
|
||||
{
|
||||
id: "FN-GATE-MERGED",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
steps: [],
|
||||
log: [],
|
||||
mergeDetails: { mergeConfirmed: true },
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverStuckMergeDeadlocks + recoverAlreadyMergedReviewTasks +
|
||||
// recoverOrphanOnlyScopeViolations: failed in-review, retries exhausted,
|
||||
// worktree present.
|
||||
{
|
||||
id: "FN-GATE-FAILED",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "failed",
|
||||
steps: [],
|
||||
log: [],
|
||||
mergeRetries: MAX_AUTO_MERGE_RETRIES,
|
||||
worktree: "/tmp/test-project/.worktrees/FN-GATE-FAILED",
|
||||
branch: "fn/FN-GATE-FAILED",
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverReviewTasksWithFailedPreMergeSteps: idle in-review whose merge is
|
||||
// blocked specifically by a failed pre-merge workflow step, worktree set.
|
||||
{
|
||||
id: "FN-GATE-PREMERGE",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
steps: [],
|
||||
log: [],
|
||||
worktree: "/tmp/test-project/.worktrees/FN-GATE-PREMERGE",
|
||||
workflowStepResults: [{ phase: "pre-merge", status: "failed" }],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverMissingWorktreeReviewFailures: failed by missing-worktree session
|
||||
// start, with step progress.
|
||||
{
|
||||
id: "FN-GATE-MISSINGWT",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "failed",
|
||||
error: "Refusing to start coding agent in missing worktree: /tmp/gone",
|
||||
steps: [{ status: "done" }],
|
||||
log: [],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverPartialProgressNoTaskDoneFailures: failed without fn_task_done,
|
||||
// partial step progress, not work-complete, retries available.
|
||||
{
|
||||
id: "FN-GATE-NOTASKDONE",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "failed",
|
||||
error: "Agent finished without calling fn_task_done",
|
||||
steps: [{ status: "done" }, { status: "pending" }],
|
||||
log: [],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverForeignOnlyContaminatedInReviewTasks: in-review with branch +
|
||||
// worktree, not merge-confirmed.
|
||||
{
|
||||
id: "FN-GATE-FOREIGN",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
branch: "fn/FN-GATE-FOREIGN",
|
||||
worktree: "/tmp/test-project/.worktrees/FN-GATE-FOREIGN",
|
||||
steps: [],
|
||||
log: [],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// recoverCompletionHandoffLimbo: idle in-review with no status/mergeDetails/
|
||||
// review, an aged "Task marked done by agent" log marker, no merge blocker.
|
||||
{
|
||||
id: "FN-GATE-HANDOFF",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
steps: [],
|
||||
log: [{ action: "Task marked done by agent", timestamp: stale }],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
// reclaimSelfOwnedBranchConflicts: in-review branch-conflict-unrecoverable.
|
||||
// (No worktree, so even absent the gate it is skipped before any git call;
|
||||
// the gate is what the assertions verify.)
|
||||
{
|
||||
id: "FN-GATE-RECLAIM",
|
||||
column: "in-review",
|
||||
paused: true,
|
||||
pausedReason: "branch-conflict-unrecoverable",
|
||||
branch: "fn/FN-GATE-RECLAIM",
|
||||
steps: [],
|
||||
log: [],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
},
|
||||
] as unknown as Task[];
|
||||
|
||||
// Resolve fixtures only for the in-review column the sweeps enumerate; other
|
||||
// columns (todo / in-progress / triage) stay empty so the non-auto-merge-
|
||||
// gated branches of reclaim/foreign-only sweeps don't reach git helpers.
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockImplementation(
|
||||
async (opts?: { column?: string }) =>
|
||||
opts?.column === "in-review" ? seededInReviewCandidates : [],
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -8227,26 +8378,105 @@ describe("autoMerge gating for mutating in-review sweeps (FN-5147)", () => {
|
||||
"recoverMissingWorktreeReviewFailures",
|
||||
"recoverPartialProgressNoTaskDoneFailures",
|
||||
"reclaimSelfOwnedBranchConflicts",
|
||||
] as const)("skips entirely when autoMerge is disabled (respects PR-based review flow): %s", async (methodName) => {
|
||||
] as const)("performs no mutations when autoMerge is disabled and no per-task override exists: %s", async (methodName) => {
|
||||
if (methodName === "recoverReviewTasksWithFailedPreMergeSteps") {
|
||||
manager = new SelfHealingManager(store, { rootDir: "/tmp/test-project", recoverFailedPreMergeStep: vi.fn() });
|
||||
}
|
||||
const result = await (manager as any)[methodName]();
|
||||
expect(result).toBe(0);
|
||||
expect(store.listTasks).not.toHaveBeenCalled();
|
||||
// Enumeration must have happened: the sweep listed real, stale in-review
|
||||
// candidates seeded above. Mutations are skipped solely because of the
|
||||
// per-task auto-merge gate (respects PR-based review flow) — so these
|
||||
// assertions are non-vacuous.
|
||||
expect(store.listTasks).toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips entirely when autoMerge is disabled (respects PR-based review flow): recoverCompletionHandoffLimbo", async () => {
|
||||
it("performs no mutations when autoMerge is disabled and no per-task override exists: recoverCompletionHandoffLimbo", async () => {
|
||||
const result = await manager.recoverCompletionHandoffLimbo();
|
||||
expect(result).toBeUndefined();
|
||||
expect(store.listTasks).not.toHaveBeenCalled();
|
||||
// The seeded FN-GATE-HANDOFF candidate carries an aged "Task marked done by
|
||||
// agent" marker and no merge blocker, so the sweep enumerates it and would
|
||||
// requeue/fail it absent the per-task gate.
|
||||
expect(store.listTasks).toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("surfaces in-review stalls for tasks with an explicit autoMerge:true override when the global setting is off", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
vi.setSystemTime(new Date("2026-01-01T00:10:00.000Z"));
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
autoMerge: false,
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
taskStuckTimeoutMs: 60_000,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-OVERRIDE",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "merging",
|
||||
autoMerge: true,
|
||||
steps: [],
|
||||
log: [],
|
||||
updatedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(),
|
||||
columnMovedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(),
|
||||
},
|
||||
]);
|
||||
|
||||
const surfaced = await manager.surfaceInReviewStalls();
|
||||
|
||||
expect(surfaced).toBe(1);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-OVERRIDE",
|
||||
expect.stringContaining("In-review stall surfaced ["),
|
||||
);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps skipping override-less siblings while processing the override task", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
vi.setSystemTime(new Date("2026-01-01T00:10:00.000Z"));
|
||||
const staleFields = {
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "merging",
|
||||
steps: [],
|
||||
log: [],
|
||||
updatedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(),
|
||||
columnMovedAt: new Date(Date.parse("2026-01-01T00:10:00.000Z") - 600_000).toISOString(),
|
||||
};
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
autoMerge: false,
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
taskStuckTimeoutMs: 60_000,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{ id: "FN-OVERRIDE", autoMerge: true, ...staleFields },
|
||||
{ id: "FN-MANUAL", ...staleFields },
|
||||
]);
|
||||
|
||||
const surfaced = await manager.surfaceInReviewStalls();
|
||||
|
||||
expect(surfaced).toBe(1);
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith(
|
||||
"FN-MANUAL",
|
||||
expect.stringContaining("In-review stall surfaced ["),
|
||||
);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("FN-5335 triple-proof no-action unit coverage", () => {
|
||||
|
||||
@@ -648,12 +648,12 @@ describe("TRIAGE_SYSTEM_PROMPT", () => {
|
||||
});
|
||||
|
||||
describe("FN-5893 invariant regression wording", () => {
|
||||
it("requires invariant-level regression coverage in standard, fast, and core triage prompts", () => {
|
||||
const corePromptSource = readFileSync(
|
||||
fileURLToPath(new URL("../../../core/src/agent-prompts.ts", import.meta.url)),
|
||||
"utf8",
|
||||
);
|
||||
const corePromptSource = readFileSync(
|
||||
fileURLToPath(new URL("../../../core/src/agent-prompts.ts", import.meta.url)),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
it("requires invariant-level regression coverage in standard, fast, and core triage prompts", () => {
|
||||
for (const prompt of [
|
||||
TRIAGE_SYSTEM_PROMPT,
|
||||
FAST_TRIAGE_SYSTEM_PROMPT,
|
||||
@@ -668,13 +668,44 @@ describe("FN-5893 invariant regression wording", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("requires a Surface Enumeration section and blocking REVISE guidance for bug-fix specs", () => {
|
||||
for (const prompt of [TRIAGE_SYSTEM_PROMPT, FAST_TRIAGE_SYSTEM_PROMPT]) {
|
||||
expect(prompt).toContain("## Surface Enumeration");
|
||||
expect(prompt).toContain("spec MUST include a `## Surface Enumeration` section");
|
||||
expect(prompt).toContain("blocking REVISE");
|
||||
expect(prompt).toContain("docs/testing.md");
|
||||
expect(prompt).toContain("duplicate / populated data states");
|
||||
expect(prompt).toContain("shared hooks/components/modules/helpers");
|
||||
}
|
||||
|
||||
expect(corePromptSource).toContain("## Surface Enumeration");
|
||||
expect(corePromptSource).toContain("spec MUST include a \\`## Surface Enumeration\\` section");
|
||||
expect(corePromptSource).toContain("blocking REVISE");
|
||||
expect(corePromptSource).toContain("docs/testing.md");
|
||||
expect(corePromptSource).toContain("duplicate / populated data states");
|
||||
expect(corePromptSource).toContain("shared hooks/components/modules/helpers");
|
||||
});
|
||||
|
||||
it("requires implementation-step testing guidance to enumerate invariant surfaces in standard and fast prompts", () => {
|
||||
for (const prompt of [TRIAGE_SYSTEM_PROMPT, FAST_TRIAGE_SYSTEM_PROMPT]) {
|
||||
expect(prompt).toContain(
|
||||
"Run targeted tests for changed files, asserting the invariant across all known surfaces",
|
||||
);
|
||||
expect(prompt).toContain(
|
||||
"For bug-fix tasks, paste and fill in this checklist in the `## Surface Enumeration` section",
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("pins the canonical docs checklist heading", () => {
|
||||
const docsTestingSource = readFileSync(
|
||||
fileURLToPath(new URL("../../../../docs/testing.md", import.meta.url)),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
expect(docsTestingSource).toContain("### Surface Enumeration checklist");
|
||||
expect(docsTestingSource).toContain("Providers / bridges / execution paths touched by the invariant");
|
||||
});
|
||||
});
|
||||
|
||||
describe("fast-mode triage", () => {
|
||||
|
||||
@@ -16,7 +16,7 @@ import type { OAuthCredentials } from "@earendil-works/pi-ai/oauth";
|
||||
|
||||
type StoredCredential = StoredAuthCredential;
|
||||
|
||||
function getHomeDir(): string {
|
||||
export function getHomeDir(): string {
|
||||
return process.env.HOME || process.env.USERPROFILE || homedir();
|
||||
}
|
||||
|
||||
@@ -24,6 +24,10 @@ export function getFusionAuthPath(home = getHomeDir()): string {
|
||||
return join(home, ".fusion", "agent", "auth.json");
|
||||
}
|
||||
|
||||
export function getFusionOAuthAlertStatePath(home = getHomeDir()): string {
|
||||
return join(home, ".fusion", "agent", "oauth-alert-state.json");
|
||||
}
|
||||
|
||||
export function getFusionModelsPath(home = getHomeDir()): string {
|
||||
return join(home, ".fusion", "agent", "models.json");
|
||||
}
|
||||
|
||||
@@ -495,14 +495,23 @@ export function hasInstallState(rootDir: string): boolean {
|
||||
export function shouldSyncDependenciesForMerge(
|
||||
stagedFiles: string[],
|
||||
installStatePresent: boolean,
|
||||
hasConfiguredInitCommand = false,
|
||||
): boolean {
|
||||
if (hasConfiguredInitCommand) return true;
|
||||
if (!installStatePresent) return true;
|
||||
return stagedFiles.some((file) =>
|
||||
DEPENDENCY_SYNC_TRIGGER_PATTERNS.some((pattern) => matchGlob(file, pattern)),
|
||||
);
|
||||
}
|
||||
|
||||
function getDependencySyncCommand(rootDir: string): string | null {
|
||||
function getConfiguredWorktreeInitCommand(settings?: Settings | null): string | null {
|
||||
const trimmed = settings?.worktreeInitCommand?.trim();
|
||||
return trimmed ? trimmed : null;
|
||||
}
|
||||
|
||||
function getDependencySyncCommand(rootDir: string, settings?: Settings | null): string | null {
|
||||
const configuredCommand = getConfiguredWorktreeInitCommand(settings);
|
||||
if (configuredCommand) return configuredCommand;
|
||||
if (existsSync(join(rootDir, "pnpm-lock.yaml"))) return "pnpm install --frozen-lockfile";
|
||||
if (existsSync(join(rootDir, "package-lock.json"))) return "npm install";
|
||||
if (existsSync(join(rootDir, "yarn.lock"))) return "yarn install --frozen-lockfile";
|
||||
@@ -550,17 +559,21 @@ async function syncDependenciesForMerge(
|
||||
store: TaskStore,
|
||||
rootDir: string,
|
||||
taskId: string,
|
||||
settings?: Settings | null,
|
||||
signal?: AbortSignal,
|
||||
): Promise<void> {
|
||||
const installCommand = getDependencySyncCommand(rootDir);
|
||||
const configuredCommand = getConfiguredWorktreeInitCommand(settings);
|
||||
const installCommand = getDependencySyncCommand(rootDir, settings);
|
||||
if (!installCommand) return;
|
||||
|
||||
const shouldUseInstallMarker = configuredCommand === null;
|
||||
|
||||
// Skip the install if node_modules is present and the lockfile content
|
||||
// matches the hash recorded after the last successful install. Caller's
|
||||
// shouldSyncDependenciesForMerge gate already filters most no-ops; this
|
||||
// covers the case where package.json (but not the lockfile) is staged, and
|
||||
// the case where multiple merge attempts hit the same worktree in a row.
|
||||
const lockHash = computeLockfileHash(rootDir);
|
||||
const lockHash = shouldUseInstallMarker ? computeLockfileHash(rootDir) : null;
|
||||
if (lockHash && hasInstallState(rootDir) && readInstallMarker(rootDir) === lockHash) {
|
||||
mergerLog.log(`${taskId}: skipping dependency sync (lockfile unchanged since last install)`);
|
||||
await store.logEntry(
|
||||
@@ -10947,8 +10960,15 @@ export async function executeMergeAttempt(
|
||||
if (testCommand || buildCommand) {
|
||||
throwIfAborted(options.signal, taskId);
|
||||
const stagedFiles = await getStagedFiles(rootDir);
|
||||
if (shouldSyncDependenciesForMerge(stagedFiles, hasInstallState(rootDir))) {
|
||||
await syncDependenciesForMerge(store, rootDir, taskId, options.signal);
|
||||
const configuredMergeInitCommand = getConfiguredWorktreeInitCommand(settings as Settings);
|
||||
if (
|
||||
shouldSyncDependenciesForMerge(
|
||||
stagedFiles,
|
||||
hasInstallState(rootDir),
|
||||
configuredMergeInitCommand !== null,
|
||||
)
|
||||
) {
|
||||
await syncDependenciesForMerge(store, rootDir, taskId, settings as Settings, options.signal);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ import type {
|
||||
MissionValidatorRun,
|
||||
AgentStore,
|
||||
Settings,
|
||||
Milestone,
|
||||
} from "@fusion/core";
|
||||
import {
|
||||
TEST_MODE_RESOLVED,
|
||||
@@ -402,17 +403,16 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
* Shared by processTaskOutcome (task-triggered) and recoverActiveMissions
|
||||
* (self-healing for features stranded mid-loop with no board task). Callers
|
||||
* are responsible for confirming the feature is eligible to validate; this
|
||||
* method handles the no-assertion auto-pass, validator run bookkeeping, and
|
||||
* method handles lazy assertion linkage, validator run bookkeeping, and
|
||||
* dispatch of the validation result.
|
||||
*/
|
||||
private async runFeatureValidation(feature: MissionFeature): Promise<void> {
|
||||
// Get linked assertions for this feature
|
||||
const assertions = this.missionStore.listAssertionsForFeature(feature.id);
|
||||
// Lazily guarantee a linked assertion before validation so every feature
|
||||
// is evaluated by the validator even when legacy data is missing links.
|
||||
let assertions = this.missionStore.listAssertionsForFeature(feature.id);
|
||||
if (assertions.length === 0) {
|
||||
loopLog.log(`Feature ${feature.id} has no linked assertions; marking as passed`);
|
||||
// No assertions = automatically pass
|
||||
await this.handleValidationPass(feature.id, undefined, "No assertions linked");
|
||||
return;
|
||||
loopLog.log(`Feature ${feature.id} has no linked assertions; lazily ensuring store-managed assertion linkage`);
|
||||
assertions = this.missionStore.ensureFeatureAssertionLinked(feature.id);
|
||||
}
|
||||
|
||||
// Mark feature as being validated
|
||||
@@ -457,8 +457,10 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
): Promise<ValidationResult> {
|
||||
loopLog.log(`Running validation for feature ${feature.id} with ${assertions.length} assertions`);
|
||||
|
||||
const milestone = this.resolveFeatureMilestone(feature);
|
||||
|
||||
// Build the validation prompt
|
||||
const prompt = this.buildValidationPrompt(feature, assertions);
|
||||
const prompt = this.buildValidationPrompt(feature, assertions, milestone);
|
||||
|
||||
// Get task context for validation
|
||||
const task = feature.taskId ? await this.taskStore.getTask(feature.taskId) : null;
|
||||
@@ -490,7 +492,7 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
runtimeHint: validationRuntimeHint,
|
||||
pluginRunner: this.pluginRunner,
|
||||
cwd: this.rootDir,
|
||||
systemPrompt: this.buildValidationSystemPrompt(feature, assertions, taskContext),
|
||||
systemPrompt: this.buildValidationSystemPrompt(feature, assertions, taskContext, milestone),
|
||||
tools: "readonly",
|
||||
defaultProvider: validationSessionModel.provider,
|
||||
defaultModelId: validationSessionModel.modelId,
|
||||
@@ -845,19 +847,27 @@ export class MissionExecutionLoop extends EventEmitter {
|
||||
/**
|
||||
* Build the validation prompt sent to the AI agent.
|
||||
*/
|
||||
private buildValidationPrompt(feature: MissionFeature, assertions: MissionContractAssertion[]): string {
|
||||
private buildValidationPrompt(
|
||||
feature: MissionFeature,
|
||||
assertions: MissionContractAssertion[],
|
||||
milestone?: Milestone,
|
||||
): string {
|
||||
const assertionTexts = assertions
|
||||
.map((a, i) => `${i + 1}. **${a.title}**: ${a.assertion}`)
|
||||
.join("\n");
|
||||
const milestoneAcceptanceCriteria = milestone?.acceptanceCriteria?.trim();
|
||||
const milestoneContext = milestoneAcceptanceCriteria
|
||||
? `\nMilestone acceptance criteria (must also be satisfied for this feature to pass):\n${milestoneAcceptanceCriteria}\n`
|
||||
: "";
|
||||
|
||||
return `Evaluate the implementation for feature "${feature.title}" against the following contract assertions:
|
||||
|
||||
${assertionTexts}
|
||||
|
||||
${assertionTexts}${milestoneContext}
|
||||
For each assertion:
|
||||
- Determine if the implementation satisfies the assertion (pass/fail/blocked)
|
||||
- If failed, explain what was expected vs what was actually observed
|
||||
- If blocked, explain what external factor prevented validation
|
||||
- Also verify that the implementation satisfies any milestone acceptance criteria provided above
|
||||
|
||||
Respond with a JSON object in this format:
|
||||
{
|
||||
@@ -882,22 +892,25 @@ Be thorough and objective. If any assertion fails, the overall status should be
|
||||
* Build the system prompt for the validation agent.
|
||||
*/
|
||||
private buildValidationSystemPrompt(
|
||||
feature: MissionFeature,
|
||||
_feature: MissionFeature,
|
||||
_assertions: MissionContractAssertion[],
|
||||
taskContext: string,
|
||||
milestone?: Milestone,
|
||||
): string {
|
||||
const milestoneAcceptanceCriteria = milestone?.acceptanceCriteria?.trim();
|
||||
return `You are a validation agent responsible for evaluating whether an implementation satisfies its contract assertions.
|
||||
|
||||
You will receive:
|
||||
1. A feature description with its acceptance criteria
|
||||
2. Contract assertions to evaluate against
|
||||
3. Task context including the implementation details
|
||||
3. Task context including the implementation details${milestoneAcceptanceCriteria ? `\n4. Milestone acceptance criteria text that also applies to this feature: ${milestoneAcceptanceCriteria}` : ""}
|
||||
|
||||
Your job is to:
|
||||
1. Carefully review the implementation as described in the task context
|
||||
2. Evaluate each contract assertion objectively
|
||||
3. Determine if the implementation fully satisfies each assertion
|
||||
4. Return a structured JSON response with your findings
|
||||
4. Verify the implementation also satisfies any milestone acceptance criteria provided for the parent milestone
|
||||
5. Return a structured JSON response with your findings
|
||||
|
||||
Be thorough and precise. A contract assertion represents a commitment made during planning - the implementation must fully satisfy it or it is considered failed.
|
||||
|
||||
@@ -906,6 +919,7 @@ Evaluation guidance:
|
||||
- "fail" means one or more assertions are unmet or only partially satisfied.
|
||||
- "blocked" means you cannot evaluate due to missing/insufficient evidence or external constraints.
|
||||
- Partial satisfaction must be marked as failed with clear expected vs actual details.
|
||||
- Milestone acceptance criteria are validator-executed requirements, not informational context.
|
||||
|
||||
Response format: Return ONLY a JSON object (no additional text) with this structure:
|
||||
{
|
||||
@@ -947,6 +961,15 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
private resolveFeatureMilestone(feature: MissionFeature): Milestone | undefined {
|
||||
const slice = this.missionStore.getSlice(feature.sliceId);
|
||||
if (!slice) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return this.missionStore.getMilestone(slice.milestoneId);
|
||||
}
|
||||
|
||||
private completeValidatorRunIfStillRunning(
|
||||
runId: string | undefined,
|
||||
status: "passed" | "failed" | "blocked" | "error",
|
||||
@@ -987,33 +1010,6 @@ ${taskContext ? `\n\nImplementation context:\n${taskContext}` : ""}`;
|
||||
this.missionStore.updateFeatureStatus(featureId, "done");
|
||||
}
|
||||
|
||||
if (!runId && feature) {
|
||||
const alreadyAutoPassed =
|
||||
feature.status === "done" &&
|
||||
feature.loopState === "passed" &&
|
||||
feature.lastValidatorStatus === "passed";
|
||||
|
||||
if (!alreadyAutoPassed) {
|
||||
// Auto-pass path has no validator run, so we must advance loop bookkeeping here.
|
||||
if (feature.loopState !== "passed" || feature.lastValidatorStatus !== "passed") {
|
||||
this.missionStore.updateFeature(featureId, {
|
||||
loopState: "passed",
|
||||
lastValidatorStatus: "passed",
|
||||
});
|
||||
}
|
||||
|
||||
this.logFeatureWarningEvent(
|
||||
featureId,
|
||||
"validation_auto_passed_no_assertions",
|
||||
`Feature ${featureId} auto-passed because no assertions were linked.`,
|
||||
{
|
||||
taskId: feature.taskId,
|
||||
reason: "No assertions linked",
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
loopLog.log(`Feature ${featureId} passed validation`);
|
||||
|
||||
// Notify autopilot if configured
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { OAuthAlertStateStore } from "../oauth-alert-state.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
function createTempStatePath(): string {
|
||||
const dir = mkdtempSync(join(tmpdir(), "oauth-alert-state-"));
|
||||
tempDirs.push(dir);
|
||||
return join(dir, "oauth-alert-state.json");
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
rmSync(dir, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
describe("OAuthAlertStateStore", () => {
|
||||
it("round-trips provider alert state using the configured path", () => {
|
||||
const statePath = createTempStatePath();
|
||||
const store = new OAuthAlertStateStore({ statePath, clock: () => 1234 });
|
||||
|
||||
store.recordAlert("openai-codex", 9999);
|
||||
|
||||
const reloaded = new OAuthAlertStateStore({ statePath });
|
||||
expect(reloaded.get("openai-codex")).toEqual({ expires: 9999, lastAlertAt: 1234 });
|
||||
});
|
||||
|
||||
it("returns empty state when the file is missing or corrupt", () => {
|
||||
const missingPath = createTempStatePath();
|
||||
const missingStore = new OAuthAlertStateStore({ statePath: missingPath });
|
||||
expect(missingStore.get("openai-codex")).toBeUndefined();
|
||||
|
||||
const corruptPath = createTempStatePath();
|
||||
writeFileSync(corruptPath, "{not json", "utf-8");
|
||||
const corruptStore = new OAuthAlertStateStore({ statePath: corruptPath });
|
||||
expect(corruptStore.get("openai-codex")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("persists only provider ids with expires and lastAlertAt", () => {
|
||||
const statePath = createTempStatePath();
|
||||
const store = new OAuthAlertStateStore({ statePath, clock: () => 5678 });
|
||||
|
||||
store.recordAlert("claude", 4321);
|
||||
|
||||
expect(JSON.parse(readFileSync(statePath, "utf-8"))).toEqual({
|
||||
claude: {
|
||||
expires: 4321,
|
||||
lastAlertAt: 5678,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("clears selected providers and all providers", () => {
|
||||
const statePath = createTempStatePath();
|
||||
const store = new OAuthAlertStateStore({ statePath, clock: () => 100 });
|
||||
|
||||
store.recordAlert("claude", 1_000);
|
||||
store.recordAlert("openai-codex", 2_000, 200);
|
||||
store.clear(["claude"]);
|
||||
|
||||
const afterSingleClear = new OAuthAlertStateStore({ statePath });
|
||||
expect(afterSingleClear.get("claude")).toBeUndefined();
|
||||
expect(afterSingleClear.get("openai-codex")).toEqual({ expires: 2_000, lastAlertAt: 200 });
|
||||
|
||||
afterSingleClear.clear();
|
||||
expect(new OAuthAlertStateStore({ statePath }).get("openai-codex")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,18 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { OAuthAlertStateStore } from "../oauth-alert-state.js";
|
||||
import { OAuthExpiryMonitor, type AuthStorageLike } from "../oauth-expiry-monitor.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
function createStatePath(): string {
|
||||
const dir = mkdtempSync(join(tmpdir(), "oauth-expiry-monitor-"));
|
||||
tempDirs.push(dir);
|
||||
return join(dir, "oauth-alert-state.json");
|
||||
}
|
||||
|
||||
function createAuthStorage(initialCredential?: { type?: string; expires?: number }): AuthStorageLike & {
|
||||
credential: { type?: string; expires?: number } | undefined;
|
||||
} {
|
||||
@@ -17,17 +29,26 @@ function createAuthStorage(initialCredential?: { type?: string; expires?: number
|
||||
};
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
rmSync(dir, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
describe("OAuthExpiryMonitor", () => {
|
||||
it("fires once when an OAuth credential is expired", async () => {
|
||||
vi.useFakeTimers();
|
||||
const authStorage = createAuthStorage({ type: "oauth", expires: Date.now() - 1_000 });
|
||||
const now = Date.now();
|
||||
const authStorage = createAuthStorage({ type: "oauth", expires: now - 1_000 });
|
||||
const dispatch = vi.fn(async () => undefined);
|
||||
|
||||
const monitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: { dispatch } as any,
|
||||
intervalMs: 100,
|
||||
clock: () => Date.now(),
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
|
||||
await monitor.start();
|
||||
@@ -45,7 +66,6 @@ describe("OAuthExpiryMonitor", () => {
|
||||
}),
|
||||
);
|
||||
monitor.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("does not fire for non-expired/non-oauth credentials", async () => {
|
||||
@@ -67,6 +87,7 @@ describe("OAuthExpiryMonitor", () => {
|
||||
notificationService: { dispatch } as any,
|
||||
intervalMs: 100,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
|
||||
await monitor.start();
|
||||
@@ -75,7 +96,6 @@ describe("OAuthExpiryMonitor", () => {
|
||||
}
|
||||
|
||||
expect(dispatch).not.toHaveBeenCalled();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("deduplicates dispatches for same provider and expiry", async () => {
|
||||
@@ -89,6 +109,7 @@ describe("OAuthExpiryMonitor", () => {
|
||||
notificationService: { dispatch } as any,
|
||||
intervalMs: 100,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
|
||||
await monitor.start();
|
||||
@@ -96,12 +117,12 @@ describe("OAuthExpiryMonitor", () => {
|
||||
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
monitor.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("re-fires after credential is replaced with a new expiry that later expires", async () => {
|
||||
vi.useFakeTimers();
|
||||
let now = Date.now();
|
||||
const statePath = createStatePath();
|
||||
const authStorage = createAuthStorage({ type: "oauth", expires: now - 1 });
|
||||
const dispatch = vi.fn(async () => undefined);
|
||||
|
||||
@@ -110,6 +131,7 @@ describe("OAuthExpiryMonitor", () => {
|
||||
notificationService: { dispatch } as any,
|
||||
intervalMs: 100,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
|
||||
await monitor.start();
|
||||
@@ -128,42 +150,126 @@ describe("OAuthExpiryMonitor", () => {
|
||||
|
||||
expect(dispatch).toHaveBeenCalledTimes(2);
|
||||
monitor.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("throttles changed expiries until min notify interval elapses", async () => {
|
||||
it("throttles changed expiries until min notify interval elapses across restarts", async () => {
|
||||
vi.useFakeTimers();
|
||||
let now = Date.now();
|
||||
const statePath = createStatePath();
|
||||
const authStorage = createAuthStorage({ type: "oauth", expires: now - 1 });
|
||||
const dispatch = vi.fn(async () => undefined);
|
||||
|
||||
const monitor = new OAuthExpiryMonitor({
|
||||
const firstMonitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: { dispatch } as any,
|
||||
intervalMs: 100,
|
||||
minNotifyIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
|
||||
await monitor.start();
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
|
||||
authStorage.credential = { type: "oauth", expires: now + 10_000 };
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
|
||||
now += 500;
|
||||
authStorage.credential = { type: "oauth", expires: now - 1 };
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
|
||||
await firstMonitor.start();
|
||||
firstMonitor.stop();
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
|
||||
now += 500;
|
||||
authStorage.credential = { type: "oauth", expires: now - 2 };
|
||||
await vi.advanceTimersByTimeAsync(100);
|
||||
const restartedMonitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: { dispatch } as any,
|
||||
minNotifyIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
|
||||
await restartedMonitor.start();
|
||||
restartedMonitor.stop();
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
|
||||
now += 500;
|
||||
authStorage.credential = { type: "oauth", expires: now - 3 };
|
||||
await restartedMonitor.start();
|
||||
restartedMonitor.stop();
|
||||
expect(dispatch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("does not persist lastAlertAt when dispatch fails", async () => {
|
||||
let now = Date.now();
|
||||
const statePath = createStatePath();
|
||||
const authStorage = createAuthStorage({ type: "oauth", expires: now - 1 });
|
||||
const dispatch = vi.fn(async () => {
|
||||
throw new Error("boom");
|
||||
});
|
||||
|
||||
const firstMonitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: { dispatch } as any,
|
||||
minNotifyIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
await firstMonitor.start();
|
||||
firstMonitor.stop();
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
|
||||
const secondDispatch = vi.fn(async () => undefined);
|
||||
now += 100;
|
||||
const restartedMonitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: { dispatch: secondDispatch } as any,
|
||||
minNotifyIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
await restartedMonitor.start();
|
||||
restartedMonitor.stop();
|
||||
|
||||
expect(secondDispatch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("clears persisted state when providers disappear", async () => {
|
||||
let now = Date.now();
|
||||
const statePath = createStatePath();
|
||||
const authStorage = createAuthStorage({ type: "oauth", expires: now - 1 });
|
||||
const dispatch = vi.fn(async () => undefined);
|
||||
|
||||
const firstMonitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: { dispatch } as any,
|
||||
minNotifyIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
await firstMonitor.start();
|
||||
firstMonitor.stop();
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
|
||||
const noProviderStorage: AuthStorageLike = {
|
||||
reload: vi.fn(),
|
||||
getOAuthProviders: () => [],
|
||||
get: () => undefined,
|
||||
};
|
||||
const clearingMonitor = new OAuthExpiryMonitor({
|
||||
authStorage: noProviderStorage,
|
||||
notificationService: { dispatch } as any,
|
||||
minNotifyIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
await clearingMonitor.start();
|
||||
clearingMonitor.stop();
|
||||
|
||||
now += 100;
|
||||
const restartedMonitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: { dispatch } as any,
|
||||
minNotifyIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
await restartedMonitor.start();
|
||||
restartedMonitor.stop();
|
||||
|
||||
expect(dispatch).toHaveBeenCalledTimes(2);
|
||||
monitor.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("stop cancels the interval", async () => {
|
||||
@@ -177,6 +283,7 @@ describe("OAuthExpiryMonitor", () => {
|
||||
notificationService: { dispatch } as any,
|
||||
intervalMs: 100,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
|
||||
await monitor.start();
|
||||
@@ -184,6 +291,5 @@ describe("OAuthExpiryMonitor", () => {
|
||||
await vi.advanceTimersByTimeAsync(500);
|
||||
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,19 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { OAuthAlertStateStore } from "../oauth-alert-state.js";
|
||||
import { OAuthValidityLogger } from "../oauth-validity-logger.js";
|
||||
import type { AuthStorageLike } from "../oauth-expiry-monitor.js";
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
function createStatePath(): string {
|
||||
const dir = mkdtempSync(join(tmpdir(), "oauth-validity-logger-"));
|
||||
tempDirs.push(dir);
|
||||
return join(dir, "oauth-alert-state.json");
|
||||
}
|
||||
|
||||
function createAuthStorage(providers: Array<{ id: string; name: string }>, credentials: Record<string, any>): AuthStorageLike {
|
||||
return {
|
||||
reload: vi.fn(),
|
||||
@@ -10,6 +22,13 @@ function createAuthStorage(providers: Array<{ id: string; name: string }>, crede
|
||||
};
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
rmSync(dir, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
describe("OAuthValidityLogger", () => {
|
||||
it("logs one line per expired oauth credential on start", async () => {
|
||||
vi.useFakeTimers();
|
||||
@@ -26,30 +45,79 @@ describe("OAuthValidityLogger", () => {
|
||||
},
|
||||
);
|
||||
|
||||
const validityLogger = new OAuthValidityLogger({ authStorage, logger, intervalMs: 1_000, clock: () => now });
|
||||
const validityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
intervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
await validityLogger.start();
|
||||
|
||||
expect(logger).toHaveBeenCalledTimes(2);
|
||||
validityLogger.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("logs again on interval without dedupe", async () => {
|
||||
it("skips repeated logs within the throttle window", async () => {
|
||||
vi.useFakeTimers();
|
||||
const now = Date.now();
|
||||
const logger = vi.fn();
|
||||
let now = Date.now();
|
||||
const statePath = createStatePath();
|
||||
const authStorage = createAuthStorage(
|
||||
[{ id: "openai-codex", name: "OpenAI Codex" }],
|
||||
{ "openai-codex": { type: "oauth", expires: now - 1_000 } },
|
||||
);
|
||||
|
||||
const validityLogger = new OAuthValidityLogger({ authStorage, logger, intervalMs: 1_000, clock: () => now });
|
||||
await validityLogger.start();
|
||||
await vi.advanceTimersByTimeAsync(1_000);
|
||||
const validityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
intervalMs: 100,
|
||||
minAlertIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
|
||||
expect(logger).toHaveBeenCalledTimes(2);
|
||||
await validityLogger.start();
|
||||
now += 500;
|
||||
await validityLogger.check();
|
||||
|
||||
expect(logger).toHaveBeenCalledTimes(1);
|
||||
validityLogger.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("persists the throttle across a restart and logs again after the window elapses", async () => {
|
||||
vi.useFakeTimers();
|
||||
const logger = vi.fn();
|
||||
let now = Date.now();
|
||||
const statePath = createStatePath();
|
||||
const authStorage = createAuthStorage(
|
||||
[{ id: "openai-codex", name: "OpenAI Codex" }],
|
||||
{ "openai-codex": { type: "oauth", expires: now - 1_000 } },
|
||||
);
|
||||
|
||||
const firstLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
minAlertIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
await firstLogger.check();
|
||||
expect(logger).toHaveBeenCalledTimes(1);
|
||||
|
||||
const restartedLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
minAlertIntervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath, clock: () => now }),
|
||||
});
|
||||
await restartedLogger.check();
|
||||
expect(logger).toHaveBeenCalledTimes(1);
|
||||
|
||||
now += 1_001;
|
||||
await restartedLogger.check();
|
||||
expect(logger).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("does not log for valid oauth, api key, or missing expires", async () => {
|
||||
@@ -69,30 +137,42 @@ describe("OAuthValidityLogger", () => {
|
||||
},
|
||||
);
|
||||
|
||||
const validityLogger = new OAuthValidityLogger({ authStorage, logger, intervalMs: 1_000, clock: () => now });
|
||||
const validityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
intervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
await validityLogger.start();
|
||||
|
||||
expect(logger).not.toHaveBeenCalled();
|
||||
validityLogger.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("stop cancels the interval", async () => {
|
||||
vi.useFakeTimers();
|
||||
const now = Date.now();
|
||||
let now = Date.now();
|
||||
const logger = vi.fn();
|
||||
const authStorage = createAuthStorage(
|
||||
[{ id: "openai-codex", name: "OpenAI Codex" }],
|
||||
{ "openai-codex": { type: "oauth", expires: now - 1_000 } },
|
||||
);
|
||||
|
||||
const validityLogger = new OAuthValidityLogger({ authStorage, logger, intervalMs: 1_000, clock: () => now });
|
||||
const validityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
intervalMs: 1_000,
|
||||
minAlertIntervalMs: 500,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
await validityLogger.start();
|
||||
validityLogger.stop();
|
||||
now += 5_000;
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
|
||||
expect(logger).toHaveBeenCalledTimes(1);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("continues iterating when one provider throws", async () => {
|
||||
@@ -113,7 +193,13 @@ describe("OAuthValidityLogger", () => {
|
||||
},
|
||||
};
|
||||
|
||||
const validityLogger = new OAuthValidityLogger({ authStorage, logger, intervalMs: 1_000, clock: () => now });
|
||||
const validityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
intervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
await validityLogger.start();
|
||||
|
||||
expect(logger).toHaveBeenCalledTimes(1);
|
||||
@@ -122,7 +208,6 @@ describe("OAuthValidityLogger", () => {
|
||||
expect.objectContaining({ providerId: "claude" }),
|
||||
);
|
||||
validityLogger.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("never includes token material in log metadata", async () => {
|
||||
@@ -141,12 +226,50 @@ describe("OAuthValidityLogger", () => {
|
||||
},
|
||||
);
|
||||
|
||||
const validityLogger = new OAuthValidityLogger({ authStorage, logger, intervalMs: 1_000, clock: () => now });
|
||||
const validityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
intervalMs: 1_000,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }),
|
||||
});
|
||||
await validityLogger.start();
|
||||
|
||||
const [, meta] = logger.mock.calls[0] ?? [];
|
||||
expect(Object.keys(meta ?? {}).sort()).toEqual(["expiresAt", "providerId", "providerName"]);
|
||||
validityLogger.stop();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("covers empty, undefined, and populated provider states", async () => {
|
||||
const logger = vi.fn();
|
||||
const now = Date.now();
|
||||
const cases: AuthStorageLike[] = [
|
||||
{
|
||||
reload: vi.fn(),
|
||||
getOAuthProviders: () => [],
|
||||
get: () => undefined,
|
||||
},
|
||||
{
|
||||
reload: vi.fn(),
|
||||
getOAuthProviders: () => [{ id: "openai-codex", name: "OpenAI Codex" }],
|
||||
get: () => undefined,
|
||||
},
|
||||
createAuthStorage(
|
||||
[{ id: "openai-codex", name: "OpenAI Codex" }],
|
||||
{ "openai-codex": { type: "oauth", expires: now - 1 } },
|
||||
),
|
||||
];
|
||||
|
||||
for (const [index, authStorage] of cases.entries()) {
|
||||
const validityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
logger,
|
||||
clock: () => now,
|
||||
alertState: new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now + index }),
|
||||
});
|
||||
await validityLogger.check();
|
||||
}
|
||||
|
||||
expect(logger).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,6 +7,9 @@ export type { WebhookProviderConfig } from "./webhook-provider.js";
|
||||
export { NotificationService } from "./notification-service.js";
|
||||
export type { NotificationServiceOptions } from "./notification-service.js";
|
||||
|
||||
export { OAuthAlertStateStore } from "./oauth-alert-state.js";
|
||||
export type { OAuthAlertStateEntry, OAuthAlertStateFs, OAuthAlertStateStoreOptions } from "./oauth-alert-state.js";
|
||||
|
||||
export { OAuthExpiryMonitor } from "./oauth-expiry-monitor.js";
|
||||
export type { AuthStorageLike as OAuthExpiryAuthStorageLike, OAuthExpiryMonitorOptions } from "./oauth-expiry-monitor.js";
|
||||
|
||||
|
||||
144
packages/engine/src/notification/oauth-alert-state.ts
Normal file
144
packages/engine/src/notification/oauth-alert-state.ts
Normal file
@@ -0,0 +1,144 @@
|
||||
import {
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
readFileSync,
|
||||
renameSync,
|
||||
rmSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { dirname } from "node:path";
|
||||
import { getFusionOAuthAlertStatePath } from "../auth-storage.js";
|
||||
|
||||
export interface OAuthAlertStateEntry {
|
||||
expires: number;
|
||||
lastAlertAt: number;
|
||||
}
|
||||
|
||||
export interface OAuthAlertStateFs {
|
||||
existsSync(path: string): boolean;
|
||||
mkdirSync(path: string, options?: { recursive?: boolean }): void;
|
||||
readFileSync(path: string, encoding: BufferEncoding): string;
|
||||
renameSync(oldPath: string, newPath: string): void;
|
||||
rmSync(path: string, options?: { force?: boolean }): void;
|
||||
writeFileSync(path: string, content: string, encoding: BufferEncoding): void;
|
||||
}
|
||||
|
||||
export interface OAuthAlertStateStoreOptions {
|
||||
statePath?: string;
|
||||
clock?: () => number;
|
||||
fs?: OAuthAlertStateFs;
|
||||
}
|
||||
|
||||
export class OAuthAlertStateStore {
|
||||
private readonly statePath: string;
|
||||
private readonly clock: () => number;
|
||||
private readonly fs: OAuthAlertStateFs;
|
||||
|
||||
constructor(options: OAuthAlertStateStoreOptions = {}) {
|
||||
this.statePath = options.statePath ?? getFusionOAuthAlertStatePath();
|
||||
this.clock = options.clock ?? Date.now;
|
||||
this.fs = options.fs ?? {
|
||||
existsSync: (path) => existsSync(path),
|
||||
mkdirSync: (path, options) => {
|
||||
mkdirSync(path, options);
|
||||
},
|
||||
readFileSync: (path, encoding) => readFileSync(path, encoding),
|
||||
renameSync: (oldPath, newPath) => {
|
||||
renameSync(oldPath, newPath);
|
||||
},
|
||||
rmSync: (path, options) => {
|
||||
rmSync(path, options);
|
||||
},
|
||||
writeFileSync: (path, content, encoding) => {
|
||||
writeFileSync(path, content, encoding);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
get(providerId: string): OAuthAlertStateEntry | undefined {
|
||||
return this.readState()[providerId];
|
||||
}
|
||||
|
||||
getLastAlertAt(providerId: string): number | undefined {
|
||||
return this.get(providerId)?.lastAlertAt;
|
||||
}
|
||||
|
||||
recordAlert(providerId: string, expires: number, lastAlertAt = this.clock()): void {
|
||||
const state = this.readState();
|
||||
state[providerId] = { expires, lastAlertAt };
|
||||
this.writeState(state);
|
||||
}
|
||||
|
||||
clear(providerIds?: Iterable<string>): void {
|
||||
if (!providerIds) {
|
||||
this.writeState({});
|
||||
return;
|
||||
}
|
||||
|
||||
const state = this.readState();
|
||||
let changed = false;
|
||||
for (const providerId of providerIds) {
|
||||
if (!(providerId in state)) {
|
||||
continue;
|
||||
}
|
||||
delete state[providerId];
|
||||
changed = true;
|
||||
}
|
||||
if (changed) {
|
||||
this.writeState(state);
|
||||
}
|
||||
}
|
||||
|
||||
private readState(): Record<string, OAuthAlertStateEntry> {
|
||||
if (!this.fs.existsSync(this.statePath)) {
|
||||
return {};
|
||||
}
|
||||
|
||||
try {
|
||||
const parsed = JSON.parse(this.fs.readFileSync(this.statePath, "utf-8")) as unknown;
|
||||
return sanitizeState(parsed);
|
||||
} catch {
|
||||
return {};
|
||||
}
|
||||
}
|
||||
|
||||
private writeState(state: Record<string, OAuthAlertStateEntry>): void {
|
||||
const sanitized = sanitizeState(state);
|
||||
const dir = dirname(this.statePath);
|
||||
this.fs.mkdirSync(dir, { recursive: true });
|
||||
|
||||
const tempPath = `${this.statePath}.${process.pid}.${this.clock()}.tmp`;
|
||||
const body = `${JSON.stringify(sanitized, null, 2)}\n`;
|
||||
this.fs.writeFileSync(tempPath, body, "utf-8");
|
||||
try {
|
||||
this.fs.renameSync(tempPath, this.statePath);
|
||||
} catch (error) {
|
||||
this.fs.rmSync(tempPath, { force: true });
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function sanitizeState(parsed: unknown): Record<string, OAuthAlertStateEntry> {
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
|
||||
return {};
|
||||
}
|
||||
|
||||
const sanitized: Record<string, OAuthAlertStateEntry> = {};
|
||||
for (const [providerId, value] of Object.entries(parsed)) {
|
||||
if (!value || typeof value !== "object" || Array.isArray(value)) {
|
||||
continue;
|
||||
}
|
||||
const expires = (value as { expires?: unknown }).expires;
|
||||
const lastAlertAt = (value as { lastAlertAt?: unknown }).lastAlertAt;
|
||||
if (typeof expires !== "number" || Number.isNaN(expires)) {
|
||||
continue;
|
||||
}
|
||||
if (typeof lastAlertAt !== "number" || Number.isNaN(lastAlertAt)) {
|
||||
continue;
|
||||
}
|
||||
sanitized[providerId] = { expires, lastAlertAt };
|
||||
}
|
||||
|
||||
return sanitized;
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { NotificationPayload } from "@fusion/core";
|
||||
import { schedulerLog } from "../logger.js";
|
||||
import { OAuthAlertStateStore } from "./oauth-alert-state.js";
|
||||
import type { NotificationService } from "./notification-service.js";
|
||||
|
||||
const DEFAULT_INTERVAL_MS = 5 * 60_000;
|
||||
@@ -28,6 +29,7 @@ export interface OAuthExpiryMonitorOptions {
|
||||
clock?: () => number;
|
||||
warnBeforeMs?: number;
|
||||
minNotifyIntervalMs?: number;
|
||||
alertState?: OAuthAlertStateStore;
|
||||
}
|
||||
|
||||
export class OAuthExpiryMonitor {
|
||||
@@ -35,15 +37,16 @@ export class OAuthExpiryMonitor {
|
||||
private readonly clock: () => number;
|
||||
private readonly warnBeforeMs: number;
|
||||
private readonly minNotifyIntervalMs: number;
|
||||
private readonly alertState: OAuthAlertStateStore;
|
||||
private timer: NodeJS.Timeout | null = null;
|
||||
private readonly dispatchedExpiryKeys = new Set<string>();
|
||||
private readonly lastNotifiedAt = new Map<string, number>();
|
||||
|
||||
constructor(private readonly opts: OAuthExpiryMonitorOptions) {
|
||||
this.intervalMs = opts.intervalMs ?? DEFAULT_INTERVAL_MS;
|
||||
this.clock = opts.clock ?? Date.now;
|
||||
this.warnBeforeMs = opts.warnBeforeMs ?? 0;
|
||||
this.minNotifyIntervalMs = opts.minNotifyIntervalMs ?? DEFAULT_MIN_NOTIFY_INTERVAL_MS;
|
||||
this.alertState = opts.alertState ?? new OAuthAlertStateStore({ clock: this.clock });
|
||||
}
|
||||
|
||||
async start(): Promise<void> {
|
||||
@@ -73,7 +76,7 @@ export class OAuthExpiryMonitor {
|
||||
const providers = this.opts.authStorage.getOAuthProviders?.();
|
||||
if (!providers?.length) {
|
||||
this.dispatchedExpiryKeys.clear();
|
||||
this.lastNotifiedAt.clear();
|
||||
this.alertState.clear();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -83,6 +86,7 @@ export class OAuthExpiryMonitor {
|
||||
for (const provider of providers) {
|
||||
const credential = this.opts.authStorage.get?.(provider.id);
|
||||
if (credential?.type !== "oauth" || typeof credential.expires !== "number") {
|
||||
this.alertState.clear([provider.id]);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -96,7 +100,7 @@ export class OAuthExpiryMonitor {
|
||||
continue;
|
||||
}
|
||||
|
||||
const previousNotificationAt = this.lastNotifiedAt.get(provider.id);
|
||||
const previousNotificationAt = this.alertState.getLastAlertAt(provider.id);
|
||||
if (
|
||||
typeof previousNotificationAt === "number" &&
|
||||
now - previousNotificationAt < this.minNotifyIntervalMs
|
||||
@@ -116,7 +120,7 @@ export class OAuthExpiryMonitor {
|
||||
try {
|
||||
await this.opts.notificationService.dispatch("oauth-token-expired", payload);
|
||||
this.dispatchedExpiryKeys.add(expiryKey);
|
||||
this.lastNotifiedAt.set(provider.id, now);
|
||||
this.alertState.recordAlert(provider.id, credential.expires, now);
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
schedulerLog.warn(`OAuth expiry notification dispatch failed provider=${provider.id}: ${message}`);
|
||||
|
||||
@@ -1,25 +1,33 @@
|
||||
import { schedulerLog } from "../logger.js";
|
||||
import type { AuthStorageLike } from "./oauth-expiry-monitor.js";
|
||||
import { OAuthAlertStateStore } from "./oauth-alert-state.js";
|
||||
|
||||
const DEFAULT_INTERVAL_MS = 24 * 60 * 60 * 1000;
|
||||
const DEFAULT_INTERVAL_MS = 12 * 60 * 60 * 1000;
|
||||
const DEFAULT_MIN_ALERT_INTERVAL_MS = 12 * 60 * 60 * 1000;
|
||||
|
||||
interface OAuthValidityLoggerOptions {
|
||||
authStorage: AuthStorageLike;
|
||||
intervalMs?: number;
|
||||
clock?: () => number;
|
||||
logger?: (msg: string, meta?: Record<string, unknown>) => void;
|
||||
alertState?: OAuthAlertStateStore;
|
||||
minAlertIntervalMs?: number;
|
||||
}
|
||||
|
||||
export class OAuthValidityLogger {
|
||||
private readonly intervalMs: number;
|
||||
private readonly clock: () => number;
|
||||
private readonly logger: (msg: string, meta?: Record<string, unknown>) => void;
|
||||
private readonly alertState: OAuthAlertStateStore;
|
||||
private readonly minAlertIntervalMs: number;
|
||||
private timer: NodeJS.Timeout | null = null;
|
||||
|
||||
constructor(private readonly opts: OAuthValidityLoggerOptions) {
|
||||
this.intervalMs = opts.intervalMs ?? DEFAULT_INTERVAL_MS;
|
||||
this.clock = opts.clock ?? Date.now;
|
||||
this.logger = opts.logger ?? ((message, meta) => schedulerLog.warn(message, meta));
|
||||
this.alertState = opts.alertState ?? new OAuthAlertStateStore({ clock: this.clock });
|
||||
this.minAlertIntervalMs = opts.minAlertIntervalMs ?? DEFAULT_MIN_ALERT_INTERVAL_MS;
|
||||
}
|
||||
|
||||
async start(): Promise<void> {
|
||||
@@ -57,11 +65,17 @@ export class OAuthValidityLogger {
|
||||
continue;
|
||||
}
|
||||
|
||||
const previousAlertAt = this.alertState.getLastAlertAt(provider.id);
|
||||
if (typeof previousAlertAt === "number" && now - previousAlertAt < this.minAlertIntervalMs) {
|
||||
continue;
|
||||
}
|
||||
|
||||
this.logger("oauth credential expired — provider re-login required", {
|
||||
providerId: provider.id,
|
||||
providerName: provider.name,
|
||||
expiresAt: new Date(credential.expires).toISOString(),
|
||||
});
|
||||
this.alertState.recordAlert(provider.id, credential.expires, now);
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
schedulerLog.warn(`OAuth validity logger failed for provider=${provider.id}: ${message}`);
|
||||
|
||||
@@ -10,7 +10,7 @@ import type {
|
||||
ScheduledTask,
|
||||
AutomationRunResult,
|
||||
} from "@fusion/core";
|
||||
import { compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, isSharedBranchGroupMemberIntegration, normalizeMergerMode, sortTasksByPriorityThenAgeAndId } from "@fusion/core";
|
||||
import { allowsAutoMergeProcessing, compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, isSharedBranchGroupMemberIntegration, normalizeMergerMode, sortTasksByPriorityThenAgeAndId } from "@fusion/core";
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { InProcessRuntime } from "./runtimes/in-process-runtime.js";
|
||||
@@ -19,10 +19,10 @@ import type { ProjectRuntimeConfig } from "./project-runtime.js";
|
||||
import { PrMonitor } from "./pr-monitor.js";
|
||||
import { PrCommentHandler } from "./pr-comment-handler.js";
|
||||
import { NtfyNotifier } from "./notifier.js";
|
||||
import { NotificationService, OAuthExpiryMonitor, OAuthValidityLogger } from "./notification/index.js";
|
||||
import { NotificationService, OAuthAlertStateStore, OAuthExpiryMonitor, OAuthValidityLogger } from "./notification/index.js";
|
||||
import type { NotificationChatStore } from "./notification/notification-service.js";
|
||||
import { GridlockDetector } from "./gridlock-detector.js";
|
||||
import { createFusionAuthStorage } from "./auth-storage.js";
|
||||
import { createFusionAuthStorage, getFusionOAuthAlertStatePath } from "./auth-storage.js";
|
||||
import { CronRunner, createAiPromptExecutor } from "./cron-runner.js";
|
||||
import type { RoutineRunner } from "./routine-runner.js";
|
||||
import { aiMergeTask, sweepStaleAutostashes, VerificationError } from "./merger.js";
|
||||
@@ -427,12 +427,19 @@ export class ProjectEngine {
|
||||
});
|
||||
await this.notificationService.start();
|
||||
const authStorage = createFusionAuthStorage();
|
||||
const oauthAlertState = new OAuthAlertStateStore({
|
||||
statePath: getFusionOAuthAlertStatePath(),
|
||||
});
|
||||
this.oauthExpiryMonitor = new OAuthExpiryMonitor({
|
||||
authStorage,
|
||||
notificationService: this.notificationService,
|
||||
alertState: oauthAlertState,
|
||||
});
|
||||
await this.oauthExpiryMonitor.start();
|
||||
this.oauthValidityLogger = new OAuthValidityLogger({ authStorage });
|
||||
this.oauthValidityLogger = new OAuthValidityLogger({
|
||||
authStorage,
|
||||
alertState: oauthAlertState,
|
||||
});
|
||||
await this.oauthValidityLogger.start();
|
||||
|
||||
// Backward-compatibility shim for gridlock notifications.
|
||||
@@ -1376,8 +1383,8 @@ export class ProjectEngine {
|
||||
* pushed wins. listTasks returns createdAt ASC — without this sort an
|
||||
* older low-priority task would start before a later urgent one.
|
||||
*/
|
||||
private allowInReviewMergeProcessing(task: Pick<Task, "branchContext">, settings: Pick<Settings, "autoMerge">): boolean {
|
||||
return settings.autoMerge || isSharedBranchGroupMemberIntegration(task);
|
||||
private allowInReviewMergeProcessing(task: Pick<Task, "branchContext" | "autoMerge">, settings: Pick<Settings, "autoMerge">): boolean {
|
||||
return allowsAutoMergeProcessing(task, settings) || isSharedBranchGroupMemberIntegration(task);
|
||||
}
|
||||
|
||||
private enqueueEligibleInReviewTasks(tasks: readonly Task[], settings: Pick<Settings, "autoMerge">): number {
|
||||
|
||||
@@ -120,6 +120,7 @@ Concrete examples:
|
||||
|
||||
### Test Gaps
|
||||
- [Missing test scenarios]
|
||||
- [For bug fixes, call out any repro-only regression test that does not assert the invariant across the enumerated surfaces. Issue REVISE when coverage stops at the single reported case instead of spanning the \`## Surface Enumeration\` checklist (FN-5893; see FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751).]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
@@ -144,6 +145,7 @@ Concrete examples:
|
||||
- **File scope accuracy:** [All affected files listed? No extras?]
|
||||
- **Dependency correctness:** [Dependencies exist and are appropriate?]
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Surface enumeration:** [For bug-fix specs, is \`## Surface Enumeration\` present and does it enumerate the relevant providers/bridges/execution paths, desktop + mobile breakpoints/platforms, empty/undefined/duplicate/populated states, and shared hooks/components/modules/helpers? Missing or incomplete coverage is a blocking REVISE.]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Dangling task-document references:** [No \`.fusion/tasks/<id>/<file>\` path is cited in Context, Steps, or File Scope unless the file exists or is explicitly created as a \`(new)\` artifact in this spec. References to nonexistent task-local artifacts are a blocking REVISE.]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
@@ -198,6 +200,7 @@ Do NOT demand function-level implementation checklists.
|
||||
|
||||
When reviewing tests, check that they verify observable behavior and regression risk (not only implementation trivia).
|
||||
Flag REVISE when key edge cases or failure modes for changed behavior are untested.
|
||||
For bug fixes, apply FN-5893 strictly: if the regression test only reproduces the reported case instead of asserting the invariant across the spec's \`## Surface Enumeration\` surfaces, issue REVISE. Use the motivating recurrences (FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751) as concrete examples of why repro-only coverage is insufficient.
|
||||
|
||||
## Worktree Boundary Review
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ import { promisify } from "node:util";
|
||||
import { setImmediate as setImmediateCb } from "node:timers";
|
||||
import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs";
|
||||
import { isAbsolute, join, relative, resolve } from "node:path";
|
||||
import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult } from "@fusion/core";
|
||||
import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, allowsAutoMergeProcessing, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult } from "@fusion/core";
|
||||
import type { MeshLeaseManager } from "./mesh-lease-manager.js";
|
||||
import { createLogger, schedulerLog } from "./logger.js";
|
||||
import { RemovalReason, classifyTaskWorktree, getRegisteredWorktreeBranchMap, getRegisteredWorktreePaths, isUsableTaskWorktree, removeWorktree, resolveWorktreeBackend, scanIdleWorktrees, scanOrphanedBranches } from "./worktree-pool.js";
|
||||
@@ -1657,6 +1657,18 @@ export class SelfHealingManager {
|
||||
log.log(`Maintenance batch 1 step "prune-operational-logs" succeeded — deleted=${deletedTotal}${detail ? ` (${detail})` : ""}`);
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "prune-agent-log-files",
|
||||
fn: async () => {
|
||||
const days = Number(settings.agentLogFileRetentionDays ?? 0);
|
||||
if (!Number.isFinite(days) || days <= 0) {
|
||||
log.log("Maintenance batch 1 step \"prune-agent-log-files\" skipped — agentLogFileRetentionDays is not enabled");
|
||||
return;
|
||||
}
|
||||
const { prunedFiles, prunedEntries, freedBytes } = this.store.pruneAgentLogFiles(days);
|
||||
log.log(`Maintenance batch 1 step "prune-agent-log-files" succeeded — files=${prunedFiles} entries=${prunedEntries} bytes=${freedBytes}`);
|
||||
},
|
||||
},
|
||||
{ name: "checkpoint-wal", fn: () => Promise.resolve(this.checkpointWal()) },
|
||||
{ name: "enforce-worktree-cap", fn: () => this.enforceWorktreeCap() },
|
||||
];
|
||||
@@ -2290,14 +2302,14 @@ export class SelfHealingManager {
|
||||
* Backward lifecycle move gated on triple proof (FN-5335).
|
||||
* When the predicate fails, emits `task:reclaim-self-owned-branch-conflict-no-action` and skips lifecycle mutation.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async reclaimSelfOwnedBranchConflicts(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const todoCandidates = await this.store.listTasks({ column: "todo", slim: true });
|
||||
const inProgressCandidates = await this.store.listTasks({ column: "in-progress", slim: true });
|
||||
const inProgressByWorktree = new Map<string, string>();
|
||||
@@ -2308,7 +2320,14 @@ export class SelfHealingManager {
|
||||
}
|
||||
const inReviewPausedCandidates = (await this.store.listTasks({ column: "in-review", slim: true }))
|
||||
.filter((task) => task.paused === true && task.pausedReason === "branch-conflict-unrecoverable");
|
||||
const candidates = [...todoCandidates, ...inProgressCandidates, ...inReviewPausedCandidates];
|
||||
// Per-task auto-merge gating applies to ALL candidate columns, not just
|
||||
// in-review: the FN-5704 regression contract ("short-circuits reclaim
|
||||
// when autoMerge is false") deliberately keeps execution-stage reclaim
|
||||
// and resume-limbo escalation inert in manual-review projects. The
|
||||
// per-task override preserves that for override-less tasks while letting
|
||||
// explicit autoMerge:true tasks recover.
|
||||
const candidates = [...todoCandidates, ...inProgressCandidates, ...inReviewPausedCandidates]
|
||||
.filter((task) => allowsAutoMergeProcessing(task, settings));
|
||||
|
||||
const activeTaskIds = new Set<string>();
|
||||
if (this.options.agentStore) {
|
||||
@@ -4472,17 +4491,18 @@ export class SelfHealingManager {
|
||||
* Backward lifecycle move gated on triple proof (FN-5335).
|
||||
* When the unproven fallback predicate fails, emits `task:finalize-no-op-review-no-action` and skips lifecycle mutation.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async finalizeNoOpReviewTasks(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const candidates = tasks.filter((t) =>
|
||||
t.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(t, settings) &&
|
||||
!t.paused &&
|
||||
!isSharedBranchGroupMemberIntegration(t) &&
|
||||
Boolean(t.worktree) &&
|
||||
@@ -4781,12 +4801,11 @@ export class SelfHealingManager {
|
||||
// "pull-request"`) — see GitHub issue #21.
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
|
||||
const mergeable = tasks.filter((t) =>
|
||||
t.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(t, settings) &&
|
||||
!t.paused &&
|
||||
t.status !== "failed" &&
|
||||
// Exclude transient merge statuses. Active merges should be left alone;
|
||||
@@ -4886,7 +4905,9 @@ export class SelfHealingManager {
|
||||
* per-task `postReviewFixCount` so a persistently-failing verifier cannot
|
||||
* ping-pong a task forever.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
* @returns Number of tasks sent back for fix
|
||||
*/
|
||||
async recoverReviewTasksWithFailedPreMergeSteps(): Promise<number> {
|
||||
@@ -4896,7 +4917,6 @@ export class SelfHealingManager {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
const maxFixes = settings.maxPostReviewFixes ?? 1;
|
||||
if (!Number.isFinite(maxFixes) || maxFixes <= 0) return 0;
|
||||
|
||||
@@ -4905,6 +4925,7 @@ export class SelfHealingManager {
|
||||
|
||||
const candidates = tasks.filter((task) => {
|
||||
if (task.column !== "in-review") return false;
|
||||
if (!allowsAutoMergeProcessing(task, settings)) return false;
|
||||
if (task.paused) return false;
|
||||
// Preserve terminal/human-handoff statuses (failed, awaiting-user-review,
|
||||
// merging, etc.). Only revive tasks that are otherwise idle.
|
||||
@@ -4982,13 +5003,14 @@ export class SelfHealingManager {
|
||||
* incomplete step instead of leaving the task stranded in review.
|
||||
* Backward lifecycle move gated on triple proof (FN-5335).
|
||||
* When the predicate fails, emits `task:stale-incomplete-review-no-action` and skips lifecycle mutation.
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverStaleIncompleteReviewTasks(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
const timeoutMs = settings.taskStuckTimeoutMs;
|
||||
if (!timeoutMs || timeoutMs <= 0) return 0;
|
||||
|
||||
@@ -4996,6 +5018,7 @@ export class SelfHealingManager {
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const staleIncomplete = tasks.filter((task) =>
|
||||
task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
!task.paused &&
|
||||
!task.status &&
|
||||
task.steps.length > 0 &&
|
||||
@@ -5044,8 +5067,9 @@ export class SelfHealingManager {
|
||||
* Final-fallback recovery for `in-review` tasks that fell through every other
|
||||
* scan and have sat untouched longer than `taskStuckTimeoutMs`.
|
||||
*
|
||||
* When `settings.autoMerge` is disabled, this sweep is a no-op because
|
||||
* PR-based manual review intentionally leaves tasks in `in-review`.
|
||||
* Tasks not eligible for auto-merge processing (global `autoMerge` off
|
||||
* without an explicit per-task `autoMerge: true` override) are skipped
|
||||
* because PR-based manual review intentionally leaves them in `in-review`.
|
||||
*
|
||||
* The other review-recovery scans each require a specific shape (failed
|
||||
* pre-merge step, incomplete steps, mergeable + worktree present, confirmed
|
||||
@@ -5066,8 +5090,10 @@ export class SelfHealingManager {
|
||||
* each kick refreshes `updatedAt`, so a task that re-enters review and gets
|
||||
* stuck again can only be kicked once per `taskStuckTimeoutMs` window.
|
||||
*
|
||||
* When `settings.autoMerge === false`, this sweep is a no-op because those
|
||||
* projects intentionally use PR-based/manual in-review ownership.
|
||||
* Tasks not eligible for auto-merge processing (global `autoMerge` off
|
||||
* without an explicit per-task `autoMerge: true` override) are skipped
|
||||
* because those projects intentionally use PR-based/manual in-review
|
||||
* ownership.
|
||||
*
|
||||
* @returns Number of tasks kicked back to todo
|
||||
*/
|
||||
@@ -5075,8 +5101,6 @@ export class SelfHealingManager {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const cycleStartMs = Date.now();
|
||||
const timeoutMs = settings.taskStuckTimeoutMs;
|
||||
if (!timeoutMs || timeoutMs <= 0) return 0;
|
||||
@@ -5088,6 +5112,7 @@ export class SelfHealingManager {
|
||||
|
||||
for (const task of tasks) {
|
||||
if (task.deletedAt) continue;
|
||||
if (!allowsAutoMergeProcessing(task, settings)) continue;
|
||||
const signal = getInReviewStallReason(task, {
|
||||
now: cycleStartMs,
|
||||
activeMergeTaskId,
|
||||
@@ -5205,14 +5230,14 @@ export class SelfHealingManager {
|
||||
* - `surfaceStalePausedReviews()` owns paused in-review tasks.
|
||||
* - `surfaceInReviewStalls()` owns reason-driven in-review stalls.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async surfaceInReviewStalled(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const cycleStartMs = Date.now();
|
||||
const thresholdMs = settings.inReviewStalledThresholdMs;
|
||||
if (!thresholdMs || thresholdMs <= 0) return 0;
|
||||
@@ -5224,6 +5249,7 @@ export class SelfHealingManager {
|
||||
|
||||
for (const task of tasks) {
|
||||
if (task.deletedAt) continue;
|
||||
if (!allowsAutoMergeProcessing(task, settings)) continue;
|
||||
if (task.paused === true) continue;
|
||||
if (task.id === activeMergeTaskId || executingTaskIds.has(task.id)) continue;
|
||||
|
||||
@@ -5377,13 +5403,14 @@ export class SelfHealingManager {
|
||||
* Backward lifecycle move gated on triple proof (FN-5335).
|
||||
* When the predicate fails, emits `task:ghost-review-no-action` and skips lifecycle mutation.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverGhostReviewTasks(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
const timeoutMs = settings.taskStuckTimeoutMs;
|
||||
if (!timeoutMs || timeoutMs <= 0) return 0;
|
||||
|
||||
@@ -5392,6 +5419,7 @@ export class SelfHealingManager {
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const ghosts = tasks.filter((task) =>
|
||||
task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
!task.paused &&
|
||||
!executingIds.has(task.id) &&
|
||||
!(task.status && GHOST_REVIEW_PRESERVED_STATUSES.has(task.status)) &&
|
||||
@@ -5453,7 +5481,9 @@ export class SelfHealingManager {
|
||||
* If no landed commit is found, it only clears the stale transient status so
|
||||
* the normal mergeable-review recovery can retry the merge.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
* @returns Number of tasks finalized or unblocked
|
||||
*/
|
||||
/**
|
||||
@@ -5474,8 +5504,9 @@ export class SelfHealingManager {
|
||||
* parked as failed and emit `merger:transient-failure-budget-exhausted`
|
||||
* once for diagnostic visibility.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false`, no `requeueForAutoMerge`
|
||||
* callback is wired, or global/engine pause is active.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without a per-task `autoMerge: true` override). No-op when no
|
||||
* `requeueForAutoMerge` callback is wired or global/engine pause is active.
|
||||
*
|
||||
* @returns Number of tasks recovered
|
||||
*/
|
||||
@@ -5484,12 +5515,12 @@ export class SelfHealingManager {
|
||||
if (!requeue) return 0;
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.autoMerge === false) return 0;
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
|
||||
const slim = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const candidates = slim.filter((t) =>
|
||||
t.column === "in-review"
|
||||
&& allowsAutoMergeProcessing(t, settings)
|
||||
&& t.status === "failed"
|
||||
&& (t.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES
|
||||
&& typeof t.error === "string"
|
||||
@@ -5630,13 +5661,13 @@ export class SelfHealingManager {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
const timeoutMs = settings.taskStuckTimeoutMs;
|
||||
if (!timeoutMs || timeoutMs <= 0) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const candidates = tasks.filter((task) =>
|
||||
task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
!task.paused &&
|
||||
Boolean(task.status && ACTIVE_MERGE_STATUSES.has(task.status)) &&
|
||||
this.isPastInterruptedMergeGrace(task, timeoutMs),
|
||||
@@ -5944,20 +5975,21 @@ export class SelfHealingManager {
|
||||
* but a later transition failed or another process moved the task before the
|
||||
* final `in-review` → `done` update completed.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
* @returns Number of tasks recovered
|
||||
*/
|
||||
async recoverMergedReviewTasks(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
|
||||
const mergedButNotDone = tasks.filter((t) =>
|
||||
!t.deletedAt &&
|
||||
t.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(t, settings) &&
|
||||
t.mergeDetails?.mergeConfirmed === true,
|
||||
);
|
||||
|
||||
@@ -6075,14 +6107,14 @@ export class SelfHealingManager {
|
||||
* When the no-landed predicate fails, emits `task:stuck-merge-deadlock-no-action` and skips lifecycle mutation.
|
||||
*/
|
||||
/**
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverStuckMergeDeadlocks(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const now = Date.now();
|
||||
const inReview = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const triage = await this.store.listTasks({ column: "triage", slim: true });
|
||||
@@ -6105,6 +6137,7 @@ export class SelfHealingManager {
|
||||
(dep) => dep.column === "triage" || dep.column === "todo",
|
||||
);
|
||||
return task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
!task.paused &&
|
||||
task.status === "failed" &&
|
||||
(task.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES &&
|
||||
@@ -6266,18 +6299,19 @@ export class SelfHealingManager {
|
||||
}
|
||||
|
||||
/**
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverOrphanOnlyScopeViolations(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const candidates = tasks.filter((task) =>
|
||||
task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
task.status === "failed" &&
|
||||
task.scopeOverride !== true &&
|
||||
task.mergeDetails?.mergeConfirmed !== true &&
|
||||
@@ -6430,19 +6464,20 @@ export class SelfHealingManager {
|
||||
*
|
||||
* Idempotency: recovered tasks are moved to `done`, status/error are cleared,
|
||||
* and mergeRetries reset to 0, so subsequent sweeps will not match them.
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverAlreadyMergedReviewTasks(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const candidates = tasks.filter((task) =>
|
||||
!task.deletedAt &&
|
||||
task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
task.status === "failed" &&
|
||||
(task.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES &&
|
||||
task.mergeDetails?.mergeConfirmed !== true &&
|
||||
@@ -6579,19 +6614,20 @@ export class SelfHealingManager {
|
||||
* Recover completed in-review tasks wedged as failed only because a post-done
|
||||
* session continuation hit a non-continuable signature.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverPostDoneNonContinuableWedge(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: false });
|
||||
let recovered = 0;
|
||||
|
||||
for (const task of tasks) {
|
||||
if (task.column !== "in-review" || task.deletedAt) continue;
|
||||
if (!allowsAutoMergeProcessing(task, settings)) continue;
|
||||
if (task.paused || task.userPaused) continue;
|
||||
if (task.status !== "failed") continue;
|
||||
if (this.options.isTaskActive?.(task.id)) continue;
|
||||
@@ -6652,18 +6688,19 @@ export class SelfHealingManager {
|
||||
}
|
||||
|
||||
/**
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverCompletionHandoffLimbo(): Promise<void> {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return;
|
||||
if (settings.autoMerge === false) return;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: false });
|
||||
const now = Date.now();
|
||||
|
||||
for (const task of tasks) {
|
||||
if (task.column !== "in-review" || task.paused) continue;
|
||||
if (!allowsAutoMergeProcessing(task, settings)) continue;
|
||||
if (task.status != null || task.mergeDetails != null || task.review != null || task.reviewState != null) continue;
|
||||
if (this.options.isTaskActive?.(task.id)) continue;
|
||||
if (getTaskMergeBlocker(task) !== undefined) continue;
|
||||
@@ -6877,28 +6914,34 @@ export class SelfHealingManager {
|
||||
}
|
||||
|
||||
/**
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverForeignOnlyContaminatedInReviewTasks(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
const inReview = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const inProgress = await this.store.listTasks({ column: "in-progress", slim: true });
|
||||
const candidates = [
|
||||
...inReview.filter((task) =>
|
||||
task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
Boolean(task.branch) &&
|
||||
Boolean(task.worktree) &&
|
||||
task.mergeDetails?.mergeConfirmed !== true &&
|
||||
!task.userPaused &&
|
||||
!executingIds.has(task.id),
|
||||
),
|
||||
// The paused in-progress contamination branch is gated per-task too:
|
||||
// pre-existing behavior kept this sweep fully inert in manual-review
|
||||
// projects (mirroring the FN-5704 reclaim contract), so override-less
|
||||
// tasks stay untouched while explicit autoMerge:true tasks recover.
|
||||
...inProgress.filter((task) =>
|
||||
task.column === "in-progress" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
task.paused === true &&
|
||||
(task.pausedReason === "branch-cross-contamination" || task.pausedReason === "branch-conflict-unrecoverable") &&
|
||||
Boolean(task.branch) &&
|
||||
@@ -7718,18 +7761,19 @@ export class SelfHealingManager {
|
||||
* `restart-recovery-coordinator.ts`.
|
||||
* We clear stale worktree metadata and failure state, keep step progress and
|
||||
* retry counters, then requeue to todo for a clean retry.
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
*/
|
||||
async recoverMissingWorktreeReviewFailures(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const candidates = tasks.filter((task) =>
|
||||
isRecoverableMissingWorktreeReviewFailureWithProgress(task)
|
||||
|| isRecoverableMissingWorktreeReviewFailureNoProgress(task),
|
||||
allowsAutoMergeProcessing(task, settings)
|
||||
&& (isRecoverableMissingWorktreeReviewFailureWithProgress(task)
|
||||
|| isRecoverableMissingWorktreeReviewFailureNoProgress(task)),
|
||||
);
|
||||
|
||||
if (candidates.length === 0) return 0;
|
||||
@@ -7801,19 +7845,20 @@ export class SelfHealingManager {
|
||||
* - `recoverNoProgressNoTaskDoneFailures`: `in-progress` with zero progress → clean requeue.
|
||||
* - This one: `in-review` with partial progress → bounded requeue preserving work.
|
||||
*
|
||||
* No-op when `settings.autoMerge === false` — PR-based review flow owns lifecycle until human merge.
|
||||
* Skips tasks not eligible for auto-merge processing (global `autoMerge`
|
||||
* off without an explicit per-task `autoMerge: true` override) — PR-based
|
||||
* review flow owns lifecycle until human merge.
|
||||
* @returns Number of tasks requeued for retry
|
||||
*/
|
||||
async recoverPartialProgressNoTaskDoneFailures(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
if (settings.autoMerge === false) return 0;
|
||||
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
|
||||
const candidates = tasks.filter((task) =>
|
||||
task.column === "in-review" &&
|
||||
allowsAutoMergeProcessing(task, settings) &&
|
||||
task.status === "failed" &&
|
||||
isNoTaskDoneFailure(task) &&
|
||||
!task.paused &&
|
||||
|
||||
@@ -116,6 +116,10 @@ Follow this structure exactly:
|
||||
|
||||
{One paragraph: what you're building and why it matters}
|
||||
|
||||
## Surface Enumeration
|
||||
|
||||
{Required for bug-fix tasks: a checklist enumerating every surface the fixed invariant must hold across. Include every provider/bridge for streaming and agent paths; desktop AND mobile breakpoints; empty/undefined/duplicate/populated data states; and every hook/component/module that shares the affected logic. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **None**
|
||||
@@ -146,6 +150,12 @@ Follow this structure exactly:
|
||||
- [ ] {Specific, verifiable outcome}
|
||||
- [ ] Run targeted tests for changed files, asserting the invariant across all known surfaces (enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states)
|
||||
|
||||
For bug-fix tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
- [ ] Providers / bridges / execution paths touched by the invariant
|
||||
- [ ] Desktop + mobile breakpoints / platforms that exercise the behavior
|
||||
- [ ] Empty / undefined / duplicate / populated data states
|
||||
- [ ] Shared hooks / components / modules / helpers reusing the logic
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
|
||||
@@ -220,6 +230,8 @@ files with assertions that run via a test runner. Typechecks and builds are NOT
|
||||
tests. Manual verification is NOT a test.
|
||||
|
||||
- Each implementation step should include writing tests for the code being changed
|
||||
- For bug fixes, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix spec as a blocking REVISE.
|
||||
- For bug fixes, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers.
|
||||
- For bug fixes, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states — not just the reported repro (see FN-5787/FN-5789/FN-5803 and FN-5751)
|
||||
- The final Testing step runs lint, the FULL test suite, and project typecheck when the repo exposes one
|
||||
- Specs must instruct executors to fix lint failures and quality-gate failures directly, even when the required edits extend beyond the original File Scope
|
||||
@@ -402,6 +414,10 @@ Follow this structure exactly:
|
||||
|
||||
{One paragraph: what to build and why it matters}
|
||||
|
||||
## Surface Enumeration
|
||||
|
||||
{Required for bug-fix tasks: a checklist enumerating every surface the fixed invariant must hold across. Include every provider/bridge for streaming and agent paths; desktop AND mobile breakpoints; empty/undefined/duplicate/populated data states; and every hook/component/module that shares the affected logic. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **None**
|
||||
@@ -432,6 +448,12 @@ Follow this structure exactly:
|
||||
- [ ] {Specific, verifiable outcome}
|
||||
- [ ] Run targeted tests for changed files, asserting the invariant across all known surfaces (enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states)
|
||||
|
||||
For bug-fix tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
- [ ] Providers / bridges / execution paths touched by the invariant
|
||||
- [ ] Desktop + mobile breakpoints / platforms that exercise the behavior
|
||||
- [ ] Empty / undefined / duplicate / populated data states
|
||||
- [ ] Shared hooks / components / modules / helpers reusing the logic
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
|
||||
@@ -501,6 +523,8 @@ If this task REMOVES existing functionality (deleting modules, settings, API end
|
||||
## Testing requirements
|
||||
- Require real automated tests with assertions that run in the project's test runner
|
||||
- Typecheck/build/manual checks are not tests and cannot replace tests
|
||||
- For bug fixes, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix spec as a blocking REVISE.
|
||||
- For bug fixes, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers.
|
||||
- For bug fixes, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states — not just the reported repro (see FN-5787/FN-5789/FN-5803 and FN-5751)
|
||||
- Include targeted tests in implementation steps and full quality-gate runs in final verification
|
||||
|
||||
|
||||
Reference in New Issue
Block a user