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

fix(core): restore an archived card to the lane it came from (#2832)

Browse Source 
## 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:
gsxdsm
2026-07-30 13:54:53 -07:00
committed by GitHub
parent 9f61180dfc
commit 1824c04584
3 changed files with 90 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/persist-pre-archive-column.md Normal file
View 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.

46
packages/core/src/task-store/archive-lifecycle-2.ts
View File

@@ -44,7 +44,31 @@ export async function taskToArchiveEntryImpl(store: TaskStore, task: Task, archi
description: task.description, description: task.description,
priority: normalizeTaskPriority(task.priority), priority: normalizeTaskPriority(task.priority),
column: "archived", 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, dependencies: task.dependencies,
steps: task.steps, steps: task.steps,
currentStep: task.currentStep, 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'`); 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); const toColumn = await store.resolveUnarchiveTargetColumn(preArchiveColumn, id);
/* /*

83
packages/engine/src/__tests__/workflow-unarchive-target-live-e2e.pg.test.ts
View File

@@ -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 `resolveUnarchiveTargetColumnImpl` decides where a restored card lands, and its comments record THREE
program — and driving it end to end shows why the track record did not improve. 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 archive-lifecycle-2.ts const preArchiveColumn = task.preArchiveColumn ?? "todo";
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.
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 WHAT THE ROLES DECIDE, from the resolver's own branches — and the review case is subtler than it looks:
through — into the archive entry, back out of it, through serialization — and nothing ever sets it archived from a WIP or REVIEW lane -> the board's HOLD lane; unfinished work returns to the queue
from `task.column` when a card is archived. Measured on both boards: 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 `.review` is derived from the `mergeOrchestration` flag, NOT from `human-review` (a distinction that
renamed: after archive column=archived preArchiveColumn=undefined -> resolver target=shipped 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
So the fallback fires for every restore that has ever happened, and the two boards diverge because of with usable history, and a card archived from it restores THERE. That is the correct answer, and the
what `"todo"` means to each: case below asserts it with the reasoning attached so the next reader does not "fix" it to hold.
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.
LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is 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. 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 () => { 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 /* Unchanged by the fix, and that is the point: the literal happened to be this board's hold lane,
board's hold lane. That coincidence is what has hidden the missing write. */ which is why the defect was invisible here for so long. */
const store = h.store(); const store = h.store();
expect(await archiveThenUnarchive(store, DEFAULT_VOCAB, "wf-default-wip", DEFAULT_VOCAB.wip)) expect(await archiveThenUnarchive(store, DEFAULT_VOCAB, "wf-default-wip", DEFAULT_VOCAB.wip))
.toBe(DEFAULT_VOCAB.hold); .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 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 `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(); 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)) 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. /* 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 Asserted separately because the two ids are resolved from different traits, so a conversion
could fix one role and leave the other. */ could fix one role and leave the other. */
const store = h.store(); 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)) 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 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 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(); const store = h.store();
expect(await archiveThenUnarchive(store, RENAMED_VOCAB, "wf-renamed-hold", RENAMED_VOCAB.hold)) 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 /* 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. */ `?? "done"` fallback would silently answer with the legacy id. */
const store = h.store(); const store = h.store();