fix(FN-3829): recover stale merging status and align chat sidebar search styling
- Add self-healing recovery for stale in-review merging statuses when no active merger owns the task - Skip merge re-enqueue for tasks already marked with transient merging statuses and add regression coverage - Wire active merge task ID provider through project runtime for safer stale-status detection - Use chat-sidebar-search-container in ChatView with matching CSS padding - Normalize fusion-plugin-reports test script to use local vitest and add changeset for @runfusion/fusion patch Fusion-Task-Id: FN-3829
This commit is contained in:
@@ -1857,6 +1857,98 @@ describe("SelfHealingManager", () => {
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("clears stale merging statuses with no active merger", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-3829-stale",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "merging",
|
||||
updatedAt: new Date(Date.now() - 10 * 60_000).toISOString(),
|
||||
steps: [{ name: "Ship it", status: "done" }],
|
||||
workflowStepResults: [],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverStaleMergingStatus();
|
||||
|
||||
expect(result).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-3829-stale", { status: null });
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-3829-stale",
|
||||
expect.stringContaining("cleared stale 'merging' status"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("keeps transient merge status when task is actively merging", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getActiveMergeTaskId: () => "FN-3829-active",
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-3829-active",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "merging-pr",
|
||||
updatedAt: new Date(Date.now() - 10 * 60_000).toISOString(),
|
||||
steps: [{ name: "Ship it", status: "done" }],
|
||||
workflowStepResults: [],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverStaleMergingStatus();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-3829-active", { status: null });
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("keeps fresh transient merge status within the default age window", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
});
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-3829-fresh",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "merging",
|
||||
updatedAt: new Date(Date.now() - 60_000).toISOString(),
|
||||
steps: [{ name: "Ship it", status: "done" }],
|
||||
workflowStepResults: [],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverStaleMergingStatus();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-3829-fresh", { status: null });
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("merges eligible in-review tasks that still have an unmerged worktree", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
@@ -2037,6 +2129,46 @@ describe("SelfHealingManager", () => {
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("does not re-enqueue tasks already marked as merging", async () => {
|
||||
const enqueueMerge = vi.fn();
|
||||
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.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-3829-merging",
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
status: "merging",
|
||||
error: null,
|
||||
mergeRetries: 0,
|
||||
worktree: "/tmp/test-project/.worktrees/fn-3829-merging",
|
||||
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.logEntry).not.toHaveBeenCalledWith(
|
||||
"FN-3829-merging",
|
||||
expect.stringContaining("re-enqueued for merge"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("does not re-enqueue retry-exhausted review tasks", async () => {
|
||||
const enqueueMerge = vi.fn();
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
@@ -3299,6 +3431,7 @@ describe("maintenance cycle concurrency", () => {
|
||||
(vi.spyOn(manager as any, "recoverCompletedTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverStaleIncompleteReviewTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverInterruptedMergingTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverStaleMergingStatus").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverMergeableReviewTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverMergedReviewTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverMisclassifiedFailures").mockResolvedValue(0) as any);
|
||||
@@ -3323,6 +3456,7 @@ describe("maintenance cycle concurrency", () => {
|
||||
(vi.spyOn(manager as any, "recoverCompletedTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverStaleIncompleteReviewTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverInterruptedMergingTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverStaleMergingStatus").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverMergeableReviewTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverMergedReviewTasks").mockResolvedValue(0) as any);
|
||||
(vi.spyOn(manager as any, "recoverMisclassifiedFailures").mockResolvedValue(0) as any);
|
||||
@@ -3391,6 +3525,7 @@ describe("maintenance cycle concurrency", () => {
|
||||
makeSlow("recoverCompletedTasks");
|
||||
makeSlow("recoverStaleIncompleteReviewTasks");
|
||||
makeSlow("recoverInterruptedMergingTasks");
|
||||
makeSlow("recoverStaleMergingStatus");
|
||||
makeSlow("recoverMergeableReviewTasks");
|
||||
makeSlow("recoverMergedReviewTasks");
|
||||
makeSlow("recoverMisclassifiedFailures");
|
||||
@@ -3416,6 +3551,7 @@ describe("maintenance cycle concurrency", () => {
|
||||
"recoverCompletedTasks",
|
||||
"recoverStaleIncompleteReviewTasks",
|
||||
"recoverInterruptedMergingTasks",
|
||||
"recoverStaleMergingStatus",
|
||||
"recoverMergeableReviewTasks",
|
||||
"recoverMergedReviewTasks",
|
||||
"recoverMisclassifiedFailures",
|
||||
|
||||
@@ -224,6 +224,7 @@ export class ProjectEngine {
|
||||
// cause `internalEnqueueMerge` to silently no-op.
|
||||
//
|
||||
// Tests substitute a minimal runtime mock that may not implement this hook.
|
||||
this.runtime.setActiveMergeTaskIdProvider?.(() => this.getActiveMergeTaskId());
|
||||
this.runtime.setMergeEnqueuer?.((taskId) => {
|
||||
// If the wedged attempt was the active one, abort its in-flight signal
|
||||
// and dispose its session so subsequent code paths can release file
|
||||
@@ -240,6 +241,10 @@ export class ProjectEngine {
|
||||
});
|
||||
}
|
||||
|
||||
getActiveMergeTaskId(): string | null {
|
||||
return this.activeMergeTaskId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Start the engine: initialize the runtime and all auxiliary subsystems.
|
||||
*/
|
||||
|
||||
@@ -118,6 +118,7 @@ export class InProcessRuntime
|
||||
* before `start()` via `setMergeEnqueuer`.
|
||||
*/
|
||||
private mergeEnqueuer?: (taskId: string) => void;
|
||||
private activeMergeTaskIdProvider?: () => string | null;
|
||||
/** Tracks whether startup recovery was intentionally deferred due to pause state. */
|
||||
private startupRecoveryDeferred = false;
|
||||
/** Prevent duplicate unpause recovery dispatches from racing each other. */
|
||||
@@ -621,6 +622,7 @@ export class InProcessRuntime
|
||||
getPlanningTaskIds: () => this.triageProcessor?.getProcessingTaskIds() ?? new Set<string>(),
|
||||
evictStaleTriageProcessing: () => this.triageProcessor?.evictStaleProcessing() ?? new Set<string>(),
|
||||
enqueueMerge: this.mergeEnqueuer ? (taskId: string) => this.mergeEnqueuer?.(taskId) : undefined,
|
||||
getActiveMergeTaskId: () => this.activeMergeTaskIdProvider?.() ?? null,
|
||||
});
|
||||
this.selfHealingManager.start();
|
||||
this.stuckTaskDetector.start();
|
||||
@@ -857,6 +859,10 @@ export class InProcessRuntime
|
||||
this.mergeEnqueuer = enqueueMerge;
|
||||
}
|
||||
|
||||
setActiveMergeTaskIdProvider(getActiveMergeTaskId: () => string | null): void {
|
||||
this.activeMergeTaskIdProvider = getActiveMergeTaskId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resume executor/self-healing activity after an unpause transition.
|
||||
*
|
||||
|
||||
@@ -77,6 +77,16 @@ export interface SelfHealingOptions {
|
||||
* the polling sweep's enqueue to silently no-op).
|
||||
*/
|
||||
enqueueMerge?: (taskId: string) => void;
|
||||
/**
|
||||
* Minimum age before a transient merge status is considered stale when no
|
||||
* active merge session is associated with that task.
|
||||
*/
|
||||
staleMergingStatusMinAgeMs?: number;
|
||||
/**
|
||||
* Returns the task ID actively merging in this engine process, if any.
|
||||
* Used to avoid clearing a transient merge status mid-merge.
|
||||
*/
|
||||
getActiveMergeTaskId?: () => string | null;
|
||||
}
|
||||
|
||||
const APPROVED_TRIAGE_RECOVERY_GRACE_MS = 60_000;
|
||||
@@ -108,6 +118,7 @@ const ORPHANED_WITH_WORKTREE_GRACE_MS = 300_000;
|
||||
*/
|
||||
const MAX_TASK_DONE_RETRIES = 3;
|
||||
const MAX_AUTO_MERGE_RETRIES = 3;
|
||||
const DEFAULT_STALE_MERGING_STATUS_MIN_AGE_MS = 5 * 60_000;
|
||||
|
||||
interface LandedTaskCommit {
|
||||
sha: string;
|
||||
@@ -649,6 +660,7 @@ export class SelfHealingManager {
|
||||
{ name: "recover-failed-pre-merge-steps", fn: () => this.recoverReviewTasksWithFailedPreMergeSteps() },
|
||||
{ name: "recover-interrupted-merging", fn: () => this.recoverInterruptedMergingTasks() },
|
||||
{ name: "recover-done-merge-metadata", fn: () => this.recoverDoneTaskMergeMetadata() },
|
||||
{ name: "recover-stale-merging-status", fn: () => this.recoverStaleMergingStatus() },
|
||||
{ name: "recover-mergeable-review", fn: () => this.recoverMergeableReviewTasks() },
|
||||
{ name: "recover-merged-review", fn: () => this.recoverMergedReviewTasks() },
|
||||
{ name: "recover-misclassified-failures", fn: () => this.recoverMisclassifiedFailures() },
|
||||
@@ -828,6 +840,59 @@ export class SelfHealingManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear stale transient merge statuses when no active merger owns the task.
|
||||
*
|
||||
* @returns Number of tasks unblocked by clearing stale status
|
||||
*/
|
||||
async recoverStaleMergingStatus(): Promise<number> {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return 0;
|
||||
|
||||
const minAgeMs = this.options.staleMergingStatusMinAgeMs ?? DEFAULT_STALE_MERGING_STATUS_MIN_AGE_MS;
|
||||
if (!Number.isFinite(minAgeMs) || minAgeMs <= 0) return 0;
|
||||
|
||||
const now = Date.now();
|
||||
const activeMergeTaskId = this.options.getActiveMergeTaskId?.() ?? null;
|
||||
const tasks = await this.store.listTasks({ column: "in-review", slim: true });
|
||||
const stale = tasks.filter((task) => {
|
||||
if (task.column !== "in-review" || task.paused) return false;
|
||||
if (!task.status || (task.status !== "merging" && task.status !== "merging-pr")) return false;
|
||||
if (activeMergeTaskId && activeMergeTaskId === task.id) return false;
|
||||
|
||||
const updatedAtMs = task.updatedAt ? Date.parse(task.updatedAt) : Number.NaN;
|
||||
if (!Number.isFinite(updatedAtMs)) return false;
|
||||
return now - updatedAtMs >= minAgeMs;
|
||||
});
|
||||
|
||||
if (stale.length === 0) return 0;
|
||||
|
||||
let recovered = 0;
|
||||
for (const task of stale) {
|
||||
const previousStatus = task.status;
|
||||
try {
|
||||
log.warn(`Clearing stale merge status for ${task.id}: ${previousStatus}`);
|
||||
await this.store.updateTask(task.id, { status: null });
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Auto-recovered: cleared stale '${previousStatus}' status (no active merger)`,
|
||||
);
|
||||
recovered++;
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Failed to clear stale merge status for ${task.id}: ${errorMessage}`);
|
||||
}
|
||||
}
|
||||
|
||||
return recovered;
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Stale merging status recovery failed: ${errorMessage}`);
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover `in-review` tasks that are fully mergeable but never had
|
||||
* `mergeTask()` invoked.
|
||||
@@ -852,6 +917,10 @@ export class SelfHealingManager {
|
||||
const mergeable = tasks.filter((t) =>
|
||||
t.column === "in-review" &&
|
||||
!t.paused &&
|
||||
// Exclude transient merge statuses. Active merges should be left alone;
|
||||
// stale ones are handled by recoverStaleMergingStatus().
|
||||
t.status !== "merging" &&
|
||||
t.status !== "merging-pr" &&
|
||||
Boolean(t.worktree) &&
|
||||
t.mergeDetails?.mergeConfirmed !== true &&
|
||||
// Mirror ProjectEngine.canMergeTask retry gate. If retries are already
|
||||
|
||||
Reference in New Issue
Block a user