mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
xarmian
d8b68c443e
* fix(web): clear attachment NodeView missing-latch on parent restore (BUG-2509)
Archiving an item 404s its attachments without deleting them (DR-13). Any
attachment surface that PROBES inside that window therefore observes exactly
what a deletion produces, latches it as permanent, and stays dead after the
restore. The strip and the timeline already reconcile this themselves via the
`parentArchived` prop plus their own epoch + no-store re-probe; the editor
NodeViews could not — their latch is closure-private state inside a Tiptap
view, unreachable from a prop — so they were routed out of PLAN-2392 3c-iii's
scope and never got the equivalent.
Two distinct defects, both reproduced live before any code was written:
1. The shared HEAD metadata cache memoizes `missing` for the page lifetime, on
the premise that a settled result is a durable fact about a content-addressed
row. True for deletion, false for archive: a `missing` observed in the
archived window is a fact WITH an expiry cached as though it had none. Every
later reader replays a 404 the server would no longer give — INCLUDING a
NodeView constructed fresh, which is why remounting the editor did not heal
the file chip.
2. The NodeView `deleted` latch is cleared only by a uuid swap, and a restore
does not change the uuid.
Which one bites depends on `canEdit`, which is forced false while archived: an
edit-permissioned user's restore flips the content branch and builds a fresh
editor (healing the image, whose load event repaints it, but not the chip, which
makes no request and reads the poisoned cache), while a viewer keeps the SAME
editor across the whole flip and sees both stay dead.
The fix is a restore channel on the attachment bus — deliberately NOT the mirror
of the deletion channel. Deletion is authoritative and subscribers latch it;
this signal carries NO VERDICT and only prompts a re-ask. Subscribers re-probe
no-store and clear the latch ONLY on an authoritative `ok`, so an attachment
genuinely deleted while its parent was archived 404s and stays dead, and a
mis-routed signal costs one HEAD and changes nothing. That is what keeps restore
from becoming an undo-resurrection vector (DR-17).
`announceAttachmentParentRestored` does both halves because they cover different
populations: the notify reaches surfaces already mounted and latched (a cache
invalidation cannot — their latch is not a cache read they repeat), and the
invalidation covers surfaces built later, which a notify cannot reach because
they did not exist when it fired. Invalidate first, so a subscriber re-probing
synchronously inside the notify is not answered from the entry being dropped.
Verified in a real browser against an isolated instance, on both legs (owner and
viewer, item opened while archived then restored with no reload), plus the DR-17
leg (attachment row genuinely deleted during the archived window — stays dead)
and the control (opened live, then archive/restore — never latches, since the
latch requires the NodeView to be CONSTRUCTED inside the archived window).
Tests: the three bus mocks now fan the restore channel out to subscribers rather
than only recording the subscription — a spy-only mock leaves the NodeView
subscribed to nothing and every reaction test passes vacuously. New coverage for
the cache invalidation, the channel (including the invalidate-before-notify
ordering, asserted through its observable consequence), and both NodeViews' heal
/ stay-dead / routing / teardown behaviour. Each new assertion was mutation-
tested: no-op'ing the invalidation fails 3, short-circuiting either listener
fails the heal tests.
* fix(web): fence the restore probe against deletion, workspace and item-switch (BUG-2509)
Three defects an independent review pass found in the previous commit. The
first is the safety property that commit claimed to protect.
1. DR-17 RESURRECTION, reachable in one interleaving. The deletion bus sets the
latch synchronously, but the restore continuation fenced only on teardown and
a uuid swap — so "restore probe starts → delete is confirmed and broadcast →
probe resolves ok" cleared the latch and repainted a row the server no longer
has. The browser leg missed it because there the delete preceded the signal,
so the probe itself 404'd; the hole is only in the overlap. Fixed with a
monotonic deletion generation captured before the probe and re-checked after —
a bare `deleted` re-check would not do, since the latch can be set and cleared
again while one probe is in flight.
Relatedly, a non-`ok` result was treated as "do nothing" when `missing` is in
fact the authoritative existence answer the probe asked for. It now latches,
which also settles two concurrent probes resolving out of order rather than
letting whichever answered first win.
2. The continuations never re-read the address after the await, so an answer
about ws-A's copy could heal a node whose editor had since moved to ws-B —
the same cross-workspace staleness that made these files read the workspace
off the live reader in the first place. Re-checked on landing, as
ItemTimeline's probe already does.
3. The ItemDetail edge was keyed on the LEVEL (`itemMatchesRef && isArchived`),
which also drops when you navigate AWAY from an archived item — so every such
navigation announced a restore that never happened, invalidating a workspace's
metadata cache and prompting probes. The latch now carries the item id: a
restore is "the same loaded, matched item went archived → live", nothing else.
Extracted as `parentRestoreEdge` and unit tested, because ItemDetail resists
jsdom mounting and that case is otherwise unpinnable.
Also: the two teardown tests passed vacuously — a leaked listener still sees
`destroyed` and returns, so they stayed green with the dispose call deleted.
They now assert the registry is empty. Mutation testing then caught the same
class in two of my own new tests: the image's mid-probe-deletion and
workspace-change cases asserted only that the placeholder was still visible,
which is true either way until a `load` fires. They now assert the observable
consequence of a wrong heal — the cache-busted reload it would arm.
Every new fence was mutation-tested individually (5 mutants, each failing only
its own tests). Full suite 1496 passed; svelte-check 0 errors; all three browser
legs re-verified against a binary built from this tree.
* fix(web): generation-fence every probe; key the restore signal by item (BUG-2509)
Round two of independent review. Three more findings, all confirmed against the
source; the third means the previous fix was incomplete for a realistic
navigation, not merely unfenced.
1. A STALE PROBE COULD STILL REACH THE LATCH PATH. The previous fence was
per-CAUSE (did a deletion land?) rather than per-TRANSITION, so it only caught
one direction. The other: an OLDER probe answering after a NEWER transition —
the image's archived-window probe still in flight when the restore heals, then
re-latching what the restore just fixed; and the chip's CONSTRUCTION probe
(issued inside the archived window, which is the whole bug) landing after the
restore signal and marking a live chip dead with nothing left to undo it.
Replaced with one monotonic per-NodeView generation, bumped on every
authoritative transition — deletion, missing-latch, heal, uuid swap, and
receipt of a restore signal — captured by every continuation that mutates
presentation. The invariant is now structural rather than a list of cases:
a continuation may only act if nothing authoritative happened since it
started. Bumping on receipt is what invalidates a probe issued before the
restore, including the early-return path where there is nothing to heal.
2. UUID FENCING WAS BY VALUE, so a swap away and back passed the check again —
and the deletion listener filters on the CURRENT uuid, so a delete arriving
while the node pointed elsewhere was ignored. Stale ok + same uuid on return =
resurrection. The swap now bumps the generation, which is what the
continuations compare.
3. THE EMITTER MISSED RESTORES THAT HAPPEN WHILE THE PANE IS AWAY. Archive A,
navigate to B, let someone else restore A, come back: no archived→live edge is
ever observed by this tab, so nothing was announced and a fresh chip read the
archived window's cached 404 — the original bug by another route. A per-mount
edge latch cannot see this, and keying it to the item id does not help; the
memory has to outlive the mount. Replaced `parentRestoreEdge` with
`archivedItemRegistry`: mark an item when seen archived, announce when it is
next seen live. That subsumes every case the latch handled (restore in place,
navigate away, mount-on-archived) and covers the one it structurally could not.
Verified live, same document throughout (asserted, not assumed — the leg stamps
the document and checks the stamp survives, since a full reload would drop the
very state under test). Control: the same leg against a binary built from the
PREVIOUS commit's emitter leaves the chip dead on return while the image heals,
which is exactly the predicted symptom — so the leg discriminates rather than
passing for free.
Tests: registry unit tests including both the false positive and the miss; the
stale-probe orderings for both NodeViews; the uuid away-and-back case; and the
restore probes' `cache: 'no-store'` (the mocks were discarding the options
argument, so the point of the re-probe was untested). Ten mutants total across
both rounds, each failing only its own tests — one (the uuid-swap bump) survived
first time and got the test it was missing.
* fix(web): make the latch fence structural; correct three stale comments (BUG-2509)
Round three of review. The generation fence I added covered the two probes I had
been looking at and missed three others that also latch: both toolbar MIME probes
and the activation probe. Same defect as the one already fixed — a probe issued
inside the archived window answering 404 after the restore healed the node — at
call sites I had not enumerated.
Rather than patch three more sites, the fence moved INTO `latchMissing`, whose
signature now REQUIRES the captured generation. Every path into it is an async
probe answering a question it asked earlier, and a latch is destructive and
permanent, so "the caller will remember" was the wrong shape: an unfenced call
site is now a type error rather than a bug found in review. Fixing this class one
site at a time is what produced the miss.
Three comments were left saying things that are no longer true, which in this
file is not cosmetic — the comments are how the next reader learns the rules:
- the PLAN-2411 seam note still said the restore channel was "stated, not built
here" and that only that future channel could clear the latch. This branch is
that channel. Rewritten to state what actually holds now (the signal never
clears anything; only a server `ok` does).
- my own generation docstring claimed EVERY mutating continuation captures it,
which the transforms and the activation-open path do not. Restated to say
what it governs (the latch and the heal) and what it deliberately does not
(opening a viewer, which `activationSeq` owns; rotate/crop, which mint a new
attachment).
- the metadata cache's "no staleness concern" predates the distinction this bug
turns on. Split: a settled `ok` cannot go stale, a settled `missing` can,
because it describes reachability rather than contents.
Two review findings are DECLINED, recorded in the code where the next reader will
ask. Re-checking `address().itemId` after the await: itemId is routing, not
ownership, and the probe's answer is about the ATTACHMENT — fencing on it would
imply a relationship the code does not have. Eviction for the archived-item
registry: it holds one uuid per archived item viewed, and dropping a mark early
reintroduces this bug silently for long sessions only, which is the worse trade.
The new toolbar-probe test passed vacuously at first (`selectAll` does not build
the toolbar, so the probe under test never ran and the stale-release handle was a
no-op default). It now drives a real NodeSelection and asserts the probe fired.
Twelve mutants across the three rounds; two survived first time and each got the
test it was missing.
Pad Web UI
SvelteKit 2 + Svelte 5 frontend for Pad, compiled to static files and embedded into the Go binary.
Development
npm install
npm run dev # Dev server at localhost:5173 (proxies API to localhost:7777)
npm run build # Production build to build/
npm run check # Type checking with svelte-check
When developing, run the Go backend separately with make dev from the project root.
Building for Production
Do not build in isolation. Always use make build from the project root — this builds the web frontend, then compiles the Go binary with the build output embedded via //go:embed.
Stack
- Svelte 5 with runes (
$state,$derived,$effect) - SvelteKit 2 with
adapter-static(SPA mode) - Tiptap block editor with markdown round-trip
- svelte-dnd-action for drag-and-drop in board/list views
- SSE for real-time updates
- TypeScript throughout
Structure
src/
routes/ SvelteKit pages
+layout.svelte App shell (sidebar + main)
+page.svelte Landing/redirect
[workspace]/
+page.svelte Dashboard (collections, phases, activity)
+layout.svelte SSE connection per workspace
[collection]/
+page.svelte Collection view (board/list)
[collection]/[item]/
+page.svelte Item detail + editor
conventions/ Purpose-built conventions page
playbooks/ Purpose-built playbooks page
settings/ Workspace settings
lib/
api/client.ts HTTP API client
components/
layout/ Sidebar, navigation
editor/ Tiptap editor, raw markdown editor
fields/ FieldEditor, relation picker
items/ ItemCard, ItemDetail
collections/ BoardView, ListView
common/ StatusBadge, badges, modals
search/ CommandPalette
activity/ ActivityFeed
stores/ Svelte 5 reactive stores
workspace.svelte.ts Workspace state
collections.svelte.ts Collection + item state
ui.svelte.ts Sidebar, mobile state
types/index.ts TypeScript types and constants
app.css Global styles and design tokens