Files
fusion/packages/engine/src/task-completion.ts
gsxdsm 8c9b84ae38 batch-core: packages/core + dashboard/src lifecycle conversion (129 → 92) (#2780)
## batch-core — `packages/core` + `packages/dashboard/src`

Shared branch: two workers are converting into it. Opening the PR
because the branch was green with none, and a branch without a PR merges
nothing.

### Census

Measured with `node scripts/lifecycle-column-census.mjs --json`.

| | guards |
|---|---|
| batch-core scope at branch point | 129 |
| batch-core scope now | **92** (51 files) |
| repo total now | 358 |

Files closed so far: `store.ts` 11→0, `task-merge.ts` 6→0,
`live-agent-count.ts` 6→0 (marked, not converted — see #2762),
`task-update.ts` 3→0, display-ordering + Wake Delta ranking 5→0,
`register-git-github.ts` 4→0.

### The `register-git-github.ts` slice

Three PR routes — `pr/create`, `pr/push-branch`, `pr/resolve-conflicts`
— plus the `CHANGES_REQUESTED` handler each compared `task.column !==
"in-review"`. On a renamed board **none** of them matched, so every PR
affordance the dashboard offers was refused for a card sitting in the
lane that board calls review, and the refusal named a column that does
not exist there.

All four now share one helper, `reviewColumnsForTask`, which gets two
things right that this program has repeatedly gotten wrong:

- **Membership, not a single id.** It takes the broad review set
(`mergeOrchestration ∪ mergeBlocker ∪ humanReview`).
`resolveLifecycleColumns` returns the *first* column per trait, so a
single-id answer silently ignores a board that declares a merge lane
**and** a separate human sign-off lane. These guards only refuse or
permit — they never move the card — so over-admitting costs nothing
while under-admitting refuses a request that should have worked.
- **An empty resolved set means UNEXPRESSED, not absent.**
`synthesizeDefaultColumns` upgrades a v1 graph by emitting every default
column with `traits: []`, so a v1-upgraded workflow resolves to an empty
review set while its `in-review` column plainly exists and holds the
card. Reading empty as "this board has no review lane" would refuse
these routes on **every pre-v2 project** — a worse regression than the
one being fixed, and invisible to any v2 test.

This is the dashboard twin of the `fn pr create` guard in
`packages/cli/src/commands/pr.ts` (#2775). The two surfaces answer the
same question and now agree — FN-5893 surface enumeration.

### Testing note: why the seam and not the routes

I wrote route-level HTTP tests first and **deleted them**. An express
fixture over `registerGitGitHubRoutes` hangs — every case, including the
pure refusals, times out at 4s, because registering the router starts
background work the fixture never satisfies. Making it run would mean
mocking git, the GitHub client, and the pollers: a mock-the-world shell,
which is what the project's do-not-add-slow-tests rule (FN-5048) says to
avoid in favour of a narrow seam.

`reviewColumnsForTask` *is* the narrow seam — it holds the entire
decision, and the four call sites now do nothing but ask it and render
its answer. Six cases pin it: the renamed lane is returned and
`in-review` is not, a two-lane board returns both, a v1-upgraded board
falls back, an unresolvable workflow falls back, and the refusal renders
lanes an operator can act on.

**Mutation-verified, both directions:** reverting the helper to the
legacy literal fails 2 of 6; treating an empty set as an answer fails 1
of 6.

One fixture bug worth recording, since it would have made the two-lane
case vacuous: the trait id is kebab-case `human-review`, not
`humanReview`, and the built-in traits must be registered via `import
"@fusion/core"` before flags resolve.

### Verification

- `pnpm --filter @fusion/dashboard exec tsc --noEmit -p tsconfig.json` →
0 errors
- `pnpm lint` → 0 errors
- `register-git-github.review-lanes.test.ts` → 6 passed

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 09:28:27 -07:00

81 lines
3.9 KiB
TypeScript

import { getTaskCompletionBlocker, type Task, type TaskStore } from "@fusion/core";
import { resolveDependencySatisfactionColumns } from "./scheduler.js";
/*
FNXC:WorkflowResolvedColumns 2026-07-31-01:45 (#2780 review — greptile, "dependency lifecycle context stays unset"):
THE INJECTION POINT EXISTED AND NOTHING IN PRODUCTION FILLED IT.
`getTaskCompletionBlocker` gained an optional `satisfactionColumnsByTaskId` so dependency
satisfaction could be judged from each dependency's OWN workflow. This wrapper is the production
path, and it omitted the option — so every real call fell through to the documented legacy default
(`done`/`in-review`/`archived`). On a renamed board a finished dependency read as unresolved and the
depending task could never complete. The conversion was inert everywhere it actually ran.
An optional parameter that only tests supply is the half-converted-pair shape in its quietest form:
the guard reads as converted, its covering test passes because it injects the value by hand, and
production keeps the literal. Filling it here is what makes the option real.
WHY THE DEPENDENCIES ARE FETCHED FIRST. `resolveDependencySatisfactionColumns` resolves per
dependency, because a dependency can run a different workflow from the task depending on it — that
difference is the whole reason the map is keyed by task id. Resolution shares one IR cache, so this
costs one workflow read per distinct workflow, not per dependency.
Failure is non-fatal by design: if the dependencies cannot be read the map is simply absent and the
gate falls back to the legacy default, rather than blocking completion on a resolution problem. That
matches the conservative contract `getTaskCompletionBlocker` documents for itself.
*/
export async function getTaskCompletionBlockerForStore(
store: Pick<TaskStore, "getTask">,
task: Task,
): Promise<string | undefined> {
const resolveTask = async (dependencyId: string) => {
try {
return await store.getTask(dependencyId);
} catch {
return null;
}
};
/*
Both edges count: `dependencies` and the single `blockedBy` marker are judged by the same helper in
`task-merge.ts`, so both must appear in the map or the blockedBy branch silently keeps the literal.
*/
const dependencyIds = [...(task.dependencies ?? [])];
const blockedBy = task.blockedBy?.trim();
if (blockedBy && !dependencyIds.includes(blockedBy)) dependencyIds.push(blockedBy);
/*
The store parameter is deliberately narrow (`Pick<TaskStore, "getTask">`) — several callers pass a
partial store that genuinely cannot resolve workflows. Feature-detect rather than widen the
signature: a caller without the selection readers keeps the legacy default, which is the same
behaviour it had before, instead of every such call site being forced to change.
*/
const canResolveWorkflows =
typeof (store as { getWorkflowDefinition?: unknown }).getWorkflowDefinition === "function";
let satisfactionColumnsByTaskId: Awaited<ReturnType<typeof resolveDependencySatisfactionColumns>> | undefined;
if (dependencyIds.length > 0 && canResolveWorkflows) {
try {
const dependencies = (await Promise.all(dependencyIds.map(resolveTask))).filter(
(dep): dep is NonNullable<typeof dep> => dep != null,
) as unknown as Task[];
if (dependencies.length > 0) {
satisfactionColumnsByTaskId = await resolveDependencySatisfactionColumns(
store as unknown as Parameters<typeof resolveDependencySatisfactionColumns>[0],
dependencies,
);
}
} catch {
/* leave unset — the gate falls back to the documented legacy default */
}
}
return getTaskCompletionBlocker(task, {
// FN-4091: return full task state from the store so completion gating can
// ignore stale blockedBy markers when the blocker is missing or terminal.
resolveTask,
...(satisfactionColumnsByTaskId ? { satisfactionColumnsByTaskId } : {}),
});
}