From 3ca5d4d6434ef6f197c91964c4172ccdaac37dd4 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 9 Aug 2026 14:23:24 -0700 Subject: [PATCH] FN-8913: prioritize older tasks in hold-release scheduling Rank eligible hold-release candidates by priority, age, and task ID to prevent newer work from starving older peers. - Reuse the core priority-age-ID comparator for hold-release evaluation. - Add regression coverage for priority, age, dependency, and overlap scheduling behavior. - Document the production hold-release fairness order and add a patch changeset. Files changed: .changeset/fn-8913-older-first-scheduling.md | 7 ++ docs/architecture.md | 4 +- .../hold-release-priority-age-order.test.ts | 113 +++++++++++++++++++++ .../__tests__/scheduler-overlap-starvation.test.ts | 76 ++++++++++++++ packages/engine/src/execution/hold-release.ts | 17 +++- 5 files changed, 212 insertions(+), 5 deletions(-) Fusion-Task-Id: FN-8913 Fusion-Task-Lineage: 89910df9-9d3a-404c-85fa-f0797e8fa001 Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-8913-older-first-scheduling.md | 7 ++ docs/architecture.md | 4 +- .../hold-release-priority-age-order.test.ts | 113 ++++++++++++++++++ .../scheduler-overlap-starvation.test.ts | 76 ++++++++++++ packages/engine/src/execution/hold-release.ts | 17 ++- 5 files changed, 212 insertions(+), 5 deletions(-) create mode 100644 .changeset/fn-8913-older-first-scheduling.md create mode 100644 packages/engine/src/__tests__/hold-release-priority-age-order.test.ts diff --git a/.changeset/fn-8913-older-first-scheduling.md b/.changeset/fn-8913-older-first-scheduling.md new file mode 100644 index 0000000000..f2d3a66efe --- /dev/null +++ b/.changeset/fn-8913-older-first-scheduling.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Prefer older same-priority tasks when scheduling after priority and overlap checks. +category: fix +dev: Hold/release auto-release candidates rank via compareTasksByPriorityThenAgeAndId (priority desc, createdAt ASC, id). diff --git a/docs/architecture.md b/docs/architecture.md index 5a4838de70..1288aa949d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -642,7 +642,7 @@ See [Memory Plugin Contract](./memory-plugin-contract.md) for the full plan. - Advisory and blocking paths are both logged to task logs for operator visibility. ### Scheduling and execution -- `Scheduler` (`scheduler.ts`) — dependency-aware task scheduling that dispatches eligible todo tasks by priority first, then dependency-unblock fanout within the same priority class (FN-4969), then FIFO (`createdAt` ascending) with task-id fallback. `urgent` always stays ahead of lower priorities, and overlap/file-scope blockers are excluded from fanout weighting. +- `Scheduler` (`scheduler.ts`) — dependency-aware task scheduling whose workflow hold/release sweep ranks auto-release candidates by priority first, then FIFO (`createdAt` ascending), then task ID. Pauses, unmet dependencies, active file-scope overlaps, and downstream capacity occupancy exclude non-runnable work before it can consume a reservation; `urgent` always stays ahead of lower priorities. This production hold/release order intentionally does not use dependency-unblock fanout as a ranking key. - **Worktree-capacity admission (FN-8822):** scheduler execute, triage specify, project-engine merge, and direct workflow-planning continuation handoffs share one serialized project coordinator. Its ceiling is `min(maxConcurrent, maxWorktrees)` when worktree limiting is enabled and counts only canonical live task claims plus transient reservations—not retained directories, stale metadata, paused/terminal tasks, or orphans. A genuinely full cap persists a deduplicated queued reason with the `maxWorktrees` gate, used/limit, and holder IDs through ordinary task status/log APIs. Retained worktrees are deliberately non-destructive: cleanup and pooling preserve active, dirty, or uniquely committed work. - `blockedBy` invariant (FN-3924/FN-4091): the field is only durable when it references a current unresolved explicit dependency (or, for dependency-free tasks, an active overlap blocker). Completion gating now validates `blockedBy` through live task resolution: missing blockers and blockers already in `done`/`archived` are treated as stale, while only still-active blockers continue to prevent `fn_task_done`. If no current blocker remains, scheduler/event reconciliation clears `blockedBy` to `null` and re-evaluates from live task state. - Dependency-cycle invariant (FN-5256): task dependency graphs are acyclic at write time (`DependencyCycleError` in `TaskStore` for `createTask`, `createTaskWithReservedId`, `updateTask`, and `applyReplicatedTaskCreate`) with `task:dependency-cycle-rejected` audit evidence. Self-healing batch 2 adds `reconcileDependencyCycles`, which emits `task:dependency-cycle-detected`, auto-repairs only bounded umbrella-back-edge loops via `task:auto-reconciled-dependency-cycle`, and leaves ambiguous cycles untouched with `task:dependency-cycle-unrepaired` for operator inspection. @@ -2289,7 +2289,7 @@ This section preserves the detailed lifecycle/self-healing contracts that were f - **Stale registration recovery (FN-5056)**: `NativeWorktreeBackend.create` and `executor.tryCreateWorktree` detect `missing but already registered worktree` failures, run `git worktree prune` (plus `remove --force` / `add -f` fallbacks) before retrying, and emit `worktree:stale-registration-{detected,recovered,recovery-failed}` audit events. - **Bare branch-collision recovery (FN-8132)**: after stale lock/registration recovery, `NativeWorktreeBackend.create` classifies a `git worktree add -b` “branch already exists” error even when its requested target path is absent. It attaches an unregistered branch only when every unique commit is attributed to the requesting task, recreates merged/subsumed or no-unique-work branches from the caller-pinned start point, and refuses foreign, unattributed, or mixed unique history without moving its ref. A live foreign worktree remains a `BranchConflictError`; recovery dispositions emit `worktree:branch-collision-recovery`. - **Raw worktree deletion must be paired with prune (FN-5058)**: any direct filesystem deletion of a worktree directory (`rm -rf` / `rmSync`) must be followed by best-effort `git worktree prune` via `pruneWorktreeAdminEntries` so `.git/worktrees/*` admin entries are not stranded in a missing-but-registered state (FN-5056 class). -- **Scheduler fanout tiebreaker (FN-4969)**: within the same priority class, scheduler dispatch prefers runnable `todo` tasks with the highest active dependency-dependent fanout; `urgent` always outranks lower priorities regardless of fanout, and `overlapBlockedBy`/file-scope overlap blockers are excluded from unblock weight. +- **Scheduler fanout comparator (FN-4969):** the core still exposes a priority-and-fanout comparator for callers that explicitly need dependency-unblock weighting, with `urgent` always ahead of lower priorities and overlap/file-scope blockers excluded from unblock weight. Workflow hold/release dispatch does not use it: its live fairness order is priority, then `createdAt`, then task ID after eligibility filters. - **Scheduler overlap priority/age guard (FN-5325)**: with `groupOverlappingFiles=true`, scheduler now defers a lower-priority (or younger same-priority) candidate when an overlapping queued todo task exists, preserving priority→age→task-id order for overlap serialization without preempting in-progress work. If the inversion is against an already-running lower-priority blocker, scheduler still defers the candidate; the per-pairing audit event was removed in FN-6174 due to zero consumers and table bloat. - **Empty-commit refusal + early empty-own-diff finalize (FN-5345/FN-5377)**: Fusion task worktrees install a `prepare-commit-msg` hook that refuses `git commit --allow-empty` and other zero-staged-diff commits, preventing verification-only tasks from manufacturing empty handoff commits that defeat the merger's no-op classifier. The hook allows legitimate empty-tree paths (amend, merge, squash, cherry-pick, revert, rebase). Amend detection tokenizes the parent process command line (`ps -o args=` with `/proc/$PPID/cmdline` fallback for Alpine/busybox) and stops at the first message-supplying flag (`-m`/`-F`/`--message`/`--file`) so a commit message containing the substring `--amend` cannot bypass the guard. In `aiMergeTask`, an early empty-own-diff fast-path runs BEFORE any reuse-handoff acquisition: when integration mode is `reuse-task-worktree`, the branch exists, `git rev-list --count ..` is > 0, and `git diff --quiet ..` exits 0, the task auto-finalizes as no-op with `mergeDetails.noOpMerge: true` and emits `task:auto-recover-finalize-already-on-main` with `reason: "empty-own-diff-early-fast-path"`. The fast-path best-effort removes the stranded worktree (FN-4811 same-task/foreign-owner guard) and deletes the `fusion/` branch so empty-own-diff residuals do not accumulate. This unsticks tasks where a stale empty handoff commit combined with drifted worktree↔branch mapping would otherwise wedge the handoff gate with `registered-branch-mismatch`. The explicit `cwd-integration-branch` mode is unchanged (`cwd-main` remains a deprecated alias normalized to it). `classifyOwnedLandedEvidence` also detects empty-own-diff (aheadCount > 0, zero net diff) and returns `proven-no-op` so downstream self-healing and post-handoff finalize paths benefit too. Additionally, merger's reuse-fallback path now consults `git worktree list --porcelain` before creating a new worktree: extant usable registrations of `fusion/` are reused directly (rather than blindly `git worktree add -f` producing a duplicate registration), and stale registrations are pruned first. The direct-reuse shortcut is guarded by FN-4811 (refuses paths owned by a different task in `activeSessionRegistry`) and FN-4954 (skipped when `recycleWorktrees=true` with a pool attached, so `WorktreePool.acquire` lease bookkeeping stays consistent). Two audit subtypes — `merge:reuse-fallback-pruned-stale-registration` and `merge:reuse-fallback-reused-existing-registration` — replace the prior overloading of `merge:reuse-fallback-new-worktree` for these cases. - **Verified no-op/duplicate executor completion (FN-6275/FN-7488)**: explicit `fn_task_done` may complete with zero branch commits only when the summary starts with a recognized sentinel (`PREMISE STALE:`, `NO-OP:`, `NOOP:`, `DUPLICATE: FN-NNNN ...`, or `REDUNDANT:`), the task already carries a no-commit contract, or the PROMPT declares a source-free gitignored task-artifact delivery. The source-free path is intentionally narrow: File Scope must be populated and limited to board/task artifacts such as `.fusion/tasks/...`, task documents/logs, or attachments; the prompt must forbid force-adding ignored `.fusion/` artifacts and fabricating empty commits or equivalently state that source-free/gitignored task artifacts are the only deliverables; and any tracked source/docs/config/test/changeset scope keeps the `no_commits` refusal active (even if `.fusion/` artifacts are also listed). These exemptions only relax the `no_commits` invariant; `wrong_toplevel`, `wrong_branch`, pending-step/review refusals, and scope-leak guards still run. Accepted sentinel completions persist `noCommitsExpected: true`, write task-log audit details with marker kind/reason/raw summary/run/agent IDs, and add a task timeline activity so the no-code terminal path remains explainable. Prompt-derived source-free completions log `prompt-derived source-free task-artifact contract` for operator audit. Ordinary zero-commit implementation completions without one of these contracts are still refused. diff --git a/packages/engine/src/__tests__/hold-release-priority-age-order.test.ts b/packages/engine/src/__tests__/hold-release-priority-age-order.test.ts new file mode 100644 index 0000000000..c5cfae821c --- /dev/null +++ b/packages/engine/src/__tests__/hold-release-priority-age-order.test.ts @@ -0,0 +1,113 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { Task, TaskStore, WorkflowIr } from "@fusion/core"; + +import { resetHoldReleaseInstrumentation, runHoldReleaseSweep } from "../execution/hold-release.js"; +import { schedulerLog } from "../logger.js"; + +const WORKFLOW_ID = "custom:priority-age"; + +function makeTask(overrides: Partial = {}): Task { + return { + id: "FN-001", + title: "task", + description: "", + column: "todo", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + columnMovedAt: "2026-01-01T00:00:00.000Z", + ...overrides, + } as Task; +} + +const workflowIr: WorkflowIr = { + version: "v2", + id: WORKFLOW_ID, + nodes: [], + edges: [], + columns: [ + { id: "todo", label: "Todo", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", label: "In Progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], +} as unknown as WorkflowIr; + +function createStore(tasks: Task[]): TaskStore { + const selection = { workflowId: WORKFLOW_ID, stepIds: [] }; + return { + getSettings: vi.fn(async () => ({ maxConcurrent: 1 })), + listTasks: vi.fn(async () => tasks), + getTask: vi.fn(async (id: string) => tasks.find((task) => task.id === id) ?? null), + moveTaskIf: vi.fn(async (id: string, column: string) => { + const task = tasks.find((candidate) => candidate.id === id)!; + task.column = column; + return { task, moved: true }; + }), + logEntry: vi.fn(async () => undefined), + 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; +} + +/* +FNXC:TaskDispatch 2026-08-09-21:04: +A limit of one with no in-progress occupants makes these fixtures a real +competition for one capacity slot. The assertions cover the production sweep's +committed move and the losing card, rather than testing the core comparator alone. +*/ +describe("hold-release priority and age ordering (FN-8913)", () => { + beforeEach(() => { + resetHoldReleaseInstrumentation(); + vi.restoreAllMocks(); + vi.spyOn(schedulerLog, "log").mockImplementation(() => {}); + vi.spyOn(schedulerLog, "debug").mockImplementation(() => {}); + vi.spyOn(schedulerLog, "warn").mockImplementation(() => {}); + }); + + it("releases the older same-priority candidate before the newer candidate", async () => { + const newer = makeTask({ id: "FN-020", createdAt: "2026-01-02T00:00:00.000Z", priority: "normal" }); + const older = makeTask({ id: "FN-010", createdAt: "2026-01-01T00:00:00.000Z", priority: "normal" }); + const result = await runHoldReleaseSweep(createStore([newer, older]), { now: () => 1_000_000 }); + + expect(result.released).toEqual(["FN-010"]); + expect(older.column).toBe("in-progress"); + expect(newer.column).toBe("todo"); + expect(result.held).toContainEqual({ taskId: "FN-020", reason: "downstream-full" }); + }); + + it("lets newer urgent work win over older normal work", async () => { + const olderNormal = makeTask({ id: "FN-010", createdAt: "2026-01-01T00:00:00.000Z", priority: "normal" }); + const newerUrgent = makeTask({ id: "FN-020", createdAt: "2026-01-02T00:00:00.000Z", priority: "urgent" }); + const result = await runHoldReleaseSweep(createStore([olderNormal, newerUrgent]), { now: () => 1_000_000 }); + + expect(result.released).toEqual(["FN-020"]); + expect(newerUrgent.column).toBe("in-progress"); + expect(olderNormal.column).toBe("todo"); + expect(result.held).toContainEqual({ taskId: "FN-010", reason: "downstream-full" }); + }); + + it("normalizes invalid priorities before ordering candidates by age", async () => { + const newerInvalid = makeTask({ id: "FN-020", createdAt: "2026-01-02T00:00:00.000Z", priority: "invalid" as Task["priority"] }); + const olderMissing = makeTask({ id: "FN-010", createdAt: "2026-01-01T00:00:00.000Z", priority: undefined }); + const result = await runHoldReleaseSweep(createStore([newerInvalid, olderMissing]), { now: () => 1_000_000 }); + + expect(result.released).toEqual(["FN-010"]); + expect(olderMissing.column).toBe("in-progress"); + expect(newerInvalid.column).toBe("todo"); + }); + + it("uses numeric task id as the deterministic tie-break for identical ages", async () => { + const laterId = makeTask({ id: "FN-020", priority: "normal" }); + const earlierId = makeTask({ id: "FN-010", priority: "normal" }); + const result = await runHoldReleaseSweep(createStore([laterId, earlierId]), { now: () => 1_000_000 }); + + expect(result.released).toEqual(["FN-010"]); + expect(earlierId.column).toBe("in-progress"); + expect(laterId.column).toBe("todo"); + }); +}); diff --git a/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts b/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts index ca94621d67..e2f8d5c050 100644 --- a/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts +++ b/packages/engine/src/__tests__/scheduler-overlap-starvation.test.ts @@ -608,6 +608,82 @@ describe("scheduler overlap starvation regression (FN-057)", () => { ]); }); + /* + FNXC:TaskDispatch 2026-08-09-21:04: + These scheduler fixtures prove hard dependency and active-lease filters run + before the new priority → age release order consumes capacity. Their limits + deliberately match occupancy: dependency uses one empty slot; lease uses two + slots because the live holder consumes one; competing holds use one empty slot. + */ + it("does not let an older urgent task with an unmet dependency consume the ready slot", async () => { + const tasks = [ + makeTask({ id: "FN-BLOCKER", column: "triage", priority: "normal" }), + makeTask({ + id: "FN-OLDER-URGENT", + column: "todo", + priority: "urgent", + dependencies: ["FN-BLOCKER"], + createdAt: "2026-01-01T00:00:00.000Z", + }), + makeTask({ id: "FN-READY-NORMAL", column: "todo", priority: "normal", createdAt: "2026-01-02T00:00:00.000Z" }), + ]; + // Limit 1 and no in-progress occupant: only the dispatchable held peer may take this slot. + const store = createStore(tasks, {}, { maxConcurrent: 1 }); + const scheduler = new Scheduler(store); + (scheduler as any).running = true; + + await scheduler.schedule(); + + expect(store.moveTask).toHaveBeenCalledWith("FN-READY-NORMAL", "in-progress", expect.anything()); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-OLDER-URGENT", "in-progress", expect.anything()); + expect(store.updateTask).toHaveBeenCalledWith("FN-OLDER-URGENT", { status: "queued", blockedBy: "FN-BLOCKER" }); + }); + + it("does not let an older urgent task blocked by an active lease starve disjoint work", async () => { + const tasks = [ + makeTask({ id: "FN-LEASE-HOLDER", column: "in-progress", priority: "normal" }), + makeTask({ id: "FN-OLDER-URGENT", column: "todo", priority: "urgent", createdAt: "2026-01-01T00:00:00.000Z" }), + makeTask({ id: "FN-READY-NORMAL", column: "todo", priority: "normal", createdAt: "2026-01-02T00:00:00.000Z" }), + ]; + // Limit 2: the in-progress lease holder occupies one slot, leaving one for the disjoint candidate. + const store = createStore(tasks, { + "FN-LEASE-HOLDER": ["packages/engine/src/scheduler.ts"], + "FN-OLDER-URGENT": ["packages/engine/src/scheduler.ts"], + "FN-READY-NORMAL": ["packages/core/src/store.ts"], + }, { maxConcurrent: 2 }); + const scheduler = new Scheduler(store); + (scheduler as any).running = true; + + await scheduler.schedule(); + + expect(store.moveTask).toHaveBeenCalledWith("FN-READY-NORMAL", "in-progress", expect.anything()); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-OLDER-URGENT", "in-progress", expect.anything()); + expect(store.updateTask).toHaveBeenCalledWith("FN-OLDER-URGENT", { + status: "queued", + blockedBy: null, + overlapBlockedBy: "FN-LEASE-HOLDER", + }); + }); + + it("lets the older same-priority overlapping peer claim the only empty slot", async () => { + const tasks = [ + makeTask({ id: "FN-NEWER", column: "todo", priority: "normal", createdAt: "2026-01-02T00:00:00.000Z" }), + makeTask({ id: "FN-OLDER", column: "todo", priority: "normal", createdAt: "2026-01-01T00:00:00.000Z" }), + ]; + // Limit 1 with zero in-progress occupants: the two overlapping holds compete for exactly one slot. + const store = createStore(tasks, { + "FN-OLDER": ["packages/engine/src/scheduler.ts"], + "FN-NEWER": ["packages/engine/src/scheduler.ts"], + }, { maxConcurrent: 1 }); + const scheduler = new Scheduler(store); + (scheduler as any).running = true; + + await scheduler.schedule(); + + expect(store.moveTask).toHaveBeenCalledWith("FN-OLDER", "in-progress", expect.anything()); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-NEWER", "in-progress", expect.anything()); + }); + it("clears an absent overlap blocker only after confirming no current overlap remains", async () => { const tasks = [ makeTask({ id: "FN-901", column: "todo", status: "queued", priority: "normal", overlapBlockedBy: "FN-MISSING" }), diff --git a/packages/engine/src/execution/hold-release.ts b/packages/engine/src/execution/hold-release.ts index 43e30480a6..28307ba6fd 100644 --- a/packages/engine/src/execution/hold-release.ts +++ b/packages/engine/src/execution/hold-release.ts @@ -44,6 +44,7 @@ import { PLAN_REVIEW_GROUP_ID, ACTIVE_WORKFLOW_WORK_ITEM_STATES, resolveCapacityPoolId, + sortTasksByPriorityThenAgeAndId, TransitionRejectionError, resolveWorkflowIrForTask, isUnplannedSeedPrompt, @@ -692,11 +693,21 @@ export async function runHoldReleaseSweep( prefetchMs = deps.now() - prefetchStartedMs; if (expired()) return logPreambleTruncation(allTasks.length); + /* + FNXC:TaskDispatch 2026-08-09-21:04: + Capacity and file-scope reservations are assigned in sweep evaluation order. + After hard eligibility gates reject paused, dependency-blocked, or overlapping + work, operators require priority first and older work before newer work within + a priority tier. Reuse core's priority → createdAt → id comparator so this + dispatcher and board ordering cannot drift; retain allTasks as the occupancy + and dependency snapshot rather than changing the global listTasks order. + */ + const tasksForReleaseEvaluation = sortTasksByPriorityThenAgeAndId(allTasks); let breakIndex: number | undefined; - for (let index = 0; index < allTasks.length; index += 1) { + for (let index = 0; index < tasksForReleaseEvaluation.length; index += 1) { if (expired()) { breakIndex = index; break; } - const task = allTasks[index]!; + const task = tasksForReleaseEvaluation[index]!; if (task.paused || task.userPaused || (task.nextRecoveryAt && Date.parse(task.nextRecoveryAt) > deps.now())) continue; if (expired()) { breakIndex = index; break; } const irStartedMs = deps.now(); @@ -742,7 +753,7 @@ export async function runHoldReleaseSweep( else { trackHeld(task.id, "move-rejected-or-no-slot", deps.now()); result.held.push({ taskId: task.id, reason: "move-rejected-or-no-slot" }); } evaluatedTaskIds.add(task.id); } - if (breakIndex !== undefined) { result.budgetTruncated = true; result.unevaluatedCount = allTasks.length - breakIndex; } + if (breakIndex !== undefined) { result.budgetTruncated = true; result.unevaluatedCount = tasksForReleaseEvaluation.length - breakIndex; } const sweepMs = deps.now() - sweepStartedMs; const longestHeldMs = result.held.reduce((max, held) => Math.max(max, deps.now() - (heldSince.get(held.taskId)?.sinceMs ?? deps.now())), 0); const summary = `Hold-release sweep: ${sweepMs}ms (prefetch ${prefetchMs}ms, ir-resolve ${irResolveMs}ms, evaluate ${Math.max(0, sweepMs - prefetchMs - irResolveMs)}ms over ${allTasks.length} tasks), released=${result.released.length}, held=${result.held.length}`