executor: orphaned tasks were never resumed after a restart on a renamed board (#2947)
`resumeOrphaned` is the only path that recovers tasks after a crash or
restart. On a board with renamed columns it recovered **nothing**.
## A missed pair, not an unconverted read
```ts
const tasks = await this.listWipLaneTasks(); // resolved by role — already converted
const inProgress = tasks.filter(
(t) => t.column === "in-progress" && …, // literal — discards everything the read found
);
```
The read was already resolved. The filter directly beneath it
re-asserted the literal on the rows that read returned, so the sweep
found the orphans and threw them all away.
**This is the worse half of the class, and it hid well:**
- the read *looks* converted, so scanning for `listTasks({ column: "…"
})` finds nothing;
- the census scores only the comparison, so the backlog number moves the
**wrong way** as you convert;
- a **structural test already existed** pinning "the read asks for
resolved lanes" — `executor-resume-query-lanes.test.ts` — and it was
green the entire time the sweep was dead. A test asserting the read
exists says nothing about the filter beneath it.
The failure only surfaces after a crash, when an operator is already
investigating the crash and has every reason to blame that instead.
## The ratchet, generalised
#2944 ratcheted this class inside `self-healing.ts` after review found
one instance and a follow-up audit found five more. This generalises it
to every engine source: a function that resolves lanes **and** compares
a column id in the same body is a pair.
Excluded, deliberately:
- the **fallback arm** of a resolved ternary (`lanes ? lanes.has(c) : c
=== "done"`) — the correct shape;
- four files whose literals are deliberate, each with the reason
recorded: `ephemeral-worker-manager` (unresolvable-workflow default),
`triage` (the U11 orphan case), `scheduler` and `replan-target` (sync
listeners on the inert sync IR reader, already pinned by
`sync-workflow-ir-is-always-default.pg.test.ts`);
- `self-healing.ts`, because it has a **dedicated** ratchet that is
strictly more precise. Two ratchets allowlisting the same site is one
fact with two owners, free to drift — the exact failure mode this
program keeps hitting. One file, one ratchet.
It carries a positive control: a wrong source path would make every case
pass by scanning nothing.
**I swept the rest of the engine with it and executor.ts was the only
genuine hit** — everything else is documented-deliberate or blocked on
the inert sync reader.
## Revert results
Each measured by restoring the literal filter and re-running:
| | reverted → |
| --- | --- |
| behavioural case | fails — the renamed card is dropped and the sweep
returns before touching it |
| the ratchet | fails, naming the site: `resumeOrphaned:
executor.ts:5974` |
A non-vacuous companion (card in the review lane → not resumed) rules
out a filter that matches everything: a card in review has no session to
resume, and re-dispatching it would restart finished work.
**Measured:** `executor.ts` column guards 8 → 7; baseline re-recorded
downward.
## Verification
`pnpm test:gate` 161 + 487 + 13 + 71; executor prompt/soft-delete/resume
suites plus the new ratchet, 357 passed; `tsc` engine clean; `pnpm
lint`, `check:changesets`, census `--strict` and
`check-sql-column-literals` clean, each run explicitly.
This commit is contained in:
7
.changeset/executor-resume-orphaned-lane.md
Normal file
7
.changeset/executor-resume-orphaned-lane.md
Normal file
@@ -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.
|
||||||
@@ -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<string, unknown>;
|
||||||
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -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<string, { resolves: boolean; literals: string[] }>();
|
||||||
|
|
||||||
|
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([]);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
@@ -5955,9 +5955,23 @@ export class TaskExecutor {
|
|||||||
return;
|
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 tasks = await this.listWipLaneTasks();
|
||||||
const inProgress = tasks.filter(
|
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;
|
if (inProgress.length === 0) return;
|
||||||
|
|||||||
@@ -3,7 +3,7 @@
|
|||||||
"byFile": {
|
"byFile": {
|
||||||
"packages/engine/src/self-healing.ts": 56,
|
"packages/engine/src/self-healing.ts": 56,
|
||||||
"packages/engine/src/scheduler.ts": 12,
|
"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/notification/notification-service.ts": 5,
|
||||||
"packages/engine/src/replan-target.ts": 4,
|
"packages/engine/src/replan-target.ts": 4,
|
||||||
"packages/engine/src/restart-recovery-coordinator.ts": 4,
|
"packages/engine/src/restart-recovery-coordinator.ts": 4,
|
||||||
|
|||||||
Reference in New Issue
Block a user