root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test(core): cover the untested log-entry archive gate; correct a deferral that named the wrong blocker (#3110)

Browse Source 
I converted `audit-ops.ts`'s archived gate, measured, and **backed it
out**. Both halves of that are the deliverable.

## The old deferral was stale on its own terms

It declined the conversion because *"the fix is the same one
`getLiveTaskColumn` needs"* and doing one of the pair would leave them
disagreeing.

But `getLiveTaskColumn` now **takes** a resolved `archivedColumns` set,
and both of its callers already pass `await resolveArchivedLanes(store)`
— including the sentinel path **twenty lines up in this same function**.
The pair it worried about was already half-converted, and this arm was
the half out of step. Converting it would have made them *agree*.

That is the third deferral I have found this session whose stated
blocker had dissolved. A deferral note records the blocker at the moment
it was written, and nothing re-checks it.

## The real blocker is one neither note named

`archived-column-gate-parity.test.ts` failed my conversion, and its
reasoning is correct and not obvious. This gate has **three encodings**:

1. TypeScript comparisons
2. Drizzle `eq`/`ne` predicates
3. raw SQL templates

Converting only the TypeScript arm makes them **diverge**: the gate
would call the row archived while the SQL side still returns it as live
— a log write rejected by its gate while its parent is listed as live.

Every builtin workflow names the column `archived`, so all three agree
*by accident* on every board we ship, and nothing except that parity
test can see the split.

Unblocking means converting all three together — the SQL sides need the
resolved id as a query-build value, including inside `for update`
transactions that receive no store today — or declaring `archived` a
non-renameable system column. That test lays out both options and owns
the inventory that has to move in the same commit. I am not doing it
here; it is a different change from a lane conversion.

## What ships

**The corrected note**, and **a test for a gate that had no coverage in
any form**.

The test asserts the legacy refusal and — the case that matters more —
that a **live lane is not refused**. A gate that refused everything
would satisfy a one-sided test and silently break every log write on the
board.

The renamed case is recorded as a **deliberate, explained omission**
rather than left as a silent hole, so the next reader knows it is a
decision.

## Measured

- 3 new cases pass; the parity gate passes.
- **MUTATION**, on the conversion before I reverted it: restoring the
literal failed the renamed case. The conversion *worked* — which is
exactly why the parity gate mattered. A working change can still be the
wrong change.
- The live-lane negative asserts **the gate did not fire**, not that the
call succeeded: past the gate the fast path performs a real Drizzle
write this fake layer cannot serve, so asserting success would drag a
database fixture into a test about a lane comparison, and asserting a
bare rejection would pass even if the gate *had* fired.
- `src/__tests__/{log-entry,archive,cold-storage,unarchive}*` — **7
files / 23 tests pass**.
- `tsc --noEmit -p packages/core` clean; census `--strict`,
`check-fnxc-future-dates` clean.

## Census

**No movement — nothing converted, deliberately.** The count stays where
it is because the gate is blocked, not because it is fine.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-31 04:46:58 -07:00
committed by GitHub
parent 15a664a8f5
commit 4afb32ef98
2 changed files with 121 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

98
packages/core/src/__tests__/log-entry-archived-lane-gate.test.ts Normal file
View File

@@ -0,0 +1,98 @@
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:20:
THE "ARCHIVED TASKS ARE LOG-READ-ONLY" GATE HAD NO TEST AT ALL — this adds one, and records why it
deliberately does NOT assert the renamed-lane case.
`logEntryImpl`'s fast path refuses a log write when the parent task is archived, asking with the
`archived` literal. On a renamed board a card the operator filed away keeps accepting log writes.
The gap is narrow rather than absent because `deletedAt` covers the soft-delete half and that is the
common path — which is also why it stayed invisible.
I CONVERTED IT, AND BACKED THE CONVERSION OUT. `archived-column-gate-parity.test.ts` caught it, and
its reasoning is correct and not obvious: this gate has THREE encodings — TypeScript comparisons,
Drizzle `eq`/`ne` predicates, and raw SQL templates — and converting only the TypeScript arm makes
them DIVERGE. TS would call the row archived while the SQL side still returns it as live: a log write
rejected by its gate while its parent is listed as live. Every builtin workflow names the column
`archived`, so all three agree by accident on every board we ship and nothing else can see the split.
So the renamed case stays uncovered ON PURPOSE, and the file says so rather than quietly omitting it.
Unblocking it means converting all three encodings together (the SQL sides need the resolved id as a
query-build value, including inside `for update` transactions that receive no store today), or
declaring `archived` a non-renameable system column — the choice that parity test lays out.
What IS asserted is the gate's existing behaviour, which had no coverage at all: it refuses the
legacy archived id, and — the case that matters more — it does NOT refuse a live lane. A gate that
refused everything would have satisfied a one-sided test and broken every log write on the board.
`readTaskRow` is mocked because the subject is the gate, not the row read.
*/
import { describe, expect, it, vi } from "vitest";
import type { TaskStore } from "../store.js";
const { readTaskRowMock } = vi.hoisted(() => ({ readTaskRowMock: vi.fn() }));
vi.mock("../task-store/async-persistence.js", async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
readTaskRow: readTaskRowMock,
}));
const { logEntryImpl } = await import("../task-store/audit-ops.js");
/** Archive lane is `filed`; the board declares no column called `archived`. */
const RENAMED_IR = {
version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [],
columns: [
{ id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
{ id: "filed", name: "Filed", traits: [{ trait: "archived" }] },
],
};
function storeWith(ir: unknown): TaskStore {
return {
withTaskLock: vi.fn(async (_id: string, fn: () => Promise<unknown>) => fn()),
listWorkflowDefinitions: vi.fn(async () => [{ ir }]),
asyncLayer: { db: {}, projectId: "p1" },
isWatching: false,
taskCache: new Map(),
emit: vi.fn(),
/* Reached only on the NON-archived path; enough for the write to complete. */
updateTaskLogFields: vi.fn(async () => undefined),
} as unknown as TaskStore;
}
function row(column: string) {
return { id: "KB-1", column, deletedAt: null, log: [] };
}
describe("the log-entry archive gate", () => {
/*
DELIBERATELY ABSENT: "refuses a log write to a card in a RENAMED archive lane". That is the
behaviour this gate SHOULD have and does not, and it cannot be fixed in the TypeScript arm alone —
see the header. Written down rather than left as a silent hole, so the next reader knows the
omission is a decision and not an oversight.
*/
it("refuses a log write to the legacy `archived` column", async () => {
readTaskRowMock.mockResolvedValue(row("archived"));
await expect(logEntryImpl(storeWith(RENAMED_IR), "KB-1", "did a thing"))
.rejects.toThrow(/archived — logging is read-only/);
});
/*
The paired negative, and the one that matters most: a gate that refused everything would satisfy
both cases above and silently break every log write on the board.
*/
it("does NOT refuse a log write to a live lane", async () => {
readTaskRowMock.mockResolvedValue(row("building"));
/*
Asserted as "the GATE did not fire", not as "the call succeeded". Past the gate the fast path
performs a real Drizzle write, which this fake layer cannot serve — so the call still rejects,
with an unrelated error. Asserting success would drag a database fixture into a test about a lane
comparison, and asserting a bare rejection would pass even if the gate HAD fired.
*/
const error = await logEntryImpl(storeWith(RENAMED_IR), "KB-1", "did a thing").catch((err: unknown) => err);
expect(String(error)).not.toMatch(/logging is read-only/);
});
});

23
packages/core/src/task-store/audit-ops.ts
View File

@@ -204,6 +204,29 @@ export async function logEntryImpl(store: TaskStore, id: string, action: string,
archived-lane set threaded into a low-level, project-scoped read — and doing it in one of the
two places would leave the pair disagreeing about what "archived" means. Recorded so the census
keeps pointing at it with the reason attached.
FNXC:WorkflowResolvedColumns 2026-07-31-23:25 (THE STATED BLOCKER IS STALE, AND THE REAL ONE IS
BIGGER — I converted this, measured, and backed it out):
The note above is out of date on its own terms: `getLiveTaskColumn` now TAKES a resolved
`archivedColumns` set and both callers already pass `await resolveArchivedLanes(store)` — the
sentinel path twenty lines up in this same function is one of them. So the pair it worries about
is already half-converted, and this arm is the half that is out of step.
The REAL blocker is one neither note named. `archived-column-gate-parity.test.ts` failed my
conversion and is right: this gate has THREE encodings — TypeScript comparisons, Drizzle
`eq`/`ne` predicates, and raw SQL templates — and converting only the TypeScript arm makes them
DIVERGE. This gate would call the row archived while the SQL side still returns it as live: a
log write rejected by its gate while its parent is listed as live. Every builtin workflow names
the column `archived`, so all three agree by accident on every board we ship and nothing else
can see the split.
Unblocking means converting all three encodings together — the SQL sides need the resolved id as
a query-build value, including inside `for update` transactions that receive no store today — or
declaring `archived` a non-renameable system column. That parity test lays out both options and
owns the inventory that has to move in the same commit.
Behaviour here is otherwise now covered by `log-entry-archived-lane-gate.test.ts`, which had no
test at all before and which records the renamed case as a deliberate, explained omission.
*/
if (pgRow.column === "archived" || pgRow.deletedAt != null) {
throw new Error(`Task ${id} is archived — logging is read-only`);