self-healing: foreign-only contamination never cleared on a renamed board (fourteenth sweep) (#2891)

`recoverForeignOnlyContaminatedInReviewTasks` classifies a branch carrying **only foreign commits** and clears the contamination park that nothing else clears. Two literal reads meant that on a renamed board it classified nothing, and the task stayed parked indefinitely. ## The two redundant guards were the interesting part Both filters carried a `task.column === …` check that was **redundant** while the query pinned the column. Under a resolved read they stop being redundant and become the per-card verdict — so they convert here rather than being deleted. Deleting them would have silently widened the sweep, which is the failure this whole class is about. ## Dedupe matters more here than elsewhere The concatenated candidate list is deduped (the P1 reviewed on #2879). It bites harder in this sweep because the two filters have **different predicates**: a column carrying both a review role and the wip role could satisfy both and classify one branch twice. Explicit `has` guard rather than `new Map(entries)` — that constructor keeps first insertion *order* but the **last** value for a repeated key, so it reads as first-bucket precedence while doing the opposite. (Corrected in #2879 and #2883 for the same reason.) ## Revert results Each applied alone and the file re-run: | conversion | reverted → | | --- | --- | | the resolved reads | fails — the card is never listed, so the classifier is never called | | the review verdict | fails — the renamed review lane does not match and the card is filtered out | Observable is **candidacy**: `classifyForeignOnlyContamination` runs once per accepted card and not at all for a rejected one, which is exactly the read-plus-verdict under test. It is a static named import, so it is intercepted with a scoped `vi.mock` (spyOn cannot rebind an already-resolved ESM binding); only that one export is overridden, so the sweeps in this file that use `inspectBranchConflict` are unaffected. A non-vacuous companion (same card in the board's hold lane → never classified) 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. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 18:26:06 -07:00
parent f6e368205e
commit c6767cb258
4 changed files with 252 additions and 9 deletions
--- a/.changeset/self-healing-foreign-contamination-query.md
+++ b/.changeset/self-healing-foreign-contamination-query.md
@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Foreign-only branch contamination is now cleared on boards with renamed columns.
+category: fix
+dev: `recoverForeignOnlyContaminatedInReviewTasks` read the literal `in-review`/`in-progress`, so a branch carrying only foreign commits was never classified on a renamed board and the task stayed parked. Reads resolve via `resolveProjectColumnsForRoles`, the two per-card column checks resolve per card, and the concatenated candidate list is deduped.
--- a/packages/engine/src/tests/self-healing-query-filter-blindness.test.ts
+++ b/packages/engine/src/tests/self-healing-query-filter-blindness.test.ts
@@ -37,6 +37,37 @@ import { EventEmitter } from "node:events";
 import type { Settings, Task, TaskStore } from "@fusion/core";
 import { resolveLifecycleColumns } from "@fusion/core";

+/*
+FNXC:WorkflowResolvedColumns 2026-07-31-04:40:
+`classifyForeignOnlyContamination` is a STATIC named import in the sweep, so `vi.spyOn` on the module
+object cannot intercept it under ESM — the binding is already resolved. Only the other named exports are
+passed through, so the sweeps in this file that use `inspectBranchConflict` are unaffected.
+*/
+/*
+FNXC:WorkflowResolvedColumns 2026-07-31-05:45:
+The sweep logs through `createLogger("self-healing")`, which writes to console.error. Spying on
+console.error does NOT work here — vitest installs its own console interceptor above the spy, so the
+line appears in the run output while the spy records nothing (it did, and read as "no warn emitted").
+Mocking the logger module captures the call itself, one level below the console.
+*/
+const selfHealingWarn = vi.fn();
+vi.mock("../logger.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../logger.js")>();
+  return {
+    ...actual,
+    createLogger: (prefix: string) => {
+      const real = actual.createLogger(prefix);
+      return prefix === "self-healing" ? { ...real, warn: (...args: unknown[]) => { selfHealingWarn(...args); real.warn(...args as [string]); } } : real;
+    },
+  };
+});
+
+const classifyForeignOnlyContamination = vi.fn(async () => ({ kind: "clean" as const }));
+vi.mock("../branch-conflicts.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../branch-conflicts.js")>();
+  return { ...actual, classifyForeignOnlyContamination: (...args: unknown[]) => classifyForeignOnlyContamination(...args as []) };
+});
+
 vi.mock("../run-audit.js", async (importOriginal) => {
  const actual = await importOriginal<typeof import("../run-audit.js")>();
  return {
@@ -770,4 +801,102 @@ describe("self-healing sweeps are bounded by a hardcoded column QUERY, not by th

    expect(updateTask).not.toHaveBeenCalled();
  });
+
+  /*
+  FNXC:WorkflowResolvedColumns 2026-07-31-04:35 (the query-filter class, fourteenth sweep):
+  `recoverForeignOnlyContaminatedInReviewTasks` classifies a branch that carries ONLY foreign commits and
+  clears the contamination park nothing else clears. Two literal reads meant that on a renamed board it
+  classified nothing and the task stayed parked indefinitely.
+
+  The two `task.column === …` checks inside its filters were redundant while the query pinned the column;
+  under a resolved read they ARE the per-card verdict, so they convert here rather than being deleted.
+
+  `classifyForeignOnlyContamination` is a module function needing git, so the observable is CANDIDACY —
+  it is called once per accepted card and not at all for a card the filters reject, which is exactly the
+  read-plus-verdict this change is about.
+
+  REVERT CHECK, measured: with the literal reads restored, this fails — the card is never listed, so the
+  classifier is never called for it.
+  */
+  it("classifies a foreign-only contaminated branch on a RENAMED review lane", async () => {
+    const parked = {
+      ...shippedCard(),
+      id: "FN-FOREIGN",
+      column: RENAMED_VOCAB.review,
+      branch: "fusion/FN-FOREIGN",
+      worktree: "/tmp/worktrees/FN-FOREIGN",
+      mergeDetails: {},
+    } as unknown as Task;
+    const { store } = productionFaithfulStore([parked]);
+    classifyForeignOnlyContamination.mockClear();
+
+    await new SelfHealingManager(store, { rootDir: "/repo" }).recoverForeignOnlyContaminatedInReviewTasks();
+
+    expect(classifyForeignOnlyContamination).toHaveBeenCalledWith(expect.objectContaining({ taskId: "FN-FOREIGN" }));
+  });
+
+  it("does not classify a card whose lane is neither review nor wip on a RENAMED board", async () => {
+    /*
+    Non-vacuous companion: without it, a read returning every column would satisfy the case above. Same
+    board, same card — only its lane changes, to the board's own hold lane.
+    */
+    const parked = {
+      ...shippedCard(),
+      id: "FN-FOREIGN",
+      column: RENAMED_VOCAB.hold,
+      branch: "fusion/FN-FOREIGN",
+      worktree: "/tmp/worktrees/FN-FOREIGN",
+      mergeDetails: {},
+    } as unknown as Task;
+    const { store } = productionFaithfulStore([parked]);
+    classifyForeignOnlyContamination.mockClear();
+
+    await new SelfHealingManager(store, { rootDir: "/repo" }).recoverForeignOnlyContaminatedInReviewTasks();
+
+    expect(classifyForeignOnlyContamination).not.toHaveBeenCalled();
+  });
+
+  /*
+  FNXC:WorkflowResolvedColumns 2026-07-31-05:50 (#2891 review P1 — the card the sweep disowned):
+  `resolveWorkflowIrForTask` does not fail; it SUBSTITUTES the built-in IR. So a card whose workflow
+  selection is missing or unreadable came back measured against `in-review`/`in-progress`, and the
+  per-card verdicts then REJECTED the very card the project-scoped query had just admitted from a renamed
+  lane. The sweep found it and immediately disowned it.
+
+  The fix falls back to the PROJECT sets that admitted the card, so it is CLASSIFIED rather than dropped.
+  This asserts that outcome rather than a log line — the observable is stronger and does not depend on
+  wording.
+
+  SUPERSEDED 2026-07-30 (#2891 review, second round): this asserted that the card IS classified, which
+  was true of the project-union fallback I shipped first and is no longer the behaviour. Review pushed
+  back that widening on an ACTION site — these verdicts clear a contamination pause — lets a column
+  carrying a recovery role only in ANOTHER workflow admit this card. The union was replaced by
+  skip-and-report: without the card's own board we do not decide, and the card is logged so it is
+  visible rather than silently mis-decided in either direction.
+
+  So the assertion is inverted rather than deleted. What it now pins is the same property from the
+  other side — a renamed-lane card with no resolvable workflow must NOT be acted on — and it still
+  fails if someone restores either earlier answer, because both of those classify it.
+  */
+  it("does NOT classify a renamed-lane card whose own workflow cannot be resolved", async () => {
+    const parked = {
+      ...shippedCard(),
+      id: "FN-NOWORKFLOW",
+      column: RENAMED_VOCAB.review,
+      branch: "fusion/FN-NOWORKFLOW",
+      worktree: "/tmp/worktrees/FN-NOWORKFLOW",
+      mergeDetails: {},
+    } as unknown as Task;
+    const { store } = productionFaithfulStore([parked]);
+    /* The PROJECT's definitions still resolve (so the card is listed); the CARD's own selection does not. */
+    Object.assign(store, {
+      getTaskWorkflowSelectionAsync: vi.fn(async () => undefined),
+      getTaskWorkflowSelection: vi.fn(() => undefined),
+    });
+    classifyForeignOnlyContamination.mockClear();
+
+    await new SelfHealingManager(store, { rootDir: "/repo" }).recoverForeignOnlyContaminatedInReviewTasks();
+
+    expect(classifyForeignOnlyContamination).not.toHaveBeenCalled();
+  });
 });
