fix(core): bind the in-transaction capacity gate — one shared pool-id convention (NOT user-visible yet — see R2) (#2488)

## The bug

`moves.ts` asked `countActiveInCapacitySlotAsync` for occupants of pool
`"builtin:coding"`, while the counter buckets selection-less rows under
`DEFAULT_WORKFLOW_POOL_ID` (`"__default-workflow__"`). Nothing ever
landed in the pool being asked about, so the count came back **0** and a
finite limit could never bind.

## Root fix, not a literal swap

A shared *constant* would not have prevented this:
**`DEFAULT_WORKFLOW_ID` was already imported in `moves.ts` and the code
still wrote a literal.** So both sides now call a shared **function**,
`resolveCapacityPoolId` — "which pool does a selection-less task belong
to" has exactly one answer and no call site is in a position to disagree
with it.

The one variable serving two masters is split: a capacity **pool key**
(a bucketing sentinel that must not collide with a workflow id) and a
**workflow id** (telemetry, must stay a real id). The emitted
`TaskTransitioned` payload is byte-identical.

## Checked, not assumed: no second copy

`scheduler.ts:2514` and `:2536` do carry `?? "builtin:coding"` — but as
an **IR resolution key** (`resolveWorkflowIrById`), where a real
workflow id is required and the pool sentinel would not resolve at all.
Same literal, different concept, correctly used. A blanket replace would
have broken it.

## Something did depend on the gate being dead — exactly one thing

`move-path-equivalence.pg.test.ts` → *"UNPROVEN: in-transaction column
capacity did NOT reject on EITHER path in this fixture"*. It left the
cause open —

> something further in (`resolveColumnCapacity`'s limit resolution, or
what `countActiveInCapacitySlotAsync` counts as an occupant — a task
with no session/agent may not count) keeps the check from firing … This
suite does not establish which.

— and predicted its own obsolescence (*"if a future change makes this
reject, that is the capacity gate coming alive"*). **Neither guess was
right; it was the pool id.** Updated to assert the divergence with the
answer recorded — **not weakened**. Its fixture also had to start each
phase from an empty wip column: once the gate binds, the inline phase's
leftovers trip the cap on the *holder* move before the contended move
under test runs.

`schema-applier.test.ts` failed only in the full-suite run and passes in
isolation both with and without the fix — cross-file contamination, not
mine.

## Before / after — measured, both directions

`maxConcurrent: 1`, real PG store, real `moveTask`:

| | flagOFF / no selection | flagOFF / selection | flagON / no selection
| flagON / selection |
|---|---|---|---|---|
| **before** | ADMITTED | ADMITTED | **ADMITTED** ← the bug | REJECTED |
| **after** | ADMITTED | ADMITTED | **REJECTED** | REJECTED |

The E2E acceptance row asserts **held at cap 1 and admitted at cap 2 on
the same fixture**, so it cannot pass by simply never admitting
anything. **With the fix reverted that row fails**; the `admitted` case
still passes, as it should. The Phase A3 ratchet's two flipped
assertions also fail with the fix reverted.

Ratchet flipped exactly as its author specified: `DEFECT (R1)` becomes a
rejection, and `it.fails` on the invariant becomes a plain `it`.

## ⚠️ This is NOT user-visible yet — please read before merging

The premise this was approved on ("once it binds, cards that currently
slip through will start being held") **does not hold for this change
alone.** The whole capacity block sits inside `if (useWorkflow &&
workflowIr && fromColumn !== toColumn)`, and `useWorkflow` is
`experimentalFeatures.workflowColumns === true` — absent from
`DEFAULT_GLOBAL_SETTINGS`, with **no writer anywhere outside tests**.
That is Phase A3's R2, still live and now retitled `DEFECT (R2, STILL
LIVE)` with the measured matrix recorded in it.

So on merge: nothing changes for any real project. Making it actually
bind means **also** removing the `useWorkflow` condition — a materially
larger, genuinely user-visible change that I have not made unilaterally.
Escalated for a decision; if that lands, the changeset here should be
re-categorised.


## Review follow-up (48e79ffd9): the convention was still duplicated —
swept and ratcheted

The first pass added the resolver and routed the transactional gate +
counters, but **hold-release still derived the pool independently**.
Swept the repo: six sites name the sentinel, **five derive the
convention** and now call `resolveCapacityPoolId`
(`hold-release.ts:116/118/442/576`, `task-store-helpers.ts:290`). The
sixth, `scheduler.ts:1558`, names the default pool as a literal in a
capacity *diagnostic* — no selection input, nothing to disagree with —
so it keeps the constant.

**Does this change hold-release behavior? No, and it was never releasing
against the wrong pool.** hold-release computed `x ??
DEFAULT_WORKFLOW_POOL_ID`, which is exactly what the counter buckets
under; `moves.ts` (`?? "builtin:coding"`) was the sole disagreeing site,
and the first commit moved *it* into agreement with hold-release, not
the reverse. `resolveCapacityPoolId(x)` **is** `x ??
DEFAULT_WORKFLOW_POOL_ID`, so every routed site computes an identical
value for every input. **No second user-visible change rides along with
this PR** — the only behavior delta remains the gate binding on the
flag-ON path, which per R2 is still not the path production takes.
Evidence: hold-release + capacity suites **43/43 identical before and
after**.

**The resolver is now the only way to compute a pool id, not merely the
newest way.** `scripts/check-capacity-pool-id.mjs` fails on any inline
`?? DEFAULT_WORKFLOW_POOL_ID` outside `workflow-capacity.ts`, wired into
**both `pretest` and the blocking `test:gate`**. A review note would not
have sufficed: the original defect landed in a file that *already
imported* the canonical constant. Verified both ways — clean run scans
1124 files and passes; reintroducing the old hold-release expression
exits 1 and names the line.


## Review follow-up (a5b675503): the ratchet was rebuilt because it
would not have caught the bug

The first ratchet matched one spelling (`?? DEFAULT_WORKFLOW_POOL_ID`)
and the real defect used another (`?? "builtin:coding"`). **Verified:
reintroducing the original defect and running the old checker exits 0.**
A guard that reports success without checking is worse than no guard —
it stops anyone looking.

Rebuilt on the TypeScript AST with two rules. **Rule 1 (sink):** a value
reaching a capacity counter's `workflowId` must come from
`resolveCapacityPoolId`, or a local initialized from it — so it fires on
the original defect regardless of which literal was used, on one line or
twenty. **Rule 2 (sentinel):** no `??` onto the sentinel at any
qualification depth or as its raw value; multiline is one AST node and
caught by construction. `?? "builtin:coding"` is deliberately *not*
banned outright — it is the legitimate default for a *workflow* id in ~8
places, and is only a bug when it reaches a capacity pool.

**Fails closed three ways** that previously reported success without
inspecting: unreadable file, unparseable file, and an empty file listing
(the old script would have printed a green tick off a broken glob).

**Acceptance was not "passes on main".** Each form was reintroduced into
the real source and confirmed to fail: the original defect in
`moves.ts`, a multiline fallback, and a deeply qualified sentinel. All
are pinned in `capacity-pool-id-check.test.ts` (12 cases: 7 must-catch
starting with the reduced actual pre-fix `moves.ts`, 4 must-not-flag, 1
fail-closed) so the guard cannot silently narrow again.

Also added to `pretest:full`, which had omitted it.


### Follow-up (0be8df6ea): a dead rule found by fixing a test title

Splitting the mislabelled fail-closed test surfaced more than a
mislabel: **`ts.createSourceFile` is error-tolerant and does not throw
on malformed syntax**, so the `try/catch` behind the `unparseable` rule
was unreachable and that rule could never fire. The earlier "fails
closed three ways" claim was overstated — the guard advertised a
capability it did not have. Detection now reads `sf.parseDiagnostics`; a
partial AST can silently lack the `??` nodes and sink calls the rules
look for, so "did not parse" must not read as "inspected and clean".
Mutation-verified: reverting the detection fails that case and only that
case.

Test-file exclusion also moved to the repo's `{test,spec}.{ts,tsx}`
guideline shape — a `.spec.ts` under `packages/<pkg>/src/` was being
scanned as production source. Verified both ways: the `.spec.ts` is
skipped, and the identical content in a non-test file is still caught,
so the exclusion is scoped rather than a hole.

## Verification

- engine + core `tsc --noEmit` clean
- `pnpm test:gate` green (299 + 10 + 71)
- E2E 20/20; capacity + move-path suites 14/14
- full core PG: **1037 passed / 3 failed** — all three reproduce with
the fix stashed (pre-existing)
- engine-default: **279 failed** vs **280 at baseline** with the fix
stashed — pre-existing red lane, no regression
- hold-release + capacity suites: **43/43 identical before and after**
the resolver routing
- `check-capacity-pool-id` ratchet: 14/14 regression cases; clean over
1124 files; exits 1 on the original defect, a multiline fallback, and a
deeply qualified sentinel reintroduced into real source

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Fixed capacity-limit accounting when workflow selection is missing by
consistently deriving the correct capacity pool id.
* Made capacity enforcement align across move and hold/release paths,
rejecting over-limit moves with `capacity-exhausted`.
* **Tests**
* Updated PostgreSQL and added an E2E scenario to verify the corrected
in-transaction gating behavior at `maxConcurrent` limits of 1 and 2.
* **Chores**
* Added an automated guard to detect inconsistent capacity pool id
fallback patterns in code.
* **Public API**
* Exposed `resolveCapacityPoolId` for consistent capacity pool id
derivation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-27 21:09:51 -07:00
committed by GitHub
parent 387e836432
commit 7871b28766
15 changed files with 668 additions and 53 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Fix the column capacity check so a task with no workflow selection is counted against the limit.
category: fix
dev: `moves.ts` asked `countActiveInCapacitySlotAsync` for pool `"builtin:coding"` while the counter buckets selection-less rows under `DEFAULT_WORKFLOW_POOL_ID`, so the count was always 0 and a finite limit could never bind. Both sides now derive the pool through the shared `resolveCapacityPoolId`. NOTE: no operator-visible change yet — the capacity block is still gated on `experimentalFeatures.workflowColumns`, which nothing in production sets (Phase A3 R2). If that gate is removed, this becomes user-visible and the changeset should be re-categorised.

View File

@@ -14,14 +14,14 @@
"type": "module", "type": "module",
"packageManager": "pnpm@10.33.0", "packageManager": "pnpm@10.33.0",
"scripts": { "scripts": {
"pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-routes-modular.mjs", "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-routes-modular.mjs",
"pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-routes-modular.mjs", "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-routes-modular.mjs",
"check:line-count": "node scripts/check-file-line-count.mjs", "check:line-count": "node scripts/check-file-line-count.mjs",
"check:routes-modular": "node scripts/check-routes-modular.mjs", "check:routes-modular": "node scripts/check-routes-modular.mjs",
"check:changesets": "node scripts/check-changeset-format.mjs", "check:changesets": "node scripts/check-changeset-format.mjs",
"check:quarantine-ledger": "node scripts/check-quarantine-ledger.mjs", "check:quarantine-ledger": "node scripts/check-quarantine-ledger.mjs",
"check:mock-completeness": "node scripts/check-mock-completeness.mjs", "check:mock-completeness": "node scripts/check-mock-completeness.mjs",
"test:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-mock-completeness.mjs && sh -c 'pnpm --filter @fusion/engine test:core & engine_pid=$!; pnpm --filter @fusion/core test:pg-gate & pg_pid=$!; status=0; wait $engine_pid || status=1; wait $pg_pid || status=1; exit $status' && pnpm --filter @runfusion/fusion test:ci-shape", "test:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-mock-completeness.mjs && sh -c 'pnpm --filter @fusion/engine test:core & engine_pid=$!; pnpm --filter @fusion/core test:pg-gate & pg_pid=$!; status=0; wait $engine_pid || status=1; wait $pg_pid || status=1; exit $status' && pnpm --filter @runfusion/fusion test:ci-shape",
"smoke:boot": "node scripts/boot-smoke.mjs", "smoke:boot": "node scripts/boot-smoke.mjs",
"local": "node scripts/start-local.mjs", "local": "node scripts/start-local.mjs",
"dev": "node scripts/dev-with-memory.mjs", "dev": "node scripts/dev-with-memory.mjs",

View File

@@ -370,34 +370,43 @@ pgTest("move-path equivalence — the flag gates MORE than side effects (Phase A
expect(hooksErr!.message).toContain("Unknown column for this workflow"); expect(hooksErr!.message).toContain("Unknown column for this workflow");
}); });
it("UNPROVEN: in-transaction column capacity did NOT reject on EITHER path in this fixture", async () => { it("DIVERGENCE: in-transaction capacity rejects on the HOOKS path only — the inline path cannot run it", async () => {
/* /*
HONEST NEGATIVE RESULT — recorded rather than dropped, because the code gate FNXC:WorkflowCapacity 2026-07-28-19:40 (pool-id sentinel fix):
is real and the practical impact is not what reading it suggests. WAS `UNPROVEN: … did NOT reject on EITHER path`. That test recorded an honest
negative result and left the cause open: "something further in
(`resolveColumnCapacity`'s limit resolution, or what
`countActiveInCapacitySlotAsync` counts as an occupant) keeps the check from
firing even when the flag is on. This suite does not establish which." It
also predicted its own obsolescence: "if a future change makes this reject,
that is the capacity gate coming alive."
STRUCTURALLY, `moves.ts` wraps the whole in-transaction capacity block in THE ANSWER, established by the sentinel fix: neither of those guesses. The
`if (useWorkflow && workflowIr && fromColumn !== toColumn)`, so it cannot run counter and the limit resolution were both fine. `moves.ts` asked the counter
on the LIVE inline path at all. The obvious inference is "converging onto the for occupants of pool `"builtin:coding"` while the counter buckets
hooks path turns store-level capacity rejection ON for every project" — a selection-less rows under `DEFAULT_WORKFLOW_POOL_ID`, so the count came back
serious blast radius, since `capacity-exhausted` is a code the graph column 0 for a pool nothing is ever placed in. Both sides now derive the pool
boundary parks on and the promote route surfaces to operators. through `resolveCapacityPoolId`, and the hooks path rejects.
EMPIRICALLY that inference does not hold up: with `maxConcurrent: 1` and a The blast-radius question this test was holding open is therefore ANSWERED for
wip column already occupied, the second move was ACCEPTED on both paths. So the hooks path and STILL OPEN for the inline one: the inline path remains
something further in (`resolveColumnCapacity`'s limit resolution, or what structurally unable to run the block (`if (useWorkflow && …)`), so converging
`countActiveInCapacitySlotAsync` counts as an occupant — a task with no the paths still turns store-level capacity rejection on for every project.
session/agent may not count) keeps the check from firing even when the flag That convergence stays an operator decision — see the R2 note in
is on. This suite does not establish which. workflow-capacity-invariant.pg.test.ts.
Consequence for the convergence decision: the capacity blast radius is
UNQUANTIFIED, not absent. It needs its own investigation before either path
is made authoritative — this test pins today's observed behavior so that
investigation starts from a fact instead of the code reading.
*/ */
const store = h.store(); const store = h.store();
await store.updateSettings({ maxConcurrent: 1 }); await store.updateSettings({ maxConcurrent: 1 });
/* Each phase starts from an EMPTY wip column. Before the gate bound, the two phases could share
one fixture because nothing ever counted occupants; now they cannot — the inline phase leaves
two cards in wip, and the hooks phase's own HOLDER move would trip the cap before the
contended move under test ever runs (observed: "column at capacity (2/1)"). Evacuating is
what keeps this a test of the contender's move rather than of fixture residue. */
async function fillThenMoveSecond(): Promise<Error | null> { async function fillThenMoveSecond(): Promise<Error | null> {
for (const stale of await store.listTasks({ includeArchived: false })) {
if (stale.column === "in-progress") await store.deleteTask(stale.id);
}
const first = await store.createTask({ description: "capacity holder" }); const first = await store.createTask({ description: "capacity holder" });
await store.moveTask(first.id, "todo"); await store.moveTask(first.id, "todo");
await store.moveTask(first.id, "in-progress"); await store.moveTask(first.id, "in-progress");
@@ -407,11 +416,14 @@ pgTest("move-path equivalence — the flag gates MORE than side effects (Phase A
} }
await setPath("inline"); await setPath("inline");
// Unchanged: the block is unreachable on this path regardless of the pool id.
expect(await fillThenMoveSecond()).toBeNull(); expect(await fillThenMoveSecond()).toBeNull();
await setPath("hooks"); await setPath("hooks");
// If a future change makes this reject, that is the capacity gate coming const hooksErr = await fillThenMoveSecond();
// alive — and this failure is the signal to re-open the blast-radius question. expect(hooksErr).toBeInstanceOf(Error);
expect(await fillThenMoveSecond()).toBeNull(); expect((hooksErr as unknown as { rejection?: { code?: string } }).rejection?.code).toBe(
"capacity-exhausted",
);
}); });
}); });

View File

@@ -111,9 +111,25 @@ pgTest("in-transaction column capacity — ground truth (Phase A3)", () => {
return { error, secondColumn: (await store.getTask(contender.id))?.column }; return { error, secondColumn: (await store.getTask(contender.id))?.column };
} }
it("DEFECT (R2): on the LIVE inline path the in-txn capacity check cannot run at all", async () => { it("DEFECT (R2, STILL LIVE): on the production inline path the in-txn capacity check cannot run at all", async () => {
// Not a surprise, but it is the reason the defect is latent rather than /*
// active today: the whole block is inside `if (useWorkflow && …)`. FNXC:WorkflowCapacity 2026-07-28-19:05:
R1 IS FIXED; R2 IS NOT, and this test is the standing evidence. The whole
capacity block sits inside `if (useWorkflow && …)`, and `useWorkflow` reads
`experimentalFeatures.workflowColumns === true`, which NOTHING in production
sets (it is absent from DEFAULT_GLOBAL_SETTINGS and has no writer outside
tests). So on the path real projects take, the gate still does not run at all
and cards still enter wip past the cap.
Concretely, measured on this suite's fixture with maxConcurrent 1:
flag OFF, no selection -> ADMITTED (this test)
flag OFF, selection -> ADMITTED
flag ON, no selection -> REFUSED (was ADMITTED before the R1 fix)
flag ON, selection -> REFUSED
Making the gate bind for real projects means removing the `useWorkflow`
condition from the capacity block — a separate, larger blast radius than the
sentinel fix, and an operator decision rather than a drive-by.
*/
const store = h.store(); const store = h.store();
await store.updateSettings({ maxConcurrent: 1 }); await store.updateSettings({ maxConcurrent: 1 });
await setPath("inline"); await setPath("inline");
@@ -124,12 +140,14 @@ pgTest("in-transaction column capacity — ground truth (Phase A3)", () => {
expect(secondColumn).toBe("in-progress"); // limit of 1, two occupants expect(secondColumn).toBe("in-progress"); // limit of 1, two occupants
}); });
it("DEFECT (R1): flag-ON, a NO-SELECTION task still slips the limit — the pool sentinels disagree", async () => { it("FIXED (R1): flag-ON, a NO-SELECTION task is now REFUSED at the limit", async () => {
/* /*
THE ACTUAL BUG. `moves.ts:319` asks the counter for pool "builtin:coding"; FNXC:WorkflowCapacity 2026-07-28-19:05:
`countActiveInCapacitySlotAsyncImpl` buckets no-selection rows under Was `DEFECT (R1)`, asserting the wrong-but-current outcome. The pool-id
"__default-workflow__". No occupant is ever counted, so the limit cannot sentinel is fixed: `moves.ts` and the counter now BOTH derive the pool through
bind. Flip this expectation to a rejection when the sentinel is fixed. `resolveCapacityPoolId`, so a selection-less row is bucketed and asked about
under the same key and the limit binds. Flipping this expectation is the
acceptance test the original ratchet named.
*/ */
const store = h.store(); const store = h.store();
await store.updateSettings({ maxConcurrent: 1 }); await store.updateSettings({ maxConcurrent: 1 });
@@ -137,8 +155,12 @@ pgTest("in-transaction column capacity — ground truth (Phase A3)", () => {
const { error, secondColumn } = await fillWipThenAdmitSecond(); const { error, secondColumn } = await fillWipThenAdmitSecond();
expect(error).toBeNull(); expect(error).toBeInstanceOf(Error);
expect(secondColumn).toBe("in-progress"); expect((error as unknown as { rejection?: { code?: string } }).rejection?.code).toBe(
"capacity-exhausted",
);
expect(secondColumn).toBe("todo"); // refused, stays put
}); });
it("DISCRIMINATOR: with an EXPLICIT builtin:coding selection the sentinels agree and the limit BINDS", async () => { it("DISCRIMINATOR: with an EXPLICIT builtin:coding selection the sentinels agree and the limit BINDS", async () => {
@@ -181,8 +203,8 @@ pgTest("in-transaction column capacity — ground truth (Phase A3)", () => {
invariant nobody can prove is an invariant nobody has; this is the proof invariant nobody can prove is an invariant nobody has; this is the proof
obligation, written down. obligation, written down.
*/ */
it.fails( it(
"INVARIANT (currently BROKEN): a move into a full capacity column is refused, even with no workflow selection", "INVARIANT (HOLDS on the flag-ON path): a move into a full capacity column is refused, even with no workflow selection",
async () => { async () => {
const store = h.store(); const store = h.store();
await store.updateSettings({ maxConcurrent: 1 }); await store.updateSettings({ maxConcurrent: 1 });

View File

@@ -416,7 +416,7 @@ export {
} from "./plugin-gate-verdict.js"; } from "./plugin-gate-verdict.js";
export type { PluginGateVerdict, ColumnPluginGate } from "./plugin-gate-verdict.js"; export type { PluginGateVerdict, ColumnPluginGate } from "./plugin-gate-verdict.js";
// ── U6: workflow capacity (WIP) resolution shared by store + sweep ─────────── // ── U6: workflow capacity (WIP) resolution shared by store + sweep ───────────
export { resolveColumnCapacity, DEFAULT_WORKFLOW_POOL_ID } from "./workflow-capacity.js"; export { resolveColumnCapacity, DEFAULT_WORKFLOW_POOL_ID, resolveCapacityPoolId } from "./workflow-capacity.js";
export type { ColumnCapacity } from "./workflow-capacity.js"; export type { ColumnCapacity } from "./workflow-capacity.js";
// ── U5: workflow lifecycle reconciliation (switch / edit / delete) ─────────── // ── U5: workflow lifecycle reconciliation (switch / edit / delete) ───────────
export { export {

View File

@@ -447,7 +447,7 @@ export {
} from "./plugin-gate-verdict.js"; } from "./plugin-gate-verdict.js";
export type { PluginGateVerdict, ColumnPluginGate } from "./plugin-gate-verdict.js"; export type { PluginGateVerdict, ColumnPluginGate } from "./plugin-gate-verdict.js";
// ── U6: workflow capacity (WIP) resolution shared by store + sweep ─────────── // ── U6: workflow capacity (WIP) resolution shared by store + sweep ───────────
export { resolveColumnCapacity, resolveWipBudgetColumns, DEFAULT_WORKFLOW_POOL_ID } from "./workflow-capacity.js"; export { resolveColumnCapacity, resolveWipBudgetColumns, DEFAULT_WORKFLOW_POOL_ID, resolveCapacityPoolId } from "./workflow-capacity.js";
export { createWorkflowEventBus, getWorkflowEventBus, emitWorkflowLifecycleEvent, resetWorkflowEventBusForTesting } from "./workflow-events.js"; export { createWorkflowEventBus, getWorkflowEventBus, emitWorkflowLifecycleEvent, resetWorkflowEventBusForTesting } from "./workflow-events.js";
export type { WorkflowEventBus, WorkflowEventSubscriber, WorkflowEventSubscription } from "./workflow-events.js"; export type { WorkflowEventBus, WorkflowEventSubscriber, WorkflowEventSubscription } from "./workflow-events.js";
export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH } from "./types/workflow-events.js"; export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH } from "./types/workflow-events.js";

View File

@@ -19,7 +19,7 @@ import {isBuiltinWorkflowId, getBuiltinWorkflow, resolveDefaultWorkflowIr, DEFAU
import {parseWorkflowIr} from "../workflow-ir.js"; import {parseWorkflowIr} from "../workflow-ir.js";
import {findWorkflowColumn, resolveColumnPluginGates} from "../plugin-gate-verdict.js"; import {findWorkflowColumn, resolveColumnPluginGates} from "../plugin-gate-verdict.js";
import {getTraitRegistry, resolveColumnFlags} from "../trait-registry.js"; import {getTraitRegistry, resolveColumnFlags} from "../trait-registry.js";
import {resolveColumnCapacity, resolveWipBudgetColumns} from "../workflow-capacity.js"; import {resolveColumnCapacity, resolveWipBudgetColumns, resolveCapacityPoolId} from "../workflow-capacity.js";
import { import {
type TransitionColumnFacts, type TransitionColumnFacts,
evaluateCapacityRejection, evaluateCapacityRejection,
@@ -315,9 +315,19 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum
// capacity check is not a guard (U6 fills the enforcement; U4 leaves a // capacity check is not a guard (U6 fills the enforcement; U4 leaves a
// pass-through slot). An explicit option value wins; otherwise derive it. // pass-through slot). An explicit option value wins; otherwise derive it.
const bypassGuards = store.resolveWorkflowBypassGuards(moveSource, options); const bypassGuards = store.resolveWorkflowBypassGuards(moveSource, options);
const effectiveWorkflowIdForMove = useWorkflow /*
? (await store.getTaskWorkflowSelectionAsync(id))?.workflowId ?? "builtin:coding" FNXC:WorkflowCapacity 2026-07-28-19:05 (pool-id sentinel fix):
: "builtin:coding"; ONE selection read, TWO derived ids, because they are two different things
that were previously conflated into one variable — and the conflation is the
defect. The capacity POOL key must match how the counter buckets rows
(`resolveCapacityPoolId`, shared); the WORKFLOW id is telemetry and must stay
a real workflow id, never the bucketing sentinel.
*/
const workflowSelectionForMove = useWorkflow
? await store.getTaskWorkflowSelectionAsync(id)
: undefined;
const effectiveWorkflowIdForMove = workflowSelectionForMove?.workflowId ?? DEFAULT_WORKFLOW_ID;
const capacityPoolIdForMove = resolveCapacityPoolId(workflowSelectionForMove?.workflowId);
const workflowIr: WorkflowIr | undefined = useWorkflow const workflowIr: WorkflowIr | undefined = useWorkflow
? await resolveTaskWorkflowIrForMove(store, id) ? await resolveTaskWorkflowIrForMove(store, id)
: undefined; : undefined;
@@ -932,7 +942,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum
store.countActiveInCapacitySlotAsync({ store.countActiveInCapacitySlotAsync({
tx, tx,
targetColumn: budgetColumn, targetColumn: budgetColumn,
workflowId: effectiveWorkflowIdForMove, workflowId: capacityPoolIdForMove,
countPending, countPending,
excludeTaskId: id, excludeTaskId: id,
}), }),

View File

@@ -9,6 +9,7 @@
* instance as its first parameter and performs byte-identical work. * instance as its first parameter and performs byte-identical work.
*/ */
import {TaskStore, storeLog, WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX, WORKFLOW_MOVE_POLICY_TIMEOUT_MS} from "../store.js"; import {TaskStore, storeLog, WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX, WORKFLOW_MOVE_POLICY_TIMEOUT_MS} from "../store.js";
import { resolveCapacityPoolId } from "../workflow-capacity.js";
import {TransitionRejectionError} from "./errors.js"; import {TransitionRejectionError} from "./errors.js";
import * as schema from "../postgres/schema/index.js"; import * as schema from "../postgres/schema/index.js";
import {and, eq, isNull, ne, or, sql} from "drizzle-orm"; import {and, eq, isNull, ne, or, sql} from "drizzle-orm";
@@ -764,7 +765,7 @@ export function countActiveInCapacitySlotSyncImpl(store: TaskStore, params: { ta
let count = 0; let count = 0;
for (const row of rows) { for (const row of rows) {
const effectiveWorkflowId = row.wid ?? TaskStore.DEFAULT_WORKFLOW_POOL_ID; const effectiveWorkflowId = resolveCapacityPoolId(row.wid);
if (effectiveWorkflowId !== workflowId) continue; if (effectiveWorkflowId !== workflowId) continue;
if (row.col === targetColumn) { if (row.col === targetColumn) {
@@ -816,7 +817,7 @@ export async function countActiveInCapacitySlotAsyncImpl(store: TaskStore, param
let count = 0; let count = 0;
for (const row of rows) { for (const row of rows) {
const effectiveWorkflowId = row.wid ?? TaskStore.DEFAULT_WORKFLOW_POOL_ID; const effectiveWorkflowId = resolveCapacityPoolId(row.wid);
if (effectiveWorkflowId !== workflowId) continue; if (effectiveWorkflowId !== workflowId) continue;
if (row.col === targetColumn) { if (row.col === targetColumn) {

View File

@@ -11,6 +11,7 @@
*/ */
import { TaskStore } from "../store.js"; import { TaskStore } from "../store.js";
import { resolveCapacityPoolId } from "../workflow-capacity.js";
import { isBuiltinWorkflowId } from "../builtin-workflows.js"; import { isBuiltinWorkflowId } from "../builtin-workflows.js";
import { InsightStore } from "../insight-store.js"; import { InsightStore } from "../insight-store.js";
import { ResearchStore } from "../research-store.js"; import { ResearchStore } from "../research-store.js";
@@ -287,7 +288,7 @@ export function resolveTaskCustomFieldDefsSyncImpl(store: TaskStore, taskId: str
export function resolveEffectiveWorkflowIdSyncImpl(store: TaskStore, taskId: string): string { export function resolveEffectiveWorkflowIdSyncImpl(store: TaskStore, taskId: string): string {
const selection = store.getTaskWorkflowSelection(taskId); const selection = store.getTaskWorkflowSelection(taskId);
return selection?.workflowId ?? TaskStore.DEFAULT_WORKFLOW_POOL_ID; return resolveCapacityPoolId(selection?.workflowId);
} }
export async function clearTaskWorkflowSelectionImpl(store: TaskStore, taskId: string): Promise<void> { export async function clearTaskWorkflowSelectionImpl(store: TaskStore, taskId: string): Promise<void> {

View File

@@ -34,6 +34,33 @@ const DEFAULT_WIP_COLUMN_ID = "in-progress";
* is not a real workflow row id (no `builtin:`/custom collision possible). */ * is not a real workflow row id (no `builtin:`/custom collision possible). */
export const DEFAULT_WORKFLOW_POOL_ID = "__default-workflow__"; export const DEFAULT_WORKFLOW_POOL_ID = "__default-workflow__";
/*
FNXC:WorkflowCapacity 2026-07-28-19:05 (pool-id sentinel fix):
THE one place the "no selection → which pool?" convention is expressed.
It exists because the convention was previously restated at each end of the
comparison, and the two restatements disagreed: the COUNTER bucketed
selection-less rows under `DEFAULT_WORKFLOW_POOL_ID` while `moves.ts` asked the
counter for pool `"builtin:coding"`. Nothing ever landed in the pool being asked
about, so the count came back 0 and a finite limit could never bind — the
in-transaction capacity gate was structurally dead for every default-workflow
task (Phase A3, R1).
A shared constant alone would NOT have prevented that: both sides had the
constant available and one of them still wrote a literal. Both sides now call
THIS function, so "what pool does a selection-less task belong to" has exactly
one answer and no call site is in a position to disagree with it.
NOT to be confused with `DEFAULT_WORKFLOW_ID` ("builtin:coding"). That is a real,
resolvable workflow row id and is the correct fallback when the value is used to
RESOLVE AN IR (as `scheduler.ts` does). This is a bucketing key that deliberately
cannot collide with any workflow id. Using either one in the other's role is the
bug this function exists to make unspellable.
*/
export function resolveCapacityPoolId(selectionWorkflowId: string | null | undefined): string {
return selectionWorkflowId ?? DEFAULT_WORKFLOW_POOL_ID;
}
/** Resolved capacity configuration for a single column. */ /** Resolved capacity configuration for a single column. */
export interface ColumnCapacity { export interface ColumnCapacity {
/** True when the column carries a capacity (`wip`/`countsTowardWip`) trait. */ /** True when the column carries a capacity (`wip`/`countsTowardWip`) trait. */

View File

@@ -0,0 +1,177 @@
/*
FNXC:WorkflowCapacity 2026-07-28-22:45 (PR #2488 review — ratchet regression suite):
THE ACCEPTANCE TEST FOR A GUARD IS NOT "IT PASSES ON MAIN".
The first version of `check-capacity-pool-id` passed on main and passed on the
REINTRODUCED ORIGINAL DEFECT, because it matched one spelling of the fallback
(`?? DEFAULT_WORKFLOW_POOL_ID`) and the real bug used another
(`?? "builtin:coding"`). Green on the bug it exists to prevent is worse than no
guard: it stops anyone looking.
So every form the guard must catch is pinned here as a case. The first fixture is
the ACTUAL PRE-FIX `moves.ts` shape, reduced — if this suite ever goes green on
that, the ratchet has silently narrowed again and this file is what says so.
The negative cases matter just as much: `?? "builtin:coding"` is the legitimate
default for a WORKFLOW id in about eight places (scheduler's IR-resolution key,
task creation, analytics). A guard that fired on those would be suppressed until
it rotted, so the rule is deliberately about what reaches a CAPACITY SINK, not
about a spelling.
*/
import { describe, expect, it } from "vitest";
import { findViolationsInSource } from "../../../../scripts/lib/capacity-pool-id-check.mjs";
const rules = (src: string): string[] =>
(findViolationsInSource("packages/core/src/task-store/example.ts", src) as Array<{ rule: string }>).map(
(v) => v.rule,
);
describe("check-capacity-pool-id ratchet — forms it MUST catch", () => {
it("catches the ORIGINAL DEFECT: a `?? \"builtin:coding\"` local reaching the counter", () => {
/* The reduced pre-fix moves.ts. The previous regex version passed on this. */
const src = `
async function moveTaskInternalImpl(store: any, id: string) {
const effectiveWorkflowIdForMove = useWorkflow
? (await store.getTaskWorkflowSelectionAsync(id))?.workflowId ?? "builtin:coding"
: "builtin:coding";
await store.countActiveInCapacitySlotAsync({
tx, targetColumn: budgetColumn, workflowId: effectiveWorkflowIdForMove, countPending,
});
}
`;
expect(rules(src)).toContain("unresolved-pool-into-capacity-sink");
});
it("catches a bare literal passed inline to the counter", () => {
const src = `
async function f(store: any) {
await store.countActiveInCapacitySlotAsync({ workflowId: "builtin:coding" });
}
`;
expect(rules(src)).toContain("unresolved-pool-into-capacity-sink");
});
it("catches a MULTILINE fallback — the AST sees one node regardless of formatting", () => {
const src = `
const poolId =
selection
?.workflowId
??
DEFAULT_WORKFLOW_POOL_ID;
`;
expect(rules(src)).toContain("sentinel-fallback");
});
it("catches a DEEPLY QUALIFIED sentinel reference", () => {
const src = `const poolId = selection?.workflowId ?? Foo.Bar.Baz.DEFAULT_WORKFLOW_POOL_ID;`;
expect(rules(src)).toContain("sentinel-fallback");
});
it("catches the sentinel's RAW STRING VALUE, not just its constant name", () => {
const src = `const poolId = selection?.workflowId ?? "__default-workflow__";`;
expect(rules(src)).toContain("sentinel-fallback");
});
it("catches an underived POSITIONAL argument to countCapacitySlot", () => {
const src = `
const workflowId = byTask.get(task.id) ?? "builtin:coding";
const n = countCapacitySlot(allTasks, byTask, budgetColumns, workflowId, countPending);
`;
expect(rules(src)).toContain("unresolved-pool-into-capacity-sink");
});
it("catches a sink fed by a local that is NOT resolver-derived", () => {
const src = `
const poolId = someOtherThing();
await store.countActiveInCapacitySlotAsync({ workflowId: poolId });
`;
expect(rules(src)).toContain("unresolved-pool-into-capacity-sink");
});
});
describe("check-capacity-pool-id ratchet — forms it must NOT flag", () => {
it("accepts a direct resolver call at the sink", () => {
const src = `
await store.countActiveInCapacitySlotAsync({
workflowId: resolveCapacityPoolId(selection?.workflowId),
});
`;
expect(rules(src)).toEqual([]);
});
it("accepts a local derived from the resolver, declared before OR after the sink", () => {
const before = `
const poolId = resolveCapacityPoolId(selection?.workflowId);
await store.countActiveInCapacitySlotAsync({ workflowId: poolId });
`;
expect(rules(before)).toEqual([]);
});
it("accepts `?? \"builtin:coding\"` when it is a WORKFLOW id and never reaches a capacity sink", () => {
/* scheduler.ts's IR-resolution key. Flagging this would make the guard noise
that gets suppressed — the rule is about the sink, not the spelling. */
const src = `
const workflowIdByTaskId = new Map<string, string>();
workflowIdByTaskId.set(task.id, selection?.workflowId ?? "builtin:coding");
const ir = await resolveWorkflowIrById(store, workflowId, cache);
`;
expect(rules(src)).toEqual([]);
});
it("accepts naming the sentinel constant where nothing is derived from a selection", () => {
/* scheduler's capacity DIAGNOSTIC label — no selection input, nothing to drift. */
const src = `const perColumnGates = [{ workflowId: DEFAULT_WORKFLOW_POOL_ID, columnId: "in-progress" }];`;
expect(rules(src)).toEqual([]);
});
});
describe("check-capacity-pool-id ratchet — it fails CLOSED", () => {
/*
FNXC:WorkflowCapacity 2026-07-28-23:30 (PR #2488 review):
These were ONE test titled "unparseable" that actually simulated a read()
throw and asserted "unreadable" — it never reached the parse path at all. A
test that misreports its own subject is this PR's entire failure mode in
miniature, so they are split and each now exercises the path it names.
Writing the second one surfaced a real defect: `ts.createSourceFile` is
error-TOLERANT and does not throw on malformed syntax, so the try/catch it was
meant to cover was unreachable and the "unparseable" rule could never fire.
Detection now reads `parseDiagnostics`.
*/
it("reports an UNREADABLE file rather than skipping it", async () => {
const { findViolations } = await import("../../../../scripts/lib/capacity-pool-id-check.mjs");
const out = (findViolations([
{
file: "packages/core/src/broken.ts",
read: () => {
throw new Error("EACCES");
},
},
]) as Array<{ rule: string }>).map((v) => v.rule);
// "could not inspect" must never render as "inspected and clean".
expect(out).toContain("unreadable");
});
it("reports an UNPARSEABLE file — real malformed syntax, reaching the parse check", () => {
/* Genuinely malformed TypeScript. A partial AST can silently lack the `??`
nodes and sink calls the rules look for, so "parsed badly" must not read
as "inspected and clean". */
const malformed = `
const x = { unclosed: ((( ;
function )( {
`;
const out = (findViolationsInSource("packages/core/src/malformed.ts", malformed) as Array<{ rule: string }>).map(
(v) => v.rule,
);
expect(out).toContain("unparseable");
});
it("does NOT report well-formed source as unparseable", () => {
/* The negative half: a diagnostics-based check that fired on valid syntax
would be noise, and noise gets suppressed. */
const fine = `const poolId = resolveCapacityPoolId(selection?.workflowId);`;
expect(rules(fine)).toEqual([]);
});
});

