From 41031dbe2c058bf5a9ac5c800c2e038699f89317 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 29 Jul 2026 10:01:14 -0700 Subject: [PATCH] =?UTF-8?q?Drift=20review=20(unowned):=20auto-claim=20cand?= =?UTF-8?q?idacy=20resolves=20hold=20+=20completion=20roles=20=E2=80=94=20?= =?UTF-8?q?three=20literals,=20two=20opposite=20failures=20(#2565)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > **Based on `main`** — independent of my U7 stack and of #2561; merges in any order. Third unowned drift-review site. `isRunnableAutoClaimCandidate` is the single source of truth for *"may an agent claim this task?"* (FN-6873), and it carried **three** lifecycle literals that fail in **opposite directions**. ## The two failures **`column === "todo"` gated candidacy** on the hold role. Keyed on the literal, a renamed workflow's candidate set was **permanently empty** — agents were never offered its work, and nothing anywhere reported it. Silence, not an error. **`dependency?.column === "done" || "archived"` gated dependency satisfaction**, and this is the more dangerous half: a dependency that finished in a renamed **complete** column was never recognised as done, so the dependent stayed **blocked forever**. One makes work invisible; the other makes it permanently ineligible. Both are silent. ## Roles resolve per task, not per pass The non-obvious part: **a dependency may sit on a different workflow from the claimant.** A single per-pass answer is wrong for one of them on any mixed board — so the map is keyed by task id, and the dependency check reads the *dependency's* roles, not the claimant's. Asserted directly: a dependency completed in `done` (default vocabulary) satisfying a claimant waiting in `drafting` (renamed). ## Shape Both callers already have the store and are async, so they resolve for real rather than taking the injected-lane fallback the *synchronous* predicates needed (#2551). The predicate itself stays synchronous — a resolved-roles map is passed in — because it runs inside two `filter`/`flatMap` bodies. Tasks absent from the map keep the legacy ids, so a partially-resolvable board degrades to today's behavior instead of silently emptying the candidate set. **Type narrowing preserved.** The two callers take `Pick`, which is what makes them testable without a real store. Rather than widening to the whole `TaskStore`, they now take `Pick & WorkflowIrResolverStore` — the minimal additional shape resolution needs. ## Revert proofs, isolated per literal | Restored | Result | |---|---| | hold literal only | **3 of 6 fail** | | dependency-completion literals only | **1 of 6 fails** | The three default-vocabulary cases pass under both. Splitting the proof matters here: it confirms the two halves are **independently** load-bearing rather than one masking the other — a single combined revert would have shown 3 failures and told me nothing about the dependency half. ## Convergence Measured on `main`, comment-stripped scan of `column === / !== "todo" | "triage"` in `packages/*/src` excluding tests: - this file alone: **103 → 102** - with #2561: **103 → 100** The `done` / `archived` literals fixed here sit outside that pattern and are not counted — same caveat as #2561's gridlock `active` filter. Two PRs now where the real fix is larger than the metric shows. ## Verification | Check | Result | |---|---| | new suite | 6/6 | | pre-existing auto-claim suite | 17/17, **no expectation edits** | | `tsc --noEmit` (engine) | clean | | `pnpm lint` | clean | | `pnpm test:gate` | green (414 + 10 + 71) | | `pnpm check:changesets` | clean | ## Remaining unowned in my area `mission-feature-sync.ts` (1, a planning-lane check) and `notification-service.ts` (1, *"has progressed past"* — a different semantic needing its own thinking, not a mechanical swap). Taking those next unless claimed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .changeset/auto-claim-resolved-columns.md | 7 + .../auto-claim-resolved-columns.test.ts | 139 ++++++++++++++++++ packages/engine/src/auto-claim-snapshot.ts | 91 ++++++++++-- 3 files changed, 228 insertions(+), 9 deletions(-) create mode 100644 .changeset/auto-claim-resolved-columns.md create mode 100644 packages/engine/src/__tests__/auto-claim-resolved-columns.test.ts diff --git a/.changeset/auto-claim-resolved-columns.md b/.changeset/auto-claim-resolved-columns.md new file mode 100644 index 0000000000..350061ad1b --- /dev/null +++ b/.changeset/auto-claim-resolved-columns.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Agents can auto-claim work on boards with renamed columns, and dependencies finished there now count as done. +category: fix +dev: U7 / R3 — unowned drift-review site. `isRunnableAutoClaimCandidate` carried three lifecycle literals: `column === "todo"` gated candidacy (a renamed workflow's candidate set was permanently empty, silently), and `dependency.column === "done" || "archived"` gated dependency satisfaction (a dependency finished in a renamed complete column was never recognised, blocking the dependent forever). Roles are resolved PER TASK because a dependency may sit on a different workflow from the claimant; both callers are async with store access so they resolve for real. Tasks absent from the resolved map keep the legacy ids, so a partially-resolvable board degrades to today's behavior instead of emptying. diff --git a/packages/engine/src/__tests__/auto-claim-resolved-columns.test.ts b/packages/engine/src/__tests__/auto-claim-resolved-columns.test.ts new file mode 100644 index 0000000000..93d85120ef --- /dev/null +++ b/packages/engine/src/__tests__/auto-claim-resolved-columns.test.ts @@ -0,0 +1,139 @@ +/* +FNXC:AutoClaimResolvedColumns 2026-07-29-14:40 (U7 / R3, R12 — workflow-owned lifecycle): + +`isRunnableAutoClaimCandidate` is the single source of truth for "may an agent claim +this task?" (FN-6873). It carried THREE lifecycle-column literals: + + `column === "todo"` — the candidate gate, i.e. the HOLD role + `dependency?.column === "done" || === "archived"` — dependency satisfaction, i.e. + the COMPLETE and ARCHIVED roles of the DEPENDENCY'S + own workflow, which need not be the claimant's + +On a renamed workflow the first makes the candidate set permanently EMPTY — agents +are simply never offered work, with no error anywhere. The second is the more +dangerous direction: a dependency that finished in a renamed complete column is not +recognised as done, so the blocked task stays blocked forever; and in a mixed board +the dependency's workflow may differ from the claimant's, which is why the roles are +resolved PER TASK rather than once for the pass. + +Both callers already have the store and are async, so this resolves for real rather +than taking the injected-lane fallback the synchronous predicates needed (#2551). +Tasks absent from the resolved map keep the legacy answer, so a partially-resolvable +board degrades to today's behavior instead of silently emptying. +*/ +import { describe, expect, it, vi } from "vitest"; +import type { Task, TaskStore, WorkflowIr } from "@fusion/core"; + +import { resolveFreshAutoClaimCandidates } from "../auto-claim-snapshot.js"; + +const DEFAULT_NAMES = { hold: "todo", complete: "done" }; +const RENAMED = { hold: "drafting", complete: "shipped" }; + +function ir(id: string, names: { hold: string; complete: string }): WorkflowIr { + return { + version: "v2", + id, + name: id, + columns: [ + { id: names.hold, name: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "wip", name: "Wip", traits: [{ trait: "wip" }] }, + { id: names.complete, name: "Complete", traits: [{ trait: "complete" }] }, + { id: "archived", name: "Archived", traits: [{ trait: "archived" }] }, + ], + nodes: [], + edges: [], + } as unknown as WorkflowIr; +} + +function task(over: Partial = {}): Task { + return { + id: "FN-1", + title: "t", + description: "", + column: "todo", + status: null, + 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", + ...over, + } as Task; +} + +/** A store where each task may sit on a DIFFERENT workflow, which is the mixed-board case. */ +function storeWith(tasks: Task[], workflowByTask: Record): TaskStore { + return { + listTasks: vi.fn(async () => tasks), + getTask: vi.fn(async (id: string) => tasks.find((t) => t.id === id)), + getTaskWorkflowSelection: vi.fn((id: string) => ({ workflowId: workflowByTask[id]?.id ?? "wf-default", stepIds: [] })), + getTaskWorkflowSelectionAsync: vi.fn(async (id: string) => ({ workflowId: workflowByTask[id]?.id ?? "wf-default", stepIds: [] })), + getWorkflowDefinition: vi.fn(async (id: string) => { + const found = Object.values(workflowByTask).find((w) => (w as unknown as { id: string }).id === id); + return { ir: found ?? ir("wf-default", DEFAULT_NAMES) }; + }), + } as unknown as TaskStore; +} + +async function claimable(tasks: Task[], workflowByTask: Record): Promise { + const store = storeWith(tasks, workflowByTask); + const resolved = await resolveFreshAutoClaimCandidates( + store, + tasks.map((t) => ({ id: t.id, title: t.title ?? "", ageHours: 0 })) as never, + () => 1_000_000, + ); + return resolved.map((c) => c.id).sort(); +} + +describe("auto-claim candidacy resolves the hold and completion roles", () => { + it("offers a card waiting in the DEFAULT hold column (no-regression half)", async () => { + const wf = ir("wf-default", DEFAULT_NAMES); + expect(await claimable([task({ id: "FN-A", column: "todo" })], { "FN-A": wf })).toEqual(["FN-A"]); + }); + + it("offers a card waiting in a RENAMED hold column", async () => { + // Pre-conversion the candidate set was permanently EMPTY for this workflow — + // agents were never offered its work, with no error anywhere. + const wf = ir("wf-renamed", RENAMED); + expect(await claimable([task({ id: "FN-A", column: "drafting" })], { "FN-A": wf })).toEqual(["FN-A"]); + }); + + it("never offers a card that is not in its own hold column", async () => { + const wf = ir("wf-renamed", RENAMED); + expect(await claimable([task({ id: "FN-A", column: "wip" })], { "FN-A": wf })).toEqual([]); + }); + + it("treats a dependency finished in a RENAMED complete column as satisfied", async () => { + // The dangerous direction: unrecognised completion blocks the dependent forever. + const wf = ir("wf-renamed", RENAMED); + const done = task({ id: "FN-DEP", column: "shipped" }); + const blocked = task({ id: "FN-A", column: "drafting", dependencies: ["FN-DEP"] }); + + expect(await claimable([blocked, done], { "FN-A": wf, "FN-DEP": wf })).toContain("FN-A"); + }); + + it("still blocks on a dependency that has NOT completed", async () => { + // The other side, so "always satisfied" cannot pass for "correctly resolved". + const wf = ir("wf-renamed", RENAMED); + const running = task({ id: "FN-DEP", column: "wip" }); + const blocked = task({ id: "FN-A", column: "drafting", dependencies: ["FN-DEP"] }); + + expect(await claimable([blocked, running], { "FN-A": wf, "FN-DEP": wf })).not.toContain("FN-A"); + }); + + it("resolves each task's roles from ITS OWN workflow on a mixed board", async () => { + /* + The claimant and its dependency may sit on different workflows, so a single + per-pass answer would be wrong for one of them. Here the dependency completed in + `done` (default vocabulary) while the claimant waits in `drafting` (renamed). + */ + const renamed = ir("wf-renamed", RENAMED); + const standard = ir("wf-default", DEFAULT_NAMES); + const done = task({ id: "FN-DEP", column: "done" }); + const blocked = task({ id: "FN-A", column: "drafting", dependencies: ["FN-DEP"] }); + + expect(await claimable([blocked, done], { "FN-A": renamed, "FN-DEP": standard })).toContain("FN-A"); + }); +}); diff --git a/packages/engine/src/auto-claim-snapshot.ts b/packages/engine/src/auto-claim-snapshot.ts index 8f6a7cfcba..6f50857142 100644 --- a/packages/engine/src/auto-claim-snapshot.ts +++ b/packages/engine/src/auto-claim-snapshot.ts @@ -1,4 +1,5 @@ -import type { Task, TaskStore } from "@fusion/core"; +import type { Task, TaskStore, WorkflowIr, WorkflowIrResolverStore } from "@fusion/core"; +import { resolveTaskLifecycleColumns } from "@fusion/core"; import { createLogger, type Logger } from "./logger.js"; /** @@ -22,7 +23,7 @@ export interface AutoClaimSnapshot { } interface AutoClaimSnapshotManagerOptions { - taskStore: Pick; + taskStore: Pick & WorkflowIrResolverStore; ttlMs?: number; logger?: Logger; now?: () => number; @@ -37,18 +38,88 @@ Auto-claim runnability must have one source of truth so the snapshot rebuild and FNXC:AutoClaim 2026-06-21-16:09: FN-6873 pins `column === "todo"` as the candidate gate after FN-6872 appeared in a heartbeat prompt while archived from a stale cache. Archived, done, triage, in-progress, in-review, soft-deleted, paused, assigned, checked-out, and dependency-blocked rows can satisfy dependencies where allowed, but must never be surfaced or claimed as auto-claim candidates. */ -export function isRunnableAutoClaimCandidate(task: Task, tasksById: ReadonlyMap): boolean { - return task.column === "todo" +/** + * FNXC:AutoClaimResolvedColumns 2026-07-29-14:40 (U7 / R3): + * Lifecycle roles per task id, so this predicate can stay SYNCHRONOUS while still + * answering per-workflow. Absent entries fall back to the legacy ids, so a + * partially-resolvable board degrades to today's behavior rather than silently + * emptying the candidate set. + */ +export interface AutoClaimLifecycleRoles { + hold?: string; + complete?: string; + archived?: string; +} + +const LEGACY_AUTO_CLAIM_ROLES: Required = { + hold: "todo", + complete: "done", + archived: "archived", +}; + +const rolesFor = ( + taskId: string, + rolesByTask?: ReadonlyMap, +): Required => { + const resolved = rolesByTask?.get(taskId); + return { + hold: resolved?.hold ?? LEGACY_AUTO_CLAIM_ROLES.hold, + complete: resolved?.complete ?? LEGACY_AUTO_CLAIM_ROLES.complete, + archived: resolved?.archived ?? LEGACY_AUTO_CLAIM_ROLES.archived, + }; +}; + +/** + * FNXC:AutoClaimResolvedColumns 2026-07-29-14:40 (U7 / R3): + * THREE lifecycle literals lived here, and they fail in opposite directions: + * + * `column === "todo"` gated candidacy on the HOLD role. Keyed on the literal, a + * renamed workflow's candidate set was permanently EMPTY — agents were never + * offered its work, with no error anywhere to say so. + * + * `dependency?.column === "done" || "archived"` is dependency SATISFACTION, and it + * is the more dangerous half: a dependency that finished in a renamed complete + * column was not recognised as done, so the dependent stayed blocked forever. + * + * Roles are resolved PER TASK, not once per pass, because a dependency may sit on a + * DIFFERENT workflow from the claimant — a mixed board makes a single per-pass + * answer wrong for one of them. + */ +export function isRunnableAutoClaimCandidate( + task: Task, + tasksById: ReadonlyMap, + rolesByTask?: ReadonlyMap, +): boolean { + return task.column === rolesFor(task.id, rolesByTask).hold && task.paused !== true && !task.assignedAgentId && !task.checkedOutBy && !task.deletedAt && task.dependencies.every((dependencyId) => { const dependency = tasksById.get(dependencyId); - return dependency?.column === "done" || dependency?.column === "archived"; + if (!dependency) return false; + // The DEPENDENCY's own roles, which need not be the claimant's. + const depRoles = rolesFor(dependencyId, rolesByTask); + return dependency.column === depRoles.complete || dependency.column === depRoles.archived; }); } +/** Resolve lifecycle roles for every task in one pass, sharing a single IR cache. */ +export async function resolveAutoClaimLifecycleRoles( + taskStore: WorkflowIrResolverStore, + tasks: readonly Task[], +): Promise> { + const irCache = new Map(); + const roles = new Map(); + for (const task of tasks) { + const resolved = await resolveTaskLifecycleColumns(taskStore, task.id, irCache); + if (resolved) { + roles.set(task.id, { hold: resolved.hold, complete: resolved.complete, archived: resolved.archived }); + } + } + return roles; +} + export function toAutoClaimCandidate(task: Task, now: number): AutoClaimCandidate { const reference = task.columnMovedAt ?? task.createdAt; const ageMs = Math.max(0, now - Date.parse(reference)); @@ -76,7 +147,7 @@ FNXC:AutoClaim 2026-06-21-16:09: The fresh slim list intentionally includes archived rows by default so the shared predicate, not storage filtering, proves archived-while-cached rows are dropped before heartbeat prompt rendering or winner selection. */ export async function resolveFreshAutoClaimCandidates( - taskStore: Pick, + taskStore: Pick & WorkflowIrResolverStore, candidates: ReadonlyArray, now: () => number = Date.now, ): Promise { @@ -86,10 +157,11 @@ export async function resolveFreshAutoClaimCandidates( const allTasks = await taskStore.listTasks({ slim: true }); const tasksById = new Map(allTasks.map((task) => [task.id, task])); + const rolesByTask = await resolveAutoClaimLifecycleRoles(taskStore, allTasks); const resolvedAt = now(); return candidates.flatMap((candidate) => { const canonicalTask = tasksById.get(candidate.id); - if (!canonicalTask || !isRunnableAutoClaimCandidate(canonicalTask, tasksById)) { + if (!canonicalTask || !isRunnableAutoClaimCandidate(canonicalTask, tasksById, rolesByTask)) { return []; } return [toAutoClaimCandidate(canonicalTask, resolvedAt)]; @@ -97,7 +169,7 @@ export async function resolveFreshAutoClaimCandidates( } export class AutoClaimSnapshotManager { - private readonly taskStore: Pick; + private readonly taskStore: Pick & WorkflowIrResolverStore; private readonly ttlMs: number; private readonly logger: Logger; private readonly now: () => number; @@ -144,10 +216,11 @@ export class AutoClaimSnapshotManager { private async rebuild(): Promise { const allTasks = await this.taskStore.listTasks({ slim: true }); const tasksById = new Map(allTasks.map((candidate) => [candidate.id, candidate])); + const rolesByTask = await resolveAutoClaimLifecycleRoles(this.taskStore, allTasks); const now = this.now(); const tasks = allTasks - .filter((candidate) => isRunnableAutoClaimCandidate(candidate, tasksById)) + .filter((candidate) => isRunnableAutoClaimCandidate(candidate, tasksById, rolesByTask)) .sort((a, b) => { const aSortAt = a.columnMovedAt ?? a.createdAt; const bSortAt = b.columnMovedAt ?? b.createdAt;