From bebbdf90835c2cb2ac10beeaf0604d92d51aa7e8 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 14:53:32 -0700 Subject: [PATCH] =?UTF-8?q?fix(tests):=20main=20red=20=E2=80=94=20archive?= =?UTF-8?q?=20restore=20returns=20to=20the=20ARCHIVED=20lane=20now=20(#283?= =?UTF-8?q?2)=20(#2847)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Red on main ``` store-archive-reads > TaskStore archived read parity (PostgreSQL) > rebuilds a missing live row before consuming its archive snapshot AssertionError: expected 'done' to be 'todo' ``` ## The product change is a real fix **#2832** found that `preArchiveColumn` has no database column — it exists on the `Task` type and in the archive snapshot and nowhere else — so the old code fell through to a literal and decided the destination the same way for **every unarchive that has ever run**. On a custom board that meant a card archived mid-implementation came back marked *finished*. That PR flipped its own characterization cases. This one, in a different file, was missed. The fixture creates the card in `done`, so `done` is the answer now. ## I measured a second sample before encoding a rule The behaviour is **narrower than "restores to the lane it came from"**. With the fixture changed to `in-progress`, restore returns **`todo`** — not `in-progress`: | archived from | restored to | |---|---| | `done` | `done` | | `in-progress` | **`todo`** | A terminal lane is preserved; a WIP lane is re-queued. That is plausible product behaviour — a card cannot resume mid-execution after a restore — but it is **not what #2832's summary describes**, so I have flagged it there rather than encoding it here. If re-queueing WIP is deliberate it deserves its own case; if it is not, this snapshot-rebuild path still carries the defect #2832 fixed elsewhere. ## An honest limitation, recorded in the test This assertion is **weaker than it looks and cannot be strengthened here**: `done` is also the complete lane, which is exactly what the pre-#2832 *"no usable history"* branch returned. A card archived from `done` therefore reads identically under both the fixed and the broken implementation. My first attempt "strengthened" it by moving the fixture to `in-progress` — that is what surfaced the second behaviour above, and shipping it would have encoded a rule inferred from two samples. Reverted; the limitation is documented instead. ## Scope Only the line-205 case is touched. Line 218 asserts `todo` for a card genuinely archived from `todo` and still passes — the two are not the same claim. Core **4813 passed / 0 failed** · gate **732 green** · lint clean. Test-only. ## Pre-flight results for the current queue Merged-with-main, engine + core on each: | PR | result | |---|---| | #2822, #2819, #2823, #2818 | only the 2 inherited `workflow-ir-resolver` failures (fixed in #2836, now merged) | | #2805 | clean | | #2828 | inherited only, once this and #2836 land | | #2830 | inherited only | | #2820, #2803, #2808 | **conflict** with main — census baseline; told the owners to regenerate rather than hand-merge | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .../postgres/store-archive-reads.pg.test.ts | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts b/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts index 071ffc1649..cb2827ba8d 100644 --- a/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts +++ b/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts @@ -202,7 +202,29 @@ pgDescribe("TaskStore archived read parity (PostgreSQL)", () => { expect(persistSpy).toHaveBeenCalledOnce(); expect(restored.id).toBe(task.id); expect(restored.description).toBe("restore from snapshot only"); - expect(restored.column).toBe("todo"); + /* + FNXC:ArchiveRestore 2026-07-31-09:25: + `done`, because #2832 made restore return a card to the lane it was ARCHIVED FROM. + + This asserted `todo`, which was the old behaviour: `preArchiveColumn` has no database column, so + the pre-#2832 code fell through to a literal and decided the destination the same way for every + restore. The fixture above creates this card in `done`, so `done` is now the answer. + + MEASURED, NOT ASSUMED — and the result is narrower than "the lane it came from". Changing the + fixture to `in-progress` and re-running returns **`todo`**, not `in-progress`. So a terminal lane + is preserved while a WIP lane is re-queued, which is plausible product behaviour (a card cannot + resume mid-execution after a restore) but is NOT what #2832's summary describes. + + Left asserting `done` rather than encoding a rule I inferred from two samples. The `in-progress` + observation is flagged on #2832 for its owner: if re-queueing WIP is deliberate it deserves its + own case, and if it is not, this snapshot-rebuild path still carries the defect #2832 fixed + elsewhere. + + Note this assertion is weaker than it looks and cannot be strengthened here: `done` is also the + complete lane, which is what the pre-#2832 "no usable history" branch returned, so a card + archived from `done` reads the same under both implementations. + */ + expect(restored.column).toBe("done"); expect(await findArchivedTaskEntry(h.layer().db, task.id, h.layer().projectId)).toBeUndefined(); });