fleet: executor.ts 57 → 15 on top of #2689 — the review/wip lanes, 4 half-conversions, 8-of-19 revert proof (#2703)
**Supersedes #2691, which I am closing.** #2689 landed the terminal-pair batch on `executor.ts` while my PR was open on the same file — we collided, that PR won the race, and 30 of my 70 conversions are now identical to its work. Rather than resolve 30 conflict hunks in a 20k-line lifecycle file (unreviewable, and the wrong artifact to hand you), I rebuilt from `origin/main`. **`executor.ts` 57 → 15.** Repo backlog 679 → **650**. ## The four that are defects, not vocabulary **1. `isReentrantPausedAbortedInFlightNode` resolved lanes at the END, for its return value, while its four `in-review` eligibility gates were literals.** On a renamed board those gates all read false — so a review card skipped the global-pause recheck, the `autoMerge === false` refusal, the shared-branch-member arbitration **and** the merge-confirmed refusal — and then the lane-resolved final line answered *"re-entrant"*. FN-7214's own comment says an auto-merge-off review row must stay terminal. **2. The REVERSE half-conversion.** `routeGraphFailureToExecutionResume`'s destination was already resolved (U7's `resolveReboundColumnFor`) behind a gate that was still three literals — so the router refused before reaching its own working move. | direction | what happens | visible? | |---|---|---| | resolved gate → literal destination | card admitted, move rejected by a board with no such column | **yes** — the move errors | | literal gate → resolved destination | card refused; the working recovery never runs | **no** | Only the second is silent, which is exactly why it survived U7's own conversion of that destination. **When you convert a destination, check the gate in front of it in the same commit.** **3. `routeUnusableWorktreeGraphFailureToRecovery` skipped FN-5147's auto-merge-off gate** on a renamed board — an automatic recovery moving a human-review-terminal card backward. #2689 converted the terminal guard at the top of that method; this is the other half of the same decision, which is the general risk when two people split one file. **4. `handleGraphFailure`'s `alreadyFinalizedToReview` / `suppressFinalizedCompletionAbort`** read `column !== "in-progress"`, so a completed, already-finalized row looked still-in-wip: FN-6644 / FN-6647's suppression never fired and the row was re-parked as an operator-action pause abort — the durability gap those tickets closed. ## Two patterns worth carrying to other files **An inert guard rarely reports "renamed board" — it reports something that sounds like a different problem.** `finalizeAlreadyReviewedTask` returned `"missing"` for a card sitting in review. The completion handoff logged *"no longer active"* for a card that was actively executing. The stuck-requeue cleanup logged *"recovered concurrently"* about a recovery that had not happened. Three different false explanations, one cause. **Directions differ inside one family, so convert per method, not per pattern.** Most wip guards read `!== "in-progress"` and REFUSE on no-match (renamed board → silently disabled). The rerun watchdog reads `=== "in-progress"` and SKIPS on match — there the literal never matched, so a rerun could fire on a card **mid-execution**. A mechanical sweep of `!== "in-progress"` fixes the refusals and leaves that admission in place. Also: the resolver choice inverts within a few lines. *"Is this card in the ONE column finalize targets?"* needs the **complete** column — the terminal union carries the legacy ids, so a card in a column merely *named* `done` reads as already finalized and the finalize is **skipped**. *"Is this card already finished, so do not move it?"* needs the **union** — over-inclusion only skips a move, under-inclusion moves a finished card out of its terminal column. Both are recorded at their sites. ## Revert proof 19 cases in `executor-graph-failure-lanes-resolved.test.ts`, on a board sharing **no** column id with the default lineage (on the default board these guards are correct by coincidence — the literals *are* the board). **8 fail on revert.** The rest are labelled **in the file** as paired positives, default-board no-change cases, or — in one instance — a guard that is genuinely redundant with a later lane check. I would rather label a case as non-evidence than count it. Two fixture corrections are recorded at their sites, both my own assertion failing to touch the behaviour it named: asserting a router's return value (which was already false for an unrelated reason — fixed by spying on the recovery call), and `allowsAutoMergeProcessing` keying on the **global** setting rather than `task.autoMerge` (fixed the fixture, not the assertion). ## The 15 that remain, each with a reason - **7 `to`/`from` move-effect parameters** — a move's endpoints, not a card's resting column. Trait-hook territory. - **2 enumeration scans** — one is a `listTasks({ column: "in-progress" })` query whose filter cannot be converted without the query (converting the filter alone reads as done and changes nothing); the other loops every task, so per-task resolution is a real cost wanting a shared memo. - **`12325`, the dependency guard** — *"is this dependency satisfied?"* is not any single lane role. The same question exists at `register-task-workflow-routes.ts:3995`; both should be decided once, together. - **`14484`** (`fromColumn === "in-review" && toColumn === "in-review"`) — a same-lane move check that belongs with the move-effect group above. ## Verification `pnpm test:gate` **158 / 10 / 487 / 71** · **135/135** across the 17 suites covering these paths · `tsc -p packages/engine` clean · `pnpm lint` clean · census `--strict` exit 0, baseline re-recorded. No changeset: `@fusion/engine` is private and the behaviour change is confined to renamed boards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved workflow execution across boards with renamed lifecycle lanes by resolving lane targets per board instead of using fixed column names. * Fixed review, WIP, completion, and failure-recovery behaviors to respect the correct board snapshot (including auto-merge and terminal work states). * Improved artifact-recovery protection timing and tightened execution-resume gating for failure scenarios. * **Tests** * Added a new lifecycle invariant test suite covering renamed-lane recovery, resume, pause/abort, and router-gating behavior. * Updated lifecycle column census baseline data. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
@@ -0,0 +1,501 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-08:10 (fleet: executor.ts graph-failure recovery family):
|
||||
|
||||
THE INVARIANT: every lifecycle question the graph-failure recovery family asks is answered from the
|
||||
task's OWN workflow, and the halves of one decision answer it from ONE snapshot.
|
||||
|
||||
Two of the conversions in this family fix behaviour rather than vocabulary, and both are the
|
||||
half-conversion shape — a gate that reads the wrong board is not merely inert, it ADMITS work the
|
||||
gate existed to refuse:
|
||||
|
||||
1. `isReentrantPausedAbortedInFlightNode` resolved lanes at the END, for its return value, while its
|
||||
four `in-review` eligibility gates were literals. On a renamed board those gates all read false,
|
||||
so a card in review skipped the global-pause recheck, the `autoMerge === false` refusal, the
|
||||
shared-branch-member arbitration and the merge-confirmed refusal — and then the lane-resolved
|
||||
final line answered "re-entrant". FN-7214's own comment says an auto-merge-off review row must
|
||||
stay terminal, so this was the documented invariant failing silently on any renamed board.
|
||||
|
||||
2. `routeUnusableWorktreeGraphFailureToRecovery` asked "already finished?" and "in review?" as three
|
||||
literals. On a renamed board it read not-finished AND not-in-review, so recovery proceeded with
|
||||
the FN-5147 gate skipped: an automatic recovery moving a human-review-terminal card backward.
|
||||
|
||||
WHY THE RENAMED BOARD IS THE FIXTURE. On the default lineage every one of these guards is correct by
|
||||
coincidence — the literals ARE the board. A test on the default board passes before and after the
|
||||
change and proves nothing, which is how this class of defect survived every previous suite.
|
||||
|
||||
REVERT PROOF, measured: restore either literal and the matching case here fails (`autoMerge:false`
|
||||
review row is admitted as re-entrant; the finished card is admitted into worktree recovery).
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import "./executor-test-helpers.js";
|
||||
import { TaskExecutor } from "../executor.js";
|
||||
import { createMockStore } from "./executor-test-helpers.js";
|
||||
import type { WorkflowIr } from "@fusion/core";
|
||||
|
||||
/** A board whose lifecycle columns share NO id with the default lineage. */
|
||||
const RENAMED_IR = {
|
||||
version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [],
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] },
|
||||
{ id: "queued", name: "Queued", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "checking", name: "Checking", traits: [{ trait: "merge" }] },
|
||||
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
|
||||
{ id: "filed", name: "Filed", traits: [{ trait: "archived" }] },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
|
||||
function harness(ir: WorkflowIr | undefined, task: Record<string, unknown>, settingsOverride: Record<string, unknown> = {}) {
|
||||
const store = createMockStore();
|
||||
const selection = { workflowId: "wf-renamed", stepIds: [] as string[] };
|
||||
const widened = store as unknown as Record<string, unknown>;
|
||||
widened.getTaskWorkflowSelection = () => (ir ? selection : undefined);
|
||||
widened.getTaskWorkflowSelectionAsync = async () => (ir ? selection : undefined);
|
||||
widened.getWorkflowDefinition = async () => (ir ? { ir } : undefined);
|
||||
widened.getTask = async () => task;
|
||||
widened.getSettings = async () => ({ maxConcurrent: 4, maxWorktrees: 4, pollIntervalMs: 1000, autoMerge: true, globalPause: false, enginePaused: false, ...settingsOverride });
|
||||
store.recordRunAuditEvent = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
const executor = new TaskExecutor(store as never, "/repo");
|
||||
return { store, executor };
|
||||
}
|
||||
|
||||
const pauseAbortResult = {
|
||||
interruptedAbortKind: "engine-pause",
|
||||
interruptedNodeId: "execute",
|
||||
visitedNodeIds: ["execute"],
|
||||
context: {},
|
||||
} as never;
|
||||
|
||||
function reentrant(executor: TaskExecutor, live: unknown): Promise<boolean> {
|
||||
return (executor as unknown as {
|
||||
isReentrantPausedAbortedInFlightNode: (
|
||||
live: unknown, result: unknown, provenance: string, pausedAborted: boolean, userCanceled: boolean,
|
||||
) => Promise<boolean>;
|
||||
}).isReentrantPausedAbortedInFlightNode(live, pauseAbortResult, "engine-abort", true, false);
|
||||
}
|
||||
|
||||
describe("FN-7214: an auto-merge-off review row stays terminal on a RENAMED board", () => {
|
||||
it("refuses re-entry for a review-lane card with autoMerge:false", async () => {
|
||||
/*
|
||||
The measured pre-fix behaviour: the four `in-review` gates compared against the literal, `checking`
|
||||
is not `in-review`, so every refusal was skipped — and the final lane-resolved line then matched
|
||||
`resumeLanes.review` and returned TRUE. The card was re-entered behind a human review gate.
|
||||
*/
|
||||
const live = { id: "FN-1", column: "checking", autoMerge: false, graphResumeRetryCount: 0 };
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await reentrant(executor, live)).toBe(false);
|
||||
});
|
||||
|
||||
it("still admits a review-lane card whose auto-merge is ON, so the fix is not 'refuse review rows'", async () => {
|
||||
// The paired positive. A gate that returns false unconditionally would pass the case above.
|
||||
const live = { id: "FN-2", column: "checking", autoMerge: true, graphResumeRetryCount: 0 };
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await reentrant(executor, live)).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps the same answers on the DEFAULT board, where the literals happened to be right", async () => {
|
||||
// The conversion must not change behaviour where the old spelling was already correct.
|
||||
const offLive = { id: "FN-3", column: "in-review", autoMerge: false, graphResumeRetryCount: 0 };
|
||||
const onLive = { id: "FN-4", column: "in-review", autoMerge: true, graphResumeRetryCount: 0 };
|
||||
|
||||
expect(await reentrant(harness(undefined, offLive).executor, offLive)).toBe(false);
|
||||
expect(await reentrant(harness(undefined, onLive).executor, onLive)).toBe(true);
|
||||
});
|
||||
|
||||
it("refuses a card in the board's COMPLETE column — and this one passes either way", async () => {
|
||||
/*
|
||||
HONEST LABEL, because I checked: reverting the terminal-pair literal here does NOT redden this case.
|
||||
The method's final line already answers "is the card in a resume lane", and `shipped` is not one, so
|
||||
the terminal guard is redundant *in this method*. Kept as the paired negative — it pins that a
|
||||
finished card is refused however the refusal is reached — but it is not evidence for the conversion.
|
||||
The terminal conversions that DO change behaviour are proven in the worktree-recovery block below.
|
||||
*/
|
||||
const live = { id: "FN-5", column: "shipped", autoMerge: true, graphResumeRetryCount: 0 };
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await reentrant(executor, live)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-08:40:
|
||||
ASSERT THE CALL, NOT THE RETURN VALUE. My first version of this block asserted
|
||||
`routeUnusableWorktreeGraphFailureToRecovery(...) === false` and passed with the literals restored —
|
||||
the pre-fix path ran the whole recovery and then returned false for an unrelated reason (the mock's
|
||||
outcome is not `requeue-todo`). A revert check that stays green is the signal I keep re-learning to
|
||||
respect: the assertion was not touching the behaviour it claimed to cover.
|
||||
|
||||
Spying on `recoverMissingWorktreeSessionStartFailure` asks the question the guard actually decides —
|
||||
was recovery ATTEMPTED on this card? — and it discriminates in both directions.
|
||||
*/
|
||||
describe("FN-5147: unusable-worktree recovery does not run on a finished or human-review card (RENAMED board)", () => {
|
||||
const SESSION_FAILURE = "Refusing to start coding agent in missing worktree: /gone";
|
||||
|
||||
function harnessWithSpy(live: Record<string, unknown>, settingsOverride: Record<string, unknown> = {}) {
|
||||
const { executor, store } = harness(RENAMED_IR, live, settingsOverride);
|
||||
const recover = vi.fn().mockResolvedValue("requeue-todo");
|
||||
(executor as unknown as Record<string, unknown>).recoverMissingWorktreeSessionStartFailure = recover;
|
||||
return { executor, store, recover };
|
||||
}
|
||||
|
||||
function route(executor: TaskExecutor, live: Record<string, unknown>): Promise<boolean> {
|
||||
return (executor as unknown as {
|
||||
routeUnusableWorktreeGraphFailureToRecovery: (task: unknown, live: unknown, result: unknown) => Promise<boolean>;
|
||||
}).routeUnusableWorktreeGraphFailureToRecovery({ id: live.id }, live, {
|
||||
context: { "node:execute:error": SESSION_FAILURE },
|
||||
visitedNodeIds: ["execute"],
|
||||
});
|
||||
}
|
||||
|
||||
it("does not attempt recovery for a card in the board's COMPLETE column", async () => {
|
||||
// Pre-fix: `shipped` is neither `done` nor `archived`, so recovery ran on a finished card.
|
||||
const live = { id: "FN-7", column: "shipped", worktree: "/gone" };
|
||||
const { executor, recover } = harnessWithSpy(live);
|
||||
|
||||
expect(await route(executor, live)).toBe(false);
|
||||
expect(recover).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not attempt recovery for a card in the board's ARCHIVED column", async () => {
|
||||
const live = { id: "FN-8", column: "filed", worktree: "/gone" };
|
||||
const { executor, recover } = harnessWithSpy(live);
|
||||
|
||||
expect(await route(executor, live)).toBe(false);
|
||||
expect(recover).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("applies the FN-5147 auto-merge gate to the board's REVIEW lane", async () => {
|
||||
/*
|
||||
The half-conversion in its most consequential form: pre-fix the `in-review` literal did not match
|
||||
`checking`, so the auto-merge-off refusal never ran and an automatic recovery moved a
|
||||
human-review-terminal card backward. `allowsAutoMergeProcessing` is what must be consulted, and it
|
||||
can only be reached once the review lane is resolved.
|
||||
*/
|
||||
/*
|
||||
THE GATE KEYS ON THE GLOBAL SETTING, not on `task.autoMerge`: `allowsAutoMergeProcessing` is
|
||||
`(settings.autoMerge !== false || task.autoMerge === true) && no manual open PR`. My first fixture
|
||||
set only `task.autoMerge: false` and the case failed — the recovery ran, correctly, because a
|
||||
per-task false does not withdraw a card from automatic processing. Correcting the fixture rather
|
||||
than the assertion: FN-5147 is about the OPERATOR turning auto-merge off for the project.
|
||||
*/
|
||||
const live = { id: "FN-9", column: "checking", worktree: "/gone" };
|
||||
const { executor, recover } = harnessWithSpy(live, { autoMerge: false });
|
||||
|
||||
expect(await route(executor, live)).toBe(false);
|
||||
expect(recover).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("STILL recovers a wip-lane card, so the fix is not 'never recover'", async () => {
|
||||
// The paired positive: the guards above must not have turned the recovery path off wholesale.
|
||||
const live = { id: "FN-10", column: "building", worktree: "/gone" };
|
||||
const { executor, recover } = harnessWithSpy(live);
|
||||
|
||||
expect(await route(executor, live)).toBe(true);
|
||||
expect(recover).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-10:30 (fleet: executor.ts wip-lane liveness family):
|
||||
|
||||
THE INVARIANT: "is this card still executing?" is the board's WIP lane.
|
||||
|
||||
TWO DIRECTIONS, and this is why the family is converted per method rather than by matching the text.
|
||||
Most of these guards read `column !== "in-progress"` and REFUSE when they do not match, so a renamed
|
||||
board silently disabled them (the completion handoff was deferred on every card, the deferred
|
||||
approval resume never resumed, the completed-task watchdog returned on every tick). But the rerun
|
||||
watchdog reads `column === "in-progress"` and SKIPS when it matches — so on a renamed board that one
|
||||
never skipped, and a rerun could fire on a card that was still mid-execution. A mechanical swap of
|
||||
every `!== "in-progress"` would have converted the refusals and left the admission.
|
||||
|
||||
`resumeApprovalAfterUnwindIfNeeded` is the case pinned here: it is a private method with a single
|
||||
boolean answer and no side effects on the refusal path, so the assertion is direct.
|
||||
*/
|
||||
describe("the wip-lane liveness family resolves the board's own wip column", () => {
|
||||
function resumeApproval(executor: TaskExecutor, taskId: string): Promise<boolean> {
|
||||
return (executor as unknown as {
|
||||
resumeApprovalAfterUnwindIfNeeded: (id: string) => Promise<boolean>;
|
||||
}).resumeApprovalAfterUnwindIfNeeded(taskId);
|
||||
}
|
||||
|
||||
function armed(live: Record<string, unknown>, ir: WorkflowIr | undefined) {
|
||||
const { executor, store } = harness(ir, live);
|
||||
// The deferral marker this method consumes, plus a stub for the dispatch it guards.
|
||||
(executor as unknown as { approvalResumeAfterUnwind: Set<string> }).approvalResumeAfterUnwind.add(live.id as string);
|
||||
const dispatch = vi.fn().mockResolvedValue(true);
|
||||
(executor as unknown as Record<string, unknown>).dispatchUnpauseResume = dispatch;
|
||||
return { executor, store, dispatch };
|
||||
}
|
||||
|
||||
it("resumes a deferred approval for a card in the board's wip lane", async () => {
|
||||
// Pre-fix: `building` !== "in-progress", so the resume refused on every renamed board.
|
||||
const live = { id: "FN-11", column: "building" };
|
||||
const { executor, dispatch } = armed(live, RENAMED_IR);
|
||||
|
||||
expect(await resumeApproval(executor, "FN-11")).toBe(true);
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("still refuses a card that has LEFT the wip lane", async () => {
|
||||
// The paired negative: the fix must not resume work on a card that moved on.
|
||||
const live = { id: "FN-12", column: "checking" };
|
||||
const { executor, dispatch } = armed(live, RENAMED_IR);
|
||||
|
||||
expect(await resumeApproval(executor, "FN-12")).toBe(false);
|
||||
expect(dispatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still refuses a PAUSED card in the wip lane, so the lane is not the only gate", async () => {
|
||||
const live = { id: "FN-13", column: "building", paused: true };
|
||||
const { executor, dispatch } = armed(live, RENAMED_IR);
|
||||
|
||||
expect(await resumeApproval(executor, "FN-13")).toBe(false);
|
||||
expect(dispatch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("behaves identically on the DEFAULT board", async () => {
|
||||
const live = { id: "FN-14", column: "in-progress" };
|
||||
const { executor, dispatch } = armed(live, undefined);
|
||||
|
||||
expect(await resumeApproval(executor, "FN-14")).toBe(true);
|
||||
expect(dispatch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-11:20 (fleet: executor.ts review-lane family):
|
||||
|
||||
THE INVARIANT: "is this card in review?" is the board's review lane.
|
||||
|
||||
`finalizeAlreadyReviewedTask` is the case pinned here because its failure is the loudest: it returns
|
||||
"missing" — a word that reads as "the task is gone" — for a card that is sitting in review on a
|
||||
renamed board. Everything downstream of the already-reviewed finalize path was therefore dead on any
|
||||
board that renamed its merge lane, and the log line said the task could not be found.
|
||||
*/
|
||||
describe("the review-lane family resolves the board's own review column", () => {
|
||||
function finalizeAlreadyReviewed(executor: TaskExecutor, taskId: string): Promise<string> {
|
||||
return (executor as unknown as {
|
||||
finalizeAlreadyReviewedTask: (id: string) => Promise<string>;
|
||||
}).finalizeAlreadyReviewedTask(taskId);
|
||||
}
|
||||
|
||||
it("does not report a review-lane card as MISSING", async () => {
|
||||
// Pre-fix: `checking` !== "in-review", so this returned "missing" for a card plainly in review.
|
||||
const live = { id: "FN-15", column: "checking", steps: [], workflowStepResults: [] };
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await finalizeAlreadyReviewed(executor, "FN-15")).not.toBe("missing");
|
||||
});
|
||||
|
||||
it("still reports a card OUTSIDE the review lane as missing", async () => {
|
||||
// The paired negative: the guard must still refuse a card that is not in review at all.
|
||||
const live = { id: "FN-16", column: "building", steps: [], workflowStepResults: [] };
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await finalizeAlreadyReviewed(executor, "FN-16")).toBe("missing");
|
||||
});
|
||||
|
||||
it("behaves identically on the DEFAULT board", async () => {
|
||||
const live = { id: "FN-17", column: "in-review", steps: [], workflowStepResults: [] };
|
||||
const { executor } = harness(undefined, live);
|
||||
|
||||
expect(await finalizeAlreadyReviewed(executor, "FN-17")).not.toBe("missing");
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-13:30 (fleet: executor.ts — the REVERSE half-conversion):
|
||||
|
||||
`routeGraphFailureToExecutionResume`'s move DESTINATION was already resolved from the workflow (U7's
|
||||
`resolveReboundColumnFor`) while its entry gate compared against three default-lineage literals. So on
|
||||
a renamed board the router refused before it ever reached the resolved move: a recovery that was fully
|
||||
implemented, never running.
|
||||
|
||||
That is the mirror image of the dangerous half-conversion. The familiar direction admits a card and
|
||||
then sends it to a column the board does not declare; this direction refuses a card whose recovery
|
||||
already worked. Both are one decision reading two boards, and only the second is silent — which is
|
||||
why it survived.
|
||||
|
||||
The gate admits three shapes, so all three are asserted plus the refusal, which is what stops a gate
|
||||
that returns true unconditionally from passing.
|
||||
*/
|
||||
describe("the execution-resume router's gate reads the same board as its destination", () => {
|
||||
function routeResume(executor: TaskExecutor, live: Record<string, unknown>, failureValue: string): Promise<boolean> {
|
||||
return (executor as unknown as {
|
||||
routeGraphFailureToExecutionResume: (live: unknown, failedNode: string, failureValue: string) => Promise<boolean>;
|
||||
}).routeGraphFailureToExecutionResume(live, "merge", failureValue);
|
||||
}
|
||||
|
||||
const withIncompleteSteps = (column: string, id: string) => ({
|
||||
id, column, worktree: "/wt", steps: [{ name: "s", status: "pending" }], workflowStepResults: [],
|
||||
});
|
||||
|
||||
it("admits a review-lane card", async () => {
|
||||
const live = withIncompleteSteps("checking", "FN-18");
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await routeResume(executor, live, "other")).toBe(true);
|
||||
});
|
||||
|
||||
it("admits a HOLD-lane card that still has unfinished steps", async () => {
|
||||
const live = withIncompleteSteps("queued", "FN-19");
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await routeResume(executor, live, "other")).toBe(true);
|
||||
});
|
||||
|
||||
it("admits a WIP-lane card after a premature merge attempt", async () => {
|
||||
// The third shape: implementation-incomplete merge failure with steps still open.
|
||||
const live = withIncompleteSteps("building", "FN-20");
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await routeResume(executor, live, "implementation-incomplete")).toBe(true);
|
||||
});
|
||||
|
||||
it("still REFUSES a card in the intake lane, which is none of the three shapes", async () => {
|
||||
const live = withIncompleteSteps("backlog", "FN-21");
|
||||
const { executor } = harness(RENAMED_IR, live);
|
||||
|
||||
expect(await routeResume(executor, live, "other")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-20:50 (PR #2703 review — greptile P1, the sync-resolver no-op):
|
||||
|
||||
THE INVARIANT: no lifecycle guard in this file resolves its lane through the SYNCHRONOUS resolver.
|
||||
|
||||
`resolvePlannerLanes` reads `store.resolveTaskWorkflowIrSync`, whose selection reader returns `undefined`
|
||||
unconditionally in PostgreSQL mode — the shipped backend. So a sync-resolved guard silently answers with
|
||||
the DEFAULT workflow's ids on every real board: the census counts the site as converted, `--strict` drops
|
||||
by one, and the behaviour is identical to the literal. That is worse than an unconverted literal, because
|
||||
the number claims the site is done.
|
||||
|
||||
This is a STRUCTURAL assertion rather than a behavioural one, and deliberately so. A behavioural test
|
||||
would need a PostgreSQL-backed store to demonstrate the no-op, and the thing worth preventing is not one
|
||||
guard misbehaving — it is the pattern being reintroduced anywhere in this file by someone who reads
|
||||
"synchronous classifier" and reaches for the synchronous resolver, exactly as I did.
|
||||
|
||||
The two legitimate `resolvePlannerLanes` call sites are MOVE destinations (`PlannerLanes` exists so a
|
||||
caller refuses rather than inventing a column), which is a different question from "which lane is this
|
||||
card in" and is why they are allowlisted here by name.
|
||||
*/
|
||||
describe("no lifecycle GUARD resolves its lane synchronously", () => {
|
||||
it("keeps resolvePlannerLanes out of column-comparison guards in executor.ts", async () => {
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const source = await readFile(new URL("../executor.ts", import.meta.url), "utf8");
|
||||
|
||||
/* Strip block comments: the notes explaining WHY the sync resolver is unsafe mention it by name. */
|
||||
const code = source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
|
||||
const callSites = [...code.matchAll(/resolvePlannerLanes\s*\(/g)].length;
|
||||
|
||||
/*
|
||||
Three call sites are the move/promotion destinations documented above (two in the planner-column
|
||||
helpers, one in the promotion path), plus the import. Any FOURTH is a new sync resolution and must be
|
||||
justified — if it is a guard, it is a no-op on PostgreSQL and the census will claim it is converted.
|
||||
*/
|
||||
expect(callSites).toBe(3);
|
||||
});
|
||||
|
||||
it("does not compare a column against a synchronously-resolved lane on the same line", async () => {
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const source = await readFile(new URL("../executor.ts", import.meta.url), "utf8");
|
||||
const code = source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
|
||||
|
||||
// The shape this guards against: `x.column === resolvePlannerLanes(...).review`.
|
||||
const inlineGuards = [...code.matchAll(/\.column\s*[!=]==\s*resolvePlannerLanes/g)];
|
||||
|
||||
expect(inlineGuards).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-02-04:30 (PR #2703 review — coderabbit MAJOR, and it is my own rule
|
||||
turned back on me):
|
||||
|
||||
THE INVARIANT: every classifier in one recovery shares ONE lane snapshot.
|
||||
|
||||
`isBenignInReviewPauseAbort` takes its review lane as a parameter precisely because a fresh resolution
|
||||
inside a classifier can disagree with the snapshot the rest of `handleGraphFailure` uses. Four sibling
|
||||
classifiers were still calling `resolveResumeLanes()` with no memo — so a workflow edit landing mid-recovery
|
||||
could have one classifier admit a card on the new board while another rejects it on the old one, and the
|
||||
recovery would take a branch neither board justifies.
|
||||
|
||||
STRUCTURAL, because the race needs a workflow edit between two awaits inside one call — reproducible only
|
||||
by instrumenting the resolver, which would pin the implementation rather than the rule. Counting
|
||||
memo-less calls in the pause-abort family is the property that actually has to hold, and it fails the
|
||||
moment someone adds a fifth classifier that resolves on its own.
|
||||
*/
|
||||
describe("one lane snapshot per recovery, across every classifier", () => {
|
||||
const MEMO_THREADED = [
|
||||
"isRetryableBenignMergePauseAbort",
|
||||
"isBenignManualMergeHoldPauseAbort",
|
||||
"handleStaleInReviewPlanPauseAbortReplay",
|
||||
"handleStaleInReviewParsePauseAbortReplay",
|
||||
"isReentrantPausedAbortedInFlightNode",
|
||||
"routeUnusableWorktreeGraphFailureToRecovery",
|
||||
"routeGraphFailureToExecutionResume",
|
||||
];
|
||||
/* Methods that own their own recovery: no caller memo to share, so the FIRST resolution is correct and a
|
||||
SECOND is the split. `handleNonContinuableSessionError` was added here after exactly that defect
|
||||
(PR #2703 review) — its eligibility check and its review branch each resolved independently. */
|
||||
const SELF_CONTAINED = ["handleNonContinuableSessionError"];
|
||||
|
||||
async function methodBody(name: string): Promise<string> {
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const source = await readFile(new URL("../executor.ts", import.meta.url), "utf8");
|
||||
const code = source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
|
||||
const start = code.indexOf(`private async ${name}(`);
|
||||
expect(start, `${name} not found — update this test, do not delete it`).toBeGreaterThan(-1);
|
||||
/*
|
||||
Bounded by the next `private ` declaration of ANY kind. My first version bounded on the next member of
|
||||
the same list, so the last entry's window ran to EOF and it accused a method of a call living 1200 lines
|
||||
away. A ratchet with the wrong window accuses the wrong function — worse than no ratchet, because the
|
||||
"fix" lands on code that was already correct.
|
||||
*/
|
||||
const next = code.indexOf("\n private ", start + 1);
|
||||
return code.slice(start, next === -1 ? code.length : next);
|
||||
}
|
||||
|
||||
it("threads the shared memo in every classifier that has one", async () => {
|
||||
const offenders: string[] = [];
|
||||
for (const name of MEMO_THREADED) {
|
||||
const body = await methodBody(name);
|
||||
// A memo-less call — `resolveResumeLanes(x)` with a single argument — is the defect.
|
||||
if (/resolveResumeLanes\(\s*[^,)]+\s*\)/.test(body)) offenders.push(name);
|
||||
}
|
||||
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
|
||||
it("resolves lanes AT MOST ONCE in a method that owns its own recovery", async () => {
|
||||
/*
|
||||
The rule differs by kind and that distinction is the point: a method called from `handleGraphFailure`
|
||||
must take the memo (zero independent resolutions), while a self-contained recovery legitimately resolves
|
||||
once. What neither may do is resolve TWICE — that is a split snapshot in both shapes.
|
||||
*/
|
||||
const offenders: Array<{ name: string; calls: number }> = [];
|
||||
for (const name of SELF_CONTAINED) {
|
||||
const body = await methodBody(name);
|
||||
const calls = [...body.matchAll(/resolveResumeLanes\(/g)].length;
|
||||
if (calls > 1) offenders.push({ name, calls });
|
||||
}
|
||||
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
|
||||
it("threads the memo from handleGraphFailure into every one of them", async () => {
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const source = await readFile(new URL("../executor.ts", import.meta.url), "utf8");
|
||||
const code = source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/.*$/gm, "");
|
||||
|
||||
// The call sites must PASS it — accepting an unused optional parameter proves nothing.
|
||||
for (const name of MEMO_THREADED.filter((n) => n !== "isReentrantPausedAbortedInFlightNode")) {
|
||||
const callSite = new RegExp(`this\\.${name}\\([^;]*resumeLanesMemo`);
|
||||
expect(callSite.test(code), `${name} call site does not pass resumeLanesMemo`).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -2356,7 +2356,10 @@ export class TaskExecutor {
|
||||
|
||||
private async finalizeAlreadyReviewedTask(taskId: string): Promise<"merged" | "blocked" | "missing"> {
|
||||
const latestTask = await this.store.getTask(taskId);
|
||||
if (!latestTask || latestTask.column !== "in-review") {
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:55 (fleet): the board's own review lane. Spelled as the
|
||||
literal, this reported "missing" — a word that reads as "the task is gone" — for a card sitting in
|
||||
review on a renamed board, and the already-reviewed finalize never ran. */
|
||||
if (!latestTask || latestTask.column !== (await this.resolveResumeLanes(taskId)).review) {
|
||||
return "missing";
|
||||
}
|
||||
|
||||
@@ -2426,7 +2429,11 @@ export class TaskExecutor {
|
||||
return true;
|
||||
}
|
||||
|
||||
if ((latestTask && latestTask.column !== "in-progress") || this.userCanceledTaskIds.has(taskId)) {
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:45 (fleet: wip-lane liveness family): "still executing"
|
||||
is the board's WIP lane. With the literal a renamed board deferred EVERY completion handoff — the
|
||||
card was never in `in-progress`, so this read "no longer active" for a card that was actively
|
||||
executing, and the handoff was dropped with a log line. */
|
||||
if ((latestTask && latestTask.column !== (await this.resolveResumeLanes(taskId)).wip) || this.userCanceledTaskIds.has(taskId)) {
|
||||
this.clearCompletedTaskWatchdog(taskId);
|
||||
executorLog.log(`${taskId}: completion handoff deferred — task no longer active (${context})`);
|
||||
await this.store.logEntry(
|
||||
@@ -3357,7 +3364,8 @@ export class TaskExecutor {
|
||||
executorLog.warn(`${taskId}: failed to read latest task state for deferred approval resume: ${error instanceof Error ? error.message : String(error)}`);
|
||||
return false;
|
||||
}
|
||||
if (latestTask.paused || latestTask.userPaused || latestTask.column !== "in-progress") return false;
|
||||
if (latestTask.paused || latestTask.userPaused
|
||||
|| latestTask.column !== (await this.resolveResumeLanes(taskId)).wip) return false;
|
||||
return this.dispatchUnpauseResume(latestTask);
|
||||
}
|
||||
|
||||
@@ -3604,7 +3612,11 @@ export class TaskExecutor {
|
||||
// Handle unpause of an in-progress task with no active session.
|
||||
// Approval can be decided while the old session is still unwinding;
|
||||
// remember that edge instead of losing the only task:updated event.
|
||||
if (!task.paused && task.column === "in-progress" && this.approvalSuspended.has(task.id)) {
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:50 (fleet): both checks in this listener ask "is
|
||||
this card still in the wip lane?"; one snapshot for the pair. With the literal neither fired on a
|
||||
renamed board — an unpaused card with no active session was never resumed. */
|
||||
const unpauseWipLane = (await this.resolveResumeLanes(task.id)).wip;
|
||||
if (!task.paused && task.column === unpauseWipLane && this.approvalSuspended.has(task.id)) {
|
||||
if (
|
||||
this.executing.has(task.id)
|
||||
|| this.activeSessions.has(task.id)
|
||||
@@ -3622,7 +3634,7 @@ export class TaskExecutor {
|
||||
// dispatchUnpauseResume owns the terminal-failure and duplicate guards.
|
||||
if (
|
||||
!task.paused
|
||||
&& task.column === "in-progress"
|
||||
&& task.column === unpauseWipLane
|
||||
&& !this.activeSessions.has(task.id)
|
||||
&& !this.activeStepExecutors.has(task.id)
|
||||
&& !this.activeWorkflowStepSessions.has(task.id)
|
||||
@@ -4271,7 +4283,8 @@ export class TaskExecutor {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!currentTask || currentTask.column !== "in-progress" || currentTask.paused) {
|
||||
if (!currentTask || currentTask.paused
|
||||
|| currentTask.column !== (await this.resolveResumeLanes(taskId)).wip) {
|
||||
return;
|
||||
}
|
||||
if (!this.isTaskWorkComplete(currentTask)) {
|
||||
@@ -4372,7 +4385,11 @@ export class TaskExecutor {
|
||||
the task back for remediation, so `in-review` must bounce back exactly like
|
||||
`in-progress` regardless of the column the completion race left it in.
|
||||
*/
|
||||
if (latestTask.column === "in-progress" || latestTask.column === "in-review") {
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:57 (fleet): both lanes from ONE snapshot — the comment
|
||||
above says in-review must bounce EXACTLY like in-progress, so resolving them separately is how the
|
||||
bounce ends up handling one lane and throwing on the other, which is the bug that comment is about. */
|
||||
const bounceLanes = await this.resolveResumeLanes(taskId);
|
||||
if (latestTask.column === bounceLanes.wip || latestTask.column === bounceLanes.review) {
|
||||
const originalExecutionStartedAt = latestTask.executionStartedAt;
|
||||
// Preserve step progress across the in-progress/in-review → todo hop:
|
||||
// moveTask's default reopen-to-todo path resets every step to
|
||||
@@ -4473,7 +4490,12 @@ export class TaskExecutor {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!currentTask || currentTask.paused || currentTask.column === "in-progress") {
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:48 (fleet): the INVERSE of the guard above — this one
|
||||
SKIPS a card that is still executing. Note the direction: with the literal on a renamed board it
|
||||
never matched, so a rerun could fire on a card mid-execution. A mechanical sweep of every
|
||||
`!== "in-progress"` would fix the refusals and leave this admission in place. */
|
||||
if (!currentTask || currentTask.paused
|
||||
|| currentTask.column === (await this.resolveResumeLanes(taskId)).wip) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -4623,12 +4645,18 @@ export class TaskExecutor {
|
||||
return await this.getCompletedTaskFinalizationDecision(taskId, taskDone) === "finalize";
|
||||
}
|
||||
|
||||
private isTaskAlreadyCompleteForNonContinuableSession(task: Task, taskDone: boolean): boolean {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-20:35 (PR #2703 review — greptile P1):
|
||||
The review lane arrives from the caller for the reason documented on `isBenignInReviewPauseAbort`: the
|
||||
synchronous resolver returns the default workflow in PostgreSQL mode, so resolving it here would have
|
||||
been a conversion that changes the census and not the behaviour.
|
||||
*/
|
||||
private isTaskAlreadyCompleteForNonContinuableSession(task: Task, taskDone: boolean, reviewLane: string): boolean {
|
||||
// FNXC:Lifecycle 2026-07-16-21:40: FN-8141 — the step-status "already complete" branch
|
||||
// must not treat skip-bypass-tainted skips as completion; an accepted done / in-review
|
||||
// column are honest completion signals and stay unaffected.
|
||||
return taskDone
|
||||
|| task.column === "in-review"
|
||||
|| task.column === reviewLane
|
||||
|| (this.isTaskWorkComplete(task) && !evaluateSkipBypassTaint(task).blocked);
|
||||
}
|
||||
|
||||
@@ -4638,7 +4666,8 @@ export class TaskExecutor {
|
||||
}
|
||||
|
||||
const liveTask = await this.store.getTask(task.id);
|
||||
if (!liveTask || !this.isTaskAlreadyCompleteForNonContinuableSession(liveTask, taskDone)) {
|
||||
const nonContinuableLanes = await this.resolveResumeLanes(task.id);
|
||||
if (!liveTask || !this.isTaskAlreadyCompleteForNonContinuableSession(liveTask, taskDone, nonContinuableLanes.review)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -4652,7 +4681,19 @@ export class TaskExecutor {
|
||||
|
||||
await this.persistTokenUsage(task.id);
|
||||
|
||||
if (liveTask.column === "in-review") {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-02-05:50 (PR #2703 review — greptile P1, and it is the same split
|
||||
I have been fixing all day, in code I wrote an hour earlier):
|
||||
ONE SNAPSHOT. The eligibility check above already resolved this task's lanes
|
||||
(`nonContinuableLanes`), and this branch resolved them AGAIN. A workflow selection or review-column
|
||||
edit between the two makes eligibility accept the card on the old board while this branch reads the new
|
||||
one — the card is then handed to `handoffTaskToReview`, reprocessing a row already in review.
|
||||
|
||||
Writing the second resolution was not carelessness about the rule; it is that the rule is invisible at
|
||||
the call site. That is the argument for the structural ratchet in
|
||||
`executor-graph-failure-lanes-resolved.test.ts` rather than for trying harder.
|
||||
*/
|
||||
if (liveTask.column === nonContinuableLanes.review) {
|
||||
this.clearCompletedTaskWatchdog(task.id);
|
||||
this.signalTaskComplete(liveTask);
|
||||
return true;
|
||||
@@ -5446,7 +5487,7 @@ export class TaskExecutor {
|
||||
source: { source: "graph-entry" | "workflow-step"; nodeId?: string },
|
||||
): Promise<void> {
|
||||
const currentTask = await this.store.getTask(task.id).catch(() => null);
|
||||
if (!currentTask || this.isRequiredArtifactRecoveryProtected(currentTask)) return;
|
||||
if (!currentTask || await this.isRequiredArtifactRecoveryProtected(currentTask)) return;
|
||||
task = currentTask;
|
||||
const decision = computeRecoveryDecision({
|
||||
recoveryRetryCount: task.recoveryRetryCount,
|
||||
@@ -5477,7 +5518,7 @@ export class TaskExecutor {
|
||||
|
||||
if (!decision.shouldRetry) {
|
||||
const liveTask = await this.store.getTask(task.id).catch(() => null);
|
||||
if (!liveTask || this.isRequiredArtifactRecoveryProtected(liveTask)) return;
|
||||
if (!liveTask || await this.isRequiredArtifactRecoveryProtected(liveTask)) return;
|
||||
const error = `REQUIRED_ARTIFACT_RECOVERY_EXHAUSTED: ${artifactKeys.join(", ")} remained missing after ${MAX_RECOVERY_RETRIES} automatic planning retries.`;
|
||||
await this.store.logEntry(task.id, error, undefined, context);
|
||||
await this.store.updateTask(task.id, {
|
||||
@@ -5499,7 +5540,7 @@ export class TaskExecutor {
|
||||
this.workflowLifecycleMovesInFlight.add(task.id);
|
||||
try {
|
||||
const liveTask = await this.store.getTask(task.id).catch(() => null);
|
||||
if (!liveTask || this.isRequiredArtifactRecoveryProtected(liveTask)) return;
|
||||
if (!liveTask || await this.isRequiredArtifactRecoveryProtected(liveTask)) return;
|
||||
await moveTaskToReplanColumn(this.store, { id: task.id, column: liveTask.column }, replanColumn);
|
||||
} finally {
|
||||
this.workflowLifecycleMovesInFlight.delete(task.id);
|
||||
@@ -5513,15 +5554,29 @@ export class TaskExecutor {
|
||||
}, context);
|
||||
}
|
||||
|
||||
private isRequiredArtifactRecoveryProtected(task: Task): boolean {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-18:25 (fleet: made ASYNC to own its resolution):
|
||||
This predicate protects a card from artifact-recovery replanning, and three of its conditions are
|
||||
lifecycle columns: the terminal pair, and a review row whose auto-merge is off (a human owns it). As
|
||||
literals they all read false on a renamed board — so a FINISHED card, or a review row a human was
|
||||
holding, could be moved to the replan column and have its status rewritten to needs-replan.
|
||||
|
||||
ASYNC rather than lane parameters: all four callers already `await store.getTask` immediately before
|
||||
calling this, so there is no new I/O ordering, and a parameter list would put the resolution in four
|
||||
places that must agree. The archived half is why the SYNC planner-lane resolver was not an option — it
|
||||
exposes no archived lane — and widening a shared resolver from inside a call-site sweep is scope creep
|
||||
that makes a conversion unreviewable.
|
||||
*/
|
||||
private async isRequiredArtifactRecoveryProtected(task: Task): Promise<boolean> {
|
||||
const terminalColumns = await resolveTerminalColumnsFor(this.store, task.id);
|
||||
const protectionReviewLane = (await this.resolveResumeLanes(task.id)).review;
|
||||
return Boolean(
|
||||
task.deletedAt
|
||||
|| task.paused
|
||||
|| task.userPaused === true
|
||||
|| task.column === "done"
|
||||
|| task.column === "archived"
|
||||
|| terminalColumns.includes(task.column)
|
||||
|| task.mergeDetails?.mergeConfirmed === true
|
||||
|| (task.column === "in-review" && task.autoMerge === false),
|
||||
|| (task.column === protectionReviewLane && task.autoMerge === false),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -9976,6 +10031,8 @@ export class TaskExecutor {
|
||||
task: Task,
|
||||
live: TaskDetail,
|
||||
result: WorkflowGraphTaskRunResult,
|
||||
/** Shared per-recovery lane snapshot — see `resolveResumeLanes`. */
|
||||
resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } },
|
||||
): Promise<boolean> {
|
||||
if (live.deletedAt) return false;
|
||||
if (live.paused || live.userPaused === true) return false;
|
||||
@@ -9991,7 +10048,11 @@ export class TaskExecutor {
|
||||
not move those tasks backward or re-enqueue them. Mirrors the gating the in-review
|
||||
self-healing sweep (recoverMissingWorktreeReviewFailures) applies before the same recovery.
|
||||
*/
|
||||
if (live.column === "in-review") {
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:25 (fleet): FN-5147 — with the literal, a renamed board
|
||||
skipped this auto-merge-off gate entirely, so an automatic recovery moved a human-review-terminal
|
||||
card backward. #2689 converted the terminal guard at the top of this method; this is the other half
|
||||
of the same decision. */
|
||||
if (live.column === (await this.resolveResumeLanes(live.id, resumeLanesMemo)).review) {
|
||||
const settings = await this.store.getSettings();
|
||||
if (!allowsAutoMergeProcessing(live, settings)) return false;
|
||||
}
|
||||
@@ -10184,6 +10245,9 @@ export class TaskExecutor {
|
||||
result: WorkflowGraphTaskRunResult,
|
||||
abortProvenance: PausedAbortProvenance | undefined,
|
||||
pausedAborted: boolean,
|
||||
/** Shared per-recovery lane snapshot — see `resolveResumeLanes`; a fresh resolution here could disagree
|
||||
* with the one the rest of `handleGraphFailure` uses. */
|
||||
resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } },
|
||||
): Promise<boolean> {
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-06-19-00:05:
|
||||
@@ -10192,7 +10256,15 @@ export class TaskExecutor {
|
||||
if (!pausedAborted) return false;
|
||||
if (abortProvenance === "global-pause" || live.userPaused === true) return false;
|
||||
if (abortProvenance === "completion-finalize") return false;
|
||||
if (live.column !== "in-review" || !this.isRetryableMergePauseAbortStatus(live.status) || live.error != null) return false;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-17:10 (fleet: executor.ts review-lane classifiers, on top of #2689):
|
||||
"IS THIS CARD IN THE REVIEW LANE?" from the task's own workflow. Five pause-abort classifiers asked it
|
||||
as the default lineage's literal, and each refusal drops the card through to the operator-action park
|
||||
these paths exist to avoid (FN-6796's benign in-review abort, the manual-merge-hold abort, the two
|
||||
stale-replay handlers, this retryable merge abort). The literal made the recovery inert, silently.
|
||||
*/
|
||||
if (live.column !== (await this.resolveResumeLanes(live.id, resumeLanesMemo)).review
|
||||
|| !this.isRetryableMergePauseAbortStatus(live.status) || live.error != null) return false;
|
||||
if (live.mergeDetails?.mergeConfirmed === true) return false;
|
||||
const failureValue = this.graphFailureValue(result);
|
||||
if (this.isTerminalMergeGraphFailureValue(failureValue)) return false;
|
||||
@@ -10213,12 +10285,33 @@ export class TaskExecutor {
|
||||
return true;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-20:30 (PR #2703 review — greptile P1, and it is the most
|
||||
important finding in this sweep):
|
||||
|
||||
THE SYNCHRONOUS RESOLVER IS A NO-OP IN PRODUCTION. `resolvePlannerLanes` reads
|
||||
`store.resolveTaskWorkflowIrSync`, whose selection reader is `getTaskWorkflowSelectionImpl` — and in
|
||||
PostgreSQL mode that function returns `undefined` unconditionally ("Backend mode cannot synchronously
|
||||
read PostgreSQL"). PostgreSQL is the shipped backend, so every sync-resolved conversion resolves the
|
||||
DEFAULT workflow and answers with the legacy ids no matter what board the task is on.
|
||||
|
||||
That makes a sync conversion cosmetic: the census counts it as converted, `--strict` goes down by one,
|
||||
and the guard behaves exactly as the literal did. Worse than leaving the literal, because the number
|
||||
says the site is done.
|
||||
|
||||
THE FIX IS TO STOP BEING SYNCHRONOUS, not to keep the literal. Both of this file's sync classifiers
|
||||
are called from async methods that have already awaited a store read, so the lane can be threaded in
|
||||
from the caller's existing snapshot — no new I/O, no second resolution, and the two halves of the
|
||||
decision provably read the same board.
|
||||
*/
|
||||
private isBenignInReviewPauseAbort(
|
||||
live: TaskDetail,
|
||||
result: WorkflowGraphTaskRunResult,
|
||||
abortProvenance: PausedAbortProvenance | undefined,
|
||||
pausedAborted: boolean,
|
||||
userCanceled: boolean,
|
||||
/** The caller's already-resolved review lane — see the note above on the sync resolver. */
|
||||
reviewLane: string,
|
||||
): boolean {
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-06-20-00:00:
|
||||
@@ -10230,7 +10323,15 @@ export class TaskExecutor {
|
||||
if (!pausedAborted) return false;
|
||||
if (!isGenericAbortProvenance(abortProvenance)) return false;
|
||||
if (userCanceled) return false;
|
||||
if (live.column !== "in-review") return false;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-20:40 (PR #2703 review — replaces my own earlier reasoning):
|
||||
This comparison used the SYNC `resolvePlannerLanes`, which I justified as the right resolver for a
|
||||
synchronous classifier. That justification was wrong in production: in PostgreSQL mode the sync
|
||||
selection reader always returns undefined, so the sync resolver hands back the DEFAULT workflow's lanes
|
||||
and the guard behaves exactly as the literal did. The lane now arrives from the caller's snapshot — see
|
||||
the note on this method.
|
||||
*/
|
||||
if (live.column !== reviewLane) return false;
|
||||
if (live.userPaused === true) return false;
|
||||
if (live.status != null || live.error != null) return false;
|
||||
if (live.mergeDetails?.mergeConfirmed === true) return false;
|
||||
@@ -10255,6 +10356,9 @@ export class TaskExecutor {
|
||||
result: WorkflowGraphTaskRunResult,
|
||||
abortProvenance: PausedAbortProvenance | undefined,
|
||||
pausedAborted: boolean,
|
||||
/** Shared per-recovery lane snapshot — see `resolveResumeLanes`; a fresh resolution here could disagree
|
||||
* with the one the rest of `handleGraphFailure` uses. */
|
||||
resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } },
|
||||
): Promise<boolean> {
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-07-09-14:54:
|
||||
@@ -10263,7 +10367,7 @@ export class TaskExecutor {
|
||||
if (!pausedAborted) return false;
|
||||
if (!isGenericAbortProvenance(abortProvenance)) return false;
|
||||
if (live.paused || live.userPaused === true) return false;
|
||||
if (live.column !== "in-review") return false;
|
||||
if (live.column !== (await this.resolveResumeLanes(live.id, resumeLanesMemo)).review) return false;
|
||||
if (live.mergeDetails?.mergeConfirmed === true) return false;
|
||||
if (this.isTerminalMergeGraphFailureValue(this.graphFailureValue(result))) return false;
|
||||
const failedNode = result.visitedNodeIds[result.visitedNodeIds.length - 1];
|
||||
@@ -10288,6 +10392,9 @@ export class TaskExecutor {
|
||||
abortProvenance: PausedAbortProvenance | undefined,
|
||||
pausedAborted: boolean,
|
||||
userCanceled: boolean,
|
||||
/** Shared per-recovery lane snapshot — see `resolveResumeLanes`; a fresh resolution here could disagree
|
||||
* with the one the rest of `handleGraphFailure` uses. */
|
||||
resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } },
|
||||
): Promise<boolean> {
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-06-28-21:05:
|
||||
@@ -10296,7 +10403,7 @@ export class TaskExecutor {
|
||||
if (!pausedAborted) return false;
|
||||
if (!isGenericAbortProvenance(abortProvenance) && abortProvenance !== "global-pause") return false;
|
||||
if (userCanceled) return false;
|
||||
if (live.column !== "in-review") return false;
|
||||
if (live.column !== (await this.resolveResumeLanes(live.id, resumeLanesMemo)).review) return false;
|
||||
if (live.paused || live.userPaused === true) return false;
|
||||
if (live.autoMerge === false) return false;
|
||||
if (live.mergeDetails?.mergeConfirmed === true) return false;
|
||||
@@ -10360,6 +10467,9 @@ export class TaskExecutor {
|
||||
abortProvenance: PausedAbortProvenance | undefined,
|
||||
pausedAborted: boolean,
|
||||
userCanceled: boolean,
|
||||
/** Shared per-recovery lane snapshot — see `resolveResumeLanes`; a fresh resolution here could disagree
|
||||
* with the one the rest of `handleGraphFailure` uses. */
|
||||
resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } },
|
||||
): Promise<boolean> {
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-06-29-01:18:
|
||||
@@ -10368,7 +10478,14 @@ export class TaskExecutor {
|
||||
if (!pausedAborted) return false;
|
||||
if (!isGenericAbortProvenance(abortProvenance) && abortProvenance !== "global-pause") return false;
|
||||
if (userCanceled) return false;
|
||||
if (live.column !== "in-review") return false;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-17:15 (fleet): ONE SNAPSHOT for the entry gate AND the deferred
|
||||
recheck inside `scheduleRetry` below — the recheck is the second half of THIS decision ("is the card
|
||||
still where it was when we admitted it?"), so resolving the board again inside the timeout callback
|
||||
would let a workflow edit make the two halves disagree.
|
||||
*/
|
||||
const replayLanes = await this.resolveResumeLanes(live.id, resumeLanesMemo);
|
||||
if (live.column !== replayLanes.review) return false;
|
||||
if (live.paused || live.userPaused === true) return false;
|
||||
if (live.autoMerge === false) return false;
|
||||
if (live.mergeDetails?.mergeConfirmed === true) return false;
|
||||
@@ -10435,7 +10552,7 @@ export class TaskExecutor {
|
||||
|| resumeTask.userPaused
|
||||
|| resumeTask.status != null
|
||||
|| resumeTask.error != null
|
||||
|| resumeTask.column !== "in-review"
|
||||
|| resumeTask.column !== replayLanes.review
|
||||
|| this.activeSessions.has(live.id)
|
||||
|| this.activeStepExecutors.has(live.id)
|
||||
|| this.activeWorkflowStepSessions.has(live.id)
|
||||
@@ -10480,15 +10597,25 @@ export class TaskExecutor {
|
||||
if (userCanceled) return false;
|
||||
if (live.paused || live.userPaused === true) return false;
|
||||
if (live.status != null || live.error != null) return false;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-17:20 (fleet: executor.ts — the split-snapshot defect):
|
||||
THE LANES ARE RESOLVED HERE, AT THE TOP, because this method already resolved them — at the very END,
|
||||
for its return value — while every eligibility check below compared against the default lineage's
|
||||
literals. On a renamed board the four `in-review` gates all read false, so a card in review skipped the
|
||||
global-pause recheck, the `autoMerge === false` refusal, the shared-branch-member arbitration and the
|
||||
merge-confirmed refusal — and then the final line, which DOES resolve lanes, answered "re-entrant".
|
||||
FN-7214's comment above says an auto-merge-off review row must stay terminal.
|
||||
*/
|
||||
const resumeLanes = await this.resolveResumeLanes(live.id, resumeLanesMemo);
|
||||
if ((await resolveTerminalColumnsFor(this.store, live.id)).includes(live.column)) return false;
|
||||
if (result.interruptedAbortKind !== WORKFLOW_NODE_ENGINE_PAUSE_ABORT_KIND) return false;
|
||||
if (!result.interruptedNodeId) return false;
|
||||
if (live.column === "in-review" && result.interruptedNodeId === "plan") return false;
|
||||
if (live.column === resumeLanes.review && result.interruptedNodeId === "plan") return false;
|
||||
if (this.isMergeGraphFailure(result.interruptedNodeId)) return false;
|
||||
if (this.isTerminalMergeGraphFailureValue(this.graphFailureValue(result))) return false;
|
||||
if ((live.graphResumeRetryCount ?? 0) >= MAX_TRANSIENT_GRAPH_RESUME_RETRIES) return false;
|
||||
let settings: Settings | undefined;
|
||||
if (abortProvenance === "global-pause" || live.column === "in-review") {
|
||||
if (abortProvenance === "global-pause" || live.column === resumeLanes.review) {
|
||||
try {
|
||||
settings = await this.store.getSettings();
|
||||
} catch {
|
||||
@@ -10496,14 +10623,13 @@ export class TaskExecutor {
|
||||
}
|
||||
if (settings.globalPause === true) return false;
|
||||
}
|
||||
if (live.column === "in-review") {
|
||||
if (live.column === resumeLanes.review) {
|
||||
if (live.autoMerge === false) return false;
|
||||
if (!settings) return false;
|
||||
const sharedBranchMember = await this.isLiveSharedBranchGroupMember(live);
|
||||
if (!sharedBranchMember && !allowsAutoMergeProcessing(live, settings)) return false;
|
||||
if (live.mergeDetails?.mergeConfirmed === true) return false;
|
||||
}
|
||||
const resumeLanes = await this.resolveResumeLanes(live.id, resumeLanesMemo);
|
||||
return live.column === resumeLanes.hold
|
||||
|| live.column === resumeLanes.review
|
||||
|| live.column === resumeLanes.wip;
|
||||
@@ -10760,6 +10886,15 @@ export class TaskExecutor {
|
||||
}
|
||||
const live = loadedLive;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-18:40 (fleet: executor.ts handleGraphFailure):
|
||||
ONE LANE SNAPSHOT FOR THE WHOLE METHOD, declared where `live` first exists. The three wip comparisons
|
||||
below run BEFORE the re-entry classifiers' memo was created, so a snapshot declared beside that memo
|
||||
is used-before-declared — which is how the two halves came to read different boards in the first
|
||||
place. The memo is seeded from this snapshot so the classifiers still share it.
|
||||
*/
|
||||
const resumeLanesMemo: { lanes?: { hold: string; wip: string; review: string } } = {};
|
||||
const failureLanes = await this.resolveResumeLanes(live.id, resumeLanesMemo);
|
||||
/*
|
||||
FNXC:Lifecycle 2026-07-16-21:22:
|
||||
FN-8141 follow-up 1 — an honest `fn_task_done(outcome="blocked")` park (status="failed",
|
||||
error "BLOCKED: <reason>", executor ~14657) must SURVIVE the same graph-teardown machinery
|
||||
@@ -10831,7 +10966,7 @@ export class TaskExecutor {
|
||||
would otherwise retry the same stale worktree in place, and the terminal sink would park
|
||||
the task failed with the signature erased (FN-7996 looped dispatch→park all day).
|
||||
*/
|
||||
if (await this.routeUnusableWorktreeGraphFailureToRecovery(task, live, result)) {
|
||||
if (await this.routeUnusableWorktreeGraphFailureToRecovery(task, live, result, resumeLanesMemo)) {
|
||||
await this.persistTokenUsage(task.id);
|
||||
return;
|
||||
}
|
||||
@@ -10852,7 +10987,7 @@ export class TaskExecutor {
|
||||
void (async () => {
|
||||
try {
|
||||
const resumeTask = await this.store.getTask(task.id);
|
||||
if (this.isRequiredArtifactRecoveryProtected(resumeTask) || resumeTask.status === "failed") return;
|
||||
if (await this.isRequiredArtifactRecoveryProtected(resumeTask) || resumeTask.status === "failed") return;
|
||||
await this.execute(resumeTask);
|
||||
} catch (err) {
|
||||
executorLog.error(`Failed required-artifact read retry for ${task.id}:`, err);
|
||||
@@ -10927,8 +11062,11 @@ export class TaskExecutor {
|
||||
FNXC:WorkflowLifecycle 2026-06-18-12:00:
|
||||
FN-6647 closes the remaining durability gap by deriving already-finalized completion from the persisted task row: non-in-progress column, completed steps, no live pause/status/error, and the finalize-to-review log entry. The volatile `completionFinalizedTaskIds` marker still helps within one executor lifecycle, but teardown/restart loss must not reclassify a completed in-review row as a hard-cancel pause abort.
|
||||
*/
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:32 (fleet): on a renamed board a completed,
|
||||
already-finalized row read as still-in-wip, so FN-6644/FN-6647's suppression never fired and the
|
||||
row was re-parked as an operator-action pause abort — the durability gap those tickets closed. */
|
||||
const alreadyFinalizedToReview = Boolean(
|
||||
live.column !== "in-progress"
|
||||
live.column !== failureLanes.wip
|
||||
&& persistedCompletedProgress
|
||||
&& live.status == null
|
||||
&& live.error == null
|
||||
@@ -10952,7 +11090,7 @@ export class TaskExecutor {
|
||||
const completionFinalized = completionFinalizeAborted || this.completionFinalizedTaskIds.has(task.id) || alreadyFinalizedToReview;
|
||||
const suppressFinalizedCompletionAbort = Boolean(
|
||||
completionFinalized
|
||||
&& live.column !== "in-progress"
|
||||
&& live.column !== failureLanes.wip
|
||||
&& !live.userPaused
|
||||
// FN-6648: `paused !== true` intentionally dropped here too — the
|
||||
// suppression is already gated on `completionFinalized` (completed
|
||||
@@ -10983,8 +11121,12 @@ export class TaskExecutor {
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-01:05 (PR #2640 review, greptile P2): one lane
|
||||
snapshot for one recovery decision — see `resolveResumeLanes`. Eligibility and re-entry are two
|
||||
halves of the SAME decision and must not read different boards.
|
||||
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-17:30 (fleet): the surrounding branches share it now too.
|
||||
This method asked "still in the wip lane?" in three more places as the default lineage's id while
|
||||
creating this memo for the classifiers — so the classifiers read the board and the branches around
|
||||
them read the default names.
|
||||
*/
|
||||
const resumeLanesMemo: { lanes?: { hold: string; wip: string; review: string } } = {};
|
||||
if (genuinePauseAbort && await this.isReentrantPausedAbortedInFlightNode(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id), resumeLanesMemo)) {
|
||||
if (await this.reenterPausedAbortedWorkflowNode(live, result, abortProvenance, resumeLanesMemo)) {
|
||||
return;
|
||||
@@ -11008,12 +11150,12 @@ export class TaskExecutor {
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (genuinePauseAbort && await this.isRetryableBenignMergePauseAbort(live, result, abortProvenance, pausedAborted)) {
|
||||
if (genuinePauseAbort && await this.isRetryableBenignMergePauseAbort(live, result, abortProvenance, pausedAborted, resumeLanesMemo)) {
|
||||
if (await this.routeGraphMergeFailureToRetry(live, result, abortProvenance)) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (genuinePauseAbort && await this.isBenignManualMergeHoldPauseAbort(live, result, abortProvenance, pausedAborted)) {
|
||||
if (genuinePauseAbort && await this.isBenignManualMergeHoldPauseAbort(live, result, abortProvenance, pausedAborted, resumeLanesMemo)) {
|
||||
/*
|
||||
FNXC:WorkflowLifecycle 2026-07-09-14:56:
|
||||
FN-7749 / Runfusion#1979: auto-merge-off manual merge hold is terminal-until-human-merged, not an executor failure. Preserve the `in-review` row for Merge & Close, do not invoke merge retry, and clear only stale pause-abort status/error so FN-5147's no-backward-move/no-reenqueue contract stays intact.
|
||||
@@ -11030,7 +11172,7 @@ export class TaskExecutor {
|
||||
await this.persistTokenUsage(task.id);
|
||||
return;
|
||||
}
|
||||
if (genuinePauseAbort && this.isBenignInReviewPauseAbort(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id))) {
|
||||
if (genuinePauseAbort && this.isBenignInReviewPauseAbort(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id), failureLanes.review)) {
|
||||
this.clearPausedAborted(task.id);
|
||||
this.activeWorktrees.delete(task.id);
|
||||
const inReviewBenign = "Workflow graph run ended during engine pause/resume while already in-review — benign, in-review state preserved";
|
||||
@@ -11039,10 +11181,10 @@ export class TaskExecutor {
|
||||
await this.persistTokenUsage(task.id);
|
||||
return;
|
||||
}
|
||||
if (genuinePauseAbort && await this.handleStaleInReviewParsePauseAbortReplay(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id))) {
|
||||
if (genuinePauseAbort && await this.handleStaleInReviewParsePauseAbortReplay(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id), resumeLanesMemo)) {
|
||||
return;
|
||||
}
|
||||
if (genuinePauseAbort && await this.handleStaleInReviewPlanPauseAbortReplay(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id))) {
|
||||
if (genuinePauseAbort && await this.handleStaleInReviewPlanPauseAbortReplay(live, result, abortProvenance, pausedAborted, this.userCanceledTaskIds.has(task.id), resumeLanesMemo)) {
|
||||
return;
|
||||
}
|
||||
if (genuinePauseAbort) {
|
||||
@@ -11078,7 +11220,7 @@ export class TaskExecutor {
|
||||
// the human-readable provenance label is ever revised.
|
||||
const isEngineInternalAbort =
|
||||
pausedAborted && !live.paused && !live.userPaused && abortProvenance !== "global-pause";
|
||||
if (live.column !== "in-progress") {
|
||||
if (live.column !== failureLanes.wip) {
|
||||
// FN-6782: a pause/resume abort that has left the task back in `todo`
|
||||
// is benign — the work is simply re-queued for a fresh dispatch, not
|
||||
// stranded. Parking it `status: "failed"` (operator action required)
|
||||
@@ -11438,7 +11580,7 @@ export class TaskExecutor {
|
||||
if (await this.routeRetryableRemediationGraphFailureToPreMergeFix(live, failedNode, failureValue)) {
|
||||
return;
|
||||
}
|
||||
if (await this.routeGraphFailureToExecutionResume(live, failedNode ?? "unknown", failureValue)) {
|
||||
if (await this.routeGraphFailureToExecutionResume(live, failedNode ?? "unknown", failureValue, resumeLanesMemo)) {
|
||||
return;
|
||||
}
|
||||
/*
|
||||
@@ -11683,6 +11825,8 @@ export class TaskExecutor {
|
||||
live: TaskDetail,
|
||||
failedNode: string,
|
||||
failureValue: string | undefined,
|
||||
/** Shared per-recovery lane snapshot — see `resolveResumeLanes`. */
|
||||
resumeLanesMemo?: { lanes?: { hold: string; wip: string; review: string } },
|
||||
): Promise<boolean> {
|
||||
/*
|
||||
* FNXC:WorkflowLifecycle 2026-06-29-11:08:
|
||||
@@ -11712,7 +11856,19 @@ export class TaskExecutor {
|
||||
const implementationIncompleteMergeFailure = this.isMergeGraphFailure(failedNode) && failureValue === "implementation-incomplete";
|
||||
if (implementationIncompleteMergeFailure && !incompleteSteps) return false;
|
||||
const prematureMergeWithIncompleteSteps = implementationIncompleteMergeFailure && incompleteSteps;
|
||||
if (live.column !== "in-review" && !(incompleteSteps && live.column === "todo") && !(prematureMergeWithIncompleteSteps && live.column === "in-progress")) return false;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-17:40 (fleet: executor.ts — the REVERSE half-conversion):
|
||||
THE DESTINATION WAS ALREADY RESOLVED HERE AND THE GATE WAS NOT. `resolveReboundColumnFor` below picks
|
||||
the board's rebound column (U7), but this gate compared against three default-lineage literals — so on
|
||||
a renamed board the router refused before ever reaching the resolved move. That is the mirror image of
|
||||
the dangerous half-conversion: instead of admitting a card and sending it nowhere, it refuses a card
|
||||
whose recovery was fully implemented, and nothing is logged as wrong. Same one-decision-two-boards
|
||||
defect, opposite direction, and the silent one.
|
||||
*/
|
||||
const resumeRouterLanes = await this.resolveResumeLanes(live.id, resumeLanesMemo);
|
||||
if (live.column !== resumeRouterLanes.review
|
||||
&& !(incompleteSteps && live.column === resumeRouterLanes.hold)
|
||||
&& !(prematureMergeWithIncompleteSteps && live.column === resumeRouterLanes.wip)) return false;
|
||||
|
||||
const message = incompleteSteps
|
||||
? `Workflow graph failed at node '${failedNode}'${failureValue ? ` (${failureValue})` : ""} with incomplete steps — moved back to todo for execution resume`
|
||||
@@ -12272,7 +12428,15 @@ export class TaskExecutor {
|
||||
// executor can still recover by falling through to the fresh-worktree
|
||||
// path below, but we emit a loud audit record so these states stop being
|
||||
// silent.
|
||||
if (task.column === "in-progress" && task.mergeDetails?.mergeConfirmed === true) {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-18:10 (fleet: execute() preflight): THREE DRIFT CHECKS, ONE
|
||||
SNAPSHOT — merge-confirmed while still executing, stale mergeDetails, and in-wip with no worktree. None
|
||||
fired on a renamed board, so every recovery they perform silently stopped happening. The third one's own
|
||||
message says it "usually indicates a partial updateTask/moveTask sequence failed" — a diagnostic that
|
||||
could never print on a renamed board.
|
||||
*/
|
||||
const preflightWipLane = (await this.resolveResumeLanes(task.id)).wip;
|
||||
if (task.column === preflightWipLane && task.mergeDetails?.mergeConfirmed === true) {
|
||||
if (await this.finalizeMergeConfirmedWorkflowGraphTask(task.id, "execute-preflight")) {
|
||||
this.executing.delete(task.id);
|
||||
executingTaskLock.release(task.id);
|
||||
@@ -12281,7 +12445,7 @@ export class TaskExecutor {
|
||||
}
|
||||
}
|
||||
|
||||
if (task.column === "in-progress" && task.mergeDetails) {
|
||||
if (task.column === preflightWipLane && task.mergeDetails) {
|
||||
executorLog.warn(`${task.id}: stale mergeDetails found while executing in-progress task — resetting merge state before continuing`);
|
||||
task = await this.cleanupMergeStateForReverification(
|
||||
task,
|
||||
@@ -12289,7 +12453,7 @@ export class TaskExecutor {
|
||||
);
|
||||
}
|
||||
|
||||
if (task.column === "in-progress" && !task.worktree) {
|
||||
if (task.column === preflightWipLane && !task.worktree) {
|
||||
executorLog.error(
|
||||
`${task.id}: drift detected — task is in-progress with no worktree. ` +
|
||||
`Recovering by creating a fresh worktree. This usually indicates a partial ` +
|
||||
@@ -13231,8 +13395,13 @@ export class TaskExecutor {
|
||||
// was unwinding; continuing the cleanup would clobber a valid
|
||||
// recovery (see the analogous block in the outer finally for the
|
||||
// full reasoning).
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-18:05 (fleet: stuck-requeue family): "has a
|
||||
concurrent recovery already moved this card on?" — the pre-completion lanes are the board's
|
||||
wip and hold. With literals a renamed board always answered "moved on", the cleanup never
|
||||
ran, and the log line blamed a concurrent recovery that had not happened. */
|
||||
const latestTask = await this.store.getTask(task.id);
|
||||
if (latestTask.column !== "in-progress" && latestTask.column !== "todo") {
|
||||
const requeueLanes = await this.resolveResumeLanes(task.id);
|
||||
if (latestTask.column !== requeueLanes.wip && latestTask.column !== requeueLanes.hold) {
|
||||
executorLog.log(
|
||||
`${task.id} stuck-requeue skipped — task is now in '${latestTask.column}' (recovered concurrently)`,
|
||||
);
|
||||
@@ -14038,7 +14207,9 @@ export class TaskExecutor {
|
||||
}
|
||||
const hasExplicitWorktreeBinding = typeof liveTask.worktree === "string" || liveTask.worktree === null;
|
||||
const hasExplicitBranchBinding = typeof liveTask.branch === "string" || liveTask.branch === null;
|
||||
const worktreeContractIntact = liveTask.column === "in-progress"
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-18:15 (fleet): the contract holds while the card is
|
||||
in ITS board's wip lane; the literal made every renamed-board retry look reclaimed. */
|
||||
const worktreeContractIntact = liveTask.column === (await this.resolveResumeLanes(task.id)).wip
|
||||
&& !liveTask.paused
|
||||
&& (!hasExplicitWorktreeBinding || liveTask.worktree === worktreePath)
|
||||
&& (!hasExplicitBranchBinding || (typeof liveTask.branch === "string" && liveTask.branch.length > 0));
|
||||
@@ -14505,7 +14676,10 @@ export class TaskExecutor {
|
||||
this.clearPausedAborted(task.id);
|
||||
const latestTask = await this.store.getTask(task.id);
|
||||
if (
|
||||
latestTask?.column === "todo" &&
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-18:18 (fleet): the HOLD lane — this recognises a card the
|
||||
abort already parked with its progress preserved, and skipping the cleanup is what keeps that
|
||||
progress. On a renamed board the cleanup ran anyway and discarded it. */
|
||||
latestTask?.column === (await this.resolveResumeLanes(task.id)).hold &&
|
||||
latestTask.paused === true &&
|
||||
((latestTask.currentStep ?? 0) > 0 || latestTask.steps?.some((step) => step.status === "done" || step.status === "in-progress"))
|
||||
) {
|
||||
@@ -15192,7 +15366,8 @@ export class TaskExecutor {
|
||||
let cleanupLockHeld = true;
|
||||
try {
|
||||
const latestTask = await this.store.getTask(task.id);
|
||||
if (latestTask.column === "in-progress" || latestTask.column === "todo") {
|
||||
const continuationLanes = await this.resolveResumeLanes(task.id);
|
||||
if (latestTask.column === continuationLanes.wip || latestTask.column === continuationLanes.hold) {
|
||||
await this.store.updateTask(task.id, {
|
||||
sessionFile: null,
|
||||
status: null,
|
||||
@@ -15244,7 +15419,8 @@ export class TaskExecutor {
|
||||
// all step progress reset, undoing valid completion. Skip the
|
||||
// entire cleanup if the column has moved on past in-progress/todo.
|
||||
const latestTask = await this.store.getTask(task.id);
|
||||
if (latestTask.column !== "in-progress" && latestTask.column !== "todo") {
|
||||
const outerRequeueLanes = await this.resolveResumeLanes(task.id);
|
||||
if (latestTask.column !== outerRequeueLanes.wip && latestTask.column !== outerRequeueLanes.hold) {
|
||||
executorLog.log(
|
||||
`${task.id} stuck-requeue skipped — task is now in '${latestTask.column}' (recovered concurrently)`,
|
||||
);
|
||||
@@ -20823,7 +20999,9 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
|
||||
`${taskId} force-requeue could not read latest task state: ${err instanceof Error ? err.message : String(err)}`,
|
||||
);
|
||||
}
|
||||
if (latestColumn && latestColumn !== "in-progress") {
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-08-01-17:52 (fleet): the board's wip lane; with the literal a
|
||||
renamed board skipped every force-requeue as "recovered concurrently". */
|
||||
if (latestColumn && latestColumn !== (await this.resolveResumeLanes(taskId)).wip) {
|
||||
executorLog.log(
|
||||
`${taskId} force-requeue skipped — task is now in '${latestColumn}' (recovered concurrently)`,
|
||||
);
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
"generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline",
|
||||
"byFile": {
|
||||
"packages/engine/src/self-healing.ts": 110,
|
||||
"packages/engine/src/executor.ts": 57,
|
||||
"packages/engine/src/executor.ts": 15,
|
||||
"packages/core/src/store.ts": 12,
|
||||
"packages/engine/src/scheduler.ts": 12,
|
||||
"packages/core/src/task-store/async-comments-attachments.ts": 9,
|
||||
|
||||
Reference in New Issue
Block a user