root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FN-8793: restore workflow review feedback selection

Browse Source 
Restore canonical workflow review feedback so revisions can select server-owned reviewer items.

- Normalize current Code Review and Plan Review workflow results into stable review items.
- Preserve verdict and reviewer type through review API mappings and reject forged client feedback.
- Add regression coverage and a patch changeset.

Files changed:
 .changeset/fn-8793-workflow-review-items.md        |   7 +
 packages/core/src/index.gate.ts                    |   2 +-
 packages/core/src/index.ts                         |   2 +-
 packages/core/src/types/task/task-review.ts        |   6 +-
 packages/dashboard/app/api/agents/run-audit.ts     |   2 +
 .../dashboard/src/__tests__/routes-tasks.test.ts   | 121 +++++++++++++++++
 .../src/routes/register-task-workflow-routes.ts    | 148 ++++++++++++++++++---
 7 files changed, 264 insertions(+), 24 deletions(-)

Fusion-Task-Id: FN-8793

Fusion-Task-Lineage: 25da04f9-5c30-4ad8-9f82-e1ac91ec481f

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-08-04 19:23:20 -07:00
parent b9c5df04af
commit 0658795181
7 changed files with 264 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/fn-8793-workflow-review-items.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Restore workflow review feedback selection for same-task revisions.
category: fix
dev: Canonical workflow-step review items now prevent forged client feedback from reaching snapshots or steering.

2
packages/core/src/index.gate.ts
View File

