mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
xarmian
2322fb273f
* fix(web): give the IDB write path the seq guard RAM already had (BUG-2609)
`upsert` and `applyRetag` hand persistUpserts a snapshot taken from RAM and do
not await it. An SSE delta for the same row can commit its own atomic
rows+cursor transaction in between, after which the older snapshot lands LAST:
IndexedDB then holds a pre-delta row while the persisted cursor sits past that
delta. Warm boot hydrates the stale row and `/items-changes?since=cursor` never
returns it again, so it stays stale until the item happens to change. RAM is
unaffected — single-threaded and already seq-guarded by mergeRow — which is why
this only ever showed up as a warm-boot regression.
The fix is the guard localIndex has had in RAM all along, applied at the layer
that lacked it: a read-modify-write inside the transaction, comparing against
what is STORED at write time rather than what was in RAM at snapshot time. Both
writers get it, because the race runs in both directions — a delta must not
overwrite a row that is already newer in the cache either.
Three boundary decisions, each with a reason rather than a default:
- EQUAL seq still writes. RAM merges same-seq projections before persisting,
so the incoming row IS the merged one; refusing it would drop that merge
and leave the cache behind RAM with no seq difference left to correct it.
- A MISSING seq on either side writes. Absence is not evidence of being
older, and refusing would silently disable the cache for any row the server
has not stamped.
- seq 0 is a value, not a blank. A falsy check here would let a stale row
through, so the comparison tests for undefined explicitly.
Worth stating because the outcome looks lossy and is not: `applyRetag` rewrites
collection_slug WITHOUT bumping seq, so a delta at a higher seq now SKIPS the
retag write. That leaves IDB agreeing with the persisted cursor while its slug
lags RAM — which self-heals through the sync-pass reconcile (BUG-2601). What it
replaces does not self-heal: a row behind the cursor is invisible to delta sync
by construction.
VERIFIED IN A REAL BROWSER, because the failure mode of getting this wrong is
silent. Awaiting inside an idb transaction risks the transaction auto-closing
mid-loop, after which the remaining puts throw into a swallowed catch and the
cache quietly stops being written — worse than the bug. In-repo precedent said
it was safe (hydrate already awaits a get and keeps using the same tx), and a
live run confirmed it: 2625 rows persisted, all seq-stamped, cursor advanced,
zero page errors. The instrument was checked against a control build whose
guard refuses every write — 0 rows, cursor still advanced, which is the
cursor-ahead-of-rows divergence this bug is about, so the check demonstrably
reads the thing it claims to.
The decision itself is covered by unit tests, each mutation-verified against
the specific assertion written for it (always-allow, strict-greater-than, and a
falsy seq check each fail only their own cases).
* fix(web): route retags through a field-level write, and stop overclaiming the guard (BUG-2609)
Codex round 1. The P2 is a correction to my own commit message, and the more
important of the two findings.
I wrote that a retag write skipped by the seq guard "self-heals through the
sync-pass reconcile (BUG-2601)". It does not, and the code says so where I
should have read it: `applyRetag` does not bump seq, a collection rename
touches no items so no item delta ever re-stamps them, and localIndex's own
comment states pendingRetags is "not persisted (the window it guards is within
a single session)". A persisted slug that loses its rename stays wrong across
reloads with nothing left to correct it — so my guard would have turned a
last-write-wins race into a permanent staleness.
That is the second time today I stated a mechanism I had not read, and this
one made it into a commit message as the justification for shipping.
Fixed by design rather than by rewording. A retag is a FIELD-LEVEL intent:
"these rows are in a collection that got renamed". Expressing it as a whole-row
put makes a second claim — that every other field still matches a RAM snapshot
— and it is that claim the guard has to refuse. persistRetag reads each row
inside the transaction and changes only collection_slug, so the newer row's
fields survive AND the rename lands. Rows absent from the cache are skipped:
nothing to rename, and inserting a snapshot there would resurrect rows a delta
may have removed.
Codex's P1 — a delayed snapshot can resurrect a HARD-deleted row, because a
deleted row leaves no `existing` for the guard to compare against — is real,
pre-existing (a blind put resurrected it too), and not fixed here. Refusing it
needs a tombstone carrying the seq it was removed at, i.e. an IDB schema change
plus a version bump. Filed as BUG-2633.
The guard's doc no longer implies it covers either case. It now names both
exclusions, which is what it should have said before Codex had to ask.
Re-verified live after the redesign: 2625 rows persisted, all seq-stamped,
cursor advanced, zero page errors.
* docs+test: name the direction persistRetag does NOT close, and stop the tests implying coverage (BUG-2609)
Codex round 2, three findings.
P1 is real and I had only closed half the problem. persistRetag stops a late
RETAG from being refused, but a delta captured BEFORE the rename can commit
after it, whole-row put an older collection_slug at a NEWER seq, and pass the
guard legitimately. The root shape is that collection_slug is OUT-OF-BAND
relative to the item's own version — it changes without seq changing — so no
seq comparison can arbitrate it in either direction. Pre-existing (a blind put
lost the same race) and filed as BUG-2634 with the fix that actually closes it:
make the rename DURABLE, persisting the retag intent and reapplying it on
hydrate, the way localIndex already does in RAM with pendingRetags. That is a
different mechanism from this bug's guard, which is why it is filed rather than
folded in. The guard's doc now names the direction it does not cover instead of
implying it covers renames outright.
The other two are accuracy defects in what I wrote. A comment still said
`applyRetag` hands snapshots to persistUpserts, two paragraphs above the
exclusion list explaining that it no longer does. And the new test block read
as persistRetag coverage while testing only shouldWriteRow — a no-op
persistRetag would pass this entire file. It now says so in its name and its
doc, and the harness note says the same about the live browser run: that was
evidence the write path commits, not a regression test, and it will not run
again on its own.
Three of this bug's four review findings have been about my prose claiming more
than the code does, which is worth noting as the pattern rather than fixing
quietly a third time.
* docs: name the equal-seq residual where the decision is made (BUG-2609)
Codex round 3. Equal seq is permitted, and the justification I gave — RAM has
already merged the projection — holds WITHIN a tab and not across them: a tab
that never had the row in RAM can persist an unmerged snapshot over another
tab's merged one at the same seq.
Measured the reachability rather than accepting the report as stated. After
this branch, persistUpserts has exactly one caller, and `localIndex.upsert`
returns early when `next.seq <= existing.seq` — so it cannot persist an
equal-seq row when it HAS the row in RAM. The regression needs two tabs, the
writing tab lacking the row, and the other having merged a same-seq projection.
Narrow, and strictly better than before this branch, when a blind put regressed
rows at ANY seq.
Not fixed here because accept-or-refuse is the wrong axis: refusing would
mirror the bug onto persistDelta, where a re-delivered same-seq row carrying
computed projection fields would be skipped. What equal seq actually wants is a
MERGE, which this layer does not have and which is a deliberate layering
decision (persistence importing projection semantics). Filed as BUG-2635.
That makes three residuals from this one bug, and they are one family rather
than three coincidences — the persisted cache has no total order and no merge
semantics of its own. BUG-2633 (deletes have no seq to compare against),
BUG-2634 (collection_slug is out-of-band relative to seq), BUG-2635 (equal seq
needs merging). Each has a distinct fix, but they touch the same two functions
and share the same testing prerequisite, so 2635 carries the cross-links and
suggests triaging them as a batch — one pass giving the cache real
order-and-merge semantics beats three patches.
* fix(web): refuse a seq-less snapshot over a stamped row (BUG-2609)
Codex round 4, and this one was a real defect in my guard that my own test
enshrined.
I treated a missing seq symmetrically — "absence is not evidence of being
older, so write it" — which is true in one direction and wrong in the other.
The optimistic reorder path deliberately clears `seq` so the row bypasses
localIndex's RAM guard and the drag paints immediately (TASK-1357, verified at
the call site). Persisting that copy is incidental to the intent, and my rule
let a delayed seq-less snapshot overwrite an authoritative row that had already
landed at a real seq.
The result is worse than an ordinary stale row: the persisted row then has NO
seq at all, so neither this guard nor the RAM guard can order it on the next
warm boot, while the cursor sits past the delta that would have corrected it.
Now asymmetric, with the reasoning in place: an incoming row carrying ordering
evidence beats a stored row with none; two unstamped rows have nothing to
arbitrate; a stored STAMPED row refuses a snapshot that has no seq. Refusing
costs the reorder nothing — RAM still shows the optimistic order, the
authoritative response persists with a real seq moments later, and the cursor
has not advanced past that response, so a warm boot in between simply refetches
it.
The test that asserted the old behaviour has been replaced rather than
adjusted, and the mutation restoring the symmetric rule now fails only the new
case. Re-verified live after the change: 2625 rows persisted, all seq-stamped,
cursor advanced, zero page errors.
Distinct from the three filed residuals — this is the guard being wrong, not a
gap it deliberately leaves.
* fix(web): re-check collection membership inside the retag transaction (BUG-2609)
Codex round 5, a defect in the function I added last round. persistRetag read
each row by id and applied the renamed collection's slug without re-checking
that the row was still IN that collection. A row that moves between the RAM
retag and this transaction would then be persisted with a collection_id and a
collection_slug that disagree — behind the cursor, so no delta repairs it.
The irony is the point: persistRetag exists BECAUSE trusting a RAM snapshot at
write time is unsafe, and it went on trusting the snapshot's collection
membership. Re-reading the row was never the whole fix; re-checking what the
row says is.
The per-row decision is extracted as shouldApplyRetag so it is reachable by
tests at all — the transaction itself is not, in a harness with no IndexedDB —
and it now covers three refusals with a reason each: a row that moved
(mismatched id/slug persisted behind the cursor), a row already carrying the
new slug (idempotence), and an absent row (inserting one would resurrect it
behind the cursor, the BUG-2633 shape).
Each mutation-verified against its own assertion: dropping the membership check
fails only the moved-row case, and inserting absent rows fails only the
resurrection case. Re-verified live: 2625 rows persisted, all seq-stamped,
cursor advanced, zero page errors.
* docs: correct two claims this file made about itself (BUG-2609)
Codex round 7, both accuracy defects in my own prose after five rounds of
edits to it.
shouldWriteRow's opening line still said it returns false ONLY for strictly
older rows, which round 4 made untrue — it also refuses a seq-less row over a
stamped one. The paragraph below described the exception correctly, so the
summary contradicted its own body. Now states both cases up front.
The test file asserted "jsdom has no IndexedDB", and this file does not run in
jsdom: a plain .test.ts belongs to vitest's `node` project, per vitest.config.
The conclusion happened to hold — Node has no indexedDB global either — which
is the part worth flagging: right answer, wrong stated reason, and I had not
opened the config before writing it. It also claimed every function in the
module is a no-op under vitest, which is false of the exported decision
helpers being tested two screens above; that distinction is the whole reason
they were extracted.
Fixing the first attempt at this broke the file, and the cause is worth
recording: the glob I wrote to name the vitest project contained the character
pair that ENDS a block comment, so the doc terminated early and the rest parsed
as code — 19 type errors and a suite that reported "no tests". Same family as
backticks inside a double-quoted shell string: content carrying a delimiter the
surrounding syntax acts on. Rephrased to avoid the sequence rather than
escaping around it.
* docs: name the abort trigger at persistRetag, folded into BUG-2634 (BUG-2609)
Codex round 9. persistRetag is best-effort like everything in this module, so
an aborted transaction (quota, eviction, tab freeze) loses the rename outright
— and a lost rename does not self-heal for the same reason it cannot be
reordered: no item delta re-stamps those rows, and pendingRetags is in-memory.
Not a fourth filing. It is BUG-2634 reached by failure instead of by racing,
and the fix already proposed there — persist the retag INTENT and reapply it on
hydrate — closes both, because a recorded intent survives a failed write as
readily as a lost race. The real defect is that a rename is persisted as an
EFFECT with no durable intent, which makes it losable by anything.
Recorded on BUG-2634 so whoever takes it builds for both triggers (an
ordering-only fix would leave the abort case open and look complete), and noted
at persistRetag so a reader there meets the limit rather than inferring the
function is reliable.
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