From b85e5f90e152b9fa84eeba2119f6f03aa3f93e68 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 16:20:38 -0700 Subject: [PATCH] fix(create): two task-CREATE destinations named a lane the board does not have (#2843) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both files sat at **census-zero** and both wrote real cards into columns no workflow declares. The census scores `===` comparisons, so a lane literal passed as a **call argument** is invisible to it — one of the four census-blind classes. These are the only two explicit-`column` creates in production: ``` packages/dashboard/src/routes/register-gitlab.ts:108 column: "triage" packages/cli/src/extension.ts:5243 column: "todo" ``` ## The two defects **`register-gitlab.ts` — `column: "triage"`, a column U11 DELETED.** This one is broken on *every* board, not only renamed ones: the default lineage is now `todo | in-progress | in-review | done | archived`. `createTask` already resolves the intake column of the workflow it selects (`resolvedEntryColumn`), and an explicit `column` **overrides** that resolution — which is why the stale literal survived U11. Nothing rejects the write and nothing logs it: the route answers `201` with a task id and the imported card is simply not on the board. Same shape as the `task-update.ts` triage defect fixed earlier in this program. Fix: omit `column` and let `createTask` resolve intake. **`extension.ts` `fn_delegate_task` — `column: "todo"`.** On a workflow whose ready lane is named anything else, the delegated card goes to an undeclared column: written, reported to the caller as delegated, never visible to the agent it was delegated to. Fix: resolve the selected workflow's `hold` lane. **Deliberately not** "omit the column like the GitLab route" — the tool's own contract is *"the task goes to the ready-to-work lane and the target agent picks it up on its next heartbeat"*, so inheriting intake resolution would park a delegated card in a manual-intake lane waiting for a human. That would be a behaviour change; `hold` is the role that names the lane the literal meant. ## New helper: `resolveWorkflowColumnForRole(store, role, workflowId?)` The **write**-shaped counterpart to `resolveProjectColumnsForRoles`. The read helper unions in the legacy ids because an extra id in a query set is inert; here the same trick is a silent wrong write (post-U12 an undeclared column is a `TransitionRejectionError` on move, a phantom lane on create), so it returns one column from one workflow, or `undefined`. **A contract I got wrong twice, now pinned by a test.** `undefined` means *"this workflow declares no such column"* and nothing else. `resolveWorkflowIrById` never throws and never returns nothing — an unregistered builtin id, a missing definition row and a failing read all resolve to the default coding IR (branded via `markFellBack`). So an unreadable workflow yields the **built-in** hold lane, not `undefined`, and both call sites' `?? "todo"` fallbacks are narrower than they look. Two of my first test cases asserted the opposite and failed; the behaviour is the resolver's, and the write it produces is identical to the caller's own legacy fallback either way. ## Revert proofs (measured, not asserted) | revert | failure | |---|---| | `column: holdColumn` -> `column: "todo"` | `extension.test.ts`: `expected 'todo' to be 'queued'` | | omitted column -> `column: "triage"` | `routes-gitlab.test.ts`: `expected 'triage' to be undefined` | The two neighbouring `fn_delegate_task` cases stay green under the first revert, because the built-in board and the test's `linearWorkflowIr` both call the lane `todo` — which is exactly why this literal survived every previous pass. The GitLab case asserts **absence** of the key rather than a resolved id: the store there is a fake whose `createTask` echoes its input, so asserting a resolved value would be testing the fake. Absence is the property that hands the decision to the real `createTask`. ## Census | file | before | after | |---|---|---| | `packages/dashboard/src/routes/register-gitlab.ts` | 1 | 0 | | `packages/cli/src/extension.ts` | 1 | 0 | Baseline tightened. It also picks up `packages/core/src/task-store/moves.ts` 2 -> 0, which was **already true on main** — not from this diff. ## Noted, deliberately not changed - `validateAssignableAgentId`'s synthetic probe a few lines above still uses `{ id: "", column: "todo" }`. It feeds `isImplementationTask`, whose `IMPLEMENTATION_TASK_COLUMNS` set an earlier worker documented as deliberately-not-converted (converting it makes the routing policy async — an agent-admission behaviour change). On a renamed board the probe is now *stricter* than the real destination, which is the safe direction and matches the pre-existing behaviour. - The third site from this bucket, `workflow-node-handlers.ts:455` (`transitionTask({ column: "in-review" })` on the `review-handoff` seam), is a **hard** failure rather than a silent one — `transitionTask` routes through `moveTask`, which post-U12 throws `TransitionRejectionError` for an undeclared destination, so the workflow walk dies at the handoff on any renamed review lane. It is engine (`batch-engine`) and fixing it properly touches `executor.ts`, which #2820 is also editing. Left for that batch rather than opened as a conflicting edit. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit -p tsconfig.json` for `@fusion/core`, `@runfusion/fusion`, `@fusion/dashboard` — clean - `node scripts/lifecycle-column-census.mjs --strict` — exit 0 - targeted: `project-lane-vocabulary.test.ts` 14/14, `routes-gitlab.test.ts` 8/8, `extension.test.ts -t fn_delegate_task` 9/9 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * GitLab-imported cards now appear in the workflow’s configured intake lane. * Delegated tasks now move to the workflow’s configured hold lane, including workflows with custom lane names or separate intake and hold lanes. * Delegation reports the task’s final lane and provides an error when it cannot be moved successfully. --------- Co-authored-by: Claude Opus 5 (1M context) --- .changeset/create-destination-lanes.md | 7 + packages/cli/src/__tests__/extension.test.ts | 460 ++++++++++++++++++ packages/cli/src/extension.ts | 197 +++++++- .../src/__tests__/routes-gitlab.test.ts | 26 + .../dashboard/src/routes/register-gitlab.ts | 12 +- .../lib/lifecycle-column-census-baseline.json | 2 - 6 files changed, 696 insertions(+), 8 deletions(-) create mode 100644 .changeset/create-destination-lanes.md diff --git a/.changeset/create-destination-lanes.md b/.changeset/create-destination-lanes.md new file mode 100644 index 0000000000..5d5e493e49 --- /dev/null +++ b/.changeset/create-destination-lanes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: GitLab imports and agent delegation now land cards in real board lanes instead of vanishing. +category: fix +dev: GitLab import passed `column: "triage"`, a column U11 deleted, so imported cards were written into a lane no workflow declares; it now omits `column` and lets `createTask` resolve the workflow's intake lane. `fn_delegate_task` passed the literal `"todo"` and now resolves the created task's own `hold` lane via `resolveTaskLifecycleColumns`, moving the card off intake when the workflow separates the two roles. diff --git a/packages/cli/src/__tests__/extension.test.ts b/packages/cli/src/__tests__/extension.test.ts index 6c6b0390ce..cd03f15e9f 100644 --- a/packages/cli/src/__tests__/extension.test.ts +++ b/packages/cli/src/__tests__/extension.test.ts @@ -4252,6 +4252,466 @@ pgTest("fn pi extension (runnable structured-output regression slice)", () => { expect(task.enabledWorkflowSteps).toHaveLength(2); }); + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-22:05: + THE INVARIANT: delegation lands in the selected workflow's HOLD lane, whatever it is named. + + `fn_delegate_task` passed the literal `"todo"`. Its contract is the ready-to-work lane — the tool + tells the caller the target agent will pick the card up on its next heartbeat — so on a workflow + that names that lane `queued` the card went to a column the workflow does not declare: written, + reported as delegated, and never visible to the agent it was delegated to. + + Note the seed IR: `queued` carries the `hold` trait, and `start` sits on it, so intake and hold + are the same lane here. That is deliberate — it keeps the case about the RESOLUTION and not about + which of the two roles wins, which the neighbouring cases already cover with the built-in board. + + REVERT PROOF, measured: restore `column: "todo"` and this case fails with + `expected 'todo' to be 'queued'`. The two cases above it stay green, because the built-in and + `linearWorkflowIr` boards both call the lane `todo` — which is exactly why the literal survived. + */ + it("delegates into the workflow's own hold lane, not the literal todo", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-renamed-lane" }); + const store = h.store(); + const renamed = await store.createWorkflowDefinition({ + name: "Renamed hold lane", + ir: { + version: "v2", + name: "Renamed hold lane", + columns: [{ id: "queued", name: "Queued", traits: [{ trait: "intake" }, { trait: "hold" }] }], + nodes: [ + { id: "start", kind: "start", column: "queued" }, + { id: "end", kind: "end", column: "queued" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + } as unknown as WorkflowIr, + }); + + const tool = api.tools.get("fn_delegate_task")!; + const result = await tool.execute( + "dt-renamed-lane", + { agent_id: agentId, description: "Work in a renamed lane", workflow_id: renamed.id }, + undefined, + undefined, + makeCtx(tmpDir), + ); + + expect(result.isError).not.toBe(true); + const { task } = await readTaskWorkflowState(tmpDir, result.details.taskId); + expect(task.column).toBe("queued"); + expect(task.assignedAgentId).toBe(agentId); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-23:55: + THE INVARIANT: when intake and hold are DIFFERENT lanes, delegation ends on hold. + + The case above cannot see this: its workflow carries both traits on one column, so the card + arrives on the right lane through `createTask`'s entry resolution and the move never runs. Here + the two roles are separate columns, so the card lands on `ideas` and must be moved to `queued` — + which is the behaviour the old `column: "todo"` literal was providing (skip intake, go straight + to the lane the agent picks up from) and the part that would silently regress if the move were + dropped as redundant. + + `workflow_id` is deliberately EXPLICIT here rather than set as the project default: the point + under test is the move, and pinning the workflow keeps the case from depending on default + resolution as well. + + REVERT PROOF, measured: delete the post-create move and this fails with + `expected 'ideas' to be 'queued'`; the sibling case above stays green, which is exactly why it + is not sufficient on its own. + */ + it("moves the card off intake onto hold when the workflow separates the two", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-split-lanes" }); + const store = h.store(); + const split = await store.createWorkflowDefinition({ + name: "Split intake and hold", + ir: { + version: "v2", + name: "Split intake and hold", + columns: [ + { id: "ideas", name: "Ideas", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "ideas" }, + { id: "end", kind: "end", column: "queued" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + } as unknown as WorkflowIr, + }); + + const tool = api.tools.get("fn_delegate_task")!; + const result = await tool.execute( + "dt-split-lanes", + { agent_id: agentId, description: "Work past intake", workflow_id: split.id }, + undefined, + undefined, + makeCtx(tmpDir), + ); + + expect(result.isError).not.toBe(true); + // No WARNING in the text: a move that fails must say so rather than report a ready card. + expect(result.content[0].text).not.toContain("WARNING"); + const { task } = await readTaskWorkflowState(tmpDir, result.details.taskId); + expect(task.column).toBe("queued"); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-15:02 (#2843 review — greptile P1, "failed move reports + successful delegation"): + + A FAILED LANDING MUST NOT LEAD WITH A SUCCESS CLAIM. + + The first version kept "will be picked up by X on their next heartbeat cycle" and appended a + ` WARNING: ...`. Assigned-agent selection skips cards outside the workflow's hold lane, so a card + stranded on intake is never dispatched — and the caller, usually another agent, reads the first + sentence and moves on. "Success with a caveat" is how a delegation silently goes nowhere. + + WHAT THIS PROVES AND WHAT IT DOES NOT, stated because the distinction matters. It pins the + HANDLING — the branch returns `isError`, never claims pickup, and still surfaces the task id so a + caller can finish by hand. It does NOT prove the failure is reachable in production, and I could + not make it so: `holdColumn` comes from the task's OWN resolved IR, so `moveTask`'s declared-column + check passes by construction. I tried a workflow declaring a `hold` column with no node on it, + expecting rejection; the move SUCCEEDED and the card landed there — `moveTask` accepts any column + the workflow declares, and node reachability does not gate it. + + So the store method is stubbed for exactly one call. Stubbing is normally how a test proves only + that a catch block runs, which is why it is worth being explicit: the catch is DEFENSIVE, and what + changed — and what regresses silently — is the shape of the message it produces. + */ + it("reports a failed landing as an ERROR and never claims the agent will pick it up", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-landing-fails" }); + const store = h.store(); + const split = await store.createWorkflowDefinition({ + name: "Split lanes (landing fails)", + ir: { + version: "v2", + name: "Split lanes (landing fails)", + columns: [ + { id: "ideas", name: "Ideas", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "ideas" }, + { id: "end", kind: "end", column: "queued" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + } as unknown as WorkflowIr, + }); + + const moveTask = vi.spyOn(store, "moveTask").mockRejectedValueOnce( + new Error("unknown-column: queued"), + ); + let result: Awaited>; + const tool = api.tools.get("fn_delegate_task")!; + try { + result = await tool.execute( + "dt-landing-fails", + { agent_id: agentId, description: "Landing will fail", workflow_id: split.id }, + undefined, + undefined, + makeCtx(tmpDir), + ); + } finally { + moveTask.mockRestore(); + } + + expect(result.isError).toBe(true); + /* The claim that must never survive a failed landing. */ + expect(result.content[0].text).not.toContain("will be picked up"); + expect(result.content[0].text).toContain("stranded on intake"); + /* The card exists either way, so the caller keeps what it needs to finish the job by hand. */ + expect(result.details.taskId).toBeTruthy(); + expect(result.details.error).toContain("unknown-column"); + + /* And it really is still on intake — the message is not merely pessimistic. */ + const { task } = await readTaskWorkflowState(tmpDir, result.details.taskId); + expect(task.column).toBe("ideas"); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-15:45 (#2843 review — coderabbit, "fail the delegation + when the hold lane cannot be resolved"): + + THE COLLAPSE WAS REAL; THE PATH THAT REACHES IT IS ALL BUT UNREACHABLE. Both halves stated. + + `resolveTaskLifecycleColumns` returns `undefined` when resolution THREW, and a struct whose `hold` + is `undefined` when the board resolved fine and declares no hold lane. Reading it as + `(await ...)?.hold` collapsed the two, so a resolver failure would skip the move and fall into the + SUCCESS response — the same "stranded on intake, reported as ready" outcome the error branch exists + to prevent, by a path that never enters it. Splitting them is correct and costs nothing. + + BUT I COULD NOT MAKE THE THROW HAPPEN, and the reason generalises: `resolveWorkflowIrForTask` does + not fail, it SUBSTITUTES the built-in IR. Making the selection read reject (below) is swallowed + there, so the resolve succeeds with the default board, `hold` comes back as `todo`, and on the + built-in board that already equals the card's column — no move, and success is the right answer. + + So this asserts the outcome that IS reachable: a degraded resolve that yields the card's own lane + reports success, because nothing went wrong. The `undefined` guard remains as defence, and it is + documented as defence rather than counted as covered. + */ + it("a resolve that DEGRADES to the built-in board still reports success — nothing was stranded", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-resolve-degrades" }); + const store = h.store(); + + const resolve = vi.spyOn(store, "getTaskWorkflowSelectionAsync").mockRejectedValueOnce( + new Error("workflow selection unreadable"), + ); + const tool = api.tools.get("fn_delegate_task")!; + let result: Awaited>; + try { + result = await tool.execute( + "dt-resolve-degrades", + { agent_id: agentId, description: "Resolve degrades" }, + undefined, + undefined, + makeCtx(tmpDir), + ); + } finally { + resolve.mockRestore(); + } + + expect(result.isError).not.toBe(true); + expect(result.content[0].text).toContain("will be picked up"); + + /* The claim has to be TRUE, not merely present: the card really is on the lane agents pick from. */ + const { task } = await readTaskWorkflowState(tmpDir, result.details.taskId); + expect(task.column).toBe("todo"); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-18:50 (#2843 review — the race, and the fix that removed + it rather than detecting it): + + NO TEST FOR A TORN SELECTION SNAPSHOT, because the shape it would test no longer exists. + + I wrote one against a version that read the selection on both sides of the resolve and refused when + they disagreed. The version that shipped is better: it drops the second read entirely and judges + the substitution by whether it would MOVE the card, so there is no window for two reads to + disagree. A test asserting "refuses on a torn snapshot" would have been asserting the mechanism I + removed. + + The negative below survives, and matters more under either design: a selection read that FAILS must + not fail the delegation. + */ + + /* + The paired negative, and the one that keeps the tear check from becoming "refuse whenever the + selection store is flaky": an UNREADABLE selection is unknown, not changed. Failing here would + turn every selection-store hiccup into a failed delegation. + */ + it("still lands the card when a selection read FAILS — unreadable is unknown, not changed", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-selection-unreadable" }); + const store = h.store(); + + const sel = vi.spyOn(store, "getTaskWorkflowSelectionAsync").mockRejectedValue( + new Error("selection store unavailable"), + ); + const tool = api.tools.get("fn_delegate_task")!; + let result: Awaited>; + try { + result = await tool.execute( + "dt-selection-unreadable", + { agent_id: agentId, description: "Selection unreadable" }, + undefined, + undefined, + makeCtx(tmpDir), + ); + } finally { + sel.mockRestore(); + } + + expect(result.isError).not.toBe(true); + expect(result.content[0].text).toContain("will be picked up"); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-15:55 (#2843 review — greptile, "missing hold reports + successful delegation"): + + A BOARD WITH NO HOLD LANE STILL CANNOT DISPATCH THE CARD. + + I first treated this as a benign no-op — the board declares no hold lane, there is nowhere to move + the card, staying on intake is correct. Both halves true, conclusion wrong: assigned-agent + selection picks cards OUT OF the hold lane, so on a board that has none, nothing ever dispatches + this card and "will be picked up on their next heartbeat cycle" is false. + + Unlike the two cases above, this one needs NO stub. A workflow that simply declares no hold-trait + column is an ordinary board someone can build, which is what makes it the reachable one of the + three and worth the most. + */ + it("reports an ERROR when the workflow declares NO hold lane, instead of promising pickup", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-no-hold" }); + const store = h.store(); + const noHold = await store.createWorkflowDefinition({ + name: "No hold lane", + ir: { + version: "v2", + name: "No hold lane", + columns: [ + { id: "ideas", name: "Ideas", traits: [{ trait: "intake" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "ideas" }, + { id: "end", kind: "end", column: "building" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + } as unknown as WorkflowIr, + }); + + const tool = api.tools.get("fn_delegate_task")!; + const result = await tool.execute( + "dt-no-hold", + { agent_id: agentId, description: "Nowhere to land", workflow_id: noHold.id }, + undefined, + undefined, + makeCtx(tmpDir), + ); + + expect(result.isError).toBe(true); + expect(result.content[0].text).not.toContain("will be picked up"); + expect(result.content[0].text).toContain("no hold"); + /* The card exists and is assigned; only the dispatch claim is withheld. */ + expect(result.details.taskId).toBeTruthy(); + expect(result.details.agentId).toBe(agentId); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-17:45 (#2843 review — greptile P1, third round): + THE INVARIANT: a SUBSTITUTED workflow is not the board's answer, even when its lane exists. + + `resolveWorkflowIrForTask` never throws — it substitutes the default coding IR — so a failed + resolution surfaced as the BUILT-IN lanes, `hold: "todo"`. I argued that was harmless because + `todo` would be undeclared on such a board and the move would be rejected. Right about the + mechanism, wrong about the population: a workflow that holds work in `queued` and ALSO declares a + `todo` column for something else gets a move that SUCCEEDS into a lane nothing dispatches from, + and a success message with it. + + The fixture is exactly that shape — `ideas` (intake), `queued` (hold), and a plain `todo` that + carries no lifecycle trait — which is why the older cases could not have caught this: theirs + declared no `todo` at all, so the wrong move was rejected for the wrong reason. + + The stub targets `getTaskWorkflowSelectionAsync`, the READER, not the writer `createTask` uses, + so creation is untouched and only the post-create resolution sees a workflow id that resolves to + nothing. That is what makes this the first case to reach the "could not be resolved" arm rather + than defending it in a comment. + + REVERT PROOF, measured: resolve through `resolveTaskLifecycleColumns` again (no provenance) and + this fails — the tool moves the card to `todo` and reports a successful delegation. + */ + it("does not trust a SUBSTITUTED workflow's lanes even when the board declares that column", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-substituted-workflow" }); + const store = h.store(); + const declaresTodo = await store.createWorkflowDefinition({ + name: "Queued hold, unrelated todo", + ir: { + version: "v2", + name: "Queued hold, unrelated todo", + columns: [ + { id: "ideas", name: "Ideas", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold" }] }, + /* Declared, carries no lifecycle role — the column the built-in fallback would name. */ + { id: "todo", name: "Someday", traits: [] }, + ], + nodes: [ + { id: "start", kind: "start", column: "ideas" }, + { id: "end", kind: "end", column: "queued" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + } as unknown as WorkflowIr, + }); + + /* Reader only: `createTask` writes the selection, it does not read it through this method. */ + const selection = vi.spyOn(store, "getTaskWorkflowSelectionAsync") + .mockResolvedValue({ workflowId: "wf-vanished", stepIds: [] }); + const tool = api.tools.get("fn_delegate_task")!; + let result: Awaited>; + try { + result = await tool.execute( + "dt-substituted", + { agent_id: agentId, description: "Workflow resolves to a substitute", workflow_id: declaresTodo.id }, + undefined, + undefined, + makeCtx(tmpDir), + ); + } finally { + selection.mockRestore(); + } + + expect(result.isError).toBe(true); + expect(result.content[0].text).not.toContain("will be picked up"); + /* The card must NOT have been moved into the built-in fallback's lane. */ + const { task } = await readTaskWorkflowState(tmpDir, result.details.taskId); + expect(task.column).not.toBe("todo"); + }); + + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-20:30 (#2843 review — greptile P1, "fallback equality + masks wrong hold"): + EQUALITY WITH A FABRICATED LANE IS NOT EVIDENCE. + + The board the review names, and the one every earlier case here misses: `todo` is INTAKE and + `queued` is hold. `createTask` puts the card on `todo`; a degraded lookup fabricates the built-in + `hold: "todo"`; the two match, so the "no move needed" rule accepted it and reported a delegation + for a card assigned-agent dispatch will never select, because the real hold lane is `queued`. + + Settled with `params.workflow_id` — caller INPUT, so there is no second snapshot to race. A + caller that named a workflow proves a real one exists, so a substitution proves we failed to read + it, and its hold lane cannot be inferred from the built-in vocabulary. + + The neighbouring degraded-resolve cases stay green precisely because they pass NO `workflow_id`: + there, "substituted" and "this project has no resolvable workflow" are the same observation and + the card belongs where it is. That distinction is the whole content of the fix. + + REVERT PROOF, measured: drop the `substituted && workflowId` arm and this fails on `isError` — + the tool reports a successful delegation for a card sitting on intake. + */ + it("refuses to claim pickup when the NAMED workflow could not be resolved, even if lanes match", async () => { + const agentId = await seedAgent(tmpDir, { name: "delegate-todo-intake-queued-hold" }); + const store = h.store(); + const todoIntake = await store.createWorkflowDefinition({ + name: "Todo intake, queued hold", + ir: { + version: "v2", + name: "Todo intake, queued hold", + columns: [ + /* `todo` is INTAKE here — the id the built-in fallback would call hold. */ + { id: "todo", name: "Inbox", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { id: "end", kind: "end", column: "queued" }, + ], + edges: [{ from: "start", to: "end", condition: "success" }], + } as unknown as WorkflowIr, + }); + + /* Reader only — `createTask` writes the selection rather than reading it through this method. */ + const selection = vi.spyOn(store, "getTaskWorkflowSelectionAsync") + .mockResolvedValue({ workflowId: "wf-vanished", stepIds: [] }); + const tool = api.tools.get("fn_delegate_task")!; + let result: Awaited>; + try { + result = await tool.execute( + "dt-todo-intake", + { agent_id: agentId, description: "Intake is called todo here", workflow_id: todoIntake.id }, + undefined, + undefined, + makeCtx(tmpDir), + ); + } finally { + selection.mockRestore(); + } + + expect(result.isError).toBe(true); + expect(result.content[0].text).not.toContain("will be picked up"); + /* Still on intake, and the message must be about the workflow rather than about a move. */ + const { task } = await readTaskWorkflowState(tmpDir, result.details.taskId); + expect(task.column).toBe("todo"); + }); + it("rejects unknown agent", async () => { const tool = api.tools.get("fn_delegate_task")!; const result = await tool.execute( diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 48f1ec2c86..7fbf64ec05 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -35,7 +35,10 @@ import { resolveTaskGithubTracking, formatCurrentTaskLine, type SecretScope, + declaresAnyLifecycleTrait, resolveTaskLifecycleColumns, + resolveLifecycleColumns, + resolveWorkflowIrForTaskWithProvenance, resolveWorkflowIrForTask, resolveReviewColumns, } from "@fusion/core"; @@ -5185,15 +5188,16 @@ export default function kbExtension(pi: ExtensionAPI) { name: "fn_delegate_task", label: "fn: Delegate Task", description: - "Create a new task and assign it to a specific agent for execution. The task goes to " + - "'todo' and will be picked up by the target agent on their next heartbeat cycle. " + + "Create a new task and assign it to a specific agent for execution. The task lands in the " + + "selected workflow's ready lane (`todo` on the built-in board, whatever that workflow calls " + + "it otherwise) and will be picked up by the target agent on their next heartbeat cycle. " + "Use fn_list_agents first to find available agents and their capabilities. " + "Optionally pass workflow_id to select a workflow at creation time; use " + "fn_workflow_list to discover valid IDs.", promptSnippet: "Delegate a task to a specific Fusion agent", promptGuidelines: [ "Use fn_list_agents first to find available agents and their capabilities", - "The task is created in 'todo' and assigned to the target agent", + "The task is created in the workflow's ready (hold) lane and assigned to the target agent", "Cannot delegate to ephemeral/runtime agents", "Implementation tasks use executor by default; durable engineer supports explicit routing without override, other non-executor roles require override=true", "Optionally specify dependencies on other tasks", @@ -5237,10 +5241,35 @@ export default function kbExtension(pi: ExtensionAPI) { // Create task assigned to the target agent const store = await getStore(ctx.cwd); const workflowId = params.workflow_id?.trim() || undefined; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-14:30: + Delegation lands in the HOLD lane of the workflow the card ACTUALLY got, not in the literal + `todo` and not in a lane resolved from a second, independent read. + + The tool's contract (see the description above) is "the task goes to the ready-to-work lane + and the target agent picks it up on its next heartbeat" — deliberately NOT intake, so this + cannot simply omit `column` and inherit `createTask`'s intake resolution: on a manual-intake + workflow the card would sit waiting for a human and never reach the agent it was delegated + to. Keyed on the literal, a workflow that calls that lane anything else received a card in an + undeclared column: written, reported as delegated, invisible to the agent. + + RESOLVED FROM THE CREATED TASK, and that ordering is the fix rather than a detail (#2843 + review, greptile P1 — it is right). My first version resolved the hold column BEFORE the + create, from `getDefaultWorkflowId()`. `createTask` then resolves the project default AGAIN + to select the workflow, so two reads answered the same question and a default changed between + them yields a column from workflow A written onto a card selected into workflow B — the exact + undeclared-lane write this conversion exists to remove, reintroduced by the fix for it. + + Resolving afterwards leaves ONE authority: the task's own selection. The move is skipped + entirely when the entry lane already carries `hold` — true on the built-in board, where entry + and hold are both `todo` — so the common path is byte-identical and costs no extra write. + + A failed move is reported, not swallowed: the card exists either way, and telling the caller + it is ready when it is sitting in intake is the failure mode this whole change is about. + */ const task = await store.createTask({ description: params.description, dependencies: params.dependencies, - column: "todo", assignedAgentId: params.agent_id, ...(workflowId ? { workflowId } : {}), source: { @@ -5249,15 +5278,173 @@ export default function kbExtension(pi: ExtensionAPI) { }, }); + let landedColumn = task.column; + let landingError: string | undefined; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-15:20 (#2843 review — coderabbit major / greptile P1): + AN UNRESOLVABLE WORKFLOW, AN UNTRAITED BOARD, AND A BOARD WITH NO HOLD LANE ARE THREE ANSWERS. + + Reading `?.hold` off the result collapsed the first two into one: both produced `undefined`, + the move was skipped, and the tool reported success — so a split-lane workflow whose IR could + not be read left the card on intake while telling the caller it was on its way. That is the + same success-with-nothing-behind-it this branch was rewritten to remove, arriving a line early. + + - `undefined` OBJECT — the workflow could not be resolved at all. Nothing can be verified, so + the landing is reported as failed rather than assumed. + - resolved, NO column declares any lifecycle trait — a v1 graph upgraded by + `synthesizeDefaultColumns`, or a fixture like `linearWorkflowIr`. Its `todo` column plainly + exists and is where agents pick work up, so the legacy vocabulary still applies and success + is honest. Failing these would break every untraited board to fix a case none of them have, + and the existing suite caught exactly that when this gate was first written without it. + - resolved, traits EXPRESSED, still no hold lane — the board has answered, and the answer is + that nothing will dispatch this card: assigned-agent selection picks OUT OF the hold lane. + Staying on intake is correct and "will be picked up" is false, so it is reported like any + other failed landing. + + A FOURTH STATE, and it is the one that made the first arm look unreachable (#2843 review, + greptile P1 — the third round, and right again). `resolveWorkflowIrForTask` never throws: an + unreadable selection, a missing definition, a malformed one and a throwing lookup ALL + SUBSTITUTE the default coding IR. So the substitution never surfaced as a failure, it surfaced + as the BUILT-IN lanes — `hold: "todo"`. + + I argued that was harmless because `todo` would be undeclared on such a board and the move + would be rejected. That is true only of a board that does not declare `todo` AT ALL. A + workflow that holds work in `queued` and ALSO declares a `todo` column for something else + gets a move that SUCCEEDS into a lane nothing dispatches from, and a success message. The + argument was right about the mechanism and wrong about the population it covers. + + `resolveWorkflowIrForTaskWithProvenance` is the seam built for exactly this: it reports + `source: "selection"` ONLY when it genuinely resolved the workflow the task selected, and + `"default"` whenever it substituted. Pairing that with the task's own selection separates the + two reasons for `"default"` — no workflow selected (legitimate; the built-in board IS the + answer) from selected-but-unresolvable (a substitution wearing the board's authority). + + This also makes the first arm REACHABLE, so it is now covered rather than defended. + */ + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-19:10 (#2843 review — greptile P1, fourth round): + ONE READ. Provenance and lanes come from the same snapshot, and a substitution is judged by + whether it would MOVE the card rather than by a second lookup. + + My previous fix paired the provenance read with an independent `getTaskWorkflowSelectionAsync` + to tell "no workflow selected" (legitimate — the built-in board IS the answer) apart from + "selected but unresolvable". Two reads answering one question is the same defect the FIRST + review round on this PR caught, reintroduced three rounds later in a different place: a + selection written or cleared between them makes `resolved.ir` describe one workflow and + `selectedWorkflowId` another, and the tool then moves the card with the wrong board's hold + lane or reports a resolution error that never happened. + + The second read is not needed. `source === "selection"` means the resolver genuinely resolved + what the task selected, so the lanes are authoritative. `"default"` means it SUBSTITUTED — + and the honest question is not why, it is whether the substitution is about to be acted on: + + - the substituted hold lane equals the card's current column: no move, nothing is being + decided on unverified information, and this is exactly the no-selection/untraited case + where the built-in vocabulary is correct. Success. + - it DIFFERS: acting would move a real card into a lane inferred from a workflow that is + not the card's. That is the misroute round three found, and it is refused. + + Judging by the action rather than by the reason needs no second lookup, so there is no window + for the two to disagree. + */ + const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id); + /* `resolveLifecycleColumns` returns undefined for an IR that declares no columns at all; + `holdColumn` is then undefined and the untraited branch below is the honest answer. */ + const holdColumn = resolveLifecycleColumns(resolved.ir)?.hold; + const substituted = resolved.source !== "selection"; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-20:30 (#2843 review — greptile P1, "fallback equality + masks wrong hold"): + A SUBSTITUTION IS FATAL WHEN THE CALLER NAMED THE WORKFLOW. Equality proves nothing on its own. + + The equality rule below accepts a substitution whose hold lane already matches the card's + column, on the grounds that no move means nothing was decided on bad information. The review + names the board where that is false: a workflow using `todo` as INTAKE and `queued` as hold + puts the card on `todo`, the degraded lookup fabricates `hold: "todo"`, the two match, and the + delegation is reported for a card assigned-agent dispatch will never select. + + `params.workflow_id` settles the decidable half WITHOUT a second read — it is caller input, so + there is no snapshot to race. When the caller NAMED a workflow and the resolver substituted, + a real workflow provably exists and we provably failed to read it, so its hold lane cannot be + inferred from the built-in vocabulary and the landing is refused. + + WHAT THIS DOES NOT COVER, stated because the gap is real: the same board reached through the + project DEFAULT with no `workflow_id` argument. There, "substituted" and "this project has no + resolvable workflow" are the same observation, and the second is a legitimate configuration + whose cards belong exactly where they are. Failing it would trade a false success for a false + alarm on a valid setup. Distinguishing them needs the read that just failed; it is not + available here, and guessing is what produced the last four rounds of this review. + */ + if (substituted && workflowId) { + landingError = `the requested workflow (${workflowId}) could not be resolved, so its ready lane is unknown`; + } else if (substituted && holdColumn && holdColumn !== task.column) { + landingError = "the task's workflow could not be resolved, so its ready lane is unknown"; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-19:35 (#2843 review — greptile P1, "lifecycle snapshot + race persists"): + THE SAME SNAPSHOT, NOT A SECOND RESOLVE. + + This read the traits through a helper that resolved the workflow AGAIN, so a selection changing + in between combined the hold column from one board with the trait state of another — the exact + two-reads-of-one-fact defect the round above removed, surviving in the branch I added to fix a + different one. `resolved.ir` is already in hand and is the only snapshot anything here should + consult. + */ + } else if (!holdColumn && declaresAnyLifecycleTrait(resolved.ir)) { + landingError = "this workflow declares no hold (ready-to-pick-up) lane"; + } else if (holdColumn && holdColumn !== task.column) { + try { + await store.moveTask(task.id, holdColumn); + landedColumn = holdColumn; + } catch (moveError) { + landingError = moveError instanceof Error ? moveError.message : String(moveError); + } + } + const deps = task.dependencies.length ? ` (depends on: ${task.dependencies.join(", ")})` : ""; const workflow = workflowId ? ` (workflow: ${workflowId})` : ""; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-14:50 (#2843 review — greptile P1, and it is right): + A FAILED landing is an ERROR result, not a success sentence with a warning appended. + + My first version kept the "will be picked up by X on their next heartbeat cycle" text and + added a ` WARNING: ...` suffix. That still LEADS with a claim that is false: assigned-agent + selection skips cards outside the workflow's hold lane, so a card stranded on intake is never + dispatched, and a caller — usually another agent — reads the first sentence and moves on. + "Reported success with a caveat" is how the delegation silently goes nowhere. + + The task id stays in `details` because the card DOES exist and the caller needs it to finish + the job by hand; what changes is that nothing in this branch claims the delegation completed. + + The MOVE-REJECTED half of this is defensive rather than a live path: `holdColumn` comes from + the task's own resolved IR, so `moveTask`'s declared-column check passes by construction. I + tried to reach it with a workflow declaring a `hold` column with no node on it, expecting a + rejection — the move SUCCEEDED and the card landed there, because `moveTask` accepts any + column the workflow DECLARES and node reachability does not gate it. The unresolvable-workflow + half above is reachable. Both are covered by the message-shape test, which is explicit that it + pins the handling and not the reachability. + */ + if (landingError) { + const target = holdColumn ? `the ready lane "${holdColumn}"` : "its ready lane"; + const remedy = holdColumn + ? `move it to "${holdColumn}" to dispatch it` + : "resolve its workflow and move it to that workflow's ready lane to dispatch it"; + const text = `ERROR: Created ${task.id}${deps}${workflow} and assigned it to ${agent!.name} (${agent!.id}), ` + + `but it could NOT be moved out of "${task.column}" into ${target}: ${landingError}. ` + + `It is stranded on intake and will NOT be picked up — ${remedy}.`; + return { + content: [{ type: "text" as const, text }], + isError: true, + details: { taskId: task.id, agentId: agent!.id, agentName: agent!.name, column: landedColumn, error: landingError }, + }; + } return { content: [{ type: "text" as const, text: `Delegated to ${agent!.name} (${agent!.id}): Created ${task.id}${deps}${workflow}. ` + `The task will be picked up by ${agent!.name} on their next heartbeat cycle.`, }], - details: { taskId: task.id, agentId: agent!.id, agentName: agent!.name }, + details: { taskId: task.id, agentId: agent!.id, agentName: agent!.name, column: landedColumn }, }; } catch (error) { if (error instanceof Error && error.message.startsWith("Task ID already exists:")) { diff --git a/packages/dashboard/src/__tests__/routes-gitlab.test.ts b/packages/dashboard/src/__tests__/routes-gitlab.test.ts index b7c4a0467e..87ce30d9d4 100644 --- a/packages/dashboard/src/__tests__/routes-gitlab.test.ts +++ b/packages/dashboard/src/__tests__/routes-gitlab.test.ts @@ -90,6 +90,32 @@ describe("GitLab import routes", () => { expect((dup.body as any).existingTaskId).toBe("FN-001"); }); + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-21:55: + THE INVARIANT: an import names no column, so the card enters the workflow's OWN intake lane. + + This route passed `column: "triage"`. U11 deleted `triage` — the default board's lanes are + `todo | in-progress | in-review | done | archived` — and an explicit `column` OVERRIDES the intake + column `createTask` resolves for the workflow it selects. So every GitLab import wrote its row into + a lane no workflow declares. Nothing rejects it and nothing logs it: the route answers 201 with a + task id and the card is simply not on the board. + + Asserted as ABSENCE rather than as a resolved id on purpose — the store here is a fake whose + `createTask` echoes its input, so a resolved value would be testing the fake. Absence is exactly + the property that hands the decision to the real `createTask`, and it is what fails on revert. + */ + it("imports without naming a column, so createTask resolves the workflow's intake lane", async () => { + const fetchImpl = vi.fn().mockImplementation(() => Promise.resolve(jsonResponse({ id: 9, iid: 9, project_id: 3, title: "Lane", description: "Body", web_url: "https://gitlab.example.com/g/p/-/issues/9", state: "opened", labels: [] }))); + const { app, store } = buildApp(fetchImpl); + + const res = await request(app, "POST", "/api/gitlab/project/issues/import", JSON.stringify({ project: 3, iid: 9 }), { "Content-Type": "application/json" }); + + expect(res.status).toBe(201); + const created = store.createTask.mock.calls[0][0]; + expect(created.column).toBeUndefined(); + expect(Object.keys(created)).not.toContain("column"); + }); + /* FNXC:IssueImportAttachments 2026-07-15-13:40: GitLab imports must hand the agent the same `.fusion/tasks//attachments/` contract as GitHub imports — the agent-facing behavior cannot depend on which forge the issue came from. diff --git a/packages/dashboard/src/routes/register-gitlab.ts b/packages/dashboard/src/routes/register-gitlab.ts index cb6ed8005c..a6d115d883 100644 --- a/packages/dashboard/src/routes/register-gitlab.ts +++ b/packages/dashboard/src/routes/register-gitlab.ts @@ -102,10 +102,20 @@ async function importItem(ctx: ApiRoutesContext, req: Parameters