FN-7186: fix reviewer-agent revision requests
Route review revision requests through canonical review data so reviewer-agent feedback can be selected reliably. - Rebuild /review/address validation from PR review details or direct reviewer-agent logs before matching selected items. - Preserve addressing state while accepting log-derived reviewer-agent item ids and rejecting mismatched sources. - Cover reviewer-agent, fallback-log, PR, and invalid-selection paths in dashboard route tests. - Document that Review revision feedback may come from PR data or reviewer-agent logs. - Add a patch changeset for the published Fusion package. Files changed: .changeset/fn-7186-review-revision-fix.md | 7 ++ docs/dashboard-guide.md | 4 +- .../dashboard/src/__tests__/routes-tasks.test.ts | 126 +++++++++++++++++---- .../src/routes/register-task-workflow-routes.ts | 76 +++++++++---- 4 files changed, 170 insertions(+), 43 deletions(-) Fusion-Task-Id: FN-7186 Fusion-Task-Lineage: 8b0f6e4c-93b1-489c-ab96-5a2541b7c18a Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7186-review-revision-fix.md
Normal file
7
.changeset/fn-7186-review-revision-fix.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Fix "Request revision" error on reviewer-agent task reviews.
|
||||||
|
category: fix
|
||||||
|
dev: review/address now validates selected items against the same canonical review source the UI renders (buildDirectTaskReviewData / getPrReviewDetails) instead of the persisted reviewState.items.
|
||||||
@@ -1008,8 +1008,8 @@ Inspect task definition, logs, review feedback, comments, artifacts, workflow ou
|
|||||||
- The **Artifacts** tab combines task documents written by agents or users with task-scoped registered media artifacts. The gallery uses thumbnail-first image/video cards, image and video previews can expand into a dismissible full-size lightbox, video and audio use native controls, document artifacts show text previews, and generic artifacts open through their media URL.
|
- The **Artifacts** tab combines task documents written by agents or users with task-scoped registered media artifacts. The gallery uses thumbnail-first image/video cards, image and video previews can expand into a dismissible full-size lightbox, video and audio use native controls, document artifacts show text previews, and generic artifacts open through their media URL.
|
||||||
- The **Review** tab is separate from **Comments**: Review shows actionable PR/reviewer feedback and same-task revision controls, while Comments remains the general collaboration thread.
|
- The **Review** tab is separate from **Comments**: Review shows actionable PR/reviewer feedback and same-task revision controls, while Comments remains the general collaboration thread.
|
||||||
- Review comments hide GitHub template HTML comments in both Markdown and Plain modes, show author avatars or User/Bot fallbacks, label Human vs Bot/agent authors, and include All/Human/Bot filtering.
|
- Review comments hide GitHub template HTML comments in both Markdown and Plain modes, show author avatars or User/Bot fallbacks, label Human vs Bot/agent authors, and include All/Human/Bot filtering.
|
||||||
- **Request revision** in Review resumes work on the same task ID (no refinement task): `in-progress` tasks get steering injection, while `in-review` tasks are moved back to `in-progress` for the same branch/worktree revision pass.
|
- **Request revision** in Review resumes work on the same task ID (no refinement task): `in-progress` tasks get steering injection, while `in-review` tasks are moved back to `in-progress` for the same branch/worktree revision pass. The selected feedback can come from either PR review data or reviewer-agent feedback shown in the tab.
|
||||||
- Review supports a manual **Refresh** action in-place: PR mode pulls latest GitHub review state/decision, while direct mode rehydrates reviewer-agent feedback from persisted task data (no GitHub call).
|
- Review supports a manual **Refresh** action in-place: PR mode pulls latest GitHub review state/decision, while direct mode rehydrates reviewer-agent feedback from task agent logs (no GitHub call).
|
||||||
- For shared `branch_groups` (tasks with `branchContext.groupId`), PR merge mode opens and tracks one group-level PR from the group integration branch to the project default branch; member tasks share that PR state.
|
- For shared `branch_groups` (tasks with `branchContext.groupId`), PR merge mode opens and tracks one group-level PR from the group integration branch to the project default branch; member tasks share that PR state.
|
||||||
- In direct/non-PR auto-merge mode, Review renders normalized reviewer-agent feedback (verdict/step/timestamp/detail) with dedicated loading/error/empty states; it does not require users to read raw agent logs.
|
- In direct/non-PR auto-merge mode, Review renders normalized reviewer-agent feedback (verdict/step/timestamp/detail) with dedicated loading/error/empty states; it does not require users to read raw agent logs.
|
||||||
|
|
||||||
|
|||||||
@@ -145,6 +145,11 @@ vi.mock("@fusion/engine", async () => {
|
|||||||
promptWithFallback: vi.fn(async (session: { prompt: (message: string) => Promise<void> }, prompt: string) => {
|
promptWithFallback: vi.fn(async (session: { prompt: (message: string) => Promise<void> }, prompt: string) => {
|
||||||
await session.prompt(prompt);
|
await session.prompt(prompt);
|
||||||
}),
|
}),
|
||||||
|
/*
|
||||||
|
FNXC:DashboardRouteTests 2026-06-27-00:08:
|
||||||
|
Route tests mock @fusion/engine wholesale, but planning/subtask helpers now resolve MCP servers before creating read-only AI sessions. Keep the default MCP result shaped so unrelated route assertions do not fail on the fallback vi.fn() returning undefined.
|
||||||
|
*/
|
||||||
|
resolveMcpServersForStore: vi.fn().mockResolvedValue({ servers: [], errors: [] }),
|
||||||
AgentReflectionService: class MockAgentReflectionService {
|
AgentReflectionService: class MockAgentReflectionService {
|
||||||
async generateReflection(): Promise<import("@fusion/core").AgentReflection | null> {
|
async generateReflection(): Promise<import("@fusion/core").AgentReflection | null> {
|
||||||
throw new Error("Reflection service unavailable in route tests");
|
throw new Error("Reflection service unavailable in route tests");
|
||||||
@@ -1962,7 +1967,7 @@ describe("POST /subtasks/*", () => {
|
|||||||
|
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
expect(res.body).toEqual({ success: true, sessionId: "session-123" });
|
expect(res.body).toEqual({ success: true, sessionId: "session-123" });
|
||||||
expect(retrySpy).toHaveBeenCalledWith("session-123", "/fake/root", undefined);
|
expect(retrySpy).toHaveBeenCalledWith("session-123", "/fake/root", undefined, store);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns 404 when subtask retry session does not exist", async () => {
|
it("returns 404 when subtask retry session does not exist", async () => {
|
||||||
@@ -2604,6 +2609,10 @@ describe("POST /subtasks/*", () => {
|
|||||||
|
|
||||||
describe("POST /tasks/:id/review/address", () => {
|
describe("POST /tasks/:id/review/address", () => {
|
||||||
let store: TaskStore;
|
let store: TaskStore;
|
||||||
|
const reviewerBlockTimestamp = "2026-01-02T03:04:05.000Z";
|
||||||
|
const reviewerBlockItemId = "reviewer-code-step-na-revise-2026-01-02T03-04-05-000Z-1";
|
||||||
|
const fallbackTimestamp = "2026-01-03T04:05:06.000Z";
|
||||||
|
const fallbackItemId = "reviewer-plan-step-2-rethink-2026-01-03T04-05-06-000Z-1";
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
store = createMockStore({ updateStep: vi.fn() } as unknown as Partial<TaskStore>);
|
store = createMockStore({ updateStep: vi.fn() } as unknown as Partial<TaskStore>);
|
||||||
@@ -2616,45 +2625,116 @@ describe("POST /tasks/:id/review/address", () => {
|
|||||||
return app;
|
return app;
|
||||||
}
|
}
|
||||||
|
|
||||||
it("resumes in-review tasks to in-progress using selected review payload", async () => {
|
function mockReviewerBlockLogs() {
|
||||||
const taskWithReview = {
|
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||||
|
{
|
||||||
|
timestamp: reviewerBlockTimestamp,
|
||||||
|
agent: "reviewer",
|
||||||
|
type: "text",
|
||||||
|
text: "## Code Review:\n\n### Verdict: REVISE\n\nFix tests before merge.",
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
function mockPrReviewDetails() {
|
||||||
|
return vi.spyOn(GitHubClient.prototype, "getPrReviewDetails").mockResolvedValue({
|
||||||
|
mode: "pull-request",
|
||||||
|
refreshable: true,
|
||||||
|
fetchedAt: "2026-01-04T05:06:07.000Z",
|
||||||
|
summary: { reviewDecision: "CHANGES_REQUESTED", reviewers: [], blockingReasons: [], checks: [] },
|
||||||
|
items: [
|
||||||
|
{
|
||||||
|
itemId: "ri-1",
|
||||||
|
sourceMode: "pull-request",
|
||||||
|
title: "Fix tests",
|
||||||
|
body: "Fix tests",
|
||||||
|
author: "reviewer",
|
||||||
|
createdAt: "2026-01-04T05:06:07.000Z",
|
||||||
|
updatedAt: "2026-01-04T05:06:07.000Z",
|
||||||
|
filePath: "src/a.ts",
|
||||||
|
line: 4,
|
||||||
|
reviewState: "CHANGES_REQUESTED",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it("resumes reviewer-agent in-review tasks using canonical log-derived review ids when reviewState is absent", async () => {
|
||||||
|
const taskWithoutPersistedItems = {
|
||||||
...FAKE_TASK_DETAIL,
|
...FAKE_TASK_DETAIL,
|
||||||
id: "FN-001",
|
id: "FN-001",
|
||||||
column: "in-review",
|
column: "in-review",
|
||||||
status: "awaiting-user-review",
|
status: "awaiting-user-review",
|
||||||
assignedAgentId: null,
|
assignedAgentId: null,
|
||||||
|
updatedAt: reviewerBlockTimestamp,
|
||||||
steps: [{ id: "s1", title: "Step 1", status: "done" }],
|
steps: [{ id: "s1", title: "Step 1", status: "done" }],
|
||||||
reviewState: {
|
reviewState: undefined,
|
||||||
source: "reviewer-agent",
|
|
||||||
items: [{ id: "ri-1", body: "Fix tests", summary: "Fix tests", author: { login: "reviewer" }, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString() }],
|
|
||||||
addressing: [],
|
|
||||||
},
|
|
||||||
};
|
};
|
||||||
const movedTask = { ...taskWithReview, column: "in-progress", status: null, sessionFile: null, assignedAgentId: null };
|
const taskAfterAddressing = {
|
||||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValueOnce(taskWithReview).mockResolvedValueOnce({ ...taskWithReview, reviewState: { ...taskWithReview.reviewState, addressing: [{ itemId: "ri-1", status: "queued", selectedAt: new Date().toISOString() }] } });
|
...taskWithoutPersistedItems,
|
||||||
|
reviewState: { source: "reviewer-agent", items: [], addressing: [{ itemId: reviewerBlockItemId, status: "queued", selectedAt: reviewerBlockTimestamp }] },
|
||||||
|
};
|
||||||
|
const movedTask = { ...taskAfterAddressing, column: "in-progress", status: null, sessionFile: null, assignedAgentId: null };
|
||||||
|
mockReviewerBlockLogs();
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValueOnce(taskWithoutPersistedItems).mockResolvedValueOnce(taskAfterAddressing);
|
||||||
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-1" });
|
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-1" });
|
||||||
(store.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue(movedTask);
|
(store.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue(movedTask);
|
||||||
|
|
||||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: "ri-1", source: "reviewer-agent", summary: "Fix tests", body: "Fix tests" }] }), { "Content-Type": "application/json" });
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: reviewerBlockItemId, source: "reviewer-agent", summary: "code review REVISE", body: "Fix tests before merge." }] }), { "Content-Type": "application/json" });
|
||||||
|
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
|
expect(store.updateTask).toHaveBeenCalledWith("FN-001", {
|
||||||
|
reviewState: expect.objectContaining({
|
||||||
|
source: "reviewer-agent",
|
||||||
|
items: [expect.objectContaining({ id: reviewerBlockItemId, source: "reviewer-agent" })],
|
||||||
|
addressing: [expect.objectContaining({ itemId: reviewerBlockItemId, status: "queued" })],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
expect(store.addSteeringComment).toHaveBeenCalledWith("FN-001", expect.stringContaining("Fix tests before merge."), "user");
|
||||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-progress", { preserveProgress: true });
|
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-progress", { preserveProgress: true });
|
||||||
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 0, "pending");
|
expect(store.updateStep).toHaveBeenCalledWith("FN-001", 0, "pending");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("for in-progress tasks injects steering without moving task", async () => {
|
it("accepts reviewer-agent fallback log review ids when no reviewer text block exists", async () => {
|
||||||
const taskWithReview = {
|
const taskWithFallbackLog = {
|
||||||
...FAKE_TASK_DETAIL,
|
...FAKE_TASK_DETAIL,
|
||||||
id: "FN-001",
|
id: "FN-001",
|
||||||
column: "in-progress",
|
column: "in-progress",
|
||||||
sessionFile: "active.session.json",
|
sessionFile: "active.session.json",
|
||||||
|
reviewState: { source: "reviewer-agent", items: [], addressing: [] },
|
||||||
|
log: [{ timestamp: fallbackTimestamp, action: "plan review Step 2: RETHINK - revise the approach" }],
|
||||||
|
};
|
||||||
|
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValueOnce(taskWithFallbackLog).mockResolvedValueOnce(taskWithFallbackLog);
|
||||||
|
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-1" });
|
||||||
|
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: fallbackItemId, source: "reviewer-agent", summary: "plan review RETHINK", body: "revise the approach" }] }), { "Content-Type": "application/json" });
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(store.updateTask).toHaveBeenCalledWith("FN-001", {
|
||||||
|
reviewState: expect.objectContaining({
|
||||||
|
items: [expect.objectContaining({ id: fallbackItemId })],
|
||||||
|
addressing: [expect.objectContaining({ itemId: fallbackItemId })],
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
expect(store.moveTask).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("for PR in-progress tasks validates canonical PR review items without moving task", async () => {
|
||||||
|
mockPrReviewDetails();
|
||||||
|
const taskWithPrReview = {
|
||||||
|
...FAKE_TASK_DETAIL,
|
||||||
|
id: "FN-001",
|
||||||
|
column: "in-progress",
|
||||||
|
sessionFile: "active.session.json",
|
||||||
|
prInfo: { number: 42, url: "https://github.com/acme/repo/pull/42", head: "feature", base: "main" },
|
||||||
reviewState: {
|
reviewState: {
|
||||||
source: "pull-request",
|
source: "pull-request",
|
||||||
items: [{ id: "ri-1", body: "Fix tests", summary: "Fix tests", author: { login: "reviewer" }, createdAt: new Date().toISOString(), path: "src/a.ts", line: 4 }],
|
items: [{ id: "ri-1", body: "Fix tests", summary: "Fix tests", author: { login: "reviewer" }, createdAt: "2026-01-04T05:06:07.000Z", path: "src/a.ts", line: 4 }],
|
||||||
addressing: [],
|
addressing: [],
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValueOnce(taskWithReview).mockResolvedValueOnce(taskWithReview);
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValueOnce(taskWithPrReview).mockResolvedValueOnce(taskWithPrReview);
|
||||||
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-1" });
|
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-1" });
|
||||||
|
|
||||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: "ri-1", source: "pr-review", summary: "Fix tests", body: "Fix tests", filePath: "src/a.ts", lineNumber: 4 }] }), { "Content-Type": "application/json" });
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: "ri-1", source: "pr-review", summary: "Fix tests", body: "Fix tests", filePath: "src/a.ts", lineNumber: 4 }] }), { "Content-Type": "application/json" });
|
||||||
@@ -2671,18 +2751,22 @@ describe("POST /tasks/:id/review/address", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("rejects unsupported review source", async () => {
|
it("rejects unsupported review source", async () => {
|
||||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...FAKE_TASK_DETAIL, id: "FN-001", reviewState: { source: "reviewer-agent", items: [{ id: "ri-1", body: "x", summary: "x", author: { login: "reviewer" }, createdAt: new Date().toISOString() }], addressing: [] } });
|
mockReviewerBlockLogs();
|
||||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: "ri-1", source: "other", summary: "x", body: "x" }] }), { "Content-Type": "application/json" });
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...FAKE_TASK_DETAIL, id: "FN-001", updatedAt: reviewerBlockTimestamp, reviewState: { source: "reviewer-agent", items: [], addressing: [] } });
|
||||||
|
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: reviewerBlockItemId, source: "other", summary: "x", body: "x" }] }), { "Content-Type": "application/json" });
|
||||||
expect(res.status).toBe(400);
|
expect(res.status).toBe(400);
|
||||||
expect(res.body.error).toContain("Unsupported review source");
|
expect(res.body.error).toContain("Unsupported review source");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("rejects source mismatch and unknown item ids", async () => {
|
it("rejects source mismatch and unknown item ids against canonical review data", async () => {
|
||||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
mockPrReviewDetails();
|
||||||
|
const taskWithPrReview = {
|
||||||
...FAKE_TASK_DETAIL,
|
...FAKE_TASK_DETAIL,
|
||||||
id: "FN-001",
|
id: "FN-001",
|
||||||
reviewState: { source: "pull-request", items: [{ id: "ri-1", body: "x", summary: "x", author: { login: "reviewer" }, createdAt: new Date().toISOString() }], addressing: [] },
|
prInfo: { number: 42, url: "https://github.com/acme/repo/pull/42", head: "feature", base: "main" },
|
||||||
});
|
reviewState: { source: "pull-request", items: [{ id: "ri-1", body: "x", summary: "x", author: { login: "reviewer" }, createdAt: "2026-01-04T05:06:07.000Z" }], addressing: [] },
|
||||||
|
};
|
||||||
|
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskWithPrReview);
|
||||||
|
|
||||||
const mismatch = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: "ri-1", source: "reviewer-agent", summary: "x", body: "x" }] }), { "Content-Type": "application/json" });
|
const mismatch = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ selectedItems: [{ id: "ri-1", source: "reviewer-agent", summary: "x", body: "x" }] }), { "Content-Type": "application/json" });
|
||||||
expect(mismatch.status).toBe(400);
|
expect(mismatch.status).toBe(400);
|
||||||
|
|||||||
@@ -3604,28 +3604,64 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
|||||||
if (unsupportedSource) {
|
if (unsupportedSource) {
|
||||||
throw badRequest(`Unsupported review source: ${String(unsupportedSource.source)}`);
|
throw badRequest(`Unsupported review source: ${String(unsupportedSource.source)}`);
|
||||||
}
|
}
|
||||||
if (!task.reviewState) {
|
let canonicalReviewData: TaskReviewData;
|
||||||
throw badRequest("Task has no reviewState payload");
|
if (task.prInfo) {
|
||||||
|
const badgeParsed = parseGitHubBadgeUrl(task.prInfo.url);
|
||||||
|
const repoInfo = getCurrentRepo(scopedStore.getRootDir());
|
||||||
|
const owner = badgeParsed?.owner ?? repoInfo?.owner;
|
||||||
|
const repo = badgeParsed?.repo ?? repoInfo?.repo;
|
||||||
|
if (!owner || !repo) {
|
||||||
|
throw badRequest("Could not determine GitHub repository for PR review fetch");
|
||||||
|
}
|
||||||
|
canonicalReviewData = await new GitHubClient(options?.githubToken ?? process.env.GITHUB_TOKEN).getPrReviewDetails(owner, repo, task.prInfo.number);
|
||||||
|
} else {
|
||||||
|
canonicalReviewData = await buildDirectTaskReviewData(task, scopedStore);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const canonicalReviewItems = canonicalReviewData.items.map((item) => ({
|
||||||
|
id: item.itemId,
|
||||||
|
body: item.body,
|
||||||
|
summary: item.title,
|
||||||
|
author: { login: item.author },
|
||||||
|
createdAt: item.createdAt ?? canonicalReviewData.fetchedAt ?? new Date(0).toISOString(),
|
||||||
|
updatedAt: item.updatedAt ?? undefined,
|
||||||
|
path: item.filePath,
|
||||||
|
line: item.line,
|
||||||
|
threadId: item.threadId,
|
||||||
|
htmlUrl: item.url,
|
||||||
|
state: item.reviewState ?? undefined,
|
||||||
|
isResolved: item.isResolved,
|
||||||
|
source: item.sourceMode === "reviewer-agent" ? "reviewer-agent" as const : "github-pr" as const,
|
||||||
|
}));
|
||||||
|
const reviewState = {
|
||||||
|
source: canonicalReviewData.mode,
|
||||||
|
summary: canonicalReviewData.summary ?? task.reviewState?.summary,
|
||||||
|
items: canonicalReviewItems,
|
||||||
|
addressing: task.reviewState?.addressing ?? [],
|
||||||
|
lastRefreshedAt: task.reviewState?.lastRefreshedAt ?? canonicalReviewData.fetchedAt ?? undefined,
|
||||||
|
refreshSource: task.reviewState?.refreshSource,
|
||||||
|
refreshStatus: task.reviewState?.refreshStatus,
|
||||||
|
refreshError: task.reviewState?.refreshError,
|
||||||
|
} satisfies NonNullable<Task["reviewState"]>;
|
||||||
|
|
||||||
|
/*
|
||||||
|
FNXC:TaskReview 2026-06-27-00:00:
|
||||||
|
Revision validation must use the same canonical review source the UI renders. Reviewer-agent ids come from buildReviewerAgentItemId via buildDirectTaskReviewData, not from persisted reviewState.items, because direct executor review addressing persists only addressing snapshots.
|
||||||
|
*/
|
||||||
const now = new Date().toISOString();
|
const now = new Date().toISOString();
|
||||||
const selectedSet = new Set(selectedItems.map((item: SelectedReviewItem) => item.id));
|
const selectedSet = new Set(selectedItems.map((item: SelectedReviewItem) => item.id));
|
||||||
const sourceById = new Map(selectedItems.map((item: SelectedReviewItem) => [item.id, item.source] as const));
|
const canonicalIds = new Set(reviewState.items.map((item) => item.id));
|
||||||
const reviewSourceMismatch = task.reviewState.items.find((item: NonNullable<typeof task.reviewState>["items"][number]) => {
|
const expectedSource = canonicalReviewData.mode === "pull-request" ? "pr-review" : "reviewer-agent";
|
||||||
const selectedSource = sourceById.get(item.id);
|
const reviewSourceMismatch = selectedItems.find((item) => canonicalIds.has(item.id) && item.source !== expectedSource);
|
||||||
if (!selectedSource || !task.reviewState) return false;
|
|
||||||
const expectedSource = task.reviewState.source === "pull-request" ? "pr-review" : "reviewer-agent";
|
|
||||||
return selectedSource !== expectedSource;
|
|
||||||
});
|
|
||||||
if (reviewSourceMismatch) {
|
if (reviewSourceMismatch) {
|
||||||
throw badRequest("Selected review source does not match task review mode");
|
throw badRequest("Selected review source does not match task review mode");
|
||||||
}
|
}
|
||||||
const matchedItems = task.reviewState.items.filter((item: NonNullable<typeof task.reviewState>["items"][number]) => selectedSet.has(item.id));
|
const hasUnknownSelection = selectedItems.some((item) => !canonicalIds.has(item.id));
|
||||||
if (matchedItems.length !== selectedSet.size) {
|
if (hasUnknownSelection) {
|
||||||
throw badRequest("selectedItems must reference existing review items");
|
throw badRequest("selectedItems must reference existing review items");
|
||||||
}
|
}
|
||||||
|
|
||||||
const modeSummary = `${task.reviewState.source === "pull-request" ? "pull-request" : "reviewer-agent"} · ${selectedItems.length} selected item(s)`;
|
const modeSummary = `${reviewState.source === "pull-request" ? "pull-request" : "reviewer-agent"} · ${selectedItems.length} selected item(s)`;
|
||||||
const steeringItems = selectedItems.map((item: SelectedReviewItem, index: number) => {
|
const steeringItems = selectedItems.map((item: SelectedReviewItem, index: number) => {
|
||||||
const location = item.filePath ? `${item.filePath}${typeof item.lineNumber === "number" ? `:${item.lineNumber}` : ""}` : undefined;
|
const location = item.filePath ? `${item.filePath}${typeof item.lineNumber === "number" ? `:${item.lineNumber}` : ""}` : undefined;
|
||||||
const snippetSource = item.body.trim() || item.summary.trim();
|
const snippetSource = item.body.trim() || item.summary.trim();
|
||||||
@@ -3635,9 +3671,9 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
|||||||
});
|
});
|
||||||
const steeringText = ["Selected review feedback to address", modeSummary, ...steeringItems].join("\n");
|
const steeringText = ["Selected review feedback to address", modeSummary, ...steeringItems].join("\n");
|
||||||
|
|
||||||
const priorAddressingById = new Map(task.reviewState.addressing.map((record) => [record.itemId, record] as const));
|
const priorAddressingById = new Map(reviewState.addressing.map((record) => [record.itemId, record] as const));
|
||||||
const nextAddressing = [
|
const nextAddressing = [
|
||||||
...task.reviewState.addressing.filter((record) => !selectedSet.has(record.itemId)),
|
...reviewState.addressing.filter((record) => !selectedSet.has(record.itemId)),
|
||||||
...selectedItems.map((item: SelectedReviewItem) => {
|
...selectedItems.map((item: SelectedReviewItem) => {
|
||||||
const existing = priorAddressingById.get(item.id);
|
const existing = priorAddressingById.get(item.id);
|
||||||
return {
|
return {
|
||||||
@@ -3650,7 +3686,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
|||||||
stale: false,
|
stale: false,
|
||||||
snapshot: {
|
snapshot: {
|
||||||
itemId: item.id,
|
itemId: item.id,
|
||||||
sourceMode: task.reviewState?.source ?? "pull-request",
|
sourceMode: reviewState.source,
|
||||||
source: item.source,
|
source: item.source,
|
||||||
summary: item.summary,
|
summary: item.summary,
|
||||||
body: item.body,
|
body: item.body,
|
||||||
@@ -3665,12 +3701,12 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
|||||||
}),
|
}),
|
||||||
];
|
];
|
||||||
|
|
||||||
const reviewState = {
|
const nextReviewState = {
|
||||||
...task.reviewState,
|
...reviewState,
|
||||||
addressing: nextAddressing,
|
addressing: nextAddressing,
|
||||||
};
|
};
|
||||||
|
|
||||||
await scopedStore.updateTask(task.id, { reviewState });
|
await scopedStore.updateTask(task.id, { reviewState: nextReviewState });
|
||||||
|
|
||||||
let steeringCommentId: string | null = null;
|
let steeringCommentId: string | null = null;
|
||||||
const steeringComment = await scopedStore.addSteeringComment(task.id, steeringText, "user");
|
const steeringComment = await scopedStore.addSteeringComment(task.id, steeringText, "user");
|
||||||
@@ -3704,7 +3740,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
|||||||
}
|
}
|
||||||
|
|
||||||
await scopedStore.logEntry(task.id, "Same-task review revision requested", `${selectedItems.length} item(s) submitted from review tab`);
|
await scopedStore.logEntry(task.id, "Same-task review revision requested", `${selectedItems.length} item(s) submitted from review tab`);
|
||||||
res.json({ task: updatedTask, reviewState });
|
res.json({ task: updatedTask, reviewState: nextReviewState });
|
||||||
} catch (err: unknown) {
|
} catch (err: unknown) {
|
||||||
if (err instanceof ApiError) {
|
if (err instanceof ApiError) {
|
||||||
throw err;
|
throw err;
|
||||||
|
|||||||
Reference in New Issue
Block a user