fix: prevent stale planning approvals and review churn (#3327)
## Summary Planning can no longer approve or execute against evidence from a superseded dependency episode. Dependency mutations, approval decisions, recovery, and execution admission now share serialized lifecycle rules, so stale planner work cannot restore an invalid approval or release an unplanned task. Review also converges instead of discovering one blocker per round. Planning performs a repository-grounded completeness pass up front; Plan Review batches all independently discoverable blockers and carries an episode-scoped decision ledger across revisions; code review traces changed invariants through production consumers and tests. Repeated feedback still advances the safety budget, while provider failures and superseded episodes stay outside the remediation ledger. The dashboard now exposes manual approval only for the intended exhausted-review state, and refusal/recovery audit events make rejected lifecycle transitions diagnosable without leaking prompt content. ## Validation - `pnpm verify:fast` — scoped typechecks/builds, CLI build, and boot smoke passed. - Focused Core and Engine regression suites — 511 tests passed. - `pnpm lint`, strict changeset validation, Core/Engine typechecks, and package builds passed. Fixes #3325. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved Plan Review approvals, rejections, and replan-cap handling across task workflows. * Added cumulative feedback and attempt tracking across repeated planning reviews. * Added safer recovery for stalled planning handoffs and interrupted approval updates. * **Bug Fixes** * Prevented stale approvals and unplanned execution after dependency changes. * Improved concurrent approval handling, retryability, and refusal-record deduplication. * Refined dashboard approval indicators and responsive approval views. * **Quality Improvements** * Strengthened planning and code-review completeness checks and blocking-finding coverage. * Preserved review history while clearly marking outdated approvals. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
7
.changeset/fn-8768-review-followups.md
Normal file
7
.changeset/fn-8768-review-followups.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Prevent stale plan approvals and strengthen planning and review completeness.
|
||||
category: fix
|
||||
dev: Adds bounded lifecycle locks, planning-episode evidence, approval serialization, and convergent planning/review ledgers.
|
||||
@@ -909,6 +909,8 @@ export async function usePgTaskStore(
|
||||
export interface SharedPgTaskStoreHarness {
|
||||
readonly rootDir: () => string;
|
||||
readonly globalDir: () => string;
|
||||
/** Direct connection URL for session-level PostgreSQL primitive tests. */
|
||||
readonly testUrl: () => string;
|
||||
readonly store: () => TaskStore;
|
||||
readonly layer: () => AsyncDataLayer;
|
||||
readonly adminDb: () => PostgresJsDatabase;
|
||||
@@ -978,6 +980,7 @@ export function createSharedPgTaskStoreTestHarness(options?: {
|
||||
return {
|
||||
rootDir: () => harness?.rootDir ?? "",
|
||||
globalDir: () => harness?.rootDir ?? "",
|
||||
testUrl: () => harness?.testUrl ?? "",
|
||||
store: () => {
|
||||
if (!store) throw new Error("SharedPgTaskStoreHarness: beforeAll not called yet");
|
||||
return store;
|
||||
@@ -1103,4 +1106,3 @@ export function createSharedPgTaskStoreTestHarness(options?: {
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -62,28 +62,40 @@ describe("resolveAgentPrompt", () => {
|
||||
expect(result).not.toContain("turn it into short, actionable task specs or follow-up tickets");
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:ReviewPromptBoundaries 2026-08-04-06:35:
|
||||
The base reviewer remains role-neutral. Planning and code-review completeness
|
||||
policies are injected only at their workflow seams, avoiding mixed contracts.
|
||||
*/
|
||||
it("returns the correct built-in prompt for reviewer when no config provided", () => {
|
||||
const result = resolveAgentPrompt("reviewer");
|
||||
expect(result).toBeTruthy();
|
||||
expect(result).toContain("independent code and plan reviewer");
|
||||
// FNXC:PlanReviewReplan 2026-07-15-11:15: convergence guidance for Plan Review REVISE thrash.
|
||||
expect(result).toContain("Spec / Plan Review Convergence");
|
||||
expect(result).toContain("concrete PROMPT.md edit");
|
||||
expect(result).not.toContain("## Mandatory Plan Review Procedure");
|
||||
expect(result).not.toContain("Mandatory Code Review Procedure");
|
||||
});
|
||||
|
||||
// FNXC:TriagePlanReviewConvergence 2026-07-16-19:40: lock the new triage-side planner sections.
|
||||
it("includes the front-loaded File Scope and Storage architecture sections in the triage prompt", () => {
|
||||
const result = resolveAgentPrompt("triage");
|
||||
expect(result).toContain("## Mandatory Planning Completeness Procedure");
|
||||
expect(result).toContain("planning ledger");
|
||||
expect(result).toContain("participant graph and both relevant orderings");
|
||||
expect(result).toContain("fresh holistic completeness pass");
|
||||
expect(result).toContain("## File Scope — front-load surface enumeration");
|
||||
expect(result).toContain("## Storage architecture");
|
||||
});
|
||||
|
||||
// FNXC:TriagePlanReviewConvergence 2026-07-16-19:40: lock the new reviewer-side spec convergence sections.
|
||||
it("keeps the fast planning seam lean while preserving completeness invariants", () => {
|
||||
const result = builtinSeamPrompt("planning-fast");
|
||||
expect(result).toContain("## Fast Planning Completeness Check");
|
||||
expect(result).toContain("both relevant orderings and failure cleanup");
|
||||
expect(result).toContain("complete cumulative ledger");
|
||||
});
|
||||
|
||||
it("includes Spec Altitude and re-review convergence sections in the reviewer prompt", () => {
|
||||
const result = resolveAgentPrompt("reviewer");
|
||||
expect(result).toContain("## Spec Altitude");
|
||||
expect(result).toContain("Converging on re-review");
|
||||
expect(result).toContain("Severity ratchet at attempt 3+");
|
||||
expect(result).not.toContain("prior-review ledger as a decision primer");
|
||||
});
|
||||
|
||||
it("returns the correct built-in prompt for merger when no config provided", () => {
|
||||
@@ -339,9 +351,12 @@ describe("resolveAgentPrompt", () => {
|
||||
expect(fastPrompt).toContain("Do not write bare `### Preflight` / `### Implementation` headings");
|
||||
expect(fastPrompt).not.toContain("## Review Level");
|
||||
expect(fastPrompt.length).toBeLessThan(standardPrompt.length / 3);
|
||||
// FNXC:OriginalDescriptionInPrompt 2026-07-14-23:35: Original Description contract
|
||||
// adds a few lines to fast planning; keep lean but allow the new mandatory section.
|
||||
expect(fastPrompt.length).toBeLessThan(6500);
|
||||
/*
|
||||
* FNXC:FastPlanningPrompt 2026-08-04-06:35:
|
||||
* The compact ledger may add one bounded paragraph, while both relative and
|
||||
* absolute caps keep fast planning materially smaller than the full prompt.
|
||||
*/
|
||||
expect(fastPrompt.length).toBeLessThan(7500);
|
||||
expect(fastPrompt.split("\n").length).toBeLessThan(120);
|
||||
});
|
||||
|
||||
|
||||
@@ -43,6 +43,18 @@ describe("codeReviewOptionalGroupNode", () => {
|
||||
expect(prompt).not.toContain('"verdict":"FAIL"');
|
||||
expect(prompt).toMatch(/git diff/);
|
||||
expect(prompt).toMatch(/out of scope/i);
|
||||
/*
|
||||
* FNXC:CodeReviewSurfaceCoverage 2026-08-04-06:35:
|
||||
* The built-in gate must trace requirements through production entry points,
|
||||
* temporal state, and every UI/API/CLI/agent consumer rather than diff only.
|
||||
*/
|
||||
expect(prompt).toContain("requirements ledger");
|
||||
expect(prompt).toContain("real production entry point");
|
||||
expect(prompt).toContain("## Symptom Verification");
|
||||
expect(prompt).toContain("## Surface Enumeration");
|
||||
expect(prompt).toContain("current state, version, or planning episode");
|
||||
expect(prompt).toContain("bounded");
|
||||
expect(prompt).toContain("UI, API, CLI, and agent consumers");
|
||||
});
|
||||
|
||||
it("builds a DEFAULT-ON optional-group with the stable group id and distinct inner id", () => {
|
||||
|
||||
@@ -659,6 +659,12 @@ describe("built-in workflows", () => {
|
||||
toolMode: "readonly",
|
||||
gateMode: "gate",
|
||||
});
|
||||
const planReviewPrompt = String(planReviewInnerConfig(ir).prompt);
|
||||
expect(planReviewPrompt).toContain("## Mandatory Plan Review Procedure");
|
||||
expect(planReviewPrompt).toContain("all independently discoverable blocking findings");
|
||||
expect(planReviewPrompt).toContain("prior-review ledger as a decision primer");
|
||||
expect(planReviewPrompt).toContain("never demote a critical defect merely because it was missed before");
|
||||
expect(planReviewPrompt).toContain("verdict notes must contain the complete blocking checklist");
|
||||
expect(byId.get("parse")?.column).toBe("in-progress");
|
||||
expect(byId.get("steps")?.column).toBe("in-progress");
|
||||
/*
|
||||
|
||||
@@ -68,6 +68,28 @@ describe("isPlanReviewSatisfied", () => {
|
||||
bypassedFromVerdict: "REVISE",
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects a historical pass or bypass superseded by a later planning episode", () => {
|
||||
expect(isPlanReviewSatisfied({
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed",
|
||||
supersededAt: "2026-08-04T02:00:00.000Z",
|
||||
supersededReason: "dependency-change",
|
||||
})).toBe(false);
|
||||
expect(isPlanReviewSatisfied({
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "skipped",
|
||||
bypassedBy: "operator",
|
||||
bypassedAt: "2026-08-04T01:00:00.000Z",
|
||||
bypassReason: "Approved at the old revision cap",
|
||||
bypassedFromStatus: "failed",
|
||||
bypassedFromVerdict: "REVISE",
|
||||
supersededAt: "2026-08-04T02:00:00.000Z",
|
||||
supersededReason: "dependency-change",
|
||||
})).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
|
||||
@@ -0,0 +1,165 @@
|
||||
import { afterAll, afterEach, beforeAll, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
pgDescribe,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../__test-utils__/pg-test-harness.js";
|
||||
import {
|
||||
PlanningLifecycleLockTransportError,
|
||||
withPlanningLifecycleAdvisoryLock,
|
||||
} from "../../postgres/advisory-locks.js";
|
||||
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_planning_lock",
|
||||
poolMax: 1,
|
||||
});
|
||||
|
||||
function lock(
|
||||
projectId: string,
|
||||
taskId: string,
|
||||
callback: () => Promise<void>,
|
||||
timeoutMs = 1_000,
|
||||
onLockAcquisitionAttempt?: () => void,
|
||||
): Promise<void> {
|
||||
return withPlanningLifecycleAdvisoryLock({
|
||||
projectId,
|
||||
taskId,
|
||||
directSessionUrl: h.testUrl(),
|
||||
provenance: "migration-override",
|
||||
runtimeUrl: h.testUrl(),
|
||||
migrationUrl: h.testUrl(),
|
||||
timeoutMs,
|
||||
onLockAcquisitionAttempt,
|
||||
}, callback);
|
||||
}
|
||||
|
||||
pgDescribe("planning lifecycle advisory lock", () => {
|
||||
beforeAll(h.beforeAll);
|
||||
afterAll(h.afterAll);
|
||||
afterEach(h.afterEach);
|
||||
|
||||
it("serializes the same project/task key on dedicated sessions", async () => {
|
||||
let releaseFirst!: () => void;
|
||||
const firstCanFinish = new Promise<void>((resolve) => { releaseFirst = resolve; });
|
||||
let firstEntered!: () => void;
|
||||
const firstIsHolding = new Promise<void>((resolve) => { firstEntered = resolve; });
|
||||
const order: string[] = [];
|
||||
|
||||
const first = lock("project-a", "FN-1", async () => {
|
||||
order.push("first-enter");
|
||||
firstEntered();
|
||||
await firstCanFinish;
|
||||
order.push("first-exit");
|
||||
});
|
||||
await firstIsHolding;
|
||||
|
||||
let secondAttempted!: () => void;
|
||||
const secondAttemptIsDispatched = new Promise<void>((resolve) => { secondAttempted = resolve; });
|
||||
const second = lock("project-a", "FN-1", async () => {
|
||||
order.push("second-enter");
|
||||
}, 1_000, secondAttempted);
|
||||
|
||||
await secondAttemptIsDispatched;
|
||||
expect(order).toEqual(["first-enter"]);
|
||||
|
||||
releaseFirst();
|
||||
await Promise.all([first, second]);
|
||||
expect(order).toEqual(["first-enter", "first-exit", "second-enter"]);
|
||||
});
|
||||
|
||||
it("does not contend across project or task keys", async () => {
|
||||
let releaseFirst!: () => void;
|
||||
const firstCanFinish = new Promise<void>((resolve) => { releaseFirst = resolve; });
|
||||
let firstEntered!: () => void;
|
||||
const firstIsHolding = new Promise<void>((resolve) => { firstEntered = resolve; });
|
||||
|
||||
const first = lock("project-a", "FN-1", async () => {
|
||||
firstEntered();
|
||||
await firstCanFinish;
|
||||
});
|
||||
await firstIsHolding;
|
||||
|
||||
await Promise.all([
|
||||
lock("project-b", "FN-1", async () => {}),
|
||||
lock("project-a", "FN-2", async () => {}),
|
||||
]);
|
||||
releaseFirst();
|
||||
await first;
|
||||
});
|
||||
|
||||
it("unlocks and closes the dedicated session when the callback throws", async () => {
|
||||
await expect(lock("project-a", "FN-1", async () => {
|
||||
throw new Error("callback failed");
|
||||
})).rejects.toThrow("callback failed");
|
||||
|
||||
await expect(lock("project-a", "FN-1", async () => {})).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it.each(["embedded-lifecycle", "migration-override"] as const)(
|
||||
"accepts a verified %s direct-session provenance",
|
||||
async (provenance) => {
|
||||
await expect(withPlanningLifecycleAdvisoryLock({
|
||||
projectId: "project-a",
|
||||
taskId: "FN-1",
|
||||
directSessionUrl: h.testUrl(),
|
||||
provenance,
|
||||
runtimeUrl: h.testUrl(),
|
||||
migrationUrl: h.testUrl(),
|
||||
timeoutMs: 1_000,
|
||||
}, async () => {})).resolves.toBeUndefined();
|
||||
},
|
||||
);
|
||||
|
||||
it("bounds lock contention with a typed transport error", async () => {
|
||||
let releaseFirst!: () => void;
|
||||
const firstCanFinish = new Promise<void>((resolve) => { releaseFirst = resolve; });
|
||||
let firstEntered!: () => void;
|
||||
const firstIsHolding = new Promise<void>((resolve) => { firstEntered = resolve; });
|
||||
const first = lock("project-a", "FN-1", async () => {
|
||||
firstEntered();
|
||||
await firstCanFinish;
|
||||
});
|
||||
await firstIsHolding;
|
||||
|
||||
await expect(lock("project-a", "FN-1", async () => {}, 50))
|
||||
.rejects.toBeInstanceOf(PlanningLifecycleLockTransportError);
|
||||
releaseFirst();
|
||||
await first;
|
||||
});
|
||||
|
||||
it("fails closed for unavailable, pooled, missing, and mismatched endpoints", async () => {
|
||||
const base = {
|
||||
projectId: "project-a",
|
||||
taskId: "FN-1",
|
||||
provenance: "migration-override" as const,
|
||||
timeoutMs: 50,
|
||||
};
|
||||
const callback = async () => {};
|
||||
|
||||
await expect(withPlanningLifecycleAdvisoryLock({
|
||||
...base,
|
||||
directSessionUrl: null,
|
||||
runtimeUrl: h.testUrl(),
|
||||
migrationUrl: h.testUrl(),
|
||||
}, callback)).rejects.toBeInstanceOf(PlanningLifecycleLockTransportError);
|
||||
await expect(withPlanningLifecycleAdvisoryLock({
|
||||
...base,
|
||||
directSessionUrl: "postgresql://localhost:5432/db?pgbouncer=true",
|
||||
runtimeUrl: "postgresql://localhost:5432/db?pgbouncer=true",
|
||||
migrationUrl: "postgresql://localhost:5432/db?pgbouncer=true",
|
||||
}, callback)).rejects.toBeInstanceOf(PlanningLifecycleLockTransportError);
|
||||
await expect(withPlanningLifecycleAdvisoryLock({
|
||||
...base,
|
||||
directSessionUrl: h.testUrl(),
|
||||
runtimeUrl: h.testUrl(),
|
||||
migrationUrl: `${h.testUrl()}_other`,
|
||||
}, callback)).rejects.toBeInstanceOf(PlanningLifecycleLockTransportError);
|
||||
await expect(withPlanningLifecycleAdvisoryLock({
|
||||
...base,
|
||||
directSessionUrl: "postgresql://127.0.0.1:1/unavailable",
|
||||
runtimeUrl: "postgresql://127.0.0.1:1/unavailable",
|
||||
migrationUrl: "postgresql://127.0.0.1:1/unavailable",
|
||||
}, callback)).rejects.toBeInstanceOf(PlanningLifecycleLockTransportError);
|
||||
});
|
||||
});
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../__test-utils__/pg-test-harness.js";
|
||||
import type { TaskStore } from "../../store.js";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../../workflows/builtin-coding-workflow-ir.js";
|
||||
|
||||
const pgTest = pgDescribe;
|
||||
|
||||
@@ -126,6 +127,144 @@ pgTest("TaskStore dependency mutations (PostgreSQL)", () => {
|
||||
expect((await store.getWorkflowWorkItem(pending.id))?.state).toBe("cancelled");
|
||||
});
|
||||
|
||||
it("keeps invalidation and continuation cancellation authoritative in a combined updateTask patch", async () => {
|
||||
const prerequisite = await store.createTask({ description: "combined prerequisite", column: "done" });
|
||||
const dependent = await store.createTask({ description: "combined dependent", column: "todo" });
|
||||
const pending = await store.replaceActiveTaskWorkflowContinuation({
|
||||
runId: `${dependent.id}:continuation:0`, taskId: dependent.id, nodeId: "plan-review",
|
||||
kind: "task", state: "runnable", stableWorkflowRunId: `${dependent.id}:workflow`,
|
||||
continuationSequence: 0, waitReason: "planning", sourceColumn: "todo", targetColumn: "todo", irHash: "ir-v1",
|
||||
});
|
||||
|
||||
await store.updateTask(dependent.id, {
|
||||
dependencies: [prerequisite.id],
|
||||
status: null,
|
||||
approvedPlanFingerprint: "sha256:current",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed",
|
||||
completedAt: "2026-08-04T02:00:00.000Z",
|
||||
}],
|
||||
});
|
||||
|
||||
const updated = await store.getTask(dependent.id);
|
||||
expect(updated.status).toBe("needs-replan");
|
||||
expect(updated.approvedPlanFingerprint).toBeUndefined();
|
||||
expect(updated.awaitingApprovalReason).toBeUndefined();
|
||||
expect(updated.workflowStepResults).toEqual([
|
||||
expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
status: "passed",
|
||||
supersededAt: expect.any(String),
|
||||
supersededReason: "dependency-change",
|
||||
}),
|
||||
]);
|
||||
expect((await store.getWorkflowWorkItem(pending.id))?.state).toBe("cancelled");
|
||||
});
|
||||
|
||||
it.each(["dedicated", "generic"] as const)(
|
||||
"preserves but supersedes Plan Review approval through the %s dependency API",
|
||||
async (api) => {
|
||||
const prerequisite = await store.createTask({ description: `${api} prerequisite`, column: "done" });
|
||||
const dependent = await store.createTask({ description: `${api} dependent`, column: "todo" });
|
||||
await store.updateTask(dependent.id, {
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed",
|
||||
completedAt: "2026-08-04T01:00:00.000Z",
|
||||
}],
|
||||
});
|
||||
const pending = await store.replaceActiveTaskWorkflowContinuation({
|
||||
runId: `${dependent.id}:continuation:0`, taskId: dependent.id, nodeId: "plan-review",
|
||||
kind: "task", state: "runnable", stableWorkflowRunId: `${dependent.id}:workflow`,
|
||||
continuationSequence: 0, waitReason: "planning", sourceColumn: "todo", targetColumn: "todo", irHash: "ir-v1",
|
||||
});
|
||||
|
||||
if (api === "dedicated") {
|
||||
await store.updateTaskDependencies(dependent.id, { operation: "add", dependency: prerequisite.id });
|
||||
} else {
|
||||
await store.updateTask(dependent.id, { dependencies: [prerequisite.id] });
|
||||
}
|
||||
|
||||
const updated = await store.getTask(dependent.id);
|
||||
expect(updated.status).toBe("needs-replan");
|
||||
expect(updated.workflowStepResults).toEqual([
|
||||
expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
status: "passed",
|
||||
supersededAt: expect.any(String),
|
||||
supersededReason: "dependency-change",
|
||||
}),
|
||||
]);
|
||||
expect((await store.getWorkflowWorkItem(pending.id))?.state).toBe("cancelled");
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["dedicated", "generic"] as const)(
|
||||
"invalidates and rehomes an exhausted split-column Plan Review through the %s dependency API",
|
||||
async (api) => {
|
||||
const definition = await store.createWorkflowDefinition({
|
||||
name: `split review dependency ${api}`,
|
||||
ir: {
|
||||
...BUILTIN_CODING_WORKFLOW_IR,
|
||||
id: `split-review-dependency-${api}`,
|
||||
nodes: BUILTIN_CODING_WORKFLOW_IR.nodes.map((node) =>
|
||||
node.id === "plan-review" ? { ...node, column: "in-review" } : node
|
||||
),
|
||||
},
|
||||
});
|
||||
const prerequisite = await store.createTask({
|
||||
description: `${api} prerequisite`,
|
||||
column: "done",
|
||||
workflowId: definition.id,
|
||||
} as never);
|
||||
const dependent = await store.createTask({
|
||||
description: `${api} dependent`,
|
||||
workflowId: definition.id,
|
||||
} as never);
|
||||
const intakeColumn = dependent.column;
|
||||
await store.moveTask(dependent.id, "in-review", {
|
||||
moveSource: "engine",
|
||||
recoveryRehome: true,
|
||||
bypassGuards: true,
|
||||
});
|
||||
await store.updateTask(dependent.id, {
|
||||
status: "awaiting-approval",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
approvedPlanFingerprint: "sha256:stale",
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
completedAt: "2026-08-04T05:00:00.000Z",
|
||||
}],
|
||||
} as never);
|
||||
|
||||
if (api === "dedicated") {
|
||||
await store.updateTaskDependencies(dependent.id, {
|
||||
operation: "add",
|
||||
dependency: prerequisite.id,
|
||||
});
|
||||
} else {
|
||||
await store.updateTask(dependent.id, { dependencies: [prerequisite.id] });
|
||||
}
|
||||
|
||||
const updated = await store.getTask(dependent.id);
|
||||
expect(updated.column).toBe(intakeColumn);
|
||||
expect(updated.status).toBe("needs-replan");
|
||||
expect(updated.awaitingApprovalReason).toBeUndefined();
|
||||
expect(updated.approvedPlanFingerprint).toBeUndefined();
|
||||
expect(updated.workflowStepResults).toContainEqual(expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
supersededReason: "dependency-change",
|
||||
}));
|
||||
},
|
||||
);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-02:05 (PR #2720 review — greptile):
|
||||
DISTINCT HOLD AND INTAKE LANES, the configuration the default lineage does not exercise.
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
import { afterAll, afterEach, beforeAll, expect, it } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import {
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
pgDescribe,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../__test-utils__/pg-test-harness.js";
|
||||
import * as schema from "../../postgres/schema/index.js";
|
||||
import type { ResolvedBackend } from "../../postgres/backend-resolver.js";
|
||||
import { createConnectionSetFromUrl } from "../../postgres/connection.js";
|
||||
import { createAsyncDataLayer } from "../../postgres/data-layer.js";
|
||||
import { TaskStore } from "../../store.js";
|
||||
import { insertTaskRow } from "../../task-store/async/async-persistence.js";
|
||||
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_unplanned_block",
|
||||
});
|
||||
|
||||
pgDescribe("unplanned execution refusal dedupe", () => {
|
||||
beforeAll(h.beforeAll);
|
||||
afterAll(h.afterAll);
|
||||
afterEach(h.afterEach);
|
||||
|
||||
it("atomically records one durable log entry for concurrent repeat calls", async () => {
|
||||
const task = await h.store().createTask({ description: "unplanned task" });
|
||||
const before = (await h.store().getTask(task.id)).updatedAt;
|
||||
|
||||
const results = await Promise.all(Array.from(
|
||||
{ length: 8 },
|
||||
() => h.store().checkAndRecordUnplannedExecutionBlock(task.id, "episode-a"),
|
||||
));
|
||||
|
||||
expect(results.filter(Boolean)).toHaveLength(1);
|
||||
const updated = await h.store().getTask(task.id);
|
||||
expect(updated.updatedAt).toBe(before);
|
||||
expect(updated.log?.filter((entry) => entry.action.includes("Execution dispatch refused"))).toHaveLength(1);
|
||||
const markers = await h.adminDb().select().from(schema.project.unplannedExecutionBlocks);
|
||||
expect(markers).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("records a new refusal when the planning episode changes", async () => {
|
||||
const task = await h.store().createTask({ description: "replanned task" });
|
||||
|
||||
await expect(h.store().checkAndRecordUnplannedExecutionBlock(task.id, "episode-a")).resolves.toBe(true);
|
||||
await expect(h.store().checkAndRecordUnplannedExecutionBlock(task.id, "episode-b")).resolves.toBe(true);
|
||||
|
||||
const updated = await h.store().getTask(task.id);
|
||||
expect(updated.log?.filter((entry) => entry.action.includes("Execution dispatch refused"))).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("keeps the same task id and episode independent across projects", async () => {
|
||||
const backend: ResolvedBackend = {
|
||||
mode: "external",
|
||||
runtimeUrl: h.testUrl(),
|
||||
migrationUrl: h.testUrl(),
|
||||
migrationUrlOverridden: true,
|
||||
directSessionUrl: h.testUrl(),
|
||||
directSessionProvenance: "migration-override",
|
||||
};
|
||||
const [connectionsA, connectionsB, rootA, rootB] = await Promise.all([
|
||||
createConnectionSetFromUrl(backend, { projectId: "project-a", useRuntimeRole: true }),
|
||||
createConnectionSetFromUrl(backend, { projectId: "project-b", useRuntimeRole: true }),
|
||||
mkdtemp(join(tmpdir(), "fusion-refusal-a-")),
|
||||
mkdtemp(join(tmpdir(), "fusion-refusal-b-")),
|
||||
]);
|
||||
try {
|
||||
const layerA = createAsyncDataLayer(connectionsA, { projectId: "project-a" });
|
||||
const layerB = createAsyncDataLayer(connectionsB, { projectId: "project-b" });
|
||||
const storeA = new TaskStore(rootA, undefined, { asyncLayer: layerA });
|
||||
const storeB = new TaskStore(rootB, undefined, { asyncLayer: layerB });
|
||||
const now = new Date().toISOString();
|
||||
const row = {
|
||||
id: "FN-SAME",
|
||||
description: "same id",
|
||||
column: "todo",
|
||||
currentStep: 0,
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
};
|
||||
await Promise.all([
|
||||
insertTaskRow(layerA, row, { lineageId: null }),
|
||||
insertTaskRow(layerB, row, { lineageId: null }),
|
||||
]);
|
||||
|
||||
await expect(Promise.all([
|
||||
storeA.checkAndRecordUnplannedExecutionBlock(row.id, "episode-a"),
|
||||
storeB.checkAndRecordUnplannedExecutionBlock(row.id, "episode-a"),
|
||||
])).resolves.toEqual([true, true]);
|
||||
|
||||
const markers = await h.adminDb().select().from(schema.project.unplannedExecutionBlocks)
|
||||
.where(eq(schema.project.unplannedExecutionBlocks.taskId, row.id));
|
||||
expect(markers.map((marker) => marker.projectId).sort()).toEqual(["project-a", "project-b"]);
|
||||
} finally {
|
||||
await Promise.allSettled([
|
||||
connectionsA.close(),
|
||||
connectionsB.close(),
|
||||
rm(rootA, { recursive: true, force: true }),
|
||||
rm(rootB, { recursive: true, force: true }),
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it("rolls back the marker when the live task guard fails", async () => {
|
||||
const archived = await h.store().createTask({ description: "archived task" });
|
||||
await h.store().archiveTask(archived.id, { cleanup: false });
|
||||
|
||||
await expect(h.store().checkAndRecordUnplannedExecutionBlock(archived.id, "episode-a"))
|
||||
.rejects.toThrow(/not found or archived/);
|
||||
|
||||
const archivedMarkers = await h.adminDb().select().from(schema.project.unplannedExecutionBlocks)
|
||||
.where(eq(schema.project.unplannedExecutionBlocks.taskId, archived.id));
|
||||
expect(archivedMarkers).toHaveLength(0);
|
||||
|
||||
const deleted = await h.store().createTask({ description: "deleted task" });
|
||||
await h.store().deleteTask(deleted.id);
|
||||
await expect(h.store().checkAndRecordUnplannedExecutionBlock(deleted.id, "episode-a"))
|
||||
.rejects.toThrow(/not found or archived/);
|
||||
const deletedMarkers = await h.adminDb().select().from(schema.project.unplannedExecutionBlocks)
|
||||
.where(eq(schema.project.unplannedExecutionBlocks.taskId, deleted.id));
|
||||
expect(deletedMarkers).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
@@ -143,6 +143,120 @@ describe("adding a dependency never parks a card in a deleted column", () => {
|
||||
expect(row.awaitingApprovalReason).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps dependency invalidation authoritative over a combined status clear", async () => {
|
||||
const { store, row } = harness({
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
status: "awaiting-approval",
|
||||
}, DEFAULT_IR);
|
||||
|
||||
await run(store, { dependencies: ["FN-2"], status: null });
|
||||
|
||||
expect(row.status).toBe("needs-replan");
|
||||
});
|
||||
|
||||
it("keeps dependency invalidation authoritative over combined current approval evidence", async () => {
|
||||
const currentResult = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed",
|
||||
completedAt: "2026-08-04T02:00:00.000Z",
|
||||
};
|
||||
const { store, row } = harness({
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
}, DEFAULT_IR);
|
||||
|
||||
await run(store, {
|
||||
dependencies: ["FN-2"],
|
||||
status: "awaiting-approval",
|
||||
approvedPlanFingerprint: "sha256:current",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
workflowStepResults: [currentResult],
|
||||
});
|
||||
|
||||
expect(row.status).toBe("needs-replan");
|
||||
expect(row.approvedPlanFingerprint).toBeUndefined();
|
||||
expect(row.awaitingApprovalReason).toBeUndefined();
|
||||
expect(row.workflowStepResults).toEqual([
|
||||
expect.objectContaining({
|
||||
...currentResult,
|
||||
supersededAt: expect.any(String),
|
||||
supersededReason: "dependency-change",
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it.each(["passed", "pending"] as const)(
|
||||
"preserves but supersedes an old %s Plan Review projection when a dependency starts a new planning episode",
|
||||
async (status) => {
|
||||
const oldResult = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status,
|
||||
...(status === "pending"
|
||||
? { startedAt: "2026-08-04T01:00:00.000Z", leaseOwner: "planner:old-episode" }
|
||||
: { completedAt: "2026-08-04T01:00:00.000Z" }),
|
||||
};
|
||||
const { store, row } = harness({
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
workflowStepResults: [oldResult],
|
||||
}, DEFAULT_IR);
|
||||
|
||||
await run(store, { dependencies: ["FN-2"] });
|
||||
|
||||
expect(row.workflowStepResults).toEqual([
|
||||
expect.objectContaining({
|
||||
...oldResult,
|
||||
supersededAt: expect.any(String),
|
||||
supersededReason: "dependency-change",
|
||||
}),
|
||||
]);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
["null", null],
|
||||
["empty", []],
|
||||
["unrelated replacement", [{
|
||||
workflowStepId: "code-review",
|
||||
workflowStepName: "Code Review",
|
||||
status: "passed",
|
||||
completedAt: "2026-08-04T02:00:00.000Z",
|
||||
}]],
|
||||
] as const)(
|
||||
"retains the superseded prior Plan Review when a combined patch supplies %s workflow results",
|
||||
async (label, workflowStepResults) => {
|
||||
const oldPlanReview = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "passed" as const,
|
||||
completedAt: "2026-08-04T01:00:00.000Z",
|
||||
};
|
||||
const { store, row } = harness({
|
||||
column: "todo",
|
||||
dependencies: [],
|
||||
workflowStepResults: [oldPlanReview],
|
||||
}, DEFAULT_IR);
|
||||
|
||||
await run(store, { dependencies: ["FN-2"], workflowStepResults });
|
||||
|
||||
expect(row.workflowStepResults).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
...oldPlanReview,
|
||||
supersededAt: expect.any(String),
|
||||
supersededReason: "dependency-change",
|
||||
}),
|
||||
]));
|
||||
if (label === "unrelated replacement") {
|
||||
expect(row.workflowStepResults).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ workflowStepId: "code-review", status: "passed" }),
|
||||
]));
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("moves a RENAMED board's hold card to its own intake lane", async () => {
|
||||
// Here intake and hold ARE different columns, so the move is real — and it goes to `inbox`,
|
||||
// a column this board actually declares.
|
||||
|
||||
@@ -51,6 +51,28 @@ describe("upsertWorkflowStepResult", () => {
|
||||
expect(next[0].priorAttempts?.[0].output).toBe("advisory-1");
|
||||
});
|
||||
|
||||
it("preserves superseded Plan Review evidence when the new planning episode starts", () => {
|
||||
const oldPass = makeResult({
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
startedAt: "T1",
|
||||
status: "passed",
|
||||
supersededAt: "T2",
|
||||
supersededReason: "dependency-change",
|
||||
});
|
||||
const nextEpisode = makeResult({
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
startedAt: "T3",
|
||||
status: "pending",
|
||||
});
|
||||
|
||||
const next = upsertWorkflowStepResult([oldPass], nextEpisode);
|
||||
|
||||
expect(next[0].status).toBe("pending");
|
||||
expect(next[0].priorAttempts).toEqual([oldPass]);
|
||||
});
|
||||
|
||||
it("does NOT snapshot when the replaced entry was passed/skipped/pending", () => {
|
||||
for (const status of ["passed", "skipped", "pending"] as const) {
|
||||
const attempt1 = makeResult({ startedAt: "T1", status, output: "attempt-1" });
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
*/
|
||||
|
||||
import type { AgentCapability, AgentPromptTemplate, AgentPromptsConfig } from "../types.js";
|
||||
import { FAST_PLANNING_COMPLETENESS_POLICY, PLANNING_COMPLETENESS_POLICY } from "./planning-review-policy.js";
|
||||
// FNXC:WorkflowLifecycleColumns 2026-07-30-11:00: these are ROLE comparisons, not column
|
||||
// guards — the planner LANE is named `triage` and stays named that. See PLANNER_AGENT_ROLE.
|
||||
import { PLANNER_AGENT_ROLE } from "../types/task/task-log.js";
|
||||
@@ -268,6 +269,8 @@ Fast planning also requires \`## Original Description\` (verbatim operator text)
|
||||
*/
|
||||
const FAST_TRIAGE_PROMPT_TEXT = `You are a task specification agent for "fn". This task is running in **fast mode**.
|
||||
|
||||
${FAST_PLANNING_COMPLETENESS_POLICY}
|
||||
|
||||
Write a lean, executable PROMPT.md quickly. Preserve safety gates, but skip heavyweight ceremony, review scoring, and proactive subtask analysis.
|
||||
|
||||
## Fast-mode priorities
|
||||
@@ -317,18 +320,10 @@ If the requested outcome is only to decide, route, or coordinate work, include \
|
||||
## Output
|
||||
Write PROMPT.md directly and stop. Do not call \`fn_review_spec()\`; workflow Plan Review is the single optional plan review gate before execution.`;
|
||||
|
||||
/*
|
||||
FNXC:TriagePlanReviewConvergence 2026-07-16-09:20:
|
||||
Planner-side convergence rules. `## File Scope — front-load surface enumeration` makes the
|
||||
planner grep ALL call-sites + persistence/backend paths BEFORE writing File Scope so Plan Review
|
||||
confirms coverage instead of surfacing a deeper missed surface each cycle (a top replan-loop
|
||||
cause). `## Storage architecture (ground truth)` encodes verified facts (Postgres-only store,
|
||||
no task-store/store.ts, tasks composite PK (project_id, id), migrations registered in
|
||||
schema-applier.ts) so the planner stops citing removed/nonexistent things and losing rounds to
|
||||
stale-codebase-fact rejections (FN-7996/FN-8105/FN-8108).
|
||||
*/
|
||||
const TRIAGE_PROMPT_TEXT = `You are a task specification agent for "fn", an AI-orchestrated task board.
|
||||
|
||||
${PLANNING_COMPLETENESS_POLICY}
|
||||
|
||||
## Your Role
|
||||
You are the specification quality gate for implementation success.
|
||||
Your job: take a rough task description and produce a fully specified PROMPT.md that another AI agent can execute autonomously in a fresh context with zero memory of this conversation.
|
||||
@@ -686,23 +681,6 @@ If the task targets a different task ID (audit, forensic walk, historical reconc
|
||||
<!-- Frontend UX criteria are applied deterministically by packages/core/src/frontend-ux-policy.ts and mirror the "frontend-ux-design" reviewer persona in packages/core/src/types.ts. -->`;;
|
||||
|
||||
// FN-6235: single source for the built-in reviewer policy; the engine REVIEWER_SYSTEM_PROMPT duplicate was removed.
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-15-11:15:
|
||||
Built-in reviewer prompt includes Spec/Plan Review Convergence rules so REVISE stays
|
||||
blocking-only with surgical fix lists, reducing planner↔Plan-Review thrash (paired with
|
||||
triage seeding existing PROMPT.md on needs-replan and reviewType "spec" for the triage gate).
|
||||
|
||||
FNXC:TriagePlanReviewConvergence 2026-07-16-09:20:
|
||||
Spec gates looped to the 8-replan cap (FN-7996/FN-8105/FN-8108) because each cycle re-reviewed
|
||||
cold and surfaced a NEW, deeper blocking issue instead of confirming prior ones were fixed
|
||||
(goalpost movement/whack-a-mole), and reviewed at implementation altitude (demanding exact SQL,
|
||||
lock/CAS design, column mapping, field-level failure values against a *spec*). Three prompt
|
||||
rules address this: (1) "Converging on re-review" in `## Spec / Plan Review Convergence` —
|
||||
verify prior issues, don't REVISE for the reviewer's own earlier miss, and at attempt 3+ ratchet
|
||||
to critical-only; (2) `## Spec Altitude` extends the plan-altitude principle to the spec gate so
|
||||
implementation decisions are deferred to code review; the per-attempt prior-feedback + attempt
|
||||
number are threaded from triage via reviewStep/buildReviewRequest.
|
||||
*/
|
||||
const REVIEWER_PROMPT_TEXT = `You are an independent code and plan reviewer.
|
||||
|
||||
## Your Role
|
||||
@@ -832,20 +810,6 @@ Concrete examples:
|
||||
- [Optional improvements, not blocking]
|
||||
\`\`\`
|
||||
|
||||
## Spec / Plan Review Convergence
|
||||
|
||||
Specs and pre-execution Plan Review share this gate. Prefer **APPROVE** / **APPROVE_WITH_NOTES** when the plan is executable enough for an agent to implement. Put optional polish only under **Suggestions**.
|
||||
|
||||
When you must **REVISE**:
|
||||
- List each blocking issue as a concrete PROMPT.md edit (which section, what to add/change/remove).
|
||||
- Do not demand a full rewrite unless the approach is fundamentally wrong (**RETHINK**).
|
||||
- Prefer fixing local PROMPT.md defects in-session when you have write tools, then **APPROVE**, instead of bouncing the task through another full replan cycle.
|
||||
|
||||
**Converging on re-review (when the request includes your prior feedback + a Plan Review attempt number):**
|
||||
- This is a spec you already reviewed. VERIFY each issue you previously raised was addressed. REVISE only for (a) a PRIOR blocking issue still unresolved, or (b) a genuinely NEW problem THIS revision introduced.
|
||||
- Do NOT introduce a new blocking issue that ALSO applied to the version you previously reviewed — that is your own earlier miss. Record it under **Suggestions**, not REVISE.
|
||||
- **Severity ratchet at attempt 3+:** gate ONLY on \`critical\` (delivery-blocking) issues. Downgrade lone \`important\`/\`minor\` spec-wording nits to **Suggestions** and APPROVE. Rationale: the executor and downstream code review are later gates — a spec need not be perfect to be executable, only executable.
|
||||
|
||||
## Spec Review — Undersplit Task Detection
|
||||
|
||||
When reviewing specs, assess whether the task should have been broken into subtasks. The bar for splitting is high — most tasks should remain whole. Coordination overhead (worktrees, dependency wiring, merge sequencing) is real, so splitting must clearly pay for itself.
|
||||
|
||||
20
packages/core/src/agents/code-review-policy.ts
Normal file
20
packages/core/src/agents/code-review-policy.ts
Normal file
@@ -0,0 +1,20 @@
|
||||
/*
|
||||
* FNXC:CodeReviewCompleteness 2026-08-04-06:35:
|
||||
* Code Review must prove every contract row through production reachability and
|
||||
* exact symptom coverage, reason about temporal state, and make a fresh
|
||||
* adversarial pass before approval.
|
||||
*/
|
||||
export const CODE_REVIEW_COMPLETENESS_POLICY = `## Mandatory Code Review Procedure
|
||||
|
||||
Apply this procedure to code reviews only. Plan/spec reviews use their dedicated criteria below.
|
||||
|
||||
Before choosing a verdict:
|
||||
|
||||
1. Build a **requirements ledger** from PROMPT.md. For every Mission outcome, Completion Criterion, relevant \`## Surface Enumeration\` item, and \`## Symptom Verification\` assertion, identify both implementation evidence and test evidence. A missing or partial required row is REVISE.
|
||||
2. Trace each changed behavior from its **real production entry point** and selector through the changed helper to its consumers. A helper-only unit test does not prove that production can reach the new branch.
|
||||
3. For bug fixes, locate an automated test that reproduces the exact reported failure and asserts it is gone across the enumerated surfaces. Green builds, nearby unit tests, and synthetic state injection alone are insufficient.
|
||||
4. For persistence, lifecycle, and concurrency changes, verify that evidence belongs to the **current state, version, or planning episode**; every read-check-write path is atomic, locked, or CAS-protected; all mutation surfaces preserve the invariant; waits and locks are bounded and cleanup-safe; and a production-shaped integration test covers both relevant orderings.
|
||||
5. For route or state changes, inspect all affected UI, API, CLI, and agent consumers plus paired success/failure or approve/reject paths for contract parity.
|
||||
6. Before APPROVE, perform a fresh adversarial validation pass: try to disprove each completed ledger row by reading the real caller, guard, consumer, and test. Cite \`file:line\` for every blocker and name the exact missing proof.
|
||||
|
||||
APPROVE only when every required ledger row has implementation and test evidence.`;
|
||||
37
packages/core/src/agents/planning-review-policy.ts
Normal file
37
packages/core/src/agents/planning-review-policy.ts
Normal file
@@ -0,0 +1,37 @@
|
||||
/*
|
||||
* FNXC:PlanningReviewCompleteness 2026-08-04-06:35:
|
||||
* Planning and Plan Review share one completeness contract: planning researches
|
||||
* and maps the full requirement ledger up front, while review evaluates the whole
|
||||
* artifact and batches every independently discoverable blocker in one round.
|
||||
*/
|
||||
export const PLANNING_COMPLETENESS_POLICY = `## Mandatory Planning Completeness Procedure
|
||||
|
||||
Before writing the final PROMPT.md:
|
||||
|
||||
1. Build an internal **planning ledger** from the Original Description, user comments, project instructions, cited issue/report, and any saved planning document. Preserve every requirement, settled decision, explicit non-goal, and acceptance outcome; do not silently replace the reporter's contract with a narrower reproduction.
|
||||
2. Research before structuring the plan. Trace the affected behavior through its real production entry points, callers, writers/readers, shared consumers, persistence boundaries, recovery paths, and existing tests. Resolve planning-time facts from the repository now; defer only details that genuinely require implementation-time discovery, and label those explicitly.
|
||||
3. For stateful, lifecycle, persistence, or concurrency work, enumerate the participant graph and both relevant orderings: stale-work cancellation/fencing, lock order and reentrancy, transaction boundaries, pool/transport limits, competing recovery paths, configuration/deployment identities, bypass/force paths, and failure cleanup. Every stated invariant must name the surfaces that preserve it.
|
||||
4. Map every ledger row to concrete File Scope entries, dependency-ordered implementation steps, and verification. Each feature-bearing step must name specific automated test scenarios with the input/state, action or ordering, and expected observable result; helper-only tests do not prove production reachability.
|
||||
5. Keep scope disciplined. Reuse current architecture and documented patterns unless the reporter contract requires a redesign. Put optional cleanup and adjacent ideas outside the active steps.
|
||||
6. Before persisting PROMPT.md, perform a fresh holistic completeness pass across the whole ledger. Try to disprove the plan by checking referenced paths and patterns, missing consumers, contradictory steps, uncovered failure orderings, and acceptance criteria without tests. Fix all gaps in one pass.
|
||||
|
||||
On a revision, treat the cumulative revision ledger as durable decisions unless a later entry explicitly supersedes one: preserve resolved items, address every unresolved item surgically, and rerun the complete procedure instead of checking only the latest comment.`;
|
||||
|
||||
export const FAST_PLANNING_COMPLETENESS_POLICY = `## Fast Planning Completeness Check
|
||||
|
||||
Before writing PROMPT.md, build a compact internal ledger from the Original Description, user comments, project rules, and acceptance outcomes. Inspect the real production callers/consumers, persistence or recovery paths, and nearby tests before choosing File Scope. For stateful or concurrent work, enumerate every participant plus both relevant orderings and failure cleanup. Map each requirement to a concrete step, file, and automated test scenario, then reread the whole plan once for missing surfaces, contradictions, and unproved outcomes. On revision, preserve prior decisions and address the complete cumulative ledger, not only the latest note.`;
|
||||
|
||||
export const PLAN_REVIEW_COMPLETENESS_POLICY = `## Mandatory Plan Review Procedure
|
||||
|
||||
Apply this procedure to plan/spec reviews only. Code reviews use their dedicated procedure.
|
||||
|
||||
Before choosing a verdict:
|
||||
|
||||
1. Build one **review ledger** for the entire PROMPT.md: Original Description and user comments; Mission and Completion Criteria; Surface Enumeration and Symptom Verification when required; every implementation step, File Scope entry, dependency, risk, and test/verification promise.
|
||||
2. Complete the full review before reporting. Check coherence and requirement traceability, feasibility against the current repository, scope discipline, execution ordering, and verification quality. When relevant, also inspect security/data integrity, state transitions, concurrency orderings, deployment/configuration boundaries, recovery competitors, and force/bypass paths.
|
||||
3. Review at specification altitude. Block when a required behavior, surface, ordering, safety constraint, or proof is missing or the stated approach cannot work. Keep optional implementation detail, wording polish, and nonessential improvements advisory.
|
||||
4. If REVISE is necessary, batch **all independently discoverable blocking findings** into this one verdict; do not stop after the first defect. Give each blocker a stable ID, cite the affected section or repository evidence, and state the concrete PROMPT.md correction. Put advisory observations in a separate list.
|
||||
5. On re-review, use the supplied prior-review ledger as a decision primer. Verify every prior blocker, do not re-raise resolved or rejected semantic duplicates, and preserve accepted decisions. A newly blocking finding must say whether the revision introduced it, which prior blocker genuinely masked it, or why it is independently delivery-blocking for correctness, security, data safety, or executability. Record an earlier reviewer miss explicitly; never demote a critical defect merely because it was missed before.
|
||||
6. After any same-session PROMPT.md edit, distrust the edit: reread the complete artifact, rebuild the ledger, and perform a fresh holistic pass before APPROVE.
|
||||
|
||||
APPROVE when the plan is executable and verifiable, not when it is cosmetically perfect. If returning REVISE, the verdict notes must contain the complete blocking checklist because those notes are the durable input to the next planning round.`;
|
||||
@@ -2252,6 +2252,7 @@ export {
|
||||
isTerminalStepResult,
|
||||
type ReviewLeaseDisposition,
|
||||
} from "./workflows/workflow-step-results.js";
|
||||
export { PLAN_REVIEW_COMPLETENESS_POLICY } from "./agents/planning-review-policy.js";
|
||||
/*
|
||||
FNXC:GateBarrelSync 2026-07-19-01:10:
|
||||
Cutover (IR-driven lifecycle) barrel sync — same failure class as the classifyReviewLease incident above, found again when U4's resolveWipBudgetColumns threw "is not a function" in the engine-core hold/release sweep and silently zeroed all scheduler releases (scheduler-workflow-cutover gate suite 10/28 red). RULE: every runtime export the cutover adds to index.ts MUST be mirrored here; the engine-core gate bundle builds @fusion/core from THIS barrel. This block mirrors the cutover's lifecycle modules (transition policy, lifecycle traits, capacity budget, IR pin/drift, review-level preset, legacy adoption, creation column).
|
||||
|
||||
@@ -2648,6 +2648,7 @@ export {
|
||||
isTerminalStepResult,
|
||||
type ReviewLeaseDisposition,
|
||||
} from "./workflows/workflow-step-results.js";
|
||||
export { PLAN_REVIEW_COMPLETENESS_POLICY } from "./agents/planning-review-policy.js";
|
||||
// FNXC:SqliteRemoval 2026-07-14: Export async audit reader so engine tests can
|
||||
// query run-audit events in backend mode (sync getRunAuditEvents returns [] in PG mode).
|
||||
export { queryRunAuditEvents } from "./task-store/async/async-audit.js";
|
||||
|
||||
@@ -19,6 +19,7 @@ cannot silently open the execution gate.
|
||||
*/
|
||||
export function isPlanReviewSatisfied(result: WorkflowStepResult): boolean {
|
||||
if (result.workflowStepId !== PLAN_REVIEW_GROUP_ID) return false;
|
||||
if (result.supersededAt != null) return false;
|
||||
if (result.status === "passed") return true;
|
||||
return result.status === "skipped"
|
||||
&& (result.bypassedFromStatus === "failed" || result.bypassedFromStatus === "advisory_failure")
|
||||
@@ -31,6 +32,24 @@ export function isPlanReviewSatisfied(result: WorkflowStepResult): boolean {
|
||||
&& result.bypassReason.trim().length > 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* FNXC:PlanReviewSupersession 2026-08-04-06:35:
|
||||
* A dependency change preserves Plan Review history for audit while retiring
|
||||
* every current gate projection, including an in-flight lease. Superseded
|
||||
* evidence belongs to the old planning episode and cannot satisfy the new gate.
|
||||
*/
|
||||
export function supersedePlanReviewResults(
|
||||
results: WorkflowStepResult[] | undefined,
|
||||
supersededAt: string,
|
||||
): WorkflowStepResult[] | undefined {
|
||||
if (!results?.some((result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.supersededAt == null)) {
|
||||
return results;
|
||||
}
|
||||
return results.map((result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID && result.supersededAt == null
|
||||
? { ...result, supersededAt, supersededReason: "dependency-change" }
|
||||
: result);
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — manual plan approval was not idempotent against unchanged plan content: an
|
||||
|
||||
@@ -30,6 +30,53 @@ export class PlanningLifecycleLockTransportError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
const DEFAULT_PLANNING_LIFECYCLE_LOCK_TIMEOUT_MS = 5_000;
|
||||
|
||||
type DedicatedPostgresClient = ReturnType<typeof postgres>;
|
||||
type PostgresBackendIdentity = {
|
||||
database: string;
|
||||
host: string | null;
|
||||
port: number | null;
|
||||
cluster: string | null;
|
||||
};
|
||||
|
||||
async function readPostgresBackendIdentity(client: DedicatedPostgresClient): Promise<PostgresBackendIdentity[]> {
|
||||
return await client<PostgresBackendIdentity[]>`
|
||||
SELECT current_database() AS database,
|
||||
inet_server_addr()::text AS host,
|
||||
inet_server_port() AS port,
|
||||
current_setting('cluster_name', true) AS cluster
|
||||
`;
|
||||
}
|
||||
|
||||
async function runBoundedTransportPhase<T>(
|
||||
client: DedicatedPostgresClient,
|
||||
timeoutMs: number,
|
||||
timeoutMessage: string,
|
||||
failureMessage: string,
|
||||
operation: () => Promise<T>,
|
||||
): Promise<T> {
|
||||
let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
return await Promise.race([
|
||||
operation(),
|
||||
new Promise<never>((_, reject) => {
|
||||
timeoutHandle = setTimeout(() => {
|
||||
void client.end({ timeout: 0 }).catch(() => undefined);
|
||||
reject(new PlanningLifecycleLockTransportError(timeoutMessage));
|
||||
}, timeoutMs);
|
||||
}),
|
||||
]);
|
||||
} catch (error) {
|
||||
if (error instanceof PlanningLifecycleLockTransportError) throw error;
|
||||
// Do not retain the driver error as `cause`: connection failures can carry
|
||||
// endpoint credentials, while this error is operator-visible.
|
||||
throw new PlanningLifecycleLockTransportError(failureMessage);
|
||||
} finally {
|
||||
if (timeoutHandle) clearTimeout(timeoutHandle);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:PlanningDependencyReseed 2026-08-04-00:43:
|
||||
* Planning handoff and dependency re-seed cross processes, so their outer lock
|
||||
@@ -45,10 +92,15 @@ export async function withPlanningLifecycleAdvisoryLock<T>(
|
||||
provenance: "embedded-lifecycle" | "migration-override" | null;
|
||||
runtimeUrl?: string | null;
|
||||
migrationUrl?: string | null;
|
||||
/** Bounds dedicated-session setup and lock acquisition, not callback work. */
|
||||
timeoutMs?: number;
|
||||
/** @internal Test seam fired when the driver dispatches the lock query. */
|
||||
onLockAcquisitionAttempt?: () => void;
|
||||
},
|
||||
callback: () => Promise<T>,
|
||||
): Promise<T> {
|
||||
const directUrl = input.directSessionUrl;
|
||||
const timeoutMs = Math.max(1, input.timeoutMs ?? DEFAULT_PLANNING_LIFECYCLE_LOCK_TIMEOUT_MS);
|
||||
if (!directUrl || !input.provenance || looksLikePoolerUrl(directUrl)) {
|
||||
throw new PlanningLifecycleLockTransportError("Planning lifecycle lock requires a direct PostgreSQL session endpoint");
|
||||
}
|
||||
@@ -81,7 +133,17 @@ export async function withPlanningLifecycleAdvisoryLock<T>(
|
||||
}
|
||||
}
|
||||
|
||||
const client = postgres(directUrl, { max: 1, prepare: false, onnotice: () => {} });
|
||||
const client = postgres(directUrl, {
|
||||
max: 1,
|
||||
connect_timeout: Math.max(1, Math.ceil(timeoutMs / 1_000)),
|
||||
prepare: false,
|
||||
onnotice: () => {},
|
||||
debug: input.onLockAcquisitionAttempt
|
||||
? (_connection, query) => {
|
||||
if (query.includes("pg_advisory_lock(")) input.onLockAcquisitionAttempt?.();
|
||||
}
|
||||
: undefined,
|
||||
});
|
||||
const key = `fusion:planning-lifecycle:${input.projectId}:${input.taskId}`;
|
||||
let acquired = false;
|
||||
try {
|
||||
@@ -91,12 +153,17 @@ export async function withPlanningLifecycleAdvisoryLock<T>(
|
||||
database before locking so an accidental migration endpoint cannot serialize
|
||||
one database while task writes target another.
|
||||
*/
|
||||
const identity = await client<{ database: string; host: string | null; port: number | null; cluster: string | null }[]>`
|
||||
SELECT current_database() AS database,
|
||||
inet_server_addr()::text AS host,
|
||||
inet_server_port() AS port,
|
||||
current_setting('cluster_name', true) AS cluster
|
||||
`;
|
||||
const identity = await runBoundedTransportPhase(
|
||||
client,
|
||||
timeoutMs,
|
||||
`Planning lifecycle lock session setup timed out after ${timeoutMs}ms`,
|
||||
"Planning lifecycle lock could not establish its dedicated session",
|
||||
async () => {
|
||||
const rows = await readPostgresBackendIdentity(client);
|
||||
await client`SELECT set_config('lock_timeout', ${`${timeoutMs}ms`}, false)`;
|
||||
return rows;
|
||||
},
|
||||
);
|
||||
if (identity[0]?.database !== directDatabase) {
|
||||
throw new PlanningLifecycleLockTransportError("Planning lifecycle lock session selected an unexpected database");
|
||||
}
|
||||
@@ -109,14 +176,20 @@ export async function withPlanningLifecycleAdvisoryLock<T>(
|
||||
cluster with the same database cannot serialize the wrong task lifecycle.
|
||||
*/
|
||||
if (operationalUrl && operationalUrl !== directUrl) {
|
||||
const operationalClient = postgres(operationalUrl, { max: 1, prepare: false, onnotice: () => {} });
|
||||
const operationalClient = postgres(operationalUrl, {
|
||||
max: 1,
|
||||
connect_timeout: Math.max(1, Math.ceil(timeoutMs / 1_000)),
|
||||
prepare: false,
|
||||
onnotice: () => {},
|
||||
});
|
||||
try {
|
||||
const operationalIdentity = await operationalClient<{ database: string; host: string | null; port: number | null; cluster: string | null }[]>`
|
||||
SELECT current_database() AS database,
|
||||
inet_server_addr()::text AS host,
|
||||
inet_server_port() AS port,
|
||||
current_setting('cluster_name', true) AS cluster
|
||||
`;
|
||||
const operationalIdentity = await runBoundedTransportPhase(
|
||||
operationalClient,
|
||||
timeoutMs,
|
||||
`Planning lifecycle lock backend identity check timed out after ${timeoutMs}ms`,
|
||||
"Planning lifecycle lock could not verify the resolved backend server identity",
|
||||
() => readPostgresBackendIdentity(operationalClient),
|
||||
);
|
||||
const expected = operationalIdentity[0];
|
||||
const actual = identity[0];
|
||||
if (!expected || !actual
|
||||
@@ -133,11 +206,28 @@ export async function withPlanningLifecycleAdvisoryLock<T>(
|
||||
await operationalClient.end({ timeout: 5 }).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
await client`SELECT pg_advisory_lock(hashtext(${key}))`;
|
||||
await runBoundedTransportPhase(
|
||||
client,
|
||||
timeoutMs,
|
||||
`Planning lifecycle lock acquisition timed out after ${timeoutMs}ms`,
|
||||
"Planning lifecycle lock acquisition failed",
|
||||
() => client`SELECT pg_advisory_lock(hashtext(${key}))`,
|
||||
);
|
||||
acquired = true;
|
||||
return await callback();
|
||||
} finally {
|
||||
if (acquired) await client`SELECT pg_advisory_unlock(hashtext(${key}))`.catch(() => undefined);
|
||||
await client.end({ timeout: 5 }).catch(() => undefined);
|
||||
try {
|
||||
if (acquired) {
|
||||
await runBoundedTransportPhase(
|
||||
client,
|
||||
timeoutMs,
|
||||
`Planning lifecycle lock cleanup timed out after ${timeoutMs}ms`,
|
||||
"Planning lifecycle lock cleanup failed",
|
||||
() => client`SELECT pg_advisory_unlock(hashtext(${key}))`,
|
||||
).catch(() => undefined);
|
||||
}
|
||||
} finally {
|
||||
await client.end({ timeout: 5 }).catch(() => undefined);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -156,7 +156,12 @@ export async function checkAndRecordUnplannedExecutionBlockImpl(
|
||||
const limit = getTaskActivityLogEntryLimit();
|
||||
if (log.length > limit) log.splice(0, log.length - limit);
|
||||
await tx.update(schema.project.tasks)
|
||||
.set({ log, updatedAt: entry.timestamp })
|
||||
/*
|
||||
* FNXC:PlanningHandoffRecovery 2026-08-04-06:35:
|
||||
* This diagnostic must not make an old planning handoff look fresh to
|
||||
* recovery grace windows. The marker timestamp records audit recency.
|
||||
*/
|
||||
.set({ log })
|
||||
.where(and(eq(schema.project.tasks.projectId, projectId), eq(schema.project.tasks.id, id)));
|
||||
return true;
|
||||
});
|
||||
|
||||
@@ -32,6 +32,8 @@ import {applyOriginalDescription} from "../tasks/original-description-policy.js"
|
||||
import {normalizeTaskReviewState} from "../task-store/review-state.js";
|
||||
import {hasOwnDeclaredSymbols, normalizeDeclaredSymbols, extractDeclaredSymbolsFromPrompt, resolveTaskSymbolsForTask} from "../tasks/task-symbol-resolution.js";
|
||||
import {assertValidProviderInstanceId} from "../provider-instance.js";
|
||||
import {supersedePlanReviewResults} from "../planner/plan-approval.js";
|
||||
import {PLAN_REVIEW_GROUP_ID} from "../workflows/builtin-plan-review-group.js";
|
||||
|
||||
export async function updateTaskUnlockedImpl(store: TaskStore, id: string, updates: Parameters<TaskStore["updateTask"]>[1], runContext?: RunMutationContext,): Promise<Task> {
|
||||
/* FNXC:CredentialInstanceSelection 2026-08-01-05:43: validate task authoring input before persistence; ids are stored but runtime credential resolution remains unchanged. */
|
||||
@@ -53,6 +55,9 @@ export async function updateTaskUnlockedImpl(store: TaskStore, id: string, updat
|
||||
const dir = store.taskDir(id);
|
||||
const task = await store.readTaskJson(dir);
|
||||
const wasFailed = task.status === "failed";
|
||||
const preUpdatePlanReviewResults = task.workflowStepResults?.filter(
|
||||
(result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID,
|
||||
);
|
||||
|
||||
// Capture title/description before mutation so the PROMPT.md stub
|
||||
// detector below can compare against the exact wrapper bytes that the
|
||||
@@ -162,11 +167,12 @@ export async function updateTaskUnlockedImpl(store: TaskStore, id: string, updat
|
||||
if (updates.workspaceWorktrees !== undefined) {
|
||||
task.workspaceWorktrees = updates.workspaceWorktrees;
|
||||
}
|
||||
// Detect new dependencies being added to a hold-lane task → re-seed for re-specification
|
||||
// New dependencies re-seed hold-lane tasks and exhausted Plan Review cap parks.
|
||||
let movedToTriage = false;
|
||||
let respecifyFromColumn: string | undefined;
|
||||
let respecifyMoveLanes: TaskMoveLanes | undefined;
|
||||
let previousDependencies: string[] | undefined;
|
||||
let planningInvalidatedAt: string | undefined;
|
||||
if (updates.dependencies !== undefined) {
|
||||
previousDependencies = (task.dependencies ?? []).map((dependency) => dependency.trim()).filter(Boolean);
|
||||
const oldDeps = new Set(previousDependencies);
|
||||
@@ -212,7 +218,11 @@ export async function updateTaskUnlockedImpl(store: TaskStore, id: string, updat
|
||||
/* DELIBERATE-LITERAL — the unresolvable-workflow default for the SOURCE lane only; the
|
||||
destination below never falls back to a literal. Reviewed 2026-07-31-02:40. */
|
||||
const holdLane = depLanes === undefined ? "todo" : depLanes.hold;
|
||||
if (hasNewDeps && holdLane !== undefined && task.column === holdLane) {
|
||||
const isPlanReviewCapPark = task.status === "awaiting-approval"
|
||||
&& task.awaitingApprovalReason === "plan-review-replan-cap";
|
||||
const shouldRespecify = hasNewDeps
|
||||
&& ((holdLane !== undefined && task.column === holdLane) || isPlanReviewCapPark);
|
||||
if (shouldRespecify) {
|
||||
const intakeLane = depLanes?.intake;
|
||||
respecifyFromColumn = task.column;
|
||||
const relocating = intakeLane !== undefined && intakeLane !== task.column;
|
||||
@@ -221,23 +231,16 @@ export async function updateTaskUnlockedImpl(store: TaskStore, id: string, updat
|
||||
task.columnMovedAt = new Date().toISOString();
|
||||
}
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-00:30:
|
||||
A new dependency invalidates a plan that is still in the hold lane. The
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-06:35:
|
||||
A new dependency invalidates a plan that is still in the hold lane or parked after
|
||||
exhausting Plan Review in a distinct review column. The
|
||||
prior null reset raced an in-flight planner after it wrote PROMPT.md but
|
||||
before its final handoff, leaving a real specification that neither
|
||||
planning discovery nor release could claim. `needs-replan` is the
|
||||
graph-owned durable re-entry signal: it preserves prompt authority and
|
||||
makes the interrupted planner's stale finalizer harmless.
|
||||
*/
|
||||
task.status = "needs-replan";
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-00:54:
|
||||
Both dependency mutation APIs invalidate the same pre-execution plan
|
||||
handoff. Clearing manual-approval evidence here prevents a newly added
|
||||
blocker from inheriting approval for the superseded specification.
|
||||
*/
|
||||
task.approvedPlanFingerprint = undefined;
|
||||
task.awaitingApprovalReason = undefined;
|
||||
planningInvalidatedAt = new Date().toISOString();
|
||||
const depLogEntry: TaskLogEntry = {
|
||||
timestamp: new Date().toISOString(),
|
||||
action: relocating
|
||||
@@ -834,6 +837,37 @@ export async function updateTaskUnlockedImpl(store: TaskStore, id: string, updat
|
||||
} else if (updates.workflowStepResults !== undefined) {
|
||||
task.workflowStepResults = updates.workflowStepResults;
|
||||
}
|
||||
if (planningInvalidatedAt !== undefined) {
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-06:35:
|
||||
Dependency invalidation is authoritative over every field in the same
|
||||
generic updateTask patch. Apply it after the ordinary status, approval,
|
||||
and workflow-result merge so a dashboard PATCH containing dependencies
|
||||
plus stale current-episode fields cannot undo the replan fence. The
|
||||
persistence transaction below also retires the pending continuation.
|
||||
|
||||
Preserve the pre-patch Plan Review projection when that same patch clears
|
||||
or replaces workflowStepResults without a Plan Review row. Dropping that
|
||||
audit projection lets the graph reconstruct the old pass from its durable
|
||||
completion log and incorrectly release the newly invalidated plan.
|
||||
*/
|
||||
task.status = "needs-replan";
|
||||
task.approvedPlanFingerprint = undefined;
|
||||
task.awaitingApprovalReason = undefined;
|
||||
const patchedResultsRetainPlanReview = task.workflowStepResults?.some(
|
||||
(result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID,
|
||||
) === true;
|
||||
const resultsWithPriorPlanReview = patchedResultsRetainPlanReview
|
||||
? (task.workflowStepResults ?? [])
|
||||
: [
|
||||
...(task.workflowStepResults ?? []),
|
||||
...(preUpdatePlanReviewResults ?? []),
|
||||
];
|
||||
task.workflowStepResults = supersedePlanReviewResults(
|
||||
resultsWithPriorPlanReview.length > 0 ? resultsWithPriorPlanReview : undefined,
|
||||
planningInvalidatedAt,
|
||||
);
|
||||
}
|
||||
if (updates.mergeDetails === null) {
|
||||
task.mergeDetails = undefined;
|
||||
} else if (updates.mergeDetails !== undefined) {
|
||||
|
||||
@@ -25,6 +25,7 @@ import {generateTaskLineageId} from "../tasks/task-lineage.js";
|
||||
import {deriveFallbackTaskTitle} from "../ai/ai-summarize.js";
|
||||
import {sanitizeFileScopeInPromptContent} from "../task-store/file-scope.js";
|
||||
import {__setTaskActivityLogLimitsForTesting} from "../task-store/comments.js";
|
||||
import {supersedePlanReviewResults} from "../planner/plan-approval.js";
|
||||
|
||||
export async function refineTaskImpl(store: TaskStore, id: string, feedback: string): Promise<Task> {
|
||||
const sourceTask = await store.getTask(id);
|
||||
@@ -440,9 +441,10 @@ async function updateTaskDependenciesWithTaskLockImpl(store: TaskStore, id: stri
|
||||
task.log ??= [];
|
||||
let movedToTriage = false;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-02-02:30 (fleet — GUARD AND DESTINATION together):
|
||||
A new dependency on a card still resting in the HOLD lane sends it back to INTAKE for
|
||||
re-specification. Both ends were literals, so this never fired on a renamed board — and converting
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-04-06:35 (FN-8768 — GUARD AND DESTINATION together):
|
||||
A new dependency on a card still resting in the HOLD lane, or parked after exhausting Plan
|
||||
Review in a distinct review column, sends it back to INTAKE for re-specification. Both ends
|
||||
were literals, so this never fired on a renamed board — and converting
|
||||
only the guard would have written an `intake` column the board may not declare directly into the row,
|
||||
which is worse than not firing: the store would hold a card in a column that does not exist.
|
||||
|
||||
@@ -453,18 +455,26 @@ async function updateTaskDependenciesWithTaskLockImpl(store: TaskStore, id: stri
|
||||
const holdColumn = respecifyLifecycle?.hold ?? "todo";
|
||||
const intakeColumn = respecifyLifecycle?.intake;
|
||||
const respecifyFromColumn = task.column;
|
||||
const isPlanReviewCapPark = task.status === "awaiting-approval"
|
||||
&& task.awaitingApprovalReason === "plan-review-replan-cap";
|
||||
const shouldRespecify = hasNewDependencies
|
||||
&& (task.column === holdColumn || isPlanReviewCapPark);
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-00:43:
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-06:35:
|
||||
A new dependency invalidates every pre-execution approval artifact even
|
||||
when merged intake/hold lanes make this a same-column transition. Leaving
|
||||
the old fingerprint would let an unchanged prompt bypass manual approval.
|
||||
*/
|
||||
if (hasNewDependencies && task.column === holdColumn) {
|
||||
if (shouldRespecify) {
|
||||
task.status = "needs-replan";
|
||||
task.approvedPlanFingerprint = undefined;
|
||||
task.awaitingApprovalReason = undefined;
|
||||
task.workflowStepResults = supersedePlanReviewResults(
|
||||
task.workflowStepResults,
|
||||
task.updatedAt,
|
||||
);
|
||||
}
|
||||
if (hasNewDependencies && task.column === holdColumn && intakeColumn !== undefined) {
|
||||
if (shouldRespecify && intakeColumn !== undefined) {
|
||||
task.column = intakeColumn;
|
||||
movedToTriage = true;
|
||||
/*
|
||||
@@ -551,4 +561,3 @@ async function updateTaskDependenciesWithTaskLockImpl(store: TaskStore, id: stri
|
||||
return task;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -313,6 +313,23 @@ export interface WorkflowStepResult {
|
||||
bypassedFromStatus?: WorkflowStepResult["status"];
|
||||
/** The `verdict` (if any) this result carried immediately before the bypass, preserved for audit only — never promoted to `verdict`. */
|
||||
bypassedFromVerdict?: WorkflowStepResult["verdict"];
|
||||
/*
|
||||
* FNXC:PlanReviewSupersession 2026-08-04-06:35:
|
||||
* Persist the boundary that invalidated this planning episode. Superseded
|
||||
* results remain auditable but cannot satisfy or repair the current gate and
|
||||
* a superseded pending result cannot be adopted as a live lease.
|
||||
*/
|
||||
supersededAt?: string;
|
||||
/** Machine-readable reason the result stopped being current. */
|
||||
supersededReason?: "dependency-change";
|
||||
/*
|
||||
* FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768):
|
||||
* Number of terminal REVISE results recorded in the current Plan Review
|
||||
* episode. Unlike `priorAttempts`, this scalar is not capped and is the
|
||||
* durable authority for revision-budget accounting and attempt numbering.
|
||||
* Reset when a superseded planning episode is replaced.
|
||||
*/
|
||||
planReviewAttemptCount?: number;
|
||||
/*
|
||||
* FNXC:WorkflowStepResults 2026-07-09-00:10:
|
||||
* FN-7727: self-healing recovery re-runs a failed pre-merge review node
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { WorkflowIrNode } from "./workflow-ir-types.js";
|
||||
import { CODE_REVIEW_COMPLETENESS_POLICY } from "../agents/code-review-policy.js";
|
||||
|
||||
/*
|
||||
FNXC:CodeReviewStep 2026-06-25-15:00:
|
||||
@@ -65,6 +66,8 @@ const CODE_REVIEW_PROMPT = `You are a senior code reviewer. Review the task's di
|
||||
5. **Error handling** — swallowed errors, unhandled rejections/exceptions, missing validation at trust boundaries, misleading error messages.
|
||||
6. **Contract / signature changes** — changed function/exported-type signatures, API request/response shapes, or serialization that breaks existing callers.
|
||||
|
||||
${CODE_REVIEW_COMPLETENESS_POLICY}
|
||||
|
||||
Be specific: cite \`file:line\` for every finding and explain the concrete failure it causes.
|
||||
|
||||
## Output Requirements
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { WorkflowIrNode } from "./workflow-ir-types.js";
|
||||
import { PLAN_REVIEW_COMPLETENESS_POLICY } from "../agents/planning-review-policy.js";
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewStep 2026-06-28-23:29:
|
||||
@@ -29,12 +30,14 @@ const PLAN_REVIEW_PROMPT = `You are a senior plan reviewer. Review the task's PR
|
||||
4. **Verification quality** — absent or weak tests/checks for the behavior being changed.
|
||||
5. **Risk callouts** — migrations, data-loss paths, external integrations, secrets, or plugin/runtime dependencies that need explicit handling.
|
||||
|
||||
${PLAN_REVIEW_COMPLETENESS_POLICY}
|
||||
|
||||
Be specific: cite the plan section or file path for every finding and explain the concrete correction.
|
||||
|
||||
## Output Requirements
|
||||
- APPROVE: the plan is ready for execution.
|
||||
- APPROVE_WITH_NOTES: execution may proceed, but include non-blocking advisory notes.
|
||||
- REVISE: the plan should be corrected before execution; include the missing or wrong requirement and the needed change.
|
||||
- REVISE: the plan should be corrected before execution; include every blocking finding and needed change in the JSON notes, not only in preceding prose.
|
||||
- Final output: output exactly one trailing JSON object on the final line (no markdown fences, no surrounding prose):
|
||||
{"verdict":"APPROVE|APPROVE_WITH_NOTES|REVISE","notes":"..."}`;
|
||||
|
||||
|
||||
@@ -377,7 +377,16 @@ export function applyReopenFieldClears(ctx: DefaultWorkflowMoveContext): void {
|
||||
const leftReviewForPlanningOrWip =
|
||||
fromColumn === reviewLane && (planning.includes(toColumn) || toColumn === wipLane);
|
||||
const leftCompleteForPlanning = fromColumn === completeLane && planning.includes(toColumn);
|
||||
if (!graphOwnedReviewToWip && (leftReviewForPlanningOrWip || leftCompleteForPlanning)) {
|
||||
/*
|
||||
FNXC:PlanReviewApproval 2026-08-04-05:35:
|
||||
A split-column manual approval first persists its audited Plan Review bypass,
|
||||
then rebounds while the awaiting-approval hold is deliberately preserved. The
|
||||
move must not erase that evidence before the final hold clear. This provenance
|
||||
is set only by the approve-plan route; ordinary operator reopens still clear
|
||||
prior review results exactly as before.
|
||||
*/
|
||||
const preservesApprovedPlanReview = ctx.workflowMoveSource === "plan-approval";
|
||||
if (!preservesApprovedPlanReview && !graphOwnedReviewToWip && (leftReviewForPlanningOrWip || leftCompleteForPlanning)) {
|
||||
task.workflowStepResults = undefined;
|
||||
}
|
||||
if (fromColumn === reviewLane && planning.includes(toColumn)) {
|
||||
|
||||
@@ -31,6 +31,10 @@ function isTerminalFailure(result: WorkflowStepResult): boolean {
|
||||
return TERMINAL_FAILURE_STATUSES.has(result.status);
|
||||
}
|
||||
|
||||
function isSupersededPlanningEvidence(result: WorkflowStepResult): boolean {
|
||||
return result.supersededAt != null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip a result down to a single-level history snapshot: its own
|
||||
* `priorAttempts` are dropped so nesting never grows beyond one level deep.
|
||||
@@ -51,8 +55,9 @@ function toSnapshot(result: WorkflowStepResult): WorkflowStepResult {
|
||||
* carried forward onto the incoming result. If the existing entry represents
|
||||
* a DIFFERENT attempt (deduped by `startedAt` — a same-run `pending`→`failed`
|
||||
* transition of the same attempt is not a new attempt) and its status is a
|
||||
* terminal failure (`failed` | `advisory_failure`), a single-level snapshot
|
||||
* of it is pushed onto the incoming result's `priorAttempts`.
|
||||
* terminal failure (`failed` | `advisory_failure`) or superseded planning
|
||||
* projection, a single-level snapshot of it is pushed onto the incoming
|
||||
* result's `priorAttempts`.
|
||||
* - `priorAttempts` is bounded to `opts.maxPriorAttempts` (default
|
||||
* `MAX_WORKFLOW_STEP_PRIOR_ATTEMPTS`), newest-first, oldest dropped.
|
||||
*
|
||||
@@ -79,7 +84,7 @@ export function upsertWorkflowStepResult(
|
||||
&& previous.startedAt === incoming.startedAt;
|
||||
|
||||
let priorAttempts = previous.priorAttempts ? [...previous.priorAttempts] : [];
|
||||
if (!isSameAttempt && isTerminalFailure(previous)) {
|
||||
if (!isSameAttempt && (isTerminalFailure(previous) || isSupersededPlanningEvidence(previous))) {
|
||||
priorAttempts = [toSnapshot(previous), ...priorAttempts];
|
||||
}
|
||||
if (priorAttempts.length > maxPriorAttempts) {
|
||||
|
||||
@@ -42,6 +42,19 @@ describe("browser layout smoke fixture", () => {
|
||||
expect(html).toContain("floating-window--github-import-detail");
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-08-04-06:35 FN-8768:
|
||||
Keep both production approval surfaces in the real-browser fixture; the executable smoke checks
|
||||
their shared responsive containment rather than treating the presence of markup as layout proof.
|
||||
*/
|
||||
it("includes the Plan Review replan-cap approval card and detail surfaces", () => {
|
||||
const html = createSmokeHtml();
|
||||
expect(html).toContain('data-smoke="plan-review-replan-cap-approval"');
|
||||
expect(html).toContain("awaiting-approval--plan-review-replan-cap");
|
||||
expect(html).toContain("detail-plan-approval-banner--replan-cap");
|
||||
expect(html).toContain("Plan Review needs approval");
|
||||
});
|
||||
|
||||
it("includes PR flow fixture sections and class hooks", () => {
|
||||
const html = createSmokeHtml();
|
||||
expect(html).toContain('data-smoke="pr-create-modal"');
|
||||
|
||||
@@ -52,7 +52,7 @@ import { ACTIVE_STATUSES, isTaskAgentActive } from "../utils/taskActivity";
|
||||
import { getPrBadgeModifierClass } from "../utils/prBadgeClass";
|
||||
import { getTotalAgentActiveMs, getEndToEndDurationMs, getTimedDurationMs, getWorkflowRuntimeMs, parseTimestampToMs } from "../utils/taskTiming";
|
||||
import { getTaskStatusBadgeLabel, type TaskStatusBadgeContext, hasTaskStatusBadge } from "../utils/taskStatusBadgeLabel";
|
||||
import { isReviewBudgetExhaustedApproval } from "../utils/reviewBudgetApproval";
|
||||
import { isReviewBudgetExhaustedApproval, isTaskAwaitingPlanApproval } from "../utils/reviewBudgetApproval";
|
||||
import { canStartPrFeedbackAddressing, getTaskPrimaryPrInfo } from "../utils/prFeedback";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
import { useConfirm } from "../hooks/useConfirm";
|
||||
@@ -1541,8 +1541,8 @@ function TaskCardComponent({
|
||||
know approval is required because Plan Review exhausted automatic REVISE replans without
|
||||
converging — Approve keeps the current PROMPT.md; Reject regenerates.
|
||||
*/
|
||||
const isAwaitingApproval = isIntakeColumn && task.status === "awaiting-approval";
|
||||
const isPlanReviewReplanCapApproval = isReviewBudgetExhaustedApproval(task);
|
||||
const isAwaitingApproval = isTaskAwaitingPlanApproval(task, isIntakeColumn);
|
||||
const isAwaitingInput = task.status === "awaiting-user-input";
|
||||
const isArchived = isArchivedColumn;
|
||||
/*
|
||||
|
||||
@@ -85,7 +85,7 @@ import { hasPendingAutomaticRecovery, isTaskManuallyRetryable } from "../utils/t
|
||||
import { findInReviewStallLogEntry, IN_REVIEW_STALL_LOG_REGEX } from "../utils/findInReviewStallLogEntry";
|
||||
import { getTaskLogEntryAction, getTaskLogEntryOutcome } from "../utils/taskLogEntryDisplay";
|
||||
import { getRelativeTimeBucket } from "../utils/relativeTimeAgo";
|
||||
import { isReviewBudgetExhaustedApproval } from "../utils/reviewBudgetApproval";
|
||||
import { isReviewBudgetExhaustedApproval, isTaskAwaitingPlanApproval } from "../utils/reviewBudgetApproval";
|
||||
import { ACTIVE_STATUSES, resolveEffectiveExecutor, resolveEffectivePlanning, resolveEffectiveValidator, type ModelSelection } from "./effective-model-resolution";
|
||||
import { TaskContextMenu, buildTaskActionMenuModel, getTaskPrAutomationLabel } from "./TaskContextMenu";
|
||||
import type { TaskContextMenuColumnFlags, TaskContextMenuColumnMetadata } from "./TaskContextMenu";
|
||||
@@ -3408,8 +3408,8 @@ export function TaskDetailContent({
|
||||
const isIntakeColumn = detailColumnFlags
|
||||
? detailColumnFlags.intake === true
|
||||
: task.column === "triage";
|
||||
const isAwaitingApproval = isIntakeColumn && task.status === "awaiting-approval";
|
||||
const isPlanReviewReplanCapApproval = isReviewBudgetExhaustedApproval(task);
|
||||
const isAwaitingApproval = isTaskAwaitingPlanApproval(task, isIntakeColumn);
|
||||
|
||||
const handleTogglePause = useCallback(async () => {
|
||||
try {
|
||||
|
||||
@@ -3070,11 +3070,11 @@ describe("TaskCard", () => {
|
||||
expect(container.querySelector(".awaiting-approval--plan-review-replan-cap")).toBeNull();
|
||||
});
|
||||
|
||||
it("renders a distinct review-budget-exhausted badge when awaitingApprovalReason is plan-review-replan-cap", () => {
|
||||
it("renders review-budget approval metadata outside the intake column", () => {
|
||||
const { container } = render(
|
||||
<TaskCard
|
||||
task={makeTask({
|
||||
column: "triage",
|
||||
column: "todo",
|
||||
status: "awaiting-approval",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
} as any)}
|
||||
|
||||
@@ -515,11 +515,11 @@ describe("TaskDetailModal", () => {
|
||||
* Replan-cap escalations must explain that Plan Review did not converge so the
|
||||
* operator knows why approval is required (not a generic require-all gate).
|
||||
*/
|
||||
it("explains Plan Review non-convergence when awaitingApprovalReason is plan-review-replan-cap", () => {
|
||||
it("offers both decisions in a split Plan Review column after the replan cap", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({
|
||||
column: "triage",
|
||||
column: "todo",
|
||||
status: "awaiting-approval",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
prompt: "# Task Spec",
|
||||
@@ -540,6 +540,8 @@ describe("TaskDetailModal", () => {
|
||||
expect(banner.contains(screen.getByTestId("detail-plan-approval-banner-actions"))).toBe(true);
|
||||
expect(banner.contains(screen.getByTestId("detail-plan-approval-banner-approve"))).toBe(true);
|
||||
expect(banner.contains(screen.getByTestId("detail-plan-approval-banner-reject"))).toBe(true);
|
||||
expect(screen.getByTestId("detail-plan-approval-footer-approve")).toBeTruthy();
|
||||
expect(screen.getByTestId("detail-plan-approval-footer-reject")).toBeTruthy();
|
||||
expect(screen.getByText("Approval needed: Plan Review did not converge")).toBeTruthy();
|
||||
expect(screen.getByText(/exhausted|without approving|stopped the replan loop/i)).toBeTruthy();
|
||||
});
|
||||
|
||||
@@ -9,3 +9,14 @@ import type { Task } from "../../../core/src/types";
|
||||
export function isReviewBudgetExhaustedApproval(task: Task): boolean {
|
||||
return task.status === "awaiting-approval" && task.awaitingApprovalReason === "plan-review-replan-cap";
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:PlanReviewReplan 2026-08-04-06:35 FN-8768:
|
||||
* Approval controls normally belong to intake, but an exhausted Plan Review remains in the
|
||||
* graph node's review column. Keep that persisted-reason exception shared by card and detail
|
||||
* surfaces so a split-column workflow never renders a hold without its operator controls.
|
||||
*/
|
||||
export function isTaskAwaitingPlanApproval(task: Task, isIntakeColumn: boolean): boolean {
|
||||
return task.status === "awaiting-approval"
|
||||
&& (isIntakeColumn || task.awaitingApprovalReason === "plan-review-replan-cap");
|
||||
}
|
||||
|
||||
@@ -26,6 +26,8 @@ const gitHubImportAfterMobileScreenshotPath = process.env.FUSION_GITHUB_IMPORT_A
|
||||
const gitHubImportAfterShortScreenshotPath = process.env.FUSION_GITHUB_IMPORT_AFTER_SHORT_SCREENSHOT;
|
||||
const resolvedGithubDesktopScreenshotPath = process.env.FUSION_RESOLVED_GITHUB_DESKTOP_SCREENSHOT;
|
||||
const resolvedGithubMobileScreenshotPath = process.env.FUSION_RESOLVED_GITHUB_MOBILE_SCREENSHOT;
|
||||
const planApprovalDesktopScreenshotPath = process.env.FUSION_PLAN_APPROVAL_DESKTOP_SCREENSHOT;
|
||||
const planApprovalMobileScreenshotPath = process.env.FUSION_PLAN_APPROVAL_MOBILE_SCREENSHOT;
|
||||
const smokeTheme = process.env.FUSION_BROWSER_SMOKE_THEME === "light" ? "light" : "dark";
|
||||
|
||||
function log(message) {
|
||||
@@ -243,6 +245,23 @@ export function createSmokeHtml() {
|
||||
</section>
|
||||
`;
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-08-04-06:35 FN-8768:
|
||||
Mirror both exhausted-review operator surfaces so Blink can prove the card badge and detail banner
|
||||
remain visible and contained at mobile and desktop widths, not merely that fixture HTML loaded.
|
||||
*/
|
||||
const planApprovalFixture = `
|
||||
<section data-smoke="plan-review-replan-cap-approval" aria-label="Plan Review approval escalation" style="width:min(680px, calc(100vw - var(--space-xl))); margin:auto; padding:var(--space-lg);">
|
||||
<article class="card" data-column="todo">
|
||||
<div class="card-header"><span class="card-id">FN-8768</span><h3 class="card-title">Dependency changed during planning</h3></div>
|
||||
<div class="card-meta"><span class="card-status-badge card-status-badge--todo awaiting-approval awaiting-approval--plan-review-replan-cap" data-awaiting-approval-reason="plan-review-replan-cap">Plan Review needs approval</span></div>
|
||||
</article>
|
||||
<div class="detail-plan-approval-banner detail-plan-approval-banner--replan-cap" data-awaiting-approval-reason="plan-review-replan-cap">
|
||||
<strong>Plan Review needs approval</strong>
|
||||
<span>Automatic revisions reached their limit. Review the latest plan, then approve it or send it back for replanning.</span>
|
||||
</div>
|
||||
</section>`;
|
||||
|
||||
const githubImportMobileActionFixture = `
|
||||
<section data-smoke="github-import-mobile-actions" aria-label="GitHub issue detail actions">
|
||||
<div class="github-import-detail-actions" data-testid="github-import-detail-actions">
|
||||
@@ -415,6 +434,7 @@ export function createSmokeHtml() {
|
||||
|
||||
${githubImportMobileActionFixture}
|
||||
${resolvedGithubTableFixture}
|
||||
${planApprovalFixture}
|
||||
|
||||
<footer class="executor-status-bar">
|
||||
<div class="executor-status-bar__segment">
|
||||
@@ -1152,6 +1172,44 @@ async function runSmokeChecks(page, pageUrl) {
|
||||
};
|
||||
})()`);
|
||||
|
||||
const collectPlanApprovalLayout = () => evaluate(page, `(() => {
|
||||
const fixture = document.querySelector('[data-smoke="plan-review-replan-cap-approval"]');
|
||||
const card = fixture.querySelector('.card');
|
||||
const badge = fixture.querySelector('.awaiting-approval--plan-review-replan-cap');
|
||||
const banner = fixture.querySelector('.detail-plan-approval-banner--replan-cap');
|
||||
const rect = (node) => {
|
||||
const box = node.getBoundingClientRect();
|
||||
return { left: box.left, right: box.right, top: box.top, bottom: box.bottom, width: box.width, height: box.height };
|
||||
};
|
||||
return {
|
||||
viewportWidth: window.innerWidth,
|
||||
documentOverflow: document.documentElement.scrollWidth - window.innerWidth,
|
||||
fixtureOverflow: fixture.scrollWidth - fixture.clientWidth,
|
||||
fixture: rect(fixture),
|
||||
card: rect(card),
|
||||
badge: rect(badge),
|
||||
banner: rect(banner),
|
||||
badgeReason: badge.getAttribute('data-awaiting-approval-reason'),
|
||||
bannerReason: banner.getAttribute('data-awaiting-approval-reason'),
|
||||
};
|
||||
})()`);
|
||||
|
||||
const planApprovalLayoutPasses = (layout) => layout.documentOverflow <= 1
|
||||
&& layout.fixtureOverflow <= 1
|
||||
&& layout.fixture.width > 0
|
||||
&& layout.card.height > 0
|
||||
&& layout.banner.height > 0
|
||||
&& layout.badge.width > 0
|
||||
&& layout.card.left >= layout.fixture.left - 1
|
||||
&& layout.card.right <= layout.fixture.right + 1
|
||||
&& layout.banner.left >= layout.fixture.left - 1
|
||||
&& layout.banner.right <= layout.fixture.right + 1
|
||||
&& layout.badge.left >= layout.card.left - 1
|
||||
&& layout.badge.right <= layout.card.right + 1
|
||||
&& layout.banner.top >= layout.card.bottom - 1
|
||||
&& layout.badgeReason === "plan-review-replan-cap"
|
||||
&& layout.bannerReason === "plan-review-replan-cap";
|
||||
|
||||
const mobileResolvedGithubTableLayout = await collectResolvedGithubTableLayout();
|
||||
assertSmokeResult(
|
||||
"resolved GitHub table wraps long content without mobile page overflow",
|
||||
@@ -1168,6 +1226,16 @@ async function runSmokeChecks(page, pageUrl) {
|
||||
await captureFixtureScreenshot(page, '[data-smoke="github-resolved-table"]', resolvedGithubMobileScreenshotPath);
|
||||
log(`saved resolved GitHub mobile screenshot to ${resolvedGithubMobileScreenshotPath}`);
|
||||
}
|
||||
if (planApprovalMobileScreenshotPath) {
|
||||
await captureFixtureScreenshot(page, '[data-smoke="plan-review-replan-cap-approval"]', planApprovalMobileScreenshotPath);
|
||||
log(`saved Plan Review approval mobile screenshot to ${planApprovalMobileScreenshotPath}`);
|
||||
}
|
||||
const mobilePlanApprovalLayout = await collectPlanApprovalLayout();
|
||||
assertSmokeResult(
|
||||
"Plan Review approval card and detail banner stay visible and contained on mobile",
|
||||
planApprovalLayoutPasses(mobilePlanApprovalLayout),
|
||||
JSON.stringify(mobilePlanApprovalLayout),
|
||||
);
|
||||
|
||||
const mobileAgentHeartbeatLayout = await collectAgentHeartbeatControlLayout();
|
||||
assertSmokeResult(
|
||||
@@ -1805,6 +1873,16 @@ async function runSmokeChecks(page, pageUrl) {
|
||||
await captureFixtureScreenshot(page, '[data-smoke="github-resolved-table"]', resolvedGithubDesktopScreenshotPath);
|
||||
log(`saved resolved GitHub desktop screenshot to ${resolvedGithubDesktopScreenshotPath}`);
|
||||
}
|
||||
if (planApprovalDesktopScreenshotPath) {
|
||||
await captureFixtureScreenshot(page, '[data-smoke="plan-review-replan-cap-approval"]', planApprovalDesktopScreenshotPath);
|
||||
log(`saved Plan Review approval desktop screenshot to ${planApprovalDesktopScreenshotPath}`);
|
||||
}
|
||||
const desktopPlanApprovalLayout = await collectPlanApprovalLayout();
|
||||
assertSmokeResult(
|
||||
"Plan Review approval card and detail banner stay visible and contained on desktop",
|
||||
planApprovalLayoutPasses(desktopPlanApprovalLayout),
|
||||
JSON.stringify(desktopPlanApprovalLayout),
|
||||
);
|
||||
|
||||
const desktopAgentHeartbeatLayout = await collectAgentHeartbeatControlLayout();
|
||||
assertSmokeResult(
|
||||
|
||||
@@ -61,6 +61,7 @@ function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||
getRootDir: vi.fn().mockReturnValue(mkdtempSync(join(tmpdir(), "kb-plan-approval-"))),
|
||||
getTask: vi.fn().mockResolvedValue(PLANNING_TASK),
|
||||
updateTask: vi.fn().mockResolvedValue(PLANNING_TASK),
|
||||
withPlanningLifecycleLock: vi.fn(async (_id, fn) => await fn()),
|
||||
moveTask: vi.fn().mockResolvedValue(PLANNING_TASK),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
// Resolve the merged workflow so the routes see its real intake column.
|
||||
|
||||
@@ -2,7 +2,7 @@ import { mkdir, rm, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import express from "express";
|
||||
import { afterEach, beforeEach, expect, it } from "vitest";
|
||||
import { computePlanApprovalFingerprint, isTaskBlockedOnApproval, type TaskStore } from "@fusion/core";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR, computePlanApprovalFingerprint, isTaskBlockedOnApproval, TaskStore } from "@fusion/core";
|
||||
import {
|
||||
createTaskStoreForTest,
|
||||
pgDescribe,
|
||||
@@ -24,13 +24,19 @@ pgDescribe("plan approval status persistence", () => {
|
||||
await harness.teardown();
|
||||
});
|
||||
|
||||
function createApp() {
|
||||
function createApp(appStore = store) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
app.use("/api", createApiRoutes(appStore));
|
||||
return app;
|
||||
}
|
||||
|
||||
function barrier() {
|
||||
let release!: () => void;
|
||||
const promise = new Promise<void>((resolve) => { release = resolve; });
|
||||
return { promise, release };
|
||||
}
|
||||
|
||||
it("clears the approval hold and persists the approved plan fingerprint", async () => {
|
||||
const task = await store.createTask({ description: "Approve this plan" });
|
||||
await store.updateTask(task.id, {
|
||||
@@ -130,6 +136,226 @@ pgDescribe("plan approval status persistence", () => {
|
||||
}));
|
||||
});
|
||||
|
||||
it("keeps a split-column approval blocked and retryable when the final hold clear fails", async () => {
|
||||
const task = await store.createTask({ description: "Retry interrupted split-column approval" });
|
||||
const splitWorkflow = await store.createWorkflowDefinition({
|
||||
name: "Split Plan Review approval",
|
||||
ir: {
|
||||
...BUILTIN_CODING_WORKFLOW_IR,
|
||||
id: "split-plan-review-approval",
|
||||
nodes: BUILTIN_CODING_WORKFLOW_IR.nodes.map((node) =>
|
||||
node.id === "plan-review" ? { ...node, column: "in-review" } : node
|
||||
),
|
||||
},
|
||||
});
|
||||
await store.selectTaskWorkflow(task.id, splitWorkflow.id);
|
||||
await store.moveTask(task.id, "in-review", {
|
||||
moveSource: "engine",
|
||||
recoveryRehome: true,
|
||||
bypassGuards: true,
|
||||
});
|
||||
await store.updateTask(task.id, {
|
||||
status: "awaiting-approval",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
}],
|
||||
} as never);
|
||||
|
||||
const originalUpdate = store.updateTask.bind(store);
|
||||
let approvalUpdates = 0;
|
||||
store.updateTask = (async (id, updates, runContext) => {
|
||||
if (id === task.id && updates.status !== undefined) {
|
||||
approvalUpdates += 1;
|
||||
if (approvalUpdates === 2) throw new Error("injected final approval write failure");
|
||||
}
|
||||
return originalUpdate(id, updates, runContext);
|
||||
}) as typeof store.updateTask;
|
||||
|
||||
try {
|
||||
const interrupted = await request(createApp(), "POST", `/api/tasks/${task.id}/approve-plan`);
|
||||
expect(interrupted.status).toBe(500);
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.column).toBe("todo");
|
||||
expect(persisted.status).toBe("awaiting-approval");
|
||||
expect(persisted.workflowStepResults).toContainEqual(expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
status: "skipped",
|
||||
bypassedBy: "dashboard-operator",
|
||||
}));
|
||||
} finally {
|
||||
store.updateTask = originalUpdate;
|
||||
}
|
||||
|
||||
const retried = await request(createApp(), "POST", `/api/tasks/${task.id}/approve-plan`);
|
||||
expect(retried.status).toBe(200);
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.status).toBeUndefined();
|
||||
expect(persisted.workflowStepResults).toContainEqual(expect.objectContaining({
|
||||
workflowStepId: "plan-review",
|
||||
status: "skipped",
|
||||
bypassedBy: "dashboard-operator",
|
||||
}));
|
||||
});
|
||||
|
||||
it("rejects an exhausted Plan Review from a split workflow's review column", async () => {
|
||||
const task = await store.createTask({ description: "Reject split-column review" });
|
||||
const splitWorkflow = await store.createWorkflowDefinition({
|
||||
name: "Split Plan Review rejection",
|
||||
ir: {
|
||||
...BUILTIN_CODING_WORKFLOW_IR,
|
||||
id: "split-plan-review-rejection",
|
||||
nodes: BUILTIN_CODING_WORKFLOW_IR.nodes.map((node) =>
|
||||
node.id === "plan-review" ? { ...node, column: "in-review" } : node
|
||||
),
|
||||
},
|
||||
});
|
||||
await store.selectTaskWorkflow(task.id, splitWorkflow.id);
|
||||
const intakeColumn = BUILTIN_CODING_WORKFLOW_IR.columns.find((column) =>
|
||||
column.traits.some((trait) => trait.trait === "intake")
|
||||
)!.id;
|
||||
await store.moveTask(task.id, "in-review", {
|
||||
moveSource: "engine",
|
||||
recoveryRehome: true,
|
||||
bypassGuards: true,
|
||||
});
|
||||
await store.updateTask(task.id, {
|
||||
status: "awaiting-approval",
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
});
|
||||
|
||||
const originalUpdate = store.updateTask.bind(store);
|
||||
let interruptFinalClear = true;
|
||||
store.updateTask = (async (id, updates, runContext) => {
|
||||
if (interruptFinalClear && updates.status === null && updates.approvedPlanFingerprint === null) {
|
||||
interruptFinalClear = false;
|
||||
throw new Error("simulated final reject clear interruption");
|
||||
}
|
||||
return originalUpdate(id, updates, runContext);
|
||||
}) as typeof store.updateTask;
|
||||
|
||||
try {
|
||||
const interrupted = await request(createApp(), "POST", `/api/tasks/${task.id}/reject-plan`);
|
||||
expect(interrupted.status).toBe(500);
|
||||
const partiallyRejected = await store.getTask(task.id);
|
||||
expect(partiallyRejected.column).toBe(intakeColumn);
|
||||
expect(partiallyRejected.status).toBe("awaiting-approval");
|
||||
expect(partiallyRejected.awaitingApprovalReason).toBe("plan-review-replan-cap");
|
||||
} finally {
|
||||
store.updateTask = originalUpdate;
|
||||
}
|
||||
|
||||
const response = await request(createApp(), "POST", `/api/tasks/${task.id}/reject-plan`);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.column).toBe(intakeColumn);
|
||||
expect(persisted.status).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each(["approve-plan", "reject-plan"] as const)(
|
||||
"does not let stale %s overwrite dependency-first invalidation",
|
||||
async (endpoint) => {
|
||||
const task = await store.createTask({ description: "Dependency wins approval race" });
|
||||
const dependency = await store.createTask({ description: "New prerequisite", column: "done" });
|
||||
await store.updateTask(task.id, { status: "awaiting-approval" });
|
||||
|
||||
const mutationStore = new TaskStore(harness.rootDir, undefined, { asyncLayer: harness.layer });
|
||||
await mutationStore.init();
|
||||
const mutationEntered = barrier();
|
||||
const allowMutation = barrier();
|
||||
const originalMutationLock = mutationStore.withPlanningLifecycleLock.bind(mutationStore);
|
||||
mutationStore.withPlanningLifecycleLock = async <T>(id: string, fn: () => Promise<T>): Promise<T> =>
|
||||
originalMutationLock(id, async () => {
|
||||
mutationEntered.release();
|
||||
await allowMutation.promise;
|
||||
return await fn();
|
||||
});
|
||||
|
||||
const approvalAttempted = barrier();
|
||||
const originalApprovalLock = store.withPlanningLifecycleLock.bind(store);
|
||||
store.withPlanningLifecycleLock = async <T>(id: string, fn: () => Promise<T>): Promise<T> => {
|
||||
approvalAttempted.release();
|
||||
return await originalApprovalLock(id, fn);
|
||||
};
|
||||
|
||||
try {
|
||||
const mutation = mutationStore.updateTaskDependencies(task.id, {
|
||||
operation: "add",
|
||||
dependency: dependency.id,
|
||||
});
|
||||
await mutationEntered.promise;
|
||||
const approval = request(createApp(), "POST", `/api/tasks/${task.id}/${endpoint}`);
|
||||
await approvalAttempted.promise;
|
||||
allowMutation.release();
|
||||
|
||||
await mutation;
|
||||
const response = await approval;
|
||||
expect(response.status).toBe(400);
|
||||
expect(response.body.error).toContain("awaiting-approval");
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.status).toBe("needs-replan");
|
||||
expect(persisted.dependencies).toContain(dependency.id);
|
||||
expect(persisted.approvedPlanFingerprint).toBeUndefined();
|
||||
} finally {
|
||||
allowMutation.release();
|
||||
mutationStore.withPlanningLifecycleLock = originalMutationLock;
|
||||
store.withPlanningLifecycleLock = originalApprovalLock;
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("lets a later dependency invalidation supersede approval-first state", async () => {
|
||||
const task = await store.createTask({ description: "Approval precedes dependency" });
|
||||
const dependency = await store.createTask({ description: "Later prerequisite", column: "done" });
|
||||
await store.updateTask(task.id, { status: "awaiting-approval" });
|
||||
|
||||
const mutationStore = new TaskStore(harness.rootDir, undefined, { asyncLayer: harness.layer });
|
||||
await mutationStore.init();
|
||||
const approvalEntered = barrier();
|
||||
const allowApproval = barrier();
|
||||
const originalApprovalLock = store.withPlanningLifecycleLock.bind(store);
|
||||
store.withPlanningLifecycleLock = async <T>(id: string, fn: () => Promise<T>): Promise<T> =>
|
||||
originalApprovalLock(id, async () => {
|
||||
approvalEntered.release();
|
||||
await allowApproval.promise;
|
||||
return await fn();
|
||||
});
|
||||
|
||||
const mutationAttempted = barrier();
|
||||
const originalMutationLock = mutationStore.withPlanningLifecycleLock.bind(mutationStore);
|
||||
mutationStore.withPlanningLifecycleLock = async <T>(id: string, fn: () => Promise<T>): Promise<T> => {
|
||||
mutationAttempted.release();
|
||||
return await originalMutationLock(id, fn);
|
||||
};
|
||||
|
||||
try {
|
||||
const approval = request(createApp(), "POST", `/api/tasks/${task.id}/approve-plan`);
|
||||
await approvalEntered.promise;
|
||||
const mutation = mutationStore.updateTaskDependencies(task.id, {
|
||||
operation: "add",
|
||||
dependency: dependency.id,
|
||||
});
|
||||
await mutationAttempted.promise;
|
||||
allowApproval.release();
|
||||
|
||||
const response = await approval;
|
||||
expect(response.status).toBe(200);
|
||||
await mutation;
|
||||
const persisted = await store.getTask(task.id);
|
||||
expect(persisted.status).toBe("needs-replan");
|
||||
expect(persisted.dependencies).toContain(dependency.id);
|
||||
expect(persisted.approvedPlanFingerprint).toBeUndefined();
|
||||
} finally {
|
||||
allowApproval.release();
|
||||
store.withPlanningLifecycleLock = originalApprovalLock;
|
||||
mutationStore.withPlanningLifecycleLock = originalMutationLock;
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps the approval hold when cap metadata has no failed REVISE result", async () => {
|
||||
const task = await store.createTask({ description: "Malformed exhausted review state" });
|
||||
await store.updateTask(task.id, {
|
||||
|
||||
@@ -212,6 +212,7 @@ function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||
createTask: vi.fn(),
|
||||
moveTask: vi.fn(),
|
||||
updateTask: vi.fn(),
|
||||
withPlanningLifecycleLock: vi.fn(async (_id, fn) => await fn()),
|
||||
deleteTask: vi.fn(),
|
||||
mergeTask: vi.fn(),
|
||||
archiveTask: vi.fn(),
|
||||
|
||||
@@ -191,6 +191,28 @@ async function resolveIntakeColumnForTask(store: TaskStore, taskId: string): Pro
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:PlanReviewReplan 2026-08-04-06:35 FN-8768:
|
||||
* Plan approval normally belongs in workflow intake. An exhausted Plan Review stays where the
|
||||
* review node ran, and both operator decisions must be accepted there. Keep approve/reject on one
|
||||
* resolver so the UI cannot offer a choice that only one endpoint understands.
|
||||
*/
|
||||
async function resolvePlanApprovalColumnsForTask(
|
||||
store: TaskStore,
|
||||
task: Task,
|
||||
): Promise<{ approvalColumn: string; intakeColumn: string }> {
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store, task.id);
|
||||
const intakeColumn = columnsWithFlag(ir, "intake")[0] ?? "triage";
|
||||
const approvalColumn = task.awaitingApprovalReason === "plan-review-replan-cap"
|
||||
? ir.nodes.find((node) => node.id === PLAN_REVIEW_GROUP_ID)?.column ?? intakeColumn
|
||||
: intakeColumn;
|
||||
return { approvalColumn, intakeColumn };
|
||||
} catch {
|
||||
return { approvalColumn: "triage", intakeColumn: "triage" };
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-27-16:15 (U10 / R8):
|
||||
Lifecycle POSITION — "is this move backward?" — resolved through `COLUMNS.indexOf(...)`, the
|
||||
@@ -4005,133 +4027,162 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
router.post("/tasks/:id/approve-plan", async (req, res) => {
|
||||
try {
|
||||
const { store: scopedStore } = await getProjectContext(req);
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
const updated = await scopedStore.withPlanningLifecycleLock(req.params.id, async () => {
|
||||
/*
|
||||
* FNXC:PlanningDependencyReseed 2026-08-04-06:35 FN-8768:
|
||||
* The task read belongs inside the same lifecycle lock used by dependency mutation.
|
||||
* Otherwise approval can validate an old hold, wait for dependency re-seed to publish
|
||||
* needs-replan, and then erase that newer state.
|
||||
*/
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — P0, post-#2515):
|
||||
Resolve the workflow's INTAKE column; do not name `triage`. #2515 removed `triage`
|
||||
from the default lineage — the single pre-implementation column is now id `todo`
|
||||
displayed as "Planning" — so comparing the card's column against the legacy
|
||||
`triage` id became TRUE for every
|
||||
default-workflow card and this route rejected all of them. A card parked
|
||||
`awaiting-approval` could not be approved OR rejected (same guard below), i.e. it
|
||||
was STUCK with no operator action able to release it. The guard did not stop
|
||||
firing; it started firing on everything.
|
||||
*/
|
||||
const approveIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id);
|
||||
let approveColumn = approveIntakeColumn;
|
||||
/*
|
||||
FNXC:PlanReviewApproval 2026-08-04-00:26:
|
||||
An exhausted Plan Review is parked in the review node's column. That column is not always
|
||||
the workflow intake column (`builtin:legacy-coding` uses todo vs triage), so the operator's
|
||||
terminal approval must be accepted where the failed review actually ran.
|
||||
*/
|
||||
if (task.awaitingApprovalReason === "plan-review-replan-cap") {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — P0, post-#2515):
|
||||
Resolve the workflow's INTAKE column; do not name `triage`. #2515 removed `triage`
|
||||
from the default lineage — the single pre-implementation column is now id `todo`
|
||||
displayed as "Planning" — so comparing the card's column against the legacy
|
||||
`triage` id became TRUE for every
|
||||
default-workflow card and this route rejected all of them. A card parked
|
||||
`awaiting-approval` could not be approved OR rejected (same guard below), i.e. it
|
||||
was STUCK with no operator action able to release it. The guard did not stop
|
||||
firing; it started firing on everything.
|
||||
*/
|
||||
const { approvalColumn: approveColumn } = await resolvePlanApprovalColumnsForTask(scopedStore, task);
|
||||
/*
|
||||
The resolved column ONLY — the legacy-`triage` disjunct this comment
|
||||
used to justify is gone (PR #2614 review — greptile: the comment outlived the code).
|
||||
It was a belt-and-braces widening added with the P0 fix, on the theory that a card
|
||||
might still be sitting in `triage`. Nothing shipped declares that column since
|
||||
#2515, so the disjunct only widened what the guard accepts, and re-adding it changed
|
||||
no test in either direction. A guard that accepts a column no workflow declares is
|
||||
not caution, it is an unreachable branch that reads like a requirement.
|
||||
*/
|
||||
if (task.status !== "awaiting-approval") {
|
||||
throw badRequest("Task must have status 'awaiting-approval' to approve plan");
|
||||
}
|
||||
const reboundColumn = await resolveReboundColumnForTask(scopedStore, task.id);
|
||||
/*
|
||||
* FNXC:PlanReviewReplan 2026-08-04-06:35 FN-8768:
|
||||
* A split-column approval keeps the hold set while moving to rebound. Accept that lane on
|
||||
* retry so a failure after the move cannot strand a safely blocked partial decision.
|
||||
*/
|
||||
if (task.column !== approveColumn && task.column !== reboundColumn) {
|
||||
throw badRequest(`Task must be in the '${approveColumn}' column to approve plan`);
|
||||
}
|
||||
// FNXC:ReleaseAuthorizationGate 2026-07-09-00:00:
|
||||
// The triage release-authorization gate was removed (it over-fired and stranded
|
||||
// ordinary tasks). The approve-plan guard that refused any task carrying the legacy
|
||||
// awaitingApprovalReason === "release-authorization" is gone too, so tasks parked by
|
||||
// the old gate can now be approved normally instead of staying stuck with no exit.
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — persist a fingerprint of the exact PROMPT.md the operator just approved
|
||||
* so a later re-specification (replan, plan-review retry, self-healing rebound) that
|
||||
* produces the identical plan can skip re-parking at awaiting-approval. Read the
|
||||
* on-disk PROMPT.md directly (best-effort) since the task row does not always carry
|
||||
* full prompt text; a missing/unreadable file leaves the fingerprint unset and the
|
||||
* manual gate falls back to today's always-re-park behavior for this task.
|
||||
*/
|
||||
let approvedPlanFingerprint: string | undefined;
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(scopedStore, task.id);
|
||||
approveColumn = ir.nodes.find((node) => node.id === PLAN_REVIEW_GROUP_ID)?.column
|
||||
?? approveIntakeColumn;
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const { join } = await import("node:path");
|
||||
const promptPath = join(scopedStore.getRootDir(), ".fusion", "tasks", task.id, "PROMPT.md");
|
||||
const promptText = await readFile(promptPath, "utf8");
|
||||
approvedPlanFingerprint = computePlanApprovalFingerprint(promptText);
|
||||
} catch {
|
||||
// Preserve the existing intake fallback when workflow resolution is unavailable.
|
||||
// No PROMPT.md to fingerprint (unusual for an awaiting-approval task) — leave unset.
|
||||
}
|
||||
}
|
||||
/*
|
||||
The resolved column ONLY — the legacy-`triage` disjunct this comment
|
||||
used to justify is gone (PR #2614 review — greptile: the comment outlived the code).
|
||||
It was a belt-and-braces widening added with the P0 fix, on the theory that a card
|
||||
might still be sitting in `triage`. Nothing shipped declares that column since
|
||||
#2515, so the disjunct only widened what the guard accepts, and re-adding it changed
|
||||
no test in either direction. A guard that accepts a column no workflow declares is
|
||||
not caution, it is an unreachable branch that reads like a requirement.
|
||||
*/
|
||||
if (task.column !== approveColumn) {
|
||||
throw badRequest(`Task must be in the '${approveColumn}' column to approve plan`);
|
||||
}
|
||||
if (task.status !== "awaiting-approval") {
|
||||
throw badRequest("Task must have status 'awaiting-approval' to approve plan");
|
||||
}
|
||||
// FNXC:ReleaseAuthorizationGate 2026-07-09-00:00:
|
||||
// The triage release-authorization gate was removed (it over-fired and stranded
|
||||
// ordinary tasks). The approve-plan guard that refused any task carrying the legacy
|
||||
// awaitingApprovalReason === "release-authorization" is gone too, so tasks parked by
|
||||
// the old gate can now be approved normally instead of staying stuck with no exit.
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — persist a fingerprint of the exact PROMPT.md the operator just approved
|
||||
* so a later re-specification (replan, plan-review retry, self-healing rebound) that
|
||||
* produces the identical plan can skip re-parking at awaiting-approval. Read the
|
||||
* on-disk PROMPT.md directly (best-effort) since the task row does not always carry
|
||||
* full prompt text; a missing/unreadable file leaves the fingerprint unset and the
|
||||
* manual gate falls back to today's always-re-park behavior for this task.
|
||||
*/
|
||||
let approvedPlanFingerprint: string | undefined;
|
||||
try {
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const { join } = await import("node:path");
|
||||
const promptPath = join(scopedStore.getRootDir(), ".fusion", "tasks", task.id, "PROMPT.md");
|
||||
const promptText = await readFile(promptPath, "utf8");
|
||||
approvedPlanFingerprint = computePlanApprovalFingerprint(promptText);
|
||||
} catch {
|
||||
// No PROMPT.md to fingerprint (unusual for an awaiting-approval task) — leave unset.
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewApproval 2026-08-04-00:26:
|
||||
Manual approval after the revision cap is durable evidence that the final REVISE was
|
||||
accepted. Persist the audited bypass with the hold clear so no consumer can observe only
|
||||
half of the operator decision and enqueue another Plan Review.
|
||||
*/
|
||||
let approvedWorkflowStepResults: Task["workflowStepResults"] | undefined;
|
||||
if (task.awaitingApprovalReason === "plan-review-replan-cap") {
|
||||
const results = [...(task.workflowStepResults ?? [])];
|
||||
let reviewIndex = -1;
|
||||
for (let index = results.length - 1; index >= 0; index -= 1) {
|
||||
const result = results[index];
|
||||
if (
|
||||
result.workflowStepId === PLAN_REVIEW_GROUP_ID
|
||||
&& (result.status === "failed" || result.status === "advisory_failure")
|
||||
&& result.verdict === "REVISE"
|
||||
) {
|
||||
reviewIndex = index;
|
||||
break;
|
||||
/*
|
||||
FNXC:PlanReviewApproval 2026-08-04-00:26:
|
||||
Manual approval after the revision cap is durable evidence that the final REVISE was
|
||||
accepted. Persist the audited bypass with the hold clear so no consumer can observe only
|
||||
half of the operator decision and enqueue another Plan Review.
|
||||
*/
|
||||
let approvedWorkflowStepResults: Task["workflowStepResults"] | undefined;
|
||||
if (task.awaitingApprovalReason === "plan-review-replan-cap") {
|
||||
const results = [...(task.workflowStepResults ?? [])];
|
||||
let reviewIndex = -1;
|
||||
for (let index = results.length - 1; index >= 0; index -= 1) {
|
||||
const result = results[index];
|
||||
if (
|
||||
result.workflowStepId === PLAN_REVIEW_GROUP_ID
|
||||
&& (result.status === "failed" || result.status === "advisory_failure")
|
||||
&& result.verdict === "REVISE"
|
||||
) {
|
||||
reviewIndex = index;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (reviewIndex === -1) {
|
||||
throw conflict("Cannot approve exhausted Plan Review: no failed REVISE result is available to override");
|
||||
if (reviewIndex === -1) {
|
||||
const alreadyBypassed = results.some((result) =>
|
||||
result.workflowStepId === PLAN_REVIEW_GROUP_ID
|
||||
&& result.status === "skipped"
|
||||
&& result.bypassedBy === "dashboard-operator"
|
||||
&& result.bypassedFromVerdict === "REVISE"
|
||||
);
|
||||
if (!alreadyBypassed) {
|
||||
throw conflict("Cannot approve exhausted Plan Review: no failed REVISE result is available to override");
|
||||
}
|
||||
} else {
|
||||
const prior = results[reviewIndex];
|
||||
const bypassed = {
|
||||
...prior,
|
||||
status: "skipped" as const,
|
||||
bypassedBy: "dashboard-operator",
|
||||
bypassedAt: new Date().toISOString(),
|
||||
bypassReason: "Approved after Plan Review did not converge",
|
||||
bypassedFromStatus: prior.status,
|
||||
bypassedFromVerdict: prior.verdict,
|
||||
};
|
||||
delete bypassed.verdict;
|
||||
results[reviewIndex] = bypassed;
|
||||
}
|
||||
approvedWorkflowStepResults = results;
|
||||
}
|
||||
|
||||
const prior = results[reviewIndex];
|
||||
const bypassed = {
|
||||
...prior,
|
||||
status: "skipped" as const,
|
||||
bypassedBy: "dashboard-operator",
|
||||
bypassedAt: new Date().toISOString(),
|
||||
bypassReason: "Approved after Plan Review did not converge",
|
||||
bypassedFromStatus: prior.status,
|
||||
bypassedFromVerdict: prior.verdict,
|
||||
};
|
||||
delete bypassed.verdict;
|
||||
results[reviewIndex] = bypassed;
|
||||
approvedWorkflowStepResults = results;
|
||||
}
|
||||
const approvalPatch = {
|
||||
status: null,
|
||||
approvedPlanFingerprint: approvedPlanFingerprint ?? null,
|
||||
...(approvedWorkflowStepResults ? { workflowStepResults: approvedWorkflowStepResults } : {}),
|
||||
} satisfies Parameters<TaskStore["updateTask"]>[1];
|
||||
|
||||
await scopedStore.logEntry(task.id, "Plan approved by user");
|
||||
|
||||
// Move to todo and clear status
|
||||
const reboundColumn = await resolveReboundColumnForTask(scopedStore, task.id);
|
||||
await scopedStore.moveTask(task.id, reboundColumn);
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-08-03-18:53:
|
||||
* Approval must clear the durable awaiting-approval hold with TaskStore's explicit
|
||||
* null sentinel; undefined omits a field from the patch. Persist the current plan's
|
||||
* fingerprint, or explicitly clear a prior fingerprint when PROMPT.md was unreadable,
|
||||
* so a stale plan can never bypass a later manual approval gate.
|
||||
*/
|
||||
const updated = await scopedStore.updateTask(task.id, {
|
||||
status: null,
|
||||
approvedPlanFingerprint: approvedPlanFingerprint ?? null,
|
||||
...(approvedWorkflowStepResults ? { workflowStepResults: approvedWorkflowStepResults } : {}),
|
||||
if (task.column !== reboundColumn) {
|
||||
/*
|
||||
* FNXC:PlanReviewReplan 2026-08-04-06:35 FN-8768:
|
||||
* Persist the decision evidence while the approval hold remains set,
|
||||
* then preserve both across the rebound. Every interruption point is
|
||||
* therefore non-schedulable and retryable; the final update below is
|
||||
* the only operation that releases the hold.
|
||||
*/
|
||||
await scopedStore.updateTask(task.id, {
|
||||
...approvalPatch,
|
||||
status: "awaiting-approval",
|
||||
});
|
||||
await scopedStore.moveTask(task.id, reboundColumn, {
|
||||
preserveStatus: true,
|
||||
workflowMoveSource: "plan-approval",
|
||||
});
|
||||
} else {
|
||||
// Preserve the historical same-column move behavior and its guards.
|
||||
await scopedStore.moveTask(task.id, reboundColumn);
|
||||
}
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-08-03-18:53:
|
||||
* Approval must clear the durable awaiting-approval hold with TaskStore's explicit
|
||||
* null sentinel; undefined omits a field from the patch. Persist the current plan's
|
||||
* fingerprint, or explicitly clear a prior fingerprint when PROMPT.md was unreadable,
|
||||
* so a stale plan can never bypass a later manual approval gate.
|
||||
*/
|
||||
const approved = await scopedStore.updateTask(task.id, approvalPatch);
|
||||
// Activity logging is secondary to the now-durable decision. Do not turn
|
||||
// a successful approval into a 500 if the bounded log append is unavailable.
|
||||
await scopedStore.logEntry(task.id, "Plan approved by user").catch((error) => {
|
||||
severityAuditLog.warn(`Failed to record plan approval activity for ${task.id}: ${error instanceof Error ? error.message : String(error)}`);
|
||||
});
|
||||
return approved;
|
||||
});
|
||||
|
||||
res.json(updated);
|
||||
@@ -4149,44 +4200,67 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
router.post("/tasks/:id/reject-plan", async (req, res) => {
|
||||
try {
|
||||
const { store: scopedStore } = await getProjectContext(req);
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
const updated = await scopedStore.withPlanningLifecycleLock(req.params.id, async () => {
|
||||
/*
|
||||
* FNXC:PlanningDependencyReseed 2026-08-04-06:35 FN-8768:
|
||||
* Match approval's locked fresh-read invariant: a stale reject must not clear needs-replan.
|
||||
*/
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
|
||||
// Same P0 as approve-plan above: resolve the intake column rather than naming
|
||||
// `triage`, which #2515 removed from the default lineage.
|
||||
const rejectIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id);
|
||||
if (task.column !== rejectIntakeColumn) {
|
||||
throw badRequest(`Task must be in the '${rejectIntakeColumn}' column to reject plan`);
|
||||
}
|
||||
if (task.status !== "awaiting-approval") {
|
||||
throw badRequest("Task must have status 'awaiting-approval' to reject plan");
|
||||
}
|
||||
// FNXC:ReleaseAuthorizationGate 2026-07-09-00:00:
|
||||
// Release-authorization gate removed — see the approve-plan handler above. A task
|
||||
// carrying the legacy release-authorization hold can now be rejected normally.
|
||||
/*
|
||||
* FNXC:WorkflowResolvedColumns 2026-08-04-06:35 FN-8768:
|
||||
* Match approve-plan by resolving the workflow-owned approval column rather than naming
|
||||
* legacy `triage`; exhausted review may deliberately park outside intake.
|
||||
*/
|
||||
const { approvalColumn: rejectColumn, intakeColumn } = await resolvePlanApprovalColumnsForTask(scopedStore, task);
|
||||
const retryingPartialCapRejection = task.awaitingApprovalReason === "plan-review-replan-cap"
|
||||
&& task.column === intakeColumn;
|
||||
if (task.column !== rejectColumn && !retryingPartialCapRejection) {
|
||||
throw badRequest(`Task must be in the '${rejectColumn}' column to reject plan`);
|
||||
}
|
||||
if (task.status !== "awaiting-approval") {
|
||||
throw badRequest("Task must have status 'awaiting-approval' to reject plan");
|
||||
}
|
||||
// FNXC:ReleaseAuthorizationGate 2026-07-09-00:00:
|
||||
// Release-authorization gate removed — see the approve-plan handler above. A task
|
||||
// carrying the legacy release-authorization hold can now be rejected normally.
|
||||
|
||||
// Log the rejection
|
||||
await scopedStore.logEntry(task.id, "Plan rejected by user", "Specification will be regenerated");
|
||||
// Log the rejection
|
||||
await scopedStore.logEntry(task.id, "Plan rejected by user", "Specification will be regenerated");
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-08-03-19:03:
|
||||
* Remove PROMPT.md before releasing the approval hold. If removal fails, the rejected
|
||||
* plan must remain blocked rather than becoming schedulable with rejected content.
|
||||
*/
|
||||
const { rm } = await import("node:fs/promises");
|
||||
const { join } = await import("node:path");
|
||||
const promptPath = join(scopedStore.getRootDir(), ".fusion", "tasks", task.id, "PROMPT.md");
|
||||
await rm(promptPath, { force: true });
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-08-03-19:03:
|
||||
* Remove PROMPT.md before releasing the approval hold. If removal fails, the rejected
|
||||
* plan must remain blocked rather than becoming schedulable with rejected content.
|
||||
*/
|
||||
const { rm } = await import("node:fs/promises");
|
||||
const { join } = await import("node:path");
|
||||
const promptPath = join(scopedStore.getRootDir(), ".fusion", "tasks", task.id, "PROMPT.md");
|
||||
await rm(promptPath, { force: true });
|
||||
|
||||
// Clear status to return to normal triage state
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — clear any previously-recorded approval fingerprint alongside the status
|
||||
* clear and PROMPT.md removal, so the regenerated plan is always treated as new and
|
||||
* requires fresh manual approval (it must never inherit the rejected plan's fingerprint).
|
||||
*/
|
||||
await scopedStore.updateTask(task.id, { status: null, approvedPlanFingerprint: null });
|
||||
if (task.column !== intakeColumn) {
|
||||
/*
|
||||
* FNXC:PlanReviewReplan 2026-08-04-06:35 FN-8768:
|
||||
* Keep awaiting-approval durable while a split-column cap park is rehomed to planning
|
||||
* intake. If the final clear fails, the safely blocked intake row can retry this route;
|
||||
* no interruption exposes rejected content to planning or execution.
|
||||
*/
|
||||
await scopedStore.moveTask(task.id, intakeColumn, {
|
||||
preserveStatus: true,
|
||||
workflowMoveSource: "plan-approval",
|
||||
});
|
||||
}
|
||||
|
||||
const updated = await scopedStore.getTask(task.id);
|
||||
// Clear status to return to normal triage state
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — clear any previously-recorded approval fingerprint alongside the status
|
||||
* clear and PROMPT.md removal, so the regenerated plan is always treated as new and
|
||||
* requires fresh manual approval (it must never inherit the rejected plan's fingerprint).
|
||||
*/
|
||||
await scopedStore.updateTask(task.id, { status: null, approvedPlanFingerprint: null });
|
||||
return await scopedStore.getTask(task.id);
|
||||
});
|
||||
res.json(updated);
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
|
||||
@@ -1071,6 +1071,15 @@ Ship FIVE kinds. Do NOT add roadmap-item in this task.
|
||||
expect(cap.last?.systemPrompt).toContain("Ship FIVE kinds. Do NOT add roadmap-item in this task.");
|
||||
expect(cap.last?.systemPrompt).toContain("PROMPT.md is the authoritative current contract");
|
||||
expect(cap.last?.systemPrompt).toContain("Do not enforce superseded requirements from the original Task Description");
|
||||
/*
|
||||
* FNXC:CodeReviewSurfaceCoverage 2026-08-04-06:35:
|
||||
* Review starts from changed files but follows necessary consumers and
|
||||
* tests, then restarts the complete procedure after any inline repair.
|
||||
*/
|
||||
expect(cap.last?.systemPrompt).toContain("modified-file list is the starting point");
|
||||
expect(cap.last?.systemPrompt).toContain("necessary callers, selectors, shared helpers, consumers, and tests");
|
||||
expect(cap.last?.systemPrompt).not.toContain("Review ONLY the files listed above");
|
||||
expect(cap.last?.systemPrompt).toContain("restart the mandatory review procedure");
|
||||
});
|
||||
|
||||
it("does not restore the historical task description when PROMPT.md is unavailable", async () => {
|
||||
|
||||
@@ -389,5 +389,252 @@ describe("browser-verification workflow-step browser capability", () => {
|
||||
expect(cap.last?.tools).toBe("readonly");
|
||||
expect(cap.last?.customTools?.map((tool) => tool.name)).toContain("fn_task_prompt_write");
|
||||
expect(cap.last?.systemPrompt).toContain("fn_task_prompt_write");
|
||||
expect(cap.last?.systemPrompt).not.toContain("## Convergence — Plan Review attempt");
|
||||
});
|
||||
|
||||
it("gives graph-owned Plan Review cumulative feedback and an attempt-three convergence ratchet", async () => {
|
||||
// FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768): The prompt must
|
||||
// carry full durable reviewer prose (not truncated activity previews), in
|
||||
// chronological order, while deriving the next attempt from raw results.
|
||||
const store = createMockStore();
|
||||
const executor = makeExecutor(store);
|
||||
const cap = captureSession();
|
||||
const task = baseTask({
|
||||
log: [
|
||||
{
|
||||
timestamp: "2026-08-03T00:00:01.000Z",
|
||||
action: "Plan Review failed — moved to todo for automatic replan (attempt 1/unbounded)",
|
||||
outcome: "PRIOR-BLOCKER-ONE\nWorkflow revision key: spec-gate",
|
||||
},
|
||||
{
|
||||
timestamp: "2026-08-03T00:00:03.000Z",
|
||||
action: "Plan Review failed — moved to todo for automatic replan (attempt 2/unbounded)",
|
||||
outcome: "PRIOR-BLOCKER-TWO\nWorkflow revision key: spec-gate",
|
||||
},
|
||||
{
|
||||
timestamp: "2026-08-03T00:00:04.000Z",
|
||||
action: "AI spec revision requested",
|
||||
outcome: "UNRELATED-PARSE-RECOVERY-MUST-NOT-LEAK",
|
||||
},
|
||||
],
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "spec-gate",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: `PRIOR-BLOCKER-TWO: define the lock ordering.\n${"x".repeat(4_100)}TAIL-BLOCKER`,
|
||||
priorAttempts: [{
|
||||
workflowStepId: "spec-gate",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "PRIOR-BLOCKER-ONE: enumerate every lifecycle writer.",
|
||||
}],
|
||||
}],
|
||||
});
|
||||
|
||||
const result = await (executor as any).executeWorkflowStep(
|
||||
task,
|
||||
planReviewStep({ optionalGroupId: "spec-gate" }),
|
||||
"/tmp/wt",
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(cap.last?.systemPrompt).toContain("## Convergence — Plan Review attempt 3");
|
||||
expect(cap.last?.systemPrompt).toContain("### Cumulative prior Plan Review ledger");
|
||||
expect(cap.last?.systemPrompt).toContain("PRIOR-BLOCKER-ONE");
|
||||
expect(cap.last?.systemPrompt).toContain("PRIOR-BLOCKER-TWO");
|
||||
expect(cap.last?.systemPrompt).toContain("TAIL-BLOCKER");
|
||||
expect(cap.last?.systemPrompt).not.toContain("UNRELATED-PARSE-RECOVERY-MUST-NOT-LEAK");
|
||||
expect(cap.last?.systemPrompt).toContain("Severity ratchet (attempt 3+)");
|
||||
expect(cap.last?.systemPrompt).toContain("must identify the revision that introduced it");
|
||||
expect(cap.last?.systemPrompt).toContain("never demote a critical defect merely because it was missed before");
|
||||
});
|
||||
|
||||
it("stops Plan Review convergence history at a superseded planning-episode boundary", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = makeExecutor(store);
|
||||
const cap = captureSession();
|
||||
const task = baseTask({
|
||||
log: [
|
||||
{
|
||||
timestamp: "2026-08-03T00:00:01.000Z",
|
||||
action: "Plan Review failed — moved to todo for automatic replan (attempt 7/unbounded)",
|
||||
outcome: "OLD-LOG-MUST-NOT-COUNT\nWorkflow revision key: plan-review",
|
||||
},
|
||||
],
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "CURRENT-EPISODE-BLOCKER",
|
||||
priorAttempts: [
|
||||
{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "SUPERSEDED-BOUNDARY",
|
||||
supersededAt: "2026-08-03T00:00:00.000Z",
|
||||
},
|
||||
{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "OLDER-EPISODE-BLOCKER",
|
||||
},
|
||||
],
|
||||
}],
|
||||
});
|
||||
|
||||
const result = await (executor as any).executeWorkflowStep(
|
||||
task,
|
||||
planReviewStep({ optionalGroupId: "plan-review" }),
|
||||
"/tmp/wt",
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(cap.last?.systemPrompt).toContain("## Convergence — Plan Review attempt 2");
|
||||
expect(cap.last?.systemPrompt).toContain("CURRENT-EPISODE-BLOCKER");
|
||||
expect(cap.last?.systemPrompt).not.toContain("SUPERSEDED-BOUNDARY");
|
||||
expect(cap.last?.systemPrompt).not.toContain("OLDER-EPISODE-BLOCKER");
|
||||
expect(cap.last?.systemPrompt).not.toContain("OLD-LOG-MUST-NOT-COUNT");
|
||||
expect(cap.last?.systemPrompt).not.toContain("Severity ratchet (attempt 3+)");
|
||||
});
|
||||
|
||||
it("excludes provider failures without a REVISE verdict from the Plan Review ledger", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = makeExecutor(store);
|
||||
const cap = captureSession();
|
||||
const task = baseTask({
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "REAL-PLAN-BLOCKER",
|
||||
priorAttempts: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
output: "PROVIDER-DIAGNOSTIC-MUST-NOT-BECOME-A-DECISION",
|
||||
}],
|
||||
}],
|
||||
});
|
||||
|
||||
const result = await (executor as any).executeWorkflowStep(
|
||||
task,
|
||||
planReviewStep({ optionalGroupId: "plan-review" }),
|
||||
"/tmp/wt",
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(cap.last?.systemPrompt).toContain("## Convergence — Plan Review attempt 2");
|
||||
expect(cap.last?.systemPrompt).toContain("REAL-PLAN-BLOCKER");
|
||||
expect(cap.last?.systemPrompt).not.toContain("PROVIDER-DIAGNOSTIC-MUST-NOT-BECOME-A-DECISION");
|
||||
});
|
||||
|
||||
it("counts repeated identical Plan Review feedback as distinct attempts while deduplicating display", async () => {
|
||||
// FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768): Display
|
||||
// deduplication is readability-only and must not reduce admission budgets.
|
||||
const store = createMockStore();
|
||||
const executor = makeExecutor(store);
|
||||
const cap = captureSession();
|
||||
const repeatedAttempt = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "REPEATED-BLOCKER",
|
||||
};
|
||||
const task = baseTask({
|
||||
workflowStepResults: [{
|
||||
...repeatedAttempt,
|
||||
priorAttempts: [{ ...repeatedAttempt }, { ...repeatedAttempt }],
|
||||
}],
|
||||
});
|
||||
|
||||
const result = await (executor as any).executeWorkflowStep(
|
||||
task,
|
||||
planReviewStep({ optionalGroupId: "plan-review" }),
|
||||
"/tmp/wt",
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(cap.last?.systemPrompt).toContain("## Convergence — Plan Review attempt 4");
|
||||
expect(cap.last?.systemPrompt?.match(/REPEATED-BLOCKER/g)).toHaveLength(1);
|
||||
expect(cap.last?.systemPrompt).toContain("Severity ratchet (attempt 3+)");
|
||||
});
|
||||
|
||||
it("does not leak Plan Review convergence history into code review", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = makeExecutor(store);
|
||||
const cap = captureSession();
|
||||
const task = baseTask({
|
||||
log: [{
|
||||
timestamp: "2026-08-03T00:00:00.000Z",
|
||||
action: "AI spec revision requested",
|
||||
outcome: "PLAN-REVIEW-HISTORY-MUST-NOT-LEAK",
|
||||
}],
|
||||
});
|
||||
|
||||
const result = await (executor as any).executeWorkflowStep(
|
||||
task,
|
||||
codeReviewStep(),
|
||||
"/tmp/wt",
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(cap.last?.systemPrompt).not.toContain("PLAN-REVIEW-HISTORY-MUST-NOT-LEAK");
|
||||
expect(cap.last?.systemPrompt).not.toContain("## Convergence — Plan Review attempt");
|
||||
});
|
||||
|
||||
it("recognizes a renamed inner step from the canonical Plan Review optional group", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = makeExecutor(store);
|
||||
const cap = captureSession();
|
||||
|
||||
const result = await (executor as any).executeWorkflowStep(
|
||||
baseTask(),
|
||||
planReviewStep({
|
||||
id: "graph:renamed-spec-check",
|
||||
name: "Specification Quality",
|
||||
optionalGroupId: "plan-review",
|
||||
}),
|
||||
"/tmp/wt",
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(cap.last?.tools).toBe("readonly");
|
||||
expect(cap.last?.customTools?.map((tool) => tool.name)).toContain("fn_task_prompt_write");
|
||||
expect(cap.last?.systemPrompt).toContain("Plan Review Scope:");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
collectPlanReviewFeedbackHistory,
|
||||
countPlanReviewRevisionAttempts,
|
||||
nextPlanReviewAttemptCount,
|
||||
PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT,
|
||||
} from "../plan-review-feedback-history.js";
|
||||
|
||||
describe("Plan Review feedback history", () => {
|
||||
it("caps rendered chronology to the current episode without capping raw attempts", () => {
|
||||
const attempt = (number: number) => ({
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: `CURRENT-EPISODE-${number}`,
|
||||
});
|
||||
const priorAttempts = Array.from({ length: 16 }, (_, index) => attempt(16 - index));
|
||||
const results = [{
|
||||
...attempt(17),
|
||||
priorAttempts: [
|
||||
...priorAttempts,
|
||||
{ ...attempt(99), notes: "SUPERSEDED-BOUNDARY", supersededAt: "2026-08-04T06:00:00.000Z" },
|
||||
{ ...attempt(98), notes: "STALE-OLDER-EPISODE" },
|
||||
],
|
||||
}, {
|
||||
...attempt(97),
|
||||
notes: "STALE-SUPERSEDED-PROJECTION",
|
||||
supersededAt: "2026-08-04T06:00:00.000Z",
|
||||
}];
|
||||
|
||||
const history = collectPlanReviewFeedbackHistory(results);
|
||||
|
||||
expect(history).toHaveLength(PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT);
|
||||
expect(history[0]).toBe("CURRENT-EPISODE-3");
|
||||
expect(history.at(-1)).toBe("CURRENT-EPISODE-17");
|
||||
expect(history).not.toContain("CURRENT-EPISODE-1");
|
||||
expect(history).not.toContain("SUPERSEDED-BOUNDARY");
|
||||
expect(history).not.toContain("STALE-OLDER-EPISODE");
|
||||
expect(history).not.toContain("STALE-SUPERSEDED-PROJECTION");
|
||||
expect(countPlanReviewRevisionAttempts(results)).toBe(17);
|
||||
});
|
||||
|
||||
it("deduplicates rendered prose but counts every same-episode revision", () => {
|
||||
const repeated = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "REPEATED",
|
||||
};
|
||||
const results = [{ ...repeated, priorAttempts: [{ ...repeated }, { ...repeated }] }];
|
||||
|
||||
expect(collectPlanReviewFeedbackHistory(results)).toEqual(["REPEATED"]);
|
||||
expect(countPlanReviewRevisionAttempts(results)).toBe(3);
|
||||
});
|
||||
|
||||
it("uses the persisted raw count after rendered history reaches its cap", () => {
|
||||
const result = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "latest",
|
||||
planReviewAttemptCount: 37,
|
||||
priorAttempts: Array.from({ length: PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT }, (_, index) => ({
|
||||
workflowStepId: "plan-review",
|
||||
verdict: "REVISE",
|
||||
notes: `retained-${index}`,
|
||||
})),
|
||||
};
|
||||
|
||||
expect(collectPlanReviewFeedbackHistory([result])).toHaveLength(PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT);
|
||||
expect(countPlanReviewRevisionAttempts([result])).toBe(37);
|
||||
expect(countPlanReviewRevisionAttempts([result], { includeCurrent: false })).toBe(36);
|
||||
});
|
||||
|
||||
it("advances the persisted count once per terminal attempt and resets after supersession", () => {
|
||||
const firstFailure = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
startedAt: "T1",
|
||||
};
|
||||
expect(nextPlanReviewAttemptCount(undefined, firstFailure)).toBe(1);
|
||||
|
||||
const pending = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
status: "pending",
|
||||
startedAt: "T2",
|
||||
};
|
||||
expect(nextPlanReviewAttemptCount({ ...firstFailure, planReviewAttemptCount: 18 }, pending)).toBe(18);
|
||||
expect(nextPlanReviewAttemptCount({ ...pending, planReviewAttemptCount: 18 }, { ...firstFailure, startedAt: "T2" })).toBe(19);
|
||||
expect(nextPlanReviewAttemptCount({ ...firstFailure, planReviewAttemptCount: 19 }, firstFailure)).toBe(19);
|
||||
expect(nextPlanReviewAttemptCount({ ...firstFailure, planReviewAttemptCount: 19, supersededAt: "T3" }, pending)).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,57 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { classifyPersistedPlanHandoff } from "../planning-handoff-recovery.js";
|
||||
|
||||
type HandoffTask = Parameters<typeof classifyPersistedPlanHandoff>[0];
|
||||
|
||||
function approvedNullTask(overrides: Partial<HandoffTask> = {}): HandoffTask {
|
||||
return {
|
||||
status: null,
|
||||
paused: false,
|
||||
userPaused: false,
|
||||
approvedPlanFingerprint: null,
|
||||
awaitingApprovalReason: null,
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "passed",
|
||||
verdict: "APPROVE",
|
||||
}],
|
||||
updatedAt: "2026-08-04T05:00:00.000Z",
|
||||
steps: [],
|
||||
worktree: undefined,
|
||||
firstExecutionAt: undefined,
|
||||
executionStartedAt: undefined,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
const options = { now: Date.parse("2026-08-04T07:00:00.000Z"), hasLivePlanningWork: false };
|
||||
|
||||
describe("classifyPersistedPlanHandoff", () => {
|
||||
it("recognizes an inert null-status task with a current Plan Review approval", () => {
|
||||
expect(classifyPersistedPlanHandoff(approvedNullTask(), options)).toBe("approved-null");
|
||||
});
|
||||
|
||||
it("does not recover an operator-held task even when Plan Review passed", () => {
|
||||
expect(classifyPersistedPlanHandoff(approvedNullTask({
|
||||
awaitingApprovalReason: "plan-review-replan-cap",
|
||||
}), options)).toBeNull();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["worktree", { worktree: "/tmp/fusion-task" }],
|
||||
["first execution timestamp", { firstExecutionAt: "2026-08-04T06:00:00.000Z" }],
|
||||
["execution segment timestamp", { executionStartedAt: "2026-08-04T06:00:00.000Z" }],
|
||||
])("does not recover approved-null tasks with %s evidence", (_label, evidence) => {
|
||||
expect(classifyPersistedPlanHandoff(approvedNullTask(evidence), options)).toBeNull();
|
||||
});
|
||||
|
||||
it("does not recover a planning-status task parked for approval", () => {
|
||||
expect(classifyPersistedPlanHandoff(approvedNullTask({
|
||||
status: "planning",
|
||||
awaitingApprovalReason: "require-all",
|
||||
}), options)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,261 @@
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-04:30:
|
||||
Production-shaped regression for reporter #3325. A real PostgreSQL TaskStore owns
|
||||
the planning episode, dependency invalidation, continuation rows, refusal dedupe,
|
||||
and release move. Only the planner/reviewer callback is replaced by a deterministic
|
||||
function; no polling, network AI, or wall-clock waits participate.
|
||||
*/
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, expect, it, vi } from "vitest";
|
||||
import { mkdirSync, writeFileSync } from "node:fs";
|
||||
import { dirname } from "node:path";
|
||||
import {
|
||||
PLAN_REVIEW_GROUP_ID,
|
||||
type Task,
|
||||
type TaskStore,
|
||||
type WorkflowStepResult,
|
||||
} from "@fusion/core";
|
||||
|
||||
import {
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
pgDescribe,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../../../core/src/__test-utils__/pg-test-harness.js";
|
||||
import { getPromptPath } from "../../execution/spec-staleness.js";
|
||||
import { promoteHeldTask, runHoldReleaseSweep } from "../../execution/hold-release.js";
|
||||
import { SelfHealingManager } from "../../self-healing.js";
|
||||
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_planning_dependency_release",
|
||||
});
|
||||
|
||||
function planReviewPass(): WorkflowStepResult {
|
||||
return {
|
||||
workflowStepId: PLAN_REVIEW_GROUP_ID,
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "passed",
|
||||
completedAt: new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
|
||||
pgDescribe("FN-8768 planning dependency release interactions", () => {
|
||||
beforeAll(h.beforeAll);
|
||||
beforeEach(h.beforeEach);
|
||||
afterEach(async () => {
|
||||
vi.useRealTimers();
|
||||
await h.afterEach();
|
||||
});
|
||||
afterAll(h.afterAll);
|
||||
|
||||
async function seedDependency(id: string): Promise<void> {
|
||||
await h.store().createTaskWithReservedId(
|
||||
{ description: `dependency ${id}`, column: "done" },
|
||||
{ taskId: id, applyDefaultWorkflowSteps: false },
|
||||
);
|
||||
}
|
||||
|
||||
async function seedPlannedTask(
|
||||
id: string,
|
||||
overrides: Parameters<TaskStore["updateTask"]>[1] = {},
|
||||
): Promise<Task> {
|
||||
const store = h.store();
|
||||
await store.createTaskWithReservedId(
|
||||
{ description: `planned ${id}`, column: "todo" },
|
||||
{ taskId: id, applyDefaultWorkflowSteps: true },
|
||||
);
|
||||
const prompt = `# ${id}\n\n## Context\nReporter #3325 plan.\n\n## Steps\n\n### Step 1: Implement\n- [ ] work\n`;
|
||||
const promptPath = getPromptPath(store.getTasksDir(), id);
|
||||
mkdirSync(dirname(promptPath), { recursive: true });
|
||||
writeFileSync(promptPath, prompt, "utf8");
|
||||
await store.updateTask(id, {
|
||||
steps: [{ name: "Implement", status: "pending" }],
|
||||
status: null,
|
||||
approvedPlanFingerprint: "approved-episode",
|
||||
workflowStepResults: [planReviewPass()],
|
||||
...overrides,
|
||||
});
|
||||
store.taskCache.delete(id);
|
||||
return store.getTask(id);
|
||||
}
|
||||
|
||||
it.each([
|
||||
{
|
||||
label: "dependency mutation API",
|
||||
taskId: "FN-8768-A",
|
||||
dependencyId: "FN-8768-DA",
|
||||
mutate: async (taskId: string, depId: string) => {
|
||||
await h.store().updateTaskDependencies(taskId, { operation: "add", dependency: depId });
|
||||
},
|
||||
},
|
||||
{
|
||||
label: "combined task update API",
|
||||
taskId: "FN-8768-B",
|
||||
dependencyId: "FN-8768-DB",
|
||||
mutate: async (taskId: string, depId: string) => {
|
||||
await h.store().updateTask(taskId, {
|
||||
dependencies: [depId],
|
||||
nodeId: null,
|
||||
// Stale fields from the same dashboard PATCH must not undo invalidation.
|
||||
status: null,
|
||||
approvedPlanFingerprint: "stale-writer",
|
||||
workflowStepResults: [planReviewPass()],
|
||||
});
|
||||
},
|
||||
},
|
||||
])("$label invalidates the current approval episode", async ({ taskId, dependencyId, mutate }) => {
|
||||
await seedDependency(dependencyId);
|
||||
await seedPlannedTask(taskId);
|
||||
|
||||
await mutate(taskId, dependencyId);
|
||||
|
||||
h.store().taskCache.delete(taskId);
|
||||
const updated = await h.store().getTask(taskId);
|
||||
expect(updated).toMatchObject({ status: "needs-replan", dependencies: [dependencyId] });
|
||||
expect(updated.approvedPlanFingerprint).toBeUndefined();
|
||||
expect(updated.workflowStepResults).toEqual([
|
||||
expect.objectContaining({
|
||||
workflowStepId: PLAN_REVIEW_GROUP_ID,
|
||||
status: "passed",
|
||||
supersededReason: "dependency-change",
|
||||
supersededAt: expect.any(String),
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it("serializes dependency-first and lifecycle-first orderings without continuation theft", async () => {
|
||||
await seedDependency("FN-8768-DC");
|
||||
const task = await seedPlannedTask("FN-8768-C", {
|
||||
approvedPlanFingerprint: null,
|
||||
workflowStepResults: [],
|
||||
});
|
||||
vi.useFakeTimers({ toFake: ["Date"] });
|
||||
vi.setSystemTime(Date.now() + 31 * 60_000);
|
||||
|
||||
const recover = vi.fn(async (candidate: Task) => {
|
||||
await h.store().updateTask(candidate.id, { status: "awaiting-approval" });
|
||||
return true;
|
||||
});
|
||||
const manager = new SelfHealingManager(h.store(), {
|
||||
rootDir: h.store().getRootDir(),
|
||||
recoverApprovedTriageTask: recover,
|
||||
getPlanningTaskIds: () => new Set(),
|
||||
});
|
||||
|
||||
// Continuation recovery sees the same durable row but defers the exact
|
||||
// legacy persisted-plan shape to lifecycle recovery.
|
||||
await expect(manager.reconcileStrandedHoldContinuations()).resolves.toBe(0);
|
||||
await expect(manager.recoverApprovedTriageTasks()).resolves.toBe(1);
|
||||
expect(recover).toHaveBeenCalledOnce();
|
||||
|
||||
// Reset the episode, then hold the lifecycle lock while dependency mutation
|
||||
// queues behind it. The mutation cannot publish until the owner releases.
|
||||
await h.store().updateTask(task.id, { status: null, steps: [{ name: "Implement", status: "pending" }] });
|
||||
let releaseOwner!: () => void;
|
||||
const ownerReleased = new Promise<void>((resolve) => { releaseOwner = resolve; });
|
||||
let ownerEntered!: () => void;
|
||||
const entered = new Promise<void>((resolve) => { ownerEntered = resolve; });
|
||||
const completionOrder: string[] = [];
|
||||
const owner = h.store().withPlanningLifecycleLock(task.id, async () => {
|
||||
completionOrder.push("owner-entered");
|
||||
ownerEntered();
|
||||
await ownerReleased;
|
||||
completionOrder.push("owner-released");
|
||||
}).then(() => { completionOrder.push("owner-completed"); });
|
||||
await entered;
|
||||
const mutation = h.store().updateTaskDependencies(task.id, {
|
||||
operation: "add",
|
||||
dependency: "FN-8768-DC",
|
||||
}).then(() => { completionOrder.push("dependency-mutation-completed"); });
|
||||
expect(completionOrder).toEqual(["owner-entered"]);
|
||||
completionOrder.push("release-requested");
|
||||
releaseOwner();
|
||||
await Promise.all([owner, mutation]);
|
||||
expect(completionOrder).toEqual([
|
||||
"owner-entered",
|
||||
"release-requested",
|
||||
"owner-released",
|
||||
"owner-completed",
|
||||
"dependency-mutation-completed",
|
||||
]);
|
||||
|
||||
h.store().taskCache.delete(task.id);
|
||||
expect(await h.store().getTask(task.id)).toMatchObject({
|
||||
status: "needs-replan",
|
||||
dependencies: ["FN-8768-DC"],
|
||||
});
|
||||
await expect(manager.reconcileStrandedHoldContinuations()).resolves.toBe(0);
|
||||
});
|
||||
|
||||
it("rechecks after acquiring the lifecycle lock when dependency mutation lands after discovery", async () => {
|
||||
await seedDependency("FN-8768-DE");
|
||||
const task = await seedPlannedTask("FN-8768-E", {
|
||||
approvedPlanFingerprint: null,
|
||||
workflowStepResults: [],
|
||||
// No parsed steps: this is an ordinary continuation-owned null episode,
|
||||
// not the conservative legacy lifecycle-recovery shape.
|
||||
steps: [],
|
||||
});
|
||||
vi.useFakeTimers({ toFake: ["Date"] });
|
||||
vi.setSystemTime(Date.now() + 2 * 60_000);
|
||||
|
||||
const store = h.store();
|
||||
const originalLock = store.withPlanningLifecycleLock.bind(store);
|
||||
let injectDependency = true;
|
||||
store.withPlanningLifecycleLock = (async <T>(id: string, callback: () => Promise<T>) => {
|
||||
if (injectDependency) {
|
||||
injectDependency = false;
|
||||
await store.updateTaskDependencies(id, { operation: "add", dependency: "FN-8768-DE" });
|
||||
}
|
||||
return originalLock(id, callback);
|
||||
}) as typeof store.withPlanningLifecycleLock;
|
||||
try {
|
||||
const manager = new SelfHealingManager(store, { rootDir: store.getRootDir() });
|
||||
|
||||
await expect(manager.reconcileStrandedHoldContinuations()).resolves.toBe(0);
|
||||
expect(await store.listWorkflowWorkItemsForTask(task.id)).toHaveLength(0);
|
||||
store.taskCache.delete(task.id);
|
||||
expect(await store.getTask(task.id)).toMatchObject({
|
||||
status: "needs-replan",
|
||||
dependencies: ["FN-8768-DE"],
|
||||
});
|
||||
} finally {
|
||||
store.withPlanningLifecycleLock = originalLock;
|
||||
}
|
||||
});
|
||||
|
||||
it("dedupes refusal evidence by episode across sweep and promote, then force-promotes", async () => {
|
||||
await seedDependency("FN-8768-DD");
|
||||
const task = await seedPlannedTask("FN-8768-D", {
|
||||
status: "needs-replan",
|
||||
approvedPlanFingerprint: null,
|
||||
workflowStepResults: [],
|
||||
});
|
||||
|
||||
const sweep = await runHoldReleaseSweep(h.store(), { now: () => Date.now() });
|
||||
expect(sweep.released).not.toContain(task.id);
|
||||
await expect(promoteHeldTask(h.store(), task.id)).resolves.toMatchObject({
|
||||
released: false,
|
||||
rejection: "unplanned-for-execution",
|
||||
});
|
||||
await promoteHeldTask(h.store(), task.id);
|
||||
|
||||
h.store().taskCache.delete(task.id);
|
||||
let live = await h.store().getTask(task.id);
|
||||
expect(live.log?.filter((entry) => entry.action.includes("Execution dispatch refused"))).toHaveLength(1);
|
||||
|
||||
// A new dependency changes the durable episode and permits one new refusal.
|
||||
await h.store().updateTaskDependencies(task.id, { operation: "add", dependency: "FN-8768-DD" });
|
||||
await promoteHeldTask(h.store(), task.id);
|
||||
h.store().taskCache.delete(task.id);
|
||||
live = await h.store().getTask(task.id);
|
||||
expect(live.log?.filter((entry) => entry.action.includes("Execution dispatch refused"))).toHaveLength(2);
|
||||
|
||||
await expect(promoteHeldTask(h.store(), task.id, {}, { force: true })).resolves.toMatchObject({
|
||||
released: true,
|
||||
toColumn: "in-progress",
|
||||
forcedUnplanned: true,
|
||||
});
|
||||
h.store().taskCache.delete(task.id);
|
||||
expect((await h.store().getTask(task.id)).column).toBe("in-progress");
|
||||
});
|
||||
});
|
||||
@@ -517,7 +517,14 @@ describe("reviewStep — spec review type", () => {
|
||||
"# Task: KB-050\n\n## Mission\nDo something great",
|
||||
);
|
||||
|
||||
/*
|
||||
* FNXC:PlanReviewPromptBoundary 2026-08-04-06:35:
|
||||
* A spec session must carry the mandatory holistic policy and batch every
|
||||
* independently discoverable blocker, without inheriting code-diff rules.
|
||||
*/
|
||||
expect(capturedPrompt).toContain("Evaluate this PROMPT.md specification");
|
||||
expect(capturedPrompt).toContain("## Mandatory Plan Review Procedure");
|
||||
expect(capturedPrompt).toContain("all independently discoverable blocking findings");
|
||||
expect(capturedPrompt).toContain("spec quality criteria");
|
||||
expect(capturedPrompt).toContain("# Task: KB-050");
|
||||
expect(capturedPrompt).toContain("dangling task-document references");
|
||||
@@ -553,104 +560,6 @@ describe("reviewStep — spec review type", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TriagePlanReviewConvergence 2026-07-16-19:40:
|
||||
Prove the spec-gate convergence block is wired through reviewStep -> buildReviewRequest. We drive
|
||||
the real (module-private) request builder by capturing the prompt string handed to the mocked
|
||||
session, exactly like the "spec review type" tests above — no test-only export is needed because
|
||||
the request text is observable at the session seam.
|
||||
*/
|
||||
describe("reviewStep — spec convergence wiring", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
function captureReviewPrompt(): { getPrompt: () => string } {
|
||||
const state = { prompt: "" };
|
||||
mockedCreateFnAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async (prompt: string) => {
|
||||
state.prompt = prompt;
|
||||
}),
|
||||
subscribe: vi.fn().mockImplementation((cb: any) => {
|
||||
cb({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "### Verdict: APPROVE\n### Summary\nOK" },
|
||||
});
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
return { getPrompt: () => state.prompt };
|
||||
}
|
||||
|
||||
it("omits the convergence block for spec reviews on attempt <= 1 or undefined", async () => {
|
||||
const cap = captureReviewPrompt();
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-CONV", 0, "Spec Review", "spec", "# Task: FN-CONV",
|
||||
undefined,
|
||||
{ priorSpecReviewFeedback: "prior REVISE text", specReviewAttempt: 1 },
|
||||
);
|
||||
expect(cap.getPrompt()).not.toContain("## Convergence — Plan Review attempt");
|
||||
expect(cap.getPrompt()).not.toContain("prior REVISE text");
|
||||
});
|
||||
|
||||
it("omits the convergence block for spec reviews when convergence fields are absent", async () => {
|
||||
const cap = captureReviewPrompt();
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-CONV", 0, "Spec Review", "spec", "# Task: FN-CONV",
|
||||
);
|
||||
expect(cap.getPrompt()).not.toContain("## Convergence — Plan Review attempt");
|
||||
});
|
||||
|
||||
it("includes the convergence block + prior feedback + verify-your-own-miss wording at attempt 2", async () => {
|
||||
const cap = captureReviewPrompt();
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-CONV", 0, "Spec Review", "spec", "# Task: FN-CONV",
|
||||
undefined,
|
||||
{ priorSpecReviewFeedback: "PRIOR-REVISE-MARKER: fix the missing Surface Enumeration", specReviewAttempt: 2 },
|
||||
);
|
||||
const prompt = cap.getPrompt();
|
||||
expect(prompt).toContain("## Convergence — Plan Review attempt 2");
|
||||
expect(prompt).toContain("PRIOR-REVISE-MARKER: fix the missing Surface Enumeration");
|
||||
expect(prompt).toContain("VERIFY each issue you raised previously was addressed");
|
||||
expect(prompt).toContain("that is your own earlier miss");
|
||||
// Attempt 2 must NOT yet ratchet severity.
|
||||
expect(prompt).not.toContain("Severity ratchet (attempt 3+)");
|
||||
});
|
||||
|
||||
it("adds the severity ratchet at attempt >= 3", async () => {
|
||||
const cap = captureReviewPrompt();
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-CONV", 0, "Spec Review", "spec", "# Task: FN-CONV",
|
||||
undefined,
|
||||
{ priorSpecReviewFeedback: "prior text", specReviewAttempt: 3 },
|
||||
);
|
||||
const prompt = cap.getPrompt();
|
||||
expect(prompt).toContain("## Convergence — Plan Review attempt 3");
|
||||
expect(prompt).toContain("Severity ratchet (attempt 3+)");
|
||||
});
|
||||
|
||||
it("never includes the convergence block for code reviews even when convergence fields are passed", async () => {
|
||||
const cap = captureReviewPrompt();
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-CONV", 1, "Code Review", "code", "# prompt", "abc123",
|
||||
{ priorSpecReviewFeedback: "prior text", specReviewAttempt: 3 } as any,
|
||||
);
|
||||
expect(cap.getPrompt()).not.toContain("## Convergence — Plan Review attempt");
|
||||
});
|
||||
|
||||
it("never includes the convergence block for plan reviews even when convergence fields are passed", async () => {
|
||||
const cap = captureReviewPrompt();
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-CONV", 1, "Plan Review", "plan", "# prompt",
|
||||
undefined,
|
||||
{ priorSpecReviewFeedback: "prior text", specReviewAttempt: 3 } as any,
|
||||
);
|
||||
expect(cap.getPrompt()).not.toContain("## Convergence — Plan Review attempt");
|
||||
});
|
||||
});
|
||||
|
||||
describe("FN-5928 surface-enumeration review-gate wording", () => {
|
||||
it("requires spec reviews to block missing or incomplete surface enumeration for bug-fix specs", () => {
|
||||
expect(DEFAULT_REVIEWER_PROMPT).toContain("**Surface enumeration:**");
|
||||
|
||||
@@ -12,7 +12,7 @@ vi.mock("@fusion/core", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("@fusion/core")>()),
|
||||
resolveWorkflowIrForTask: resolveWorkflowIrForTaskMock,
|
||||
}));
|
||||
vi.mock("../run-audit.js", async (importOriginal) => ({
|
||||
vi.mock("../util/run-audit.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("../util/run-audit.js")>()),
|
||||
createRunAuditor: vi.fn(() => ({ database: recordRunAuditEventMock })),
|
||||
}));
|
||||
@@ -57,6 +57,7 @@ function storeFor(task: Task, settings: Partial<Settings> = {}) {
|
||||
items.push(item);
|
||||
return { seeded: true, workItemId: item.id };
|
||||
}),
|
||||
withPlanningLifecycleLock: vi.fn(async (_id: string, callback: () => Promise<unknown>) => callback()),
|
||||
getTasksDir: vi.fn(() => ""),
|
||||
// FNXC:StrandedHoldContinuation 2026-08-02-00:15: 713e9320b0 routed continuation dispatch through createPlanningContinuationDispatcher, whose capacity gate reads projectId from store.getRootDir(); the mock must expose it so drainWorkflowContinuations does not throw.
|
||||
getRootDir: vi.fn(() => "fn8592-project"),
|
||||
@@ -94,6 +95,7 @@ describe("FN-8592 stranded hold continuation recovery", () => {
|
||||
|
||||
await expect(manager.reconcileStrandedHoldContinuations()).resolves.toBe(1);
|
||||
expect(store.seedStrandedPlanReviewContinuation).toHaveBeenCalledOnce();
|
||||
expect(store.withPlanningLifecycleLock).toHaveBeenCalledWith(task.id, expect.any(Function));
|
||||
expect(store._items).toHaveLength(1);
|
||||
expect(recordRunAuditEventMock).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "task:reconcile-stranded-hold-continuation",
|
||||
@@ -102,6 +104,50 @@ describe("FN-8592 stranded hold continuation recovery", () => {
|
||||
expect(JSON.stringify(recordRunAuditEventMock.mock.calls)).not.toContain("Real work");
|
||||
});
|
||||
|
||||
it("defers the exact stale null-status persisted-plan episode to lifecycle recovery", () => {
|
||||
const stale = new Date(Date.now() - 31 * 60_000).toISOString();
|
||||
const task = strandedTask({
|
||||
status: null as never,
|
||||
steps: [{ name: "Persisted planner step", status: "pending" }],
|
||||
updatedAt: stale,
|
||||
columnMovedAt: stale,
|
||||
});
|
||||
|
||||
expect(evaluateStrandedHoldContinuation({
|
||||
task,
|
||||
columnFlags: { hold: true, intake: true },
|
||||
ir: workflow,
|
||||
continuations: [],
|
||||
stepResults: [],
|
||||
effectiveSettings: {},
|
||||
enginePaused: false,
|
||||
promptContent: "# Task\n\n## Steps\n\n### Step 0: Persisted planner step",
|
||||
live: false,
|
||||
stalenessMs: 31 * 60_000,
|
||||
graceMs: 60_000,
|
||||
now: Date.now(),
|
||||
})).toMatchObject({ stranded: false, candidate: false, reason: "planning-recovery-owned" });
|
||||
});
|
||||
|
||||
it("keeps ordinary null-status continuation recovery outside the conservative legacy shape", () => {
|
||||
const task = strandedTask({ status: null as never, steps: [] });
|
||||
|
||||
expect(evaluateStrandedHoldContinuation({
|
||||
task,
|
||||
columnFlags: { hold: true, intake: true },
|
||||
ir: workflow,
|
||||
continuations: [],
|
||||
stepResults: [],
|
||||
effectiveSettings: {},
|
||||
enginePaused: false,
|
||||
promptContent: "# Task\n\n## Steps\n\n### Step 0: Ordinary continuation",
|
||||
live: false,
|
||||
stalenessMs: 120_000,
|
||||
graceMs: 60_000,
|
||||
now: Date.now(),
|
||||
})).toMatchObject({ stranded: true, candidate: true, reason: "ready" });
|
||||
});
|
||||
|
||||
it("drives the repaired continuation through the runtime processor and graph reviewer seam", async () => {
|
||||
const task = strandedTask();
|
||||
const store = storeFor(task);
|
||||
|
||||
@@ -8493,6 +8493,78 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
|
||||
describe("recoverApprovedTriageTasks", () => {
|
||||
it("selects the stale legacy null-status persisted-plan handoff reported in #3325", async () => {
|
||||
const legacy = {
|
||||
id: "FN-8768-LEGACY",
|
||||
column: "todo",
|
||||
status: null,
|
||||
paused: false,
|
||||
approvedPlanFingerprint: undefined,
|
||||
awaitingApprovalReason: undefined,
|
||||
workflowStepResults: undefined,
|
||||
steps: [{ title: "Implement", status: "pending" }],
|
||||
log: [],
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
} as unknown as Task;
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const recoveryStore = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([legacy]),
|
||||
getTask: vi.fn().mockResolvedValue(legacy),
|
||||
});
|
||||
const managerWithRecovery = new SelfHealingManager(recoveryStore, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverApprovedTriageTask: recoverFn,
|
||||
getPlanningTaskIds: () => new Set<string>(),
|
||||
});
|
||||
vi.setSystemTime(new Date("2026-01-01T00:31:00.000Z"));
|
||||
|
||||
await expect(managerWithRecovery.recoverApprovedTriageTasks()).resolves.toBe(1);
|
||||
expect(recoverFn).toHaveBeenCalledWith(legacy);
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["recent", { updatedAt: "2026-01-01T00:04:30.000Z" }, new Set<string>()],
|
||||
["paused", { paused: true }, new Set<string>()],
|
||||
["user-paused", { userPaused: true }, new Set<string>()],
|
||||
["actively planning", {}, new Set(["FN-8768-CONTROL"])],
|
||||
["approval evidence", { approvedPlanFingerprint: "current-plan" }, new Set<string>()],
|
||||
["approval hold", { awaitingApprovalReason: "plan-review-replan-cap" }, new Set<string>()],
|
||||
["unsatisfied graph evidence", { workflowStepResults: [{ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "failed" }] }, new Set<string>()],
|
||||
["missing persisted steps", { steps: [] }, new Set<string>()],
|
||||
["worktree", { worktree: "/tmp/executing" }, new Set<string>()],
|
||||
["execution stamp", { firstExecutionAt: "2026-01-01T00:10:00.000Z" }, new Set<string>()],
|
||||
])("does not select a %s null-status task as the legacy handoff", async (_label, patch, planningIds) => {
|
||||
const candidate = {
|
||||
id: "FN-8768-CONTROL",
|
||||
column: "todo",
|
||||
status: null,
|
||||
paused: false,
|
||||
approvedPlanFingerprint: undefined,
|
||||
awaitingApprovalReason: undefined,
|
||||
workflowStepResults: undefined,
|
||||
steps: [{ title: "Implement", status: "pending" }],
|
||||
log: [],
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
...patch,
|
||||
} as unknown as Task;
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const recoveryStore = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([candidate]),
|
||||
getTask: vi.fn().mockResolvedValue(candidate),
|
||||
});
|
||||
const managerWithRecovery = new SelfHealingManager(recoveryStore, {
|
||||
rootDir: "/tmp/test-project",
|
||||
recoverApprovedTriageTask: recoverFn,
|
||||
getPlanningTaskIds: () => planningIds,
|
||||
});
|
||||
vi.setSystemTime(new Date("2026-01-01T00:31:00.000Z"));
|
||||
|
||||
await expect(managerWithRecovery.recoverApprovedTriageTasks()).resolves.toBe(0);
|
||||
expect(recoverFn).not.toHaveBeenCalled();
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("recovers specified planning triage tasks that are not actively processing", async () => {
|
||||
const recoverFn = vi.fn().mockResolvedValue(true);
|
||||
const getPlanning = vi.fn().mockReturnValue(new Set<string>());
|
||||
|
||||
@@ -158,14 +158,18 @@ describe("triage replan feedback falls back to Plan Review REVISE output", () =>
|
||||
rootDir = undefined;
|
||||
});
|
||||
|
||||
it("seeds the planner prompt from the latest plan-review REVISE output when no comment feedback exists", async () => {
|
||||
it("seeds the planner prompt from the latest durable Plan Review REVISE notes when no comment feedback exists", async () => {
|
||||
const reviseOutput = "PLAN-REVIEW-REVISE-MARKER: the plan omits the required migration step and must add it.";
|
||||
const rejectedDraft = "# Existing rejected plan\n\n## Mission\nDo not lose this body during replan.\n";
|
||||
const task = createTask({
|
||||
id: "FN-REPLAN-FEEDBACK-WSR",
|
||||
// No user comments and no "AI spec revision requested" log entry — the only
|
||||
// available feedback is the Plan Review REVISE result in workflowStepResults.
|
||||
log: [],
|
||||
// The activity log is only a bounded operator preview. The full durable
|
||||
// remediation contract comes from workflowStepResults.
|
||||
log: [{
|
||||
timestamp: "2026-07-13T00:00:20.000Z",
|
||||
action: "AI spec revision requested",
|
||||
outcome: "Revision source: plan-review/plan-review\nTRUNCATED-PREVIEW-MUST-NOT-WIN",
|
||||
}],
|
||||
workflowStepResults: [
|
||||
{
|
||||
workflowStepId: "plan-review",
|
||||
@@ -173,8 +177,8 @@ describe("triage replan feedback falls back to Plan Review REVISE output", () =>
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
output: reviseOutput,
|
||||
notes: "Needs a migration step.",
|
||||
output: "Reviewer prose may be incomplete.",
|
||||
notes: reviseOutput,
|
||||
},
|
||||
],
|
||||
});
|
||||
@@ -195,11 +199,13 @@ describe("triage replan feedback falls back to Plan Review REVISE output", () =>
|
||||
expect(mockPromptWithFallback).toHaveBeenCalled();
|
||||
expect(capturedPrompt).toBeDefined();
|
||||
expect(capturedPrompt).toContain(reviseOutput);
|
||||
expect(capturedPrompt).not.toContain("TRUNCATED-PREVIEW-MUST-NOT-WIN");
|
||||
// Surgical revision: rejected PROMPT body + feedback, not a fresh respec from title alone.
|
||||
expect(capturedPrompt).toContain("Revise this task");
|
||||
expect(capturedPrompt).toContain("Existing Specification");
|
||||
expect(capturedPrompt).toContain("Do not lose this body during replan");
|
||||
expect(capturedPrompt).toContain("Converge — do not rewrite from scratch");
|
||||
expect(capturedPrompt).toContain("PLAN-REVIEW-REVISE-MARKER");
|
||||
expect(capturedPrompt).not.toContain("Re-specify this task");
|
||||
});
|
||||
|
||||
@@ -224,6 +230,14 @@ describe("triage replan feedback falls back to Plan Review REVISE output", () =>
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
output: reviseOutput,
|
||||
priorAttempts: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: "failed",
|
||||
verdict: "REVISE",
|
||||
notes: "PRIOR-PLAN-REVIEW-FEEDBACK: preserve the lifecycle-writer audit.",
|
||||
}],
|
||||
},
|
||||
],
|
||||
});
|
||||
@@ -243,6 +257,13 @@ describe("triage replan feedback falls back to Plan Review REVISE output", () =>
|
||||
expect(capturedPrompt).toBeDefined();
|
||||
expect(capturedPrompt).toContain(explicitFeedback);
|
||||
expect(capturedPrompt).not.toContain(reviseOutput);
|
||||
expect(capturedPrompt).toContain("Cumulative Revision Decision Ledger");
|
||||
expect(capturedPrompt).toContain("### PR1");
|
||||
expect(capturedPrompt).toContain("PRIOR-PLAN-REVIEW-FEEDBACK");
|
||||
const ledger = capturedPrompt?.split("## Cumulative Revision Decision Ledger\n", 2)[1]
|
||||
?.split("\n\nRevise the specification above", 1)[0];
|
||||
expect(ledger).toBeDefined();
|
||||
expect(ledger).not.toContain("EXPLICIT-COMMENT-FEEDBACK");
|
||||
expect(capturedPrompt).toContain("Existing Specification");
|
||||
expect(capturedPrompt).toContain("Keep this under surgical revision");
|
||||
});
|
||||
|
||||
@@ -441,6 +441,12 @@ describe("buildSpecificationPrompt", () => {
|
||||
[],
|
||||
existingPrompt,
|
||||
feedback,
|
||||
{
|
||||
planReviewFeedbackHistory: [
|
||||
"Round one: enumerate every lifecycle writer.",
|
||||
"Round two: define the lock order and both race orderings.",
|
||||
],
|
||||
},
|
||||
);
|
||||
|
||||
expect(prompt).toContain("Revise this task");
|
||||
@@ -448,6 +454,12 @@ describe("buildSpecificationPrompt", () => {
|
||||
expect(prompt).toContain("Existing Specification");
|
||||
expect(prompt).toContain("Revision Feedback");
|
||||
expect(prompt).toContain("Converge — do not rewrite from scratch");
|
||||
expect(prompt).toContain("Cumulative Revision Decision Ledger");
|
||||
expect(prompt).toContain("### PR1");
|
||||
expect(prompt).toContain("Round one: enumerate every lifecycle writer.");
|
||||
expect(prompt).toContain("### PR2");
|
||||
expect(prompt).toContain("Round two: define the lock order and both race orderings.");
|
||||
expect(prompt).toContain("rerun the full Mandatory Planning Completeness Procedure");
|
||||
expect(prompt).toContain("surgical");
|
||||
expect(prompt).toContain(existingPrompt);
|
||||
expect(prompt).toContain(feedback);
|
||||
|
||||
@@ -738,9 +738,80 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(calls).toEqual(["execute"]);
|
||||
expect(requestFix).not.toHaveBeenCalled();
|
||||
expect(logs).toContain("[pre-merge] Workflow step already passed: Plan Review");
|
||||
expect(logs).toContain("[pre-merge] Workflow step already satisfied: Plan Review");
|
||||
});
|
||||
|
||||
it.each(["passed", "live pending lease"] as const)(
|
||||
"reruns Plan Review when a dependency superseded the prior %s and its old completion log",
|
||||
async (priorState) => {
|
||||
const calls: string[] = [];
|
||||
const logs: string[] = [];
|
||||
const ir: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "plan-review-superseded",
|
||||
columns: [{ id: "work", name: "Work", traits: [] }],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{
|
||||
id: "plan-review",
|
||||
kind: "optional-group",
|
||||
config: {
|
||||
name: "Plan Review",
|
||||
defaultOn: true,
|
||||
template: {
|
||||
nodes: [{ id: "plan-review-step", kind: "prompt", config: { prompt: "review replanned spec" } }],
|
||||
edges: [],
|
||||
},
|
||||
},
|
||||
},
|
||||
{ id: "execute", kind: "prompt", config: { prompt: "execute" } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "plan-review" },
|
||||
{ from: "plan-review", to: "execute", condition: "success" },
|
||||
{ from: "execute", to: "end" },
|
||||
],
|
||||
};
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: {
|
||||
prompt: async (node) => {
|
||||
calls.push(node.id);
|
||||
return { outcome: "success" };
|
||||
},
|
||||
},
|
||||
runLoopNowForTests: () => Date.parse("2026-08-04T02:00:01.000Z"),
|
||||
logTaskEntry: (summary) => { logs.push(summary); },
|
||||
});
|
||||
|
||||
const result = await executor.run({
|
||||
...taskWith(["plan-review"]),
|
||||
id: "FN-plan-review-superseded",
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge",
|
||||
status: priorState === "passed" ? "passed" : "pending",
|
||||
...(priorState === "live pending lease"
|
||||
? { startedAt: "2026-08-04T02:00:00.000Z", leaseOwner: "planner:old-episode" }
|
||||
: { completedAt: "2026-08-04T01:00:00.000Z" }),
|
||||
supersededAt: "2026-08-04T02:00:00.000Z",
|
||||
supersededReason: "dependency-change",
|
||||
}],
|
||||
log: [{
|
||||
timestamp: "2026-08-04T01:00:00.000Z",
|
||||
action: "[pre-merge] Workflow step completed: Plan Review",
|
||||
}],
|
||||
} as TaskDetail, settingsOn(), ir);
|
||||
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(calls).toEqual(["plan-review-step", "execute"]);
|
||||
expect(logs).not.toContain(
|
||||
"[pre-merge] Plan Review already in progress (lease held) — not dispatching a second reviewer",
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("repairs missing Plan Review result from the latest completed log before execution", async () => {
|
||||
const records: Array<{ workflowStepId: string; status: string; notes?: string }> = [];
|
||||
const calls: string[] = [];
|
||||
|
||||
@@ -46,6 +46,25 @@ function revisionLog(stepName: string, key: string, attempt: number) {
|
||||
};
|
||||
}
|
||||
|
||||
function repeatedPlanReviewResult(attemptCount: number): NonNullable<Task["workflowStepResults"]>[number] {
|
||||
const attempt = {
|
||||
workflowStepId: "plan-review",
|
||||
workflowStepName: "Plan Review",
|
||||
phase: "pre-merge" as const,
|
||||
status: "failed" as const,
|
||||
verdict: "REVISE" as const,
|
||||
notes: "same unresolved blocker",
|
||||
};
|
||||
return {
|
||||
...attempt,
|
||||
planReviewAttemptCount: attemptCount,
|
||||
priorAttempts: Array.from(
|
||||
{ length: Math.min(attemptCount - 1, 15) },
|
||||
() => ({ ...attempt }),
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
@@ -409,12 +428,14 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
|
||||
const cappedStore = createMockStore();
|
||||
const exhaustedTask = task({
|
||||
postReviewFixCount: 2,
|
||||
postReviewFixCount: 20,
|
||||
column: "in-progress",
|
||||
log: [revisionLog("Plan Review", "plan-review", 1), revisionLog("Plan Review", "plan-review", 2)],
|
||||
// The rendered/audit window is capped at 15, but the persisted scalar
|
||||
// must still exhaust a valid finite budget above that cap.
|
||||
workflowStepResults: [repeatedPlanReviewResult(21)],
|
||||
});
|
||||
cappedStore.getTask.mockResolvedValue(exhaustedTask);
|
||||
cappedStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 9, planReviewMaxRevisions: 2 });
|
||||
cappedStore.getSettings.mockResolvedValue({ maxPostReviewFixes: 9, planReviewMaxRevisions: 20 });
|
||||
const cappedExecutor = new TaskExecutor(cappedStore, "/tmp/test");
|
||||
|
||||
await expect((cappedExecutor as any).requestPreMergeOptionalStepFix(exhaustedTask.id, exhaustedTask, {
|
||||
@@ -451,7 +472,12 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
it("keeps replanning an unbounded Plan Review loop just below the safety cap", async () => {
|
||||
const store = createMockStore();
|
||||
const belowLog = Array.from({ length: 14 }, (_, i) => revisionLog("Plan Review", "plan-review", i + 1));
|
||||
const loopingTask = task({ postReviewFixCount: 14, column: "in-progress", log: belowLog });
|
||||
const loopingTask = task({
|
||||
postReviewFixCount: 14,
|
||||
column: "in-progress",
|
||||
log: belowLog,
|
||||
workflowStepResults: [repeatedPlanReviewResult(15)],
|
||||
});
|
||||
store.getTask.mockResolvedValue(loopingTask);
|
||||
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 9 }); // no planReviewMaxRevisions → unbounded
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
@@ -480,7 +506,12 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
it("halts the unbounded Plan Review replan loop at the safety cap and leaves the task for a human", async () => {
|
||||
const store = createMockStore();
|
||||
const cappedLog = Array.from({ length: 15 }, (_, i) => revisionLog("Plan Review", "plan-review", i + 1));
|
||||
const loopingTask = task({ postReviewFixCount: 15, column: "in-progress", log: cappedLog });
|
||||
const loopingTask = task({
|
||||
postReviewFixCount: 15,
|
||||
column: "in-progress",
|
||||
log: cappedLog,
|
||||
workflowStepResults: [repeatedPlanReviewResult(16)],
|
||||
});
|
||||
store.getTask.mockResolvedValue(loopingTask);
|
||||
store.getSettings.mockResolvedValue({ maxPostReviewFixes: 9 }); // unbounded default
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
|
||||
@@ -795,6 +795,7 @@ async function issueRelease(
|
||||
live,
|
||||
stalenessMs: deps.now() - new Date(task.columnMovedAt ?? task.updatedAt).getTime(),
|
||||
graceMs: 60_000,
|
||||
now: deps.now(),
|
||||
});
|
||||
const key = `${task.id}:${task.column}`;
|
||||
if (stranded.stranded && !strandedHoldWarningMemo.has(key)) {
|
||||
|
||||
@@ -13,6 +13,7 @@ import type { TaskStore, TaskComment, AgentPromptsConfig, Settings } from "@fusi
|
||||
import {
|
||||
buildReviewerMemoryInstructions,
|
||||
hasConfiguredFallbackLane,
|
||||
PLAN_REVIEW_COMPLETENESS_POLICY,
|
||||
resolveAgentMemoryInclusionMode,
|
||||
resolveAgentPrompt,
|
||||
resolvePersistAgentThinkingLog,
|
||||
@@ -144,18 +145,6 @@ export interface ReviewOptions {
|
||||
pluginRunner?: import("../plugins/plugin-runner.js").PluginRunner;
|
||||
/** Allow this reviewer to fix in-scope findings in the same session before returning its final verdict. */
|
||||
allowInlineFixes?: boolean;
|
||||
/*
|
||||
FNXC:TriagePlanReviewConvergence 2026-07-16-09:20:
|
||||
Spec-gate-only convergence context. The triage Plan Review gate feeds the reviewer its OWN
|
||||
prior REVISE feedback plus the 1-based replan attempt so a re-review VERIFIES its earlier
|
||||
issues were addressed instead of surfacing a fresh, deeper blocking issue every cycle
|
||||
(whack-a-mole/goalpost movement burned all 8 replans on FN-7996/FN-8105/FN-8108). Only
|
||||
injected when reviewType === "spec" and attempt > 1; CODE review and normal PLAN review are
|
||||
unaffected because these fields stay undefined on those paths.
|
||||
*/
|
||||
priorSpecReviewFeedback?: string;
|
||||
/** 1-based current Plan Review attempt (= (task.planReviewReplanCount ?? 0) + 1). Spec gate only. */
|
||||
specReviewAttempt?: number;
|
||||
/**
|
||||
* Fired immediately after the reviewer's `AgentSession` is created. The
|
||||
* caller can register the session in a per-task subagent map so that the
|
||||
@@ -251,13 +240,8 @@ export async function reviewStep(
|
||||
&& reviewType !== "code"
|
||||
&& Boolean(options.store && options.taskId);
|
||||
|
||||
// FNXC:TriagePlanReviewConvergence 2026-07-16-09:20: spec-gate-only convergence context (see ReviewOptions).
|
||||
const specConvergence: SpecReviewConvergence | undefined =
|
||||
reviewType === "spec"
|
||||
? { priorFeedback: options.priorSpecReviewFeedback, attempt: options.specReviewAttempt }
|
||||
: undefined;
|
||||
let request = buildReviewRequest(
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments, specConvergence,
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments,
|
||||
);
|
||||
if (options.allowInlineFixes === true) {
|
||||
/*
|
||||
@@ -618,10 +602,8 @@ export async function reviewStep(
|
||||
}
|
||||
|
||||
reviewText = "";
|
||||
// FNXC:TriagePlanReviewConvergence 2026-07-16-09:20: reuse the single `specConvergence`
|
||||
// computed above so the context-limit retry carries byte-identical spec convergence context.
|
||||
let reducedRequest = buildReducedReviewRequest(
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments, specConvergence,
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments,
|
||||
);
|
||||
if (options.allowInlineFixes === true) {
|
||||
reducedRequest = appendSameSessionFixPolicy(reducedRequest, reviewType, canWritePromptInline);
|
||||
@@ -877,19 +859,6 @@ function buildReducedTaskPromptSummary(promptContent: string): string {
|
||||
return sections.join("\n\n").trim();
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TriagePlanReviewConvergence 2026-07-16-09:20:
|
||||
Spec-gate-only convergence context threaded into the review request. `priorFeedback` is the
|
||||
reviewer's own most recent Plan Review REVISE text; `attempt` is the 1-based replan attempt.
|
||||
Present only for reviewType === "spec"; drives the per-attempt convergence + severity-ratchet
|
||||
block injected into the request so the re-review confirms prior issues rather than moving the
|
||||
goalposts each cycle.
|
||||
*/
|
||||
interface SpecReviewConvergence {
|
||||
priorFeedback?: string;
|
||||
attempt?: number;
|
||||
}
|
||||
|
||||
function buildReducedReviewRequest(
|
||||
taskId: string,
|
||||
stepNumber: number,
|
||||
@@ -899,7 +868,6 @@ function buildReducedReviewRequest(
|
||||
cwd: string,
|
||||
baseline?: string,
|
||||
userComments?: TaskComment[],
|
||||
specConvergence?: SpecReviewConvergence,
|
||||
): string {
|
||||
/*
|
||||
FNXC:AgentSteering 2026-06-30-17:09:
|
||||
@@ -915,7 +883,6 @@ function buildReducedReviewRequest(
|
||||
cwd,
|
||||
baseline,
|
||||
userComments,
|
||||
specConvergence,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -928,7 +895,6 @@ function buildReviewRequest(
|
||||
cwd: string,
|
||||
baseline?: string,
|
||||
userComments?: TaskComment[],
|
||||
specConvergence?: SpecReviewConvergence,
|
||||
): string {
|
||||
const parts = [
|
||||
`Review request for task ${taskId}, Step ${stepNumber}: ${stepName}`,
|
||||
@@ -941,15 +907,15 @@ function buildReviewRequest(
|
||||
"",
|
||||
];
|
||||
|
||||
/*
|
||||
* FNXC:PlanReviewPromptBoundary 2026-08-04-06:35:
|
||||
* Only spec reviews receive the holistic Plan Review procedure; code reviews
|
||||
* use their dedicated production-reachability and implementation evidence gate.
|
||||
*/
|
||||
if (reviewType === "spec") {
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-15-11:15:
|
||||
Spec/Plan Review REVISE loops burn planner+reviewer turns when feedback is vague or
|
||||
when polish is treated as blocking. Prefer fix-and-APPROVE / Suggestions for non-
|
||||
blocking nits; when REVISE is required, list concrete PROMPT.md edits the planner can
|
||||
apply surgically so the next cycle converges.
|
||||
*/
|
||||
parts.push(
|
||||
PLAN_REVIEW_COMPLETENESS_POLICY,
|
||||
"",
|
||||
"## What to review",
|
||||
"Evaluate this PROMPT.md specification for completeness and quality.",
|
||||
"Assess against the spec quality criteria: mission clarity, step specificity/verifiability,",
|
||||
@@ -960,51 +926,8 @@ function buildReviewRequest(
|
||||
"Read relevant source files to verify the spec references real files, functions, and patterns.",
|
||||
"Check that steps have concrete, verifiable outcomes — not vague instructions.",
|
||||
"Ensure testing requirements demand real automated tests with assertions.",
|
||||
"",
|
||||
"## Convergence rules (blocking REVISE budget)",
|
||||
"- Prefer APPROVE or APPROVE_WITH_NOTES when the plan is executable; put polish and optional improvements under **Suggestions** only.",
|
||||
"- Issue REVISE only for blocking defects that would cause the implementor to redo work or violate a hard gate (missing Surface Enumeration / Symptom Verification for bug-class tasks, dangling task-document refs, untestable steps, missing mission, user comments ignored, external-integration evidence gaps when required).",
|
||||
"- When you REVISE, list each blocking fix as a concrete edit the planner can apply to this PROMPT.md (section + what to add/change). Do not request a full rewrite unless the approach is fundamentally wrong (RETHINK).",
|
||||
"- If same-session PROMPT.md repair is available and a fix is local, apply it and APPROVE rather than bouncing to another replan cycle.",
|
||||
);
|
||||
|
||||
/*
|
||||
FNXC:TriagePlanReviewConvergence 2026-07-16-09:20:
|
||||
On replan attempt > 1 the reviewer is re-reviewing a spec IT already rejected. Feed it the
|
||||
prior REVISE text and attempt number so it verifies those issues were addressed instead of
|
||||
surfacing a fresh deeper issue each cycle (the whack-a-mole that burned all 8 replans on
|
||||
FN-7996/FN-8105/FN-8108). At attempt >= 3, ratchet severity: gate only on delivery-blocking
|
||||
`critical` issues so a spec that is executable (executor + code review are later gates) does
|
||||
not loop on wording nits.
|
||||
*/
|
||||
const specAttempt = specConvergence?.attempt ?? 0;
|
||||
if (specAttempt > 1) {
|
||||
parts.push(
|
||||
"",
|
||||
`## Convergence — Plan Review attempt ${specAttempt}`,
|
||||
`You (the reviewer) already reviewed an earlier version of this spec; the planner has since revised the PROMPT.md above. This is attempt ${specAttempt}.`,
|
||||
"- VERIFY each issue you raised previously was addressed. REVISE only if (a) a PRIOR blocking issue is still unresolved, or (b) this revision introduced a GENUINELY NEW problem.",
|
||||
"- Do NOT introduce a new blocking issue that ALSO applied to the version you previously reviewed — that is your own earlier miss; record it under **Suggestions**, not REVISE.",
|
||||
);
|
||||
const priorFeedback = specConvergence?.priorFeedback?.trim();
|
||||
if (priorFeedback) {
|
||||
parts.push(
|
||||
"",
|
||||
"Your prior Plan Review feedback (confirm each item is resolved):",
|
||||
"```",
|
||||
priorFeedback,
|
||||
"```",
|
||||
);
|
||||
}
|
||||
if (specAttempt >= 3) {
|
||||
parts.push(
|
||||
"",
|
||||
"### Severity ratchet (attempt 3+)",
|
||||
"Gate ONLY on `critical` (delivery-blocking) issues. Downgrade lone `important`/`minor` spec-wording nits to **Suggestions** and APPROVE. Rationale: the executor and downstream code review are later gates; a spec need not be perfect to be executable.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Add user comment coverage check for spec reviews
|
||||
if (userComments && userComments.length > 0) {
|
||||
parts.push(
|
||||
|
||||
@@ -16,7 +16,7 @@ import { DEFAULT_PROVIDER_INSTANCE_ID, type ProviderInstanceRef, type TaskStore,
|
||||
import { getUnmetSchedulingDependencies } from "./scheduler.js";
|
||||
import type { ImplementationExit, ImplementationExitReporter } from "./executor/implementation-exit.js";
|
||||
import { emitWorkflowLifecycleEvent } from "@fusion/core";
|
||||
import { resolveTaskLifecycleColumns, resolveProjectColumnsForRoles, resolveWipTargetForTask, resolveTerminalColumns, RetryStormError, serializeRetryStormError, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, resolveWorkflowIrForTask, columnsWithFlag, evaluateForeachMergeProof, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveLifecycleColumns, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, isLiveSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveMaxConsecutiveToolFailureRetries, resolveConsecutiveToolFailureRetryBackoffMs, resolveConsecutiveToolFailureThreshold, resolveExecutorEscalationTarget, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, DEFAULT_MAX_POST_REVIEW_FIXES, COMPLETION_SUMMARY_NODE_ID, upsertWorkflowStepResult, AWAITING_APPROVAL_PAUSE_REASON, THINKING_LEVELS, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AgentStore, resolveExecutorFallbackModel, resolveValidatorFallbackModel, parseExplicitDuplicateMarker, nonExecutableDuplicateRedirectReason } from "@fusion/core";
|
||||
import { resolveTaskLifecycleColumns, resolveProjectColumnsForRoles, resolveWipTargetForTask, resolveTerminalColumns, RetryStormError, serializeRetryStormError, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, resolveWorkflowIrForTask, columnsWithFlag, evaluateForeachMergeProof, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveLifecycleColumns, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, isLiveSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveMaxConsecutiveToolFailureRetries, resolveConsecutiveToolFailureRetryBackoffMs, resolveConsecutiveToolFailureThreshold, resolveExecutorEscalationTarget, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, DEFAULT_MAX_POST_REVIEW_FIXES, COMPLETION_SUMMARY_NODE_ID, PLAN_REVIEW_GROUP_ID, upsertWorkflowStepResult, AWAITING_APPROVAL_PAUSE_REASON, THINKING_LEVELS, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AgentStore, resolveExecutorFallbackModel, resolveValidatorFallbackModel, parseExplicitDuplicateMarker, nonExecutableDuplicateRedirectReason } from "@fusion/core";
|
||||
import {
|
||||
BLOCKED_THRASH_LIMIT,
|
||||
buildExternalBlockMetadataPatch,
|
||||
@@ -103,6 +103,13 @@ import {
|
||||
type VerificationResult,
|
||||
} from "./execution/verification-utils.js";
|
||||
import { canonicalFusionBranchName, canonicalStepInstanceBranchName, generateWorktreeName, resolveTaskWorkingBranch } from "./worktree/worktree-names.js";
|
||||
import {
|
||||
collectPlanReviewFeedbackHistory,
|
||||
countPlanReviewRevisionAttempts,
|
||||
formatPlanReviewRevisionFeedback,
|
||||
nextPlanReviewAttemptCount,
|
||||
PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT,
|
||||
} from "./plan-review-feedback-history.js";
|
||||
import { resolveTaskWorktreePath, resolveWorktreesDir } from "./worktree/worktree-paths.js";
|
||||
import { Type, type Static } from "@earendil-works/pi-ai";
|
||||
import { describeModel, formatModelMarkerDetails, promptWithFallback, compactSessionContext } from "./pi.js";
|
||||
@@ -521,6 +528,38 @@ function optionalStepRevisionLogOutcome(details: string, key: string): string {
|
||||
return `${details}\n${OPTIONAL_STEP_REVISION_KEY_MARKER} ${key}`;
|
||||
}
|
||||
|
||||
function buildGraphPlanReviewConvergenceContext(
|
||||
task: Pick<Task, "workflowStepResults">,
|
||||
revisionKey: string,
|
||||
): string {
|
||||
// FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768): Retry numbering uses
|
||||
// the uncapped durable attempt ledger, while prompt prose uses the separately
|
||||
// bounded, deduplicated same-episode decision history.
|
||||
const priorAttemptCount = countPlanReviewRevisionAttempts(task.workflowStepResults, { revisionKey });
|
||||
const attempt = priorAttemptCount + 1;
|
||||
if (attempt <= 1) return "";
|
||||
|
||||
const history = collectPlanReviewFeedbackHistory(task.workflowStepResults, { revisionKey });
|
||||
const lines = [
|
||||
`## Convergence — Plan Review attempt ${attempt}`,
|
||||
"Treat the cumulative prior feedback below as a decision primer. Verify each prior blocker against the current PROMPT.md before looking for new findings.",
|
||||
"- Do not re-raise a resolved or semantically duplicate blocker.",
|
||||
"- A newly blocking finding must identify the revision that introduced it, the prior blocker that genuinely masked it, or why it is independently delivery-blocking for correctness, security, data safety, or executability. Record an earlier reviewer miss explicitly; never demote a critical defect merely because it was missed before.",
|
||||
];
|
||||
if (attempt >= 3) {
|
||||
lines.push(
|
||||
"- Severity ratchet (attempt 3+): only delivery-blocking critical defects may return REVISE; important/minor wording or implementation-detail findings are advisory.",
|
||||
);
|
||||
}
|
||||
if (history.length > 0) {
|
||||
lines.push("", "### Cumulative prior Plan Review ledger");
|
||||
history.forEach((feedback, index) => {
|
||||
lines.push(`#### PR${index + 1}`, feedback);
|
||||
});
|
||||
}
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
const STEP_STATUSES: StepStatus[] = ["pending", "in-progress", "done", "skipped"];
|
||||
|
||||
function canonicalizePath(path: string): string {
|
||||
@@ -5626,7 +5665,25 @@ export class TaskExecutor {
|
||||
return false;
|
||||
}
|
||||
const revisionKey = optionalStepRevisionKey(info.nodeId ?? "plan-review", info.stepName);
|
||||
const currentCount = countOptionalStepRevisionAttempts(liveTask, revisionKey, info.stepName);
|
||||
// FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768): The terminal
|
||||
// result is persisted before remediation. Budget from the durable raw
|
||||
// same-episode count, not the capped prompt history or cross-episode log.
|
||||
const currentEpisodeAttemptCount = countPlanReviewRevisionAttempts(
|
||||
liveTask.workflowStepResults,
|
||||
{ revisionKey },
|
||||
);
|
||||
const matchingProjection = liveTask.workflowStepResults?.find((result) =>
|
||||
result.workflowStepId === revisionKey
|
||||
|| (revisionKey === PLAN_REVIEW_GROUP_ID && result.workflowStepName === "Plan Review"),
|
||||
);
|
||||
const hasEpisodeBoundary = matchingProjection?.supersededAt != null
|
||||
|| matchingProjection?.priorAttempts?.some((attempt) => attempt.supersededAt != null) === true;
|
||||
const nextCount = currentEpisodeAttemptCount > 0
|
||||
? currentEpisodeAttemptCount
|
||||
: hasEpisodeBoundary
|
||||
? 1
|
||||
: countOptionalStepRevisionAttempts(liveTask, revisionKey, info.stepName) + 1;
|
||||
const currentCount = nextCount - 1;
|
||||
if (!budget.unbounded && currentCount >= budget.max) {
|
||||
// U3: finite replan budget exhausted → park awaiting-approval (cap park
|
||||
// re-owned from the deleted triage gate), not a silent leave-in-place.
|
||||
@@ -5639,20 +5696,18 @@ export class TaskExecutor {
|
||||
* FNXC:PlanReviewReplanCap 2026-07-05-17:28:
|
||||
* FN-7561: an unset Plan Review revision budget resolves to "unbounded" (see FNXC:WorkflowRevisionBudget above), which by design skips the ceiling check — so a task whose planner and reviewer persistently disagree, or whose reviewer keeps hard-failing, replans triage↔plan-review forever, silently burning a triage + review LLM call every cycle (FN-7525 ran 13+ attempts overnight with zero operator visibility). Enforce a finite safety ceiling even when unbounded: once hit, emit a loud halting log entry and STOP replanning (return false) so the gate falls through to a visible failed/parked state a human can act on, instead of looping indefinitely. Explicit numeric operator budgets are still honored as-is above; this only backstops the unbounded DEFAULT.
|
||||
*/
|
||||
const PLAN_REVIEW_REPLAN_HARD_CAP = 15;
|
||||
if (budget.unbounded && currentCount >= PLAN_REVIEW_REPLAN_HARD_CAP) {
|
||||
if (budget.unbounded && currentCount >= PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT) {
|
||||
// U3: the unbounded-default safety ceiling now parks awaiting-approval with
|
||||
// the replan-cap reason (re-owned from the deleted triage gate) so the
|
||||
// non-convergence surfaces to a human instead of silently sitting in place.
|
||||
await this.parkPlanReviewReplanCapExhausted(
|
||||
taskId,
|
||||
String(PLAN_REVIEW_REPLAN_HARD_CAP),
|
||||
String(PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT),
|
||||
currentCount,
|
||||
feedback,
|
||||
);
|
||||
return true;
|
||||
}
|
||||
const nextCount = currentCount + 1;
|
||||
const totalFixCount = (liveTask.postReviewFixCount ?? 0) + 1;
|
||||
const budgetLabel = budget.unbounded ? "unbounded" : String(budget.max);
|
||||
await this.store.updateTask(taskId, { postReviewFixCount: totalFixCount }, this.getRunContextFor(taskId));
|
||||
@@ -5660,7 +5715,7 @@ export class TaskExecutor {
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
"AI spec revision requested",
|
||||
`Plan Review requested a planning revision before execution.\n\nStatus: ${info.status}\nFeedback:\n${feedback}`,
|
||||
formatPlanReviewRevisionFeedback(revisionKey, info.status, feedback),
|
||||
this.getRunContextFor(taskId),
|
||||
);
|
||||
/*
|
||||
@@ -6691,7 +6746,22 @@ export class TaskExecutor {
|
||||
fix) must preserve the prior `status:"failed"` entry's history in
|
||||
`priorAttempts` rather than silently overwriting it.
|
||||
*/
|
||||
const existing = upsertWorkflowStepResult(live?.workflowStepResults, result);
|
||||
const isPlanReviewResult = result.workflowStepId === PLAN_REVIEW_GROUP_ID
|
||||
|| result.workflowStepName === "Plan Review";
|
||||
const resultToPersist = isPlanReviewResult
|
||||
? {
|
||||
...result,
|
||||
planReviewAttemptCount: nextPlanReviewAttemptCount(
|
||||
live?.workflowStepResults?.find((existing) => existing.workflowStepId === result.workflowStepId),
|
||||
result,
|
||||
),
|
||||
}
|
||||
: result;
|
||||
const existing = upsertWorkflowStepResult(
|
||||
live?.workflowStepResults,
|
||||
resultToPersist,
|
||||
isPlanReviewResult ? { maxPriorAttempts: PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT } : undefined,
|
||||
);
|
||||
await this.store.updateTask(taskId, { workflowStepResults: existing }, this.getRunContextFor(taskId));
|
||||
} catch {
|
||||
// Result recording is additive visibility — never affect the run.
|
||||
@@ -18801,13 +18871,16 @@ ${scopeGuard}
|
||||
// assumptions and proceed instead of parking on a question. Explicit opt-in
|
||||
// only (default false = board run); see runGraphCustomNode / KTD-3.
|
||||
const unattended = stepOptions?.unattended === true;
|
||||
const isPlanReviewStep = workflowStep.id === "graph:plan-review-step" || workflowStep.name === "Plan Review";
|
||||
const workflowStepMetadata = workflowStep as WorkflowStep & {
|
||||
optionalGroupId?: string;
|
||||
reviewCanFixInline?: boolean;
|
||||
requireExternalIntegrationEvidence?: boolean;
|
||||
};
|
||||
const optionalGroupId = workflowStepMetadata.optionalGroupId;
|
||||
const isPlanReviewStep = workflowStep.id === "graph:plan-review-step"
|
||||
|| workflowStep.name === "Plan Review"
|
||||
|| optionalGroupId === PLAN_REVIEW_GROUP_ID;
|
||||
const planReviewRevisionKey = optionalStepRevisionKey(optionalGroupId, workflowStep.name);
|
||||
const isReviewTypeWorkflowStep =
|
||||
isPlanReviewStep
|
||||
|| workflowStepMetadata.reviewCanFixInline === true
|
||||
@@ -18849,6 +18922,9 @@ ${scopeGuard}
|
||||
}
|
||||
const workflowReviewSpecText = typeof workflowReviewSpecArtifact === "string" ? workflowReviewSpecArtifact : "";
|
||||
const planReviewSpecText = isPlanReviewStep ? workflowReviewSpecText : "";
|
||||
const planReviewConvergenceContext = isPlanReviewStep
|
||||
? buildGraphPlanReviewConvergenceContext(task, planReviewRevisionKey)
|
||||
: "";
|
||||
|
||||
/*
|
||||
FNXC:PlanReview 2026-07-21-16:30:
|
||||
@@ -18951,14 +19027,15 @@ ${workflowReviewSpecText}
|
||||
|
||||
--- BEGIN PROMPT.md ---
|
||||
${planReviewSpecText}
|
||||
--- END PROMPT.md ---`
|
||||
--- END PROMPT.md ---${planReviewConvergenceContext ? `\n\n${planReviewConvergenceContext}` : ""}`
|
||||
: `Diff Scope (files changed by THIS task vs base):
|
||||
${scopeFileBlock}${diffShortstat ? `\nDiff stat: ${diffShortstat}` : ""}
|
||||
|
||||
CRITICAL SCOPING RULES — read before doing anything else:
|
||||
- Review ONLY the files listed above. Do NOT analyze unmodified files or unrelated parts of the codebase.
|
||||
- If NONE of the files in the diff scope are relevant to your review category (e.g. a UX/design reviewer with no UI/CSS/component files in scope, a security reviewer with no auth/network code in scope, an a11y reviewer with no markup changes), respond IMMEDIATELY with a single short approval line such as "No relevant changes in scope — approved." and STOP. Do not start exploring the codebase.
|
||||
- Your wall-clock budget is short. Spending it browsing unmodified files will cause this step to time out and block merge.${approvedContractBlock}`;
|
||||
- The modified-file list is the starting point and primary reporting scope, not a prohibition on reading code required to validate the change.
|
||||
- Read necessary callers, selectors, shared helpers, consumers, and tests outside that list when they establish production reachability, invariant coverage, or API/UI parity. Do not report unrelated pre-existing issues.
|
||||
- If NONE of the modified files are relevant to your review category, confirm that from the list and fast-bail without broad repository exploration.
|
||||
- Keep adjacent reads bounded to the changed behavior and its immediate production/test chain so the review finishes within its wall-clock budget.${approvedContractBlock}`;
|
||||
|
||||
const latestTaskForUserComments = await this.store.getTask(task.id).catch(() => task);
|
||||
const workflowStepUserComments = selectUserCommentsForAgentContext(latestTaskForUserComments, { limit: null });
|
||||
@@ -19015,7 +19092,8 @@ This review-type node may fix issues it finds before returning a final verdict.
|
||||
- If you find an in-scope issue you can fix safely, edit the relevant files in this same session, run the smallest relevant verification, and then return APPROVE or APPROVE_WITH_NOTES.
|
||||
- Return REVISE only when the issue is still present, cannot be safely fixed in this reviewer session, needs broader executor remediation, or needs user input.
|
||||
- Plan Review may use fn_task_prompt_write to replace the task's PROMPT.md with the complete revised plan. Do not implement product code from Plan Review.
|
||||
- Code Review and Browser Verification may fix implementation issues inside the assigned task worktree and should mention the fix in notes.`
|
||||
- Code Review and Browser Verification may fix implementation issues inside the assigned task worktree and should mention the fix in notes.
|
||||
- After any inline edit, treat your own change as untrusted: re-read the fresh diff, restart the mandatory review procedure from its requirements ledger and production-reachability checks, and rerun the smallest relevant verification. Never approve solely because the local fix compiles or its narrow test passes.`
|
||||
: "";
|
||||
|
||||
const systemPrompt = `You are a workflow step agent executing: ${workflowStep.name}
|
||||
|
||||
@@ -10,10 +10,15 @@ import {
|
||||
type WorkflowWorkItem,
|
||||
} from "@fusion/core";
|
||||
import { resolvePreReleasePlanReviewNode } from "./execution/hold-release.js";
|
||||
import {
|
||||
classifyPersistedPlanHandoff,
|
||||
LEGACY_NULL_PLAN_HANDOFF_STALE_MS,
|
||||
} from "./planning-handoff-recovery.js";
|
||||
|
||||
export type StrandedHoldContinuationReason =
|
||||
| "not-hold-column" | "no-pre-release-review" | "active-continuation"
|
||||
| "plan-review-passed" | "seed-prompt" | "prompt-missing" | "triage-owned"
|
||||
| "planning-recovery-owned"
|
||||
| "awaiting-approval"
|
||||
| "paused" | "engine-paused" | "live" | "too-fresh" | "auto-merge-off" | "ready";
|
||||
|
||||
@@ -91,8 +96,12 @@ export async function seedPreReleasePlanReviewContinuation(
|
||||
* tokens represent an audit-worthy race loss from the conditional store op.
|
||||
*/
|
||||
export function evaluateStrandedHoldContinuation(input: {
|
||||
task: Pick<Task, "id" | "title" | "description" | "column" | "status" | "paused" | "userPaused" | "pausedReason">;
|
||||
columnFlags: { hold?: boolean };
|
||||
task: Pick<Task,
|
||||
| "id" | "title" | "description" | "column" | "status" | "paused" | "userPaused" | "pausedReason"
|
||||
| "approvedPlanFingerprint" | "awaitingApprovalReason" | "workflowStepResults" | "updatedAt" | "steps"
|
||||
| "worktree" | "firstExecutionAt" | "executionStartedAt"
|
||||
>;
|
||||
columnFlags: { hold?: boolean; intake?: boolean };
|
||||
ir: WorkflowIr;
|
||||
continuations: WorkflowWorkItem[];
|
||||
stepResults: Task["workflowStepResults"];
|
||||
@@ -102,6 +111,7 @@ export function evaluateStrandedHoldContinuation(input: {
|
||||
live: boolean;
|
||||
stalenessMs: number;
|
||||
graceMs: number;
|
||||
now?: number;
|
||||
}): { stranded: boolean; candidate: boolean; reason: StrandedHoldContinuationReason } {
|
||||
if (!input.columnFlags.hold) return { stranded: false, candidate: false, reason: "not-hold-column" };
|
||||
const review = resolvePreReleasePlanReviewNode(input.ir);
|
||||
@@ -112,6 +122,22 @@ export function evaluateStrandedHoldContinuation(input: {
|
||||
if (isUnplannedSeedPrompt(input.promptContent, input.task.id, input.task.title, input.task.description)) return { stranded: false, candidate: false, reason: "seed-prompt" };
|
||||
if (input.task.status === "planning" || input.task.status === "needs-replan") return { stranded: false, candidate: false, reason: "triage-owned" };
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-04:10:
|
||||
A pre-U11 planning handoff can have a real PROMPT, persisted parsed steps, and
|
||||
null status before lifecycle recovery publishes approval/continuation state.
|
||||
On a merged intake+hold lane that shape also looks like a stranded Plan Review
|
||||
continuation. Give the conservative legacy shape to exactly one owner: planning
|
||||
lifecycle recovery. Ordinary null-status held cards remain continuation-owned.
|
||||
*/
|
||||
if (input.columnFlags.intake && classifyPersistedPlanHandoff(input.task, {
|
||||
now: input.now ?? Date.now(),
|
||||
hasLivePlanningWork: input.live,
|
||||
legacyStaleMs: LEGACY_NULL_PLAN_HANDOFF_STALE_MS,
|
||||
requirePersistedSteps: true,
|
||||
}) === "legacy-null") {
|
||||
return { stranded: false, candidate: false, reason: "planning-recovery-owned" };
|
||||
}
|
||||
/*
|
||||
FNXC:PlanApprovalHold 2026-07-27-19:30 (U7 / R4):
|
||||
An approval-held card is not stranded — it is exactly where the operator's
|
||||
pending decision left it, so re-seeding would run Plan Review on an unapproved
|
||||
|
||||
144
packages/engine/src/plan-review-feedback-history.ts
Normal file
144
packages/engine/src/plan-review-feedback-history.ts
Normal file
@@ -0,0 +1,144 @@
|
||||
const PLAN_REVIEW_REVISION_SOURCE_MARKER = "Revision source: plan-review/";
|
||||
const LEGACY_PLAN_REVIEW_FEEDBACK_PREFIX = "Plan Review requested a planning revision before execution.";
|
||||
|
||||
type ReviewResultLike = {
|
||||
workflowStepId?: string;
|
||||
workflowStepName?: string;
|
||||
verdict?: string;
|
||||
status?: string;
|
||||
output?: string;
|
||||
notes?: string;
|
||||
startedAt?: string;
|
||||
supersededAt?: string;
|
||||
planReviewAttemptCount?: number;
|
||||
priorAttempts?: ReviewResultLike[];
|
||||
};
|
||||
|
||||
type RevisionLogEntry = {
|
||||
action?: string;
|
||||
outcome?: string;
|
||||
};
|
||||
|
||||
// FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768): Prompt history is
|
||||
// bounded independently of persistence and retry accounting. Persisted attempts
|
||||
// remain the raw budget ledger; only reviewer-facing prose is capped here.
|
||||
export const PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT = 15;
|
||||
|
||||
function normalizeRevisionKey(value: string | undefined): string {
|
||||
return (value ?? "").trim().toLowerCase();
|
||||
}
|
||||
|
||||
function feedbackText(result: ReviewResultLike): string | undefined {
|
||||
const text = result.notes?.trim() || result.output?.trim();
|
||||
return text || undefined;
|
||||
}
|
||||
|
||||
function isPlanReviewRevision(result: ReviewResultLike): boolean {
|
||||
return result.verdict === "REVISE";
|
||||
}
|
||||
|
||||
/*
|
||||
* FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768):
|
||||
* `priorAttempts` is persisted newest-first, so reverse the retained slice for
|
||||
* chronological rendering. A superseded attempt is the planning-episode
|
||||
* boundary: it and every older snapshot are excluded from both prose and count.
|
||||
*/
|
||||
function sameEpisodeRevisionAttempts(
|
||||
result: ReviewResultLike,
|
||||
includeCurrent: boolean,
|
||||
): ReviewResultLike[] {
|
||||
const newestFirst: ReviewResultLike[] = [];
|
||||
for (const attempt of result.priorAttempts ?? []) {
|
||||
if (attempt.supersededAt) break;
|
||||
if (isPlanReviewRevision(attempt)) newestFirst.push(attempt);
|
||||
}
|
||||
const oldestFirst = newestFirst.reverse();
|
||||
if (includeCurrent && isPlanReviewRevision(result)) oldestFirst.push(result);
|
||||
return oldestFirst;
|
||||
}
|
||||
|
||||
function matchesPlanReviewResult(result: ReviewResultLike, requestedKey: string): boolean {
|
||||
const resultKey = normalizeRevisionKey(result.workflowStepId);
|
||||
const isNamedPlanReview = normalizeRevisionKey(result.workflowStepName) === "plan review";
|
||||
return requestedKey ? resultKey === requestedKey : isNamedPlanReview || resultKey === "plan-review";
|
||||
}
|
||||
|
||||
export function formatPlanReviewRevisionFeedback(revisionKey: string, status: string, feedback: string): string {
|
||||
return `${PLAN_REVIEW_REVISION_SOURCE_MARKER}${revisionKey}\n${LEGACY_PLAN_REVIEW_FEEDBACK_PREFIX}\n\nStatus: ${status}\nFeedback:\n${feedback}`;
|
||||
}
|
||||
|
||||
export function isPlanReviewRevisionLog(entry: RevisionLogEntry): boolean {
|
||||
const outcome = entry.outcome?.trim() ?? "";
|
||||
return entry.action === "AI spec revision requested"
|
||||
&& (outcome.startsWith(PLAN_REVIEW_REVISION_SOURCE_MARKER) || outcome.startsWith(LEGACY_PLAN_REVIEW_FEEDBACK_PREFIX));
|
||||
}
|
||||
|
||||
/** Read same-episode Plan Review decisions for a bounded reviewer prompt. */
|
||||
export function collectPlanReviewFeedbackHistory(
|
||||
results: ReviewResultLike[] | undefined,
|
||||
options: { revisionKey?: string; exclude?: string; includeCurrent?: boolean } = {},
|
||||
): string[] {
|
||||
const requestedKey = normalizeRevisionKey(options.revisionKey);
|
||||
const excluded = options.exclude?.trim();
|
||||
const seen = new Set<string>();
|
||||
const history: string[] = [];
|
||||
|
||||
for (const result of results ?? []) {
|
||||
if (result.supersededAt) continue;
|
||||
if (!matchesPlanReviewResult(result, requestedKey)) continue;
|
||||
|
||||
for (const attempt of sameEpisodeRevisionAttempts(result, options.includeCurrent !== false)) {
|
||||
const feedback = feedbackText(attempt);
|
||||
if (!feedback || feedback === excluded || seen.has(feedback)) continue;
|
||||
seen.add(feedback);
|
||||
history.push(feedback);
|
||||
}
|
||||
}
|
||||
|
||||
return history.slice(-PLAN_REVIEW_FEEDBACK_HISTORY_LIMIT);
|
||||
}
|
||||
|
||||
// FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768): Do not reuse the
|
||||
// rendering cap here. Revision admission must count every same-episode REVISE,
|
||||
// including duplicate prose, or attempts beyond the prompt window evade budgets.
|
||||
export function countPlanReviewRevisionAttempts(
|
||||
results: ReviewResultLike[] | undefined,
|
||||
options: { revisionKey?: string; includeCurrent?: boolean } = {},
|
||||
): number {
|
||||
const requestedKey = normalizeRevisionKey(options.revisionKey);
|
||||
let count = 0;
|
||||
for (const result of results ?? []) {
|
||||
if (result.supersededAt || !matchesPlanReviewResult(result, requestedKey)) continue;
|
||||
const persistedCount = result.planReviewAttemptCount;
|
||||
if (typeof persistedCount === "number" && Number.isInteger(persistedCount) && persistedCount >= 0) {
|
||||
const excludesCurrentRevision = options.includeCurrent === false && isPlanReviewRevision(result);
|
||||
count += Math.max(0, persistedCount - (excludesCurrentRevision ? 1 : 0));
|
||||
continue;
|
||||
}
|
||||
count += sameEpisodeRevisionAttempts(result, options.includeCurrent !== false).length;
|
||||
}
|
||||
return count;
|
||||
}
|
||||
|
||||
/*
|
||||
* FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768):
|
||||
* Advance the durable Plan Review counter while a result is upserted.
|
||||
* A pending result carries the completed-revision count forward; its terminal
|
||||
* REVISE transition increments once. Repeated writes for the same terminal
|
||||
* attempt are idempotent, and a superseded projection starts a new episode.
|
||||
*/
|
||||
export function nextPlanReviewAttemptCount(
|
||||
previous: ReviewResultLike | undefined,
|
||||
incoming: ReviewResultLike,
|
||||
): number {
|
||||
const sameAttempt = previous?.startedAt !== undefined
|
||||
&& incoming.startedAt !== undefined
|
||||
&& previous.startedAt === incoming.startedAt;
|
||||
const previousCount = previous?.supersededAt
|
||||
? 0
|
||||
: countPlanReviewRevisionAttempts(previous ? [previous] : undefined);
|
||||
|
||||
if (!isPlanReviewRevision(incoming)) return previousCount;
|
||||
if (sameAttempt && previous && isPlanReviewRevision(previous)) return previousCount;
|
||||
return previousCount + 1;
|
||||
}
|
||||
51
packages/engine/src/planning-handoff-recovery.ts
Normal file
51
packages/engine/src/planning-handoff-recovery.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
import { isPlanReviewSatisfied, type Task } from "@fusion/core";
|
||||
|
||||
export const LEGACY_NULL_PLAN_HANDOFF_STALE_MS = 30 * 60 * 1000;
|
||||
|
||||
export type PersistedPlanHandoffKind = "planning" | "approved-null" | "legacy-null";
|
||||
|
||||
/**
|
||||
* Shared persisted-state classifier for planning handoff recovery. It deliberately
|
||||
* excludes graph work-item/step-instance evidence, which callers must check at
|
||||
* their own store boundary before acting on a `legacy-null` result.
|
||||
*/
|
||||
export function classifyPersistedPlanHandoff(
|
||||
task: Pick<Task,
|
||||
| "status"
|
||||
| "paused"
|
||||
| "userPaused"
|
||||
| "approvedPlanFingerprint"
|
||||
| "awaitingApprovalReason"
|
||||
| "workflowStepResults"
|
||||
| "updatedAt"
|
||||
| "steps"
|
||||
| "worktree"
|
||||
| "firstExecutionAt"
|
||||
| "executionStartedAt"
|
||||
>,
|
||||
options: {
|
||||
now: number;
|
||||
hasLivePlanningWork: boolean;
|
||||
legacyStaleMs?: number;
|
||||
requirePersistedSteps?: boolean;
|
||||
},
|
||||
): PersistedPlanHandoffKind | null {
|
||||
if (task.paused || task.userPaused || options.hasLivePlanningWork) return null;
|
||||
// FNXC:PlanningHandoffRecovery 2026-08-04-06:35 (FN-8768): Manual approval
|
||||
// parks and execution evidence outrank stale planning projections. In particular,
|
||||
// a retained Plan Review approval must never make an operator-held or already-
|
||||
// executing task eligible for planning-handoff recovery.
|
||||
if (task.awaitingApprovalReason) return null;
|
||||
if (task.worktree || task.firstExecutionAt || task.executionStartedAt) return null;
|
||||
if (task.status === "planning") return "planning";
|
||||
if (task.status != null) return null;
|
||||
if (task.workflowStepResults?.some(isPlanReviewSatisfied)) return "approved-null";
|
||||
if (task.approvedPlanFingerprint != null) return null;
|
||||
if (task.workflowStepResults?.length) return null;
|
||||
if (options.requirePersistedSteps && !task.steps?.length) return null;
|
||||
|
||||
const staleMs = options.legacyStaleMs ?? 0;
|
||||
const updatedAt = new Date(task.updatedAt).getTime();
|
||||
if (!Number.isFinite(updatedAt) || options.now - updatedAt < staleMs) return null;
|
||||
return "legacy-null";
|
||||
}
|
||||
@@ -77,6 +77,7 @@ import { finalizeProvenAutoMergeTask, validateWorkflowDoneMergeProof } from "./m
|
||||
import { AutoRecoveryDispatcher } from "./healing/auto-recovery.js";
|
||||
import { activeSessionRegistry, executingTaskLock } from "./agents/active-session-registry.js";
|
||||
import { isTaskStillInPlanningStage } from "./execution/replan-target.js";
|
||||
import { classifyPersistedPlanHandoff, LEGACY_NULL_PLAN_HANDOFF_STALE_MS } from "./planning-handoff-recovery.js";
|
||||
import { getPromptPath } from "./execution/spec-staleness.js";
|
||||
import { evaluateStrandedHoldContinuation, seedPreReleasePlanReviewContinuation } from "./plan-review-continuation.js";
|
||||
/*
|
||||
@@ -7403,6 +7404,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
stepResults: task.workflowStepResults, effectiveSettings: { autoMerge: resolveEffectiveAutoMerge(task, freshSettings) },
|
||||
enginePaused: freshEnginePaused, promptContent, live: live(task.id),
|
||||
stalenessMs: Date.now() - new Date(task.columnMovedAt ?? task.updatedAt).getTime(), graceMs,
|
||||
now: Date.now(),
|
||||
}),
|
||||
};
|
||||
};
|
||||
@@ -7420,19 +7422,30 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
if (type.endsWith("no-action")) this.strandedHoldContinuationNoActionAudited.add(key);
|
||||
};
|
||||
if (!initial.result.stranded) { await audit("task:reconcile-stranded-hold-continuation-no-action", initial.result.reason); continue; }
|
||||
// FNXC:StrandedHoldContinuation 2026-07-26-14:15:
|
||||
// Re-read every predicate input immediately before the atomic insert.
|
||||
// This reduces stale pause/liveness/settings decisions; the shared
|
||||
// store lock remains the correctness guard for continuation/results.
|
||||
const fresh = await evaluate(snapshot.id);
|
||||
if (!fresh || !fresh.result.stranded) {
|
||||
if (fresh) await audit("task:reconcile-stranded-hold-continuation-no-action", fresh.result.reason, fresh);
|
||||
continue;
|
||||
}
|
||||
const seeded = await seedPreReleasePlanReviewContinuation(this.store, fresh.task, fresh.ir, { atomic: true });
|
||||
if (!seeded.seeded) { await audit("task:reconcile-stranded-hold-continuation-no-action", seeded.reason, fresh); continue; }
|
||||
repaired += 1;
|
||||
await audit("task:reconcile-stranded-hold-continuation", undefined, fresh);
|
||||
// FNXC:PlanningDependencyReseed 2026-08-04-04:10:
|
||||
// Re-read and seed under the same cross-process lifecycle lock used
|
||||
// by dependency invalidation and planning finalization. A dependency
|
||||
// mutation that wins first is therefore visible to this predicate;
|
||||
// one that wins second supersedes the continuation after it commits.
|
||||
const reconcileUnderLifecycleLock = async () => {
|
||||
const fresh = await evaluate(snapshot.id);
|
||||
if (!fresh || !fresh.result.stranded) {
|
||||
if (fresh) await audit("task:reconcile-stranded-hold-continuation-no-action", fresh.result.reason, fresh);
|
||||
return false;
|
||||
}
|
||||
const seeded = await seedPreReleasePlanReviewContinuation(this.store, fresh.task, fresh.ir, { atomic: true });
|
||||
if (!seeded.seeded) {
|
||||
await audit("task:reconcile-stranded-hold-continuation-no-action", seeded.reason, fresh);
|
||||
return false;
|
||||
}
|
||||
await audit("task:reconcile-stranded-hold-continuation", undefined, fresh);
|
||||
return true;
|
||||
};
|
||||
const lifecycleLock = (this.store as Partial<TaskStore>).withPlanningLifecycleLock;
|
||||
const seeded = lifecycleLock
|
||||
? await lifecycleLock.call(this.store, snapshot.id, reconcileUnderLifecycleLock)
|
||||
: await reconcileUnderLifecycleLock();
|
||||
if (seeded) repaired += 1;
|
||||
} catch (error) { log.warn(`reconcileStrandedHoldContinuations: failed for ${snapshot.id}: ${error instanceof Error ? error.message : String(error)}`); }
|
||||
}
|
||||
if (tasks.length < 500) break;
|
||||
@@ -14375,12 +14388,16 @@ const movedTask = await this.store.moveTask(task.id, completeLane);
|
||||
const planningIds = this.options.getPlanningTaskIds?.() ?? new Set<string>();
|
||||
const now = Date.now();
|
||||
|
||||
const orphanedApproved = tasks.filter((t) =>
|
||||
t.status === "planning" &&
|
||||
!t.paused &&
|
||||
!planningIds.has(t.id) &&
|
||||
now - new Date(t.updatedAt).getTime() >= APPROVED_TRIAGE_RECOVERY_GRACE_MS
|
||||
);
|
||||
const orphanedApproved = tasks.filter((task) => {
|
||||
const handoffKind = classifyPersistedPlanHandoff(task, {
|
||||
now,
|
||||
hasLivePlanningWork: planningIds.has(task.id),
|
||||
legacyStaleMs: LEGACY_NULL_PLAN_HANDOFF_STALE_MS,
|
||||
requirePersistedSteps: true,
|
||||
});
|
||||
return handoffKind != null
|
||||
&& now - new Date(task.updatedAt).getTime() >= APPROVED_TRIAGE_RECOVERY_GRACE_MS;
|
||||
});
|
||||
|
||||
if (orphanedApproved.length === 0) return 0;
|
||||
|
||||
@@ -14395,7 +14412,17 @@ const movedTask = await this.store.moveTask(task.id, completeLane);
|
||||
// Narrow test/legacy adapters can return an unrelated fixture row; only use a
|
||||
// re-read when it identifies the requested candidate.
|
||||
const recoveryTask = live?.id === task.id ? live : task;
|
||||
if (!isTaskStillInPlanningStage(recoveryTask)) continue;
|
||||
const handoffKind = classifyPersistedPlanHandoff(recoveryTask, {
|
||||
now,
|
||||
hasLivePlanningWork: planningIds.has(recoveryTask.id),
|
||||
legacyStaleMs: LEGACY_NULL_PLAN_HANDOFF_STALE_MS,
|
||||
requirePersistedSteps: true,
|
||||
});
|
||||
if (!handoffKind) continue;
|
||||
// The legacy null handoff deliberately contains parsed task steps; the
|
||||
// generic planning-stage predicate interprets null+steps as execution
|
||||
// progress, so its exact classifier owns that one compatibility shape.
|
||||
if (handoffKind !== "legacy-null" && !isTaskStillInPlanningStage(recoveryTask)) continue;
|
||||
log.log(`Recovering specified triage task ${task.id}: ${task.title || task.description?.slice(0, 60) || "(untitled)"}`);
|
||||
const success = await recoverFn(recoveryTask);
|
||||
if (success) recovered++;
|
||||
|
||||
@@ -42,7 +42,6 @@ import {
|
||||
workflowHasColumn,
|
||||
getStepParser,
|
||||
computePlanApprovalFingerprint,
|
||||
isPlanReviewSatisfied,
|
||||
extractIntentSignature,
|
||||
findNearDuplicates,
|
||||
isNearDuplicateCanonicalInactive, resolveColumnFlags,
|
||||
@@ -135,6 +134,7 @@ import type {
|
||||
} from "@earendil-works/pi-coding-agent";
|
||||
import { ModelFallbackExhaustedError, describeModel, formatModelMarkerDetails, promptWithFallback } from "./pi.js";
|
||||
import { hasAdvancedPastPlanning, isTaskStillInPlanningStage, resolvePlannerLanesForTaskAsync } from "./execution/replan-target.js";
|
||||
import { classifyPersistedPlanHandoff, LEGACY_NULL_PLAN_HANDOFF_STALE_MS } from "./planning-handoff-recovery.js";
|
||||
import {
|
||||
createResolvedAgentSession,
|
||||
extractRuntimeHint,
|
||||
@@ -228,6 +228,7 @@ import { createRunAuditor, generateSyntheticRunId } from "./util/run-audit.js";
|
||||
import { resolveAndEmitGoalContext } from "./goals/goal-injection-diagnostics.js";
|
||||
import { accumulateSessionTokenUsage } from "./execution/session-token-usage.js";
|
||||
import { finalizePlanningSegment, startPlanningSegment } from "@fusion/core";
|
||||
import { collectPlanReviewFeedbackHistory, isPlanReviewRevisionLog } from "./plan-review-feedback-history.js";
|
||||
import type { AgentActionGateContext } from "./agents/agent-action-gate.js";
|
||||
import { buildAgentGatedActionSummary } from "./agents/permanent-agent-gating.js";
|
||||
|
||||
@@ -1188,7 +1189,7 @@ export class TriageProcessor {
|
||||
* protected regardless of elapsed time; a stuck-aborted session is still
|
||||
* reclaimable because its promise may never reach the cleanup `finally`.
|
||||
*/
|
||||
private static readonly STALE_PROCESSING_THRESHOLD_MS = 30 * 60 * 1000;
|
||||
private static readonly STALE_PROCESSING_THRESHOLD_MS = LEGACY_NULL_PLAN_HANDOFF_STALE_MS;
|
||||
|
||||
/**
|
||||
* Evict stale tasks from `processing` only when their triage promise is no
|
||||
@@ -1260,15 +1261,6 @@ export class TriageProcessor {
|
||||
return evicted;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewApproval 2026-08-04-00:26:
|
||||
Recovery treats an audited operator acceptance as terminal Plan Review evidence, without
|
||||
fabricating a reviewer pass or allowing an unaudited skip to release the task.
|
||||
*/
|
||||
private hasSatisfiedPlanReview(task: Pick<Task, "workflowStepResults">): boolean {
|
||||
return task.workflowStepResults?.some(isPlanReviewSatisfied) === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover a triage task whose PROMPT.md was already written but the final
|
||||
* handoff out of planning never completed.
|
||||
@@ -1289,8 +1281,6 @@ export class TriageProcessor {
|
||||
the validation below still rejects seeds/partial plans and finalization
|
||||
evaluates manual approval before graph continuation.
|
||||
*/
|
||||
const hasNoPlanningHandoffEvidence = task.approvedPlanFingerprint == null
|
||||
&& !(task.workflowStepResults?.length);
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-01:04:
|
||||
Null status alone is not a planning handoff. Claim the legacy reseed hole
|
||||
@@ -1300,15 +1290,18 @@ export class TriageProcessor {
|
||||
*/
|
||||
const continuationReader = (this.store as Partial<Pick<TaskStore, "listWorkflowWorkItemsForTask">>).listWorkflowWorkItemsForTask;
|
||||
const stepInstanceReader = (this.store as Partial<Pick<TaskStore, "hasWorkflowRunStepInstancesForTask">>).hasWorkflowRunStepInstancesForTask;
|
||||
const legacyNullStatusCandidate = task.status == null
|
||||
&& hasNoPlanningHandoffEvidence
|
||||
&& !task.awaitingApprovalReason
|
||||
&& !this.hasLivePlanningWork(task.id)
|
||||
const handoffKind = classifyPersistedPlanHandoff(task, {
|
||||
now: Date.now(),
|
||||
hasLivePlanningWork: this.hasLivePlanningWork(task.id),
|
||||
// Legacy unit fixtures have no graph-work-item reader; production always
|
||||
// applies the real stuck-processing grace.
|
||||
legacyStaleMs: continuationReader ? TriageProcessor.STALE_PROCESSING_THRESHOLD_MS : 0,
|
||||
});
|
||||
const legacyNullStatusCandidate = handoffKind === "legacy-null"
|
||||
// FNXC:PlanningDependencyReseed 2026-08-04-01:04: Legacy unit fixtures
|
||||
// have no graph-work-item reader; production always applies this fence.
|
||||
&& (!continuationReader || (
|
||||
Date.now() - new Date(task.updatedAt).getTime() >= TriageProcessor.STALE_PROCESSING_THRESHOLD_MS
|
||||
&& (await continuationReader.call(this.store, task.id)).length === 0
|
||||
(await continuationReader.call(this.store, task.id)).length === 0
|
||||
/*
|
||||
FNXC:PlanningDependencyReseed 2026-08-04-02:10:
|
||||
A graph run can persist foreach step-instance rows before it creates a
|
||||
@@ -1318,9 +1311,9 @@ export class TriageProcessor {
|
||||
*/
|
||||
&& (!stepInstanceReader || !(await stepInstanceReader.call(this.store, task.id)))
|
||||
));
|
||||
const recoverableStatus =
|
||||
task.status === "planning"
|
||||
|| (task.status == null && (this.hasSatisfiedPlanReview(task) || legacyNullStatusCandidate));
|
||||
const recoverableStatus = handoffKind === "planning"
|
||||
|| handoffKind === "approved-null"
|
||||
|| legacyNullStatusCandidate;
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-07-29-09:05 (U11): the INTAKE lane, not
|
||||
the literal. Converting only the `todo` sites left this one rejecting every
|
||||
card whose workflow renames its planner column, so the release below was
|
||||
@@ -2973,6 +2966,7 @@ export class TriageProcessor {
|
||||
const isReplan = task.status === "needs-replan";
|
||||
let existingPrompt: string | undefined;
|
||||
let feedback: string | undefined;
|
||||
let planReviewFeedbackHistory: string[] | undefined;
|
||||
|
||||
if (isReplan) {
|
||||
// Prefer explicit re-specification feedback logged by comment-triggered
|
||||
@@ -2982,7 +2976,7 @@ export class TriageProcessor {
|
||||
.find((entry) =>
|
||||
entry.action === "User comment requested re-specification of planned task"
|
||||
|| entry.action === "User comment invalidated spec approval — task needs re-specification"
|
||||
|| entry.action === "AI spec revision requested"
|
||||
|| (entry.action === "AI spec revision requested" && !isPlanReviewRevisionLog(entry))
|
||||
|| entry.action === TRIAGE_STUCK_RESUME_LOG_ACTION
|
||||
|| entry.action === TRIAGE_MARKER_CLEARED_REPLAN_LOG_ACTION
|
||||
);
|
||||
@@ -3021,7 +3015,7 @@ export class TriageProcessor {
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-13-00:00:
|
||||
FNXC:PlanReviewReplan 2026-08-04-06:35 (FN-8768):
|
||||
When re-planning and neither an explicit user/AI re-specification comment nor a
|
||||
user comment supplied feedback, fall back to the most recent Plan Review REVISE
|
||||
verdict recorded in `workflowStepResults`. The pre-execution Plan Review gate
|
||||
@@ -3035,13 +3029,21 @@ export class TriageProcessor {
|
||||
const latestPlanReviewRevise = [...(currentTask.workflowStepResults || [])]
|
||||
.reverse()
|
||||
.find((result) =>
|
||||
result.workflowStepId === PLAN_REVIEW_GROUP_ID
|
||||
(result.workflowStepId === PLAN_REVIEW_GROUP_ID || result.workflowStepName === "Plan Review")
|
||||
&& result.verdict === "REVISE"
|
||||
&& Boolean((result.output ?? result.notes)?.trim()),
|
||||
&& Boolean((result.notes ?? result.output)?.trim()),
|
||||
);
|
||||
feedback = latestPlanReviewRevise?.output ?? latestPlanReviewRevise?.notes ?? feedback;
|
||||
feedback = latestPlanReviewRevise?.notes ?? latestPlanReviewRevise?.output ?? feedback;
|
||||
}
|
||||
|
||||
// FNXC:PlanReviewConvergence 2026-08-04-06:35 (FN-8768): Exclude
|
||||
// the latest feedback because it renders in Revision Feedback; retain
|
||||
// the bounded earlier decisions as a non-duplicated convergence ledger.
|
||||
planReviewFeedbackHistory = collectPlanReviewFeedbackHistory(currentTask.workflowStepResults, {
|
||||
exclude: feedback,
|
||||
includeCurrent: false,
|
||||
});
|
||||
|
||||
planLog.log(
|
||||
`${task.id} re-planning with feedback: ${feedback?.slice(0, 100)}...`
|
||||
+ (existingPrompt ? " (seeded existing PROMPT.md for surgical revision)" : " (no existing draft — fresh respec)"),
|
||||
@@ -3062,6 +3064,7 @@ export class TriageProcessor {
|
||||
{
|
||||
plan: typeof planDocument?.content === "string" ? planDocument.content : undefined,
|
||||
originalDescription: typeof originalDescriptionDocument?.content === "string" ? originalDescriptionDocument.content : undefined,
|
||||
planReviewFeedbackHistory,
|
||||
},
|
||||
);
|
||||
await promptWithFallback(
|
||||
@@ -5091,7 +5094,7 @@ export function buildSpecificationPrompt(
|
||||
attachmentContents?: AttachmentContent[],
|
||||
existingPrompt?: string,
|
||||
feedback?: string,
|
||||
planningContext?: { plan?: string; originalDescription?: string },
|
||||
planningContext?: { plan?: string; originalDescription?: string; planReviewFeedbackHistory?: string[] },
|
||||
): string {
|
||||
const hasFeedback = Boolean(feedback?.trim());
|
||||
const planDocument = planningContext?.plan?.trim();
|
||||
@@ -5204,6 +5207,15 @@ Keep every \`##\`/\`###\` section heading, machine marker, the verbatim \`## Ori
|
||||
|
||||
let revisionSection = "";
|
||||
if (isRevision) {
|
||||
// FNXC:PlanningPromptConvergence 2026-08-04-06:35 (FN-8768): Prior review
|
||||
// decisions are first-class revision input. Rendering them separately from
|
||||
// the latest feedback prevents resolved requirements from disappearing;
|
||||
// the full-spec completeness rerun below catches blockers beyond the delta.
|
||||
const cumulativeReviewLedger = (planningContext?.planReviewFeedbackHistory ?? [])
|
||||
.map((entry) => entry.trim())
|
||||
.filter(Boolean)
|
||||
.map((entry, index) => `### PR${index + 1}\n${entry}`)
|
||||
.join("\n\n");
|
||||
/*
|
||||
FNXC:PlanReviewReplan 2026-07-15-11:15:
|
||||
Plan Review REVISE and user re-spec feedback share this path. Label feedback generically
|
||||
@@ -5222,7 +5234,8 @@ You are revising an existing task specification based on Plan Review or user fee
|
||||
- Apply **surgical** edits that fully resolve every blocking issue in the revision feedback below.
|
||||
- Preserve wording, steps, file scope, and acceptance criteria the feedback does not criticize.
|
||||
- Do not expand scope, invent new deliverables, or churn File Scope to "improve" an otherwise approved plan.
|
||||
- After editing, re-check each blocking item so a subsequent Plan Review can APPROVE without a new round of objections.
|
||||
- Treat every item in the cumulative ledger as a durable review decision unless a later entry explicitly supersedes it. Preserve resolved items, address every unresolved item, and do not regress an earlier correction while fixing the latest feedback.
|
||||
- After editing, rerun the full Mandatory Planning Completeness Procedure against the entire revised specification — not only the latest feedback — so a subsequent Plan Review can evaluate all remaining blockers in one pass.
|
||||
|
||||
## Existing Specification
|
||||
\`\`\`markdown
|
||||
@@ -5232,6 +5245,8 @@ ${existingPrompt}
|
||||
## Revision Feedback
|
||||
${feedback}
|
||||
|
||||
${cumulativeReviewLedger ? `## Cumulative Revision Decision Ledger\n${cumulativeReviewLedger}\n` : ""}
|
||||
|
||||
Revise the specification above to address this feedback. Persist the complete revised PROMPT.md with \`fn_task_prompt_write\`.`;
|
||||
} else if (isFreshRespecification) {
|
||||
revisionSection = `
|
||||
|
||||
@@ -814,8 +814,8 @@ export class WorkflowGraphExecutor {
|
||||
? node.config.name.trim()
|
||||
: node.id;
|
||||
/*
|
||||
FNXC:PlanReview 2026-06-29-02:40:
|
||||
Triage runs Plan Review before releasing a task to execution so the task stays in the triage column during review. When the execution graph later reaches the same optional group, treat an existing passed Plan Review result as satisfied and do not launch a duplicate reviewer session.
|
||||
FNXC:PlanReview 2026-08-04-06:35:
|
||||
Triage runs Plan Review before releasing a task to execution so the task stays in the triage column during review. When the execution graph later reaches the same optional group, only an unsuperseded projection may satisfy, repair, or hold the gate; old-episode evidence must never suppress the current reviewer session.
|
||||
*/
|
||||
if (
|
||||
node.id === PLAN_REVIEW_GROUP_ID
|
||||
@@ -825,7 +825,10 @@ export class WorkflowGraphExecutor {
|
||||
this.deps.logTaskEntry?.("[pre-merge] Workflow step already satisfied: Plan Review");
|
||||
return await traverseChildren(node, { outcome: "success", value: "already-passed" });
|
||||
}
|
||||
const repairedPlanReview = node.id === PLAN_REVIEW_GROUP_ID
|
||||
const hasPlanReviewProjection = task.workflowStepResults?.some(
|
||||
(result) => result.workflowStepId === PLAN_REVIEW_GROUP_ID,
|
||||
) === true;
|
||||
const repairedPlanReview = node.id === PLAN_REVIEW_GROUP_ID && !hasPlanReviewProjection
|
||||
? recoverPassedPlanReviewFromLatestLog(task)
|
||||
: undefined;
|
||||
if (repairedPlanReview) {
|
||||
@@ -855,7 +858,7 @@ export class WorkflowGraphExecutor {
|
||||
);
|
||||
}
|
||||
const lease = classifyReviewLease(
|
||||
task.workflowStepResults,
|
||||
task.workflowStepResults?.filter((result) => result.supersededAt == null),
|
||||
node.id,
|
||||
this.deps.runLoopNowForTests?.() ?? Date.now(),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user