fix(FN-6088): suppress queued merge stalled badges
Suppress legacy stalledReview hydration for tasks already owned by the merge queue and avoid repeated mergeable-review re-enqueue churn for queued tasks. Fusion-Task-Id: FN-6088
This commit is contained in:
5
.changeset/merge-queued-stalled-review.md
Normal file
5
.changeset/merge-queued-stalled-review.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Suppress legacy stalled-review badges and re-enqueue churn for tasks already owned by the merge queue.
|
||||
@@ -56,4 +56,18 @@ describe("TaskStore stalledReview hydration", () => {
|
||||
expect(detail.stalledReview?.heuristic).toBe("reenqueue-churn");
|
||||
expect(detail.stalledReview?.matchCount).toBe(3);
|
||||
});
|
||||
|
||||
it("omits stalledReview for tasks already queued for merge", async () => {
|
||||
const task = await seedStalledInReviewTask();
|
||||
await store.enqueueMergeQueue(task.id);
|
||||
|
||||
const slimTasks = await store.listTasks({ slim: true, column: "in-review" });
|
||||
expect(slimTasks.find((entry) => entry.id === task.id)?.stalledReview).toBeUndefined();
|
||||
|
||||
const fullTasks = await store.listTasks({ slim: false, column: "in-review" });
|
||||
expect(fullTasks.find((entry) => entry.id === task.id)?.stalledReview).toBeUndefined();
|
||||
|
||||
const detail = await store.getTask(task.id);
|
||||
expect(detail.stalledReview).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4833,7 +4833,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
engineActiveSinceMs: settings.engineActiveSinceMs,
|
||||
engineActivationGraceMs: settings.engineActivationGraceMs,
|
||||
});
|
||||
task.stalledReview = detectStalledReview(task, { now });
|
||||
task.stalledReview = mergeQueuedTaskIds.has(task.id) ? undefined : detectStalledReview(task, { now });
|
||||
// Derived at read time only; retrySummary is never persisted to SQLite.
|
||||
task.retrySummary = computeRetrySummary(task);
|
||||
|
||||
@@ -5384,7 +5384,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
}
|
||||
}
|
||||
}
|
||||
task.stalledReview = detectStalledReview(task, { now });
|
||||
task.stalledReview = isMergeQueued ? undefined : detectStalledReview(task, { now });
|
||||
// Derived at read time only; retrySummary is never persisted to SQLite.
|
||||
task.retrySummary = computeRetrySummary(task);
|
||||
|
||||
@@ -5907,7 +5907,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
}
|
||||
}
|
||||
task.timedExecutionMs = this.computeTimedExecutionMs(task.log);
|
||||
task.stalledReview = detectStalledReview(task, { now });
|
||||
task.stalledReview = isMergeQueued ? undefined : detectStalledReview(task, { now });
|
||||
// Derived at read time only; retrySummary is never persisted to SQLite.
|
||||
task.retrySummary = computeRetrySummary(task);
|
||||
task.log = [];
|
||||
|
||||
@@ -175,6 +175,7 @@ function createMockStore(overrides: Record<string, unknown> = {}): TaskStore & E
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
handoffToReview: vi.fn().mockResolvedValue(undefined),
|
||||
enqueueMergeQueue: vi.fn().mockResolvedValue(undefined),
|
||||
peekMergeQueue: vi.fn().mockReturnValue([]),
|
||||
mergeTask: vi.fn().mockResolvedValue(undefined),
|
||||
archiveTaskAndCleanup: vi.fn().mockResolvedValue({} as Task),
|
||||
walCheckpoint: vi.fn().mockReturnValue({ busy: 0, log: 5, checkpointed: 5 }),
|
||||
@@ -3282,6 +3283,58 @@ describe("SelfHealingManager", () => {
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("does not re-enqueue mergeable review tasks already held by the merge queue", async () => {
|
||||
const enqueueMerge = vi.fn().mockReturnValue(true);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
enqueueMerge,
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
autoMerge: true,
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
});
|
||||
(store.peekMergeQueue as ReturnType<typeof vi.fn>).mockReturnValue([
|
||||
{
|
||||
taskId: "FN-6088",
|
||||
enqueuedAt: "2026-06-09T16:03:19.080Z",
|
||||
priority: "normal",
|
||||
leasedBy: null,
|
||||
leasedAt: null,
|
||||
leaseExpiresAt: null,
|
||||
attemptCount: 0,
|
||||
lastError: null,
|
||||
},
|
||||
]);
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-6088",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: null,
|
||||
error: null,
|
||||
worktree: "/tmp/test-project/.worktrees/fn-6088",
|
||||
steps: [{ name: "Ship it", status: "done" }],
|
||||
workflowStepResults: [{ id: "ws-1", status: "passed", phase: "pre-merge" }],
|
||||
mergeDetails: undefined,
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverMergeableReviewTasks();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(enqueueMerge).not.toHaveBeenCalled();
|
||||
expect(store.mergeTask).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith(
|
||||
"FN-6088",
|
||||
expect.stringContaining("re-enqueued for merge"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("FN-4084: recoverMergeableReviewTasks escalates after repeated no-op re-enqueues", async () => {
|
||||
const enqueueMerge = vi.fn().mockReturnValue(false);
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
|
||||
@@ -5100,25 +5100,26 @@ export class SelfHealingManager {
|
||||
(t.mergeRetries ?? 0) < MAX_AUTO_MERGE_RETRIES &&
|
||||
getTaskMergeBlocker(t) === undefined,
|
||||
);
|
||||
const unownedMergeable = mergeable.filter((task) => !this.isMergeLaneOwned(task.id));
|
||||
|
||||
const inReviewIds = new Set(tasks.map((task) => task.id));
|
||||
const mergeableIds = new Set(mergeable.map((task) => task.id));
|
||||
const mergeableIds = new Set(unownedMergeable.map((task) => task.id));
|
||||
for (const taskId of [...this.mergeStarvationDrops.keys()]) {
|
||||
if (!inReviewIds.has(taskId) || !mergeableIds.has(taskId)) {
|
||||
this.mergeStarvationDrops.delete(taskId);
|
||||
}
|
||||
}
|
||||
|
||||
if (mergeable.length === 0) return 0;
|
||||
if (unownedMergeable.length === 0) return 0;
|
||||
|
||||
log.warn(`Found ${mergeable.length} mergeable review task(s) stuck in in-review`);
|
||||
log.warn(`Found ${unownedMergeable.length} mergeable review task(s) stuck in in-review`);
|
||||
|
||||
// Prefer the engine's merge queue so `mergeStrategy` (direct vs.
|
||||
// pull-request) is honored. Fall back to a direct store merge only
|
||||
// when no enqueue callback is wired (standalone/tests).
|
||||
const enqueueMerge = this.options.enqueueMerge;
|
||||
let recovered = 0;
|
||||
for (const task of mergeable) {
|
||||
for (const task of unownedMergeable) {
|
||||
try {
|
||||
if (enqueueMerge) {
|
||||
const queued = enqueueMerge(task.id);
|
||||
|
||||
Reference in New Issue
Block a user