feat(FN-3282): add durable review addressing snapshots
This merge adds two substantial features: a rooms and chat sidebar system (FN-3806) with a new `CreateRoomModal` and enriched `ChatView`, wired through plugin routes and a new `listTasksModifiedSince` core API; and a durable review addressing workflow (FN-3282) that persists snapshots, renders addre Fusion-Task-Id: FN-3282
This commit is contained in:
@@ -4183,11 +4183,24 @@ describe("TaskStore", () => {
|
||||
|
||||
it("persists reviewState independently from legacy review", async () => {
|
||||
const created = await store.createTask({ description: "Task with review state" });
|
||||
const selectedAt = new Date().toISOString();
|
||||
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() }],
|
||||
items: [{ id: "ri-1", body: "Fix this", author: { login: "octocat" }, createdAt: selectedAt }],
|
||||
addressing: [{
|
||||
itemId: "ri-1",
|
||||
status: "queued",
|
||||
selectedAt,
|
||||
snapshot: {
|
||||
itemId: "ri-1",
|
||||
sourceMode: "pull-request",
|
||||
source: "pr-review",
|
||||
summary: "Fix this",
|
||||
body: "Fix this",
|
||||
authorLogin: "octocat",
|
||||
},
|
||||
}],
|
||||
};
|
||||
|
||||
await store.updateTask(created.id, { reviewState });
|
||||
@@ -4196,6 +4209,38 @@ describe("TaskStore", () => {
|
||||
expect(reloaded.review).toBeUndefined();
|
||||
});
|
||||
|
||||
it("hydrates legacy addressing records with snapshots", async () => {
|
||||
const created = await store.createTask({ description: "Legacy review state" });
|
||||
const selectedAt = new Date().toISOString();
|
||||
await store.updateTask(created.id, {
|
||||
reviewState: {
|
||||
source: "reviewer-agent",
|
||||
items: [{
|
||||
id: "review-1",
|
||||
body: "Update tests for regression",
|
||||
summary: "Update tests",
|
||||
author: { login: "reviewer" },
|
||||
createdAt: selectedAt,
|
||||
source: "reviewer-agent",
|
||||
}],
|
||||
addressing: [{ itemId: "review-1", status: "queued", selectedAt }],
|
||||
},
|
||||
});
|
||||
|
||||
const reloaded = await store.getTask(created.id);
|
||||
expect(reloaded.reviewState?.addressing[0].snapshot).toEqual({
|
||||
itemId: "review-1",
|
||||
sourceMode: "reviewer-agent",
|
||||
source: "reviewer-agent",
|
||||
summary: "Update tests",
|
||||
body: "Update tests for regression",
|
||||
authorLogin: "reviewer",
|
||||
filePath: undefined,
|
||||
threadId: undefined,
|
||||
url: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves review metadata through archive and unarchive", async () => {
|
||||
const review: NonNullable<Task["review"]> = {
|
||||
mode: "pull-request",
|
||||
|
||||
@@ -179,6 +179,40 @@ interface ActivityLogRow {
|
||||
metadata: string | null;
|
||||
}
|
||||
|
||||
function normalizeTaskReviewState(reviewState: Task["reviewState"] | undefined): Task["reviewState"] | undefined {
|
||||
if (!reviewState) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const itemsById = new Map(reviewState.items.map((item) => [item.id, item]));
|
||||
const sourceMode = reviewState.source;
|
||||
const normalizedAddressing = reviewState.addressing.map((record) => {
|
||||
const item = itemsById.get(record.itemId);
|
||||
const source = item?.source === "reviewer-agent" ? "reviewer-agent" : "pr-review";
|
||||
const summary = item?.summary?.trim() || item?.body?.trim().slice(0, 160) || `Review item ${record.itemId}`;
|
||||
const body = item?.body ?? summary;
|
||||
return {
|
||||
...record,
|
||||
snapshot: record.snapshot ?? {
|
||||
itemId: record.itemId,
|
||||
sourceMode,
|
||||
source,
|
||||
summary,
|
||||
body,
|
||||
authorLogin: item?.author?.login,
|
||||
filePath: item?.path,
|
||||
threadId: item?.threadId,
|
||||
url: item?.htmlUrl,
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
return {
|
||||
...reviewState,
|
||||
addressing: normalizedAddressing,
|
||||
};
|
||||
}
|
||||
|
||||
const TASK_ACTIVITY_LOG_ENTRY_LIMIT = 1_000;
|
||||
const TASK_ACTIVITY_LOG_OUTCOME_LIMIT = 4_000;
|
||||
const ARCHIVE_AGENT_LOG_SNAPSHOT_LIMIT = 25;
|
||||
@@ -754,7 +788,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.reviewState) ?? undefined,
|
||||
reviewState: normalizeTaskReviewState(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),
|
||||
@@ -3516,7 +3550,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
if (updates.reviewState === null) {
|
||||
task.reviewState = undefined;
|
||||
} else if (updates.reviewState !== undefined) {
|
||||
task.reviewState = updates.reviewState;
|
||||
task.reviewState = normalizeTaskReviewState(updates.reviewState);
|
||||
}
|
||||
if (updates.workflowStepResults === null) {
|
||||
task.workflowStepResults = undefined;
|
||||
|
||||
@@ -792,14 +792,30 @@ export interface TaskReviewStateItem {
|
||||
summary?: string;
|
||||
}
|
||||
|
||||
export type ReviewAddressingStatus = "queued" | "in-progress" | "addressed" | "failed";
|
||||
|
||||
export interface ReviewAddressingSnapshot {
|
||||
itemId: string;
|
||||
sourceMode: "pull-request" | "reviewer-agent";
|
||||
source: "pr-review" | "reviewer-agent";
|
||||
summary: string;
|
||||
body: string;
|
||||
authorLogin?: string;
|
||||
filePath?: string;
|
||||
lineNumber?: number;
|
||||
threadId?: string;
|
||||
url?: string;
|
||||
}
|
||||
|
||||
export interface ReviewAddressingRecord {
|
||||
itemId: string;
|
||||
status: "queued" | "in-progress" | "addressed" | "failed";
|
||||
status: ReviewAddressingStatus;
|
||||
selectedAt: string;
|
||||
startedAt?: string;
|
||||
completedAt?: string;
|
||||
error?: string;
|
||||
stale?: boolean;
|
||||
snapshot?: ReviewAddressingSnapshot;
|
||||
}
|
||||
|
||||
export interface ReviewerTaskReviewSummary {
|
||||
|
||||
@@ -13,8 +13,22 @@ interface Props {
|
||||
}
|
||||
|
||||
const REVIEW_LOAD_ERROR_MESSAGE = "Failed to load review data.";
|
||||
const DIRECT_MODE_EMPTY_MESSAGE =
|
||||
"No reviewer feedback yet — this task has not produced reviewer-agent feedback in direct mode.";
|
||||
const DIRECT_MODE_EMPTY_MESSAGE = "No reviewer feedback yet — this task has not produced reviewer-agent feedback in direct mode.";
|
||||
|
||||
type ReviewState = NonNullable<TaskDetail["reviewState"]>;
|
||||
type ReviewItem = ReviewState["items"][number];
|
||||
type AddressingRecord = ReviewState["addressing"][number];
|
||||
|
||||
type DisplayReviewItem = {
|
||||
id: string;
|
||||
summary: string;
|
||||
body: string;
|
||||
path?: string;
|
||||
createdAt?: string;
|
||||
status: "queued" | "in-progress" | "addressed" | "failed";
|
||||
addressing?: AddressingRecord;
|
||||
item?: ReviewItem;
|
||||
};
|
||||
|
||||
function formatTimestamp(value?: string): string {
|
||||
if (!value) return "Never";
|
||||
@@ -27,10 +41,36 @@ function formatRefreshSource(source?: "manual" | "auto" | "initial-load"): strin
|
||||
return "Initial load";
|
||||
}
|
||||
|
||||
type ReviewItem = NonNullable<TaskDetail["reviewState"]>["items"][number];
|
||||
function getDisplayReviewItems(review: ReviewState): DisplayReviewItem[] {
|
||||
const addressingById = new Map(review.addressing.map((record) => [record.itemId, record] as const));
|
||||
const items = review.items.map((item) => {
|
||||
const addressing = addressingById.get(item.id);
|
||||
return {
|
||||
id: item.id,
|
||||
summary: item.summary ?? item.body.slice(0, 120),
|
||||
body: item.body,
|
||||
path: item.path,
|
||||
createdAt: item.createdAt,
|
||||
status: addressing?.status ?? "queued",
|
||||
addressing,
|
||||
item,
|
||||
} satisfies DisplayReviewItem;
|
||||
});
|
||||
|
||||
function getItemStatus(review: NonNullable<TaskDetail["reviewState"]>, item: ReviewItem): "queued" | "in-progress" | "addressed" | "failed" {
|
||||
return review.addressing.find((record) => record.itemId === item.id)?.status ?? "queued";
|
||||
const existingIds = new Set(items.map((item) => item.id));
|
||||
const snapshots = review.addressing
|
||||
.filter((record) => !existingIds.has(record.itemId) && record.snapshot)
|
||||
.map((record) => ({
|
||||
id: record.itemId,
|
||||
summary: record.snapshot?.summary ?? record.itemId,
|
||||
body: record.snapshot?.body ?? record.snapshot?.summary ?? record.itemId,
|
||||
path: record.snapshot?.filePath,
|
||||
createdAt: record.selectedAt,
|
||||
status: record.status,
|
||||
addressing: record,
|
||||
} satisfies DisplayReviewItem));
|
||||
|
||||
return [...items, ...snapshots];
|
||||
}
|
||||
|
||||
export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Props) {
|
||||
@@ -44,6 +84,7 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
|
||||
const canRevise = selected.length > 0 && !revising;
|
||||
const isPrMode = review?.source === "pull-request";
|
||||
const displayItems = useMemo(() => (review ? getDisplayReviewItems(review) : []), [review]);
|
||||
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
@@ -71,11 +112,11 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
if (!review) return "No review feedback captured yet.";
|
||||
if (review.source === "pull-request") {
|
||||
const prSummary = review.summary as { reviewDecision?: string } | undefined;
|
||||
return `${prSummary?.reviewDecision ?? "REVIEW_REQUIRED"} · ${review.items.length} review item(s)`;
|
||||
return `${prSummary?.reviewDecision ?? "REVIEW_REQUIRED"} · ${displayItems.length} review item(s)`;
|
||||
}
|
||||
const reviewerSummary = review.summary as { summary?: string } | undefined;
|
||||
return `${reviewerSummary?.summary ?? "reviewer-agent"} · ${review.items.length} review item(s)`;
|
||||
}, [review]);
|
||||
return `${reviewerSummary?.summary ?? "reviewer-agent"} · ${displayItems.length} review item(s)`;
|
||||
}, [review, displayItems.length]);
|
||||
|
||||
const decisionLabel = !review
|
||||
? undefined
|
||||
@@ -84,22 +125,13 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
: (review.summary as { verdict?: string } | undefined)?.verdict;
|
||||
|
||||
const refreshStatus = refreshing ? "refreshing" : (review?.refreshStatus ?? "ready");
|
||||
const refreshToneClass =
|
||||
refreshStatus === "error"
|
||||
? "status-dot status-dot--error"
|
||||
: refreshStatus === "refreshing"
|
||||
? "status-dot status-dot--pending"
|
||||
: "status-dot status-dot--online";
|
||||
const refreshLabel =
|
||||
refreshStatus === "error"
|
||||
? "Refresh failed"
|
||||
: refreshStatus === "refreshing"
|
||||
? "Refreshing"
|
||||
: "Up to date";
|
||||
const refreshToneClass = refreshStatus === "error"
|
||||
? "status-dot status-dot--error"
|
||||
: refreshStatus === "refreshing"
|
||||
? "status-dot status-dot--pending"
|
||||
: "status-dot status-dot--online";
|
||||
|
||||
const toggleSelected = (id: string) => {
|
||||
setSelected((prev) => (prev.includes(id) ? prev.filter((value) => value !== id) : [...prev, id]));
|
||||
};
|
||||
const toggleSelected = (id: string) => setSelected((prev) => (prev.includes(id) ? prev.filter((v) => v !== id) : [...prev, id]));
|
||||
|
||||
const onRefresh = async () => {
|
||||
try {
|
||||
@@ -114,7 +146,6 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
addToast(refreshMessage, "error");
|
||||
return;
|
||||
}
|
||||
setError(null);
|
||||
addToast("Review refreshed", "success");
|
||||
} catch (refreshError) {
|
||||
const message = refreshError instanceof Error ? refreshError.message : REVIEW_LOAD_ERROR_MESSAGE;
|
||||
@@ -130,22 +161,37 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
if (!review) return;
|
||||
setError(null);
|
||||
setRevising(true);
|
||||
const selectedItems: SelectedReviewItem[] = review.items
|
||||
const selectedItems: SelectedReviewItem[] = displayItems
|
||||
.filter((item) => selected.includes(item.id))
|
||||
.map((item) => {
|
||||
const itemRecord = item as unknown as Record<string, unknown>;
|
||||
if (!item.item) {
|
||||
return {
|
||||
id: item.id,
|
||||
source: review.source === "pull-request" ? "pr-review" : "reviewer-agent",
|
||||
threadId: item.addressing?.snapshot?.threadId,
|
||||
filePath: item.addressing?.snapshot?.filePath,
|
||||
lineNumber: item.addressing?.snapshot?.lineNumber,
|
||||
author: item.addressing?.snapshot?.authorLogin,
|
||||
summary: item.summary,
|
||||
body: item.body,
|
||||
url: item.addressing?.snapshot?.url,
|
||||
};
|
||||
}
|
||||
|
||||
const itemRecord = item.item as unknown as Record<string, unknown>;
|
||||
return {
|
||||
id: item.id,
|
||||
id: item.item.id,
|
||||
source: review.source === "pull-request" ? "pr-review" : "reviewer-agent",
|
||||
threadId: typeof itemRecord.threadId === "string" ? itemRecord.threadId : undefined,
|
||||
filePath: item.path,
|
||||
filePath: item.item.path,
|
||||
lineNumber: typeof itemRecord.line === "number" ? itemRecord.line : undefined,
|
||||
author: item.author?.login,
|
||||
summary: item.summary ?? item.body.slice(0, 120),
|
||||
body: item.body,
|
||||
author: item.item.author?.login,
|
||||
summary: item.item.summary ?? item.item.body.slice(0, 120),
|
||||
body: item.item.body,
|
||||
url: typeof itemRecord.url === "string" ? itemRecord.url : undefined,
|
||||
};
|
||||
});
|
||||
|
||||
const result = await reviseTaskReviewItems(task.id, selectedItems, projectId);
|
||||
setReview(result.reviewState);
|
||||
onTaskUpdated?.({ ...result.task, reviewState: result.reviewState } as Task);
|
||||
@@ -165,9 +211,7 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
<div className="task-review-tab__header">
|
||||
<div className="task-review-tab__summary-wrap">
|
||||
<p className="task-review-tab__summary">{summaryText}</p>
|
||||
{decisionLabel ? (
|
||||
<span className={`task-review-tab__decision task-review-tab__decision--${decisionLabel}`}>{decisionLabel}</span>
|
||||
) : null}
|
||||
{decisionLabel ? <span className={`task-review-tab__decision task-review-tab__decision--${decisionLabel}`}>{decisionLabel}</span> : null}
|
||||
</div>
|
||||
<div className="task-review-tab__actions">
|
||||
<button className="btn btn-sm" onClick={onRefresh} disabled={refreshing || loading}>{refreshing ? "Refreshing…" : "Refresh"}</button>
|
||||
@@ -176,77 +220,32 @@ export function TaskReviewTab({ task, projectId, onTaskUpdated, addToast }: Prop
|
||||
</div>
|
||||
<div className="task-review-tab__meta task-review-tab__refresh-meta" aria-live="polite">
|
||||
<span className={refreshToneClass} aria-hidden="true" />
|
||||
<span>{refreshLabel} · Last refreshed: {formatTimestamp(review?.lastRefreshedAt)} · {formatRefreshSource(review?.refreshSource)}</span>
|
||||
<span>{refreshStatus === "error" ? "Refresh failed" : refreshStatus === "refreshing" ? "Refreshing" : "Up to date"} · Last refreshed: {formatTimestamp(review?.lastRefreshedAt)} · {formatRefreshSource(review?.refreshSource)}</span>
|
||||
</div>
|
||||
{loading ? <div className="task-review-tab__meta">Loading review data…</div> : null}
|
||||
{!loading && error ? <div className="task-review-tab__error">{error}</div> : null}
|
||||
{!loading && !error && !isPrMode && review?.items?.length === 0 ? (
|
||||
<div className="task-review-tab__empty">{emptyMessage ?? DIRECT_MODE_EMPTY_MESSAGE}</div>
|
||||
) : null}
|
||||
{isPrMode && review?.summary && "reviewers" in review.summary && review.summary.reviewers?.length ? (
|
||||
<ul className="task-review-tab__reviewers">
|
||||
{review.summary.reviewers.map((reviewer) => (
|
||||
<li key={`${reviewer.login}-${reviewer.state}`} className="task-review-tab__reviewer">@{reviewer.login} · {reviewer.state}</li>
|
||||
{!loading && !error && !isPrMode && displayItems.length === 0 ? <div className="task-review-tab__empty">{emptyMessage ?? DIRECT_MODE_EMPTY_MESSAGE}</div> : null}
|
||||
{!loading && !error && displayItems.length > 0 ? (
|
||||
<ul className="task-review-tab__list">
|
||||
{displayItems.map((item) => (
|
||||
<li key={item.id} className="task-review-tab__item card">
|
||||
<label className="task-review-tab__direct-item task-review-tab__direct-item--selectable">
|
||||
<div className="task-review-tab__summary-wrap">
|
||||
<input type="checkbox" checked={selected.includes(item.id)} onChange={() => toggleSelected(item.id)} />
|
||||
<span className="task-review-tab__item-summary">{item.path ? `${item.path}: ` : ""}{item.summary}</span>
|
||||
<span className={`task-review-tab__status task-review-tab__status--${item.status}`}>{item.status}</span>
|
||||
</div>
|
||||
<div className="task-review-tab__meta">{formatTimestamp(item.createdAt)}</div>
|
||||
{item.addressing ? (
|
||||
<div className="task-review-tab__meta">Selected: {formatTimestamp(item.addressing.selectedAt)}{item.addressing.startedAt ? ` · Started: ${formatTimestamp(item.addressing.startedAt)}` : ""}{item.addressing.completedAt ? ` · Completed: ${formatTimestamp(item.addressing.completedAt)}` : ""}{item.addressing.error ? ` · Error: ${item.addressing.error}` : ""}</div>
|
||||
) : null}
|
||||
<pre className="task-review-tab__body">{item.body}</pre>
|
||||
</label>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
) : null}
|
||||
{isPrMode && review?.summary && "blockingReasons" in review.summary && review.summary.blockingReasons?.length ? (
|
||||
<ul className="task-review-tab__blockers">
|
||||
{review.summary.blockingReasons.map((reason) => <li key={reason}>{reason}</li>)}
|
||||
</ul>
|
||||
) : null}
|
||||
{isPrMode && review?.items?.length ? (
|
||||
<ul className="task-review-tab__list">
|
||||
{review.items.map((item) => {
|
||||
const status = review?.addressing.find((record) => record.itemId === item.id)?.status ?? "queued";
|
||||
return (
|
||||
<li key={item.id} className="task-review-tab__item card">
|
||||
<label className="task-review-tab__row">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={selected.includes(item.id)}
|
||||
onChange={() => toggleSelected(item.id)}
|
||||
/>
|
||||
<span className="task-review-tab__item-summary">{item.path ? `${item.path}: ` : ""}{item.body}</span>
|
||||
<span className={`task-review-tab__status task-review-tab__status--${status}`}>{status}</span>
|
||||
</label>
|
||||
</li>
|
||||
);
|
||||
})}
|
||||
</ul>
|
||||
) : null}
|
||||
{!loading && !error && !isPrMode && review?.items?.length ? (
|
||||
<ul className="task-review-tab__list">
|
||||
{review.items
|
||||
.slice()
|
||||
.sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt))
|
||||
.map((item) => {
|
||||
const status = getItemStatus(review, item);
|
||||
return (
|
||||
<li key={item.id} className="task-review-tab__item card">
|
||||
<label className="task-review-tab__direct-item task-review-tab__direct-item--selectable">
|
||||
<div className="task-review-tab__summary-wrap">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={selected.includes(item.id)}
|
||||
onChange={() => toggleSelected(item.id)}
|
||||
/>
|
||||
<span className="task-review-tab__decision">reviewer-agent</span>
|
||||
{item.reviewType ? <span className="task-review-tab__meta">{item.reviewType} review</span> : null}
|
||||
{typeof item.step === "number" ? <span className="task-review-tab__meta">Step {item.step}</span> : null}
|
||||
{item.verdict ? <span className="task-review-tab__decision">{item.verdict}</span> : null}
|
||||
<span className={`task-review-tab__status task-review-tab__status--${status}`}>{status}</span>
|
||||
</div>
|
||||
{item.summary ? <p className="task-review-tab__summary">{item.summary}</p> : null}
|
||||
<div className="task-review-tab__meta">{formatTimestamp(item.createdAt)}</div>
|
||||
<pre className="task-review-tab__body">{item.body}</pre>
|
||||
</label>
|
||||
</li>
|
||||
);
|
||||
})}
|
||||
</ul>
|
||||
) : null}
|
||||
{isPrMode && !loading && !error && !review?.items?.length ? <div className="task-review-tab__empty">No review items yet.</div> : null}
|
||||
{isPrMode && !loading && !error && displayItems.length === 0 ? <div className="task-review-tab__empty">No review items yet.</div> : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -50,7 +50,7 @@ describe("TaskReviewTab", () => {
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Refresh" }));
|
||||
expect(apiMocks.refreshTaskReview).toHaveBeenCalledWith(task.id, undefined);
|
||||
expect(await screen.findByText("APPROVED")).toBeInTheDocument();
|
||||
expect(screen.getByText("Looks good")).toBeInTheDocument();
|
||||
expect(screen.getAllByText("Looks good").length).toBeGreaterThan(0);
|
||||
expect(addToast).toHaveBeenCalledWith("Review refreshed", "success");
|
||||
});
|
||||
|
||||
@@ -187,7 +187,7 @@ describe("TaskReviewTab", () => {
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Refresh" }));
|
||||
|
||||
expect((await screen.findAllByText("APPROVE")).length).toBeGreaterThan(0);
|
||||
expect(screen.getByText("Step 3")).toBeInTheDocument();
|
||||
expect(screen.getByText("code review Step 3: APPROVE")).toBeInTheDocument();
|
||||
expect(addToast).toHaveBeenCalledWith("Review refreshed", "success");
|
||||
});
|
||||
|
||||
@@ -216,11 +216,40 @@ describe("TaskReviewTab", () => {
|
||||
});
|
||||
|
||||
render(<TaskReviewTab task={task} addToast={vi.fn()} />);
|
||||
expect(await screen.findByText("reviewer-agent")).toBeInTheDocument();
|
||||
expect(screen.getByText("Step 2")).toBeInTheDocument();
|
||||
expect(await screen.findByText("code review Step 2: REVISE")).toBeInTheDocument();
|
||||
expect(screen.getAllByText("REVISE").length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("renders persisted addressing snapshot entries after reload", async () => {
|
||||
const task = makeTask();
|
||||
apiMocks.fetchTaskReview.mockResolvedValue({
|
||||
reviewState: {
|
||||
source: "pull-request",
|
||||
summary: { reviewDecision: "CHANGES_REQUESTED", reviewers: [], blockingReasons: [], checks: [] },
|
||||
items: [],
|
||||
addressing: [{
|
||||
itemId: "ri-stale",
|
||||
status: "failed",
|
||||
selectedAt: new Date().toISOString(),
|
||||
error: "Patch failed",
|
||||
snapshot: {
|
||||
itemId: "ri-stale",
|
||||
sourceMode: "pull-request",
|
||||
source: "pr-review",
|
||||
summary: "Fix edge case",
|
||||
body: "Fix edge case in parser",
|
||||
},
|
||||
}],
|
||||
},
|
||||
automationStatus: null,
|
||||
emptyMessage: null,
|
||||
});
|
||||
|
||||
render(<TaskReviewTab task={task} addToast={vi.fn()} />);
|
||||
expect(await screen.findByText("Fix edge case")).toBeInTheDocument();
|
||||
expect(screen.getByText(/Error: Patch failed/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("submits reviewer-agent selections through same revision action", async () => {
|
||||
const task = makeTask();
|
||||
apiMocks.fetchTaskReview.mockResolvedValue({
|
||||
|
||||
@@ -1832,9 +1832,34 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
});
|
||||
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 nextAddressing = [
|
||||
...task.reviewState.addressing.filter((record) => !selectedSet.has(record.itemId)),
|
||||
...selectedItems.map((item: SelectedReviewItem) => ({ itemId: item.id, status: "queued" as const, selectedAt: now })),
|
||||
...selectedItems.map((item: SelectedReviewItem) => {
|
||||
const existing = priorAddressingById.get(item.id);
|
||||
return {
|
||||
itemId: item.id,
|
||||
status: "queued" as const,
|
||||
selectedAt: now,
|
||||
startedAt: undefined,
|
||||
completedAt: undefined,
|
||||
error: undefined,
|
||||
stale: false,
|
||||
snapshot: {
|
||||
itemId: item.id,
|
||||
sourceMode: task.reviewState?.source ?? "pull-request",
|
||||
source: item.source,
|
||||
summary: item.summary,
|
||||
body: item.body,
|
||||
authorLogin: item.author,
|
||||
filePath: item.filePath,
|
||||
lineNumber: item.lineNumber,
|
||||
threadId: item.threadId,
|
||||
url: item.url,
|
||||
},
|
||||
...(existing ? { startedAt: existing.startedAt, completedAt: existing.completedAt } : {}),
|
||||
};
|
||||
}),
|
||||
];
|
||||
|
||||
const reviewState = {
|
||||
|
||||
@@ -151,6 +151,58 @@ describe("buildExecutionPrompt", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskExecutor review addressing transitions", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
});
|
||||
|
||||
it("moves queued addressing records to in-progress", async () => {
|
||||
const store = createMockStore();
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "FN-001",
|
||||
column: "in-progress",
|
||||
status: null,
|
||||
reviewState: {
|
||||
source: "pull-request",
|
||||
items: [],
|
||||
addressing: [{ itemId: "ri-1", status: "queued", selectedAt: new Date().toISOString() }],
|
||||
},
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await (executor as any).transitionReviewAddressing("FN-001", ["queued"], "in-progress");
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", {
|
||||
reviewState: expect.objectContaining({
|
||||
addressing: [expect.objectContaining({ status: "in-progress", startedAt: expect.any(String) })],
|
||||
}),
|
||||
});
|
||||
});
|
||||
|
||||
it("marks in-progress addressing records as failed", async () => {
|
||||
const store = createMockStore();
|
||||
store.getTask.mockResolvedValue({
|
||||
id: "FN-001",
|
||||
column: "in-review",
|
||||
status: "failed",
|
||||
reviewState: {
|
||||
source: "reviewer-agent",
|
||||
items: [],
|
||||
addressing: [{ itemId: "ri-1", status: "in-progress", selectedAt: new Date().toISOString() }],
|
||||
},
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await (executor as any).transitionReviewAddressing("FN-001", ["in-progress"], "failed");
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", {
|
||||
reviewState: expect.objectContaining({
|
||||
addressing: [expect.objectContaining({ status: "failed", completedAt: expect.any(String) })],
|
||||
}),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskExecutor action gate context", () => {
|
||||
it("pauses task and agent for approval and marks completion", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
@@ -2266,8 +2266,11 @@ export class TaskExecutor {
|
||||
// true = requeue to todo, false = budget exhausted (already marked failed).
|
||||
let stuckRequeue: boolean | null = null;
|
||||
let taskDone = false;
|
||||
let reviewAddressingActivated = false;
|
||||
|
||||
try {
|
||||
await this.transitionReviewAddressing(task.id, ["queued"], "in-progress");
|
||||
reviewAddressingActivated = true;
|
||||
// Check dependencies
|
||||
const allTasks = await this.store.listTasks({ slim: true, includeArchived: false });
|
||||
const unmetDeps = task.dependencies.filter((depId) => {
|
||||
@@ -3823,6 +3826,15 @@ export class TaskExecutor {
|
||||
this.options.onError?.(task, err instanceof Error ? err : new Error(errorMessage));
|
||||
}
|
||||
} finally {
|
||||
if (reviewAddressingActivated) {
|
||||
const latestTask = await this.store.getTask(task.id);
|
||||
if (taskDone) {
|
||||
await this.transitionReviewAddressing(task.id, ["in-progress", "queued"], "addressed");
|
||||
} else if (latestTask.status === "failed") {
|
||||
await this.transitionReviewAddressing(task.id, ["in-progress", "queued"], "failed");
|
||||
}
|
||||
}
|
||||
|
||||
this.executing.delete(task.id);
|
||||
// Clear run context at end of execute() lifecycle
|
||||
this.currentRunContext = undefined;
|
||||
@@ -4130,6 +4142,41 @@ export class TaskExecutor {
|
||||
};
|
||||
}
|
||||
|
||||
private async transitionReviewAddressing(taskId: string, from: Array<"queued" | "in-progress" | "addressed" | "failed">, to: "queued" | "in-progress" | "addressed" | "failed"): Promise<void> {
|
||||
const task = await this.store.getTask(taskId);
|
||||
const reviewState = task.reviewState;
|
||||
if (!reviewState || reviewState.addressing.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
const now = new Date().toISOString();
|
||||
let changed = false;
|
||||
const addressing = reviewState.addressing.map((record) => {
|
||||
if (!from.includes(record.status)) {
|
||||
return record;
|
||||
}
|
||||
changed = true;
|
||||
return {
|
||||
...record,
|
||||
status: to,
|
||||
startedAt: to === "in-progress" ? now : record.startedAt,
|
||||
completedAt: to === "addressed" || to === "failed" ? now : record.completedAt,
|
||||
error: to === "addressed" ? undefined : record.error,
|
||||
};
|
||||
});
|
||||
|
||||
if (!changed) {
|
||||
return;
|
||||
}
|
||||
|
||||
await this.store.updateTask(taskId, {
|
||||
reviewState: {
|
||||
...reviewState,
|
||||
addressing,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
private createTaskDoneTool(taskId: string, onDone: () => void): ToolDefinition {
|
||||
const store = this.store;
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user