diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 356ea895f8..4525e4c933 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -2451,16 +2451,25 @@ export default function kbExtension(pi: ExtensionAPI) { ...buildManualRetryResetPatch({ resetMergeRetries: true }), }); - // Move to todo column + /* + FNXC:TaskRetry 2026-07-31-23:59 (the SECOND instance of the review finding on #3152): + A reviewer caught the chat `fn_task_retry` tool resolving its move target while still reporting + `"todo"` in the log, the response text and `details.newColumn`. This CLI retry had the identical + gap — same PR, same conversion, same three unconverted report sites — so it is fixed with it + rather than waiting for the same comment on the next surface. + + Resolve once, then use that value everywhere the operator or a downstream tool reads it. + */ // FNXC:ToolPermissionGates 2026-07-26-13:55: user-facing retry move carries the user/hard-cancel source (Move-Task contract). - await store.moveTask(params.id, 'todo', { moveSource: "user" }); - + const retryTarget = await fusionCore.resolveReboundTargetForTask(store, params.id); + await store.moveTask(params.id, retryTarget, { moveSource: "user" }); + // Log the retry action - await store.logEntry(params.id, "Retry requested via Fusion extension", "Task reset to todo for retry"); - + await store.logEntry(params.id, "Retry requested via Fusion extension", `Task reset to ${retryTarget} for retry`); + return { - content: [{ type: "text", text: `Retried ${params.id} → todo (failure state cleared)` }], - details: { taskId: params.id, newColumn: 'todo' }, + content: [{ type: "text", text: `Retried ${params.id} → ${retryTarget} (failure state cleared)` }], + details: { taskId: params.id, newColumn: retryTarget }, }; }, }); diff --git a/packages/core/src/task-store/branch-and-pr-entities.ts b/packages/core/src/task-store/branch-and-pr-entities.ts index 4205cc4a15..cd8e009fd2 100644 --- a/packages/core/src/task-store/branch-and-pr-entities.ts +++ b/packages/core/src/task-store/branch-and-pr-entities.ts @@ -650,7 +650,7 @@ export async function updateTaskImpl(store: TaskStore, throw new Error(validation.message); } if (validation.requiresFinalize) { - await store.moveTask(id, "done", { + await store.moveTask(id, (await resolveTaskLifecycleColumns(store, id))?.complete ?? "done", { moveSource: "engine", recoveryRehome: true, preserveProgress: true, diff --git a/packages/dashboard/src/server.ts b/packages/dashboard/src/server.ts index 7bad81c771..45b30bdd2d 100644 --- a/packages/dashboard/src/server.ts +++ b/packages/dashboard/src/server.ts @@ -20,7 +20,7 @@ import type { AgentLogEntry, RunAuditEvent, } from "@fusion/core"; -import { AgentStore, ChatStore, queryRunAuditEvents, resolveGlobalDir, resolveProjectColumnsForRoles, REVIEW_ROLES, setRunningAgentCountSource } from "@fusion/core"; +import { AgentStore, ChatStore, queryRunAuditEvents, resolveGlobalDir, resolveProjectColumnsForRoles, resolveReboundTargetForTask, REVIEW_ROLES, setRunningAgentCountSource } from "@fusion/core"; import type { AuthStorageLike, ModelRegistryLike } from "./routes.js"; import { createApiRoutes } from "./routes.js"; import { createSSE, disconnectSSEClient, markSSEClientAlive } from "./sse.js"; @@ -761,10 +761,19 @@ type CliRelaunchSessionStore = ServerOptions["cliSessionTransport"] extends infe : never : never; +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:59: +`column` WIDENED from the literal `"todo"` to `string`, because the target is now resolved. + +The narrow literal type was not a constraint anyone chose — it was inferred from the single call +below, which passed `"todo"`. It then made the type system ENFORCE the bug: a resolved rebound target +is a `string`, so the correct value could not be passed without this edit. A type that only admits the +legacy id is a lint against fixing it. +*/ interface CliRelaunchTaskStoreLike { getTask(taskId: string): Promise; updateTask(taskId: string, patch: Record): Promise; - moveTask(taskId: string, column: "todo", options?: Record): Promise; + moveTask(taskId: string, column: string, options?: Record): Promise; logEntry(taskId: string, message: string, details?: string): Promise; } @@ -804,7 +813,7 @@ export function wireCliRelaunchListener(options: { `CLI session relaunch requested from ${info.sessionId} — clearing resume linkage and re-enqueueing for a fresh executor run`, ); await taskStore.updateTask(info.taskId, { paused: false, status: null, error: null }); - await taskStore.moveTask(info.taskId, "todo", { + await taskStore.moveTask(info.taskId, await resolveReboundTargetForTask(taskStore as never, info.taskId), { preserveProgress: true, moveSource: "engine", recoveryRehome: true, diff --git a/packages/engine/src/__tests__/no-legacy-move-targets.test.ts b/packages/engine/src/__tests__/no-legacy-move-targets.test.ts new file mode 100644 index 0000000000..7f76ef7e4d --- /dev/null +++ b/packages/engine/src/__tests__/no-legacy-move-targets.test.ts @@ -0,0 +1,175 @@ +// @vitest-environment node + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:59: +A MOVE TARGET IS AN ARGUMENT, SO NO EXISTING GATE COUNTS IT. + +The lifecycle-column census counts COMPARISONS against legacy ids. `moveTask(task.id, "todo")` contains +no comparison, so the census reported zero while 25 such targets sat in `self-healing.ts` alone and 31 +across the tree. + +The failure mode is worse than a stale guard's. `moveTaskInternal` REJECTS a target the workflow does +not declare — `TransitionRejectionError: unknown-column` — which `task-store/moves.ts` documents after +a completion handoff was found THROWING on every renamed board. A guard that fails to match degrades to +"no rescue"; a target that throws is "no rescue, plus an exception in the sweep", and every one of these +sites is a recovery path. + +So this ratchet exists because the population was invisible, not because it was large. Fixing it once +without a guard means it is invisible again the moment someone adds the next one. + +WHAT IS AND IS NOT A TARGET. Only the SECOND argument of a `moveTask(...)` call counts. A legacy id as +a FALLBACK (`resolveReboundTargetForTask` returning `"todo"`, or `lifecycle?.complete ?? "done"`) is +the degraded answer every resolver in this program is required to have, and is not flagged — the +resolver is what makes the call correct, and its fallback is what keeps default boards working. + +BASELINE, NOT ZERO. Files outside the converted set keep their current counts so this can land without +blocking other lanes; the ratchet fails on any INCREASE, and on a decrease that was not re-recorded — +a stale allowance is a hole the same guards can regrow through. +*/ + +import { describe, expect, it } from "vitest"; +import { execFileSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const REPO_ROOT = resolve(fileURLToPath(import.meta.url), "../../../../.."); + +/** Legacy lifecycle column ids, as move TARGETS. */ +const LEGACY = "(?:todo|triage|in-progress|in-review|done|archived)"; +const MOVE_TARGET = new RegExp(`moveTask\\s*\\(\\s*[^,()]+,\\s*["']${LEGACY}["']`, "g"); + +/* +Files that still hold literal targets, with the count each is allowed. + +EMPTY, and that is the end state rather than a starting one: every `moveTask` call in the tree now +resolves its target. It was populated when this guard was written — `cli/src/extension.ts`, +`core/task-store/branch-and-pr-entities.ts`, `dashboard/src/server.ts` and `engine/agent-tools.ts` +each held one — and the stale-allowance case below is what forced this map to be emptied when those +four were converted in the same change. Leaving the entries behind would have left four slots open +for the class to regrow through while the guard stayed green. + +A NOTE FOR WHOEVER ADDS ONE. If a call genuinely cannot resolve its target, record it here with a +comment saying why, rather than widening the matcher. The failure this guards is not "a literal +appears" — it is `TransitionRejectionError: unknown-column` thrown at runtime on a renamed board. +*/ +const ALLOWED: Record = {}; + +function sourceFiles(): string[] { + return execFileSync( + "git", + ["ls-files", "packages/*/src/**/*.ts", "packages/*/src/*.ts", "packages/*/app/**/*.ts", "packages/*/app/**/*.tsx"], + { cwd: REPO_ROOT, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }, + ) + .split("\n") + .filter((f) => f && !f.includes("__tests__") && !/\.(test|spec)\.tsx?$/.test(f)); +} + +/** Blank comments in place so prose describing a past call is not counted as one. */ +function stripComments(source: string): string { + return source + .replace(/\/\*[\s\S]*?\*\//g, (m) => m.replace(/[^\n]/g, " ")) + .replace(/\/\/[^\n]*/g, ""); +} + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:59: +MEMOISED for gate admission. Each case called this independently and every call re-read the whole +corpus, so the scan ran five times for one tree: measured 660ms of the suite's 800ms. The tree cannot +change mid-run, so the repeat reads bought nothing. + +This matters because the gate's admission bar is about cost and determinism, and a guard that is +gratuitously slow is a guard someone eventually moves back out of the gate. +*/ +let countsCache: Record | undefined; + +function countByFile(): Record { + if (countsCache) return countsCache; + const counts: Record = {}; + for (const file of sourceFiles()) { + let source: string; + try { + source = readFileSync(resolve(REPO_ROOT, file), "utf8"); + } catch { + continue; + } + const hits = stripComments(source).match(MOVE_TARGET); + if (hits && hits.length > 0) counts[file] = hits.length; + } + countsCache = counts; + return counts; +} + +describe("moveTask targets resolve the board's own lane", () => { + /* + ANTI-VACUITY, and its first version was self-defeating: it asserted the scan finds at least one + offender, which can only hold while the defect exists. Converting the last four sites turned it red + — the guard failing precisely because the tree became correct. + + A clean tree is the expected end state, so the proof has to be that the scan REACHES the right code + rather than that it finds something wrong in it. Two independent legs, both of which break if the + glob stops resolving or `moveTask` is renamed: the corpus is a real file list, and files that + genuinely call `moveTask` are inside it. The matcher itself is covered by the case table below. + */ + it("scans a real corpus that still reaches the moveTask call sites", () => { + const files = sourceFiles(); + expect(files.length).toBeGreaterThan(200); + + const callers = files.filter((file) => { + try { + return /moveTask\s*\(/.test(stripComments(readFileSync(resolve(REPO_ROOT, file), "utf8"))); + } catch { + return false; + } + }); + expect(callers.length).toBeGreaterThan(5); + expect(callers).toContain("packages/engine/src/self-healing.ts"); + }); + + /* The matcher is covered, because the scan is only as good as it: each case is a real shape. */ + it.each<[source: string, shouldFlag: boolean, why: string]>([ + ['await store.moveTask(task.id, "todo");', true, "the plain form"], + ['await this.store.moveTask(taskId, "archived", { moveSource: "engine" });', true, "with options"], + ["await store.moveTask(id, 'done');", true, "single quotes"], + ['await store.moveTask(task.id, await resolveReboundTargetForTask(store, task.id));', false, "resolved"], + ['await store.moveTask(task.id, completeLane);', false, "resolved via a local"], + ['const target = lifecycle?.complete ?? "done";', false, "a FALLBACK is not a target"], + ['return resolveReboundTarget(ir) ?? "todo";', false, "a resolver's own degraded answer"], + ['if (task.column === "todo") return;', false, "a comparison — the census owns that class"], + ])("matcher: %s -> %s (%s)", (source, shouldFlag) => { + expect(new RegExp(MOVE_TARGET.source).test(source)).toBe(shouldFlag); + }); + + it("self-healing.ts has NO literal move targets", () => { + /* The file this ratchet was written for: 25 sites, now zero. Asserted by name because a + regression here is a recovery path that throws on a renamed board. */ + expect(countByFile()["packages/engine/src/self-healing.ts"] ?? 0).toBe(0); + }); + + it("no file exceeds its recorded allowance", () => { + const counts = countByFile(); + const violations: string[] = []; + for (const [file, count] of Object.entries(counts)) { + const allowed = ALLOWED[file] ?? 0; + if (count > allowed) violations.push(`${file}: ${count} literal move target(s), allowed ${allowed}`); + } + expect( + violations, + "A moveTask target the workflow does not declare is REJECTED (TransitionRejectionError: " + + "unknown-column), so this throws on a renamed board rather than degrading.\n" + + "Resolve the target (resolveReboundTargetForTask / resolveArchiveTargetForTask /\n" + + "resolveTaskLifecycleColumns) — a legacy id is fine as the resolver's FALLBACK, not as the " + + "argument:\n" + violations.join("\n"), + ).toEqual([]); + }); + + it("no allowance is stale", () => { + /* A recorded allowance that exceeds the tree is a hole the same targets can regrow through — + the same reason the census baseline fails on an unrecorded DROP. */ + const counts = countByFile(); + const stale = Object.entries(ALLOWED) + .filter(([file, allowed]) => (counts[file] ?? 0) < allowed) + .map(([file, allowed]) => `${file}: allows ${allowed}, tree has ${counts[file] ?? 0}`); + expect(stale, `Lower the allowance to match the tree:\n${stale.join("\n")}`).toEqual([]); + }); +}); diff --git a/packages/engine/src/__tests__/self-healing-rebound-target-renamed-hold.test.ts b/packages/engine/src/__tests__/self-healing-rebound-target-renamed-hold.test.ts new file mode 100644 index 0000000000..79be366040 --- /dev/null +++ b/packages/engine/src/__tests__/self-healing-rebound-target-renamed-hold.test.ts @@ -0,0 +1,139 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-23:59: +THE REBOUND TARGET, not the guard that selects the card. + +`self-healing.ts` held 18 `moveTask(task.id, "todo", …)` calls — every one a RECOVERY. A move target +is an ARGUMENT, not a comparison, so the lifecycle-column census never counted them and no gate ever +pointed here. The failure mode is also harder than a guard's: `moveTaskInternal` REJECTS a target the +workflow does not declare (`TransitionRejectionError: unknown-column`, documented in +`task-store/moves.ts`). So on a renamed board these sweeps did not degrade to "no rescue" — they threw, +and the strand each sweep exists to clear survived while the sweep reported failure. + +`reconcileInReviewUnmetDependencies` is driven here as the representative: it is a public entry point +with a documented contract (FN-6793), and its rebound is one of the 18. + +WHY THE DEFAULT-BOARD CASE IS THE CONTROL AND NOT AN AFTERTHOUGHT. `resolveReboundTargetForTask` +degrades to `"todo"` whenever the workflow declares no hold/intake lane, so the conversion is a no-op +on every board we ship. The control pins that; without it a regression that made the resolver always +answer `"todo"` would leave the renamed case failing and look like a fixture problem. +*/ +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import type { Task, TaskStore, WorkflowIr } from "@fusion/core"; + +import { SelfHealingManager } from "../self-healing.js"; + +const WF = "custom:wf"; + +/** An in-review card whose dependency is unmet — the FN-6793 rebound case. */ +function inReviewTask(column: string): Task { + return { + id: "FN-DEP", + title: "t", + description: "", + column, + dependencies: ["FN-BLOCKER"], + steps: [], + currentStep: 0, + log: [], + createdAt: "2026-01-01T00:00:00.000Z", + columnMovedAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + } as unknown as Task; +} + +/** The blocker it depends on, still unfinished, so the dependency stays unmet. */ +function blockerTask(column: string): Task { + return { + id: "FN-BLOCKER", + title: "blocker", + description: "", + column, + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: "2026-01-01T00:00:00.000Z", + columnMovedAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + } as unknown as Task; +} + +/** A board whose hold lane is `drafting` and whose review lane is `checking`. */ +function renamedIr(): WorkflowIr { + return { + version: "v2", + id: WF, + nodes: [], + edges: [], + columns: [ + { id: "drafting", label: "Drafting", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "building", label: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", label: "Checking", traits: [{ trait: "merge" }] }, + { id: "shipped", label: "Shipped", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; +} + +function createStore(tasks: Task[], workflowIr: WorkflowIr | undefined) { + const moveTask = vi.fn(async (_id: string, _column: string) => tasks[0]); + const selection = { workflowId: WF, stepIds: [] }; + const store = { + getSettings: vi.fn().mockResolvedValue({ autoMerge: true }), + listTasks: vi.fn(async (opts?: { column?: string }) => + opts?.column ? tasks.filter((t) => t.column === opts.column) : tasks, + ), + getTask: vi.fn(async (id: string) => tasks.find((t) => t.id === id) ?? null), + moveTask, + updateTask: vi.fn().mockResolvedValue(undefined), + logEntry: vi.fn().mockResolvedValue(undefined), + recordRunAuditEvent: vi.fn().mockResolvedValue(undefined), + getCompletionHandoffAcceptedMarker: vi.fn().mockResolvedValue(null), + getTaskWorkflowSelection: vi.fn(() => selection), + getTaskWorkflowSelectionAsync: vi.fn(async () => selection), + getWorkflowDefinition: vi.fn(async () => (workflowIr ? { ir: workflowIr } : null)), + /* The sweep selects rows with `resolveProjectColumnsForRoles`, which reads the PROJECT's workflow + definitions rather than the task's own selection. Without this the renamed card is never even + considered, and the test fails upstream of the target it is about. */ + listWorkflowDefinitions: vi.fn(async () => (workflowIr ? [{ ir: workflowIr }] : [])), + } as unknown as TaskStore; + return { store, moveTask }; +} + +function manager(store: TaskStore) { + return new SelfHealingManager(store, { rootDir: "/tmp/test-project" }); +} + +describe("the self-healing rebound TARGET follows the board's own hold lane", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + /* + The defect: the target was the literal `"todo"`, which this board does not declare, so the move was + rejected outright rather than rebounding the card. `drafting` collides with no legacy id, so a + surviving literal cannot pass by luck. + */ + it("rebounds an in-review card with unmet dependencies to the RENAMED hold lane", async () => { + const tasks = [inReviewTask("checking"), blockerTask("building")]; + const { store, moveTask } = createStore(tasks, renamedIr()); + + await manager(store).reconcileInReviewUnmetDependencies(); + + expect(moveTask).toHaveBeenCalled(); + expect(moveTask.mock.calls[0]?.[1]).toBe("drafting"); + }); + + /* + CONTROL. `resolveReboundTargetForTask` falls back to `"todo"` when no workflow resolves, so the + default board must be byte-identical to the pre-conversion behaviour. This is what makes the + conversion safe to land across 18 recovery paths at once. + */ + it("default vocabulary: still rebounds to `todo` when no workflow resolves", async () => { + const tasks = [inReviewTask("in-review"), blockerTask("in-progress")]; + const { store, moveTask } = createStore(tasks, undefined); + + await manager(store).reconcileInReviewUnmetDependencies(); + + expect(moveTask).toHaveBeenCalled(); + expect(moveTask.mock.calls[0]?.[1]).toBe("todo"); + }); +}); diff --git a/packages/engine/src/agent-tools.ts b/packages/engine/src/agent-tools.ts index 08c9e4c8ed..b65056f2e5 100644 --- a/packages/engine/src/agent-tools.ts +++ b/packages/engine/src/agent-tools.ts @@ -3206,9 +3206,21 @@ export function createTaskRetryTool(store: TaskStore): ToolDefinition { return { content: [{ type: "text" as const, text: `Task ${params.id} is not in a retryable state (status: ${task.status || "none"})` }], details: { taskId: params.id, currentStatus: task.status }, isError: true }; } await store.updateTask(params.id, { status: null, error: null }); - await store.moveTask(params.id, "todo"); - await store.logEntry(params.id, "Retry requested via chat tool", "Task reset to todo for retry"); - return { content: [{ type: "text" as const, text: `Retried ${params.id} → todo` }], details: { taskId: params.id, newColumn: "todo" } }; + /* + FNXC:TaskRetry 2026-07-31-23:59 (review finding on #3152 — the move resolved, the REPORT did not): + The rebound target is resolved once and reused for the move, the log line, the response text + and `details.newColumn`. Converting only the `moveTask` argument left three places still + naming `"todo"`, so on a renamed board the card correctly landed in (say) `backlog` while the + operator and the task log were both told it went to `todo` — a lie that is worse than the + original literal, because the original at least agreed with itself. + + `details.newColumn` is the one that travels: it is machine-readable output other tooling can + act on, so a wrong value there is not merely cosmetic. + */ + const retryTarget = await fusionCore.resolveReboundTargetForTask(store, params.id); + await store.moveTask(params.id, retryTarget); + await store.logEntry(params.id, "Retry requested via chat tool", `Task reset to ${retryTarget} for retry`); + return { content: [{ type: "text" as const, text: `Retried ${params.id} → ${retryTarget}` }], details: { taskId: params.id, newColumn: retryTarget } }; } catch (err: unknown) { return { content: [{ type: "text" as const, text: `ERROR: Failed to retry task: ${toolErrorMessage(err)}` }], details: {}, isError: true }; } diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 3cd3438ecf..98fc6044a5 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -30,7 +30,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, import { readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { isAbsolute, join, relative, resolve } from "node:path"; -import { type TaskMoveLanes, resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr, +import { type TaskMoveLanes, resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, resolveReboundTargetForTask, resolveArchiveTargetForTask, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr, LEGACY_COLUMN_IDS_BY_ROLE, TERMINAL_ROLES, resolveProjectColumnsForRoles, @@ -251,7 +251,7 @@ export async function archiveAsGhostBug( }); // #1411: recovery/terminal move — recoveryRehome skips order-derived adjacency // so a custom-workflow card can always reach the terminal column. - await store.moveTask(taskId, "archived", { moveSource: "engine", recoveryRehome: true }); + await store.moveTask(taskId, await resolveArchiveTargetForTask(store, taskId), { moveSource: "engine", recoveryRehome: true }); } async function classifyOwnedLandedEvidenceForSelfHealing(rootDir: string, task: Task, mergeTargetBranch: string): Promise { @@ -2053,7 +2053,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { await this.store.updateTask(taskId, parkUpdate); try { - await this.store.moveTask(taskId, "todo", { + await this.store.moveTask(taskId, await resolveReboundTargetForTask(this.store, taskId), { preserveProgress: true, preserveStatus: true, // #1411: backward recovery — skip order-derived adjacency. @@ -3800,7 +3800,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { status: null, error: null, }); - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { moveSource: "engine", // #1411: backward recovery — skip order-derived adjacency. recoveryRehome: true, @@ -4088,7 +4088,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { signalReason: liveExecutionSignal.reason, }, }); - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { moveSource: "engine", recoveryRehome: true, preserveProgress: true, @@ -4263,7 +4263,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { if (!reviewProof?.ok) { await this.emitBackwardMoveNoAction(task, "reclaim-self-owned-branch-conflict", "task:reclaim-self-owned-branch-conflict-no-action", reviewProof!); } else { - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { moveSource: "engine", // #1411: backward recovery — skip order-derived adjacency. recoveryRehome: true, @@ -4368,7 +4368,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { if (!reviewProof?.ok) { await this.emitBackwardMoveNoAction(task, "reclaim-self-owned-branch-conflict", "task:reclaim-self-owned-branch-conflict-no-action", reviewProof!); } else { - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { moveSource: "engine", // #1411: backward recovery — skip order-derived adjacency. recoveryRehome: true, @@ -4463,7 +4463,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { const idleAnchor = task.executionStartedAt ?? task.columnMovedAt ?? task.updatedAt; const idleAnchorMs = Date.parse(idleAnchor ?? ""); const idleMs = Number.isFinite(idleAnchorMs) ? Math.max(0, Date.now() - idleAnchorMs) : null; - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { moveSource: "engine", // #1411: backward recovery — skip order-derived adjacency. recoveryRehome: true, @@ -4532,7 +4532,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { if (!reviewProof?.ok) { await this.emitBackwardMoveNoAction(task, "reclaim-self-owned-branch-conflict", "task:reclaim-self-owned-branch-conflict-no-action", reviewProof!); } else { - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { moveSource: "engine", // #1411: backward recovery — skip order-derived adjacency. recoveryRehome: true, @@ -5619,7 +5619,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { continue; } - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, preserveResumeState: true, @@ -6416,7 +6416,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { continue; } - await this.store.moveTask(holder.id, "todo", { + await this.store.moveTask(holder.id, await resolveReboundTargetForTask(this.store, holder.id), { preserveProgress: true, preserveWorktree: true, preserveResumeState: true, @@ -6679,7 +6679,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { } try { - await this.store.moveTask(task.id, "todo", { + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, preserveWorktree: true, preserveResumeState: true, @@ -7768,7 +7768,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { continue; } // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); continue; } @@ -7831,7 +7831,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { lane: "self-healing-finalize-no-op-review", }); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); recovered++; continue; } @@ -7851,7 +7851,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { baseRef: classification.baseRef, }); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); recovered++; continue; } @@ -7876,7 +7876,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence { await this.store.logEntry(task.id, `Auto-finalized no-op (proven): start point on ${classification.baseRef}; modifiedFiles cleared`); } - const movedTask = await this.store.moveTask(task.id, "done"); + const completeLane = (await resolveTaskLifecycleColumns(this.store, task.id))?.complete ?? "done"; +const movedTask = await this.store.moveTask(task.id, completeLane); this.emitTaskMerged(movedTask, { mergeConfirmed: true }); recovered++; } @@ -8635,7 +8636,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { "Auto-recovered: in-review task still had incomplete steps — moved back to todo for retry", ); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); log.log(`Recovered stale incomplete review task ${task.id}: moved back to todo`); recovered++; } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err); @@ -9123,7 +9124,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { "Auto-recovered: in-review task idle past stuck-task timeout — kicked back to todo", ); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); log.log(`Kicked ghost review task ${task.id} back to todo`); recovered++; } catch (err: unknown) { @@ -9517,7 +9518,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence { mergeDetails, }); await this.recordSelfHealingBranchGroupMemberLanding(task, mergeTarget, "recover-interrupted-merging"); - const movedTask = await this.store.moveTask(task.id, "done"); + const completeLane = (await resolveTaskLifecycleColumns(this.store, task.id))?.complete ?? "done"; +const movedTask = await this.store.moveTask(task.id, completeLane); this.emitTaskMerged(movedTask, { mergeConfirmed: true }); await this.cleanupInterruptedMergeArtifacts(task); await this.store.logEntry( @@ -10722,7 +10724,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence { mergeDetails, }); await this.recordSelfHealingBranchGroupMemberLanding(task, mergeTarget, "recover-stuck-merge-deadlocks"); - const movedTask = await this.store.moveTask(task.id, "done"); + const completeLane = (await resolveTaskLifecycleColumns(this.store, task.id))?.complete ?? "done"; +const movedTask = await this.store.moveTask(task.id, completeLane); this.emitTaskMerged(movedTask, { mergeConfirmed: true }); await this.cleanupInterruptedMergeArtifacts(task); @@ -10962,7 +10965,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence { mergeDetails, }); await this.recordSelfHealingBranchGroupMemberLanding(task, mergeTarget, "recover-orphan-only-scope-violations"); - const movedTask = await this.store.moveTask(task.id, "done"); + const completeLane = (await resolveTaskLifecycleColumns(this.store, task.id))?.complete ?? "done"; +const movedTask = await this.store.moveTask(task.id, completeLane); this.emitTaskMerged(movedTask, { mergeConfirmed: true }); await this.store.logEntry( task.id, @@ -11226,7 +11230,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence { }); const worktreeHint = task.worktree; await this.recordSelfHealingBranchGroupMemberLanding(task, mergeTarget, "recover-already-merged-review"); - const movedTask = await this.store.moveTask(task.id, "done"); + const completeLane = (await resolveTaskLifecycleColumns(this.store, task.id))?.complete ?? "done"; +const movedTask = await this.store.moveTask(task.id, completeLane); this.emitTaskMerged(movedTask, { mergeConfirmed: true }); await this.store.logEntry( task.id, @@ -11876,7 +11881,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence { // merger.ts completeTask() and project-engine.ts auto-merge already-confirmed path. await this.store.updateTask(task.id, { status: null, error: null, paused: false }); await this.recordSelfHealingBranchGroupMemberLanding(task, mergeTarget, "recover-branch-misbound-in-review"); - const movedTask = await this.store.moveTask(task.id, "done"); + const completeLane = (await resolveTaskLifecycleColumns(this.store, task.id))?.complete ?? "done"; +const movedTask = await this.store.moveTask(task.id, completeLane); this.emitTaskMerged(movedTask, { mergeConfirmed: true }); await this.store.logEntry( task.id, @@ -12767,7 +12773,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { }, }); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); recovered++; } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err); @@ -13848,7 +13854,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { "Auto-recovered no-progress no-task_done failure — clean worktree, moved back to todo", ); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { moveSource: "engine", recoveryRehome: true }); recovered++; } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err); log.error(`Failed to recover no-progress no-task_done failure ${task.id}: ${errorMessage}`); @@ -14137,7 +14143,7 @@ export class SelfHealingManager extends SelfHealingGitEvidence { `Auto-retry ${nextCount}/${MAX_TASK_DONE_RETRIES}: agent finished without fn_task_done — requeuing to todo to resume partial work`, ); // #1411: backward recovery — skip order-derived adjacency. - await this.store.moveTask(task.id, "todo", { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id), { preserveProgress: true, moveSource: "engine", recoveryRehome: true }); recovered++; } catch (err: unknown) { const errorMessage = err instanceof Error ? err.message : String(err); diff --git a/packages/engine/vitest.config.ts b/packages/engine/vitest.config.ts index 57a0e43146..6dac85b8bd 100644 --- a/packages/engine/vitest.config.ts +++ b/packages/engine/vitest.config.ts @@ -200,6 +200,28 @@ export default defineConfig({ same number. */ "src/__tests__/legacy-column-literal-census.test.ts", + /* + FNXC:EngineTests 2026-07-31-23:59: + THE MOVE-TARGET RATCHET BELONGS HERE FOR THE SAME REASON THE CENSUS RATCHET ABOVE DOES, + and the argument is stronger for this one. + + The census counts COMPARISONS; a move target is an ARGUMENT, so nothing above sees it. And + the failure mode is harder than a stale guard's: `moveTaskInternal` REJECTS a target the + workflow does not declare (`TransitionRejectionError: unknown-column`), so a regression + here THROWS on a renamed board instead of degrading — in recovery paths, which are the + ones that run when something has already gone wrong. + + Outside the gate this repeats the pattern this program keeps paying for: a correct signal + that fires in a non-blocking run while the PR merges anyway. That happened to my own + #3114 this week — `check-inert-sync-lanes` flagged it correctly and it landed regardless. + + Admission evidence, on the same terms as the entries around it: pure computation over one + `git ls-files` plus file reads. No store, no network, no timers, no subprocess beyond that + one call. Deterministic by construction — same tree, same counts. Measured 509ms / 508ms + across runs, 12 tests, after memoising the corpus scan (it was 800ms when each case + re-read every file). + */ + "src/__tests__/no-legacy-move-targets.test.ts", "src/__tests__/merger-merge-lifecycle.test.ts", "src/__tests__/merger-conflict-resolution.test.ts", "src/__tests__/merger-diff-scope.test.ts",