diff --git a/.changeset/executor-resume-orphaned-lane.md b/.changeset/executor-resume-orphaned-lane.md new file mode 100644 index 0000000000..30201b52c9 --- /dev/null +++ b/.changeset/executor-resume-orphaned-lane.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Orphaned tasks are resumed after a restart on boards with renamed columns. +category: fix +dev: `resumeOrphaned` read the wip lane by role via `listWipLaneTasks()` but its filter still compared `t.column === "in-progress"`, so on a renamed board the read found the orphans and the filter discarded all of them. The filter now tests membership of the resolved wip columns. diff --git a/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts b/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts index 11fa99a39e..3ed646d060 100644 --- a/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts +++ b/packages/engine/src/__tests__/executor-resume-lanes-resolved.test.ts @@ -217,3 +217,66 @@ describe("resume lanes come from the task's own workflow", () => { }); }); }); + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-19:30 (a MISSED PAIR in resumeOrphaned): +`listWipLaneTasks()` already resolved the wip lane by role, and the filter beneath it did NOT — it +re-asserted the literal `in-progress` on the rows that read returned. So on a renamed board the read +found the orphans and the filter discarded every one. + +That is the worse half of this pattern and the reason the sibling structural test was not enough: the +read looks converted, the census scores only the comparison, and the sweep silently does nothing. The +consequence here is that orphaned tasks are NEVER resumed after a crash or restart — the single path +that recovers them — and the failure surfaces only once an operator is already investigating a crash. + +`isTaskWorkComplete` is the first thing done per surviving task, before any dispatch, worktree probing +or git, so it is the observable that needs no live registry. + +REVERT CHECK, measured: with the filter back on `t.column === "in-progress"`, this fails — the renamed +card is dropped and the sweep returns before touching it. +*/ +describe("resumeOrphaned filters by the board's OWN wip lane, not the literal", () => { + function orphanHarness(column: string) { + const { store, executor } = harness(RENAMED_IR); + const task = { + id: "FN-ORPHAN-RESUME", + column, + title: "orphaned by a restart", + description: "", + dependencies: [], + steps: [{ id: "s1", status: "pending" }], + currentStep: 0, + log: [], + createdAt: "2026-07-30T00:00:00.000Z", + updatedAt: "2026-07-30T00:00:00.000Z", + }; + const widened = store as unknown as Record; + widened.getSettings = async () => ({ globalPause: false, enginePaused: false }); + widened.listTasks = vi.fn(async (options?: { column?: string }) => + (options?.column === undefined || options.column === column ? [task] : [])); + widened.listWorkflowDefinitions = async () => [{ ir: RENAMED_IR }]; + const isTaskWorkComplete = vi.fn(() => true); + Object.assign(executor, { isTaskWorkComplete, recoverCompletedTask: vi.fn(async () => undefined) }); + return { executor, isTaskWorkComplete }; + } + + it("reaches an orphan sitting in the RENAMED wip lane", async () => { + const { executor, isTaskWorkComplete } = orphanHarness("building"); + + await executor.resumeOrphaned(); + + expect(isTaskWorkComplete).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-ORPHAN-RESUME" })); + }); + + it("does not resume a card outside the wip lane", async () => { + /* + Non-vacuous companion: a card in the board's REVIEW lane is not an orphaned execution — it has no + session to resume, and re-dispatching it would restart finished work. + */ + const { executor, isTaskWorkComplete } = orphanHarness("checking"); + + await executor.resumeOrphaned(); + + expect(isTaskWorkComplete).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/engine/src/__tests__/resolved-read-with-literal-filter.test.ts b/packages/engine/src/__tests__/resolved-read-with-literal-filter.test.ts new file mode 100644 index 0000000000..c2d50a92de --- /dev/null +++ b/packages/engine/src/__tests__/resolved-read-with-literal-filter.test.ts @@ -0,0 +1,151 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-19:55 (the MISSED PAIR ratchet, generalised past self-healing): + +THE DEFECT THIS CATCHES, found in `executor.ts` after the sibling ratchet found five in +`self-healing.ts`: a function resolves its lane by ROLE and then re-asserts a column LITERAL on the +rows it just read. + + const tasks = await this.listWipLaneTasks(); // resolved by role + const inProgress = tasks.filter((t) => t.column === "in-progress" && …); // literal + +On a renamed board the read finds the work and the filter discards all of it. This is worse than an +unconverted read, and much harder to notice: + + - the read LOOKS converted, so a reviewer scanning for `listTasks({ column: "…" })` sees nothing; + - the census scores only the comparison, so the backlog number moves the wrong way; + - a STRUCTURAL test that pins "the read asks for resolved lanes" passes — `resumeOrphaned` had one, + and it was green for the entire time the sweep was dead. + +`resumeOrphaned` is the only path that recovers orphaned tasks after a crash or restart, so its +failure surfaced only when an operator was already investigating a crash. + +WHAT COUNTS AS A PAIR: the same function both resolves lanes and compares a column id. The fallback +arm of a resolved ternary is NOT a pair — `lanes ? lanes.has(c) : c === "done"` is the correct shape, +and the literal only answers when resolution produced nothing. +*/ +import { describe, expect, it } from "vitest"; +import { readFileSync, readdirSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { join } from "node:path"; + +const SRC = fileURLToPath(new URL("..", import.meta.url)); + +const RESOLVER_CALLS = [ + "resolveProjectColumnsForRoles(", + "listWipLaneTasks(", + "resolveWorkflowIrForTaskWithProvenance(", +]; + +const LITERAL = /\.column\s*(?:!==|===)\s*"(?:todo|in-progress|in-review|done|archived|triage)"/; +/** `lanes ? lanes.has(task.column) : task.column === "done"` — the resolved answer wins; correct. */ +const RESOLVED_FALLBACK_ARM = /(?:\.includes|\.has)\(\s*[A-Za-z_.]*\.column\s*\)/; + +/* +Documented exceptions. Each is a literal that survives ON PURPOSE, with the reason recorded at the +site. An entry here asserts the degraded answer is harmless — not that the literal is invisible. +*/ +const ALLOWED: ReadonlyArray<{ file: string; because: string }> = [ + { + file: "ephemeral-worker-manager.ts", + because: "the unresolvable-workflow default: when no IR resolves there is nothing to resolve against", + }, + { + file: "triage.ts", + because: "the U11 orphan case — a row resting in a column its workflow no longer declares has no trait to resolve", + }, + { + file: "scheduler.ts", + because: "sync event listeners; resolveTaskWorkflowIrSync is inert in production and adding an await reorders handlers (pinned by sync-workflow-ir-is-always-default.pg.test.ts)", + }, + { + file: "replan-target.ts", + because: "same inert sync reader as scheduler.ts", + }, +]; + +function stripComments(source: string): string[] { + const out: string[] = []; + let inBlock = false; + for (const line of source.split("\n")) { + const trimmed = line.trim(); + if (inBlock) { + out.push(""); + if (line.includes("*/")) inBlock = false; + continue; + } + if (trimmed.startsWith("/*")) { + out.push(""); + if (!line.includes("*/")) inBlock = true; + continue; + } + if (trimmed.startsWith("//")) { out.push(""); continue; } + out.push(line); + } + return out; +} + +/* +Files with a DEDICATED ratchet of their own are skipped here, not double-allowlisted. + +`self-healing.ts` is owned by `self-healing-converted-sweeps-have-no-literal-lane-guards.test.ts`, +which is strictly more precise: it derives its converted-sweep list from the source and allows exactly +one documented literal (the log-dedup closure in `clearStaleBlockedBy`). Adding a second allowance for +the same site here would give one fact two owners that can drift apart — the failure mode this whole +program keeps hitting. One file, one ratchet. +*/ +const OWNED_ELSEWHERE = new Set(["self-healing.ts"]); + +/** Engine sources, excluding tests. Shallow by design — the class lives in the big lane files. */ +function engineSources(): string[] { + return readdirSync(SRC) + .filter((name) => name.endsWith(".ts") && !name.endsWith(".d.ts") && !OWNED_ELSEWHERE.has(name)) + .map((name) => join(SRC, name)); +} + +function owningFunction(lines: string[], index: number): string | null { + for (let i = index; i >= 0; i--) { + const match = /^ {2}(?:private |public |static )*(?:async )?([a-zA-Z][A-Za-z0-9_]*)\s*[(<]/.exec(lines[i]!); + if (match) return match[1]!; + } + return null; +} + +describe("a function that resolves lanes does not also compare a column id", () => { + const files = engineSources(); + + it("finds engine sources to scan (guards against a vacuous sweep)", () => { + /* A wrong SRC path would make every case below pass by scanning nothing. */ + expect(files.length).toBeGreaterThan(20); + }); + + for (const path of files) { + const name = path.split("/").pop()!; + const allowance = ALLOWED.find((entry) => entry.file === name); + + it(`${name} keeps no literal beside a resolved read`, () => { + const lines = stripComments(readFileSync(path, "utf8")); + const byFunction = new Map(); + + lines.forEach((line, index) => { + const owner = owningFunction(lines, index); + if (!owner) return; + const entry = byFunction.get(owner) ?? { resolves: false, literals: [] }; + if (RESOLVER_CALLS.some((call) => line.includes(call))) entry.resolves = true; + if (LITERAL.test(line) && !RESOLVED_FALLBACK_ARM.test(line)) { + entry.literals.push(`${name}:${index + 1} — ${line.trim()}`); + } + byFunction.set(owner, entry); + }); + + const pairs = [...byFunction.entries()] + .filter(([, entry]) => entry.resolves && entry.literals.length > 0) + .flatMap(([fn, entry]) => entry.literals.map((l) => `${fn}: ${l}`)); + + if (allowance) { + expect(pairs.length, `${name} is allowed documented literals (${allowance.because}) — review any change here:\n${pairs.join("\n")}`).toBeGreaterThanOrEqual(0); + return; + } + expect(pairs, `${name} resolves lanes AND compares a column id in the same function:\n${pairs.join("\n")}`).toEqual([]); + }); + } +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 9203ee7364..69ad821d28 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -5955,9 +5955,23 @@ export class TaskExecutor { return; } + /* + FNXC:WorkflowResolvedColumns 2026-07-31-19:20 (a MISSED PAIR, the class #2879 ratcheted): + `listWipLaneTasks()` above already resolves the wip lane by role. This filter did not — it re-asserted + the literal `in-progress` on the rows that read returned, so on a renamed board the read found the + orphans and the filter dropped every one. + + That is the worse half of the pattern: the read looks converted, the census counts only the + comparison, and the sweep silently does nothing. Here it means orphaned tasks are NEVER resumed after + a crash or restart — the one path that recovers them. + + The rows come from a `listTasks({ column })` per resolved column, so a row is in that column by + definition; the re-assert only ever had value as a stale-snapshot guard, which membership preserves. + */ + const wipColumns = await resolveProjectColumnsForRoles(this.store, ["countsTowardWip"]); const tasks = await this.listWipLaneTasks(); const inProgress = tasks.filter( - (t) => t.column === "in-progress" && !t.deletedAt && !this.executing.has(t.id) && !t.paused, + (t) => wipColumns.has(t.column) && !t.deletedAt && !this.executing.has(t.id) && !t.paused, ); if (inProgress.length === 0) return; diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index c5d34d5d5e..554d92f495 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -3,7 +3,7 @@ "byFile": { "packages/engine/src/self-healing.ts": 56, "packages/engine/src/scheduler.ts": 12, - "packages/engine/src/executor.ts": 8, + "packages/engine/src/executor.ts": 7, "packages/engine/src/notification/notification-service.ts": 5, "packages/engine/src/replan-target.ts": 4, "packages/engine/src/restart-recovery-coordinator.ts": 4,