## Escalation — a conversion on main changed nothing
**#3051 ("scheduler.ts 12 → 2 lifecycle-column guards") is inert.** It
widened `resolveTaskParkedColumnsSync` from `{hold,intake}` to the full
role set and replaced ten handler literals with `parked.review` /
`parked.wip` / `parked.complete` / `parked.archived`. The census fell by
ten. The behaviour did not change, on any board.
Everything rests on one line in that helper:
```ts
const l = resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId));
```
`resolveTaskWorkflowIrSync` resolves through the sync workflow
**selection** reader, which answers `undefined` for every task under
PostgreSQL — the shipped backend. The resolver takes its `!workflowId`
branch and returns the **default builtin IR**.
Note the shape precisely, because the obvious reading is wrong and this
PR corrected itself on it mid-run: the helper does **not** get
`undefined` and fall through to `?? legacy.review`. It gets a **real IR
that resolves real traits** — the default board's. So `parked.review` is
`"in-review"` for every card on every board, the `?? legacy` arms are
dead code, and the helper answers with full confidence. It looks
resolved at every level except the one that decides the answer.
## Evidence (live PostgreSQL, 3/3 passing)
For a card bound to a **stored** renamed workflow and sitting in that
board's review column (`checking`, carrying `human-review` +
`merge-blocker` + `merge`):
| | sync path (what every converted arm uses) | async resolver | board
actually declares |
|---|---|---|---|
| review | `in-review` | `checking` | `checking` |
| wip | `in-progress` | — | `building` |
| complete | `done` | — | `shipped` |
The async arm is in the test on purpose: it attributes the failure to
the **sync path** and nothing else.
The **control** is the point of the whole thing — on the default board
the sync answer is *correct*, by coincidence rather than resolution.
That is why every default-board scheduler test passes either way, and
how ten inert conversions read as a fix.
## Why this is worse than leaving the literals
A conversion that changes nothing is worse than an unconverted literal,
because **the literal was counted and this is not.** Ten guards left the
backlog, the file now reads as converted, and the next reader has no
reason to look again.
Two further signals that this was not a deliberate trade-off:
1. The FNXC block still standing directly above the handler (unmodified
by #3051) **contradicts the code beneath it** — it says these ten arms
cannot be converted this way and names `resolveTaskParkedColumnsSync` as
the hazard-avoidance device, not the fix.
2. #3051's own added note asserts the fix as fact: *"so on a renamed
board PR monitoring never started or stopped, failure bookkeeping never
recorded, and terminal cleanup never ran."* Those failures are real.
This conversion does not fix them.
## Scope — driven vs argued, stated in the file
- **Driven:** the roles the sync path yields for a real card on a real
stored renamed board, against a real PostgreSQL store, versus the async
resolver on the same card.
- **Not driven, and the file says so rather than substituting a spy:**
the `parked.review` arm's own side effect. Its only outputs are four
dispatch-oscillation fields that do not round-trip through `updateTask`
on this store (measured: writing `dispatchStormCount: 3` reads back
`undefined`), so there is no persisted observable. The behavioural half
is carried by the sibling
`workflow-scheduler-parked-columns-live-e2e.pg.test.ts`, which drives
the **same helper** on the hold role through to persisted state.
## The real unblock
Unchanged from the note already in the file: carry the resolved lanes
**on the `task:moved` payload**, so no listener resolves at all. That
removes the class rather than one instance, and it is the only option
that survives the synchronous-prologue constraint — these listeners run
in the same tick as a synchronous emitter, which is why an `await`
cannot simply be added.
## Verification
`test:gate` exit 0 · live-PG E2E surface **174/174** · census exit 0 ·
`pnpm lint` clean. Test-only; no production file touched.
## Recommendation
Do not revert #3051 — the widened helper is harmless and the note it
added is useful once the resolution is real. **Restore the ten guards to
the census**, or land the payload change. Either way the count must not
read as paid.
Related: **#3055, #3050 and #3049 are all converting `self-healing.ts`
concurrently** — three PRs, one file, 51 guards. Worth de-conflicting
before any of them merges.
This commit is contained in:
@@ -0,0 +1,165 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-11:30 (E2E evidence — PR #3051's scheduler conversion is INERT):
|
||||
|
||||
ESCALATION, not a characterization. PR #3051 ("scheduler.ts 12 -> 2 lifecycle-column guards") converted
|
||||
the ten `task:moved` / `task:updated` handler arms by widening `resolveTaskParkedColumnsSync` from
|
||||
`{hold,intake}` to the full role set and replacing the literals with `parked.review` /
|
||||
`parked.complete` / `parked.archived` / `parked.wip`. The census fell by ten. The behaviour did not
|
||||
change at all, on any board.
|
||||
|
||||
The reason is one line inside that helper:
|
||||
|
||||
const l = resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId));
|
||||
|
||||
`resolveTaskWorkflowIrSync` resolves through the sync workflow SELECTION reader, which under
|
||||
PostgreSQL — the shipped backend — answers `undefined` for EVERY task. The resolver then takes its
|
||||
`!workflowId` branch and returns the DEFAULT builtin IR. Note the shape precisely, because the obvious
|
||||
reading is wrong and this file corrected itself on it: the helper does NOT get `undefined` back and
|
||||
fall through to `?? legacy.review`. It gets a REAL IR that resolves REAL traits — the default board's
|
||||
— so `l.review` is `"in-review"` for every card on every board and the `?? legacy` arms beside it are
|
||||
dead code that never runs. Each converted arm is byte-identical in outcome to the literal it replaced,
|
||||
and it looks resolved at every level except the one that decides the answer.
|
||||
|
||||
This is already established at the store level
|
||||
(`core/.../postgres/sync-workflow-ir-is-always-default.pg.test.ts`) and for the hold/intake half of
|
||||
this very helper (`workflow-scheduler-parked-columns-live-e2e.pg.test.ts`, PR #2789). What had no
|
||||
executable evidence is the claim #3051 actually makes — that the NEWLY added roles fixed something.
|
||||
Its own note states the fix as fact:
|
||||
|
||||
"so on a renamed board PR monitoring never started or stopped, failure bookkeeping never
|
||||
recorded, and terminal cleanup never ran"
|
||||
|
||||
Those failures are real. This file shows the conversion does not fix them. Note also that the older
|
||||
FNXC block still standing directly above the handler contradicts the code beneath it: it says the ten
|
||||
arms CANNOT be converted this way and names `resolveTaskParkedColumnsSync` as the hazard-avoidance
|
||||
device, not the fix.
|
||||
|
||||
WHY IT IS WORTH ITS OWN FILE. A conversion that changes nothing is worse than an unconverted literal,
|
||||
because the literal is COUNTED and this is not. Ten guards left the backlog, the file reads as
|
||||
converted at every level except running it, and the next reader has no reason to look again. That is
|
||||
the inert-conversion class this program exists to catch, and it has now landed on main.
|
||||
|
||||
WHAT IS DRIVEN, AND WHAT IS NOT — stated plainly rather than dressed up.
|
||||
|
||||
DRIVEN, against a real PostgreSQL store and a really persisted renamed workflow: the single line every
|
||||
converted arm rests on. For a card bound to a renamed board, `store.resolveTaskWorkflowIrSync(id)`
|
||||
returns `undefined`, and the lifecycle columns the helper derives from it are the DEFAULT board's
|
||||
ids — `in-review`, `in-progress`, `done` — none of which that board contains. Since each arm is
|
||||
exactly `to === parked.<role>`, an arm that compares against a column the board does not have cannot
|
||||
fire, and the pre-conversion literal compared against the same string.
|
||||
|
||||
NOT DRIVEN: the `parked.review` arm's own side effect. Its only effect is clearing four
|
||||
dispatch-oscillation fields, and those do not round-trip through `updateTask` on this store
|
||||
(measured: writing `dispatchStormCount: 3` reads back `undefined`), so there is no persisted
|
||||
observable to assert on. Rather than substitute a spy and call it end-to-end, the behavioural half is
|
||||
left to the sibling file `workflow-scheduler-parked-columns-live-e2e.pg.test.ts`, which drives the
|
||||
SAME helper on the hold role through to persisted state (a dependent that is never unblocked). The
|
||||
mechanism is one line shared by every role; this file proves that line is inert for the roles #3051
|
||||
added, and the sibling proves what an inert answer costs.
|
||||
|
||||
LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is
|
||||
unaffected. Throwaway per-file database; never port 4040.
|
||||
*/
|
||||
import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest";
|
||||
import "@fusion/core"; // registers the built-in column traits
|
||||
import { resolveWorkflowIrForTask, type TaskStore } from "@fusion/core";
|
||||
|
||||
import {
|
||||
pgDescribe,
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../../core/src/__test-utils__/pg-test-harness.js";
|
||||
|
||||
import { resolveLifecycleColumns } from "../../../core/src/workflow-lifecycle-traits.js";
|
||||
import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";
|
||||
|
||||
pgDescribe("scheduler sync-role conversion (#3051) against a live store", () => {
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_sched_syncrole",
|
||||
});
|
||||
|
||||
beforeAll(h.beforeAll);
|
||||
afterAll(h.afterAll);
|
||||
beforeEach(async () => { await h.beforeEach(); });
|
||||
afterEach(async () => { await h.afterEach(); });
|
||||
|
||||
/** A real persisted workflow and a card bound to it, resting in that board's REVIEW column. */
|
||||
async function cardOnBoard(store: TaskStore, v: Vocabulary, key: string): Promise<string> {
|
||||
const created = await store.createWorkflowDefinition({
|
||||
name: `SyncRole ${key}`,
|
||||
kind: "workflow",
|
||||
ir: lifecycleIr(v, `custom:${key}`, { mergeOrchestration: true }),
|
||||
} as never);
|
||||
const workflowId = (created as { id: string }).id;
|
||||
|
||||
const task = await store.createTask({ description: `sync-role probe ${key}` });
|
||||
await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
store.taskCache.delete(task.id);
|
||||
await store.moveTask(task.id, v.review as never, { recoveryRehome: true } as never);
|
||||
store.taskCache.delete(task.id);
|
||||
return task.id;
|
||||
}
|
||||
|
||||
/** Exactly what `resolveTaskParkedColumnsSync` computes, on the same sync path it uses. */
|
||||
function syncRoles(store: TaskStore, taskId: string) {
|
||||
return resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId));
|
||||
}
|
||||
|
||||
it("FIXTURE — the renamed board really is stored, and the card really is in its review column", async () => {
|
||||
/* First, because the refutation below is a claim about a board that must actually exist. If the
|
||||
selection had not persisted, every assertion under it would pass for the wrong reason. */
|
||||
const store = h.store();
|
||||
const taskId = await cardOnBoard(store, RENAMED_VOCAB, "wf-renamed-fixture");
|
||||
|
||||
expect((await store.getTask(taskId))?.column).toBe(RENAMED_VOCAB.review);
|
||||
expect(RENAMED_VOCAB.review).not.toBe("in-review");
|
||||
});
|
||||
|
||||
it("CONTROL — on the DEFAULT board the sync answer is right, which is why this hid", async () => {
|
||||
/* The helper's constant answer IS this board's vocabulary, so every converted arm compares
|
||||
against the correct id here — by coincidence, not by resolution. Every default-board test of
|
||||
the scheduler therefore passes either way, which is exactly how ten inert conversions read as
|
||||
a fix. */
|
||||
const store = h.store();
|
||||
const taskId = await cardOnBoard(store, DEFAULT_VOCAB, "wf-default-syncrole");
|
||||
|
||||
const roles = syncRoles(store, taskId);
|
||||
expect(roles?.review).toBe(DEFAULT_VOCAB.review);
|
||||
expect(roles?.wip).toBe(DEFAULT_VOCAB.wip);
|
||||
expect(roles?.complete).toBe(DEFAULT_VOCAB.complete);
|
||||
});
|
||||
|
||||
it("REFUTATION — on a RENAMED board the sync path still answers with the DEFAULT ids", async () => {
|
||||
/*
|
||||
The decisive fact. The card is bound to a stored workflow whose review lane is `checking`
|
||||
(carrying `human-review`, `merge-blocker` and `merge`), and it is sitting in that column. The
|
||||
sync selection read still answers `undefined`, so the roles resolve to the DEFAULT board.
|
||||
|
||||
Every arm #3051 converted is `to === parked.<role>`, so on this board each one compares the card
|
||||
against a column the board does not contain — precisely what the literal it replaced did. The
|
||||
conversion removed ten guards from the census and changed no behaviour.
|
||||
*/
|
||||
const store = h.store();
|
||||
const taskId = await cardOnBoard(store, RENAMED_VOCAB, "wf-renamed-syncrole");
|
||||
|
||||
/* The sync path DOES return an IR — that is the trap. It is simply not this card's IR, so the
|
||||
fail-soft `?? legacy` arms never engage and the helper answers with full confidence. */
|
||||
const syncIr = store.resolveTaskWorkflowIrSync(taskId);
|
||||
expect(syncIr).toBeDefined();
|
||||
|
||||
const roles = syncRoles(store, taskId);
|
||||
expect(roles?.review).toBe("in-review");
|
||||
expect(roles?.wip).toBe("in-progress");
|
||||
expect(roles?.complete).toBe("done");
|
||||
|
||||
/* ...none of which this board declares. */
|
||||
expect(roles?.review).not.toBe(RENAMED_VOCAB.review);
|
||||
expect(roles?.wip).not.toBe(RENAMED_VOCAB.wip);
|
||||
expect(roles?.complete).not.toBe(RENAMED_VOCAB.complete);
|
||||
|
||||
/* And the async resolver — the one an await-tolerant call site would use — gets it right, which
|
||||
attributes the failure to the SYNC path and nothing else. */
|
||||
const asyncIr = await resolveWorkflowIrForTask(store, taskId);
|
||||
expect(resolveLifecycleColumns(asyncIr)?.review).toBe(RENAMED_VOCAB.review);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user