feat(FN-4217): complete Step 1 — add in-review stall helper
Fusion-Task-Id: FN-4217 Fusion-Task-Lineage: f5434e90-ad0c-41c5-bee9-1244cb69622b
This commit is contained in:
92
packages/core/src/__tests__/in-review-stall.test.ts
Normal file
92
packages/core/src/__tests__/in-review-stall.test.ts
Normal file
@@ -0,0 +1,92 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
DEFAULT_MAX_AUTO_MERGE_RETRIES,
|
||||
DEFAULT_STALE_MERGING_MIN_AGE_MS,
|
||||
getInReviewStallReason,
|
||||
} from "../in-review-stall.js";
|
||||
|
||||
const NOW = Date.parse("2026-05-12T12:00:00.000Z");
|
||||
|
||||
const baseTask = {
|
||||
id: "FN-4110",
|
||||
column: "in-review" as const,
|
||||
paused: false,
|
||||
status: undefined as string | undefined,
|
||||
error: undefined as string | undefined,
|
||||
steps: [{ name: "Step 1", status: "done" as const }],
|
||||
workflowStepResults: undefined,
|
||||
worktree: "/tmp/fn-4110",
|
||||
mergeDetails: {},
|
||||
mergeRetries: 0,
|
||||
updatedAt: new Date(NOW).toISOString(),
|
||||
};
|
||||
|
||||
describe("getInReviewStallReason", () => {
|
||||
it("returns transient-merge-status-no-owner for FN-4110 fixture", () => {
|
||||
const signal = getInReviewStallReason(
|
||||
{
|
||||
...baseTask,
|
||||
status: "merging",
|
||||
mergeRetries: 0,
|
||||
worktree: "/tmp/fn-4110",
|
||||
updatedAt: new Date(NOW - DEFAULT_STALE_MERGING_MIN_AGE_MS - 60_000).toISOString(),
|
||||
},
|
||||
{ now: NOW },
|
||||
);
|
||||
|
||||
expect(signal?.code).toBe("transient-merge-status-no-owner");
|
||||
});
|
||||
|
||||
it("returns undefined when active merger owns task", () => {
|
||||
expect(getInReviewStallReason({ ...baseTask, status: "merging" }, { now: NOW, activeMergeTaskId: "FN-4110" })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined when task is currently executing", () => {
|
||||
expect(getInReviewStallReason({ ...baseTask, status: "merging" }, { now: NOW, executingTaskIds: new Set(["FN-4110"]) })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns merge-retries-exhausted", () => {
|
||||
const signal = getInReviewStallReason({ ...baseTask, mergeRetries: DEFAULT_MAX_AUTO_MERGE_RETRIES, mergeDetails: { mergeConfirmed: false } }, { now: NOW });
|
||||
expect(signal?.code).toBe("merge-retries-exhausted");
|
||||
});
|
||||
|
||||
it("returns no-worktree-no-merge-confirmed", () => {
|
||||
const signal = getInReviewStallReason({ ...baseTask, worktree: undefined, mergeDetails: {} }, { now: NOW });
|
||||
expect(signal?.code).toBe("no-worktree-no-merge-confirmed");
|
||||
});
|
||||
|
||||
it("returns undefined for awaiting-user-review", () => {
|
||||
expect(getInReviewStallReason({ ...baseTask, status: "awaiting-user-review" }, { now: NOW })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined for paused tasks", () => {
|
||||
expect(getInReviewStallReason({ ...baseTask, paused: true }, { now: NOW })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns undefined when merge is confirmed", () => {
|
||||
expect(getInReviewStallReason({ ...baseTask, mergeDetails: { mergeConfirmed: true }, status: "merging" }, { now: NOW })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns merge-blocker for failed pre-merge workflow step", () => {
|
||||
const signal = getInReviewStallReason({
|
||||
...baseTask,
|
||||
workflowStepResults: [{ workflowStepId: "WS-1", workflowStepName: "gate", status: "failed", phase: "pre-merge" as const }],
|
||||
}, { now: NOW });
|
||||
expect(signal?.code).toBe("merge-blocker");
|
||||
expect(signal?.reason).toContain("failed pre-merge workflow steps");
|
||||
});
|
||||
|
||||
it("returns undefined when all clear", () => {
|
||||
expect(getInReviewStallReason({ ...baseTask }, { now: NOW })).toBeUndefined();
|
||||
});
|
||||
|
||||
it("prioritizes transient merge status over retries exhausted", () => {
|
||||
const signal = getInReviewStallReason({
|
||||
...baseTask,
|
||||
status: "merging",
|
||||
mergeRetries: DEFAULT_MAX_AUTO_MERGE_RETRIES,
|
||||
updatedAt: new Date(NOW - DEFAULT_STALE_MERGING_MIN_AGE_MS - 60_000).toISOString(),
|
||||
}, { now: NOW });
|
||||
expect(signal?.code).toBe("transient-merge-status-no-owner");
|
||||
});
|
||||
});
|
||||
102
packages/core/src/in-review-stall.ts
Normal file
102
packages/core/src/in-review-stall.ts
Normal file
@@ -0,0 +1,102 @@
|
||||
import { getTaskMergeBlocker } from "./task-merge.js";
|
||||
import type { Task } from "./types.js";
|
||||
|
||||
/**
|
||||
* State-based in-review stall detection. This is complementary to FN-4168's
|
||||
* planned heuristic `stalledReview` signal.
|
||||
*
|
||||
* Returning a signal is diagnostic-only and does not trigger any mutation by
|
||||
* itself. Callers MUST NOT use this helper as an auto-completion signal.
|
||||
*/
|
||||
export type InReviewStallCode =
|
||||
| "merge-blocker"
|
||||
| "transient-merge-status-no-owner"
|
||||
| "merge-retries-exhausted"
|
||||
| "no-worktree-no-merge-confirmed";
|
||||
|
||||
export interface InReviewStallSignal {
|
||||
reason: string;
|
||||
code: InReviewStallCode;
|
||||
observedAt: string;
|
||||
}
|
||||
|
||||
export interface InReviewStallContext {
|
||||
now?: number;
|
||||
activeMergeTaskId?: string | null;
|
||||
executingTaskIds?: ReadonlySet<string>;
|
||||
staleMergingMinAgeMs?: number;
|
||||
maxAutoMergeRetries?: number;
|
||||
}
|
||||
|
||||
/** Keep aligned with engine DEFAULT_STALE_MERGING_STATUS_MIN_AGE_MS. */
|
||||
export const DEFAULT_STALE_MERGING_MIN_AGE_MS = 5 * 60_000;
|
||||
/** Keep aligned with engine MAX_AUTO_MERGE_RETRIES (core must not import engine). */
|
||||
export const DEFAULT_MAX_AUTO_MERGE_RETRIES = 3;
|
||||
|
||||
const TRANSIENT_MERGE_STATUSES = new Set(["merging", "merging-pr", "merging-fix"]);
|
||||
|
||||
export function getInReviewStallReason(
|
||||
task: Pick<Task, "id" | "column" | "paused" | "status" | "error" | "steps" | "workflowStepResults" | "worktree" | "mergeDetails" | "mergeRetries" | "updatedAt">,
|
||||
context: InReviewStallContext = {},
|
||||
): InReviewStallSignal | undefined {
|
||||
if (task.column !== "in-review" || task.paused === true) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const now = context.now ?? Date.now();
|
||||
const observedAt = new Date(now).toISOString();
|
||||
const staleMergingMinAgeMs = context.staleMergingMinAgeMs ?? DEFAULT_STALE_MERGING_MIN_AGE_MS;
|
||||
const maxAutoMergeRetries = context.maxAutoMergeRetries ?? DEFAULT_MAX_AUTO_MERGE_RETRIES;
|
||||
|
||||
if (task.mergeDetails?.mergeConfirmed === true) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (context.activeMergeTaskId === task.id || context.executingTaskIds?.has(task.id)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (task.status === "awaiting-user-review" || task.status === "awaiting-approval") {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
if (task.status && TRANSIENT_MERGE_STATUSES.has(task.status)) {
|
||||
const updatedAtMs = Date.parse(task.updatedAt);
|
||||
if (Number.isFinite(updatedAtMs) && now - updatedAtMs >= staleMergingMinAgeMs) {
|
||||
const minutes = Math.max(1, Math.floor(staleMergingMinAgeMs / 60_000));
|
||||
return {
|
||||
code: "transient-merge-status-no-owner",
|
||||
reason: `In transient '${task.status}' state with no active merger for >= ${minutes} min`,
|
||||
observedAt,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
const mergeRetries = task.mergeRetries ?? 0;
|
||||
if (mergeRetries >= maxAutoMergeRetries && task.mergeDetails?.mergeConfirmed !== true) {
|
||||
return {
|
||||
code: "merge-retries-exhausted",
|
||||
reason: `Auto-merge retries exhausted (${mergeRetries}/${maxAutoMergeRetries}) without confirmed merge`,
|
||||
observedAt,
|
||||
};
|
||||
}
|
||||
|
||||
if (!task.worktree && task.mergeDetails?.mergeConfirmed !== true && task.mergeDetails?.noOpMerge !== true) {
|
||||
return {
|
||||
code: "no-worktree-no-merge-confirmed",
|
||||
reason: "No worktree on disk and merge not confirmed",
|
||||
observedAt,
|
||||
};
|
||||
}
|
||||
|
||||
const mergeBlocker = getTaskMergeBlocker(task);
|
||||
if (mergeBlocker) {
|
||||
return {
|
||||
code: "merge-blocker",
|
||||
reason: mergeBlocker,
|
||||
observedAt,
|
||||
};
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
Reference in New Issue
Block a user