diff --git a/.changeset/self-healing-foreign-contamination-query.md b/.changeset/self-healing-foreign-contamination-query.md new file mode 100644 index 0000000000..17887aa53b --- /dev/null +++ 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. 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 89b89c4ff3..48720fa070 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 @@ -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(); + 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(); + return { ...actual, classifyForeignOnlyContamination: (...args: unknown[]) => classifyForeignOnlyContamination(...args as []) }; +}); + vi.mock("../run-audit.js", async (importOriginal) => { const actual = await importOriginal(); 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(); + }); }); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 80b3fc5a15..c236a0bf4f 100644 --- 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(); - 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): 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 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; wip: Set }>(); + const unresolvedContaminationCards: string[] = []; + for (const task of [...inReview, ...inProgress]) { + if (contaminationLanes.has(task.id)) continue; + const lanes = { + review: new Set(LEGACY_COLUMN_IDS_BY_ROLE.mergeOrchestration ?? []), + wip: new Set(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(LEGACY_COLUMN_IDS_BY_ROLE.mergeOrchestration ?? []), + wip: new Set(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(); + 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); diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 43c22c1fdf..5753061c5b 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": 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 } }