--- a/packages/engine/src/self-healing.ts
+++ b/packages/engine/src/self-healing.ts
@@ -31,6 +31,7 @@ import { readFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { isAbsolute, join, relative, resolve } from "node:path";
 import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr,
+  LEGACY_COLUMN_IDS_BY_ROLE,
  resolveProjectColumnsForRoles,
  REVIEW_ROLES,
 } from "@fusion/core";
@@ -11033,11 +11034,107 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
      const settings = await this.store.getSettings();
      if (settings.globalPause || settings.enginePaused) return 0;
      const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
-      const inReview = await this.store.listTasks({ column: "in-review", slim: true });
-      const inProgress = await this.store.listTasks({ column: "in-progress", slim: true });
-      const candidates = [
+      /*
+      FNXC:WorkflowResolvedColumns 2026-07-31-04:30 (the query-filter class, fourteenth sweep):
+      Two literal reads, and two per-card `task.column === …` checks inside the filters below. Those
+      checks were redundant while the query pinned the column; under a resolved read they become the
+      per-card verdict, so they convert in the same change rather than being deleted.
+
+      On a renamed board both reads returned empty, so a branch carrying ONLY foreign commits was never
+      classified and the task stayed parked on a contamination pause that nothing else clears.
+      */
+      const contaminationReviewColumns = await resolveProjectColumnsForRoles(this.store, REVIEW_ROLES);
+      const contaminationWipColumns = await resolveProjectColumnsForRoles(this.store, ["countsTowardWip"]);
+      const readContaminationBucket = async (columns: ReadonlySet<string>): Promise<Task[]> => {
+        const byId = new Map<string, Task>();
+        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 inReview = await readContaminationBucket(contaminationReviewColumns);
+      const inProgress = await readContaminationBucket(contaminationWipColumns);
+      /*
+      FNXC:WorkflowResolvedColumns 2026-07-30-20:15 (#2891 review, second round — greptile P1,
+      "fallback crosses workflow boundaries"): A CARD WHOSE BOARD CANNOT BE READ IS SKIPPED, NOT GUESSED.
+
+      Two wrong answers were tried before this one, and both are worth recording because each looked
+      correct in isolation:
+
+        1. LEGACY IDS ONLY (original). Guarantees rejection for exactly the renamed cards the
+           project-scoped query was widened to find — the sweep admits a card and then disowns it.
+        2. THE PROJECT UNION (my first fix). Removes that, and crosses workflow boundaries: a column
+           that carries a recovery role only in ANOTHER workflow starts admitting this card. This is an
+           ACTION site — the verdicts below clear a contamination pause — and rule 3 of
+           `project-union-versus-per-task-lanes.md` says over-inclusion is free for a read and costly
+           for an action. I widened on an action site, which my own note says not to do.
+
+      The honest third answer: without the card's board we cannot say whether its column carries the
+      role, so we do not decide — EXCEPT where the card sits on a legacy id, which is deliberate and
+      worth stating because it looks like an inconsistency (#2891 review, third round).
+
+      A card in literal `in-review`/`in-progress` still passes, because the legacy seed above is not a
+      guess: it is the documented degraded vocabulary, the same three-state answer used everywhere in
+      this program when a board cannot be read. Skipping those would REGRESS every unconverted and
+      legacy-id project — cards this sweep has always recovered would stop being recovered — to gain
+      consistency with a case that only arises on renamed boards. The renamed card is the one we
+      genuinely cannot classify, and it is the one that is skipped and reported. The card keeps the legacy ids seeded above — which is what it had
+      before any of this — and is REPORTED, so a card the sweep cannot classify is visible instead of
+      silently mis-decided in either direction. Same shape as the done-integrity sweep's unresolvable
+      report: the fix for "cannot answer" is to say so, not to pick a side.
+      */
+      const contaminationLanes = new Map<string, { review: Set<string>; wip: Set<string> }>();
+      const unresolvedContaminationCards: string[] = [];
+      for (const task of [...inReview, ...inProgress]) {
+        if (contaminationLanes.has(task.id)) continue;
+        const lanes = {
+          review: new Set<string>(LEGACY_COLUMN_IDS_BY_ROLE.mergeOrchestration ?? []),
+          wip: new Set<string>(LEGACY_COLUMN_IDS_BY_ROLE.countsTowardWip ?? []),
+        };
+        /*
+        FNXC:WorkflowResolvedColumns 2026-07-30-19:40 (#2891 review — greptile P1, "fallback workflow
+        rejects renamed lanes"): NARROW WHEN THE CARD CAN ANSWER, BROAD WHEN IT CANNOT.
+
+        `resolveWorkflowIrForTask` does not fail — it SUBSTITUTES the built-in IR. So a card whose
+        selection is missing or unreadable came back with `in-review`/`in-progress`, and the per-card
+        verdicts below then REJECTED the very card the project-scoped query had just admitted from a
+        renamed lane. The sweep found it and immediately disowned it.
+
+        Provenance separates "this card's board says X" from "nobody could say, here is the default".
+        Only the first is a per-card answer. When it is a substitution the card falls back to the
+        PROJECT sets that admitted it — which is broader than its own board but is exactly the
+        vocabulary this sweep already trusted to select it, and over-inclusion here costs at most a
+        contamination check on a card that did not need one.
+
+        The alternative — legacy ids only, as before — is the one shape that cannot be right: it
+        guarantees rejection for precisely the renamed cards the query was widened to find.
+        */
+        try {
+          const { ir, source } = await resolveWorkflowIrForTaskWithProvenance(this.store, task.id);
+          if (source === "default") unresolvedContaminationCards.push(task.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, "countsTowardWip")) lanes.wip.add(id);
+          }
+        } catch {
+          unresolvedContaminationCards.push(task.id);
+        }
+        contaminationLanes.set(task.id, lanes);
+      }
+      if (unresolvedContaminationCards.length > 0) {
+        log.warn(
+          `contamination sweep: ${unresolvedContaminationCards.length} card(s) left unclassified because their `
+          + `own workflow could not be resolved (${unresolvedContaminationCards.slice(0, 5).join(", ")}); `
+          + "a renamed lane there is neither cleared nor rejected.",
+        );
+      }
+      const contaminationLanesOf = (id: string) => contaminationLanes.get(id) ?? {
+        review: new Set<string>(LEGACY_COLUMN_IDS_BY_ROLE.mergeOrchestration ?? []),
+        wip: new Set<string>(LEGACY_COLUMN_IDS_BY_ROLE.countsTowardWip ?? []),
+      };
+      const contaminationCandidates = [
        ...inReview.filter((task) =>
-          task.column === "in-review" &&
+          contaminationLanesOf(task.id).review.has(task.column) &&
          allowsAutoMergeProcessing(task, settings) &&
          Boolean(task.branch) &&
          Boolean(task.worktree) &&
@@ -11050,7 +11147,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
        // projects (mirroring the FN-5704 reclaim contract), so override-less
        // tasks stay untouched while explicit autoMerge:true tasks recover.
        ...inProgress.filter((task) =>
-          task.column === "in-progress" &&
+          contaminationLanesOf(task.id).wip.has(task.column) &&
          allowsAutoMergeProcessing(task, settings) &&
          task.paused === true &&
          (task.pausedReason === "branch-cross-contamination" || task.pausedReason === "branch-conflict-unrecoverable") &&
@@ -11060,6 +11157,17 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
          !executingIds.has(task.id),
        ),
      ];
+      /*
+      Deduped across the buckets, the hazard reviewed on #2879. The two literal reads were disjoint by
+      construction; resolved ones are not, and the two filters here have DIFFERENT predicates, so a column
+      carrying both a review role and the wip role could match both and classify the same branch twice.
+      Explicit `has` guard rather than `new Map(entries)`, which keeps the LAST value for a repeated key.
+      */
+      const contaminationById = new Map<string, Task>();
+      for (const task of contaminationCandidates) {
+        if (!contaminationById.has(task.id)) contaminationById.set(task.id, task);
+      }
+      const candidates = [...contaminationById.values()];

      let recovered = 0;
      const integrationBranch = await resolveIntegrationBranch(this.options.rootDir, settings);
--- 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": 87,
+    "packages/engine/src/self-healing.ts": 84,
    "packages/engine/src/scheduler.ts": 12,
    "packages/engine/src/executor.ts": 8,
    "packages/core/src/task-store/async-comments-attachments.ts": 6,
@@ -128,7 +128,7 @@
    "plugins/fusion-plugin-reports/src/store/report-types.ts\u0000archived": 1
  },
  "queryByFile": {
-    "packages/engine/src/self-healing.ts": 35,
+    "packages/engine/src/self-healing.ts": 32,
    "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,
@@ -136,7 +136,6 @@
    "packages/core/src/task-store/async-archive-lineage.ts": 1,
    "packages/core/src/task-store/async-self-healing.ts": 1,
    "packages/engine/src/agent-tools.ts": 1,
-    "packages/engine/src/auto-merge-finalization.ts": 1,
-    "packages/engine/src/workflow-node-handlers.ts": 1
+    "packages/engine/src/auto-merge-finalization.ts": 1
  }
 }