View File

@@ -686,6 +686,59 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
expect(r.acted).toBe(false); expect(r.acted).toBe(false);
}); });
}); });
/*
FNXC:WorkflowCapacity 2026-07-28-19:20 (pool-id sentinel fix — E2E acceptance):
The gate must BIND, and it must bind IN BOTH DIRECTIONS. A test that only asserts "held at cap"
passes trivially if the mover simply never admits anything — including if a future change breaks
admission outright — so each case is run twice against the SAME fixture with only the cap
changed: at limit 1 the card is HELD, at limit 2 the identical card is ADMITTED.
NO WORKFLOW SELECTION, deliberately. That is the whole defect: `moves.ts` asked the counter for
pool `"builtin:coding"` while the counter buckets selection-less rows under
`DEFAULT_WORKFLOW_POOL_ID`, so nothing was ever counted. A card WITH a selection makes both
sentinels agree and the gate already bound before the fix — seeding one here would have produced a
green test that never touched the bug. (Verified: with the fix reverted, this row fails.)
FLAG-ON, and labelled as such. The capacity block sits inside `if (useWorkflow && …)` and
`useWorkflow` reads a settings key nothing in production sets (Phase A3 R2, still live). So this
row proves the SENTINEL is fixed on the only path where the gate can execute at all; it does NOT
prove production behavior changed. Read the R2 note in workflow-capacity-invariant.pg.test.ts
before treating this as "capacity is enforced".
*/
describe.each([
{ limit: 1, expected: "held" as const },
{ limit: 2, expected: "admitted" as const },
])("in-transaction capacity gate (maxConcurrent=$limit → $expected)", ({ limit, expected }) => {
it(`a selection-less card at the wip boundary is ${expected}`, async () => {
const store = h.store();
await store.updateSettings({ maxConcurrent: limit } as never);
// The capacity block's enclosing flag is GLOBAL-scoped; a project-scoped write is silently
// dropped and the test then measures the unflagged path while claiming otherwise.
await store.updateGlobalSettings({ experimentalFeatures: { workflowColumns: true } } as never);
const holder = await store.createTask({ description: "capacity holder" });
await store.moveTask(holder.id, "todo");
await store.moveTask(holder.id, "in-progress");
const contender = await store.createTask({ description: "capacity contender" });
await store.moveTask(contender.id, "todo");
const error = await store
.moveTask(contender.id, "in-progress")
.then(() => null, (e: unknown) => e as Error);
store.taskCache.delete(contender.id);
const persisted = (await store.getTask(contender.id)).column;
if (expected === "held") {
expect((error as unknown as { rejection?: { code?: string } })?.rejection?.code).toBe("capacity-exhausted");
expect(persisted).toBe("todo"); // observed state: refused, stays put
} else {
expect(error).toBeNull();
expect(persisted).toBe("in-progress"); // the same fixture admits once the cap allows it
}
});
});
}); });
/* /*

View File

@@ -43,7 +43,7 @@ import {
resolveColumnAdjacency, resolveColumnAdjacency,
PLAN_REVIEW_GROUP_ID, PLAN_REVIEW_GROUP_ID,
ACTIVE_WORKFLOW_WORK_ITEM_STATES, ACTIVE_WORKFLOW_WORK_ITEM_STATES,
DEFAULT_WORKFLOW_POOL_ID, resolveCapacityPoolId,
TransitionRejectionError, TransitionRejectionError,
resolveWorkflowIrForTask, resolveWorkflowIrForTask,
isUnplannedSeedPrompt, isUnplannedSeedPrompt,
@@ -113,9 +113,11 @@ export interface HoldReleaseResult {
async function effectiveWorkflowId(store: TaskStore, taskId: string): Promise<string> { async function effectiveWorkflowId(store: TaskStore, taskId: string): Promise<string> {
try { try {
return (await store.getTaskWorkflowSelectionAsync(taskId))?.workflowId ?? DEFAULT_WORKFLOW_POOL_ID; return resolveCapacityPoolId((await store.getTaskWorkflowSelectionAsync(taskId))?.workflowId);
} catch { } catch {
return DEFAULT_WORKFLOW_POOL_ID; /* An unreadable selection is indistinguishable from no selection for pooling
purposes, so it takes the same bucket rather than a second convention. */
return resolveCapacityPoolId(undefined);
} }
} }
@@ -439,7 +441,7 @@ function countCapacitySlot(
): number { ): number {
let count = 0; let count = 0;
for (const t of allTasks) { for (const t of allTasks) {
if ((effectiveWorkflowIdByTask.get(t.id) ?? DEFAULT_WORKFLOW_POOL_ID) !== workflowId) continue; if (resolveCapacityPoolId(effectiveWorkflowIdByTask.get(t.id)) !== workflowId) continue;
if (budgetColumns.has(t.column)) { if (budgetColumns.has(t.column)) {
count += 1; count += 1;
continue; continue;
@@ -573,7 +575,7 @@ export async function runHoldReleaseSweep(
} }
const capacity = resolveColumnCapacity(ir, target, settings); const capacity = resolveColumnCapacity(ir, target, settings);
if (capacity.hasCapacity && Number.isFinite(capacity.limit)) { if (capacity.hasCapacity && Number.isFinite(capacity.limit)) {
const workflowId = effectiveWorkflowIdByTask.get(task.id) ?? DEFAULT_WORKFLOW_POOL_ID; const workflowId = resolveCapacityPoolId(effectiveWorkflowIdByTask.get(task.id));
// U4/KTD-9: count occupants across every column sharing the target's // U4/KTD-9: count occupants across every column sharing the target's
// budget (a shared `limitSetting` pools multiple wip columns). // budget (a shared `limitSetting` pools multiple wip columns).
const budgetColumns = new Set(resolveWipBudgetColumns(ir, target)); const budgetColumns = new Set(resolveWipBudgetColumns(ir, target));

View File

@@ -0,0 +1,68 @@
#!/usr/bin/env node
/*
FNXC:WorkflowCapacity 2026-07-28-22:30 (PR #2488 review — ratchet rebuilt):
CLI wrapper. The rules and their rationale live in
scripts/lib/capacity-pool-id-check.mjs; the regression suite that pins each form
this guard must catch lives in
packages/engine/src/__tests__/capacity-pool-id-check.test.ts.
Wired into `pretest`, `pretest:full`, and the blocking `test:gate`.
*/
import { readFileSync } from "node:fs";
import { execSync } from "node:child_process";
import { findViolations, RESOLVER } from "./lib/capacity-pool-id-check.mjs";
let files;
try {
files = execSync("git ls-files 'packages/*/src/**/*.ts' 'packages/*/src/*.ts'", {
encoding: "utf8",
maxBuffer: 64 * 1024 * 1024,
})
.split("\n")
.map((f) => f.trim())
.filter(Boolean)
// Test sources are excluded using the repo's own guideline shape — the
// `{test,spec}.{ts,tsx}` family — because the earlier `.test.ts`-only suffix
// would have scanned a `.spec.ts` sitting directly under `packages/<pkg>/src/`
// as production source and flagged its fixtures as real violations.
.filter((f) => !f.includes("__tests__") && !/\.(test|spec)\.tsx?$/.test(f));
} catch (err) {
// FAIL CLOSED: if we cannot even enumerate the files, we have checked nothing.
console.error(`check-capacity-pool-id: could not list files — ${err?.message ?? err}`);
process.exit(1);
}
if (files.length === 0) {
console.error("check-capacity-pool-id: file list is EMPTY — refusing to report success on zero files.");
process.exit(1);
}
const violations = findViolations(files.map((file) => ({ file, read: () => readFileSync(file, "utf8") })));
if (violations.length > 0) {
const byRule = {
"unresolved-pool-into-capacity-sink":
`a value reaching a capacity counter's \`workflowId\` must come from ${RESOLVER}().\n` +
" Two enforcement surfaces that each derive the pool themselves WILL drift: that is how the\n" +
' in-transaction gate silently stopped binding (moves.ts derived `?? "builtin:coding"` while the\n' +
" counter bucketed under the sentinel, so nothing was ever counted).",
"sentinel-fallback":
"the pool sentinel must not be restated in a `??` fallback — call the resolver instead.",
unreadable: "a tracked source file could not be read, so it was NOT inspected.",
unparseable: "a tracked source file could not be parsed, so it was NOT inspected.",
};
console.error("\ncheck-capacity-pool-id: FAILED\n");
for (const rule of Object.keys(byRule)) {
const hits = violations.filter((v) => v.rule === rule);
if (hits.length === 0) continue;
console.error(`${rule}: ${byRule[rule]}\n`);
for (const v of hits) console.error(` ${v.file}:${v.line}: ${v.text}`);
console.error("");
}
console.error(`Fix by deriving the pool through ${RESOLVER}(selection?.workflowId).\n`);
process.exit(1);
}
console.log(`check-capacity-pool-id: ok (${files.length} files inspected)`);

View File

@@ -0,0 +1,235 @@
/*
FNXC:WorkflowCapacity 2026-07-28-22:30 (PR #2488 review — ratchet rebuilt):
WHY THIS IS AN AST CHECK AND NOT A REGEX.
The first version of this guard matched `?? DEFAULT_WORKFLOW_POOL_ID` on one line.
It therefore MISSED `?? "builtin:coding"` — the actual defect the whole change
exists to fix — and passed green on the reintroduced bug (verified, not assumed).
A guard that reports success without checking is worse than no guard, because it
stops anyone looking. The claim being made is structural ("no capacity pool id is
derived except through the resolver"), so it is checked structurally.
TWO COMPLEMENTARY RULES. Neither alone is sufficient:
RULE 1 — THE SINK RULE (the one that catches the original defect).
Every argument that flows into a capacity counter's `workflowId` must be
`resolveCapacityPoolId(...)`, or a local whose initializer is that call. The
original defect passed `effectiveWorkflowIdForMove`, a local initialized from a
`??` literal — so this rule fires on it regardless of WHICH literal was used, on
one line or twenty. This is the rule that expresses the real invariant: the
banned thing is not a spelling, it is an underived value reaching the counter.
RULE 2 — THE SENTINEL RULE (cheap, catches restatements of the convention).
No `??` whose right-hand side is the pool sentinel — under ANY qualification
depth (`X.Y.Z.DEFAULT_WORKFLOW_POOL_ID`) or as its raw string value
("__default-workflow__") — outside the module that owns the convention. Being
AST-based, a fallback split across lines is the same node and is caught.
WHY `?? "builtin:coding"` IS NOT BANNED OUTRIGHT. That literal is the legitimate
default for a WORKFLOW id in ~8 places (scheduler's IR-resolution key,
task-creation, analytics). It is only a bug when it reaches a CAPACITY POOL, and
that distinction is exactly what Rule 1 encodes. Banning the spelling everywhere
would be cargo-culting the rule past the thing it protects, and would have to be
suppressed so often it would rot.
FAIL CLOSED. An unreadable or unparseable file is reported as a violation, never
skipped — "could not inspect" must not render as "inspected and clean", which is
the same failure shape as the regex that could not see the defect.
*/
import ts from "typescript";
/** The module that owns the convention; the resolver's own `??` lives here. */
export const CONVENTION_OWNER = "packages/core/src/workflow-capacity.ts";
/** The canonical resolver every pool-id derivation must go through. */
export const RESOLVER = "resolveCapacityPoolId";
/** Capacity counters whose `workflowId` input is a pool id. */
export const CAPACITY_SINKS = new Set([
"countActiveInCapacitySlotAsync",
"countActiveInCapacitySlotSync",
"countActiveInCapacitySlot",
"countCapacitySlot",
]);
/** `countCapacitySlot(allTasks, byTask, budgetColumns, workflowId, countPending)` */
const POSITIONAL_SINKS = { countCapacitySlot: 3 };
const SENTINEL_CONST = "DEFAULT_WORKFLOW_POOL_ID";
const SENTINEL_VALUE = "__default-workflow__";
/** Right-most name of a possibly-qualified reference, at any depth. */
function tailName(node) {
let cur = node;
while (ts.isPropertyAccessExpression(cur)) cur = cur.name;
return ts.isIdentifier(cur) ? cur.text : undefined;
}
function isResolverCall(node) {
return (
node &&
ts.isCallExpression(node) &&
tailName(node.expression) === RESOLVER
);
}
/** True when `expr` is the resolver call, or a local initialized from one. */
function isDerivedThroughResolver(expr, resolverLocals) {
if (!expr) return false;
if (isResolverCall(expr)) return true;
if (ts.isIdentifier(expr)) return resolverLocals.has(expr.text);
// `cond ? resolver(a) : resolver(b)` is still derived through the resolver.
if (ts.isConditionalExpression(expr)) {
return (
isDerivedThroughResolver(expr.whenTrue, resolverLocals) &&
isDerivedThroughResolver(expr.whenFalse, resolverLocals)
);
}
if (ts.isAsExpression(expr) || ts.isParenthesizedExpression(expr)) {
return isDerivedThroughResolver(expr.expression, resolverLocals);
}
return false;
}
/**
* Analyse one source file.
* @returns {Array<{file:string,line:number,rule:string,text:string}>}
*/
export function findViolationsInSource(file, text) {
const violations = [];
/*
FNXC:WorkflowCapacity 2026-07-28-23:30 (PR #2488 review):
Parse failure is detected via `parseDiagnostics`, NOT via a try/catch.
`ts.createSourceFile` is error-TOLERANT: given `function )( {` it returns a
source file carrying diagnostics rather than throwing, so the catch this
replaced was unreachable and the "unparseable" rule could never fire. That made
the guard's own fail-closed claim overstated in exactly the way this PR is
about — a check advertising a capability it did not have. A file whose syntax
did not parse yields a partial AST, so its `??` nodes and sink calls may simply
be absent: reporting it clean would be reporting "not inspected" as "inspected".
The defensive catch is kept for a genuine internal error, but detection is the
diagnostics check.
*/
let sf;
try {
sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
} catch (err) {
return [{ file, line: 0, rule: "unparseable", text: `parser threw: ${String(err && err.message)}` }];
}
const parseErrors = sf.parseDiagnostics ?? [];
if (parseErrors.length > 0) {
const first = ts.flattenDiagnosticMessageText(parseErrors[0].messageText, " ");
return [
{
file,
line: 0,
rule: "unparseable",
text: `${parseErrors.length} syntax error(s), first: ${first} — a file that did not parse was NOT inspected`,
},
];
}
const lineOf = (node) => sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1;
const snippet = (node) => node.getText(sf).replace(/\s+/g, " ").slice(0, 140);
// Locals whose initializer is a resolver call — collected first so a sink that
// reads one is accepted regardless of declaration order within the file.
const resolverLocals = new Set();
const collect = (node) => {
if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name)) {
if (isDerivedThroughResolver(node.initializer, resolverLocals)) resolverLocals.add(node.name.text);
}
ts.forEachChild(node, collect);
};
collect(sf);
// Second pass: a local initialized from another resolver local.
collect(sf);
const visit = (node) => {
// ── RULE 2: sentinel restated in a `??` fallback ──────────────────────────
if (
ts.isBinaryExpression(node) &&
node.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken &&
file !== CONVENTION_OWNER
) {
const rhs = node.right;
const isSentinelConst = tailName(rhs) === SENTINEL_CONST;
const isSentinelValue = ts.isStringLiteral(rhs) && rhs.text === SENTINEL_VALUE;
if (isSentinelConst || isSentinelValue) {
violations.push({
file,
line: lineOf(node),
rule: "sentinel-fallback",
text: snippet(node),
});
}
}
// ── RULE 1: something underived reaching a capacity counter ───────────────
if (ts.isCallExpression(node)) {
const callee = tailName(node.expression);
if (callee && CAPACITY_SINKS.has(callee)) {
// Object-literal form: `count...({ workflowId: <expr> })`
for (const arg of node.arguments) {
if (!ts.isObjectLiteralExpression(arg)) continue;
for (const prop of arg.properties) {
if (!ts.isPropertyAssignment(prop)) continue;
if (tailName(prop.name) !== "workflowId") continue;
if (!isDerivedThroughResolver(prop.initializer, resolverLocals)) {
violations.push({
file,
line: lineOf(prop),
rule: "unresolved-pool-into-capacity-sink",
text: snippet(prop),
});
}
}
}
// Positional form.
const idx = POSITIONAL_SINKS[callee];
if (idx !== undefined && node.arguments.length > idx) {
const arg = node.arguments[idx];
if (!isDerivedThroughResolver(arg, resolverLocals)) {
violations.push({
file,
line: lineOf(arg),
rule: "unresolved-pool-into-capacity-sink",
text: snippet(arg),
});
}
}
}
}
ts.forEachChild(node, visit);
};
visit(sf);
return violations;
}
/**
* @param {Array<{file:string, read:() => string}>} entries
* @returns {Array<{file:string,line:number,rule:string,text:string}>}
*/
export function findViolations(entries) {
const out = [];
for (const entry of entries) {
let text;
try {
text = entry.read();
} catch (err) {
// FAIL CLOSED: an uninspectable file is a violation, not a pass.
out.push({
file: entry.file,
line: 0,
rule: "unreadable",
text: `could not read (${String(err && err.message)}) — a file that cannot be inspected must not report as clean`,
});
continue;
}
out.push(...findViolationsInSource(entry.file, text));
}
return out;
}