From a453912ddfbdfa58a79a105309eba593b794b7c8 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 17:22:41 -0700 Subject: [PATCH] self-healing: merged-but-unfinished tasks never finalized on a renamed board (fifteenth sweep) (#2897) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `recoverMergedReviewTasks` finalizes a task whose merge is **confirmed** but which never reached the complete lane. Two literal reads meant that on a renamed board it was never found, so a card whose commit is already on the base branch sat in review or hold indefinitely — merged work the board still shows as unfinished. ## The two redundant guards convert, they don't get deleted Both `t.column === …` checks were redundant while the query pinned the column. Under a resolved read they become the per-card verdict. Deleting them would have silently widened the sweep — the same trap called out in #2891. ## Carries the two shapes review established earlier in this series - **Narrow when the card can answer, broad when it cannot** (#2891). `resolveWorkflowIrForTask` *substitutes* the built-in IR rather than failing, so a card with an unreadable selection would otherwise be rejected by the very verdict that the project-scoped query had just admitted it under. It falls back to the project sets instead. - **Deduped across the buckets** (#2879), so a column carrying both a review role and the hold role cannot finalize one card twice. Both were review findings on earlier PRs in this series, applied here up front rather than waiting to be caught again. ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed | | the per-card review verdict | fails — the renamed review lane does not match | Observable is `resolveSelfHealingMergeTarget`, a private method called once per candidate, so the assertion sits downstream of both halves without a git fixture. A non-vacuous companion (merge-confirmed card in the wip lane → untouched) rules out a read that returns everything. ## Verification `pnpm test:gate` 161 + 487 + 13 + 71, plus `self-healing.test.ts` 412; `tsc` engine clean; `pnpm lint`, `check:changesets`, census `--strict` clean, each run explicitly. --- .../self-healing-merged-review-query.md | 7 ++ ...elf-healing-query-filter-blindness.test.ts | 63 ++++++++++++++++++ packages/engine/src/self-healing.ts | 65 ++++++++++++++++--- .../lib/lifecycle-column-census-baseline.json | 4 +- 4 files changed, 129 insertions(+), 10 deletions(-) create mode 100644 .changeset/self-healing-merged-review-query.md diff --git a/.changeset/self-healing-merged-review-query.md b/.changeset/self-healing-merged-review-query.md new file mode 100644 index 0000000000..89dcc57efb --- /dev/null +++ b/.changeset/self-healing-merged-review-query.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Merged-but-unfinished tasks are now finalized on boards with renamed columns. +category: fix +dev: `recoverMergedReviewTasks` read the literal `in-review`/`todo`, so a card whose merge was confirmed sat unfinished on a renamed board while its commit was already on the base branch. Reads resolve via `resolveProjectColumnsForRoles`, the two per-card column checks resolve per card (falling back to the project sets when a card's own workflow is unreadable), and the candidate list is deduped. diff --git a/packages/engine/src/__tests__/self-healing-query-filter-blindness.test.ts b/packages/engine/src/__tests__/self-healing-query-filter-blindness.test.ts index e96ee00fc1..ec7dfd9b5f 100644 --- a/packages/engine/src/__tests__/self-healing-query-filter-blindness.test.ts +++ b/packages/engine/src/__tests__/self-healing-query-filter-blindness.test.ts @@ -657,4 +657,67 @@ describe("self-healing sweeps are bounded by a hardcoded column QUERY, not by th expect(pastBlocker).toHaveBeenCalled(); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-06:15 (the query-filter class, fifteenth sweep): + `recoverMergedReviewTasks` finalizes a task whose merge is CONFIRMED but which never reached the + complete lane. Two literal reads meant that on a renamed board the card sat in review or hold forever + while its commit was already on the base branch — merged work that the board still shows as unfinished. + + Observable is `resolveSelfHealingMergeTarget`, a private method called once per candidate, so the + assertion sits downstream of both the read and the per-card verdict without needing a git fixture. + + REVERT CHECKS, both measured, each alone: + - literal reads restored -> fails, the card is never listed + - verdict back to `t.column === "in-review"` -> fails, the renamed review lane does not match + */ + it("finalizes a merge-confirmed card stranded on a RENAMED review lane", async () => { + const merged = { + ...shippedCard(), + id: "FN-MERGED", + column: RENAMED_VOCAB.review, + mergeDetails: { mergeConfirmed: true, commitSha: "abcdef1234567890" }, + } as unknown as Task; + const { store } = productionFaithfulStore([merged]); + const manager = new SelfHealingManager(store, { rootDir: "/repo" }); + const resolveTarget = vi.fn(async () => ({ branch: "main", source: "settings" })); + Object.assign(manager, { + resolveSelfHealingMergeTarget: resolveTarget, + isCommitReachableFromBranch: vi.fn(async () => false), + recordSharedGroupDefaultTargetGuard: vi.fn(async () => undefined), + }); + + await manager.recoverMergedReviewTasks(); + + expect(resolveTarget).toHaveBeenCalledWith( + expect.objectContaining({ id: "FN-MERGED" }), + expect.anything(), + "recover-merged-review", + ); + }); + + it("ignores a merge-confirmed card sitting in the RENAMED wip lane", async () => { + /* + Non-vacuous companion: without it, a read returning every column would satisfy the case above. This + sweep covers review and hold only — a card mid-execution is not its business. + */ + const merged = { + ...shippedCard(), + id: "FN-MERGED", + column: RENAMED_VOCAB.wip, + mergeDetails: { mergeConfirmed: true, commitSha: "abcdef1234567890" }, + } as unknown as Task; + const { store } = productionFaithfulStore([merged]); + const manager = new SelfHealingManager(store, { rootDir: "/repo" }); + const resolveTarget = vi.fn(async () => ({ branch: "main", source: "settings" })); + Object.assign(manager, { + resolveSelfHealingMergeTarget: resolveTarget, + isCommitReachableFromBranch: vi.fn(async () => false), + recordSharedGroupDefaultTargetGuard: vi.fn(async () => undefined), + }); + + await manager.recoverMergedReviewTasks(); + + expect(resolveTarget).not.toHaveBeenCalled(); + }); }); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 6d0ed32841..2c23e6818f 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -9547,15 +9547,64 @@ export class SelfHealingManager extends SelfHealingGitEvidence { try { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - const [reviewTasks, todoTasks] = await Promise.all([ - this.store.listTasks({ column: "in-review", slim: true }), - this.store.listTasks({ column: "todo", slim: true }), - ]); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-06:10 (the query-filter class, fifteenth sweep): + A task whose merge is CONFIRMED but which never reached the complete lane. Two literal reads meant + that on a renamed board it was never found, so a card whose work is merged sat in review or hold + forever while its commit was already on the base branch. - const mergedButNotDone = [ - ...reviewTasks.filter((t) => t.column === "in-review"), - ...todoTasks.filter((t) => t.column === "todo"), - ].filter((t) => + The two `t.column === …` checks were redundant while the query pinned the column; under a resolved + read they become the per-card verdict, so they convert rather than being deleted. + */ + const mergedReviewColumns = await resolveProjectColumnsForRoles(this.store, REVIEW_ROLES); + const mergedHoldColumns = await resolveProjectColumnsForRoles(this.store, ["hold"]); + const readMergedBucket = async (columns: ReadonlySet): Promise => { + const byId = new Map(); + for (const column of columns) { + for (const entry of await this.store.listTasks({ column, slim: true })) byId.set(entry.id, entry); + } + return [...byId.values()]; + }; + const [reviewTasks, todoTasks] = await Promise.all([ + readMergedBucket(mergedReviewColumns), + readMergedBucket(mergedHoldColumns), + ]); + /* + Per-card lanes. NARROW WHEN THE CARD CAN ANSWER, BROAD WHEN IT CANNOT — the shape #2891 settled on: + `resolveWorkflowIrForTask` SUBSTITUTES the built-in IR rather than failing, so a card with an + unreadable selection would otherwise be rejected by the verdict that the project-scoped query had + just admitted from a renamed lane. Falling back to the project sets keeps it. + */ + const mergedLanes = new Map; hold: Set }>(); + for (const task of [...reviewTasks, ...todoTasks]) { + if (mergedLanes.has(task.id)) continue; + const lanes = { review: new Set(), hold: new Set() }; + try { + const { ir, source } = await resolveWorkflowIrForTaskWithProvenance(this.store, task.id); + if (source === "default") { + for (const id of mergedReviewColumns) lanes.review.add(id); + for (const id of mergedHoldColumns) lanes.hold.add(id); + } else { + for (const role of REVIEW_ROLES) for (const id of columnsWithFlag(ir, role)) lanes.review.add(id); + for (const id of columnsWithFlag(ir, "hold")) lanes.hold.add(id); + } + } catch { + for (const id of mergedReviewColumns) lanes.review.add(id); + for (const id of mergedHoldColumns) lanes.hold.add(id); + } + mergedLanes.set(task.id, lanes); + } + const mergedLanesOf = (id: string) => mergedLanes.get(id) ?? { review: mergedReviewColumns, hold: mergedHoldColumns }; + + /* Deduped across the buckets — the P1 reviewed on #2879; a dual-role column would otherwise finalize one card twice. */ + const mergedCandidateById = new Map(); + for (const task of [ + ...reviewTasks.filter((t) => mergedLanesOf(t.id).review.has(t.column)), + ...todoTasks.filter((t) => mergedLanesOf(t.id).hold.has(t.column)), + ]) { + if (!mergedCandidateById.has(task.id)) mergedCandidateById.set(task.id, task); + } + const mergedButNotDone = [...mergedCandidateById.values()].filter((t) => !t.deletedAt && allowsAutoMergeProcessing(t, settings) && t.mergeDetails?.mergeConfirmed === true, diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 73b1a60db0..5ef3f3a24f 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -1,7 +1,7 @@ { "generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline", "byFile": { - "packages/engine/src/self-healing.ts": 89, + "packages/engine/src/self-healing.ts": 87, "packages/engine/src/scheduler.ts": 12, "packages/core/src/task-store/async-comments-attachments.ts": 9, "packages/engine/src/executor.ts": 8, @@ -129,7 +129,7 @@ "plugins/fusion-plugin-reports/src/store/report-types.ts\u0000archived": 1 }, "queryByFile": { - "packages/engine/src/self-healing.ts": 37, + "packages/engine/src/self-healing.ts": 35, "packages/core/src/task-store/async-persistence.ts": 2, "packages/core/src/task-store/merge-queue-ops.ts": 2, "packages/core/src/async-mission-store.ts": 1,