@@ -172,7 +172,7 @@ export {
type ResolveAgentMemoryInclusionModeInput,
type ResolvedAgentMemoryInclusionMode,
} from "./agents/agent-memory-mode.js";
export type { TaskReviewData, TaskReviewSummary, TaskReviewItem } from "./types.js";
export type { TaskReviewData, TaskReviewSummary, TaskReviewItem, TaskReviewVerdict, TaskReviewerType } from "./types.js";
export type {
TaskCommitAssociation,
TaskCommitAssociationConfidence,

2
packages/core/src/index.ts
View File

@@ -198,7 +198,7 @@ export {
type ResolveAgentMemoryInclusionModeInput,
type ResolvedAgentMemoryInclusionMode,
} from "./agents/agent-memory-mode.js";
export type { TaskReviewData, TaskReviewSummary, TaskReviewItem } from "./types.js";
export type { TaskReviewData, TaskReviewSummary, TaskReviewItem, TaskReviewVerdict, TaskReviewerType } from "./types.js";
/* FNXC:TaskVerificationRequest 2026-07-30-00:00: FN-8296 makes the persisted verification read model available to dashboard task and Command Center surfaces without exporting a subprocess runner. */
export type { TaskVerificationRequest, TaskVerificationResultSummary, TaskVerificationStatus, TaskVerificationProfile } from "./types.js";
export type {

6
packages/core/src/types/task/task-review.ts
View File

@@ -6,7 +6,7 @@
export type TaskReviewMode = "pull-request" | "direct";
export type TaskReviewSource = "github-pr" | "reviewer-agent";
export type TaskReviewDecision = "approved" | "changes-requested" | "commented" | "pending";
export type TaskReviewVerdict = "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
export type TaskReviewVerdict = "APPROVE" | "APPROVE_WITH_NOTES" | "REVISE" | "RETHINK" | "UNAVAILABLE";
export type TaskReviewerType = "plan" | "code";
export type TaskReviewItemStatus = "queued" | "in-progress" | "addressed" | "failed";
@@ -163,6 +163,10 @@ export interface TaskReviewDataItem {
line?: number;
threadId?: string;
reviewState?: string | null;
/** Machine-readable reviewer verdict when the source supplied one. */
verdict?: TaskReviewVerdict;
/** Review lane that produced the item when known. */
reviewType?: TaskReviewerType;
isResolved?: boolean;
progressStatus?: "queued" | "in-progress" | "addressed" | "failed" | null;
}

2
packages/dashboard/app/api/agents/run-audit.ts
View File

@@ -257,6 +257,8 @@ function mapTaskReviewDataToLegacy(data: TaskReviewData): TaskReviewResponse {
threadId: item.threadId,
htmlUrl: item.url,
state: item.reviewState ?? undefined,
verdict: item.verdict,
reviewType: item.reviewType,
summary: item.title ?? undefined,
isResolved: item.isResolved,
...(typeof item.line === "number" ? { line: item.line } : {}),

121
packages/dashboard/src/__tests__/routes-tasks.test.ts
View File

@@ -2749,6 +2749,65 @@ describe("POST /tasks/:id/review/address", () => {
});
}
it("normalizes current workflow review results into stable canonical items", async () => {
const taskWithWorkflowReviews = {
...FAKE_TASK_DETAIL,
id: "FN-009",
updatedAt: "2026-01-01T00:00:00.000Z",
workflowStepResults: [
{ workflowStepId: "code-review", workflowStepName: "Code Review", phase: "pre-merge", status: "passed", verdict: "APPROVE_WITH_NOTES", output: "1. Keep the assertion focused.", completedAt: "2026-01-02T00:00:00.000Z" },
{ workflowStepId: "plan-review", workflowStepName: "Plan Review", phase: "pre-merge", status: "failed", verdict: "REVISE", notes: "Clarify the rollback plan.", startedAt: "2026-01-03T00:00:00.000Z" },
{ workflowStepId: "code-review", workflowStepName: "Code Review", phase: "pre-merge", status: "advisory_failure", verdict: "APPROVE", completedAt: "2026-01-04T00:00:00.000Z" },
{ workflowStepId: "custom-review", workflowStepName: "Custom Review", status: "passed", verdict: "REVISE", output: "Must not be inferred." },
{ workflowStepId: "code-review", workflowStepName: "Code Review", status: "pending", verdict: "REVISE" },
{ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "skipped", verdict: "REVISE" },
{ workflowStepId: "code-review", workflowStepName: "Code Review", status: "passed", verdict: "REVISE", supersededAt: "2026-01-05T00:00:00.000Z" },
{ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "failed", verdict: "REVISE", bypassedAt: "2026-01-05T00:00:00.000Z" },
],
log: [{ timestamp: reviewerBlockTimestamp, action: "code review Step 1: REVISE - legacy fallback must not duplicate structured data" }],
};
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskWithWorkflowReviews);
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([{ agent: "reviewer", type: "text", text: "## Code Review:\\n### Verdict: REVISE" }]);
const first = await REQUEST(buildApp(), "GET", "/api/tasks/FN-009/review");
const refreshed = await REQUEST(buildApp(), "POST", "/api/tasks/FN-009/review/refresh");
expect(first.status).toBe(200);
expect(refreshed.status).toBe(200);
expect(first.body.items).toHaveLength(3);
expect(first.body.items).toEqual(expect.arrayContaining([
expect.objectContaining({ title: "Code Review APPROVE_WITH_NOTES", body: "1. Keep the assertion focused.", verdict: "APPROVE_WITH_NOTES", reviewType: "code" }),
expect.objectContaining({ title: "Plan Review REVISE", body: "Clarify the rollback plan.", verdict: "REVISE", reviewType: "plan" }),
expect.objectContaining({ verdict: "APPROVE", body: "No written feedback was provided by this review step." }),
]));
expect(first.body.summary).toEqual(expect.objectContaining({ verdict: "APPROVE" }));
expect(first.body.items.map((item: { itemId: string }) => item.itemId)).toEqual(refreshed.body.items.map((item: { itemId: string }) => item.itemId));
expect(store.getAgentLogs).not.toHaveBeenCalled();
});
it("uses legacy activity review feedback when workflow results are absent or unsupported", async () => {
const taskWithUnsupportedWorkflowReview = {
...FAKE_TASK_DETAIL,
id: "FN-010",
workflowStepResults: [{ workflowStepId: "custom-review", workflowStepName: "Custom Review", status: "passed", verdict: "REVISE", output: "not a current built-in review node" }],
log: [{ timestamp: fallbackTimestamp, action: "plan review Step 2: RETHINK - revise the approach" }],
};
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskWithUnsupportedWorkflowReview);
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
const res = await REQUEST(buildApp(), "GET", "/api/tasks/FN-010/review");
expect(res.status).toBe(200);
expect(res.body.items).toEqual([expect.objectContaining({
itemId: fallbackItemId,
title: "plan review RETHINK",
body: "plan review Step 2: RETHINK - revise the approach",
verdict: "RETHINK",
reviewType: "plan",
})]);
expect(store.getAgentLogs).toHaveBeenCalledWith("FN-010");
});
it("resumes reviewer-agent in-review tasks using canonical log-derived review ids when reviewState is absent", async () => {
const taskWithoutPersistedItems = {
...FAKE_TASK_DETAIL,
@@ -2785,6 +2844,68 @@ describe("POST /tasks/:id/review/address", () => {
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 0, "pending");
});
it("addresses workflow review items by canonical id without trusting forged client feedback", async () => {
const taskWithWorkflowReview = {
...FAKE_TASK_DETAIL,
id: "FN-009",
column: "in-review",
status: "awaiting-user-review",
assignedAgentId: null,
sessionFile: null,
workflowStepResults: [{
workflowStepId: "code-review",
workflowStepName: "Code Review",
phase: "pre-merge",
status: "passed",
verdict: "APPROVE_WITH_NOTES",
output: "Canonical advisory: preserve this text.",
completedAt: "2026-01-05T00:00:00.000Z",
}],
reviewState: undefined,
};
const movedTask = { ...taskWithWorkflowReview, column: "in-progress", status: null };
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskWithWorkflowReview);
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-1" });
(store.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue(movedTask);
const review = await REQUEST(buildApp(), "GET", "/api/tasks/FN-009/review");
const itemId = review.body.items[0].itemId;
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-009/review/address", JSON.stringify({
selectedItems: [{
id: itemId,
source: "reviewer-agent",
summary: "FORGED summary",
body: "FORGED body",
author: "attacker",
filePath: "forged.ts",
lineNumber: 999,
url: "https://invalid.example/forged",
}],
}), { "Content-Type": "application/json" });
expect(res.status).toBe(200);
expect(store.updateTask).toHaveBeenCalledWith("FN-009", {
reviewState: expect.objectContaining({
addressing: [expect.objectContaining({
itemId,
snapshot: expect.objectContaining({
summary: "Code Review APPROVE_WITH_NOTES",
body: "Canonical advisory: preserve this text.",
authorLogin: "reviewer-agent",
filePath: undefined,
lineNumber: undefined,
url: undefined,
}),
})],
}),
});
const steering = (store.addSteeringComment as ReturnType<typeof vi.fn>).mock.calls[0][1] as string;
expect(steering).toContain("Canonical advisory: preserve this text.");
expect(steering).not.toContain("FORGED");
expect(steering).not.toContain("invalid.example");
expect(store.moveTask).toHaveBeenCalledWith("FN-009", "in-progress", { preserveProgress: true });
});
it("accepts reviewer-agent fallback log review ids when no reviewer text block exists", async () => {
const taskWithFallbackLog = {
...FAKE_TASK_DETAIL,

148
packages/dashboard/src/routes/register-task-workflow-routes.ts
View File

@@ -9,6 +9,7 @@ const severityAuditLog = createLogger("dashboard-register-task-workflow-routes")
* than turning one board load into thousands of file reads. Truncation is logged, never silent.
*/
const AWAITING_PLANNING_ENRICH_LIMIT = 200;
import { createHash } from "node:crypto";
import { createReadStream } from "node:fs";
import { readFile, stat } from "node:fs/promises";
import { join } from "node:path";
@@ -21,6 +22,8 @@ import type {
TaskReviewData,
TaskReviewItem,
TaskReviewSummary,
TaskReviewVerdict,
WorkflowStepResult,
GithubIssueAction,
DuplicateCandidate,
DuplicateMatch,
@@ -810,7 +813,101 @@ function buildReviewerAgentItemId(input: { index: number; reviewType: "plan" | "
return `reviewer-${input.reviewType}-${stepPart}-${verdictPart}-${timePart}-${input.index + 1}`;
}
const CURRENT_WORKFLOW_REVIEW_STEP_IDS = new Set(["code-review", "plan-review"]);
const CURRENT_WORKFLOW_REVIEW_STATUSES = new Set<WorkflowStepResult["status"]>(["passed", "failed", "advisory_failure"]);
function parseTaskReviewVerdict(value: string | undefined): TaskReviewVerdict | undefined {
switch (value) {
case "APPROVE":
case "APPROVE_WITH_NOTES":
case "REVISE":
case "RETHINK":
case "UNAVAILABLE":
return value;
default:
return undefined;
}
}
/**
* FNXC:TaskReview 2026-08-05-01:57:
* The current graph-owned Code Review and Plan Review ids are the only structured review sources:
* current, terminal, verdict-bearing results win over compatibility-only reviewer prose/activity
* parsing. Their persisted identity produces canonical ids so GET, refresh, and address reconstruct
* the same server-owned feedback; address snapshots and steering must never trust client prose.
* Custom step names, historical attempts, pending/skipped, superseded, and bypassed results remain
* non-selectable until workflow results gain an authoritative persisted review-kind marker.
*/
function isCurrentWorkflowReviewResult(result: WorkflowStepResult): result is WorkflowStepResult & { verdict: TaskReviewVerdict } {
return CURRENT_WORKFLOW_REVIEW_STEP_IDS.has(result.workflowStepId)
&& CURRENT_WORKFLOW_REVIEW_STATUSES.has(result.status)
&& result.verdict !== undefined
&& result.supersededAt === undefined
&& result.bypassedBy === undefined
&& result.bypassedAt === undefined
&& result.bypassReason === undefined
&& result.bypassedFromStatus === undefined
&& result.bypassedFromVerdict === undefined;
}
function buildWorkflowReviewItemId(task: Task, result: WorkflowStepResult): string {
const identity = JSON.stringify({
taskId: task.id,
workflowStepId: result.workflowStepId,
workflowStepName: result.workflowStepName,
phase: result.phase,
status: result.status,
verdict: result.verdict,
completedAt: result.completedAt,
startedAt: result.startedAt,
output: result.output,
notes: result.notes,
});
return `workflow-review-${createHash("sha256").update(identity).digest("hex").slice(0, 24)}`;
}
function buildWorkflowReviewItems(task: Task): TaskReviewItem[] {
return (task.workflowStepResults ?? [])
.filter(isCurrentWorkflowReviewResult)
.map((result): TaskReviewItem => {
const reviewType = result.workflowStepId === "plan-review" ? "plan" as const : "code" as const;
const body = result.output?.trim() || result.notes?.trim() || "No written feedback was provided by this review step.";
const timestamp = result.completedAt ?? result.startedAt ?? task.updatedAt ?? task.createdAt;
return {
itemId: buildWorkflowReviewItemId(task, result),
sourceMode: "reviewer-agent",
title: `${result.workflowStepName || result.workflowStepId} ${result.verdict}`,
body,
author: "reviewer-agent",
createdAt: timestamp,
updatedAt: timestamp,
reviewState: result.verdict,
verdict: result.verdict,
reviewType,
progressStatus: null,
};
})
.sort((a, b) => Date.parse(b.createdAt ?? "") - Date.parse(a.createdAt ?? "") || a.itemId.localeCompare(b.itemId));
}
function buildDirectReviewSummary(items: TaskReviewItem[]): TaskReviewSummary | null {
const latest = items[0];
return latest
? { summary: latest.title, verdict: latest.verdict }
: null;
}
async function buildDirectTaskReviewData(task: Task, store: TaskStore): Promise<TaskReviewData> {
const structuredItems = buildWorkflowReviewItems(task);
if (structuredItems.length > 0) {
return {
mode: "reviewer-agent",
refreshable: true,
fetchedAt: new Date().toISOString(),
summary: buildDirectReviewSummary(structuredItems),
items: structuredItems,
};
}
const agentLogs = await store.getAgentLogs(task.id);
const reviewerText = agentLogs.filter((entry) => entry.agent === "reviewer" && entry.type === "text").map((entry) => entry.text).join("\n");
const fallbackLogs = (task.log ?? []).filter((entry) => REVIEW_STEP_RE.test(entry.action));
@@ -833,6 +930,8 @@ async function buildDirectTaskReviewData(task: Task, store: TaskStore): Promise<
createdAt,
updatedAt: createdAt,
reviewState: verdict ?? null,
verdict: parseTaskReviewVerdict(verdict),
reviewType,
progressStatus: null,
});
}
@@ -851,25 +950,20 @@ async function buildDirectTaskReviewData(task: Task, store: TaskStore): Promise<
createdAt: entry.timestamp,
updatedAt: entry.timestamp,
reviewState: verdict ?? null,
verdict: parseTaskReviewVerdict(verdict),
reviewType,
progressStatus: null,
});
});
}
const sorted = [...items].sort((a, b) => Date.parse(b.createdAt ?? "") - Date.parse(a.createdAt ?? ""));
const latest = sorted[0];
const summary: TaskReviewSummary | null = latest
? {
summary: latest.title,
verdict: (latest.reviewState as "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE" | null | undefined) ?? undefined,
}
: null;
const sorted = [...items].sort((a, b) => Date.parse(b.createdAt ?? "") - Date.parse(a.createdAt ?? "") || a.itemId.localeCompare(b.itemId));
return {
mode: "reviewer-agent",
refreshable: true,
fetchedAt: new Date().toISOString(),
summary,
summary: buildDirectReviewSummary(sorted),
items: sorted,
};
}
@@ -6046,20 +6140,13 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
type SelectedReviewItem = {
id: string;
source: "pr-review" | "reviewer-agent";
threadId?: string;
filePath?: string;
lineNumber?: number;
author?: string;
summary: string;
body: string;
url?: string;
};
const selectedItems: SelectedReviewItem[] = Array.isArray(req.body?.selectedItems)
? req.body.selectedItems.filter((value: unknown): value is SelectedReviewItem => {
if (!value || typeof value !== "object") return false;
const item = value as Record<string, unknown>;
return typeof item.id === "string" && item.id.trim().length > 0 && typeof item.summary === "string" && typeof item.body === "string";
return typeof item.id === "string" && item.id.trim().length > 0 && typeof item.source === "string";
})
: [];
@@ -6098,6 +6185,8 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
state: item.reviewState ?? undefined,
isResolved: item.isResolved,
source: item.sourceMode === "reviewer-agent" ? "reviewer-agent" as const : "github-pr" as const,
verdict: item.verdict,
reviewType: item.reviewType,
}));
const reviewState = {
source: canonicalReviewData.mode,
@@ -6117,7 +6206,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
const now = new Date().toISOString();
const selectedSet = new Set(selectedItems.map((item: SelectedReviewItem) => item.id));
const canonicalIds = new Set(reviewState.items.map((item) => item.id));
const expectedSource = canonicalReviewData.mode === "pull-request" ? "pr-review" : "reviewer-agent";
const expectedSource: "pr-review" | "reviewer-agent" = canonicalReviewData.mode === "pull-request" ? "pr-review" : "reviewer-agent";
const reviewSourceMismatch = selectedItems.find((item) => canonicalIds.has(item.id) && item.source !== expectedSource);
if (reviewSourceMismatch) {
throw badRequest("Selected review source does not match task review mode");
@@ -6127,8 +6216,25 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
throw badRequest("selectedItems must reference existing review items");
}
const modeSummary = `${reviewState.source === "pull-request" ? "pull-request" : "reviewer-agent"} · ${selectedItems.length} selected item(s)`;
const steeringItems = selectedItems.map((item: SelectedReviewItem, index: number) => {
const canonicalById = new Map(reviewState.items.map((item) => [item.id, item] as const));
const canonicalSelections = selectedItems.map((selected) => {
const item = canonicalById.get(selected.id);
if (!item) throw badRequest("selectedItems must reference existing review items");
return {
id: item.id,
source: expectedSource,
summary: item.summary ?? item.body.slice(0, 120),
body: item.body,
author: item.author.login,
filePath: item.path,
lineNumber: item.line,
threadId: item.threadId,
url: item.htmlUrl,
};
});
const modeSummary = `${reviewState.source === "pull-request" ? "pull-request" : "reviewer-agent"} · ${canonicalSelections.length} selected item(s)`;
const steeringItems = canonicalSelections.map((item, index) => {
const location = item.filePath ? `${item.filePath}${typeof item.lineNumber === "number" ? `:${item.lineNumber}` : ""}` : undefined;
const snippetSource = item.body.trim() || item.summary.trim();
const snippet = snippetSource.length > 220 ? `${snippetSource.slice(0, 220)}…` : snippetSource;
@@ -6140,7 +6246,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
const priorAddressingById = new Map(reviewState.addressing.map((record) => [record.itemId, record] as const));
const nextAddressing = [
...reviewState.addressing.filter((record) => !selectedSet.has(record.itemId)),
...selectedItems.map((item: SelectedReviewItem) => {
...canonicalSelections.map((item) => {
const existing = priorAddressingById.get(item.id);
return {
itemId: item.id,