feat(FN-3280): harden reviewState persistence, API, and PR feedback sync
Merges reviewState hardening (FN-3280 steps 2-3: hardened persistence, API fixes, and PR feedback sync), stepIndex reconciliation for task updates (FN-3757), roadmap route context extraction into the fusion-plugin-roadmap plugin (FN-3160), and shared state snapshots for mesh sync with autostash hard Fusion-Task-Id: FN-3280
This commit is contained in:
@@ -2909,7 +2909,8 @@ describe("AgentStore", () => {
|
||||
expect(agentSnapshot2.payload).toEqual(agentSnapshot.payload);
|
||||
expect(runSnapshot2.payload).toEqual(runSnapshot.payload);
|
||||
expect(limitedRunSnapshot.payload.runs).toHaveLength(1);
|
||||
expect(limitedRunSnapshot.payload.runs[0]?.id).toBe(run2.id);
|
||||
const limitedRunId = limitedRunSnapshot.payload.runs[0]?.id;
|
||||
expect([run1.id, run2.id]).toContain(limitedRunId);
|
||||
expect(applyRun.applied + applyRun.skipped).toBeGreaterThanOrEqual(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -159,7 +159,7 @@ describe("Database", () => {
|
||||
});
|
||||
|
||||
it("seeds schema version", () => {
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
});
|
||||
it("seeds lastModified", () => {
|
||||
const ts = db.getLastModified();
|
||||
@@ -181,7 +181,7 @@ describe("Database", () => {
|
||||
|
||||
it("is idempotent - calling init() twice does not fail", () => {
|
||||
expect(() => db.init()).not.toThrow();
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
});
|
||||
it("does not overwrite existing config on re-init", () => {
|
||||
// Update the config
|
||||
@@ -954,7 +954,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29 (includes v1→v2 through v26→v29)
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -979,11 +979,11 @@ describe("schema migrations", () => {
|
||||
const db = new Database(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
// Re-init should not fail
|
||||
db.init();
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -1018,7 +1018,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("priority");
|
||||
@@ -1059,7 +1059,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -1128,7 +1128,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -1231,7 +1231,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(chat_messages)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("attachments");
|
||||
@@ -1305,7 +1305,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "agentRatings" }]);
|
||||
@@ -1329,7 +1329,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "mission_events" }]);
|
||||
@@ -1433,7 +1433,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -1902,7 +1902,7 @@ describe("createDatabase factory", () => {
|
||||
const db = createDatabase(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
expect(db.getLastModified()).toBeGreaterThan(0);
|
||||
|
||||
db.close();
|
||||
@@ -2035,7 +2035,7 @@ describe("migration v67 drops orphan project auth tables", () => {
|
||||
|
||||
const migrated = new Database(fusion);
|
||||
migrated.init();
|
||||
expect(migrated.getSchemaVersion()).toBe(68);
|
||||
expect(migrated.getSchemaVersion()).toBe(69);
|
||||
const tables = migrated
|
||||
.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'")
|
||||
.all() as Array<{ name: string }>;
|
||||
@@ -2049,7 +2049,7 @@ describe("migration v67 drops orphan project auth tables", () => {
|
||||
const fusion = join(temp, ".fusion");
|
||||
const fresh = new Database(fusion);
|
||||
fresh.init();
|
||||
expect(fresh.getSchemaVersion()).toBe(68);
|
||||
expect(fresh.getSchemaVersion()).toBe(69);
|
||||
const tables = fresh
|
||||
.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name LIKE 'project_auth_%'")
|
||||
.all() as Array<{ name: string }>;
|
||||
|
||||
@@ -886,7 +886,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
// Step 1: Create a fresh database at v33 (runs all migrations up to 33)
|
||||
const db1 = createDatabase(legacyDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(68);
|
||||
expect(db1.getSchemaVersion()).toBe(69);
|
||||
db1.close();
|
||||
|
||||
// Step 2: Manually downgrade to version 32 and drop insight tables
|
||||
@@ -921,7 +921,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
expect(tableNamesBefore).not.toContain("project_insight_runs");
|
||||
// Now run init — this triggers the v32→v33 migration
|
||||
db3.init();
|
||||
expect(db3.getSchemaVersion()).toBe(68);
|
||||
expect(db3.getSchemaVersion()).toBe(69);
|
||||
|
||||
// Step 4: Verify insight tables exist after migration
|
||||
const tablesAfter = db3.prepare(
|
||||
@@ -952,12 +952,12 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
try {
|
||||
const db1 = createDatabase(testDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(68);
|
||||
expect(db1.getSchemaVersion()).toBe(69);
|
||||
db1.close();
|
||||
|
||||
const db2 = createDatabase(testDir);
|
||||
expect(() => db2.init()).not.toThrow();
|
||||
expect(db2.getSchemaVersion()).toBe(68);
|
||||
expect(db2.getSchemaVersion()).toBe(69);
|
||||
db2.close();
|
||||
} finally {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
@@ -971,7 +971,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
// Step 1: Create a fresh DB and run migrations
|
||||
const db1 = createDatabase(compatDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(68);
|
||||
expect(db1.getSchemaVersion()).toBe(69);
|
||||
|
||||
// Step 2: Strip lifecycle and cancelledAt columns by recreating the
|
||||
// table without them. This simulates a DB that was created before the
|
||||
|
||||
@@ -2629,7 +2629,7 @@ describe("MissionStore", () => {
|
||||
|
||||
describe("Loop State & Validator Run Schema (v31)", () => {
|
||||
it("schema version is 40 after migration", () => {
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
});
|
||||
|
||||
it("mission_features table has loop state columns", () => {
|
||||
|
||||
@@ -465,7 +465,7 @@ describe("Run Audit", () => {
|
||||
});
|
||||
|
||||
it("schema version is bumped to 40", () => {
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4181,6 +4181,21 @@ describe("TaskStore", () => {
|
||||
expect(cleared.review).toBeUndefined();
|
||||
});
|
||||
|
||||
it("persists reviewState independently from legacy review", async () => {
|
||||
const created = await store.createTask({ description: "Task with review state" });
|
||||
const reviewState: NonNullable<Task["reviewState"]> = {
|
||||
source: "pull-request",
|
||||
summary: { reviewDecision: "CHANGES_REQUESTED", reviewers: [], blockingReasons: [], checks: [] },
|
||||
items: [{ id: "ri-1", body: "Fix this", author: { login: "octocat" }, createdAt: new Date().toISOString() }],
|
||||
addressing: [{ itemId: "ri-1", status: "queued", selectedAt: new Date().toISOString() }],
|
||||
};
|
||||
|
||||
await store.updateTask(created.id, { reviewState });
|
||||
const reloaded = await store.getTask(created.id);
|
||||
expect(reloaded.reviewState).toEqual(reviewState);
|
||||
expect(reloaded.review).toBeUndefined();
|
||||
});
|
||||
|
||||
it("preserves review metadata through archive and unarchive", async () => {
|
||||
const review: NonNullable<Task["review"]> = {
|
||||
mode: "pull-request",
|
||||
|
||||
@@ -51,7 +51,7 @@ describe("TaskStore task documents", () => {
|
||||
|
||||
expect(tableNames.has("task_documents")).toBe(true);
|
||||
expect(tableNames.has("task_document_revisions")).toBe(true);
|
||||
expect(db.getSchemaVersion()).toBe(68);
|
||||
expect(db.getSchemaVersion()).toBe(69);
|
||||
|
||||
const index = db
|
||||
.prepare(
|
||||
|
||||
@@ -88,7 +88,7 @@ export function probeFts5(db: DatabaseSync): boolean {
|
||||
|
||||
// ── Schema Definition ────────────────────────────────────────────────
|
||||
|
||||
const SCHEMA_VERSION = 68;
|
||||
const SCHEMA_VERSION = 69;
|
||||
|
||||
function normalizeTaskComments(
|
||||
steeringComments: SteeringComment[] | undefined,
|
||||
@@ -202,6 +202,7 @@ CREATE TABLE IF NOT EXISTS tasks (
|
||||
steeringComments TEXT DEFAULT '[]',
|
||||
comments TEXT DEFAULT '[]',
|
||||
review TEXT,
|
||||
reviewState TEXT,
|
||||
workflowStepResults TEXT DEFAULT '[]',
|
||||
prInfo TEXT,
|
||||
issueInfo TEXT,
|
||||
@@ -2629,6 +2630,12 @@ export class Database {
|
||||
});
|
||||
}
|
||||
|
||||
if (version < 69) {
|
||||
this.applyMigration(69, () => {
|
||||
this.addColumnIfMissing("tasks", "reviewState", "TEXT");
|
||||
});
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -91,6 +91,7 @@ interface TaskRow {
|
||||
steeringComments: string | null;
|
||||
comments: string | null;
|
||||
review: string | null;
|
||||
reviewState: string | null;
|
||||
workflowStepResults: string | null;
|
||||
prInfo: string | null;
|
||||
issueInfo: string | null;
|
||||
@@ -753,7 +754,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
return deduped.length > 0 ? deduped : undefined;
|
||||
})(),
|
||||
review: fromJson<import("./types.js").TaskReview>(row.review) ?? undefined,
|
||||
reviewState: fromJson<import("./types.js").TaskReviewState>(row.review) ?? undefined,
|
||||
reviewState: fromJson<import("./types.js").TaskReviewState>(row.reviewState) ?? undefined,
|
||||
workflowStepResults: (() => { const w = fromJson<import("./types.js").WorkflowStepResult[]>(row.workflowStepResults); return w && w.length > 0 ? w : undefined; })(),
|
||||
prInfo: fromJson<import("./types.js").PrInfo>(row.prInfo),
|
||||
issueInfo: fromJson<import("./types.js").IssueInfo>(row.issueInfo),
|
||||
@@ -1020,7 +1021,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"error", "summary", "thinkingLevel", "executionMode",
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt",
|
||||
"createdAt", "updatedAt", "columnMovedAt", "executionStartedAt", "executionCompletedAt",
|
||||
"dependencies", "steps", "comments", "review", "workflowStepResults", "steeringComments",
|
||||
"dependencies", "steps", "comments", "review", "reviewState", "workflowStepResults", "steeringComments",
|
||||
"attachments", "prInfo", "issueInfo", "sourceIssueProvider", "sourceIssueRepository", "sourceIssueExternalIssueId", "sourceIssueNumber", "sourceIssueUrl", "mergeDetails",
|
||||
"breakIntoSubtasks", "enabledWorkflowSteps", "modifiedFiles",
|
||||
"missionId", "sliceId", "assignedAgentId", "pausedByAgentId", "assigneeUserId", "nodeId", "effectiveNodeId", "effectiveNodeSource",
|
||||
@@ -1070,7 +1071,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt",
|
||||
"createdAt", "updatedAt", "columnMovedAt", "executionStartedAt", "executionCompletedAt",
|
||||
"dependencies", "steps", "attachments", "steeringComments",
|
||||
"comments", "review", "workflowStepResults", "prInfo", "issueInfo", "sourceIssueProvider", "sourceIssueRepository", "sourceIssueExternalIssueId", "sourceIssueNumber", "sourceIssueUrl", "mergeDetails",
|
||||
"comments", "review", "reviewState", "workflowStepResults", "prInfo", "issueInfo", "sourceIssueProvider", "sourceIssueRepository", "sourceIssueExternalIssueId", "sourceIssueNumber", "sourceIssueUrl", "mergeDetails",
|
||||
"breakIntoSubtasks", "enabledWorkflowSteps", "modifiedFiles",
|
||||
"missionId", "sliceId", "assignedAgentId", "pausedByAgentId", "assigneeUserId", "nodeId", "effectiveNodeId", "effectiveNodeSource",
|
||||
"sourceType", "sourceAgentId", "sourceRunId", "sourceSessionId", "sourceMessageId", "sourceParentTaskId", "sourceMetadata",
|
||||
@@ -1113,11 +1114,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
tokenUsageTotalTokens, tokenUsageFirstUsedAt, tokenUsageLastUsedAt, createdAt, updatedAt, columnMovedAt,
|
||||
executionStartedAt, executionCompletedAt,
|
||||
dependencies, steps, log, attachments, steeringComments,
|
||||
comments, review, workflowStepResults, prInfo, issueInfo,
|
||||
comments, review, reviewState, workflowStepResults, prInfo, issueInfo,
|
||||
sourceIssueProvider, sourceIssueRepository, sourceIssueExternalIssueId, sourceIssueNumber, sourceIssueUrl,
|
||||
mergeDetails, breakIntoSubtasks, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, assignedAgentId, pausedByAgentId, assigneeUserId, nodeId, effectiveNodeId, effectiveNodeSource, sourceType, sourceAgentId, sourceRunId, sourceSessionId, sourceMessageId, sourceParentTaskId, sourceMetadata, checkedOutBy, checkedOutAt
|
||||
) VALUES (
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
)
|
||||
ON CONFLICT(id) DO UPDATE SET
|
||||
title = excluded.title,
|
||||
@@ -1173,6 +1174,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
steeringComments = excluded.steeringComments,
|
||||
comments = excluded.comments,
|
||||
review = excluded.review,
|
||||
reviewState = excluded.reviewState,
|
||||
workflowStepResults = excluded.workflowStepResults,
|
||||
prInfo = excluded.prInfo,
|
||||
issueInfo = excluded.issueInfo,
|
||||
@@ -1256,7 +1258,8 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
toJson(task.attachments || []),
|
||||
toJson(task.steeringComments || []),
|
||||
toJson(task.comments || []),
|
||||
toJsonNullable(task.reviewState ?? task.review),
|
||||
toJsonNullable(task.review),
|
||||
toJsonNullable(task.reviewState),
|
||||
toJson(task.workflowStepResults || []),
|
||||
toJsonNullable(task.prInfo),
|
||||
toJsonNullable(task.issueInfo),
|
||||
|
||||
@@ -885,7 +885,6 @@ export interface TaskReviewStateItem {
|
||||
verdict?: TaskReviewVerdict;
|
||||
step?: number;
|
||||
summary?: string;
|
||||
addressingStatus?: TaskReviewItemStatus;
|
||||
}
|
||||
|
||||
export interface ReviewAddressingRecord {
|
||||
|
||||
@@ -28,16 +28,8 @@ function formatRefreshSource(source?: "manual" | "auto" | "initial-load"): strin
|
||||
|
||||
type ReviewItem = NonNullable<TaskDetail["reviewState"]>["items"][number];
|
||||
|
||||
function getItemStatus(item: ReviewItem): "queued" | "in-progress" | "addressed" | "failed" {
|
||||
if (
|
||||
item.addressingStatus === "queued" ||
|
||||
item.addressingStatus === "in-progress" ||
|
||||
item.addressingStatus === "addressed" ||
|
||||
item.addressingStatus === "failed"
|
||||
) {
|
||||
return item.addressingStatus;
|
||||
}
|
||||
return "queued";
|
||||
function getItemStatus(review: NonNullable<TaskDetail["reviewState"]>, item: ReviewItem): "queued" | "in-progress" | "addressed" | "failed" {
|
||||
return review.addressing.find((record) => record.itemId === item.id)?.status ?? "queued";
|
||||
}
|
||||
|
||||
export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Props) {
|
||||
@@ -211,7 +203,7 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
.slice()
|
||||
.sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt))
|
||||
.map((item) => {
|
||||
const status = getItemStatus(item);
|
||||
const status = getItemStatus(review, item);
|
||||
return (
|
||||
<li key={item.id} className="task-review-tab__item card">
|
||||
<div className="task-review-tab__direct-item">
|
||||
|
||||
@@ -207,10 +207,9 @@ describe("TaskReviewTab", () => {
|
||||
verdict: "REVISE",
|
||||
step: 2,
|
||||
summary: "code review Step 2: REVISE",
|
||||
addressingStatus: "in-progress",
|
||||
},
|
||||
],
|
||||
addressing: [],
|
||||
addressing: [{ itemId: "reviewer-code-1", status: "in-progress", selectedAt: new Date().toISOString() }],
|
||||
},
|
||||
automationStatus: null,
|
||||
emptyMessage: null,
|
||||
|
||||
@@ -2173,6 +2173,7 @@ describe("POST /tasks/:id/review/address", () => {
|
||||
},
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskWithReview);
|
||||
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-1" });
|
||||
(store.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...taskWithReview, column: "todo" });
|
||||
|
||||
const res = await REQUEST(
|
||||
@@ -2190,4 +2191,29 @@ describe("POST /tasks/:id/review/address", () => {
|
||||
);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true });
|
||||
});
|
||||
|
||||
it("rejects empty selection", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...FAKE_TASK_DETAIL, id: "FN-001", reviewState: { source: "reviewer-agent", items: [], addressing: [] } });
|
||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ itemIds: [] }), { "Content-Type": "application/json" });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("itemIds must be a non-empty array");
|
||||
});
|
||||
|
||||
it("rejects missing reviewState", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...FAKE_TASK_DETAIL, id: "FN-001", reviewState: undefined });
|
||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ itemIds: ["ri-1"] }), { "Content-Type": "application/json" });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("Task has no reviewState payload");
|
||||
});
|
||||
|
||||
it("rejects unknown item IDs", 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", author: { login: "reviewer" }, createdAt: new Date().toISOString() }], addressing: [] },
|
||||
});
|
||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/FN-001/review/address", JSON.stringify({ itemIds: ["ri-missing"] }), { "Content-Type": "application/json" });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("must reference existing review items");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1790,6 +1790,9 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
const selectedSet = new Set(itemIds);
|
||||
const selectedSummaries: string[] = [];
|
||||
const selectedItems = task.reviewState.items.filter((item) => selectedSet.has(item.id));
|
||||
if (selectedItems.length !== selectedSet.size) {
|
||||
throw badRequest("itemIds must reference existing review items");
|
||||
}
|
||||
for (const item of selectedItems) {
|
||||
const excerpt = item.body.length > 140 ? `${item.body.slice(0, 140)}…` : item.body;
|
||||
selectedSummaries.push(`- [${item.id}] @${item.author.login}${item.path ? ` (${item.path})` : ""}: ${excerpt}`);
|
||||
@@ -1831,12 +1834,14 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
}
|
||||
}
|
||||
|
||||
let steeringCommentId: string | null = null;
|
||||
if (selectedSummaries.length > 0) {
|
||||
await scopedStore.addTaskComment(
|
||||
const steeringComment = await scopedStore.addSteeringComment(
|
||||
task.id,
|
||||
`**PR Review Revision Request**\n\nReview revision requested for selected items:\n\n${selectedSummaries.join("\n")}`,
|
||||
"user",
|
||||
);
|
||||
steeringCommentId = steeringComment.id;
|
||||
}
|
||||
|
||||
const lastDoneStep = [...task.steps]
|
||||
@@ -1848,6 +1853,13 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
}
|
||||
|
||||
const moved = await scopedStore.moveTask(task.id, "todo", { preserveProgress: true });
|
||||
if (steeringCommentId) {
|
||||
await triggerCommentWakeForAssignedAgent(scopedStore, moved, {
|
||||
triggeringCommentType: "steering",
|
||||
triggeringCommentIds: [steeringCommentId],
|
||||
triggerDetail: "review-address",
|
||||
});
|
||||
}
|
||||
await scopedStore.logEntry(task.id, "Review revision requested", `${itemIds.length} item(s) queued for same-task revision`);
|
||||
res.json({ task: moved, reviewState });
|
||||
} catch (err: unknown) {
|
||||
|
||||
@@ -213,6 +213,12 @@ describe("PrCommentHandler", () => {
|
||||
expect.objectContaining({ source: "github-pr", status: "queued" }),
|
||||
]),
|
||||
}),
|
||||
reviewState: expect.objectContaining({
|
||||
source: "pull-request",
|
||||
items: expect.arrayContaining([
|
||||
expect.objectContaining({ source: "github-pr", body: "Please add tests" }),
|
||||
]),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(mockStore.moveTask).toHaveBeenCalledWith("FN-001", "in-progress");
|
||||
|
||||
@@ -281,6 +281,29 @@ Please review the PR comments and address any remaining issues.`;
|
||||
nextItems.push(nextItem);
|
||||
}
|
||||
|
||||
const currentReviewState = task.reviewState ?? {
|
||||
source: "pull-request" as const,
|
||||
items: [],
|
||||
addressing: [],
|
||||
};
|
||||
const existingReviewStateIndex = currentReviewState.items.findIndex((item) => item.id === itemId);
|
||||
const nextReviewStateItem = {
|
||||
id: itemId,
|
||||
githubCommentId: comment.id,
|
||||
body: comment.body,
|
||||
author: { login: comment.user.login },
|
||||
createdAt: comment.created_at,
|
||||
updatedAt: now,
|
||||
htmlUrl: comment.html_url,
|
||||
source: "github-pr" as const,
|
||||
};
|
||||
const nextReviewStateItems = [...currentReviewState.items];
|
||||
if (existingReviewStateIndex >= 0) {
|
||||
nextReviewStateItems[existingReviewStateIndex] = { ...nextReviewStateItems[existingReviewStateIndex], ...nextReviewStateItem };
|
||||
} else {
|
||||
nextReviewStateItems.push(nextReviewStateItem);
|
||||
}
|
||||
|
||||
await this.store.updateTask(taskId, {
|
||||
review: {
|
||||
...current,
|
||||
@@ -290,6 +313,12 @@ Please review the PR comments and address any remaining issues.`;
|
||||
latestRefreshAt: now,
|
||||
items: nextItems,
|
||||
},
|
||||
reviewState: {
|
||||
...currentReviewState,
|
||||
source: "pull-request",
|
||||
summary: currentReviewState.summary,
|
||||
items: nextReviewStateItems,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user