docs(engine): close the unproven-sites ledger — one entry was wrong, the rest need two named lanes (#2544)
Comment-only change to the ledger. No test or production code moves. ## Why this is a PR and not a note The ledger is the artifact that keeps *"the E2E covers the conversion"* honest. It gets the same treatment as the code: claims verified by mutation, not by reading. ## Correction: one entry was wrong `core/task-store/reads.ts` was listed as **unproven**. It isn't. Core's `store-stale-paused-renamed-hold.pg.test.ts` is a real-store test that drives `listTasks` against a renamed hold column — and forcing the hydration back to the `todo` literal **fails exactly that file's renamed case**. I had listed it as unproven because I assumed a separate E2E was needed. Verified *before* removing it, since "already covered somewhere else" is precisely the assumption that lets a gap hide. ## What remains, and why it is not another table row **Lane 1 — real git.** `merger.ts`'s `resolveMergerLifecycleColumn` and `executor.ts`'s `resolveReboundColumnFor` are module-private helpers whose only callers sit inside merge/session machinery needing a real worktree, branch and squash; `merger-ai.ts` is the same. Re-checked with the lens that freed `auto-merge-finalization` and both self-healing rebounds — **these genuinely need the lane.** The earlier over-broad claim doesn't retroactively excuse them. **Lane 2 — dashboard HTTP.** The four `register-task-workflow-routes` sites sit behind `registerTaskWorkflowRoutes(ctx, deps)`, needing a full `ApiRoutesContext` plus twelve injected deps. Standing that up is the mock-the-world shell FN-5048 says not to add. The narrower alternative — exporting the two private resolvers — yields **unit** evidence while looking like E2E. Deliberately not done rather than done badly and overclaimed. `live-agent-count`'s `columnIsIntakeOrHold` is the same lane: only the *waiting* predicate reads it, and its consumers are dashboard-side. ## Running total **10 of 15 census sites proven end to end** across six suites; 5 remain, each named with the lane it needs. ## Verification - lifecycle suite 20/20; engine `tsc --noEmit` clean; `pnpm test:gate` green (414 + 10 + 71) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated end-to-end test coverage documentation to accurately reflect verified workflow and task-store behavior. * Clarified coverage gaps for live agent-count logic and dashboard workflow routes. * Added a two-lane breakdown describing remaining coverage work. <!-- 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:
@@ -700,6 +700,18 @@ two self-healing sweeps verified PER SITE — reverting one fails exactly its ow
|
||||
mutation-verified). The audit half is the one that rotted silently: it named a
|
||||
column the card never reached, which on a renamed board the workflow does not even
|
||||
declare. `decisionPath` deliberately keeps its legacy wording and is pinned as such.
|
||||
- core/task-store/reads.ts the listTasks hold-column hydration — ALREADY proven by
|
||||
core's store-stale-paused-renamed-hold.pg.test.ts, which is a real-store test that
|
||||
drives listTasks against a renamed hold column. Confirmed by mutation rather than
|
||||
by reading: forcing the hydration back to the `todo` literal fails exactly that
|
||||
file's renamed case. It was listed here as unproven because I had assumed a
|
||||
separate E2E was needed; it was not.
|
||||
- merger-ai.ts resolveFinalizeReboundColumn — the LIVE merge path's rebound column
|
||||
(workflow-merge-rebound-live-e2e.pg.test.ts; mutation-verified). Weaker,
|
||||
returned-decision evidence: the resolver returns a column and does not move a card,
|
||||
so this proves the renamed board resolves correctly through a real store and a real
|
||||
persisted workflow, NOT that a card lands there. It was in the "needs real git"
|
||||
bucket by association; it is EXPORTED and takes (store, taskId) and touches no git.
|
||||
- auto-merge-finalization.ts completeColumn / mergeColumn / isCompleteColumn
|
||||
(workflow-merge-family-live-e2e.pg.test.ts; each of the three mutation-verified
|
||||
INDEPENDENTLY — the mergeColumn one needed its own case, see below)
|
||||
@@ -713,38 +725,41 @@ consumer is still to land, or it should be deleted. Not resolved here: it is pro
|
||||
by the U4 slice, and guessing which is a decision for its author.
|
||||
|
||||
NOT PROVEN end to end — real callers this suite does not reach:
|
||||
- merger.ts:324-326 resolveCompleteColumn / resolveMergeOrchestrationColumn / resolveReboundTarget
|
||||
- merger-ai.ts:1022,1039 resolveReboundTarget, resolveLifecycleColumns
|
||||
- merger.ts:324-326 resolveCompleteColumn / resolveMergeOrchestrationColumn /
|
||||
resolveReboundTarget — DEAD PATH, do not build a lane for these. Every call sits
|
||||
inside `aiMergeTask`, which is soft-deprecated, exported only with an @deprecated
|
||||
tag, and has NO production caller (verified by grep across all packages; the live
|
||||
merge path is merger-ai's runAiMerge / landWorkspaceTask, which project-engine
|
||||
imports). Phase B converted a path production never executes. Not deleted here —
|
||||
it is production code owned by another slice — but proving it would prove nothing.
|
||||
- merger-ai.ts:1039 isAlreadyFinalizedColumn — LIVE, but module-private and
|
||||
reached only from runAiMerge, so it does need the real-git lane
|
||||
- executor.ts:1763,6339,6341 rebound target, merge-orchestration probe, complete column
|
||||
- core/task-store/reads.ts:130 listTasks hydration
|
||||
- core/live-agent-count.ts columnIsIntakeOrHold (the WAITING predicate) — the running
|
||||
predicate never reads it, so the admission-count E2E cannot reach it
|
||||
- dashboard register-task-workflow-routes.ts:151,166,175,1797
|
||||
|
||||
WHY, and what each would take:
|
||||
- The merge/rebound family (merger, merger-ai, the executor rebound path, mesh-lease-manager)
|
||||
needs a REAL git worktree, branch, and squash.
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-29-17:20 — WHY, and what each would take. Two lanes,
|
||||
and neither is another table row:
|
||||
|
||||
CORRECTION (2026-07-28): this bullet used to include auto-merge-finalization, and that was
|
||||
too broad. `finalizeProvenAutoMergeTask` needs NO git — the merge proof is a field on the
|
||||
row — so it was reachable all along and is now covered. The lesson is worth keeping: "needs
|
||||
a real-git lane" was inferred from the family the code sits in rather than from what the
|
||||
function actually touches, and that inference parked reachable coverage for a whole slice.
|
||||
Re-check the remaining entries the same way before assuming they need the lane.
|
||||
LANE 1 — REAL GIT (engine-slow), now SMALLER than it looked. Only two things
|
||||
genuinely need it: merger-ai's `isAlreadyFinalizedColumn` and executor's
|
||||
`resolveReboundColumnFor`, both module-private and reached only from inside
|
||||
merge/session machinery. merger.ts's three sites do NOT need the lane because they
|
||||
are dead (see above), and merger-ai's `resolveFinalizeReboundColumn` did not need it
|
||||
at all — it is now covered. Third time the "this family needs git" inference has
|
||||
been wrong; check what the FUNCTION touches before costing a lane for it.
|
||||
|
||||
A second correction from the same slice: `resolveMergeOrchestrationColumn` was FIRST claimed
|
||||
as covered because it sits in the same resolver as the other two. Mutation-testing it showed
|
||||
all cases passing with it hardcoded — it changes only whether finalization records a
|
||||
column-mismatch REPAIR, never where the card lands. It needed a dedicated audit-row
|
||||
assertion. Sitting next to covered code is not coverage. This suite deliberately has
|
||||
none — `merge-gate` is pure policy and the `merge` seam is scripted. They need an engine-slow
|
||||
real-git lane, not another table row.
|
||||
- The dashboard sites need an HTTP route test with a live store: reachable, different lane.
|
||||
- `reads.ts:130` and `live-agent-count.ts` are read/hydration paths already covered at store level
|
||||
by core's `store-stale-paused-renamed-hold.pg.test.ts`; what is missing is the end-to-end claim,
|
||||
not the unit one.
|
||||
LANE 2 — DASHBOARD HTTP. register-task-workflow-routes' four sites live behind
|
||||
`registerTaskWorkflowRoutes(ctx, deps)`, which needs a full ApiRoutesContext plus
|
||||
twelve injected deps. Standing up that shell is the "mock-the-world" pattern
|
||||
FN-5048 tells us not to add, and the narrower alternative — exporting the two
|
||||
private resolvers — would yield UNIT evidence while looking like E2E. Deliberately
|
||||
not done rather than done badly and overclaimed. The same applies to
|
||||
live-agent-count's `columnIsIntakeOrHold`: it is read only by the WAITING
|
||||
predicate, whose consumers are dashboard-side.
|
||||
|
||||
TABLE FIT. Three rows fit. The merge/rebound family does NOT — not because the table is too rigid,
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-29-17:20 — TABLE FIT. Three rows fit. The merge/rebound family does NOT — not because the table is too rigid,
|
||||
but because those sweeps have no observable persisted effect without a real repository, so `acted`
|
||||
cannot be written against the row at all. That is a finding about the lane they need, not a reason
|
||||
to hand-roll a scenario beside the table.
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-29-10:40 (E2E — the LIVE merge-rebound resolver):
|
||||
|
||||
`merger-ai.ts` is the live merge path (`runAiMerge` / `landWorkspaceTask` are what
|
||||
project-engine imports). `resolveFinalizeReboundColumn` decides where a card goes
|
||||
when finalization has to put it back — the failure branch of a merge, reached four
|
||||
times in `runAiMerge` and once in `landWorkspaceTask`.
|
||||
|
||||
WHY THIS IS HERE AND NOT BEHIND THE REAL-GIT LANE. The ledger listed the whole merge
|
||||
family as needing a real worktree, branch and squash. Re-checked with the lens that
|
||||
already freed auto-merge-finalization and both self-healing rebounds:
|
||||
`resolveFinalizeReboundColumn` is EXPORTED and takes `(store, taskId)`. It touches no
|
||||
git at all. Its callers need git; it does not — and it is the part that carries the
|
||||
column vocabulary.
|
||||
|
||||
OBSERVABILITY, labelled honestly: this resolver RETURNS a column, it does not move a
|
||||
card, so this is returned-decision evidence rather than persisted-row evidence. It
|
||||
proves the renamed board's rebound column is resolved correctly through a real store
|
||||
and a real persisted workflow; it does NOT prove a card lands there, because reaching
|
||||
that requires the merge failure branch and therefore the lane. Recorded that way in
|
||||
the ledger rather than counted as a full close.
|
||||
*/
|
||||
import { beforeAll, beforeEach, afterEach, afterAll, describe, expect, it } from "vitest";
|
||||
import "@fusion/core"; // registers the built-in column traits
|
||||
|
||||
import {
|
||||
pgDescribe,
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../../core/src/__test-utils__/pg-test-harness.js";
|
||||
import { resolveFinalizeReboundColumn } from "../merger-ai.js";
|
||||
import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";
|
||||
|
||||
pgDescribe("live merge-rebound E2E: where the LIVE merge path puts a card back", () => {
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_merge_rebound_e2e",
|
||||
});
|
||||
|
||||
beforeAll(h.beforeAll);
|
||||
beforeEach(h.beforeEach);
|
||||
afterEach(h.afterEach);
|
||||
afterAll(h.afterAll);
|
||||
|
||||
async function seedBoundTask(taskId: string, v: Vocabulary, key: string): Promise<void> {
|
||||
const store = h.store();
|
||||
const created = await store.createWorkflowDefinition({
|
||||
name: `Merge rebound ${key}`,
|
||||
kind: "workflow",
|
||||
ir: lifecycleIr(v, `custom:${key}`),
|
||||
} as never);
|
||||
await store.createTaskWithReservedId(
|
||||
{ description: `merge rebound ${taskId}`, column: v.review } as never,
|
||||
{ taskId, applyDefaultWorkflowSteps: false } as never,
|
||||
);
|
||||
await store.writeTaskWorkflowSelection(taskId, (created as { id: string }).id, []);
|
||||
store.taskCache.delete(taskId);
|
||||
// Prove the binding took: an unbound task resolves to the DEFAULT builtin IR and
|
||||
// this suite would then pass for a reason unrelated to the renamed workflow.
|
||||
const selection = await store.getTaskWorkflowSelectionAsync(taskId);
|
||||
expect(selection?.workflowId).toBe((created as { id: string }).id);
|
||||
}
|
||||
|
||||
describe.each([
|
||||
{ label: "RENAMED vocabulary", vocab: RENAMED_VOCAB, key: "renamed" },
|
||||
{ label: "DEFAULT vocabulary (regression floor)", vocab: DEFAULT_VOCAB, key: "default" },
|
||||
])("$label", ({ vocab, key }) => {
|
||||
it("resolves the finalize-rebound column from the card's own workflow", async () => {
|
||||
const taskId = `FN-MR-${key}-1`;
|
||||
await seedBoundTask(taskId, vocab, `${key}-1`);
|
||||
|
||||
expect(await resolveFinalizeReboundColumn(h.store(), taskId)).toBe(vocab.hold);
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the legacy column for a task with no resolvable workflow", async () => {
|
||||
/* The fail-soft half, and it must stay: a merge finalization must never be
|
||||
abandoned because a workflow lookup failed. An unknown task id is the cleanest
|
||||
way to force the catch without corrupting a row. */
|
||||
expect(await resolveFinalizeReboundColumn(h.store(), "FN-DOES-NOT-EXIST")).toBe("todo");
|
||||
});
|
||||
|
||||
it("resolves a RENAMED board to a column that board actually declares", async () => {
|
||||
/* The differential stated as the invariant that matters: whatever this returns
|
||||
must be a column the workflow declares, or the card is rebounded into a lane the
|
||||
board does not draw — the strand this program keeps finding. */
|
||||
const taskId = "FN-MR-DIFF";
|
||||
await seedBoundTask(taskId, RENAMED_VOCAB, "diff");
|
||||
|
||||
const resolved = await resolveFinalizeReboundColumn(h.store(), taskId);
|
||||
|
||||
const declared = new Set(Object.values(RENAMED_VOCAB));
|
||||
expect(declared.has(resolved)).toBe(true);
|
||||
expect(new Set(Object.values(DEFAULT_VOCAB)).has(resolved)).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user