the unwired-seam audit: 9 defects the census cannot see, incl. a reviewed card that cannot merge (#2820)
**Nine operator-visible defects in a class the census cannot see, plus
the audit method that found them.**
The census scans for lifecycle-column **comparisons**. This PR is about
guards that have no literal to find: a helper takes an optional
*resolved* lane set, its own test passes it, the census entry is gone —
and the callers pass nothing. **A resolved seam nobody wired is
indistinguishable from no seam at all.**
## What was broken
| defect | operator sees |
| --- | --- |
| `getTaskMergeBlocker` unwired in `mergeTaskImpl` | `Cannot merge FN-1:
task is in 'checking', must be in 'in-review'` — **a reviewed card
cannot merge** |
| …and in the completion move | `Cannot move FN-1 to done: …` — **and
cannot complete** |
| `isParkedTaskColumn` unwired ×2 (`agent-heartbeat`) | a durable agent
keeps claiming a parked card; **Health Check renders it RUNNING** |
| `resolveLinkSyncColumnRoles` first-per-role | link hygiene skips a
**second hold lane** entirely |
| `executor` active-task predicate first-per-role | a card in a **second
wip lane reads as INACTIVE**; its prompt file becomes reclaimable |
| `isPlanningContinuationTaskDispatchable` partially threaded | a board
declaring `done` as *non-terminal* stalls its cards — **stalled by a
lane name** |
| `default-workflow-hooks:72`, `executor:2404` | resolved gate admits
the move, unresolved blocker refuses it |
## The recurring shape, which is sharper than "a caller forgot an
argument"
Four sites resolve the lane and then re-ask with the literal, **a few
lines apart in the same function**:
- `task-artifacts-ops` resolves `completeColumn`, then asks the blocker
with the literal.
- `default-workflow-hooks:72` gates on `lifecycleColumns?.review`, then
the literal.
- `executor:2404` compares `resolveResumeLanes(…).review`, then the
literal.
- `resolvePlanningContinuationCandidate` applies the caller's terminal
set, then delegates without it.
**Grep for the helper, not the literal.** The literal is one function
away, correctly annotated as a fallback — which is exactly why the
census is blind to all of it.
## The arity trap, named and measured (six occurrences, one caught by
review here)
`resolveLifecycleColumns` answers *"which column is **the** hold
lane?"*. A `.includes()`/`.has()` test asks *"is this **any** hold
lane?"*. Nothing distinguishes them — same types, no literal.
**A default-vs-renamed differential cannot catch it**, because the
default board declares one column per role and therefore cannot express
the failing shape. It needs a *structurally* different fixture. That is
a sharper rule than "test both vocabularies", and it would have caught
all six.
Scanned: 12 candidate sites. **4 fixed · 3 blocked (2 on the inert sync
IR reader; `triage:833` also query-shaped) · 1 needs a hook-contract
change · 3 not defects (a returned tuple; an ordering-sensitive
precedence list) · 1 false positive of my own scan.**
A sweep over all twelve would have broken the ordering-sensitive pair,
delivered nothing at the sync-blocked ones, and "fixed" a site that was
already correct.
## Two traps in fixing this class — I hit both here
1. **The legacy id is a FALLBACK, not a member.** Pre-seeding
`"in-review"` admits a board that *declares* `in-review` as its WIP
column — a card mid-implementation merges prematurely. A real resolved
answer must **replace** the default. (Caught by review; it is the same
unscoped-legacy-acceptance the glasses plugin's review caught earlier,
which I had read and reintroduced.)
2. **Two guards, one assertion.** `toContain("must be in")` passed with
`mergeTaskImpl` reverted, because the *completion* guard caught the card
instead. The assertion now names the site (`Cannot merge` vs `Cannot
move … to done`) so the two fail independently.
## Corrections I made to my own work, recorded rather than quietly fixed
- My first PG test was **vacuous three ways**:
`saveWorkflowDefinition?.()`/`setTaskWorkflowSelection?.()` do not exist
(the `?.` swallowed both, so the task kept the builtin workflow),
`updateTask({column})` does not move a card, and a two-node IR made
every setup move illegal. Premise is now **asserted**, not assumed.
- My doc claimed the audit was complete. It enumerated **helpers**, not
every **caller** — `getTaskMergeBlocker` alone has 13 call sites.
Corrected in place, with the still-unwired ones listed by file and line
and a note to distrust any "audit complete" claim including mine.
- A severity correction to another worker's E2E:
`selectActionablePlanningContinuations` has **no production caller**, so
its stated consequence is latent, not live.
## Verification
- `pnpm test:gate` — 161 + 487 + 13 + 71
- `tsc` on core and engine; `pnpm lint`; `check:changesets`; census
`--strict` — all clean, each run explicitly
- Every fix revert-measured; each has a non-vacuous companion. The
two-hold-lane and repurposed-`in-review` cases exist because the default
board cannot express those shapes.
## Deliberately not done, with reasons in
`resolved-seams-nobody-wired.md`
`isTaskReadyForMerge` (dead in production — wiring it would be the
anti-pattern itself); `getTaskHardMergeBlocker` (3 of 4 callers are
query-gated sweeps); `getInReviewStallReason` (needs a **batch
prefetch**, not a per-task resolve — its callers decorate every task on
every list read; the in-review stall badge is wrong on renamed boards
until then); `default-workflow-hooks` planning/live-work sets (needs
`DefaultWorkflowMoveContext` to carry the IR — a shared contract
change).
This commit is contained in:
gsxdsm
7
.changeset/heartbeat-parked-link-renamed-lanes.md
Normal file
7
.changeset/heartbeat-parked-link-renamed-lanes.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Agents no longer appear to be running a parked task on boards with renamed columns.
|
||||
category: fix
|
||||
dev: Both `isParkedTaskColumn` call sites in `agent-heartbeat.ts` omitted the resolved `parkedColumns` argument and took the legacy `todo`/`triage` default, so the stale-link clear never fired on a renamed board. Both now pass the task's resolved `hold`/`intake` lanes.
|
||||
7
.changeset/merge-blocker-renamed-review-lane.md
Normal file
7
.changeset/merge-blocker-renamed-review-lane.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Tasks can be merged and completed on boards whose review column is renamed.
|
||||
category: fix
|
||||
dev: Two `getTaskMergeBlocker` callers omitted the optional resolved `reviewColumns`, so the identity check fell back to the literal `in-review` and refused a card sitting in its own board's review lane — `mergeTaskImpl` threw "Cannot merge …" and the completion move threw "Cannot move … to done". Both now pass `resolveReviewColumns` from the task's workflow, unioned with the legacy id.
|
||||
@@ -0,0 +1,251 @@
|
||||
---
|
||||
category: architecture-patterns
|
||||
module: workflow-resolved-columns
|
||||
date: 2026-07-30
|
||||
problem_type: systemic_gap
|
||||
component: core
|
||||
severity: high
|
||||
applies_when:
|
||||
- "Converting a guard by adding an optional resolved-lane parameter"
|
||||
- "Reviewing a conversion that added a parameter"
|
||||
- "Auditing what a renamed board still breaks after the census reaches zero"
|
||||
tags:
|
||||
- workflow-resolved-columns
|
||||
- column-census
|
||||
- unwired-parameter
|
||||
- census-invisible
|
||||
---
|
||||
|
||||
# A resolved seam nobody wired is indistinguishable from no seam at all
|
||||
|
||||
## The shape
|
||||
|
||||
The standard conversion in this program adds an optional resolved parameter with a legacy default:
|
||||
|
||||
```ts
|
||||
export function isParkedTaskColumn(
|
||||
task: Pick<Task, "column">,
|
||||
parkedColumns: readonly string[] = LEGACY_PARKED_COLUMNS,
|
||||
): boolean
|
||||
```
|
||||
|
||||
The helper is now correct, its own test passes, and the census entry is gone — the literal moved into a
|
||||
documented default. **But every caller that does not pass the argument still gets the legacy behaviour**,
|
||||
and nothing in the codebase records that.
|
||||
|
||||
This is worse than an unconverted literal in one specific way: the literal is *visible* to the census and
|
||||
to grep. A converted helper with unwired callers looks finished from every angle except the call site.
|
||||
|
||||
## Why the seam test cannot catch it
|
||||
|
||||
The seam test supplies the parameter — that is what it is testing. It proves the helper honours a resolved
|
||||
set. It says nothing about whether anyone supplies one. So the suite is green, the census is clean, and
|
||||
the guard is inert in production.
|
||||
|
||||
Same root as the optional-flags blind spot (`optional-flags-seam-hides-unconverted-column-guards.md`), one
|
||||
level up: there the *test* omits the parameter, here the *caller* does.
|
||||
|
||||
## The audit, and its result
|
||||
|
||||
Method — cheap and repeatable:
|
||||
|
||||
1. Find helpers with an optional resolved-lane parameter:
|
||||
|
||||
```bash
|
||||
grep -rn "ReadonlySet<string>\|ColumnRoleFlags\|LifecycleColumns" packages/*/src --include=*.ts \
|
||||
| grep -v __tests__
|
||||
```
|
||||
then keep the signatures where that type appears on an **optional** parameter (`name?: ReadonlySet<string>`).
|
||||
|
||||
2. For each, grep every call site and check whether the argument is actually passed.
|
||||
|
||||
Result across `core`, `engine`, `dashboard`, `cli` — **13 such helpers**:
|
||||
|
||||
| disposition | count | notes |
|
||||
| --- | ---: | --- |
|
||||
| callers wired | 7 | `isStaleBlockedByBlocker`, `areAllDependenciesDone`, `enqueue`/`dequeueMergeQueueInTransaction`, the three `restart-recovery-coordinator` predicates, `isTerminalTaskStatus` |
|
||||
| **unwired — fixed** | **5** | see below |
|
||||
| left deliberately | 1 | `selectActionablePlanningContinuations` — **no production caller**; wiring a parameter into a function nothing calls is the unwired-parameter anti-pattern the caller audit (#2803) removed five of |
|
||||
| not lifecycle | 1 | `isBuiltinWorkflowEnabled` |
|
||||
|
||||
**SCOPE OF THAT TABLE, corrected.** It enumerates the HELPERS and the call sites reached while fixing
|
||||
them. It does NOT claim every caller of every helper was audited — and for at least one helper that
|
||||
distinction matters a lot. `getTaskMergeBlocker` alone has **13 call sites** across core and engine;
|
||||
two are fixed here, two were already wired (`moves.ts:821` passes `moveLifecycle?.review`;
|
||||
`moves.ts:647` deliberately passes `skipColumnIdentityCheck` because the trait was already proven), four
|
||||
sit inside self-healing sweeps that are query-gated and never run on a renamed board anyway, and the
|
||||
**remaining five are genuinely unwired**:
|
||||
|
||||
```text
|
||||
packages/core/src/default-workflow-hooks.ts:72
|
||||
packages/core/src/task-merge.ts:355, 377
|
||||
packages/core/src/in-review-stall.ts:237
|
||||
packages/engine/src/merger.ts:6645
|
||||
packages/engine/src/merger-ai.ts:1173
|
||||
packages/engine/src/executor.ts:2404
|
||||
```
|
||||
|
||||
Run step 2 of the method per helper before believing any "audit complete" claim, including this one.
|
||||
|
||||
Two of those five are now fixed (`default-workflow-hooks:72`, `executor.ts:2404` — both had the resolved
|
||||
lane already in scope). The remaining three are each blocked on something other than effort, and the
|
||||
reason matters more than the count:
|
||||
|
||||
- **`task-merge.ts:377` (`isTaskReadyForMerge`) — dead in production.** Exported and referenced only by
|
||||
the index barrels and its own test. Wiring a parameter into a function nothing calls is the
|
||||
unwired-parameter anti-pattern itself; it is left, like `selectActionablePlanningContinuations`.
|
||||
- **`task-merge.ts:355` (`getTaskHardMergeBlocker`) — mostly query-gated.** Three of its four callers are
|
||||
self-healing sweeps behind hardcoded `listTasks({ column: "in-review" })`, so they never run on a
|
||||
renamed board regardless (see the self-healing doc). Only `project-engine.ts:3537` is live.
|
||||
- **`in-review-stall.ts:237` (`getInReviewStallReason`) — needs a batch prefetch, not a per-task resolve.**
|
||||
Its four callers are in `reads.ts`, which decorates EVERY task on every list read. A per-task
|
||||
`resolveWorkflowIrForTask` there costs one IR resolution per row on a hot path. The correct shape is the
|
||||
prefetched per-workflow map used by the converted self-healing sweeps — resolve once per workflow for
|
||||
the batch, then index by task. That is a performance-shaped change, not a one-argument wiring, which is
|
||||
why it is recorded here rather than done in passing.
|
||||
|
||||
The operator-visible cost of the last one is the in-review stall badge: on a renamed board the stall
|
||||
reason is computed against the legacy lane, so the badge can be wrong for every card in review.
|
||||
|
||||
## The recurring shape: outer question resolved, inner one not
|
||||
|
||||
The sharpest instances are not "a caller forgot an argument" but "the same function resolved the lane and
|
||||
then re-asked with the literal", a few lines apart:
|
||||
|
||||
- `task-artifacts-ops` resolves `completeColumn` from the workflow, then calls the blocker with the literal.
|
||||
- `default-workflow-hooks:72` gates on `lifecycleColumns?.review` / `?.complete`, then calls it with the literal.
|
||||
- `executor.ts:2404` compares against `(await this.resolveResumeLanes(taskId)).review`, then calls it with the literal.
|
||||
- `resolvePlanningContinuationCandidate` applies the caller's resolved terminal set, then delegates without it.
|
||||
|
||||
A conversion that stops at the outer question looks complete at the call site and is not. Grep for the
|
||||
helper, not for the literal — the literal is one function away, where it is correctly annotated as a
|
||||
fallback.
|
||||
|
||||
The five that were unwired at the sites reached here, and what each cost on a renamed board:
|
||||
|
||||
- **`isParkedTaskColumn` ×2** (`agent-heartbeat`) — the stale-link clear never fired, so a durable agent
|
||||
kept claiming a parked card and Reports Health Check rendered it **RUNNING**.
|
||||
- **`getTaskMergeBlocker` ×2** (`mergeTaskImpl`, the completion move) — a card that had passed review
|
||||
**could not be merged or completed at all**: `Cannot merge …: task is in 'checking', must be in
|
||||
'in-review'`.
|
||||
- **`isPlanningContinuationTaskDispatchable`** (`in-process-runtime`) — partially threaded: the enclosing
|
||||
function applied the caller's resolved set to its own check, then delegated *without* it.
|
||||
|
||||
Note the second entry: `task-artifacts-ops` **already resolved the completion lane four lines above** the
|
||||
call that re-asked with the literal. The outer question was resolved and the inner one was not, inside one
|
||||
function.
|
||||
|
||||
## The arity trap: first-per-role where membership was meant
|
||||
|
||||
Six occurrences in this program, and the sixth was found only because a reviewer caught the fifth. It
|
||||
deserves naming separately because it survives every check the others fail:
|
||||
|
||||
```ts
|
||||
const parked = [lifecycle.hold, lifecycle.intake].filter(isString); // FIRST per role
|
||||
…
|
||||
if (parked.includes(task.column)) … // MEMBERSHIP test
|
||||
```
|
||||
|
||||
`resolveLifecycleColumns` / `resolveTaskLifecycleColumns` answer **"which column is THE hold lane?"**.
|
||||
A `.includes()` / `.has()` test asks **"is this column ANY hold lane?"**. On a workflow declaring two
|
||||
columns with the same trait, the first answer silently covers one of them.
|
||||
|
||||
**Why nothing catches it:**
|
||||
|
||||
- The census sees no literal — the lanes are resolved.
|
||||
- The types are identical: `string | undefined` either way.
|
||||
- A default-vs-renamed differential passes, because **the default board declares one column per role and
|
||||
therefore cannot express the failing shape**. It needs a *structurally* different fixture, not a
|
||||
differently-named one.
|
||||
|
||||
Measured across production code — collections built from two or more first-per-role reads: **12 sites**.
|
||||
Two are fixed here (`agent-heartbeat` ×2 call sites, `task-agent-sync.resolveLinkSyncColumnRoles`). The
|
||||
rest are recorded for per-site classification, since some are legitimately ordered tuples rather than
|
||||
membership sets:
|
||||
|
||||
| site | disposition |
|
||||
| --- | --- |
|
||||
| `engine/src/agent-heartbeat.ts` ×2 | **fixed** |
|
||||
| `engine/src/task-agent-sync.ts:62-63` | **fixed** |
|
||||
| `engine/src/executor.ts:12535` | **fixed** — a card in a second wip lane read as INACTIVE and its prompt file as reclaimable |
|
||||
| `engine/src/executor.ts:3041` | **blocked** — reads `resolvePlannerLanes`, i.e. the sync IR reader, which returns the default workflow for every task in production. Converting the arity here changes nothing until that is fixed. |
|
||||
| `engine/src/scheduler.ts:1593` | **blocked** — same, via `resolveTaskParkedColumnsSync` |
|
||||
| `core/src/workflow-lifecycle-traits.ts:231` | **not a membership use** — a returned 2-tuple |
|
||||
| `engine/src/planner-lane-resolution.ts:70, 84` | **ordering-sensitive** — a precedence list, not a set; converting would change which lane wins |
|
||||
| `core/src/default-workflow-hooks.ts:274, 280` | **genuine, but a contract change** — `planningColumnsOf`/`liveWorkColumnsOf` take a `LifecycleColumns`, which is first-per-role *by construction*. Fixing the arity means `DefaultWorkflowMoveContext` carrying the IR (or trait sets) instead, which is a shared hook contract touching every caller. Not a local edit. |
|
||||
| `engine/src/triage.ts:833` | **blocked twice over** — built from `resolvePlannerLanes` (the sync reader) *and* used to build `listTasks({ column })` queries, so it is in the query-filter class too |
|
||||
| `dashboard/src/routes/register-task-workflow-routes.ts:304` | **FALSE POSITIVE of the scan** — already correct: `columnsWithFlag` for both roles, unioned, legacy only when the resolved set is empty |
|
||||
|
||||
So of twelve: **four fixed, three blocked (two on the sync reader, one also query-shaped), one needs a
|
||||
contract change, three are not defects, one was my scan misfiring.**
|
||||
|
||||
Two things that spread is worth saying out loud:
|
||||
|
||||
1. A sweep converting all twelve would have **broken** the ordering-sensitive pair, delivered **nothing**
|
||||
at the sync-blocked ones, and "fixed" a site that was already right.
|
||||
2. **The scan itself has false positives**, because it classifies by syntax — two role-shaped reads and a
|
||||
bracket on one line. That is the same failure this program has documented about the census: an
|
||||
instrument that counts syntax and is read as counting meaning. Treat the twelve as candidates, never
|
||||
as a backlog.
|
||||
|
||||
Re-measure with: for each file importing a lifecycle resolver, flag lines containing **two or more**
|
||||
`.hold`/`.intake`/`.review`/`.wip`/`.complete`/`.archived` reads **and** an array or `new Set(`.
|
||||
|
||||
The fix is always `columnsWithFlag(ir, trait)`, which returns every column carrying it.
|
||||
|
||||
## Two traps when fixing these
|
||||
|
||||
**1. The legacy id is a FALLBACK, not a member.** The tempting shape is wrong:
|
||||
|
||||
```ts
|
||||
const reviewColumns = new Set(["in-review"]); // WRONG
|
||||
for (const c of resolveReviewColumns(ir)) reviewColumns.add(c);
|
||||
```
|
||||
|
||||
That admits a board which *declares* `in-review` as its WIP column — a card mid-implementation passes the
|
||||
merge check. The legacy id is only correct when the board tells us nothing:
|
||||
|
||||
```ts
|
||||
let reviewColumns: ReadonlySet<string> = new Set(["in-review"]);
|
||||
const resolved = ir ? resolveReviewColumns(ir) : [];
|
||||
if (resolved.length > 0) reviewColumns = new Set(resolved); // a real answer REPLACES the default
|
||||
```
|
||||
|
||||
**2. Two guards, one assertion.** When two call sites can refuse the same operation, a loose assertion
|
||||
passes with either fixed. Measured: `expect(message).toContain("must be in")` passed with `mergeTaskImpl`
|
||||
reverted, because the completion guard caught the card instead. Assert something that names the site —
|
||||
here the message prefix (`Cannot merge` vs `Cannot move … to done`) — so the sites fail independently.
|
||||
|
||||
## When the conservative verdict needs reporting — and when it does not
|
||||
|
||||
`resolveWorkflowIrForTask` **substitutes** the built-in IR rather than failing, so `resolved.length > 0`
|
||||
reads as *"this board answered"* when nobody did. Both self-healing sweeps in #2838 hit this: a card whose
|
||||
workflow could not be resolved was measured against the built-in lane, rejected, and rejected again on
|
||||
every pass **with nothing recorded**. They now use `...WithProvenance` — not to change the verdict
|
||||
(measured: identical in every state, because the built-in lane already *is* the legacy id) but to make the
|
||||
unrepaired card **reportable**.
|
||||
|
||||
The merge-blocker sites in this PR sit on the same shape and deliberately do **not** get that treatment.
|
||||
Measured the verdict first — also identical — then checked visibility, which is where they differ:
|
||||
|
||||
```ts
|
||||
throw new Error(`Cannot merge ${id}: ${mergeBlocker}`);
|
||||
// → "Cannot merge FN-1: task is in 'checking', must be in 'in-review'"
|
||||
```
|
||||
|
||||
The refusal is already surfaced to the operator, and the message **names the lane it expected**, which is
|
||||
the diagnostic a report would have added. A sweep that skips silently needs the report; a call that throws
|
||||
with the expected lane in the string does not.
|
||||
|
||||
**The rule, so this is not applied by reflex:** provenance buys *visibility*, never a different answer.
|
||||
Add it where the conservative verdict would otherwise be invisible. Adding it where the failure already
|
||||
surfaces is ceremony — and, since it changes no behaviour, ceremony that reads like a fix.
|
||||
|
||||
## Related
|
||||
|
||||
- `docs/solutions/architecture-patterns/hardcoded-movetask-destinations-are-census-invisible.md` — the
|
||||
destination half of a conversion, also invisible to the census.
|
||||
- `docs/solutions/architecture-patterns/self-healing-sweeps-are-blind-on-a-renamed-board.md` — the query
|
||||
half; a guard behind a hardcoded `listTasks({ column })` never runs at all.
|
||||
- `docs/solutions/test-failures/optional-flags-seam-hides-unconverted-column-guards.md` — the same blind
|
||||
spot one level down, in the tests rather than the callers.
|
||||
@@ -0,0 +1,185 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-00:45 (the unwired-parameter class, cf. #2803):
|
||||
|
||||
`getTaskMergeBlocker(task, { reviewColumns })` has taken a RESOLVED lane set since its own conversion.
|
||||
`mergeTaskImpl` — the merge path itself — omitted it, so the identity check fell back to the literal
|
||||
`in-review` and produced:
|
||||
|
||||
Cannot merge FN-X: task is not in 'in-review'
|
||||
|
||||
for a card sitting correctly in ITS OWN board's review lane. A hard, operator-visible merge failure on
|
||||
every renamed board. A resolved seam nobody wired is indistinguishable from no seam at all.
|
||||
|
||||
WHY THIS ASSERTS AN ABSENCE. The blocker check runs BEFORE any git or worktree work, so proving the fix
|
||||
does not require a merge to succeed — it requires that this particular refusal is gone. The card has no
|
||||
worktree here, so the call still fails; the assertion is that it no longer fails for the WRONG reason.
|
||||
Asserting success instead would drag a git fixture into a test about a lane set.
|
||||
|
||||
REVERT CHECK, measured: dropping the `{ reviewColumns }` argument restores the
|
||||
"task is not in 'in-review'" refusal on the renamed board.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { pgDescribe, createTaskStoreForTest, type PgTestHarness } from "../__test-utils__/pg-test-harness.js";
|
||||
import type { TaskStore, WorkflowIr } from "../types.js";
|
||||
|
||||
/** A board whose review lane is not the legacy id, and which declares a merge-class node for it. */
|
||||
const RENAMED_IR = {
|
||||
version: "v2",
|
||||
id: "merge-lifecycle",
|
||||
name: "renamed",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "checking", name: "Review", traits: [{ trait: "merge-blocker" }, { trait: "human-review" }, { trait: "merge" }] },
|
||||
{ id: "shipped", name: "Done", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
/*
|
||||
A real lifecycle spine, not a stub. Column ADJACENCY is derived from the graph
|
||||
(`resolveAllowedColumns`), so an IR with no node in a column cannot be moved into it — my first
|
||||
version's two-node graph made every setup move illegal and the failure looked like the subject.
|
||||
*/
|
||||
nodes: [
|
||||
{ id: "start", kind: "start", column: "backlog" },
|
||||
{ id: "exec", kind: "prompt", column: "building", config: { seam: "execute" } },
|
||||
{ id: "merge-gate", kind: "merge-gate", column: "checking", config: { gate: "auto-merge" } },
|
||||
{ id: "end", kind: "end", column: "shipped" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "exec" },
|
||||
{ from: "exec", to: "merge-gate", condition: "success" },
|
||||
{ from: "merge-gate", to: "end", condition: "success" },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
|
||||
pgDescribe("mergeTask resolves the review lane from the task's own workflow", () => {
|
||||
let harness: PgTestHarness;
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(async () => {
|
||||
harness = await createTaskStoreForTest({ prefix: "fusion_merge_blocker_lane" });
|
||||
store = harness.store;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await harness?.teardown();
|
||||
});
|
||||
|
||||
it("does not refuse a card sitting in a RENAMED review lane", async () => {
|
||||
/*
|
||||
The REAL API. My first version called `saveWorkflowDefinition?.()` and `setTaskWorkflowSelection?.()`
|
||||
— neither exists on TaskStore, and the optional-call `?.` swallowed both silently, so the task kept
|
||||
resolving the BUILTIN workflow and every assertion was about the wrong board. That is what made the
|
||||
first version pass with the fix reverted.
|
||||
*/
|
||||
const created = await store.createWorkflowDefinition({ name: "renamed merge lanes", ir: RENAMED_IR as never });
|
||||
const task = await store.createTask({ description: "renamed review lane" });
|
||||
await store.selectTaskWorkflow(task.id, created.id);
|
||||
/*
|
||||
moveTask, NOT updateTask. My first version used `updateTask({ column })`, which does not move a
|
||||
card — so it sat in `todo` and the whole case was vacuous: the refusal it asserted about never
|
||||
concerned the renamed lane at all. The premise is asserted below rather than assumed, which is the
|
||||
rule `live-move-path-undeclared-target.test.ts` exists to enforce.
|
||||
*/
|
||||
/* The card is created in the builtin intake, so it enters the custom board through `backlog`. */
|
||||
for (const lane of ["backlog", "building", "checking"]) {
|
||||
await store.moveTask(task.id, lane as never, { moveSource: "user" } as never);
|
||||
}
|
||||
expect((await store.getTask(task.id)).column).toBe("checking");
|
||||
|
||||
let message = "";
|
||||
try {
|
||||
await store.mergeTask(task.id);
|
||||
} catch (err) {
|
||||
message = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
// It still fails — there is no worktree — but NOT because of the lane.
|
||||
expect(message).not.toContain("must be in 'in-review'");
|
||||
});
|
||||
|
||||
it("still refuses a card that is in no review lane at all on the RENAMED board", async () => {
|
||||
/*
|
||||
Non-vacuous companion: without it, a merge path that had simply stopped checking lane identity would
|
||||
satisfy the case above. Same board, same workflow — only the card's column changes.
|
||||
*/
|
||||
const created = await store.createWorkflowDefinition({ name: "renamed merge lanes", ir: RENAMED_IR as never });
|
||||
const task = await store.createTask({ description: "not in review" });
|
||||
await store.selectTaskWorkflow(task.id, created.id);
|
||||
for (const lane of ["backlog", "building"]) {
|
||||
await store.moveTask(task.id, lane as never, { moveSource: "user" } as never);
|
||||
}
|
||||
expect((await store.getTask(task.id)).column).toBe("building");
|
||||
|
||||
let message = "";
|
||||
try {
|
||||
await store.mergeTask(task.id);
|
||||
} catch (err) {
|
||||
message = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
expect(message).toContain("must be in");
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:10 (#2820 review — coderabbit, Major):
|
||||
THE REPURPOSED-COLUMN DIRECTION. My first version pre-seeded `in-review` into the resolved set, which
|
||||
admits a board that declares `in-review` as its WIP lane — a card mid-implementation would pass the
|
||||
merge-identity check and merge prematurely.
|
||||
|
||||
This is the converse direction the optional-flags doc names: not "the lane was renamed" but "the lane
|
||||
still CARRIES a lifecycle name while its traits say otherwise", which is what a project gets by
|
||||
repurposing a default column. A rename-only test cannot see it.
|
||||
|
||||
REVERT CHECK, measured: pre-seeding the legacy id back into the set makes this fail — the card merges
|
||||
from a WIP lane.
|
||||
*/
|
||||
it("still refuses a card in a column NAMED in-review that its workflow declares as WIP", async () => {
|
||||
const repurposed = {
|
||||
...RENAMED_IR,
|
||||
columns: [
|
||||
{ id: "backlog", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] },
|
||||
/* The trap: legacy NAME, wip TRAITS. */
|
||||
{ id: "in-review", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "signoff", name: "Review", traits: [{ trait: "merge-blocker" }, { trait: "human-review" }, { trait: "merge" }] },
|
||||
{ id: "shipped", name: "Done", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start", column: "backlog" },
|
||||
{ id: "exec", kind: "prompt", column: "in-review", config: { seam: "execute" } },
|
||||
{ id: "merge-gate", kind: "merge-gate", column: "signoff", config: { gate: "auto-merge" } },
|
||||
{ id: "end", kind: "end", column: "shipped" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "exec" },
|
||||
{ from: "exec", to: "merge-gate", condition: "success" },
|
||||
{ from: "merge-gate", to: "end", condition: "success" },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
|
||||
const created = await store.createWorkflowDefinition({ name: "repurposed in-review", ir: repurposed as never });
|
||||
const task = await store.createTask({ description: "mid-implementation" });
|
||||
await store.selectTaskWorkflow(task.id, created.id);
|
||||
for (const lane of ["backlog", "in-review"]) {
|
||||
await store.moveTask(task.id, lane as never, { moveSource: "user" } as never);
|
||||
}
|
||||
expect((await store.getTask(task.id)).column).toBe("in-review");
|
||||
|
||||
let message = "";
|
||||
try {
|
||||
await store.mergeTask(task.id);
|
||||
} catch (err) {
|
||||
message = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
// The board's review lane is `signoff`; a card in the WIP lane must not merge.
|
||||
/*
|
||||
NAMES THE SITE, deliberately. A looser `toContain("must be in")` passes when EITHER guard refuses —
|
||||
and it did: with merge-queue-ops reverted, the completion guard in task-artifacts-ops caught the card
|
||||
instead and the assertion still held. `Cannot merge` is merge-queue-ops' wording; `Cannot move … to
|
||||
done` is the other. Asserting the prefix is what makes the two sites independently provable.
|
||||
*/
|
||||
expect(message).toContain("Cannot merge");
|
||||
expect(message).toContain("must be in");
|
||||
expect(message).toContain("signoff");
|
||||
});
|
||||
});
|
||||
@@ -12,6 +12,8 @@ import type {Task, MergeResult, MergeQueueEntry, MergeQueueAcquireOptions} from
|
||||
import {assertNotWorkspaceTaskMerge} from "../types.js";
|
||||
import "../builtin-traits.js";
|
||||
import {getTaskMergeBlocker, resolveTaskMergeTarget} from "../task-merge.js";
|
||||
import {resolveWorkflowIrForTask} from "../workflow-ir-resolver.js";
|
||||
import {resolveReviewColumns} from "../workflow-lifecycle-traits.js";
|
||||
import {__setTaskActivityLogLimitsForTesting} from "../task-store/comments.js";
|
||||
import {assertSafeGitBranchName, assertSafeAbsolutePath} from "../task-store/shell-safety.js";
|
||||
import {acquireMergeQueueLease as acquireMergeQueueLeaseAsync} from "../task-store/async-merge-coordination.js";
|
||||
@@ -388,7 +390,36 @@ export async function mergeTaskImpl(store: TaskStore, id: string): Promise<Merge
|
||||
return result;
|
||||
}
|
||||
|
||||
const mergeBlocker = getTaskMergeBlocker(task);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-00:45 (unwired-parameter class, cf. #2803):
|
||||
`getTaskMergeBlocker` has taken an optional RESOLVED `reviewColumns` since its own conversion, and
|
||||
this caller omitted it — so the identity check fell back to the literal `in-review` and threw
|
||||
`Cannot merge <id>: task is not in 'in-review'` for a card sitting correctly in ITS OWN board's
|
||||
review lane. A hard, operator-visible merge failure on every renamed board.
|
||||
|
||||
A resolved seam nobody wired is indistinguishable from no seam at all.
|
||||
|
||||
MEMBERSHIP, not first-per-role: `resolveReviewColumns` unions mergeOrchestration, mergeBlocker and
|
||||
humanReview, so a workflow splitting those across columns has all of them accepted. Unioned with
|
||||
the legacy id because `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than throwing.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:10 (#2820 review — coderabbit, Major):
|
||||
THE LEGACY ID IS A FALLBACK, NOT A MEMBER. My first version pre-seeded `in-review` into the set and
|
||||
unioned the resolved lanes on top. That admits a board which declares `in-review` as its WIP column:
|
||||
a card mid-implementation would pass the merge-identity check and merge prematurely.
|
||||
|
||||
The legacy id is only correct when the board tells us NOTHING — an empty resolved set, or a
|
||||
resolution that threw. A non-empty resolved answer replaces it outright; that is the same
|
||||
"unscoped legacy acceptance" the glasses plugin's own review caught, and I reintroduced it here.
|
||||
*/
|
||||
let reviewColumns: ReadonlySet<string> = new Set<string>(["in-review"]);
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store, id);
|
||||
const resolved = ir ? resolveReviewColumns(ir) : [];
|
||||
if (resolved.length > 0) reviewColumns = new Set(resolved);
|
||||
} catch { /* degraded: the board told us nothing, so the legacy id stands */ }
|
||||
const mergeBlocker = getTaskMergeBlocker(task, { reviewColumns });
|
||||
if (mergeBlocker) {
|
||||
throw new Error(`Cannot merge ${id}: ${mergeBlocker}`);
|
||||
}
|
||||
|
||||
@@ -496,7 +496,38 @@ export async function moveToDoneImpl(store: TaskStore, task: Task, dir: string):
|
||||
}
|
||||
|
||||
const fromColumn = task.column;
|
||||
const mergeBlocker = getTaskMergeBlocker(task);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-01:10 (unwired-parameter class, cf. #2803):
|
||||
THE OUTER QUESTION WAS RESOLVED AND THE INNER ONE WAS NOT — in the same function, four lines apart.
|
||||
`completeColumn` above comes from the task's workflow, then this call re-asked with the literal and
|
||||
refused: on a renamed board the completion move threw
|
||||
|
||||
Cannot move FN-1 to done: task is in 'checking', must be in 'in-review'
|
||||
|
||||
for a card sitting correctly in its own review lane. `getTaskMergeBlocker`'s own note calls out this
|
||||
exact half-conversion shape in `moves.ts`; this is the same shape in a second site it did not cover.
|
||||
|
||||
MEMBERSHIP via `resolveReviewColumns` (mergeOrchestration ∪ mergeBlocker ∪ humanReview), so a board
|
||||
splitting those across columns has all of them accepted, unioned with the legacy id because
|
||||
`resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than throwing.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:10 (#2820 review — coderabbit, Major):
|
||||
THE LEGACY ID IS A FALLBACK, NOT A MEMBER. My first version pre-seeded `in-review` into the set and
|
||||
unioned the resolved lanes on top. That admits a board which declares `in-review` as its WIP column:
|
||||
a card mid-implementation would pass the merge-identity check and merge prematurely.
|
||||
|
||||
The legacy id is only correct when the board tells us NOTHING — an empty resolved set, or a
|
||||
resolution that threw. A non-empty resolved answer replaces it outright; that is the same
|
||||
"unscoped legacy acceptance" the glasses plugin's own review caught, and I reintroduced it here.
|
||||
*/
|
||||
let reviewColumns: ReadonlySet<string> = new Set<string>(["in-review"]);
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store, task.id);
|
||||
const resolved = ir ? resolveReviewColumns(ir) : [];
|
||||
if (resolved.length > 0) reviewColumns = new Set(resolved);
|
||||
} catch { /* degraded: the board told us nothing, so the legacy id stands */ }
|
||||
const mergeBlocker = getTaskMergeBlocker(task, { reviewColumns });
|
||||
if (mergeBlocker) {
|
||||
throw new Error(`Cannot move ${task.id} to done: ${mergeBlocker}`);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,236 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-23:50 (the unwired-parameter class, cf. #2803):
|
||||
|
||||
`isParkedTaskColumn(task, parkedColumns?)` has taken a RESOLVED lane set since its own conversion, and
|
||||
`task-agent-sync-renamed-columns.test.ts` proves the seam works when the set is supplied. But BOTH call
|
||||
sites in `agent-heartbeat.ts` passed nothing and silently took the legacy `todo`/`triage` default, so on
|
||||
a board whose hold and intake lanes are renamed the check returned false for every card.
|
||||
|
||||
A resolved seam nobody wired is indistinguishable from no seam at all — which is why the seam test alone
|
||||
could not catch this, and why the caller audit (#2803) found five more of the same shape.
|
||||
|
||||
CONSEQUENCE. `reconcileOrphanedRunningAgents` clears a durable agent's task link when the card is parked
|
||||
with no live execution proof. With the check inert, the link is kept: the agent goes on claiming a card
|
||||
nobody is working, and Reports Health Check renders it as RUNNING.
|
||||
|
||||
Reached through the private method for the same reason as `executor-worktree-owner-renamed-lanes.test.ts`
|
||||
— the public route is the heartbeat poll loop, and standing that up would make this a test about polling
|
||||
rather than about the lane set.
|
||||
|
||||
REVERT CHECK, measured: dropping the resolved `parkedColumns` argument (back to `isParkedTaskColumn(
|
||||
linkedTask)`) fails the RENAMED case — the stale link is not cleared.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Task, TaskStore, WorkflowIr } from "@fusion/core";
|
||||
import { HeartbeatMonitor } from "../agent-heartbeat.js";
|
||||
import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";
|
||||
|
||||
function parkedCard(vocab: Vocabulary): Task {
|
||||
return {
|
||||
id: "FN-PARKED",
|
||||
title: "parked, nobody working it",
|
||||
description: "",
|
||||
/* The HOLD lane — parked by definition, and renamed on the custom board. */
|
||||
column: vocab.hold,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-07-30T00:00:00.000Z",
|
||||
updatedAt: "2026-07-30T00:00:00.000Z",
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function harness(vocab: Vocabulary) {
|
||||
const ir: WorkflowIr = lifecycleIr(vocab, "heartbeat-parked");
|
||||
const agent = { id: "a1", name: "A", role: "executor", state: "running", taskId: "FN-PARKED" };
|
||||
const store = {
|
||||
listAgents: vi.fn().mockResolvedValue([agent]),
|
||||
getAgent: vi.fn().mockResolvedValue(agent),
|
||||
getCachedAgent: vi.fn().mockReturnValue(null),
|
||||
getActiveHeartbeatRun: vi.fn().mockResolvedValue(null),
|
||||
updateAgent: vi.fn(),
|
||||
updateAgentState: vi.fn(),
|
||||
assignTask: vi.fn(),
|
||||
recordHeartbeat: vi.fn(),
|
||||
getAgentsByReportsTo: vi.fn().mockResolvedValue([]),
|
||||
/*
|
||||
Load-bearing: the clear path calls this, and `reconcileOrphanedRunningAgents` CATCHES its own
|
||||
errors and only warns. Omit it and the sweep silently does nothing — the first version of this
|
||||
test "passed" its negative case that way, which is the incomplete-fake defect this program has
|
||||
documented.
|
||||
*/
|
||||
syncExecutionTaskLink: vi.fn(),
|
||||
endHeartbeatRun: vi.fn(),
|
||||
};
|
||||
const taskStore = {
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
getTask: vi.fn().mockResolvedValue(parkedCard(vocab)),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
updateTask: vi.fn(),
|
||||
moveTask: vi.fn(),
|
||||
logEntry: vi.fn(),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async () => ({ workflowId: "heartbeat-parked", stepIds: [] })),
|
||||
getTaskWorkflowSelection: vi.fn(() => ({ workflowId: "heartbeat-parked", stepIds: [] })),
|
||||
getWorkflowDefinition: vi.fn(async (id: string) => (id === "heartbeat-parked" ? { ir } : undefined)),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
const monitor = new HeartbeatMonitor({ store: store as never, taskStore, rootDir: "/repo" });
|
||||
return { monitor, store, taskStore };
|
||||
}
|
||||
|
||||
/** The private sweep under test; see the header for why this is reached directly. */
|
||||
function reconcile(monitor: HeartbeatMonitor): Promise<void> {
|
||||
return (monitor as unknown as { reconcileOrphanedRunningAgents: () => Promise<void> })
|
||||
.reconcileOrphanedRunningAgents();
|
||||
}
|
||||
|
||||
describe("the parked-link sweep resolves its lanes by ROLE, not by the legacy default", () => {
|
||||
for (const [label, vocab] of [["DEFAULT", DEFAULT_VOCAB], ["RENAMED", RENAMED_VOCAB]] as const) {
|
||||
it(`clears a stale link to a card parked in a ${label} hold lane (${vocab.hold})`, async () => {
|
||||
const { monitor, store } = harness(vocab);
|
||||
|
||||
await reconcile(monitor);
|
||||
|
||||
// The link is dropped: the agent stops claiming a card nobody is working.
|
||||
expect(store.syncExecutionTaskLink).toHaveBeenCalledWith("a1", undefined);
|
||||
});
|
||||
}
|
||||
|
||||
it("leaves the link alone when the card is NOT in a parked lane on a RENAMED board", async () => {
|
||||
/*
|
||||
Non-vacuous companion: without it, a sweep that cleared every link would satisfy both cases above.
|
||||
Same renamed board, same agent — only the card's lane changes, to the one lane where work is live.
|
||||
*/
|
||||
const { monitor, store, taskStore } = harness(RENAMED_VOCAB);
|
||||
(taskStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...parkedCard(RENAMED_VOCAB),
|
||||
column: RENAMED_VOCAB.wip,
|
||||
});
|
||||
|
||||
await reconcile(monitor);
|
||||
|
||||
expect(store.syncExecutionTaskLink).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-00:20 (#2820 review — greptile P2):
|
||||
THE SECOND CALL SITE. The commit converted BOTH `isParkedTaskColumn` callers but the tests drove only
|
||||
`reconcileOrphanedRunningAgents`. `buildReportsHealthSection` resolves its lanes independently and
|
||||
rewrites the rendered report, so it is a separate surface and needed its own case — converting two
|
||||
copies and testing one is the Surface Enumeration failure this program keeps paying for.
|
||||
|
||||
What it does on the parked path: renders the state as `active` rather than `running` and annotates the
|
||||
task as "queued/no live run", which is the operator-visible half of the same defect. With the lanes
|
||||
unresolved on a renamed board the report kept saying RUNNING.
|
||||
|
||||
REVERT CHECK, measured: dropping the resolved argument here fails the RENAMED case — the section still
|
||||
reports `running`.
|
||||
*/
|
||||
function buildHealth(monitor: HeartbeatMonitor, agentStore: unknown): Promise<string | null> {
|
||||
return (monitor as unknown as {
|
||||
buildReportsHealthSection: (agentId: string, agentStore: unknown) => Promise<string | null>;
|
||||
}).buildReportsHealthSection("boss", agentStore);
|
||||
}
|
||||
|
||||
describe("the reports health section resolves its parked lanes by ROLE", () => {
|
||||
for (const [label, vocab] of [["DEFAULT", DEFAULT_VOCAB], ["RENAMED", RENAMED_VOCAB]] as const) {
|
||||
it(`renders a parked report as queued on a ${label} hold lane (${vocab.hold})`, async () => {
|
||||
const { monitor, store } = harness(vocab);
|
||||
/* The direct report is the running agent linked to the parked card. */
|
||||
store.getAgentsByReportsTo.mockResolvedValue([
|
||||
{ id: "a1", name: "A", role: "executor", state: "running", taskId: "FN-PARKED", lastHeartbeatAt: new Date().toISOString() },
|
||||
]);
|
||||
|
||||
const section = await buildHealth(monitor, store);
|
||||
|
||||
expect(section).toContain("queued/no live run");
|
||||
});
|
||||
}
|
||||
|
||||
it("still reports a live report as running on a RENAMED board", async () => {
|
||||
/*
|
||||
Non-vacuous companion: without it, a section that annotated every report would satisfy the cases
|
||||
above. Same renamed board, same agent — only the card's lane changes to the wip one.
|
||||
*/
|
||||
const { monitor, store, taskStore } = harness(RENAMED_VOCAB);
|
||||
(taskStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...parkedCard(RENAMED_VOCAB),
|
||||
column: RENAMED_VOCAB.wip,
|
||||
});
|
||||
store.getAgentsByReportsTo.mockResolvedValue([
|
||||
{ id: "a1", name: "A", role: "executor", state: "running", taskId: "FN-PARKED", lastHeartbeatAt: new Date().toISOString() },
|
||||
]);
|
||||
|
||||
const section = await buildHealth(monitor, store);
|
||||
|
||||
expect(section).not.toContain("queued/no live run");
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:10 (#2820 review — greptile P1):
|
||||
THE ARITY TRAP, fifth occurrence in this program. My first version read the parked lanes through
|
||||
`resolveTaskLifecycleColumns`, which returns the FIRST column carrying each trait. A workflow declaring
|
||||
TWO hold lanes had only one recognised, so a card parked in the SECOND one still read as live and its
|
||||
stale link was never cleared — the very defect the fix exists to close, one degree narrower.
|
||||
|
||||
`resolveLifecycleColumns` answers "which column is THE hold lane?"; this code needs "is this card in ANY
|
||||
parked lane?". Those are different questions and nothing in the types distinguishes them, which is why
|
||||
this keeps recurring.
|
||||
|
||||
The default board cannot express this shape — it has one hold lane — so only a multi-lane fixture can
|
||||
catch it.
|
||||
|
||||
REVERT CHECK, measured: reading the lanes through `resolveTaskLifecycleColumns` again fails this case,
|
||||
because `secondary-hold` is not the first hold column.
|
||||
*/
|
||||
const TWO_HOLD_IR = {
|
||||
version: "v2",
|
||||
id: "heartbeat-parked",
|
||||
name: "two holds",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "secondary-hold", name: "Blocked", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", name: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
],
|
||||
nodes: [{ id: "start", kind: "start", column: "backlog" }],
|
||||
edges: [],
|
||||
} as unknown as WorkflowIr;
|
||||
|
||||
describe("every parked lane counts, not just the first one the resolver returns", () => {
|
||||
it("clears a stale link to a card parked in the SECOND hold lane", async () => {
|
||||
const tasksById = new Map([["FN-PARKED", { ...parkedCard(DEFAULT_VOCAB), column: "secondary-hold" } as Task]]);
|
||||
const agent = { id: "a1", name: "A", role: "executor", state: "running", taskId: "FN-PARKED" };
|
||||
const store = {
|
||||
listAgents: vi.fn().mockResolvedValue([agent]),
|
||||
getAgent: vi.fn().mockResolvedValue(agent),
|
||||
getCachedAgent: vi.fn().mockReturnValue(null),
|
||||
getActiveHeartbeatRun: vi.fn().mockResolvedValue(null),
|
||||
updateAgent: vi.fn(),
|
||||
updateAgentState: vi.fn(),
|
||||
assignTask: vi.fn(),
|
||||
recordHeartbeat: vi.fn(),
|
||||
getAgentsByReportsTo: vi.fn().mockResolvedValue([]),
|
||||
syncExecutionTaskLink: vi.fn(),
|
||||
endHeartbeatRun: vi.fn(),
|
||||
};
|
||||
const taskStore = {
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
getTask: vi.fn(async (id: string) => tasksById.get(id)),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
updateTask: vi.fn(),
|
||||
moveTask: vi.fn(),
|
||||
logEntry: vi.fn(),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async () => ({ workflowId: "heartbeat-parked", stepIds: [] })),
|
||||
getTaskWorkflowSelection: vi.fn(() => ({ workflowId: "heartbeat-parked", stepIds: [] })),
|
||||
getWorkflowDefinition: vi.fn(async (id: string) => (id === "heartbeat-parked" ? { ir: TWO_HOLD_IR } : undefined)),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
const monitor = new HeartbeatMonitor({ store: store as never, taskStore, rootDir: "/repo" });
|
||||
await reconcile(monitor);
|
||||
|
||||
expect(store.syncExecutionTaskLink).toHaveBeenCalledWith("a1", undefined);
|
||||
});
|
||||
});
|
||||
@@ -193,3 +193,61 @@ describe("task-agent-sync under a renamed column vocabulary", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:40 (the arity trap, sixth site in this program):
|
||||
`resolveLinkSyncColumnRoles` built its `parked` and `terminal` sets from `resolveTaskLifecycleColumns`,
|
||||
which returns the FIRST column carrying each trait — then handed them to `.includes()` membership tests.
|
||||
A workflow declaring TWO hold lanes had link hygiene applied to only one of them: a card moved into the
|
||||
second stayed linked to its agent, which is the state this whole module exists to clean up.
|
||||
|
||||
The DEFAULT board cannot express this — it declares one column per role — so no rename differential
|
||||
catches it. It needs a structurally different board, not a differently-named one.
|
||||
|
||||
REVERT CHECK, measured: rebuilding the sets from `resolveTaskLifecycleColumns` fails this case, because
|
||||
`overflow-hold` is not the first hold column.
|
||||
*/
|
||||
describe("every lane carrying a role counts, not just the first", () => {
|
||||
function twoHoldIr(): WorkflowIr {
|
||||
return {
|
||||
version: "v2",
|
||||
id: WF,
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: "inbox", label: "Intake", traits: [{ trait: "intake" }] },
|
||||
{ id: "backlog", label: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "overflow-hold", label: "Second Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", label: "Wip", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "shipped", label: "Complete", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
}
|
||||
|
||||
it("treats a SECOND hold lane as parked when clearing the agent link", async () => {
|
||||
const selection = { workflowId: WF, stepIds: [] };
|
||||
const syncCalls: Array<string | undefined> = [];
|
||||
let handler: ((e: { task: { id: string }; from: string; to: string }) => Promise<void>) | undefined;
|
||||
|
||||
const store = {
|
||||
on: vi.fn((_evt: string, h: typeof handler) => { handler = h; }),
|
||||
off: vi.fn(),
|
||||
getTaskWorkflowSelection: vi.fn(() => selection),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
|
||||
getWorkflowDefinition: vi.fn(async () => ({ ir: twoHoldIr() })),
|
||||
getTask: vi.fn(async () => ({ id: "FN-1", column: "overflow-hold" }) as Task),
|
||||
} as unknown as TaskStore;
|
||||
|
||||
const agentStore = {
|
||||
listAgents: vi.fn(async () => [{ id: "A1", taskId: "FN-1", state: "running" } as Agent]),
|
||||
getActiveHeartbeatRun: vi.fn(async () => null),
|
||||
updateAgentState: vi.fn(async () => {}),
|
||||
syncExecutionTaskLink: vi.fn(async (_id: string, taskId: string | undefined) => { syncCalls.push(taskId); }),
|
||||
} as unknown as AgentStore;
|
||||
|
||||
attachAgentLinkSync({ store, agentStore, logger: { log: () => {}, warn: () => {} } });
|
||||
await handler?.({ task: { id: "FN-1" }, from: "building", to: "overflow-hold" });
|
||||
|
||||
expect(syncCalls).toContain(undefined);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -117,3 +117,63 @@ describe("selectActionablePlanningContinuations", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-01:40 (closing the partially-threaded half of the gap that
|
||||
workflow-planning-continuation-terminal-gap-live-e2e.pg.test.ts documented):
|
||||
|
||||
`resolvePlanningContinuationCandidate` applied the caller's resolved terminal set to its OWN check and
|
||||
then delegated to `isPlanningContinuationTaskDispatchable(task)` WITHOUT it, so the inner predicate
|
||||
re-tested against the legacy `done`/`archived` pair.
|
||||
|
||||
THE REACHABLE CASE is a board that declares `done` as a NON-terminal column id — legal, and the shape a
|
||||
project gets by repurposing a default column rather than renaming one. The outer check passes (the
|
||||
resolved set says not terminal), the inner one calls it terminal per the legacy pair, and the card is
|
||||
skipped as "paused": stalled by a lane name.
|
||||
|
||||
REVERT CHECK, measured: dropping the threaded set makes this fail — the candidate resolves `skip`
|
||||
instead of `actionable`.
|
||||
*/
|
||||
describe("the inner dispatchable predicate uses the caller's resolved terminal set", () => {
|
||||
it("does not treat a NON-terminal `done` column as terminal", () => {
|
||||
const item = { taskId: "FN-1", waitReason: "planning" } as never;
|
||||
const task = {
|
||||
id: "FN-1",
|
||||
column: "done",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-07-30T00:00:00.000Z",
|
||||
updatedAt: "2026-07-30T00:00:00.000Z",
|
||||
} as never;
|
||||
|
||||
/* This board declares `done` as an ordinary lane; its terminal lane is `shipped`. */
|
||||
const resolved = resolvePlanningContinuationCandidate(item, task, {
|
||||
terminalColumns: new Set(["shipped", "boxed"]),
|
||||
});
|
||||
|
||||
expect(resolved.kind).toBe("actionable");
|
||||
});
|
||||
|
||||
it("still treats the board's OWN terminal lane as terminal", () => {
|
||||
/* Non-vacuous companion: without it, a predicate that never classified anything terminal passes. */
|
||||
const item = { taskId: "FN-2", waitReason: "planning" } as never;
|
||||
const task = {
|
||||
id: "FN-2",
|
||||
column: "shipped",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-07-30T00:00:00.000Z",
|
||||
updatedAt: "2026-07-30T00:00:00.000Z",
|
||||
} as never;
|
||||
|
||||
const resolved = resolvePlanningContinuationCandidate(item, task, {
|
||||
terminalColumns: new Set(["shipped", "boxed"]),
|
||||
});
|
||||
|
||||
expect(resolved.kind).toBe("orphan");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
FNXC:WorkflowScheduling 2026-07-31-07:10 (E2E evidence — the optional-role-parameter class, third instance):
|
||||
FNXC:WorkflowScheduling 2026-07-30-07:10 (E2E evidence — the optional-role-parameter class, third instance):
|
||||
|
||||
Third measured instance of the pattern from #2795 and #2798, and the sharpest form of it: the
|
||||
converted and unconverted call sites are in the SAME FILE, and one is nested inside the other's call
|
||||
@@ -35,6 +35,26 @@ runtime module's SYNTAX (parsed, not string-matched — see the note on that cas
|
||||
drain needs the runtime's full dependency set, which I did not build; the audit case says so rather
|
||||
than dressing it up.
|
||||
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-01:40 (follow-up — one severity correction and one site closed):
|
||||
|
||||
TWO REFINEMENTS to the above, both measured rather than argued.
|
||||
|
||||
1. `selectActionablePlanningContinuations` has NO PRODUCTION CALLER. Grepped across the repo excluding
|
||||
node_modules and dist: the only references are this file and
|
||||
`workflow-continuation-selection.test.ts`. The live drain is `drainDuePlanningContinuations:386`,
|
||||
which IS converted. So the stated consequence — a completed card re-entering plan-review "silently,
|
||||
on every custom board" — is NOT reachable today. The finding is real but LATENT: the helper is
|
||||
exported, so the first production caller inherits the bug. Worth keeping, worth not over-stating.
|
||||
|
||||
2. The THIRD site is closed. `resolvePlanningContinuationCandidate` now threads its own resolved
|
||||
`terminal` into `isPlanningContinuationTaskDispatchable`. Its reachable case is a board that declares
|
||||
`done` as a NON-terminal column id: the outer check passes, the inner one calls it terminal per the
|
||||
legacy pair, and the card is skipped as "paused" — stalled by a lane name. Pinned by two cases in
|
||||
`workflow-continuation-selection.test.ts`, revert-measured.
|
||||
|
||||
The first site is deliberately left: wiring a parameter into a function nothing calls would be an
|
||||
unwired parameter, which is the anti-pattern the caller audit (#2803) removed five of.
|
||||
|
||||
LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is
|
||||
unaffected. Throwaway per-file database; never port 4040.
|
||||
*/
|
||||
@@ -157,7 +177,7 @@ pgDescribe("planning-continuation terminal columns, measured on a live store", (
|
||||
NOT driven: reaching the drain needs the runtime's full dependency set. Asserted against the
|
||||
module's SYNTAX and labelled as such.
|
||||
|
||||
FNXC:WorkflowScheduling 2026-07-31-10:15 (PR #2799 review — greptile P2):
|
||||
FNXC:WorkflowScheduling 2026-07-30-10:15 (PR #2799 review — greptile P2):
|
||||
AST, not string splitting. The first version found call sites by splitting on the callee name and
|
||||
then reasoning about the text that followed — whitespace, the next `;`, whether the slice started
|
||||
with a parameter name. A formatting-only change to the runtime could fail this suite or, worse,
|
||||
|
||||
@@ -1232,7 +1232,33 @@ export class HeartbeatMonitor {
|
||||
FNXC:AgentTaskStateDrift 2026-06-23-09:02:
|
||||
Reports Health Check must not render a durable direct report as running a parked todo/triage task unless a fresh heartbeat run or tracked executor signal proves live execution. Clearing Agent.taskId here preserves overlapBlockedBy on the task row; the file-scope lease remains the scheduler's source of truth.
|
||||
*/
|
||||
if (isParkedTaskColumn(linkedTask) && !parkedProof.shouldPreserveParkedLink) {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-23:50 (unwired-parameter class, cf. #2803):
|
||||
`isParkedTaskColumn` has taken a resolved `parkedColumns` since its own conversion, but BOTH
|
||||
call sites here passed nothing and silently took the legacy `todo`/`triage` default. On a board
|
||||
whose hold and intake lanes are renamed the check returned false for every card, so this clear
|
||||
never fired: a durable agent kept its task link to a parked card with no live execution proof,
|
||||
and Reports Health Check went on rendering it as RUNNING.
|
||||
|
||||
A resolved seam nobody wired is indistinguishable from no seam at all — which is exactly what
|
||||
the caller audit found five of.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:10 (#2820 review — greptile P1):
|
||||
MEMBERSHIP, not first-per-role. `resolveTaskLifecycleColumns` returns the FIRST column carrying
|
||||
each trait, so a workflow declaring TWO hold lanes (or a hold plus a second intake) had only one
|
||||
of them recognised as parked — a card in the secondary lane still read as live, and the stale
|
||||
link was never cleared for it. Same defect this fix exists to close, one degree narrower.
|
||||
|
||||
`columnsWithFlag` returns EVERY column carrying the trait, so both halves are unions. This is the
|
||||
fifth time this program has hit first-per-role where it wanted membership; the two are not
|
||||
interchangeable and the compiler cannot tell them apart.
|
||||
*/
|
||||
const parkedIr = await resolveWorkflowIrForTask(this.taskStore!, linkedTask.id).catch(() => undefined);
|
||||
const parkedColumns = parkedIr
|
||||
? [...new Set([...columnsWithFlag(parkedIr, "hold"), ...columnsWithFlag(parkedIr, "intake")])]
|
||||
: [];
|
||||
if (isParkedTaskColumn(linkedTask, parkedColumns.length > 0 ? parkedColumns : undefined) && !parkedProof.shouldPreserveParkedLink) {
|
||||
reason = `parked ${linkedTask.column} task ${agent.taskId} without live execution proof`;
|
||||
clearTaskLink = true;
|
||||
taskIdToClear = agent.taskId;
|
||||
@@ -3100,7 +3126,7 @@ export class HeartbeatMonitor {
|
||||
try {
|
||||
const assignedOpen = await this.taskStore.getTasksByAssignedAgent(agentId, { excludeArchived: true });
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-13:40:
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-13:40:
|
||||
Pass the resolved lane flags so the ranking's terminal filter is not the literal pair.
|
||||
|
||||
`rankAssignedTasksForWakeDelta` gained `flagsByColumnId` and this, its only production
|
||||
@@ -3712,7 +3738,15 @@ export class HeartbeatMonitor {
|
||||
if (report.state === "running" && !isEphemeralAgent(report) && report.taskId && this.taskStore) {
|
||||
try {
|
||||
const linkedTask = await this.taskStore.getTask(report.taskId);
|
||||
if (isParkedTaskColumn(linkedTask)) {
|
||||
/* FNXC:WorkflowResolvedColumns 2026-07-30-23:50: same unwired parameter as above — the health
|
||||
report rendered a parked card as running on any board with renamed hold/intake lanes. */
|
||||
/* FNXC:WorkflowResolvedColumns 2026-07-30-15:10 (#2820 review — greptile P1): membership, not
|
||||
first-per-role — see the note on the sweep above. */
|
||||
const reportParkedIr = await resolveWorkflowIrForTask(this.taskStore, report.taskId).catch(() => undefined);
|
||||
const reportParkedColumns = reportParkedIr
|
||||
? [...new Set([...columnsWithFlag(reportParkedIr, "hold"), ...columnsWithFlag(reportParkedIr, "intake")])]
|
||||
: [];
|
||||
if (isParkedTaskColumn(linkedTask, reportParkedColumns.length > 0 ? reportParkedColumns : undefined)) {
|
||||
const activeRun = await agentStore.getActiveHeartbeatRun(report.id);
|
||||
const proof = evaluateParkedAgentTaskLink({
|
||||
agent: report,
|
||||
|
||||
@@ -2401,7 +2401,16 @@ export class TaskExecutor {
|
||||
return "missing";
|
||||
}
|
||||
|
||||
const blocker = getTaskMergeBlocker(latestTask);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:40 (outer question resolved, inner one not):
|
||||
The guard directly above compares against `(await this.resolveResumeLanes(taskId)).review`, then this
|
||||
call re-asked with the literal — so a card that just PASSED the resolved lane check was refused by the
|
||||
unresolved blocker on any renamed board.
|
||||
*/
|
||||
const resumeReviewLane = (await this.resolveResumeLanes(taskId)).review;
|
||||
const blocker = getTaskMergeBlocker(latestTask, {
|
||||
reviewColumns: new Set([resumeReviewLane ?? "in-review"]),
|
||||
});
|
||||
if (blocker) {
|
||||
await this.store.logEntry(taskId, "Task already in-review; merge deferred", blocker, this.getRunContextFor(taskId));
|
||||
return "blocked";
|
||||
@@ -12548,10 +12557,23 @@ export class TaskExecutor {
|
||||
throwing when a definition is missing or corrupt, so a degraded resolution must not NARROW this
|
||||
set — narrowing it re-opens the interruption this fixes.
|
||||
*/
|
||||
const activeLifecycle = resolveLifecycleColumns(await resolveWorkflowIrForTask(this.store, task.id));
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-16:10 (the arity trap, seventh site):
|
||||
MEMBERSHIP, not first-per-role. `activeColumns` is a `.has()` test, but was filled from
|
||||
`resolveLifecycleColumns`, which returns the FIRST column carrying each trait — so a workflow with two
|
||||
wip lanes, or a review lane plus a second merge-blocking one, had only one of each recognised as
|
||||
active. A card in the second read as INACTIVE and its prompt file was treated as reclaimable.
|
||||
|
||||
The IR is already in hand one line up; `columnsWithFlag` returns every column carrying the trait.
|
||||
The legacy trio stays unioned in — this predicate is about liveness, and under-reporting active is
|
||||
the destructive direction.
|
||||
*/
|
||||
const activeIr = await resolveWorkflowIrForTask(this.store, task.id);
|
||||
const activeColumns = new Set<string>(["in-progress", "in-review", "done"]);
|
||||
for (const lane of [activeLifecycle?.wip, activeLifecycle?.review, activeLifecycle?.complete]) {
|
||||
if (lane !== undefined) activeColumns.add(lane);
|
||||
if (activeIr) {
|
||||
for (const flag of ["countsTowardWip", "mergeOrchestration", "mergeBlocker", "humanReview", "complete"] as const) {
|
||||
for (const lane of columnsWithFlag(activeIr, flag)) activeColumns.add(lane);
|
||||
}
|
||||
}
|
||||
const activeMergeStatuses = new Set(["merging", "merging-pr", "merging-fix"]);
|
||||
const isActiveTask = activeColumns.has(task.column) || activeMergeStatuses.has(task.status ?? "");
|
||||
|
||||
@@ -196,7 +196,21 @@ export function resolvePlanningContinuationCandidate(
|
||||
if (task.paused === true || task.userPaused === true) {
|
||||
return { kind: "skip", item, reason: "paused" };
|
||||
}
|
||||
if (!isPlanningContinuationTaskDispatchable(task)) {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-01:40 (the partially-threaded conversion named by
|
||||
workflow-planning-continuation-terminal-gap-live-e2e.pg.test.ts):
|
||||
THREAD THE SET THIS FUNCTION ALREADY RESOLVED. The terminal test at the top of this function uses the
|
||||
caller's `terminal`; this delegation then re-tested against `LEGACY_TERMINAL_PAIR`, so the conversion
|
||||
was whole at the call site and not whole inside it.
|
||||
|
||||
The reachable case is narrow but real: a board that DECLARES `done` as a non-terminal column id. The
|
||||
outer check passes (not terminal per the resolved set), then the inner predicate calls it terminal per
|
||||
the legacy pair and the continuation is skipped as "paused" — a card stalled by a lane name.
|
||||
|
||||
A partially threaded conversion is indistinguishable from a complete one at every call site that looks
|
||||
converted, which is why this is worth closing even though the outer check dominates the common case.
|
||||
*/
|
||||
if (!isPlanningContinuationTaskDispatchable(task, terminal)) {
|
||||
return { kind: "skip", item, reason: "paused" };
|
||||
}
|
||||
return { kind: "actionable", item, task };
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { resolveTaskLifecycleColumns } from "@fusion/core";
|
||||
import { resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core";
|
||||
import type { Agent, AgentHeartbeatRun, AgentStore, Task, TaskStore, WorkflowIr } from "@fusion/core";
|
||||
|
||||
export const PARKED_AGENT_LINK_FRESH_RUN_MS = 5 * 60_000;
|
||||
@@ -56,11 +56,22 @@ async function resolveLinkSyncColumnRoles(
|
||||
taskId: string,
|
||||
cache?: Map<string, WorkflowIr>,
|
||||
): Promise<LinkSyncColumnRoles> {
|
||||
const lifecycle = await resolveTaskLifecycleColumns(store, taskId, cache);
|
||||
if (!lifecycle) return LEGACY_COLUMN_ROLES;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:40 (the arity trap, sixth site):
|
||||
MEMBERSHIP, not first-per-role. These two sets are consumed by `roles.parked.includes(to)` and
|
||||
`roles.clear.includes(to)` — membership tests — but were built from `resolveTaskLifecycleColumns`,
|
||||
which returns the FIRST column carrying each trait. A workflow declaring two hold lanes, or an
|
||||
archive lane plus a second terminal one, had link hygiene applied to only one of them.
|
||||
|
||||
const parked = [lifecycle.hold, lifecycle.intake].filter((c): c is string => typeof c === "string");
|
||||
const terminal = [lifecycle.complete, lifecycle.archived].filter((c): c is string => typeof c === "string");
|
||||
`resolveLifecycleColumns` answers "which column is THE hold lane?"; a membership test asks "is this
|
||||
column ANY hold lane". Nothing in the types distinguishes them, which is why this program has now hit
|
||||
it six times. `columnsWithFlag` returns every column carrying the trait.
|
||||
*/
|
||||
const ir = await resolveWorkflowIrForTask(store, taskId, cache).catch(() => undefined);
|
||||
if (!ir) return LEGACY_COLUMN_ROLES;
|
||||
|
||||
const parked = [...new Set([...columnsWithFlag(ir, "hold"), ...columnsWithFlag(ir, "intake")])];
|
||||
const terminal = [...new Set([...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")])];
|
||||
const clear = [...terminal, ...parked];
|
||||
|
||||
// A v2 workflow declaring none of the four roles yields an empty clear set,
|
||||
|
||||
Reference in New Issue
Block a user