engine tail: nine files to zero — incl. a census-INVISIBLE requeue into a lane that does not exist (−13) (#2797)
Follow-up to #2785. Nine engine files to **zero**, all one question — *"is this task finished?"* — asked in nine places, wrong in every one on a renamed board. ## Census, per file (measured, `--strict` verified) | file | main | here | | --- | ---: | ---: | | `agent-reflection.ts` | 2 | **0** | | `merger-scope-auto-widen.ts` | 2 | **0** | | `worktree-pool.ts` | 2 | **0** | | `cli-agent/state-machine.ts` | 2 | **0** (reclassified — see below) | | `auto-recovery-handlers/branch-worktree.ts` | 1 | **0** | | `cli-agent/task-session.ts` | 1 | **0** (reclassified) | | `merger-integration-worktree.ts` | 1 | **0** | | `merger-orphan-rehome.ts` | 1 | **0** | | `plugin-runner.ts` | 1 | **0** | | **net** | | **−13** | ## The one worth reading: `branch-worktree` had TWO defects, and the census could only see one ```ts if (task.column === "in-progress") { …clear branch… } // counted await this.deps.taskStore.moveTask(task.id, "todo", { … }); // INVISIBLE ``` The census scores **comparisons**. The requeue *destination* is a call argument, so nothing in the backlog ever pointed at it — and it is the worse of the two: a board with no `todo` column was requeued into a lane **that does not exist**. The counted literal is the smaller half (a renamed wip lane meant the stale branch was never cleared, so the card carried a dead branch back into execution). Converting the comparison alone would have dropped a census count and left the board requeuing into nowhere. Destination now resolves through `resolveReboundTarget` (KTD-10 ordering: hold → intake → first column). Reverted **independently**: destination restored → 2 fail (`moveTask` called with `"todo"`, not `"backlog"`); wip test restored → 1 fail (`updateTask` never called). ## The rest - **`plugin-runner`** — `onTaskCompleted` never fired on a renamed board. Every plugin that closes an issue, posts a notification, or records a metric on completion **silently stopped**, with nothing logged. Resolved *asynchronously* inside the existing fire-and-forget seam, not via `resolveTaskWorkflowIrSync` — per `sync-workflow-ir-callsite-allowlist` that reader returns the DEFAULT workflow for every task in production, so a sync guard here would read as converted and still be wrong. The listener is already `void`-dispatched, so awaiting inside it changes no observable ordering (the shape `NotificationService` already uses). - **`merger-orphan-rehome`** — a renamed complete lane made every source task read as unfinished, so orphaned commits were never rehomed and stayed stranded off the integration branch. Resolves by the **trailer id**, not `sourceTask.id`, which the fake store does not populate. - **`agent-reflection`** — `classifyOutcome` returned `null` for every finished task, so both callers treated completed work as nothing to reflect on: one recorded `reflection:skipped` with reason `"not-completed"`, the other silently `continue`d. Reflection captured **nothing at all** on a custom board. - **`worktree-pool`** — shipped tasks' worktrees stayed in the ACTIVE set, so the reclaim pass never returned them and the board walks into worktree exhaustion — a stall whose cause is invisible from the symptom. - **`merger-scope-auto-widen`** — finished cards counted as active claimants, so a merge was blocked by a task that no longer exists in any meaningful sense. - **`merger-integration-worktree`** — a shipped task still counted as a live worktree user, so the integration worktree could never be reused and the merge path took the slower rebuild every time. ## The census OVERSTATED the engine backlog by 3 `cli-agent/state-machine.ts` and `cli-agent/task-session.ts` compare against `done` — but that is a **`CliMachineState`** (`ready`/`busy`/`waitingOnInput`/`done`/`resuming`/`idle`) tracking one CLI agent process. It never reads a board column. The census matches the bare string. Marked `DELIBERATE-LITERAL` rather than left for a later sweep to "convert" a process state into a workflow role. Worth flagging fleet-wide: the backlog total includes at least these three non-columns. ## Revert results (measured, each run) | conversion | reverted → | | --- | --- | | `plugin-runner` complete gate | RENAMED case fails — `onTaskCompleted` never invoked | | `merger-orphan-rehome` source gate | RENAMED case fails — `orphan:false, reason:"source-task-not-done"` | | `branch-worktree` destination | 2 fail — `moveTask` called with `"todo"` | | `branch-worktree` wip test | 1 fail — `updateTask` never called | Each has a **non-vacuous companion** (renamed board, non-complete lane / mid-flight source / non-wip column) so a guard that fired unconditionally would not pass. **Four are NOT revert-proven, and I am not claiming otherwise:** `agent-reflection`, `merger-scope-auto-widen`, `merger-integration-worktree`, `worktree-pool`. Their suites omit a workflow and therefore assert the legacy fallback — they pass before and after. `merger-scope-auto-widen` has no test file at all; `scanIdleWorktrees` is mocked in every suite that touches it and driving it for real needs git worktrees on disk. All four strictly **widen** the finished set (resolved roles ∪ the legacy ids), so default boards are byte-identical. That is the argument for shipping them, not a substitute for coverage. ## Examined and deliberately NOT converted - **`backlog-pressure-reporter:173`** — fed by `listTasks({ column: "todo" })`, a hardcoded **query** filter. On a renamed board `todoFull` is empty and the predicate never runs. Converting it drops a census count and changes nothing observable; the fix belongs at the query layer. - **`auto-merge-finalization:28`** — the catch-arm legacy fallback, which must stay for the same reason `columnRoles.ts` keeps its id fallback. - **`auto-merge-finalization:84`** — only selects between two diagnostic reason strings that are **both** `ok: false`, on a pure validator with no store in scope. Converting it would thread a store through a pure function to change a label. ## Merge resolution note Merging main brought conflicts in `agent-assignment.ts` and `ephemeral-worker-manager.ts`. **Main's versions won both** and mine are dropped: main threads an optional `activeColumns` from `scheduler.ts:2340` (a cleaner seam than widening the store type to resolve internally), and its `isAgentIdle` carries a greptile P1 fix mine lacked — `columnsWithFlag` membership rather than first-per-role, so a workflow declaring two wip lanes has both recognised. That is the fourth time in this program main's version of a contested file was the better one. ## Verification - `pnpm test:gate` — 161 + 487 + 13 + 71, green - `npx tsc -p packages/engine/tsconfig.json --noEmit` — clean - `pnpm lint` — clean - `--strict` exits 0 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Workflow-dependent task completion now recognizes custom lifecycle columns, including renamed boards. * Recovery requeues tasks to the configured destination and clears branch details only from the appropriate work-in-progress column. * Improved handling of completed tasks, orphaned work, shared worktrees, and scope evaluation across custom workflows. * Plugin completion hooks now trigger for any column configured as complete. * **Tests** * Added coverage for renamed workflow columns and custom completion, recovery, and rehoming behavior. * **Documentation** * Clarified CLI state terminology in internal developer comments. <!-- 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:
@@ -1,6 +1,8 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { AutoRecoveryContext, AutoRecoveryDecision, AutoRecoveryFailure } from "../auto-recovery.js";
|
||||
import { BranchWorktreeAutoRecoveryHandler } from "../auto-recovery-handlers/branch-worktree.js";
|
||||
import { RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js";
|
||||
import { TransitionRejectionError } from "@fusion/core";
|
||||
|
||||
const branchConflictMocks = vi.hoisted(() => ({
|
||||
inspectBranchConflict: vi.fn(),
|
||||
@@ -27,11 +29,24 @@ function createTask(overrides: Record<string, unknown> = {}) {
|
||||
} as any;
|
||||
}
|
||||
|
||||
function createFixtures(taskOverrides: Record<string, unknown> = {}, mode = "programmatic") {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:50 (batch-engine tail):
|
||||
`ir` is optional so every existing case is byte-identical: with no workflow the resolver degrades to the
|
||||
built-in coding IR, whose wip lane is `in-progress` and whose rebound target is `todo` — exactly what
|
||||
those cases already assert.
|
||||
*/
|
||||
function createFixtures(taskOverrides: Record<string, unknown> = {}, mode = "programmatic", ir?: unknown) {
|
||||
const task = createTask(taskOverrides);
|
||||
const taskStore = {
|
||||
updateTask: vi.fn(async () => undefined),
|
||||
moveTask: vi.fn(async () => undefined),
|
||||
...(ir
|
||||
? {
|
||||
getTaskWorkflowSelectionAsync: async () => ({ workflowId: "recovery-lifecycle", stepIds: [] }),
|
||||
getTaskWorkflowSelection: () => ({ workflowId: "recovery-lifecycle", stepIds: [] }),
|
||||
getWorkflowDefinition: async (id: string) => (id === "recovery-lifecycle" ? { ir } : undefined),
|
||||
}
|
||||
: {}),
|
||||
} as any;
|
||||
const runAudit = { database: vi.fn(async () => undefined), git: vi.fn(), filesystem: vi.fn() } as any;
|
||||
const logger = { warn: vi.fn(), log: vi.fn(), error: vi.fn() } as any;
|
||||
@@ -57,6 +72,147 @@ describe("BranchWorktreeAutoRecoveryHandler", () => {
|
||||
expect(f.runAudit.database).toHaveBeenCalledWith(expect.objectContaining({ type: "branch-worktree:auto-requeue" }));
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:50 (batch-engine tail):
|
||||
TWO defects, one case. The requeue destination was the hardcoded `todo` — CENSUS-INVISIBLE, because
|
||||
the census scores comparisons and that is a call argument — so a board with no `todo` column was
|
||||
requeued into a lane that does not exist. And the WIP test was the id `in-progress`, so the stale
|
||||
branch/baseCommitSha were never cleared and the card carried a dead branch back into execution.
|
||||
|
||||
REVERT CHECK, measured (both, independently):
|
||||
- `moveTask(..., "todo", ...)` restored -> this fails; moveTask is called with "todo", not "backlog".
|
||||
- `task.column === "in-progress"` restored -> this fails; updateTask is never called.
|
||||
The legacy cases pass both ways, which is why they are kept alongside.
|
||||
*/
|
||||
it("requeues to the RESOLVED rebound target and clears the branch on a RENAMED board", async () => {
|
||||
const f = createFixtures(
|
||||
{ column: RENAMED_VOCAB.wip },
|
||||
"programmatic",
|
||||
lifecycleIr(RENAMED_VOCAB, "recovery-lifecycle"),
|
||||
);
|
||||
branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" });
|
||||
|
||||
await f.handler.issueRetry(f.failure, f.decision, f.ctx);
|
||||
|
||||
expect(f.taskStore.updateTask).toHaveBeenCalledWith("FN-4536", { branch: null, baseCommitSha: null });
|
||||
expect(f.taskStore.moveTask).toHaveBeenCalledWith(
|
||||
"FN-4536",
|
||||
RENAMED_VOCAB.hold,
|
||||
expect.objectContaining({ moveSource: "engine", preserveWorktree: false }),
|
||||
);
|
||||
expect(f.taskStore.moveTask).not.toHaveBeenCalledWith("FN-4536", "todo", expect.anything());
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:45 (#2797 review — greptile):
|
||||
|
||||
THE REQUEUE MUST NOT DIE ON A DESTINATION THE BOARD DOES NOT DECLARE.
|
||||
|
||||
The review pointed at the `catch` retaining `reboundTarget = "todo"`. Writing the test for that
|
||||
branch DISPROVED it as the main route: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather
|
||||
than throwing, so a task whose custom workflow cannot be read still resolves `todo` from the DEFAULT
|
||||
board and the catch never runs. A guard on the throw path alone would have passed review and fixed
|
||||
almost nothing.
|
||||
|
||||
What actually bites is the move. `moveTaskInternal` REJECTS a column the workflow does not declare,
|
||||
and unhandled that throws out of the recovery handler whose entire job is to unstick the task — so
|
||||
the recovery became a second way to stay stuck, with no audit row explaining it.
|
||||
|
||||
This drives the rejection itself, which is the behaviour every route ends at.
|
||||
*/
|
||||
it("records a skip instead of throwing when the rebound destination is rejected", async () => {
|
||||
const f = createFixtures({ column: "building" }, "programmatic");
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1):
|
||||
A REAL TransitionRejectionError, not a look-alike Error whose message happens to read like one.
|
||||
The reason is now derived from the typed `rejection.code`, so a plain Error must NOT be classified
|
||||
as a lane problem — which is the point of the companion case below.
|
||||
*/
|
||||
f.taskStore.moveTask.mockRejectedValue(
|
||||
new TransitionRejectionError(
|
||||
{ code: "unknown-column", messageKey: "transition.rejected.unknownColumn", retryable: false, detail: "Column 'todo' is not defined in this task's workflow" } as never,
|
||||
"Invalid transition: 'building' -> 'todo'. Unknown column for this workflow.",
|
||||
),
|
||||
);
|
||||
branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" });
|
||||
|
||||
/* The handler must not propagate — that is the regression. */
|
||||
await expect(f.handler.issueRetry(f.failure, f.decision, f.ctx)).resolves.not.toThrow();
|
||||
|
||||
expect(f.runAudit.database).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "branch-worktree:auto-requeue-skipped",
|
||||
metadata: expect.objectContaining({ reason: "rebound-target-rejected", rejectionCode: "unknown-column", reboundTarget: "todo" }),
|
||||
}));
|
||||
/* And the success audit must NOT be written for a move that did not happen. */
|
||||
expect(f.runAudit.database).not.toHaveBeenCalledWith(expect.objectContaining({ type: "branch-worktree:auto-requeue" }));
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1 "move failures become
|
||||
successful retries"):
|
||||
A moveTask failure that is NOT a lane problem — capacity exhaustion, a guard rejection, a deleted
|
||||
task, a persistence error — was labelled `rebound-target-rejected` all the same, so the audit row
|
||||
asserted a lane cause for something that has nothing to do with lanes and anyone debugging a stuck
|
||||
card would chase the wrong thing.
|
||||
|
||||
REVERT CHECK, measured: collapsing the reason back to the single literal makes this fail — the row
|
||||
reads `rebound-target-rejected` for a plain persistence error.
|
||||
*/
|
||||
it("names a non-lane move failure honestly instead of blaming the rebound target", async () => {
|
||||
const f = createFixtures({ column: "in-progress" }, "programmatic");
|
||||
f.taskStore.moveTask.mockRejectedValue(new Error("database connection lost"));
|
||||
branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" });
|
||||
|
||||
await expect(f.handler.issueRetry(f.failure, f.decision, f.ctx)).resolves.not.toThrow();
|
||||
|
||||
expect(f.runAudit.database).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "branch-worktree:auto-requeue-skipped",
|
||||
metadata: expect.objectContaining({ reason: "requeue-move-failed" }),
|
||||
}));
|
||||
expect(f.runAudit.database).not.toHaveBeenCalledWith(expect.objectContaining({
|
||||
metadata: expect.objectContaining({ reason: "rebound-target-rejected" }),
|
||||
}));
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1 "rejected move erases
|
||||
branch linkage"):
|
||||
The branch/baseCommitSha clear used to run BEFORE the move, so a rejected move left the card in its
|
||||
wip lane with the only pointers back to its work already erased — the recovery destroyed the linkage
|
||||
and then declined to requeue. Half-applied is worse than not applied; nothing reconstructs the branch
|
||||
from the row afterwards.
|
||||
|
||||
REVERT CHECK, measured: moving the clear back above the move makes this fail — updateTask is called
|
||||
with { branch: null, baseCommitSha: null } on a move that never landed.
|
||||
*/
|
||||
it("preserves the branch linkage when the requeue move is rejected", async () => {
|
||||
const f = createFixtures({ column: "in-progress" }, "programmatic");
|
||||
f.taskStore.moveTask.mockRejectedValue(new Error("database connection lost"));
|
||||
branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" });
|
||||
|
||||
await f.handler.issueRetry(f.failure, f.decision, f.ctx);
|
||||
|
||||
expect(f.taskStore.updateTask).not.toHaveBeenCalledWith("FN-4536", { branch: null, baseCommitSha: null });
|
||||
});
|
||||
|
||||
it("does not clear the branch when a RENAMED board's card is not in its wip lane", async () => {
|
||||
/*
|
||||
Non-vacuous companion: without it, a guard that cleared unconditionally would pass the case above.
|
||||
Same renamed board, same failure — only the card's lane changes.
|
||||
*/
|
||||
const f = createFixtures(
|
||||
{ column: RENAMED_VOCAB.review },
|
||||
"programmatic",
|
||||
lifecycleIr(RENAMED_VOCAB, "recovery-lifecycle"),
|
||||
);
|
||||
branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "fully-subsumed", livePath: "/tmp/wt", tipSha: "abc" });
|
||||
|
||||
await f.handler.issueRetry(f.failure, f.decision, f.ctx);
|
||||
|
||||
expect(f.taskStore.updateTask).not.toHaveBeenCalled();
|
||||
expect(f.taskStore.moveTask).toHaveBeenCalledWith("FN-4536", RENAMED_VOCAB.hold, expect.anything());
|
||||
});
|
||||
|
||||
it("reanchors bootstrap misbinding then requeues", async () => {
|
||||
const f = createFixtures();
|
||||
branchConflictMocks.inspectBranchConflict.mockResolvedValue({ kind: "reclaimable", livePath: "/tmp/wt", tipSha: "abc", taskAttributedCommitCount: 0, strandedCommits: [] });
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, it, expect, afterAll } from "vitest";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js";
|
||||
import { tmpdir } from "node:os";
|
||||
import { execSync } from "node:child_process";
|
||||
import {
|
||||
@@ -41,9 +42,21 @@ function setupRepo() {
|
||||
return dir;
|
||||
}
|
||||
|
||||
function makeFakeStore(tasks: Record<string, { column: string }>) {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:20 (batch-engine tail):
|
||||
`ir` is optional so every existing caller is byte-identical: with no workflow the resolver degrades to
|
||||
the built-in coding IR, whose complete lane IS `done`, which is what these cases already assert.
|
||||
*/
|
||||
function makeFakeStore(tasks: Record<string, { column: string }>, ir?: unknown) {
|
||||
return {
|
||||
getTask: async (id: string) => tasks[id.toUpperCase()] ?? null,
|
||||
...(ir
|
||||
? {
|
||||
getTaskWorkflowSelectionAsync: async () => ({ workflowId: "orphan-lifecycle", stepIds: [] }),
|
||||
getTaskWorkflowSelection: () => ({ workflowId: "orphan-lifecycle", stepIds: [] }),
|
||||
getWorkflowDefinition: async (id: string) => (id === "orphan-lifecycle" ? { ir } : undefined),
|
||||
}
|
||||
: {}),
|
||||
} as any;
|
||||
}
|
||||
|
||||
@@ -88,6 +101,79 @@ describe("classifyOrphanOurAdvance", () => {
|
||||
}
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:20 (batch-engine tail):
|
||||
The case above proves orphan classification for the LEGACY complete id, which is what the guard
|
||||
compared against — it passed before this conversion and would pass for a broken one. On a renamed
|
||||
complete lane the source read as unfinished, so orphaned commits were never rehomed and stayed
|
||||
stranded off the integration branch with no error surfaced.
|
||||
|
||||
REVERT CHECK, measured: with `sourceTask.column !== "done"` restored, this fails with
|
||||
`orphan: false, reason: "source-task-not-done"`.
|
||||
*/
|
||||
it("classifies the orphan when the source task sits in a RENAMED complete lane", async () => {
|
||||
const dir = setupRepo();
|
||||
try {
|
||||
git(dir, "git checkout -b sibling-orphan-renamed");
|
||||
writeFileSync(join(dir, "orphan-renamed.txt"), "orphan\n");
|
||||
git(dir, "git add orphan-renamed.txt");
|
||||
git(dir, `git commit -m "feat(FN-5551): orphaned squash" -m "Fusion-Task-Id: FN-5551"`);
|
||||
const orphanSha = git(dir, "git rev-parse HEAD");
|
||||
git(dir, "git checkout main");
|
||||
|
||||
const result = await classifyOrphanOurAdvance({
|
||||
repoDir: dir,
|
||||
taskStore: makeFakeStore(
|
||||
{ "FN-5551": { column: RENAMED_VOCAB.complete } },
|
||||
lifecycleIr(RENAMED_VOCAB, "orphan-lifecycle"),
|
||||
),
|
||||
integrationBranch: "main",
|
||||
currentTaskId: "FN-5419",
|
||||
commitSha: orphanSha,
|
||||
commitSubject: "feat(FN-5551): orphaned squash",
|
||||
commitBody: "Fusion-Task-Id: FN-5551\n",
|
||||
});
|
||||
|
||||
expect(result.orphan).toBe(true);
|
||||
} finally {
|
||||
removeTmpDirSync(dir);
|
||||
}
|
||||
});
|
||||
|
||||
it("still refuses on a RENAMED board when the source task is mid-flight", async () => {
|
||||
/*
|
||||
Non-vacuous companion: without it, a guard that treated every source as finished would pass the case
|
||||
above. Same renamed board, same shape — only the source lane changes.
|
||||
*/
|
||||
const dir = setupRepo();
|
||||
try {
|
||||
git(dir, "git checkout -b sibling-orphan-wip");
|
||||
writeFileSync(join(dir, "orphan-wip.txt"), "orphan\n");
|
||||
git(dir, "git add orphan-wip.txt");
|
||||
git(dir, `git commit -m "feat(FN-5551): orphaned squash" -m "Fusion-Task-Id: FN-5551"`);
|
||||
const orphanSha = git(dir, "git rev-parse HEAD");
|
||||
git(dir, "git checkout main");
|
||||
|
||||
const result = await classifyOrphanOurAdvance({
|
||||
repoDir: dir,
|
||||
taskStore: makeFakeStore(
|
||||
{ "FN-5551": { column: RENAMED_VOCAB.wip } },
|
||||
lifecycleIr(RENAMED_VOCAB, "orphan-lifecycle"),
|
||||
),
|
||||
integrationBranch: "main",
|
||||
currentTaskId: "FN-5419",
|
||||
commitSha: orphanSha,
|
||||
commitSubject: "feat(FN-5551): orphaned squash",
|
||||
commitBody: "Fusion-Task-Id: FN-5551\n",
|
||||
});
|
||||
|
||||
expect(result.orphan).toBe(false);
|
||||
if (!result.orphan) expect(result.reason).toBe("source-task-not-done");
|
||||
} finally {
|
||||
removeTmpDirSync(dir);
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses when source task is not done", async () => {
|
||||
const dir = setupRepo();
|
||||
try {
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { PluginRunner, type PluginRunnerOptions } from "../plugin-runner.js";
|
||||
import { RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js";
|
||||
import {
|
||||
__resetWorkflowExtensionRegistryForTests,
|
||||
getWorkflowExtensionRegistry,
|
||||
@@ -1627,6 +1628,62 @@ describe("PluginRunner", () => {
|
||||
);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-12:40 (batch-engine tail):
|
||||
The case above proves `onTaskCompleted` fires for the LEGACY id, which is what the guard compared
|
||||
against — it passed before this conversion and would pass for a broken one. On a board whose complete
|
||||
lane is renamed the hook NEVER fired: every plugin that closes an issue, posts a notification, or
|
||||
records a metric on completion silently stopped, with nothing logged.
|
||||
|
||||
REVERT CHECK, measured: with `if (to === "done")` restored, this fails — `invokeHook` is never called
|
||||
with `onTaskCompleted`. The legacy case above passes both ways, which is why both are kept.
|
||||
*/
|
||||
it("should invoke onTaskCompleted when the complete lane is RENAMED", async () => {
|
||||
const ir = lifecycleIr(RENAMED_VOCAB, "plugin-runner-lifecycle");
|
||||
mockTaskStore.getTaskWorkflowSelectionAsync = vi.fn(async () => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] }));
|
||||
mockTaskStore.getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] }));
|
||||
mockTaskStore.getWorkflowDefinition = vi.fn(async (id: string) => (id === "plugin-runner-lifecycle" ? { ir } : undefined));
|
||||
mockPluginLoader.invokeHook = vi.fn();
|
||||
await pluginRunner.init();
|
||||
|
||||
const movedHandler = mockTaskStore.on.mock.calls.find(
|
||||
call => call[0] === "task:moved"
|
||||
)?.[1];
|
||||
|
||||
const mockTask = { id: "FN-001", title: "Test Task" };
|
||||
if (movedHandler) {
|
||||
movedHandler({ task: mockTask, from: RENAMED_VOCAB.review, to: RENAMED_VOCAB.complete });
|
||||
}
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(mockPluginLoader.invokeHook).toHaveBeenCalledWith("onTaskCompleted", mockTask);
|
||||
});
|
||||
|
||||
it("should NOT invoke onTaskCompleted when a RENAMED board moves the card to a non-complete lane", async () => {
|
||||
/*
|
||||
Non-vacuous companion: without it, a guard that fired on EVERY move would satisfy the case above.
|
||||
Same renamed board, same handler — only the destination lane changes.
|
||||
*/
|
||||
const ir = lifecycleIr(RENAMED_VOCAB, "plugin-runner-lifecycle");
|
||||
mockTaskStore.getTaskWorkflowSelectionAsync = vi.fn(async () => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] }));
|
||||
mockTaskStore.getTaskWorkflowSelection = vi.fn(() => ({ workflowId: "plugin-runner-lifecycle", stepIds: [] }));
|
||||
mockTaskStore.getWorkflowDefinition = vi.fn(async (id: string) => (id === "plugin-runner-lifecycle" ? { ir } : undefined));
|
||||
mockPluginLoader.invokeHook = vi.fn();
|
||||
await pluginRunner.init();
|
||||
|
||||
const movedHandler = mockTaskStore.on.mock.calls.find(
|
||||
call => call[0] === "task:moved"
|
||||
)?.[1];
|
||||
|
||||
const mockTask = { id: "FN-001", title: "Test Task" };
|
||||
if (movedHandler) {
|
||||
movedHandler({ task: mockTask, from: RENAMED_VOCAB.hold, to: RENAMED_VOCAB.wip });
|
||||
}
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(mockPluginLoader.invokeHook).not.toHaveBeenCalledWith("onTaskCompleted", mockTask);
|
||||
});
|
||||
|
||||
it("should NOT invoke onTaskCompleted when task moves elsewhere", async () => {
|
||||
mockPluginLoader.invokeHook = vi.fn();
|
||||
await pluginRunner.init();
|
||||
|
||||
@@ -13,7 +13,7 @@ import type {
|
||||
TaskStore,
|
||||
} from "@fusion/core";
|
||||
import { createLogger } from "./logger.js";
|
||||
import { resolveProjectDefaultModel } from "@fusion/core";
|
||||
import { resolveProjectDefaultModel, resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core";
|
||||
import { createFnAgent, promptWithFallback } from "./pi.js";
|
||||
import { resolveMcpServersForStore } from "./mcp-resolution.js";
|
||||
import { createRunAuditor, generateSyntheticRunId, type EngineRunContext, type RunAuditor } from "./run-audit.js";
|
||||
@@ -243,7 +243,7 @@ export class AgentReflectionService {
|
||||
return null;
|
||||
}
|
||||
|
||||
const outcome = this.classifyOutcome(task);
|
||||
const outcome = await this.classifyOutcome(task);
|
||||
if (!outcome || outcome === "stuck") {
|
||||
await this.emitReflectionAudit(auditor, "reflection:skipped", agentId, trigger, { taskId, ...options }, {
|
||||
reason: "not-completed",
|
||||
@@ -519,7 +519,7 @@ export class AgentReflectionService {
|
||||
continue;
|
||||
}
|
||||
|
||||
const outcome = this.classifyOutcome(task);
|
||||
const outcome = await this.classifyOutcome(task);
|
||||
if (!outcome) {
|
||||
continue;
|
||||
}
|
||||
@@ -700,7 +700,23 @@ export class AgentReflectionService {
|
||||
return ids;
|
||||
}
|
||||
|
||||
private classifyOutcome(task: Task): TaskOutcome["outcome"] | null {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:10 (batch-engine tail):
|
||||
"Completed" here is the COMPLETE and REVIEW roles, not the two ids. Keyed on the literals, a renamed
|
||||
board returned `null` for every finished task, so BOTH callers treated it as nothing-to-reflect-on:
|
||||
the per-task path recorded `reflection:skipped` with reason "not-completed" for work that had in fact
|
||||
completed, and the sweep path silently `continue`d. Agent performance reflection therefore captured
|
||||
nothing at all on a custom board.
|
||||
|
||||
Async because the resolution is: the sync IR reader returns the DEFAULT workflow for every task in
|
||||
production (see sync-workflow-ir-callsite-allowlist), so a sync guard here would read as converted and
|
||||
still be wrong. Both call sites already sit inside async functions, so this adds no new seam.
|
||||
|
||||
Unioned with the legacy pair — `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than
|
||||
throwing, and without the union a degraded board resolves a set excluding its own terminal lanes,
|
||||
which reproduces the bug.
|
||||
*/
|
||||
private async classifyOutcome(task: Task): Promise<TaskOutcome["outcome"] | null> {
|
||||
const normalizedStatus = task.status?.toLowerCase() ?? "";
|
||||
const hasStuckSignal =
|
||||
normalizedStatus.includes("stuck")
|
||||
@@ -717,7 +733,16 @@ export class AgentReflectionService {
|
||||
return "failed";
|
||||
}
|
||||
|
||||
if (task.column === "done" || task.column === "in-review") {
|
||||
const completedColumns = new Set<string>(["done", "in-review"]);
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(this.taskStore, task.id);
|
||||
if (ir) {
|
||||
for (const flag of ["complete", "mergeOrchestration", "mergeBlocker", "humanReview"] as const) {
|
||||
for (const id of columnsWithFlag(ir, flag)) completedColumns.add(id);
|
||||
}
|
||||
}
|
||||
} catch { /* degraded: legacy pair only */ }
|
||||
if (completedColumns.has(task.column)) {
|
||||
return "completed";
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { exec } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { promisify } from "node:util";
|
||||
import type { Task, TaskStore } from "@fusion/core";
|
||||
import { resolveWorkflowIrForTask, columnsWithFlag, resolveReboundTarget, TransitionRejectionError } from "@fusion/core";
|
||||
import {
|
||||
classifyBootstrapMisbinding,
|
||||
inspectBranchConflict,
|
||||
@@ -122,15 +123,122 @@ export class BranchWorktreeAutoRecoveryHandler {
|
||||
|
||||
private async requeueAfterRecovery(task: Task, failure: AutoRecoveryFailure, rationale: string, evidence: RecoveryEvidence): Promise<void> {
|
||||
if (task.userPaused) return;
|
||||
if (task.column === "in-progress") {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:40 (batch-engine tail):
|
||||
TWO defects here, and fixing only the counted one would have been half a fix.
|
||||
|
||||
1. The WIP test was the id `in-progress`, so on a renamed board the stale branch/baseCommitSha were
|
||||
never cleared and the requeued card carried a dead branch back into execution.
|
||||
2. The requeue DESTINATION was the hardcoded `todo` — census-invisible, because the census scores
|
||||
comparisons and this is a call argument. A board without a `todo` column was requeued into a lane
|
||||
that does not exist. `resolveReboundTarget` is the shared helper for exactly this (KTD-10 ordering:
|
||||
hold -> intake -> first column), and it is why the destination is resolved rather than guessed.
|
||||
|
||||
Both fall back to the legacy ids: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than
|
||||
throwing, so a degraded board behaves exactly as before.
|
||||
*/
|
||||
let wipColumns = new Set<string>(["in-progress"]);
|
||||
let reboundTarget = "todo";
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:40 (#2797 review — coderabbit):
|
||||
The catch no longer swallows. Falling back to the legacy ids stays — this is a RECOVERY path, and
|
||||
refusing to act because a workflow could not be read would strand the very task the handler exists
|
||||
to unstick — but the fallback is now RECORDED rather than silent, so "why did this land in `todo`"
|
||||
is answerable after the fact.
|
||||
|
||||
Note the fallback is rarely what routes a custom board here: `resolveWorkflowIrForTask` degrades to
|
||||
the BUILT-IN IR instead of throwing, so an unreadable custom workflow resolves `todo` through the
|
||||
resolver and never reaches this catch. That is why the real protection is the move-rejection guard
|
||||
below, not this branch — this one only makes the rare hard failure visible.
|
||||
*/
|
||||
let laneResolutionError: string | undefined;
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(this.deps.taskStore, task.id);
|
||||
if (ir) {
|
||||
const resolvedWip = columnsWithFlag(ir, "countsTowardWip");
|
||||
if (resolvedWip.length > 0) wipColumns = new Set<string>(resolvedWip);
|
||||
reboundTarget = resolveReboundTarget(ir) ?? "todo";
|
||||
}
|
||||
} catch (err) {
|
||||
laneResolutionError = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:45 (#2797 review — greptile):
|
||||
THE REQUEUE MUST NOT DIE ON A DESTINATION THE BOARD DOES NOT DECLARE.
|
||||
|
||||
The review flagged the `catch` above retaining `reboundTarget = "todo"`. Tracing it, the exposure
|
||||
is wider than that branch: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than
|
||||
throwing, so a task whose custom workflow cannot be read resolves a target from the DEFAULT board
|
||||
— also `todo` — without the catch ever running. Guarding only the throw path would have looked
|
||||
like a fix and changed almost nothing.
|
||||
|
||||
What actually bites is the move: `moveTaskInternal` REJECTS a destination the workflow does not
|
||||
declare (`TransitionRejectionError: unknown-column`). Unhandled, that throws out of the recovery
|
||||
handler whose whole job is to unstick the task — so the recovery became a second way for it to stay
|
||||
stuck, with no audit row to explain it.
|
||||
|
||||
Catching at the move covers every route to a wrong destination, resolved or guessed. A task left
|
||||
parked WITH a record beats one parked by an exception nobody sees.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-17:45 (#2797 review — greptile P1 x2):
|
||||
|
||||
ORDER: MOVE FIRST, THEN CLEAR THE BRANCH LINKAGE. The clear used to run BEFORE the move, so a
|
||||
rejected move left the card in its wip lane with `branch`/`baseCommitSha` already erased — the
|
||||
recovery destroyed the only pointers back to the work and then declined to requeue. Half-applied is
|
||||
worse than not applied: nothing else can reconstruct the branch from the row afterwards. The clear
|
||||
now happens only once the requeue has actually landed.
|
||||
|
||||
REASON MUST NAME THE ACTUAL FAILURE. The catch labelled EVERY moveTask failure
|
||||
`rebound-target-rejected` — capacity exhaustion, a guard rejection, a deleted task, a persistence
|
||||
error — so the audit asserted a lane problem for causes that have nothing to do with lanes, and a
|
||||
reader debugging a stuck card would chase the wrong thing. `TransitionRejectionError` carries a typed
|
||||
`rejection.code`, so the unknown-column case is distinguishable exactly rather than by message match.
|
||||
|
||||
Everything is still CAUGHT (an exception thrown out of the recovery handler is invisible), but the
|
||||
row now says which failure it was.
|
||||
*/
|
||||
let moveFailure: { reason: string; code?: string } | undefined;
|
||||
try {
|
||||
await this.deps.taskStore.moveTask(task.id, reboundTarget, {
|
||||
moveSource: "engine",
|
||||
preserveResumeState: true,
|
||||
preserveProgress: true,
|
||||
preserveWorktree: false,
|
||||
});
|
||||
} catch (err) {
|
||||
const code = err instanceof TransitionRejectionError ? err.rejection.code : undefined;
|
||||
moveFailure = {
|
||||
reason: code === "unknown-column" ? "rebound-target-rejected" : "requeue-move-failed",
|
||||
...(code ? { code } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
if (moveFailure) {
|
||||
await this.deps.runAudit.database({
|
||||
type: "branch-worktree:auto-requeue-skipped",
|
||||
target: task.id,
|
||||
metadata: {
|
||||
class: failure.class,
|
||||
reason: moveFailure.reason,
|
||||
...(moveFailure.code ? { rejectionCode: moveFailure.code } : {}),
|
||||
rationale,
|
||||
...(laneResolutionError ? { laneResolutionError } : {}),
|
||||
reboundTarget,
|
||||
column: task.column,
|
||||
/* Branch linkage deliberately NOT cleared on this path — see the note above. */
|
||||
branchPreserved: true,
|
||||
evidence,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (wipColumns.has(task.column)) {
|
||||
await this.deps.taskStore.updateTask(task.id, { branch: null, baseCommitSha: null });
|
||||
}
|
||||
await this.deps.taskStore.moveTask(task.id, "todo", {
|
||||
moveSource: "engine",
|
||||
preserveResumeState: true,
|
||||
preserveProgress: true,
|
||||
preserveWorktree: false,
|
||||
});
|
||||
await this.deps.runAudit.database({
|
||||
type: "branch-worktree:auto-requeue",
|
||||
target: task.id,
|
||||
@@ -138,6 +246,7 @@ export class BranchWorktreeAutoRecoveryHandler {
|
||||
class: failure.class,
|
||||
rationale,
|
||||
prevPausedReason: task.pausedReason ?? null,
|
||||
...(laneResolutionError ? { laneResolutionError } : {}),
|
||||
evidence,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -290,6 +290,13 @@ export class CliSessionStateMachine {
|
||||
this.transition("busy");
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:CliAgentStateMachine 2026-07-30-12:20 DELIBERATE-LITERAL:
|
||||
`done` here is a `CliMachineState`, NOT a lifecycle column — this machine tracks one CLI agent
|
||||
process (`ready`/`busy`/`waitingOnInput`/`done`/`resuming`/`idle`) and never reads a board column.
|
||||
The lifecycle-column census matches the bare string and counted it; converting it to a workflow role
|
||||
would be nonsense, so it is marked rather than left to be "fixed" by a later sweep.
|
||||
*/
|
||||
/** done → busy (follow-up). Alias for injectPrompt from the done state. */
|
||||
followUp(): void {
|
||||
if (this.state !== "done") {
|
||||
@@ -365,6 +372,7 @@ export class CliSessionStateMachine {
|
||||
* Idle / output progress never reach here.
|
||||
*/
|
||||
signalDone(): void {
|
||||
/* DELIBERATE-LITERAL — `CliMachineState`, not a board column. See followUp() above. */
|
||||
if (this.state === "done") return; // idempotent
|
||||
if (this.state !== "busy" && this.state !== "waitingOnInput") {
|
||||
throw new InvalidCliTransitionError(this.state, "signalDone");
|
||||
|
||||
@@ -405,6 +405,8 @@ export class CliTaskSession {
|
||||
const machine = this.hub.getStateMachine(this.sessionId);
|
||||
if (machine) {
|
||||
try {
|
||||
/* DELIBERATE-LITERAL — `CliMachineState` from the CLI agent state machine, not a lifecycle
|
||||
column. See `state-machine.ts#followUp`. */
|
||||
if (machine.getState() === "done") machine.followUp();
|
||||
else if (machine.getState() === "ready" || machine.getState() === "resuming") {
|
||||
machine.injectPrompt();
|
||||
|
||||
@@ -3,6 +3,8 @@ import { exec, execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import {
|
||||
normalizeMergeIntegrationWorktreeMode,
|
||||
resolveWorkflowIrForTask,
|
||||
columnsWithFlag,
|
||||
} from "@fusion/core";
|
||||
import type {
|
||||
MergeIntegrationWorktreeMode,
|
||||
@@ -10,6 +12,7 @@ import type {
|
||||
ProjectSettings,
|
||||
Task,
|
||||
TaskStore,
|
||||
WorkflowIr,
|
||||
} from "@fusion/core";
|
||||
import {
|
||||
activeSessionRegistry,
|
||||
@@ -334,11 +337,27 @@ export async function probeIntegrationWorktreeState(
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:10 (batch-engine tail):
|
||||
"Someone else is still using this worktree" excludes tasks that have FINISHED. Keyed on the literal, a
|
||||
renamed complete lane meant a shipped task still counted as a live user, so the integration worktree
|
||||
could never be reused and the merge path took the slower rebuild every time.
|
||||
|
||||
Resolved per CANDIDATE task (each may run its own workflow), one IR cache for the scan, and only for
|
||||
the tasks that actually share the worktree path — the common case resolves nothing at all.
|
||||
*/
|
||||
async function findOtherWorktreeUser(store: TaskStore, worktreePath: string, excludeTaskId: string): Promise<string | null> {
|
||||
const tasks = await store.listTasks({ slim: true, includeArchived: false } as never);
|
||||
const irCache = new Map<string, WorkflowIr>();
|
||||
for (const task of tasks) {
|
||||
if (task.id === excludeTaskId) continue;
|
||||
if (task.worktree === worktreePath && task.column !== "done") {
|
||||
if (task.worktree !== worktreePath) continue;
|
||||
const completeColumns = new Set<string>(["done"]);
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store, task.id, irCache);
|
||||
if (ir) for (const id of columnsWithFlag(ir, "complete")) completeColumns.add(id);
|
||||
} catch { /* degraded: legacy id only */ }
|
||||
if (!completeColumns.has(task.column)) {
|
||||
return task.id;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -25,6 +25,7 @@
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import type { TaskStore } from "@fusion/core";
|
||||
import { resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core";
|
||||
import type { RunAuditor } from "./run-audit.js";
|
||||
import {
|
||||
advanceIntegrationBranchRef,
|
||||
@@ -91,7 +92,18 @@ export async function classifyOrphanOurAdvance(
|
||||
|
||||
const sourceTask = await input.taskStore.getTask(sourceTaskId);
|
||||
if (!sourceTask) return { orphan: false, reason: "source-task-not-found" };
|
||||
if (sourceTask.column !== "done") return { orphan: false, reason: "source-task-not-done" };
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:10 (batch-engine tail):
|
||||
Orphan rehoming only applies once the SOURCE task has finished. Keyed on the literal, a renamed
|
||||
complete lane made every source read as unfinished, so orphaned commits were never rehomed and simply
|
||||
stayed stranded off the integration branch.
|
||||
*/
|
||||
const sourceCompleteColumns = new Set<string>(["done"]);
|
||||
try {
|
||||
const sourceIr = await resolveWorkflowIrForTask(input.taskStore, sourceTaskId);
|
||||
if (sourceIr) for (const id of columnsWithFlag(sourceIr, "complete")) sourceCompleteColumns.add(id);
|
||||
} catch { /* degraded: legacy id only */ }
|
||||
if (!sourceCompleteColumns.has(sourceTask.column)) return { orphan: false, reason: "source-task-not-done" };
|
||||
|
||||
const integrationRef = `refs/heads/${input.integrationBranch}`;
|
||||
if (await isAncestor(input.repoDir, input.commitSha, integrationRef)) {
|
||||
|
||||
@@ -3,7 +3,8 @@ import { readFile, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
import type { Task, TaskStore } from "@fusion/core";
|
||||
import type { Task, TaskStore, WorkflowIr } from "@fusion/core";
|
||||
import { resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core";
|
||||
|
||||
import { toTaskToken } from "./merger.js";
|
||||
|
||||
@@ -118,11 +119,36 @@ export async function evaluateScopeAutoWiden(params: EvaluateScopeAutoWidenParam
|
||||
const allTasks = typeof store.listTasks === "function"
|
||||
? await store.listTasks({ slim: true, includeArchived: false })
|
||||
: [];
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-12:55 (batch-engine tail):
|
||||
"Still active" is the COMPLETE and ARCHIVED roles, not the two ids. On a renamed board every FINISHED
|
||||
card counted as an active other, so auto-widen refused to widen into files whose only other claimant
|
||||
had already shipped — a merge blocked by a task that no longer exists in any meaningful sense.
|
||||
|
||||
NOT the query-filter class: this query passes no `column`, so the predicate is the only lane gate here.
|
||||
|
||||
Resolved per OTHER TASK (each may run its own workflow), one IR cache for the pass, unioned with the
|
||||
legacy pair because `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than throwing —
|
||||
without the union a degraded board would treat every finished card as active, which is this bug.
|
||||
*/
|
||||
const irCache = new Map<string, WorkflowIr>();
|
||||
const terminalByTaskId = new Map<string, ReadonlySet<string>>();
|
||||
for (const other of allTasks) {
|
||||
const columns = new Set<string>(["done", "archived"]);
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store as TaskStore, other.id, irCache);
|
||||
if (ir) {
|
||||
for (const flag of ["complete", "archived"] as const) {
|
||||
for (const id of columnsWithFlag(ir, flag)) columns.add(id);
|
||||
}
|
||||
}
|
||||
} catch { /* degraded: legacy pair only */ }
|
||||
terminalByTaskId.set(other.id, columns);
|
||||
}
|
||||
const activeOtherTasks = allTasks.filter((other) => (
|
||||
other.id !== taskId &&
|
||||
other.deletedAt == null &&
|
||||
other.column !== "done" &&
|
||||
other.column !== "archived"
|
||||
terminalByTaskId.get(other.id)?.has(other.column) !== true
|
||||
));
|
||||
|
||||
for (const file of candidateFiles) {
|
||||
|
||||
@@ -43,6 +43,7 @@ import {
|
||||
evaluatePromptConditionDetailed,
|
||||
resolveEffectivePluginSettings,
|
||||
resolveWorkflowIrForTask,
|
||||
columnsWithFlag,
|
||||
workflowExtensionRegistryId,
|
||||
} from "@fusion/core";
|
||||
import { createLogger, executorLog } from "./logger.js";
|
||||
@@ -1459,10 +1460,33 @@ export class PluginRunner {
|
||||
// Fire and forget - don't await
|
||||
void this.invokeHookSafe("onTaskMoved", task, from, to);
|
||||
|
||||
// If task completed, invoke onTaskCompleted hook
|
||||
if (to === "done") {
|
||||
void this.invokeHookSafe("onTaskCompleted", task);
|
||||
}
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-12:30 (batch-engine tail):
|
||||
"Completed" is the COMPLETE role, not the id `done`. Keyed on the literal, `onTaskCompleted` NEVER
|
||||
fired on a board whose complete lane is renamed — every plugin that closes an issue, posts a
|
||||
notification, or records a metric on completion silently stopped, with nothing logged.
|
||||
|
||||
Resolved ASYNCHRONOUSLY inside the existing fire-and-forget seam rather than through the sync IR
|
||||
reader: per `sync-workflow-ir-callsite-allowlist`, `resolveTaskWorkflowIrSync` returns the DEFAULT
|
||||
workflow for every task in production, so a sync-resolved guard here would read as converted and
|
||||
still be wrong for every custom board. This listener is already `void`-dispatched (the line above),
|
||||
so awaiting inside it changes no ordering the caller can observe — the same shape
|
||||
`NotificationService` uses for its `task:moved` handler.
|
||||
|
||||
Unioned with the legacy id because `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather
|
||||
than throwing; without it a degraded board resolves a complete set excluding its own complete lane
|
||||
and the hook goes silent again.
|
||||
*/
|
||||
void (async () => {
|
||||
const completeColumns = new Set<string>(["done"]);
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(this.options.taskStore, task.id);
|
||||
if (ir) for (const id of columnsWithFlag(ir, "complete")) completeColumns.add(id);
|
||||
} catch { /* degraded: legacy id only */ }
|
||||
if (completeColumns.has(to)) {
|
||||
await this.invokeHookSafe("onTaskCompleted", task);
|
||||
}
|
||||
})();
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -807,6 +807,14 @@ export type DatabaseMutationType =
|
||||
| "message-delivery:retry-issued"
|
||||
| "message-delivery:park"
|
||||
| "branch-worktree:auto-requeue"
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-13:25 (#2797 review):
|
||||
Emitted when the branch-worktree auto-requeue cannot resolve a rebound destination from the task's
|
||||
own workflow. The requeue is SKIPPED rather than aimed at the legacy `todo`, because `moveTask`
|
||||
rejects a column the board does not declare and the resulting throw left the task parked with no
|
||||
record at all. Metadata stays ids/outcomes-only.
|
||||
*/
|
||||
| "branch-worktree:auto-requeue-skipped"
|
||||
| "branch-worktree:ai-session-spawned"
|
||||
| "branch-worktree:irreducible-pause"
|
||||
| "branch-worktree:foreign-branch-discarded"
|
||||
|
||||
@@ -913,10 +913,36 @@ export async function scanIdleWorktrees(
|
||||
// Find worktree paths assigned to non-done tasks (active worktrees)
|
||||
const tasks = await store.listTasks({ slim: true, includeArchived: false, startupMemo: true });
|
||||
const activeWorktrees = new Set<string>();
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:05 (batch-engine tail):
|
||||
"Still holding its worktree" excludes tasks that have FINISHED. Keyed on the id, a renamed complete
|
||||
lane kept every shipped task's worktree in the ACTIVE set, so this reclaim pass never returned it and
|
||||
the board walked into worktree exhaustion — a stall whose cause is invisible from the symptom.
|
||||
|
||||
NOT the query-filter class: this listTasks call passes no `column`.
|
||||
|
||||
Resolved per TASK (each may run its own workflow) and ONLY for tasks that actually record a worktree,
|
||||
with one IR cache for the pass. Unioned with the legacy id because `resolveWorkflowIrForTask` degrades
|
||||
to the BUILT-IN IR rather than throwing — without the union a degraded board would hold every worktree
|
||||
forever, which is this bug.
|
||||
*/
|
||||
const reclaimIrCache = new Map<string, Awaited<ReturnType<typeof resolveWorkflowIrForTask>>>();
|
||||
const completeByTaskId = new Map<string, ReadonlySet<string>>();
|
||||
for (const task of tasks) {
|
||||
if (task.worktree && task.column !== "done" && registeredWorktrees.has(resolve(task.worktree))) {
|
||||
if (!task.worktree) continue;
|
||||
const columns = new Set<string>(["done"]);
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store, task.id, reclaimIrCache);
|
||||
if (ir) for (const id of columnsWithFlag(ir, "complete")) columns.add(id);
|
||||
} catch { /* degraded: legacy id only */ }
|
||||
completeByTaskId.set(task.id, columns);
|
||||
}
|
||||
const isUnfinished = (task: { id: string; column: string }) =>
|
||||
completeByTaskId.get(task.id)?.has(task.column) !== true;
|
||||
for (const task of tasks) {
|
||||
if (task.worktree && isUnfinished(task) && registeredWorktrees.has(resolve(task.worktree))) {
|
||||
activeWorktrees.add(resolve(task.worktree));
|
||||
} else if (task.worktree && task.column !== "done") {
|
||||
} else if (task.worktree && isUnfinished(task)) {
|
||||
worktreePoolLog.debug(`Ignoring task ${task.id} worktree metadata because it is not a registered git worktree: ${task.worktree}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -45,11 +45,7 @@
|
||||
"packages/dashboard/app/utils/taskTiming.ts": 2,
|
||||
"packages/dashboard/src/github-tracking-state.ts": 2,
|
||||
"packages/dashboard/src/server.ts": 2,
|
||||
"packages/engine/src/agent-reflection.ts": 2,
|
||||
"packages/engine/src/auto-merge-finalization.ts": 2,
|
||||
"packages/engine/src/cli-agent/state-machine.ts": 2,
|
||||
"packages/engine/src/merger-scope-auto-widen.ts": 2,
|
||||
"packages/engine/src/worktree-pool.ts": 2,
|
||||
"packages/core/src/eval-automation.ts": 1,
|
||||
"packages/core/src/eval-signal-collector.ts": 1,
|
||||
"packages/core/src/in-review-stall.ts": 1,
|
||||
@@ -87,22 +83,15 @@
|
||||
"packages/dashboard/src/task-planner-chat-context.ts": 1,
|
||||
"packages/dashboard/src/task-planner-chat-metrics.ts": 1,
|
||||
"packages/dashboard/src/test/mockCoreEngine.ts": 1,
|
||||
"packages/engine/src/auto-recovery-handlers/branch-worktree.ts": 1,
|
||||
"packages/engine/src/backlog-pressure-reporter.ts": 1,
|
||||
"packages/engine/src/cli-agent/task-session.ts": 1,
|
||||
"packages/engine/src/ephemeral-worker-manager.ts": 1,
|
||||
"packages/engine/src/merger-integration-worktree.ts": 1,
|
||||
"packages/engine/src/merger-orphan-rehome.ts": 1,
|
||||
"packages/engine/src/merger.ts": 1,
|
||||
"packages/engine/src/plugin-runner.ts": 1,
|
||||
"packages/engine/src/pr-comment-handler.ts": 1,
|
||||
"packages/engine/src/runtimes/in-process-runtime.ts": 1,
|
||||
"packages/engine/src/triage.ts": 1
|
||||
},
|
||||
"deliberateByFile": {
|
||||
"packages/dashboard/src/reliability-metrics.ts\u0000in-review": 4,
|
||||
"packages/engine/src/scheduler.ts\u0000in-progress": 3,
|
||||
"packages/engine/src/scheduler.ts\u0000in-review": 3,
|
||||
"packages/core/src/live-agent-count.ts\u0000in-progress": 2,
|
||||
"packages/core/src/live-agent-count.ts\u0000in-review": 2,
|
||||
"packages/core/src/store.ts\u0000in-review": 2,
|
||||
@@ -111,6 +100,9 @@
|
||||
"packages/core/src/task-merge.ts\u0000in-review": 2,
|
||||
"packages/dashboard/app/components/TaskCard.tsx\u0000triage": 2,
|
||||
"packages/dashboard/app/components/TaskDetailModal.tsx\u0000triage": 2,
|
||||
"packages/engine/src/cli-agent/state-machine.ts\u0000done": 2,
|
||||
"packages/engine/src/scheduler.ts\u0000in-progress": 2,
|
||||
"packages/engine/src/scheduler.ts\u0000in-review": 2,
|
||||
"packages/engine/src/usage-limit-detector.ts\u0000archived": 2,
|
||||
"packages/engine/src/usage-limit-detector.ts\u0000done": 2,
|
||||
"plugins/fusion-plugin-reports/src/store/report-store.ts\u0000archived": 2,
|
||||
@@ -148,13 +140,13 @@
|
||||
"packages/dashboard/src/routes/register-task-workflow-routes.ts\u0000triage": 1,
|
||||
"packages/engine/src/agent-heartbeat.ts\u0000archived": 1,
|
||||
"packages/engine/src/agent-heartbeat.ts\u0000done": 1,
|
||||
"packages/engine/src/cli-agent/task-session.ts\u0000done": 1,
|
||||
"packages/engine/src/hold-release.ts\u0000archived": 1,
|
||||
"packages/engine/src/hold-release.ts\u0000done": 1,
|
||||
"packages/engine/src/hold-release.ts\u0000in-review": 1,
|
||||
"packages/engine/src/project-engine.ts\u0000in-review": 1,
|
||||
"packages/engine/src/scheduler.ts\u0000archived": 1,
|
||||
"packages/engine/src/scheduler.ts\u0000done": 1,
|
||||
"packages/engine/src/scheduler.ts\u0000todo": 1,
|
||||
"packages/engine/src/triage.ts\u0000triage": 1,
|
||||
"plugins/fusion-plugin-even-cards/src/cards/board-cards.ts\u0000archived": 1,
|
||||
"plugins/fusion-plugin-even-cards/src/cards/board-cards.ts\u0000done": 1,
|
||||
|
||||
Reference in New Issue
Block a user