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:
7
.changeset/capacity-pool-sentinel.md
Normal file
7
.changeset/capacity-pool-sentinel.md
Normal 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.
|
||||||
@@ -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",
|
||||||
|
|||||||
@@ -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",
|
||||||
|
);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -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 });
|
||||||
|
|||||||
@@ -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 {
|
||||||
|
|||||||
@@ -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";
|
||||||
|
|||||||
@@ -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,
|
||||||
}),
|
}),
|
||||||
|
|||||||
@@ -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) {
|
||||||
|
|||||||
@@ -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> {
|
||||||
|
|||||||
@@ -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. */
|
||||||
|
|||||||
177
packages/engine/src/__tests__/capacity-pool-id-check.test.ts
Normal file
177
packages/engine/src/__tests__/capacity-pool-id-check.test.ts
Normal 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([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|||||||
@@ -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));
|
||||||
|
|||||||
68
scripts/check-capacity-pool-id.mjs
Normal file
68
scripts/check-capacity-pool-id.mjs
Normal 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)`);
|
||||||
235
scripts/lib/capacity-pool-id-check.mjs
Normal file
235
scripts/lib/capacity-pool-id-check.mjs
Normal 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;
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user