From 25fa5e7c44d566a007c581a26fcc64673c5e307e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 10:27:31 -0700 Subject: [PATCH] =?UTF-8?q?fix(core):=20the=20log-entry=20archive=20gate,?= =?UTF-8?q?=20converted=20=E2=80=94=20the=20parity=20objection=20is=20met,?= =?UTF-8?q?=20not=20bypassed=20(#3165)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I converted this in #3110, the parity gate failed, and I reverted it. **The gate was right** — and the reason was subtler than "one encoding moved". Knowing it is what makes this conversion possible. ## Why the first attempt failed My version hoisted the comparison onto a local: ```ts const pgRowColumn = String(pgRow.column ?? ""); const rowIsArchivedLane = archivedLanes ? archivedLanes.has(pgRowColumn) : pgRowColumn === "archived"; ``` That gate's TS scan keys on the **property** being named `column` — deliberately, because the receiver is variously `task`, `row`, `dep`, `t`. Losing the `.column` access dropped the TS count while SQL and raw held steady, which it reads as divergence. **Behaviourally identical, structurally invisible.** Same failure mode I hit from the other direction in #3163, where I collapsed a Drizzle fallback into a string array. ## The fix Keep `pgRow.column === "archived"` **verbatim** as the fallback; add the resolved path in front of it. No encoding's count moves, an unwired or degraded caller behaves exactly as before, and the gate is **satisfied rather than worked around** — the same additive shape as the six Drizzle LANE sites (#3160, #3162, #3163). ## What it fixes A LANE question: *"is this row in the board's archive lane, so logging is read-only?"* Against the literal, a card the operator filed away on a renamed board kept **accepting log writes** — new activity accruing on closed work. `deletedAt` covers the soft-delete half, which is why the gap is narrow and why it stayed invisible: the common path is soft-delete. ## The recorded omission is retired properly `log-entry-archived-lane-gate.test.ts` carried the renamed case as a **deliberate omission** with its reason. It is now the first case in the file, and the note explains why the earlier judgement changed rather than quietly disappearing — a deferral that vanishes without explanation is how the next reader loses the thread. ## Measured - **3/3** in that file (renamed case added); parity test **2/2**, inventories unmoved. - **MUTATION**: dropping the resolved branch fails the renamed case and leaves the legacy **control** and the live-lane **negative** green. - log-entry / archived / audit suites — **3 files / 9 tests pass**. - `tsc --noEmit -p packages/core` clean; census `--strict`, `check-sql-column-literals` clean. - `check-fnxc-future-dates` is red on `main` from `task-update.ts` (another lane's stamps), not from these files. ## Census **Unchanged** — the literal remains the fallback arm, by design. Co-authored-by: Claude Opus 5 (1M context) --- .../log-entry-archived-lane-gate.test.ts | 19 +++++++++++---- packages/core/src/task-store/audit-ops.ts | 24 ++++++++++++++++++- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/packages/core/src/__tests__/log-entry-archived-lane-gate.test.ts b/packages/core/src/__tests__/log-entry-archived-lane-gate.test.ts index 8d3166a85c..886537b179 100644 --- a/packages/core/src/__tests__/log-entry-archived-lane-gate.test.ts +++ b/packages/core/src/__tests__/log-entry-archived-lane-gate.test.ts @@ -8,7 +8,7 @@ deliberately does NOT assert the renamed-lane case. 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 +I CONVERTED IT, BACKED IT OUT, AND HAVE NOW CONVERTED IT PROPERLY. `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 @@ -67,11 +67,20 @@ function row(column: string) { 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. + FNXC:WorkflowResolvedColumns 2026-07-31-23:59: the renamed case is NO LONGER ABSENT — it is the + first case below. The omission recorded here said the gate "cannot be fixed in the TypeScript arm + alone". That was the right call on the evidence then and is now wrong: the conversion is ADDITIVE, + keeping `pgRow.column === "archived"` verbatim as the fallback, so no encoding's literal count moves + and the parity gate is satisfied rather than bypassed. */ + it("refuses a log write to a card in a RENAMED archive lane", async () => { + readTaskRowMock.mockResolvedValue(row("filed")); + + await expect(logEntryImpl(storeWith(RENAMED_IR), "KB-1", "did a thing")) + .rejects.toThrow(/archived — logging is read-only/); + }); + + /* CONTROL: the resolved set is legacy-seeded, so the built-in id must still refuse. */ it("refuses a log write to the legacy `archived` column", async () => { readTaskRowMock.mockResolvedValue(row("archived")); diff --git a/packages/core/src/task-store/audit-ops.ts b/packages/core/src/task-store/audit-ops.ts index 51918b77ef..8db6e14467 100644 --- a/packages/core/src/task-store/audit-ops.ts +++ b/packages/core/src/task-store/audit-ops.ts @@ -228,7 +228,29 @@ export async function logEntryImpl(store: TaskStore, id: string, action: string, 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) { + /* + FNXC:WorkflowResolvedColumns 2026-07-31-23:59 (converted — the parity gate's objection is met, + not worked around): + A LANE question: "is this row in the board's archive lane, so logging is read-only?" Against the + literal, a card the operator filed away on a renamed board kept ACCEPTING log writes — new + activity accruing on closed work. `deletedAt` covers the soft-delete half, which is why the gap + is narrow and why it stayed invisible: the common path is soft-delete. + + I converted this once before and REVERTED it, because the parity gate failed. The gate was + right, and the reason was subtler than "one encoding moved": my version hoisted the comparison + onto a local (`pgRowColumn === "archived"`), and that gate's TS scan keys on the PROPERTY being + named `column`. Dropping the `.column` access dropped the TS count while SQL and raw held, which + it reads as divergence. + + So the fallback keeps `pgRow.column === "archived"` VERBATIM. The resolved path is added in + front of it, no encoding's count moves, and an unwired or degraded caller behaves exactly as + before — the same additive shape as the six Drizzle LANE sites. + */ + const archivedLanes = await resolveArchivedLanes(store); + const rowIsArchivedLane = archivedLanes + ? archivedLanes.has(String(pgRow.column ?? "")) + : pgRow.column === "archived"; + if (rowIsArchivedLane || pgRow.deletedAt != null) { throw new Error(`Task ${id} is archived — logging is read-only`); } // PG jsonb columns arrive already-parsed; convert to the TaskLogEntry[] shape.