fix(core): the archived-document guards failed in OPPOSITE directions on a renamed lane (#2886)

Two of the four convertible sites my own learnings doc **miscounted as
sentinels** — the #2877 review corrected "8 of 9 must not be converted"
to "5 of 9", and these are two of the three that correction freed.

They read `task.column` straight off a row `select`, so they are board
lanes by exactly the test that document gives, and a renamed archived
column is simply not seen. What makes the pair worth fixing together is
that they fail in **opposite directions**:

| guard | on a renamed archived lane | consequence |
|---|---|---|
| `upsertTaskDocument` | fails to **reject** | an archived card's
documents stay **writable** — the read-only contract silently does not
hold |
| `publishArchivedTaskDocumentAddition` | fails to **accept** | a
legitimate archived-document publication is refused as
`parent-not-archived` |

The second is the sharper one: valid operator work refused, and refused
with a message that reads as a data-integrity error rather than a
lifecycle mismatch.

## Shape

Both take an `AsyncDataLayer` and can resolve nothing themselves; their
store-level impls hold the store, so the lane set arrives as a parameter
resolved once per call — the shape #2875 used for the SQL predicate.

**One shared `resolveArchivedLanes` for both paths**, deliberately: if
the write guard and the publication guard could disagree about whether a
card is archived, a card ends up both read-only *and* un-publishable.

## The revert proof caught my own fixture first

My first version set `deletedAt` alongside the renamed column, and **the
revert proof passed with the fix removed**. Both guards are
`column-is-archived || deletedAt != null`, so a soft-deleted fixture
short-circuits the exact comparison under test — the assertion was
holding for an unrelated reason.

Dropping `deletedAt` isolates it, and is also the *real* shape: a live
row in a workflow-declared archived lane is what a renamed board
produces, and what `getLiveTaskColumn` was written to catch.

Revert proof, measured honestly the second time: restore `task.column
=== "archived"` and the renamed-lane case fails — the upsert resolves
instead of rejecting.

## Real PostgreSQL, deliberately

These are row predicates inside a transaction. A mocked store would
assert the arguments and prove nothing about the comparison that runs —
the same reasoning as #2875.

Three cases: the renamed lane rejects, the **legacy** `archived` id
still rejects (most boards never rename anything), and a live card is
still allowed through (a guard that rejects everything is its own bug).

## Verification

- `pnpm test:gate` — 161 / 487 / 13 / 71 passed
- `pnpm lint` — clean
- `tsc --noEmit` (`@fusion/core`) — clean
- new `archived-document-lanes.pg.test.ts` + existing
`artifacts-documents-evals.pg.test.ts` — 12 passed against real
PostgreSQL

Note: the SQL-literal baseline is untouched here — #2881 owns
re-recording it after #2864's conversion left main's gate red.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-30 17:40:30 -07:00
committed by GitHub
parent 63e1f81244
commit c143327d4b
5 changed files with 212 additions and 6 deletions

View File

@@ -3,8 +3,8 @@
"byFile": {
"packages/engine/src/self-healing.ts": 87,
"packages/engine/src/scheduler.ts": 12,
"packages/core/src/task-store/async-comments-attachments.ts": 9,
"packages/engine/src/executor.ts": 8,
"packages/core/src/task-store/async-comments-attachments.ts": 6,
"packages/engine/src/notification/notification-service.ts": 5,
"packages/engine/src/replan-target.ts": 4,
"packages/engine/src/restart-recovery-coordinator.ts": 4,