fix(core): restore an archived card to the lane it came from (#2832)
## What Fixes the defect #2824 measured. That PR's characterization cases — merged and asserting the wrong-but-real behaviour — are flipped here to the correct lanes, which is what they were written to do. ## The bug ```ts // archive-lifecycle-2.ts const preArchiveColumn = task.preArchiveColumn ?? "todo"; ``` **`preArchiveColumn` has no database column.** It exists on the `Task` type and in the archive snapshot, and nowhere else — so the in-place restore cannot carry it, `store.getTask(id)` reads a live row that never had it, and the literal decided the destination for **every unarchive that has ever run**. | board | what happened | |---|---| | **default** | `todo` is declared, so the resolver returned it. Restores landed in the queue and **looked right**. | | **custom** | `todo` is declared nowhere, so the resolver took its "no usable history" branch and returned the **complete** lane. A card archived mid-implementation came back marked **finished**. | That coincidence is why this survived **three** separate fixes to `resolveUnarchiveTargetColumnImpl` — a `?? "done"` that invented a column, an `isColumn` legacy-enum gate, and the same gate one function over. Every one was correcting how the resolver interprets a value that never arrived. ## The fix is two halves, and either alone does nothing 1. **Capture** — `taskToArchiveEntryImpl` records `task.column` into the snapshot. That is the last place the original is still in hand, since the entry's own `column` is set to `"archived"` on the line above. 2. **Read** — `unarchiveTaskImpl` reads the **snapshot it already loaded**, not the restored row. I shipped half of this first and watched the destination stay wrong, which is how I found that the field has no row to live on. Mutation matrix: | state | result | |---|---| | both halves | **5/5 pass** | | capture only (read reverted) | **3 fail** | | read only (capture reverted) | **3 fail** | I also tried carrying it through `restoreTaskFromArchive`'s row update — that fails to typecheck, which is the proof that no such column exists and the snapshot is the only source. ## Behaviour changes, deliberately - **Custom boards** — a card returns to the lane it was archived from instead of appearing finished. - **Default board** — a card archived from `done` restored to `todo` under the literal and now restores to `done`. Returning finished work to the queue was the fallback showing through, not a rule anyone chose; the resolver's own branches say a card archived from a declared column goes back to it. ## One expectation of mine was wrong, and the resolver was right I expected a card archived from the review lane to return to **hold**. It returns to the review lane, and that is correct: `.review` is derived from the `mergeOrchestration` flag, **not** from `human-review`. The fixture's review column declares `human-review` + `merge-blocker` only, so it is not a `.review` lane to the resolver — just a declared column with usable history. The case now asserts that with the reasoning attached, so the next reader does not "fix" it back to hold. ## Verification - unarchive suite — **5/5**, mutation matrix above - `pnpm test:gate` — **exit 0** - full live-PG E2E surface — **164/164** - `pnpm lint` — clean Changeset included (`patch`, category `fix`). 🤖 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:
7
.changeset/persist-pre-archive-column.md
Normal file
7
.changeset/persist-pre-archive-column.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Restore an archived card to the lane it was archived from, not to Done.
|
||||
category: fix
|
||||
dev: `preArchiveColumn` was never captured — it has no `project.tasks` column and lives only in the archive snapshot — so `unarchiveTaskImpl` always fell to its `?? "todo"` literal. On a custom workflow `todo` is undeclared, so the destination resolver took its no-usable-history branch and returned the complete lane. Captures the column into the snapshot on archive and reads the snapshot (not the restored row) on unarchive; both halves are required.
|
||||
@@ -44,7 +44,31 @@ export async function taskToArchiveEntryImpl(store: TaskStore, task: Task, archi
|
||||
description: task.description,
|
||||
priority: normalizeTaskPriority(task.priority),
|
||||
column: "archived",
|
||||
preArchiveColumn: task.preArchiveColumn,
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-11:30 (PR #2824's finding, fixed):
|
||||
CAPTURE THE COLUMN THE CARD WAS IN. This field was only ever COPIED — here, back out of the
|
||||
entry on restore, and through serialization — and never SET from anywhere, so it was `undefined`
|
||||
for every archive that has ever happened. `unarchiveTaskImpl` then fell to its `?? "todo"` and
|
||||
the restore destination was decided by a literal instead of by history.
|
||||
|
||||
On the default board `todo` is a declared column, so restores landed in the queue and looked
|
||||
right — which is why this survived three separate fixes to `resolveUnarchiveTargetColumnImpl`,
|
||||
all of which were correcting how it interprets a value that never arrived. On a renamed board
|
||||
`todo` is declared nowhere, so the resolver took its "no usable history" branch and returned the
|
||||
COMPLETE lane: a card archived mid-implementation came back marked finished. Proven end to end
|
||||
in `workflow-unarchive-target-live-e2e.pg.test.ts`.
|
||||
|
||||
`task.column` is the pre-archive column at this point — the entry's own `column` is set to
|
||||
`"archived"` on the line above, so this is the last place the original is still in hand. The
|
||||
`??` keeps an already-captured value, so a re-archive of a restored card does not overwrite the
|
||||
history with an intermediate lane.
|
||||
|
||||
DEFAULT-BOARD BEHAVIOUR CHANGES, deliberately: a card archived from `done` restored to `todo`
|
||||
under the literal and now restores to `done`. Returning finished work to the queue was the
|
||||
fallback showing through, not a rule anyone chose — the resolver's own branches say a card
|
||||
archived from a declared column goes back to it.
|
||||
*/
|
||||
preArchiveColumn: task.preArchiveColumn ?? (task.column as ArchivedTaskEntry["preArchiveColumn"]),
|
||||
dependencies: task.dependencies,
|
||||
steps: task.steps,
|
||||
currentStep: task.currentStep,
|
||||
@@ -438,7 +462,25 @@ export async function unarchiveTaskImpl(store: TaskStore, id: string): Promise<T
|
||||
throw new Error(`Cannot unarchive ${id}: task is in '${task.column}', must be in 'archived'`);
|
||||
}
|
||||
|
||||
const preArchiveColumn = task.preArchiveColumn ?? "todo";
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-12:40 (PR #2824's finding, fixed — read the SNAPSHOT):
|
||||
THE HISTORY LIVES IN COLD STORAGE, NOT ON THE ROW. `preArchiveColumn` has no column in
|
||||
`project.tasks` — it exists on the `Task` type and in the archive entry, and nowhere else. So the
|
||||
in-place restore above cannot carry it, `store.getTask(id)` reads a live row that never had it,
|
||||
and `task.preArchiveColumn` was `undefined` for every unarchive that has ever run. The `?? "todo"`
|
||||
then decided the destination by literal instead of by history.
|
||||
|
||||
On the default board `todo` is declared, so restores landed in the queue and looked right — which
|
||||
is why this survived three separate fixes to `resolveUnarchiveTargetColumnImpl`, every one of them
|
||||
correcting how it interprets a value that never arrived. On a renamed board `todo` is declared
|
||||
nowhere, so the resolver took its "no usable history" branch and returned the COMPLETE lane: a
|
||||
card archived mid-implementation came back marked finished.
|
||||
|
||||
`entry` is the snapshot this function already loaded, and it is the only place the original column
|
||||
survives. Preferred over the row, which falls back to it, which falls back to the literal for a
|
||||
row so old it was archived before the column was captured at all.
|
||||
*/
|
||||
const preArchiveColumn = entry?.preArchiveColumn ?? task.preArchiveColumn ?? "todo";
|
||||
const toColumn = await store.resolveUnarchiveTargetColumn(preArchiveColumn, id);
|
||||
|
||||
/*
|
||||
|
||||
@@ -1,44 +1,38 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-05:50 (E2E evidence — restore lands every custom-board card in DONE):
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-01-13:10 (restore lands a card back on its OWN board — the fix):
|
||||
|
||||
The one lifecycle role with no live coverage, in the function with the worst track record in this
|
||||
program — and driving it end to end shows why the track record did not improve.
|
||||
`resolveUnarchiveTargetColumnImpl` decides where a restored card lands, and its comments record THREE
|
||||
defects fixed on those few lines: a `?? "done"` that invented an undeclared column, an `isColumn`
|
||||
legacy-enum gate that rejected every renamed id, and the same gate one function over that dropped a
|
||||
renamed board's stored history on read. All three were reasoned from source, none had a live test, and
|
||||
the path stayed broken — because the value they argue over never arrived.
|
||||
|
||||
`resolveUnarchiveTargetColumnImpl` decides where a restored card lands, and its own comments record
|
||||
THREE separate defects fixed on these few lines: a `?? "done"` that invented an undeclared column, an
|
||||
`isColumn` legacy-enum gate that rejected every renamed id, and the same gate one function over that
|
||||
dropped a renamed board's stored history on read. All three were reasoned from source. None had a
|
||||
live-store test.
|
||||
archive-lifecycle-2.ts const preArchiveColumn = task.preArchiveColumn ?? "todo";
|
||||
|
||||
WITH ONE, THE PATH IS STILL BROKEN, and the cause is upstream of everything those fixes touched:
|
||||
`preArchiveColumn` HAS NO DATABASE COLUMN. It exists on the `Task` type and in the archive snapshot,
|
||||
and nowhere else — so the in-place restore cannot carry it, `store.getTask(id)` reads a live row that
|
||||
never had it, and the literal decided every destination. On the default board `todo` is declared, so
|
||||
restores landed in the queue and looked right; on a renamed board `todo` is declared nowhere, the
|
||||
resolver took its "no usable history" branch, and a card archived mid-implementation came back marked
|
||||
FINISHED.
|
||||
|
||||
archive-lifecycle-2.ts:441 const preArchiveColumn = task.preArchiveColumn ?? "todo";
|
||||
TWO HALVES, and the first alone does nothing — established by shipping it first and watching the
|
||||
destination stay wrong:
|
||||
capture `taskToArchiveEntryImpl` records `task.column` into the snapshot, which is the last place
|
||||
the original column is still in hand (the entry's own `column` is set to `"archived"`).
|
||||
read `unarchiveTaskImpl` reads the SNAPSHOT it already loaded, not the restored row.
|
||||
|
||||
`preArchiveColumn` IS NEVER WRITTEN. Across `packages/core` every occurrence READS it or copies it
|
||||
through — into the archive entry, back out of it, through serialization — and nothing ever sets it
|
||||
from `task.column` when a card is archived. Measured on both boards:
|
||||
WHAT THE ROLES DECIDE, from the resolver's own branches — and the review case is subtler than it looks:
|
||||
archived from a WIP or REVIEW lane -> the board's HOLD lane; unfinished work returns to the queue
|
||||
archived from any other declared -> that same column; its history is usable as-is
|
||||
archived from `archived`, or from a -> the board's COMPLETE lane
|
||||
column the board no longer declares
|
||||
|
||||
default: after archive column=archived preArchiveColumn=undefined -> resolver target=todo
|
||||
renamed: after archive column=archived preArchiveColumn=undefined -> resolver target=shipped
|
||||
|
||||
So the fallback fires for every restore that has ever happened, and the two boards diverge because of
|
||||
what `"todo"` means to each:
|
||||
|
||||
DEFAULT BOARD `todo` is a declared column, so the resolver returns it. Every restore lands in the
|
||||
queue. That is right for a card archived mid-implementation and wrong for one
|
||||
archived from `done` — but it LOOKS right, which is why this survived.
|
||||
RENAMED BOARD `todo` is declared nowhere, so the resolver takes its "no usable history" branch and
|
||||
returns the COMPLETE lane. Archive a card mid-implementation, restore it, and it
|
||||
comes back marked FINISHED.
|
||||
|
||||
That is the operator-visible defect, and it explains the track record: the three earlier fixes were
|
||||
correcting how the resolver interprets a value that never arrives.
|
||||
|
||||
THE CASES BELOW ARE CHARACTERIZATION. They assert today's wrong-but-real behaviour so the defect is
|
||||
executable rather than argued, and they are written to flip: the fix is to persist the card's column
|
||||
when archiving, at which point every renamed expectation becomes the lane the card was actually in.
|
||||
See the PR body — that fix also changes DEFAULT-board placement for cards archived from `done`, which
|
||||
is why it is proposed rather than smuggled in here.
|
||||
`.review` is derived from the `mergeOrchestration` flag, NOT from `human-review` (a distinction that
|
||||
cost me a wrong expectation here). This fixture's review column declares `human-review` and
|
||||
`merge-blocker` only, so it is not a `.review` lane to the resolver — it is simply a declared column
|
||||
with usable history, and a card archived from it restores THERE. That is the correct answer, and the
|
||||
case below asserts it with the reasoning attached so the next reader does not "fix" it to hold.
|
||||
|
||||
LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is
|
||||
unaffected. Throwaway per-file database; never port 4040.
|
||||
@@ -91,15 +85,15 @@ pgDescribe("unarchive destination resolves the card's own board", () => {
|
||||
}
|
||||
|
||||
it("CONTROL — on the DEFAULT board a restore lands in the queue, and looks correct", async () => {
|
||||
/* Correct for this card by coincidence: the hardcoded `"todo"` fallback happens to be the default
|
||||
board's hold lane. That coincidence is what has hidden the missing write. */
|
||||
/* Unchanged by the fix, and that is the point: the literal happened to be this board's hold lane,
|
||||
which is why the defect was invisible here for so long. */
|
||||
const store = h.store();
|
||||
|
||||
expect(await archiveThenUnarchive(store, DEFAULT_VOCAB, "wf-default-wip", DEFAULT_VOCAB.wip))
|
||||
.toBe(DEFAULT_VOCAB.hold);
|
||||
});
|
||||
|
||||
it("CHARACTERIZATION — a RENAMED board's card archived from WIP comes back marked FINISHED", async () => {
|
||||
it("a RENAMED board's card archived from WIP returns to ITS queue", async () => {
|
||||
/*
|
||||
The card was mid-implementation, so it goes back to the queue — but the queue is `backlog`, not
|
||||
`todo`. A legacy-enum gate anywhere upstream sends it to the complete lane or to an id this board
|
||||
@@ -108,22 +102,23 @@ pgDescribe("unarchive destination resolves the card's own board", () => {
|
||||
*/
|
||||
const store = h.store();
|
||||
|
||||
/* Expected: `hold` — the queue it should return to. Actual: the COMPLETE lane. */
|
||||
expect(await archiveThenUnarchive(store, RENAMED_VOCAB, "wf-renamed-wip", RENAMED_VOCAB.wip))
|
||||
.toBe(RENAMED_VOCAB.complete);
|
||||
.toBe(RENAMED_VOCAB.hold);
|
||||
});
|
||||
|
||||
it("CHARACTERIZATION — one archived from REVIEW does too", async () => {
|
||||
it("one archived from a human-review lane returns to that lane", async () => {
|
||||
/* The review lane is grouped with WIP by the resolver — unfinished work returns to the queue.
|
||||
Asserted separately because the two ids are resolved from different traits, so a conversion
|
||||
could fix one role and leave the other. */
|
||||
const store = h.store();
|
||||
|
||||
/* Its OWN lane, not hold — see the header: this column declares `human-review` but no merge
|
||||
orchestration, so it is a declared column with usable history rather than a `.review` lane. */
|
||||
expect(await archiveThenUnarchive(store, RENAMED_VOCAB, "wf-renamed-review", RENAMED_VOCAB.review))
|
||||
.toBe(RENAMED_VOCAB.complete);
|
||||
.toBe(RENAMED_VOCAB.review);
|
||||
});
|
||||
|
||||
it("CHARACTERIZATION — and so does one archived from the HOLD lane it should return to", async () => {
|
||||
it("one archived from the HOLD lane returns there", async () => {
|
||||
/*
|
||||
The differential that stops the two cases above passing for a trivial reason. If the resolver
|
||||
simply returned the hold lane for everything, all three would be green — this one distinguishes
|
||||
@@ -133,10 +128,10 @@ pgDescribe("unarchive destination resolves the card's own board", () => {
|
||||
const store = h.store();
|
||||
|
||||
expect(await archiveThenUnarchive(store, RENAMED_VOCAB, "wf-renamed-hold", RENAMED_VOCAB.hold))
|
||||
.toBe(RENAMED_VOCAB.complete);
|
||||
.toBe(RENAMED_VOCAB.hold);
|
||||
});
|
||||
|
||||
it("a card archived from COMPLETE lands in the complete lane — the only one that is right, and only by accident", async () => {
|
||||
it("a card archived from COMPLETE stays finished", async () => {
|
||||
/* Finished work keeps its history too, and its id must be the board's own `shipped` — the case a
|
||||
`?? "done"` fallback would silently answer with the legacy id. */
|
||||
const store = h.store();
|
||||
|
||||
Reference in New Issue
Block a user