diff --git a/packages/engine/src/__tests__/scheduler-renamed-dependency-and-review-lanes.test.ts b/packages/engine/src/__tests__/scheduler-renamed-dependency-and-review-lanes.test.ts new file mode 100644 index 0000000000..d6c26086a7 --- /dev/null +++ b/packages/engine/src/__tests__/scheduler-renamed-dependency-and-review-lanes.test.ts @@ -0,0 +1,494 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { Scheduler } from "../scheduler.js"; +import type { Settings, Task, TaskStore, WorkflowIr } from "@fusion/core"; + +/* +The hydration path bails early unless the root dir resolves to a real GitHub repo. That guard is not +what these cases are about, so it is stubbed — leaving it live made the assertion pass vacuously +(BOTH vocabularies hydrated nothing, so "they match" was true for the wrong reason). +*/ +vi.mock("@fusion/core", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getCurrentRepo: () => ({ owner: "acme", repo: "widgets" }) }; +}); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-19:05: +Two scheduler decisions that were keyed on column-id literals, on a RENAMED board. + +DEFECT 1 — DEPENDENCY SATISFACTION. `isLegacyDependencySatisfied` answered "is this dependency +finished?" with `column === "done" || "in-review" || "archived"`. On a board whose complete column is +`shipped`, a FINISHED dependency matched none of the three, so `getUnmetSchedulingDependencies` +reported it unmet and every dependent stayed queued with `blockedBy` set — permanently, because the +dependency can never move anywhere that would satisfy the literal. This is the expensive direction to +be wrong in: work stops and nothing rescues it. + +DEFECT 2 — THE REVIEW HALF OF THE FILE-SCOPE LEASE. The sweep that builds `activeScopes` had its wip +half converted to traits (2026-07-30-16:30, scheduler-renamed-wip-file-scope-lease.test.ts) while its +review half still read `column === "in-review"`. On a renamed board no review card entered the +registry, so a merging card's worktree files read as FREE and an overlapping candidate dispatched on +top of them. Half a fix leaves one registry with two disagreeing halves. + +WHY THESE ARE DIFFERENTIAL. Each scenario runs twice against the same workflow SHAPE under two +vocabularies with identical traits; only the ids differ. Any behavioural difference between the runs +is attributable to a surviving column-id literal and nothing else. The default-vocabulary run is the +control: it passes before and after, so a change that breaks these paths generally cannot hide here. +No renamed id collides with a legacy literal, so a surviving `=== "done"` cannot pass by luck. +*/ + +const WF = "custom:renamed-lanes"; + +const PASSED_PLAN_REVIEW = { + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + status: "passed" as const, + source: "node" as const, + phase: "pre-merge" as const, +}; + +interface Names { + hold: string; + wip: string; + review: string; + complete: string; +} + +const DEFAULT_NAMES: Names = { hold: "todo", wip: "in-progress", review: "in-review", complete: "done" }; +const RENAMED_NAMES: Names = { hold: "drafting", wip: "building", review: "checking", complete: "shipped" }; + +function makeTask(overrides: Partial = {}): Task { + return { + id: "FN-001", + title: "task", + description: "", + column: "todo", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + workflowStepResults: [PASSED_PLAN_REVIEW], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + ...overrides, + } as Task; +} + +/** One workflow shape; the traits are identical under both vocabularies. */ +function ir(names: Names): WorkflowIr { + return { + version: "v2", + id: WF, + nodes: [], + edges: [], + columns: [ + { id: names.hold, label: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: names.wip, label: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: names.review, label: "Review", traits: [{ trait: "merge" }, { trait: "human-review" }] }, + { id: names.complete, label: "Complete", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; +} + +function createStore( + tasks: Task[], + scopes: Record, + workflowIr: WorkflowIr, + settings: Partial = {}, +): TaskStore { + const resolved = { maxConcurrent: 10, maxWorktrees: 10, groupOverlappingFiles: true, ...settings }; + const selection = { workflowId: WF, stepIds: [] }; + const updateTask = vi.fn(async (id: string, patch: Partial) => { + const task = tasks.find((candidate) => candidate.id === id); + if (task) Object.assign(task, patch); + return task as Task; + }); + const moveTask = vi.fn(async (id: string, column: Task["column"], _opts?: Record) => { + const task = tasks.find((candidate) => candidate.id === id); + if (task) task.column = column; + return task as Task; + }); + const moveTaskIf = vi.fn(async ( + id: string, + column: Task["column"], + predicate: (live: Task) => boolean | Promise, + opts?: Record, + ) => { + const task = tasks.find((candidate) => candidate.id === id); + if (!task) return { task: task as unknown as Task, moved: false }; + if (!(await predicate(task)) || task.column === column) return { task, moved: false }; + const movedTask = await moveTask(id, column, opts as never); + return { task: movedTask ?? task, moved: true }; + }); + + return { + listTasks: vi.fn(async () => tasks), + getSettings: vi.fn(async () => resolved), + updateSettings: vi.fn(async () => resolved), + parseFileScopeFromPrompt: vi.fn(async (id: string) => scopes[id] ?? []), + updateTask, + moveTask, + moveTaskIf, + getTask: vi.fn(async (id: string) => tasks.find((task) => task.id === id) ?? null), + logEntry: vi.fn(async () => undefined), + getRootDir: vi.fn(() => "/tmp/project"), + getTasksDir: vi.fn(() => "/tmp/project/.fusion/tasks"), + on: vi.fn(), + off: vi.fn(), + recordRunAuditEvent: vi.fn(async () => undefined), + getCompletionHandoffAcceptedMarker: vi.fn(async () => null), + getTaskWorkflowSelection: vi.fn(() => selection), + getTaskWorkflowSelectionAsync: vi.fn(async () => selection), + getWorkflowDefinition: vi.fn(async () => ({ ir: workflowIr })), + } as unknown as TaskStore; +} + +async function runSweep(store: TaskStore): Promise { + const scheduler = new Scheduler(store); + (scheduler as unknown as { running: boolean }).running = true; + await scheduler.schedule(); +} + +/** + * A ready candidate in the hold column whose single dependency already rests in the board's COMPLETE + * column. Reported in role terms so both runs are directly comparable. + */ +async function finishedDependencyScenario(names: Names) { + const tasks = [ + makeTask({ id: "FN-DEP", column: names.complete }), + makeTask({ id: "FN-CAND", column: names.hold, priority: "urgent", dependencies: ["FN-DEP"] }), + ]; + const store = createStore(tasks, {}, ir(names)); + await runSweep(store); + + const dispatched = (store.moveTask as ReturnType).mock.calls.some( + (call: unknown[]) => call[0] === "FN-CAND" && call[1] === names.wip, + ); + const queuedAsBlocked = (store.updateTask as ReturnType).mock.calls.some( + (call: unknown[]) => + call[0] === "FN-CAND" && (call[1] as Partial | undefined)?.blockedBy === "FN-DEP", + ); + + return { dispatched, queuedAsBlocked }; +} + +/** The paired negative: an UNFINISHED dependency must still block, on both vocabularies. */ +async function unfinishedDependencyScenario(names: Names) { + const tasks = [ + makeTask({ id: "FN-DEP", column: names.wip }), + makeTask({ id: "FN-CAND", column: names.hold, priority: "urgent", dependencies: ["FN-DEP"] }), + ]; + const store = createStore(tasks, {}, ir(names)); + await runSweep(store); + + const dispatched = (store.moveTask as ReturnType).mock.calls.some( + (call: unknown[]) => call[0] === "FN-CAND" && call[1] === names.wip, + ); + const queuedAsBlocked = (store.updateTask as ReturnType).mock.calls.some( + (call: unknown[]) => + call[0] === "FN-CAND" && (call[1] as Partial | undefined)?.blockedBy === "FN-DEP", + ); + + return { dispatched, queuedAsBlocked }; +} + +/** + * An occupant in the REVIEW column holding a worktree and a file scope, and an overlapping candidate + * in the hold column. The review occupant must hold its file-scope lease. + */ +async function reviewLeaseScenario(names: Names) { + const tasks = [ + makeTask({ id: "FN-OCC", column: names.review, worktree: "/tmp/project/.worktrees/FN-OCC" }), + makeTask({ id: "FN-CAND", column: names.hold, priority: "urgent" }), + ]; + const store = createStore( + tasks, + { + "FN-OCC": ["packages/engine/src/scheduler.ts"], + "FN-CAND": ["packages/engine/src/scheduler.ts"], + }, + ir(names), + ); + await runSweep(store); + + const queuedOnLease = (store.updateTask as ReturnType).mock.calls.some( + (call: unknown[]) => + call[0] === "FN-CAND" + && (call[1] as Partial | undefined)?.overlapBlockedBy === "FN-OCC", + ); + const dispatched = (store.moveTask as ReturnType).mock.calls.some( + (call: unknown[]) => call[0] === "FN-CAND" && call[1] === names.wip, + ); + + return { queuedOnLease, dispatched }; +} + +describe("scheduler dependency satisfaction on a RENAMED board", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(Scheduler.prototype as never as { validateTaskFilesystem: () => unknown }, "validateTaskFilesystem") + .mockResolvedValue({ valid: true } as never); + }); + + /* Control. Passes before and after; a general break in dependency gating fails here too. */ + it("default vocabulary: a dependency in the complete column satisfies its dependent", async () => { + const outcome = await finishedDependencyScenario(DEFAULT_NAMES); + + expect(outcome.dispatched).toBe(true); + expect(outcome.queuedAsBlocked).toBe(false); + }); + + /* + The defect. Before the fix `shipped` matched none of `done`/`in-review`/`archived`, so the finished + dependency read as unmet and the dependent was parked `blockedBy` with nothing able to clear it. + */ + it("renamed vocabulary: a dependency in the complete column satisfies its dependent", async () => { + const outcome = await finishedDependencyScenario(RENAMED_NAMES); + + expect(outcome.dispatched).toBe(true); + expect(outcome.queuedAsBlocked).toBe(false); + }); + + /* Satisfaction must not degrade into "always satisfied" — the direction the fix could overshoot. */ + it("an UNFINISHED dependency still blocks, under both vocabularies", async () => { + const [byDefault, renamed] = await Promise.all([ + unfinishedDependencyScenario(DEFAULT_NAMES), + unfinishedDependencyScenario(RENAMED_NAMES), + ]); + + expect(byDefault.dispatched).toBe(false); + expect(renamed.dispatched).toBe(false); + expect(renamed).toEqual(byDefault); + }); + + /* States the invariant directly: the two vocabularies must be indistinguishable. */ + it("both vocabularies reach the SAME outcome — no column-id literal survives on this path", async () => { + const [byDefault, renamed] = await Promise.all([ + finishedDependencyScenario(DEFAULT_NAMES), + finishedDependencyScenario(RENAMED_NAMES), + ]); + + expect(renamed).toEqual(byDefault); + }); +}); + +describe("scheduler file-scope lease is held for a RENAMED review column", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(Scheduler.prototype as never as { validateTaskFilesystem: () => unknown }, "validateTaskFilesystem") + .mockResolvedValue({ valid: true } as never); + }); + + /* Control. */ + it("default vocabulary: the candidate overlapping a review card is queued, not dispatched", async () => { + const outcome = await reviewLeaseScenario(DEFAULT_NAMES); + + expect(outcome.queuedOnLease).toBe(true); + expect(outcome.dispatched).toBe(false); + }); + + /* + The defect. Before the fix the review pass skipped the occupant (its column is not literally + `in-review`), so its merging worktree's files read as free and the candidate dispatched on top. + */ + it("renamed vocabulary: the candidate overlapping a review card is queued, not dispatched", async () => { + const outcome = await reviewLeaseScenario(RENAMED_NAMES); + + expect(outcome.queuedOnLease).toBe(true); + expect(outcome.dispatched).toBe(false); + }); + + it("both vocabularies reach the SAME outcome — no column-id literal survives on this path", async () => { + const [byDefault, renamed] = await Promise.all([ + reviewLeaseScenario(DEFAULT_NAMES), + reviewLeaseScenario(RENAMED_NAMES), + ]); + + expect(renamed).toEqual(byDefault); + }); +}); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-20:40: +Three more scheduler decisions that were keyed on column-id literals. + +DEFECT 3 — PR-MONITOR STARTUP HYDRATION. `configurePrMonitoring` rehydrated watchers behind +`column !== "in-review"`. On a renamed board an engine restart hydrated NOTHING, so every open PR +silently stopped being watched — no error, just a background watcher that never runs again. + +DEFECT 4 — BASE-BRANCH STACKING. `resolveBaseBranch` decides whether a starting task stacks its +branch on a predecessor still in review. Keyed on the literal, a renamed board started the task from +HEAD instead, so it silently rebuilt work its predecessor had already done. + +DEFECT 5 — MISSION COMPLETION ADVANCE. `handleMissionTaskMove` reconciled the feature status through +a trait-aware path and THEN gated the follow-on completion on `toColumn === "done"`. On a renamed +board the two halves of one handler disagreed about whether the same move was a completion, so the +roadmap showed the work finished while mission execution stalled. +*/ + +async function baseBranchScenario(names: Names) { + const tasks = [ + makeTask({ id: "FN-PRED", column: names.review, worktree: "/tmp/project/.worktrees/FN-PRED" }), + makeTask({ id: "FN-CAND", column: names.hold, priority: "urgent", dependencies: ["FN-PRED"] }), + ]; + const store = createStore(tasks, {}, ir(names)); + const started: Array<{ id: string; baseBranch: string | null }> = []; + const scheduler = new Scheduler(store, { + onTaskStart: ((task: Task, _worktree: string, baseBranch: string | null) => { + started.push({ id: task.id, baseBranch }); + }) as never, + } as never); + (scheduler as unknown as { running: boolean }).running = true; + await scheduler.schedule(); + + const resolved = (scheduler as unknown as { + resolveBaseBranch: (t: Task, all: Task[], isReview: (c: Task) => boolean) => string | null; + }); + return { started, resolved, tasks }; +} + +describe("scheduler base-branch stacking on a RENAMED board", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(Scheduler.prototype as never as { validateTaskFilesystem: () => unknown }, "validateTaskFilesystem") + .mockResolvedValue({ valid: true } as never); + }); + + /* + The predicate is now REQUIRED, so the compiler asks the question at the one call site. This asserts + the behaviour that matters: a review-lane predecessor with a worktree supplies the base branch, and + the answer is identical under both vocabularies. + */ + it("stacks on a review-lane predecessor under BOTH vocabularies", async () => { + const outcomes = await Promise.all([DEFAULT_NAMES, RENAMED_NAMES].map(async (names) => { + const { resolved, tasks } = await baseBranchScenario(names); + const reviewIds = new Set([tasks[0].id]); + return resolved.resolveBaseBranch( + tasks[1], + tasks, + (candidate: Task) => reviewIds.has(candidate.id), + ); + })); + + expect(outcomes[0]).not.toBeNull(); + expect(outcomes[1]).toEqual(outcomes[0]); + }); + + /* The paired negative: no review-lane predecessor means start from HEAD, not from something else. */ + it("returns null when no predecessor is in the review lane", async () => { + const { resolved, tasks } = await baseBranchScenario(RENAMED_NAMES); + + expect(resolved.resolveBaseBranch(tasks[1], tasks, () => false)).toBeNull(); + }); +}); + +describe("scheduler PR-monitor hydration on a RENAMED board", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(Scheduler.prototype as never as { validateTaskFilesystem: () => unknown }, "validateTaskFilesystem") + .mockResolvedValue({ valid: true } as never); + }); + + async function hydrationScenario(names: Names) { + const tasks = [ + makeTask({ + id: "FN-PR", + column: names.review, + prInfo: { number: 7, url: "https://example.invalid/pr/7", branch: "fusion/FN-PR" }, + } as Partial), + ]; + const store = createStore(tasks, {}, ir(names)); + const scheduler = new Scheduler(store); + const startMonitoring = vi.fn(); + scheduler.configurePrMonitoring({ + prMonitor: { startMonitoring, getTrackedPrs: () => new Map(), updatePrInfo: vi.fn() } as never, + }); + // The hydration is intentionally off the hot path (`void ... .then`), so let it settle. + await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); + return startMonitoring.mock.calls.map((call) => call[0]); + } + + it("hydrates the review-lane PR under BOTH vocabularies", async () => { + const [byDefault, renamed] = await Promise.all([ + hydrationScenario(DEFAULT_NAMES), + hydrationScenario(RENAMED_NAMES), + ]); + + expect(byDefault).toEqual(["FN-PR"]); + expect(renamed).toEqual(byDefault); + }); +}); + +describe("scheduler mission completion advance on a RENAMED board", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + /** + * Drives `handleMissionTaskMove` directly with a task whose linked feature exists, and reports + * whether the follow-on completion fired. The feature-status half of this handler already resolves + * traits; this asserts the completion half agrees with it. + */ + async function missionMoveScenario(names: Names) { + const task = makeTask({ id: "FN-M", column: names.complete, sliceId: "slice-1" } as Partial); + const store = createStore([task], {}, ir(names)); + const feature = { id: "feat-1", sliceId: "slice-1", status: "in-progress", taskId: "FN-M", title: "task" }; + const missionStore = { + listFeatures: vi.fn(async () => [feature]), + listAssertionsForFeature: vi.fn(async () => []), + updateFeatureStatus: vi.fn(async () => undefined), + getFeature: vi.fn(async () => feature), + getFeatureByTaskId: vi.fn(async () => feature), + }; + + const scheduler = new Scheduler(store, { missionStore } as never); + const completion = vi + .spyOn(scheduler as never as { handleMissionTaskCompletion: () => Promise }, "handleMissionTaskCompletion") + .mockResolvedValue(undefined as never); + + await (scheduler as unknown as { + handleMissionTaskMove: (id: string, to: string) => Promise; + }).handleMissionTaskMove("FN-M", names.complete); + + return completion.mock.calls.length; + } + + /* Control: the default vocabulary advances the mission. */ + it("default vocabulary: moving into the complete column advances mission execution", async () => { + expect(await missionMoveScenario(DEFAULT_NAMES)).toBe(1); + }); + + /* + The defect. Before the fix `shipped` failed `toColumn === "done"`, so the feature status was + reconciled but mission execution never advanced — the roadmap showed the work finished while the + mission stalled. + */ + it("renamed vocabulary: moving into the complete column advances mission execution", async () => { + expect(await missionMoveScenario(RENAMED_NAMES)).toBe(1); + }); + + /* The paired negative: a non-complete destination must NOT advance, under either vocabulary. */ + it("a move into the REVIEW column does not advance mission execution", async () => { + const task = makeTask({ id: "FN-M", column: RENAMED_NAMES.review, sliceId: "slice-1" } as Partial); + const store = createStore([task], {}, ir(RENAMED_NAMES)); + const feature = { id: "feat-1", sliceId: "slice-1", status: "in-progress", taskId: "FN-M", title: "task" }; + const scheduler = new Scheduler(store, { + missionStore: { + listFeatures: vi.fn(async () => [feature]), + listAssertionsForFeature: vi.fn(async () => []), + updateFeatureStatus: vi.fn(async () => undefined), + getFeature: vi.fn(async () => feature), + getFeatureByTaskId: vi.fn(async () => feature), + }, + } as never); + const completion = vi + .spyOn(scheduler as never as { handleMissionTaskCompletion: () => Promise }, "handleMissionTaskCompletion") + .mockResolvedValue(undefined as never); + + await (scheduler as unknown as { + handleMissionTaskMove: (id: string, to: string) => Promise; + }).handleMissionTaskMove("FN-M", RENAMED_NAMES.review); + + expect(completion).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index 6c6bd90434..9d0db2dfea 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -40,7 +40,7 @@ import { StaleTaskReporter } from "./stale-task-reporter.js"; import { BacklogPressureReporter } from "./backlog-pressure-reporter.js"; import { UnlinkedMissionsAdvisoryReporter } from "./unlinked-missions-advisory-reporter.js"; import { createRunAuditor, generateSyntheticRunId } from "./run-audit.js"; -import { resolveWorkflowIrForTask, resolveWorkflowIrById, resolveColumnFlags, resolveWorktreeCapacityLimit, resolveLifecycleColumns, isWipColumnRole } from "@fusion/core"; +import { resolveWorkflowIrForTask, resolveWorkflowIrById, resolveColumnFlags, resolveWorktreeCapacityLimit, resolveLifecycleColumns, isWipColumnRole, isReviewColumnRole, isCompleteColumnRole, columnsWithFlag } from "@fusion/core"; import type { ColumnRoleTraitFlags } from "@fusion/core"; import type { WorkflowIr, WorkflowIrV2 } from "@fusion/core"; import { runHoldReleaseSweep, isUnplannedForExecution, type SlotReservation } from "./hold-release.js"; @@ -228,16 +228,93 @@ export function isCoordinationOnlyTask(task: Task, scope: string[]): boolean { return isCoordinationSafeScope(scope); } -function isLegacyDependencySatisfied(dep: Task | undefined): boolean { - return !!dep && (dep.column === "done" || dep.column === "in-review" || dep.column === "archived"); +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-19:05 (fleet — scheduler dependency satisfaction): +A dependency's satisfaction is decided on ITS OWN board, because a dependency edge may cross +workflows: the dependent can sit on the default board while the dependency lives on a renamed one. + +THE TWO RULES THIS FILE ALREADY HAD, PRESERVED EXACTLY: + legacy (live) satisfied = COMPLETE or ARCHIVED or the REVIEW lane (mergeBlocker/humanReview) + marker (shadow) satisfied = COMPLETE or ARCHIVED, else the handoff marker decides + +They genuinely differ on the review lane, and the conversion does NOT reconcile them — that is a +product decision, not a vocabulary one. See the PR body: #2720 settled "satisfied = complete or +archived" for `update-task-deps.ts`, which matches the MARKER rule, so the live legacy rule here is +the broader of the two. Narrowing it silently would strand every dependent of an in-review card. + +`columns` is resolved per dependency and passed in by the caller. Omitted (or absent for a given +dependency) → the legacy literals, i.e. exactly today's behaviour, so no unconverted call site +changes meaning and an unresolvable workflow fails soft rather than reading as unsatisfied forever. +*/ +export interface DependencySatisfactionColumns { + /** COMPLETE ∪ ARCHIVED for the dependency's own workflow. */ + terminal: ReadonlySet; + /** The dependency's own review lane (mergeBlocker ∪ humanReview). */ + review: ReadonlySet; } -function isMarkerDependencySatisfied(dep: Task | undefined, markerAccepted: boolean): boolean { +/* +DELIBERATE-LITERAL — the no-metadata fallback, reviewed 2026-07-30-20:40. +Resolved columns win when present; the literal answers only when the caller could not resolve the +dependency's workflow at all. DELETING it does not "finish the conversion" — it makes an +unresolvable workflow read as `terminal.size === 0`, i.e. NEVER satisfied, which blocks every +dependent forever. That is strictly worse than the legacy behaviour it would replace. +*/ +function isTerminalDependencyColumn(dep: Task, columns: DependencySatisfactionColumns | undefined): boolean { + if (columns) return columns.terminal.has(dep.column); + return dep.column === "done" || dep.column === "archived"; +} + +/* DELIBERATE-LITERAL — same no-metadata fallback as above, reviewed 2026-07-30-20:40. */ +function isLegacyDependencySatisfied(dep: Task | undefined, columns?: DependencySatisfactionColumns): boolean { if (!dep) return false; - if (dep.column === "done" || dep.column === "archived") return true; + if (isTerminalDependencyColumn(dep, columns)) return true; + if (columns) return columns.review.has(dep.column); + return dep.column === "in-review"; +} + +function isMarkerDependencySatisfied( + dep: Task | undefined, + markerAccepted: boolean, + columns?: DependencySatisfactionColumns, +): boolean { + if (!dep) return false; + if (isTerminalDependencyColumn(dep, columns)) return true; return markerAccepted; } +/** + * FNXC:WorkflowLifecycleColumns 2026-07-30-19:05: + * Build the per-dependency column vocabulary for {@link getUnmetSchedulingDependencies}. + * + * `cache` is caller-owned for the reason documented on `resolveTaskLifecycleColumns`: a sweep over + * many dependents spanning three workflows must read three IRs, not one per dependency. A + * dependency whose workflow cannot be resolved is simply OMITTED from the map, which lands that + * dependency on the literal fallback rather than on an empty set (an empty set would read as + * "never satisfied" and block the dependent forever — the expensive direction to be wrong in). + */ +export async function resolveDependencySatisfactionColumns( + store: Parameters[0], + dependencies: readonly Task[], + cache?: Map, +): Promise> { + const resolved = new Map(); + const irCache = cache ?? new Map(); + for (const dep of dependencies) { + try { + const ir = await resolveWorkflowIrForTask(store, dep.id, irCache); + if (!ir) continue; + const terminal = new Set([...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")]); + const review = new Set([...columnsWithFlag(ir, "mergeBlocker"), ...columnsWithFlag(ir, "humanReview")]); + if (terminal.size === 0 && review.size === 0) continue; + resolved.set(dep.id, { terminal, review }); + } catch { + // Unresolvable workflow: leave it unmapped so this dependency keeps the legacy literals. + } + } + return resolved; +} + export function computeShadowLeaseParityState(mergeRequestState: string | null): { shadowExecutorLeaseApplied: boolean; shadowMergeLockApplied: boolean; @@ -269,13 +346,23 @@ export function getUnmetSchedulingDependencies( options?: { markerAcceptedByTaskId?: Map; onParityDiff?: (diff: SchedulingDependencyParityDiff) => void; + /** + * Per-dependency resolved column vocabulary from + * {@link resolveDependencySatisfactionColumns}. Omitted → the legacy literals. + */ + satisfactionColumnsByTaskId?: Map; }, ): string[] { return task.dependencies.filter((depId) => { const dep = tasks.find((candidate) => candidate.id === depId); if (!dep) return false; - const legacySatisfied = isLegacyDependencySatisfied(dep); - const markerSatisfied = isMarkerDependencySatisfied(dep, options?.markerAcceptedByTaskId?.get(depId) === true); + const satisfactionColumns = options?.satisfactionColumnsByTaskId?.get(depId); + const legacySatisfied = isLegacyDependencySatisfied(dep, satisfactionColumns); + const markerSatisfied = isMarkerDependencySatisfied( + dep, + options?.markerAcceptedByTaskId?.get(depId) === true, + satisfactionColumns, + ); if (options?.onParityDiff && legacySatisfied !== markerSatisfied) { options.onParityDiff({ taskId: task.id, @@ -357,7 +444,15 @@ export function shouldHoldActiveFileScopeLease( scheduler-renamed-wip-file-scope-lease.test.ts. */ if (task.paused || task.userPaused) return false; + /* + DELIBERATE-LITERAL — the documented default for an unconverted caller, reviewed 2026-07-30-20:40. + Both scheduler call sites now pass the resolved answer, so these defaults are dead on the + scheduler's own path; they exist for the self-healing / repair callers this predicate is shared + with. Removing them would silently change those callers' meaning, which is the coupling the + optional-parameter shape exists to avoid. + */ const isWipColumn = options?.isWipColumn ?? task.column === "in-progress"; + /* DELIBERATE-LITERAL — the review half of the same shared-caller default. */ const isReviewColumn = options?.isReviewColumn ?? task.column === "in-review"; if (isWipColumn) { return getUnmetSchedulingDependencies(task, tasks, options?.schedulingDependencyOptions).length === 0; @@ -869,6 +964,8 @@ export class Scheduler { const settings = await this.store.getSettings(); if (!settings.globalPause && !settings.enginePaused) { const todoTasks = await this.store.listTasks({ column: parked.hold, slim: true }); + /* One IR cache for the whole reconciliation, per the caller-owned-cache contract. */ + const dependencySatisfactionIrCache = new Map(); for (const dependent of todoTasks) { const mentionsCompletedTask = dependent.dependencies.includes(task.id); const currentlyBlockedByCompletedTask = dependent.blockedBy === task.id; @@ -884,17 +981,25 @@ export class Scheduler { const dependencyTasks = (await Promise.all( dependent.dependencies.map((dependencyId) => this.store.getTask(dependencyId).catch(() => null)), )).filter((candidate) => candidate !== null); + const satisfactionColumnsByTaskId = await resolveDependencySatisfactionColumns( + this.store, + [task, ...dependencyTasks], + dependencySatisfactionIrCache, + ); const unresolvedDeps = getUnmetSchedulingDependencies( dependent, [dependent, task, ...dependencyTasks], - markerAcceptedByTaskId - ? { - markerAcceptedByTaskId, - onParityDiff: (diff) => { - this.emitDependencyParityDiff(diff); - }, - } - : undefined, + { + satisfactionColumnsByTaskId, + ...(markerAcceptedByTaskId + ? { + markerAcceptedByTaskId, + onParityDiff: (diff: SchedulingDependencyParityDiff) => { + this.emitDependencyParityDiff(diff); + }, + } + : {}), + }, ); try { @@ -1084,6 +1189,8 @@ export class Scheduler { const todoTasks = await this.store.listTasks({ column: deletedParked.hold, slim: true }); const inProgressTasks = await this.store.listTasks({ column: "in-progress", slim: true }); const dependents = [...todoTasks, ...inProgressTasks]; + /* One IR cache for the whole reconciliation, per the caller-owned-cache contract. */ + const deletedDependencyIrCache = new Map(); for (const dependent of dependents) { const mentionsDeletedTask = dependent.dependencies.includes(task.id); @@ -1096,17 +1203,25 @@ export class Scheduler { const dependencyTasks = (await Promise.all( dependent.dependencies.map((dependencyId) => this.store.getTask(dependencyId).catch(() => null)), )).filter((candidate) => candidate !== null); + const satisfactionColumnsByTaskId = await resolveDependencySatisfactionColumns( + this.store, + dependencyTasks, + deletedDependencyIrCache, + ); const unresolvedDeps = getUnmetSchedulingDependencies( dependent, [dependent, ...dependencyTasks], - markerAcceptedByTaskId - ? { - markerAcceptedByTaskId, - onParityDiff: (diff) => { - this.emitDependencyParityDiff(diff); - }, - } - : undefined, + { + satisfactionColumnsByTaskId, + ...(markerAcceptedByTaskId + ? { + markerAcceptedByTaskId, + onParityDiff: (diff: SchedulingDependencyParityDiff) => { + this.emitDependencyParityDiff(diff); + }, + } + : {}), + }, ); try { @@ -1416,13 +1531,37 @@ export class Scheduler { return; } + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-20:40 (fleet — PR-monitor startup hydration): + Startup rehydration of PR monitoring resolves the review lane per task. + + Keyed on `column !== "in-review"`, a renamed board hydrated NOTHING on engine restart: every + open PR silently stopped being watched, so merges, review comments and closures went unnoticed + until something else moved the card. The failure is invisible precisely because it is a missing + background watcher rather than an error. + + This is a one-shot startup pass over an already-fetched list, and it is already off the hot path + (`void ... .then(...)`), so a per-task resolution with a shared IR cache costs one read per + distinct workflow. Fail-soft: an unresolvable workflow falls back to the legacy literal via + `isReviewColumnRole`'s documented no-metadata behaviour rather than dropping the watcher. + */ void this.store.listTasks({ slim: true, includeArchived: false, startupMemo: true }) - .then((tasks) => { + .then(async (tasks) => { const repo = getCurrentRepo(this.store.getRootDir()); if (!repo) return; + const hydrationIrCache = new Map(); for (const task of tasks) { - if (task.column !== "in-review" || !task.prInfo) continue; + if (!task.prInfo) continue; + let flags: ColumnRoleTraitFlags | undefined; + try { + const ir = await resolveWorkflowIrForTask(this.store, task.id, hydrationIrCache); + const column = (ir as WorkflowIrV2).columns?.find((candidate) => candidate.id === task.column); + if (column) flags = resolveColumnFlags(column); + } catch { + // Unresolvable workflow: `isReviewColumnRole` falls back to the legacy id below. + } + if (!isReviewColumnRole(flags, task.column)) continue; options.prMonitor!.startMonitoring(task.id, repo.owner, repo.repo, task.prInfo); } }) @@ -1439,20 +1578,38 @@ export class Scheduler { * or `null` if the task should start from HEAD (default). * * Priority: explicit dep in-review (first with worktree) > blockedBy in-review. + * + * FNXC:WorkflowLifecycleColumns 2026-07-30-20:40 (fleet — scheduler base-branch stacking): + * `isReviewColumn` is a REQUIRED resolved answer, not an optional one with a literal default. + * + * This decides whether a starting task stacks its branch on a predecessor still in review. Keyed + * on the literal, a renamed board resolved every predecessor as "not in review" and the task + * started from HEAD instead of from its dependency's branch — so the dependent silently rebuilt + * work its predecessor had already done, and the first merge of the two conflicted. That is a + * quiet wrong answer, not a crash, which is why it would survive unnoticed. + * + * Required rather than optional because this method has exactly ONE caller, inside + * `runHoldReleaseSweepPass`, which has already resolved per-task trait flags for the whole board. + * An optional parameter with a literal default would let a future caller reintroduce the bug by + * omission; making it required means the compiler asks the question. */ - private resolveBaseBranch(task: Task, allTasks: Task[]): string | null { - // Check explicit dependencies for in-review tasks with worktrees + private resolveBaseBranch( + task: Task, + allTasks: Task[], + isReviewColumn: (candidate: Task) => boolean, + ): string | null { + // Check explicit dependencies for review-lane tasks with worktrees for (const depId of task.dependencies) { const dep = allTasks.find((t) => t.id === depId); - if (dep && dep.column === "in-review" && dep.worktree) { + if (dep && isReviewColumn(dep) && dep.worktree) { return resolveTaskWorkingBranch(dep); } } - // Check implicit blockedBy for in-review task with worktree + // Check implicit blockedBy for a review-lane task with worktree if (task.blockedBy) { const blocker = allTasks.find((t) => t.id === task.blockedBy); - if (blocker && blocker.column === "in-review" && blocker.worktree) { + if (blocker && isReviewColumn(blocker) && blocker.worktree) { return resolveTaskWorkingBranch(blocker); } } @@ -1610,6 +1767,9 @@ export class Scheduler { */ private async renewActiveMissionSymbolLocks(tasks: Task[]): Promise { for (const task of tasks) { + /* DELIBERATE-LITERAL — seeds the trait resolution immediately below, which overwrites it + whenever the workflow resolves; the literal survives only as the documented fallback for an + unresolvable workflow. Reviewed 2026-07-30-20:40. */ let isImplementationColumn = task.column === "in-progress"; try { const ir = await resolveWorkflowIrForTask(this.store, task.id); @@ -1722,6 +1882,18 @@ export class Scheduler { columnFlagsByWorkflowId.get(workflowIdByTaskId.get(task.id) ?? "builtin:coding")?.get(task.column); const isWipColumnTask = (task: Task): boolean => isWipColumnRole(columnFlagsForTask(task), task.column); + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-19:05: + The review-lane twin of `isWipColumnTask`, over the SAME resolved flags map. + + The file-scope lease sweep below had a renamed-board hole in the shape 2026-07-30-16:30 already + fixed for the wip half: the review pass gated on the literal `column === "in-review"`, so on a + renamed board no review card entered `activeScopes` and its worktree's files read as free while + the merge was still in flight. Fixing only the wip half left the two halves of one registry + disagreeing. Same predicate, same flags, so they cannot drift. + */ + const isReviewColumnTask = (task: Task): boolean => + isReviewColumnRole(columnFlagsForTask(task), task.column); const wipTaskIds = tasks.filter(isWipColumnTask).map((task) => task.id); let reservedWorktreeSlots = wipTaskIds.length; let reservedConcurrentSlots = reservedWorktreeSlots; @@ -1755,14 +1927,27 @@ export class Scheduler { markerAcceptedByTaskId.set(depId, (await this.store.getCompletionHandoffAcceptedMarker(depId)) !== null); } } + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-19:05: + Resolved ONCE per sweep, over the dependency cards actually referenced — not per dependent, + and not over the whole board. `tasks` is already in memory, so this adds workflow-IR reads + bounded by the number of distinct workflows, not by the number of dependency edges. + */ + const referencedDependencyIds = new Set(tasks.flatMap((candidate) => candidate.dependencies)); + const satisfactionColumnsByTaskId = await resolveDependencySatisfactionColumns( + this.store, + tasks.filter((candidate) => referencedDependencyIds.has(candidate.id)), + new Map(), + ); const schedulingDependencyOptions = mergeShadowEnabled ? { + satisfactionColumnsByTaskId, markerAcceptedByTaskId, onParityDiff: (diff: SchedulingDependencyParityDiff) => { this.emitDependencyParityDiff(diff); }, } - : undefined; + : { satisfactionColumnsByTaskId }; if (settings.groupOverlappingFiles) { for (const task of tasks) { @@ -1792,16 +1977,18 @@ export class Scheduler { const reviewHandoffMarkerMap = new Map(); if (settings.mergeRequestContractShadowEnabled === true) { for (const t of tasks) { - if (t.column === "in-review") { + if (isReviewColumnTask(t)) { reviewHandoffMarkerMap.set(t.id, (await this.store.getCompletionHandoffAcceptedMarker(t.id)) !== null); } } } const inReviewWithWorktree = tasks.filter( - (task) => task.column === "in-review" && shouldHoldActiveFileScopeLease(task, tasks, { + (task) => isReviewColumnTask(task) && shouldHoldActiveFileScopeLease(task, tasks, { mergeRequestContractShadowEnabled: settings.mergeRequestContractShadowEnabled, handoffAccepted: reviewHandoffMarkerMap.get(task.id) ?? false, schedulingDependencyOptions, + /* Both halves of the gate agree: the filter said review, so the predicate is told so. */ + isReviewColumn: true, }), ); for (const task of inReviewWithWorktree) { @@ -2328,7 +2515,7 @@ export class Scheduler { } dispatchPrepByTaskId.set(task.id, { - baseBranch: this.resolveBaseBranch(freshTask, tasks), + baseBranch: this.resolveBaseBranch(freshTask, tasks, isReviewColumnTask), dispatchStormCount: nextDispatchStormCount, dispatchTimestamp, effectiveNodeId: effectiveNode.nodeId ?? null, @@ -2478,7 +2665,27 @@ export class Scheduler { ); } - if (toColumn === "done") { + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-20:40 (fleet — mission completion advance): + "Did this task just COMPLETE?" resolved from the destination column's own trait. + + Keyed on `toColumn === "done"`, a renamed board never advanced mission execution: the feature + status above was reconciled (that path already resolves traits), but the follow-on completion + never fired, so the mission stalled with its roadmap showing the work finished. The two halves + of one handler disagreed about whether the same move was a completion. + + Fail-soft to the legacy literal via `isCompleteColumnRole`'s documented no-metadata behaviour — + an unresolvable workflow keeps today's behaviour rather than stalling the mission. + */ + let completionFlags: ColumnRoleTraitFlags | undefined; + try { + const ir = await resolveWorkflowIrForTask(this.store, taskId); + const column = (ir as WorkflowIrV2).columns?.find((candidate) => candidate.id === toColumn); + if (column) completionFlags = resolveColumnFlags(column); + } catch { + // Unresolvable workflow: fall back to the legacy `done` literal below. + } + if (isCompleteColumnRole(completionFlags, toColumn)) { await this.handleMissionTaskCompletion(taskId, sliceIdBeforeUpdate); } } catch (err) { diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index ad3bb97ad1..3be22b9a50 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -3,8 +3,8 @@ "byFile": { "packages/engine/src/self-healing.ts": 110, "packages/engine/src/executor.ts": 57, - "packages/engine/src/scheduler.ts": 26, "packages/core/src/store.ts": 12, + "packages/engine/src/scheduler.ts": 12, "packages/core/src/task-store/async-comments-attachments.ts": 9, "packages/dashboard/app/components/TaskContextMenu.tsx": 9, "packages/dashboard/src/github-tracking-comments.ts": 9, @@ -27,7 +27,6 @@ "packages/engine/src/merger.ts": 5, "packages/engine/src/project-engine.ts": 5, "packages/engine/src/restart-recovery-coordinator.ts": 5, - "packages/core/src/agent-store.ts": 2, "packages/core/src/blocker-fanout.ts": 4, "packages/core/src/task-store/task-store-helpers.ts": 4, "packages/dashboard/app/components/TaskReviewTab.tsx": 4, @@ -55,6 +54,7 @@ "packages/engine/src/planner-overseer.ts": 3, "packages/engine/src/worktree-pool.ts": 3, "packages/cli/src/commands/dashboard-tui/app.tsx": 2, + "packages/core/src/agent-store.ts": 2, "packages/core/src/assigned-task-ranking.ts": 2, "packages/core/src/duplicate-intake.ts": 2, "packages/core/src/near-duplicate-canonical.ts": 2, @@ -139,6 +139,8 @@ "deliberateByFile": { "packages/dashboard/app/components/TaskCard.tsx\u0000triage": 2, "packages/dashboard/app/components/TaskDetailModal.tsx\u0000triage": 2, + "packages/engine/src/scheduler.ts\u0000in-progress": 2, + "packages/engine/src/scheduler.ts\u0000in-review": 2, "packages/cli/src/commands/task.ts\u0000archived": 1, "packages/cli/src/commands/task.ts\u0000done": 1, "packages/cli/src/extension.ts\u0000archived": 1, @@ -158,6 +160,8 @@ "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/scheduler.ts\u0000archived": 1, + "packages/engine/src/scheduler.ts\u0000done": 1, "packages/engine/src/triage.ts\u0000triage": 1 }, "queryByFile": {