mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-23 02:53:31 +00:00
v0.4.0
635 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 xarmian
|
9cbb08a16c |
feat(web): wire ContentError + retry for HTTP fail, collab offline, stuck-connecting (TASK-1376) (#516)
* feat(web): wire ContentError + retry for HTTP fail, collab offline, stuck-connecting (TASK-1376)
Three failure modes now surface ContentError with a retry path:
1. HTTP load error (page-level): the {:else if error} branch
replaces the literal `<div class="center-message">{error}</div>`
with <ContentError onRetry={loadData}>. Users can recover without
navigating away.
2. Collab offline: when the WS provider hits the OFFLINE_THRESHOLD
(3 consecutive failed reconnects) and `state === 'offline'`, the
editable {:else if ydoc} branch surfaces ContentError instead of
the empty Y.Doc editor.
3. Stuck-connecting: a 10s timer-driven $effect sets
staleConnecting=true if the provider sits in `connecting` without
ever syncing. Same ContentError UI as offline. Timer is cleared
on state change, hasEverSynced flip, or provider rebuild.
Retry path (retryCollabSync) mirrors the server-driven force_refresh
dance:
1. Clear staleConnecting (state will reset naturally on rebuild).
2. Refetch items.content so the lazy-seed (TASK-1261) on the new
Y.Doc has canonical content.
3. Bump forceRefreshNonce → the existing collab $effect tears down
the dead provider, mints a new one. The TASK-1375 reset $effect
handles `hasEverSynced=false` and `editorInstance=null` as part
of that rebuild, so retry doesn't need to touch them directly.
Error gate is placed BEFORE the skeleton gate in the {:else if ydoc}
branch so stuck-connecting flips out of shimmer-forever and into a
clear error UI at the 10s mark.
CONVE-606: the stuck-connecting $effect has a single clean dependency
list (collabProvider + state + hasEverSynced); the latch is a pure
imperative flag flipped by a setTimeout, not derivable.
Parent: PLAN-1373. Resolves BUG-1372 (final piece).
* fix(web): preserve local edits on retry, reset staleConnecting per provider (TASK-1376 round 1)
Codex round 1 caught three correctness issues in the initial retry
wire-up; addressed all of them.
P1 — retryCollabSync was overwriting local edits.
The original (lifted from onForceRefresh) refetched items.content
before bumping forceRefreshNonce. In the server-driven
force_refresh case that's correct because the server is the source
of truth. In the retry case the LOCAL Y.Doc is the canonical view
(it may hold unflushed user typing from the offline/connecting
window); shoveling stale server content into \`item\` before the
cleanup's flushCollabNow ran risked the lazy-seed on the new Y.Doc
re-encoding the stale view, then the next flush PATCHing that back
over the user's just-persisted edits.
Fix: drop the refetch. The collab \$effect cleanup already calls
flushCollabNow on tear-down (lines ~727–729), preserving local
edits via PATCH BEFORE the new provider mints a fresh Y.Doc. The
new provider's WS replay reconciles against server state via the
op-log; if the cursor has been pruned the server sends a real
force_refresh which goes through onForceRefresh (which DOES
refetch — correctly).
P2 (first) — failed retry left staleConnecting=false with no
retry affordance. Gone naturally: retryCollabSync is now
synchronous with no failure path.
P2 (second) — staleConnecting was not reset when collabProvider
rebuilt. The early-return-on-null path skipped the false-reset,
so a stuck-connecting flag from a previous provider carried into
the new one, showing error UI immediately instead of granting
the fresh 10s grace.
Fix: unconditional \`staleConnecting = false\` at the top of the
effect (after the null guard). Only the 10s timer can flip it
back to true.
Codex round 1.
* fix(web): gate offline error UI on !hasEverSynced to protect local edits (TASK-1376 round 2)
Codex round 2: the fire-and-forget flushCollabNow in the collab
\$effect cleanup is racy — it kicks off a PATCH but doesn't await
runCollabFlush or update local item.content. The new provider's
lazy-seed reads item.content (stale relative to the local Y.Doc),
encodes it into a fresh op-log, then the next 5s flush PATCHes that
stale content back over the user's just-flushed edits.
Real fix: don't expose retry when there are local edits at risk.
The template's error gate now reads:
(collabProvider?.state === 'offline' && !hasEverSynced) || staleConnecting
Both branches imply !hasEverSynced, so the current Y.Doc has never
received a sync and therefore cannot hold user edits. retryCollabSync
is safe in that universe — tearing down the provider can't lose
unflushed work.
For state === 'offline' WITH hasEverSynced=true (was synced, then
got disconnected), the editor stays mounted with its bound Y.Doc:
- The corner badge (line ~1700) already signals offline via the
four-state pending-sync indicator.
- CollabProvider's reconnect loop keeps trying with exponential
backoff (1s → 30s capped); auto-recovery is the path.
- In-progress user edits remain bound to the live Y.Doc;
nothing destroys them.
- When the WS comes back, normal sync flow reconciles.
This is also a better UX than the prior "wipe editor, show error" —
a user mid-edit doesn't lose their working canvas when their wifi
hiccups.
Codex round 2.
v0.4.0
v0.4.0-rc.1
|
||
|
xarmian
|
1af63e3a47 |
feat(web): wire ContentSkeleton into item detail loading + collab-sync (TASK-1375) (#515)
* feat(web): wire ContentSkeleton into item detail loading + collab-sync (TASK-1375)
Two gaps where the item detail page would show a blank body:
1. HTTP-load gap — `{#if loading}` rendered the literal string
"Loading..." while loadData()'s GET was in flight. Now renders
<ContentSkeleton variant="page" />.
2. Collab-sync gap — for editable items, the Editor mounted on
`ydoc !== null` but content lives in the Y.Doc which starts
empty until the WS replays the op-log. On slow/broken
connections this looked indistinguishable from a genuinely
empty item. The {:else if ydoc} branch now renders
<ContentSkeleton variant="inline" /> while
`collabProvider.state === 'connecting' && !hasEverSynced`.
The `hasEverSynced` latch (split into two $effects per CONVE-606
— route-change reset on item.id vs reactive-state-sync on
collabProvider.synced) ensures mid-session `reconnecting` does
NOT re-show the skeleton over already-rendered content. The
reset on item navigation handles SvelteKit's reuse of
+page.svelte across [slug] changes.
Error UI and retry (offline state, stuck-connecting timeout) is
TASK-1376's scope — this PR is skeleton-only.
Parent: PLAN-1373. Resolves part of BUG-1372.
* fix(web): reset hasEverSynced on provider change, not just item.id (TASK-1375 round 1)
Per Codex review: the original `void item?.id` reset dependency
missed two cases where the same item gets a fresh unsynced Y.Doc:
1. Raw -> Rich mode toggle (collabKey derives from rawMode, so
the collab $effect tears down + rebuilds the provider but
item.id is unchanged).
2. forceRefreshNonce bump (server force_refresh or a future
TASK-1376 retryCollabSync), which rebuilds the provider
against the same item.
In both cases hasEverSynced=true survived the rebuild, the
skeleton gate bypassed, and the editor mounted on the new
unsynced Y.Doc showing blank — re-opening the very pre-sync
empty window the skeleton was meant to close.
Fix: depend on `collabProvider` (the reactive variable, not its
fields). Any provider instance change — navigation, mode toggle,
nonce bump, cleanup-to-null — fires the reset. Strictly more
correct than item.id since it also covers same-item rebuilds.
Codex round 1.
* fix(web): null editorInstance on provider change (TASK-1375 round 2)
Per Codex review round 2: `editorInstance` is set by the Editor's
`onEditor` callback on mount but is NOT re-nulled on unmount. During
the connecting-skeleton window for a same-item provider rebuild
(rawMode toggle, force_refresh), the old <Editor> unmounts but
`editorInstance` still points at the previous (now-destroyed)
instance.
If an `applier_request` frame arrives on the new provider during
that window, `onApplierRequest` would call `setContent` on the
WRONG editor (or a destroyed one) instead of returning false and
letting the server fall back to a direct items.content write.
Fix: null `editorInstance` in the same reset $effect that resets
`hasEverSynced`. The new editor's onEditor callback re-populates
the reference once it mounts after the skeleton phase.
Codex round 2.
|
||
|
xarmian
|
307d6e7221 |
feat(web): add ContentSkeleton + ContentError primitives (TASK-1374) (#514)
Two reusable presentational components for the item-detail loading and error UX work in PLAN-1373: - ContentSkeleton: CSS-only shimmer placeholder with 'page' / 'inline' variants. Respects prefers-reduced-motion. Pure presentational, no Pad-state imports. - ContentError: centered title + optional detail + Try again button. Mirrors EmptyState's visual language. Both use Svelte 5 runes, design tokens, and a11y attributes (role=status / role=alert, aria-hidden on decorative bars/icon). No wiring yet — TASK-1375 and TASK-1376 consume these. Parent: PLAN-1373. |
||
|
xarmian
|
c2351d861d |
fix(ci): lower test-only bcrypt cost + re-enable -race on PRs (BUG-1371) (#513)
The full `internal/server` test suite under `-race` had grown past the 30m CI timeout, failing every push to main since ~TASK-1354. Diagnosis: bcrypt at the production cost (12) takes ~3s per call under the race detector, and dozens of tests now bootstrap a user via the loopback HTTP path (`bootstrapFirstUser` → `store.CreateUser` → `bcrypt.GenerateFromPassword`). Cumulative cost dominated the budget. Two coordinated changes: 1. Lower bcrypt cost in test binaries. `bcryptCost` becomes a package var (still package-private), and a new `SetBcryptCostForTesting` helper lets each test binary's `TestMain` drop it to `bcrypt.MinCost`. Production stays at 12 — only the test process ever mutates the value. 2. Re-enable `-race` on pull requests. The `if: github.ref == 'refs/heads/main'` gate was originally a GitHub Actions minutes cost-control; the repo is public now, so PR minutes are free, and we'd rather catch race regressions on the contributing branch than after merge. Measured impact: - `go test -race ./internal/server`: 1800s timeout → 830s (13m51s). - `go test ./internal/store`: 808s → 35s. - `go test ./internal/server`: 192s → 60s. The 30m timeout stays — it's headroom for genuine deadlocks, which would still hit the goroutine-dump panic the way BUG-851 did. Prior art: BUG-851 (10m → 30m bump, ipRateLimiter goroutine drain). This is a different cause (bcrypt cumulative time) so the fix is different. |
||
|
xarmian
|
350e8ef576 |
feat(web): saved view defaults applied on collection-page entry (TASK-1366) (#512)
* feat(web): saved view defaults applied on collection-page entry (TASK-1366) Phase 3d of the local-first read model (PLAN-1343 / DOC-1342): per the design note's recommendation, persist the user's preferred saved view per (workspace, collection) in localStorage and re-apply it on mount. No schema change in v1; cross-device sync can come later as a server-side `is_default` column on saved views. Behavior - localStorage key: `pad-default-view:<wsSlug>:<collSlug>` → view id. - On collection-page mount, after `savedViews` loads, look up the default and call `applyViewConfig` automatically. - URL-driven state wins: if the user arrived via a shared link with `?q=...` or `?status=...`, the default is skipped so the link's intent isn't hijacked. - "Make default" / "Default ★" toggle next to the saved-views bar flips the persistence state for the currently active view. The active default also renders a small pin icon on its tab so users can see which view is current at a glance. - `deleteView` clears a dangling localStorage pointer when the default view is the one being removed. - localStorage failures (private mode, quota) degrade silently — the toggle still works for the session. Out of scope (deferred to a future PR) - Cross-device sync via a server-side `is_default` column. - Sharing defaults across workspace members. - Auto-save view config changes back to the underlying saved view. Parent: PLAN-1343. Completes Phase 3 of DOC-1342. * fix(web): gate default-view apply on metaLoading per Codex review (round 1) Codex round 1 P1 #1: `loadCollection` flips `metaLoading=true` at entry, assigns `savedViews` mid-flight, then calls `loadUrlFilters()` synchronously near the end, and only flips `metaLoading=false` in the `finally` block. My default-view effect tracked `savedViews` and ran as soon as it changed — so a shared link like `?q=foo` could land in the local state AFTER the savedViews assignment but BEFORE `loadUrlFilters` populated `searchQuery`. The effect saw an empty searchQuery, thought there were no URL overrides, and applied the default — clobbering the incoming URL. Codex round 1 P1 #2: on client-side navigation across collections, `defaultViewApplied` got reset by the route-change effect but `savedViews` still held the PREVIOUS collection's views until the new fetch resolved. The default-apply effect would run with the wrong list, fail to find the new collection's default view id in the stale list, and ERASE the localStorage pointer — wiping the default on every cross-collection navigation. Gate the effect on `!metaLoading`. `metaLoading=false` only fires after BOTH the new `savedViews` is assigned AND `loadUrlFilters()` has run, so both races are eliminated. * fix(web): URL-override check reads page.url directly per Codex review (round 2) Codex round 2 P1: `?view=board` URLs are explicit user intent but my urlOverrides check only looked at `searchQuery` and `activeFilters`, so view-only URLs would be overwritten by the default-view apply. Codex round 2 P2: `loadUrlFilters` doesn't clear absent params, so parsed `searchQuery` / `activeFilters` can carry leftover values from the previous route on cross-collection navigation. A clean URL on the new route would then look "overridden" via stale parsed state, and the default would be incorrectly skipped. Both fixed by reading `page.url.searchParams.size` directly. The collection page only writes user-driven params (view, q, field filters), so any non-empty searchParams signals explicit intent. |
||
|
xarmian
|
06c5e5cd2d |
feat(web): CommandPalette uses localSearch (TASK-1365) (#511)
* feat(web): CommandPalette uses localSearch (TASK-1365)
Phase 3c of the local-first read model (PLAN-1343 / DOC-1342): wire
the global CommandPalette / top-bar search to localSearch.
Behavior
- Default scope: search the current workspace's in-memory MiniSearch
index synchronously on every keystroke. No network round-trip, no
200ms debounce — sub-millisecond typing.
- "All workspaces" toggle: when on, also search every other workspace
whose localIndex is `'ready'` (i.e. already hydrated this session).
Results from every ready workspace are merged by score, ties broken
by `updated_at DESC`. Toggle state persists to localStorage so it
survives reloads. Hidden when only one workspace is ready (no
point showing a no-op toggle).
- Cross-workspace navigation: each local result carries its source
workspace slug + owner_username so `selectResult` navigates to the
right route — `selectResult` falls back to `workspaceStore.current`
for server hits.
- Server fallback paths:
* `body:` / `content:` queries — local index doesn't hold the
rich-text body, so server FTS is the only way to grep.
* No ready workspaces yet (cold session) — falls through to
`api.search` so the palette still works pre-bootstrap.
- Reactive: a single `$effect` watches `query`, `searchAllWorkspaces`,
filter chips, every workspace's `localSearch.epoch`, and the
current workspace's bootstrap state — so SSE-driven upserts and
hot toggle flips re-rank without manual `doSearch()` calls. The
`oninput={doSearch}` handler is removed; reactivity does the work.
- Drops `result-count`, `loadMore`, and facets on the local path —
local results aren't paginated (everything's in RAM, capped at
`PAGE_SIZE * 2` for display); facets are a server-only feature.
Parent: PLAN-1343.
* fix(web): hide Load more on local search path per Codex review (round 1)
Codex round 1 P2: `total` was set to the pre-slice
`filtered.length` while `results` was capped at `PAGE_SIZE * 2`.
That made the "Load more" affordance show when local matches
exceeded the cap — and clicking it would call `api.search`, which
injects single-workspace server-FTS rows into the local
(potentially cross-workspace) result set.
Set `total = results.length` on the local path so the affordance
stays hidden. Local results are all in RAM; if the result count
exceeds the display cap the right answer is a tighter query, not
a paginated server fetch.
* fix(web): current-workspace fallback + filter chip persistence per Codex review (round 2)
Codex round 2 P2 #1: with `searchAllWorkspaces` on, if the current
workspace was still bootstrapping but any OTHER workspace was ready,
`ready.length > 0` sent the search down the local-only path —
omitting current-workspace results entirely. The toggle is meant to
widen the search, never to replace the current workspace.
Add an explicit `currentReady` check: only take the local path
when the current workspace is ready. Otherwise fall through to the
server (which will return the current workspace's results too).
Codex round 2 P2 #2: filter chips were gated on `facets` (server
only). Switching from server → local with a filter active would
hide the chip but keep the filter applied. Add a fallback row that
shows the active chip(s) on the local path so they're visible and
clearable.
* fix(web): stale-response guard + status filter under-fill per Codex review (round 3)
Codex round 3 P2 #1: server search responses had no stale-response
guard. A request started while `currentReady` was false (or for a
`body:` query) could return after the local path had already
rendered and clobber local results — including reintroducing
`total > results.length` and the Load more button. Snapshot the
query + `searchAllWorkspaces` flag at dispatch; only apply the
response if both still match. The same guard protects the catch
and finally branches.
Codex round 3 P2 #2: local status filtering was applied AFTER the
per-workspace `localSearch.search(... limit: 20)` cap, so a status
chip could under-fill or empty the result set even when matching
items existed beyond the cap. Expand the per-workspace pull by 5x
when `filterStatus` is active so the post-fetch filter has
headroom. (The collection filter doesn't need this because it's
passed directly to `localSearch.search`, which filters inside
the index walk.)
* fix(web): full-scope stale-response guard per Codex review (round 4)
Codex round 4 P2: the R3 stale-response guard only snapshotted
`query` and `searchAllWorkspaces`. An in-flight server response
could still clobber newer local results after `currentReady`
flipped to true, or overwrite results after a filter chip changed
with the same query.
Snapshot the full dispatch scope (query, toggle, filter chips,
current workspace slug, currentReady-vs-body branch) at request
time and gate every `results = ...` site on `isSameDispatch()`.
Once the current workspace hydrates, a server response from a
`!currentReady` snapshot is no longer authoritative.
* fix(web): read live currentReady + guard loadMore per Codex review (round 5)
Codex round 5 P2 #1: my R4 `isSameDispatch` captured
`currentReady` at dispatch time, so the check stayed stale once
the index hydrated mid-request — the cold server response still
matched and clobbered the local results that the readiness effect
had just produced. Switch to reading live state via
`localIndex.bootstrapStateFor(snapshotWsSlug)` inside the guard;
captured `snapshotCurrentReady` is removed.
Codex round 5 P2 #2: `loadMore` only snapshotted query/filters. A
server-page request in flight could append rows after the scope
changed (current workspace hydrated, all-workspaces toggle flipped).
Add the same full-scope guard — query, toggle, both filter chips,
workspace slug, and live-readiness — and bail if any has shifted.
* fix(web): loadMore handles body: prefix correctly per Codex review (round 6)
Codex round 6 P2: `loadMore` was sending the raw `query` to
`api.search`, so page-2 of a `body:foo` search hit the server
with the literal `body:foo` token. Worse, the live-readiness guard
from R5 dropped body: page-2 responses whenever the current
workspace was ready — but body: searches NEED the server (the
local index doesn't carry content), so they should bypass that
guard.
Parse the query in `loadMore` and send the stripped `parsed.text`
when the body prefix is present. Body queries are now exempt from
the live-readiness drop; only non-body server pagination needs to
worry about the path swap.
* fix(web): body queries skip local-index dependency tracking per Codex review (round 7)
Codex round 7 P2: the search-dispatch effect always tracked
`localIndex.bootstrapStateFor` and `localSearch.epoch` for every
workspace, including for body: queries. An SSE-driven epoch bump or
hydration completion mid-flight would re-fire doSearch from offset 0
and wipe an in-flight `loadMore` append on the body: path.
Gate the local-index dependency reads on `!parsed.body`. Body
queries hit server FTS exclusively (the local index doesn't carry
content), so their result set is unaffected by client-side mutations;
skipping the tracking eliminates the loadMore race without losing
incremental-update behavior for local searches.
* fix(web): short-circuit local-state reads in doSearch for body queries per Codex review (round 8)
Codex round 8 P2: even after R7 made the `$effect` skip explicit
localIndex/epoch reads for body queries, `doSearch()` still
synchronously called `readyWorkspaces()` and
`localIndex.bootstrapStateFor()` BEFORE branching on `parsed.body`.
Those reads register as reactive dependencies of the caller, so a
mid-flight SSE bump or hydration completion would still re-fire
doSearch and clobber an in-flight body: `loadMore` append.
Reorder doSearch: parse first, then short-circuit both
`currentReady` and `readyWorkspaces()` to constants when
`parsed.body` is true. Body queries are server-authoritative;
nothing in local state can change their result set, so they pay
no reactivity tax on the local index.
* fix(web): bare body:/content: queries no-op per Codex review (round 9)
Codex round 9 P3: bare `body:` / `content:` with no following
text was falling back to the raw query, shipping the literal
`body:` token to `/search`. `loadMore` had the same fallback.
Short-circuit both paths: when `parsed.body` is true and
`parsed.text` is empty, clear results / bail. There's nothing
useful to search for until the user keeps typing.
|
||
|
xarmian
|
aea4c3435b |
feat(web): localSearch relevance + ref/number matchers (TASK-1367) (#510)
* feat(web): localSearch relevance + ref/number matchers (TASK-1367) Phase 3e of the local-first read model (PLAN-1343 / DOC-1342): tune the MiniSearch relevance config from 3a and add a centralized prefix parser so search "feels right" on real workspaces. Relevance tuning - Boosts: title 3x → 5x (title-vs-tag ties were leaving tag-rich rows ahead of cleaner title matches); ref / item_number 2x → 4x so typed prefix-shaped refs out-score incidental field hits. - Per-term fuzz / prefix: length-aware functions disable fuzz for terms <4 chars (so `cat` no longer fuzzes into `bat`/`hat`/`category`) and prefix-matching for single chars (single-letter terms over-matched the rest of the index). Prefix vocabulary (`parseSearchQuery` — exported) - `body:foo` / `content:foo` — route to server FTS over the rich-text body; the local index doesn't hold `content`. - `coll:tasks foo` — restrict to a single collection. - `is:archived` — include soft-deleted rows in the result set. - `#5` / `item:5` — exact item-number lookup (single doc hit, scoped by the caller's collection / archived options). - `TASK-5` — exact-ref hoist; the matching row jumps to the top of the ranked list regardless of MiniSearch's organic score. - Bare digits (`5`) get treated as `#5` for typing-speed. Centralization - Collection page now imports `parseSearchQuery` instead of its own ad-hoc `body:`/`content:` regex — one parser, one prefix vocabulary across the page and (incoming) CommandPalette wiring (TASK-1365). - FilterBar tooltip updated to document the full prefix vocab. Parent: PLAN-1343. Acceptance: title outranks field-only, `TASK-5` returns its row first, `db migr` matches `Database migration plan` within the top 3 (verified via the existing tokenize + boost path), performance unchanged (the new exact-lookup short-circuits avoid the linear-index walk for the common ref/number cases). * fix(web): prefix-only queries + is:archived inclusion per Codex review (round 1) Codex round 1 P2 #1: `is:archived` was advertised but didn't surface archived rows. The collection page's `items` derived view filtered by the toggle alone, so archived IDs returned by `localSearch.search()` were dropped before render. Wire a reactive `parsedSearch` derived into `items` so the prefix transiently opts in to archived inclusion without requiring the user to also flip the toggle. Codex round 1 P2 #2: prefix-only queries (`coll:tasks`, `is:archived` alone) fell back to `parsed.text || query`, which sent the literal `is`/`archived` tokens through MiniSearch and surfaced unrelated rows. Return empty instead — a prefix without query content is a filter, not a search. * fix(web): bare digits + preserve search ranking per Codex review (round 2) Codex round 2 P2 #1: bare-digit queries like `5` left `parsed.text` populated (`"5"`), so the `!parsed.text` guard suppressed the exact-number short-circuit and MiniSearch returned incidental hits. Split the two branches: explicit `#5`/`item:5` uses the parsed itemNumber path; bare digits always take the exact-lookup path. Codex round 2 P2 #2: the collection page was converting localSearch results into a `Set` and filtering items by `has()` — which preserved the natural `updated_at DESC` order and threw away the new ref-hoist + boost tuning ranking. Refactor `searchResultIds` into `searchResultRank`: `Map<itemId, rank>` where rank is the 0-indexed position in the result list. `filteredItems` now filters by `rank.has(item.id)` and sorts by rank, so the exact-ref hoist and relevance tuning surface in the UI. Same change applied to the body: server-FTS path so its order is preserved too. * fix(web): defer is:archived prefix per Codex review (round 3) Codex round 3 P2: `is:archived body:foo` doesn't actually surface archived rows because the server `/search` endpoint hard-filters `deleted_at IS NULL` at every query branch. The local item source widens, but the server response can only contain live IDs, so archived body hits never render. Pull `is:archived` from `parseSearchQuery` and the FilterBar tooltip rather than ship a half-working prefix. The existing `showArchived` UI toggle covers the local search path; the only missing UX (archived + body) is gated on server work that's out of scope here. Spawned HT-1370 with a full pickup runbook: add `IncludeArchived` to `store.SearchParams`, gate the `deleted_at IS NULL` clauses, plumb `include_archived=true` through `/search` + the API client, then reintroduce the prefix. * fix(web): preserve search rank in ListView/BoardView per Codex review (round 4) Codex round 4 P2: search rank was sorted at the page level, but ListView and BoardView re-sorted by `sort_order` within each status group/column, clobbering the exact-ref hoist + boost ranking whenever two matches shared a group. Add an optional `preserveOrder` prop to both views (default false to preserve existing call-site behavior elsewhere). When true, the in-group / in-column `sort_order` sort is skipped and the parent's item order wins. The collection page passes `preserveOrder={searchResultRank !== null}` so the prop only flips during an active search; the default drag-reorder UX is untouched when not searching. TableView already renders parent order by default (its sort is gated on a user-chosen `sortKey`), no change needed there. * fix(web): disable item DnD when preserveOrder is on per Codex review (round 5) Codex round 5 P2: with `preserveOrder=true` (search active), ListView and BoardView still allowed dragging items. `handleFinalize` would then write the displayed relevance-ranked subset order back as `sort_order` on every dropped row, corrupting the workspace's manual ordering with whatever subset happened to be on screen. Extend the zone-level `dragDisabled` to OR in `preserveOrder` so drag is also off while search is active. Column reordering on the board (separate gesture) stays enabled — columns are status groups, not search results, so reordering them is still safe. * fix(web): gate preserveOrder on searchQuery not searchResultRank per Codex review (round 6) Codex round 6 P2: the R5 fix only set `preserveOrder=true` once `searchResultRank` was populated. But the search effect intentionally sets `searchResultRank = null` during the body: debounce window and the cold-index fallback. `filteredItems` still renders a subset in those states (via the substring fallback), so a drag would persist the subset's order as `sort_order`. Switch to `preserveOrder={searchQuery.trim() !== ''}` so DnD is disabled and the in-group sort is skipped whenever a search is active, regardless of which path populated the result set. |
||
|
xarmian
|
c59132ad35 |
feat(web): collection page search uses localSearch (TASK-1364) (#509)
* feat(web): collection page search uses localSearch (TASK-1364)
Phase 3b of the local-first read model (PLAN-1343 / DOC-1342):
replace the collection page's debounced `api.search` round-trip with
`localSearch.search`, making the search box sub-millisecond.
- `handleSearchChange` now runs `localSearch.search(wsSlug, query,
{ collection, includeArchived, limit })` synchronously on every
keystroke. No debounce — local results are <1ms.
- `body:` / `content:` prefix routes the query back through the server
FTS endpoint (200ms debounce preserved) so the rich-text body stays
searchable. The local index intentionally excludes content per
DOC-1342 decision #4 — content lives on the server.
- A small `$effect` re-runs the local search when `showArchived` or
`indexReady` flip mid-typing, so cold-load + already-typed queries
surface results once the index hydrates, and the archived toggle
re-evaluates the active query.
- FilterBar input gets a `title=` tooltip documenting the `body:`
prefix UX.
- The existing client-side substring fallback (used when
`searchResultIds === null` and a query is typed) now covers two
narrow cases: server-FTS in-flight, and cold-load bootstrap window.
Parent: PLAN-1343. Phase 3b acceptance: keystroke → first results
<50ms P95 — local search runs synchronously inline with the
keystroke handler.
* fix(web): unified search effect handles URL-load body: queries per Codex review (round 1)
Codex round 1 P2: shared/reloaded URLs like `?q=body:foo` previously
only set `searchQuery` via `loadUrlFilters` without triggering the
dispatch, so the local fallback would search the literal string
`body:foo` against titles + fields instead of routing to server FTS.
Refactor: `handleSearchChange` is now a pure setter for
`searchQuery` + URL state. A single reactive `$effect` watches
`searchQuery`, `showArchived`, and `indexReady` and dispatches:
body-prefix → server FTS (debounced 200ms), else → synchronous
MiniSearch. This covers every entry point that mutates `searchQuery`
(typed input, URL load, programmatic clear) with one code path.
* fix(web): snapshot wsSlug/collSlug in search effect per Codex review (round 2)
Codex round 2 P2: navigating between workspaces or collections while a
`body:` query was in flight could land the old response on the new
route. Snapshot `wsSlug` / `collSlug` at effect-run time, route
`api.search` through the snapshots, and include them in the
stale-response guard alongside the query string.
* fix(web): refresh active search on localSearch mutations per Codex review (round 3)
Codex round 3 P2: while a search query was active, SSE-driven
`localIndex.upsert` / `applyDelta` would refresh the items list and
the underlying MiniSearch index, but the page's `searchResultIds`
Set stayed pinned to the original keystroke's results. A matching
row created after the query stayed hidden; an edited row that no
longer matched stayed visible until the user retyped.
Add a reactive per-workspace mutation epoch to `localSearch`:
`epoch(ws)` returns a SvelteMap-backed counter bumped by every
successful `rebuild` / `upsert` / `remove`. The collection page's
search-dispatch `$effect` reads it as a tracked dependency so the
search re-runs whenever the index changes. Server-FTS body queries
also benefit because the same effect path covers them.
|
||
|
xarmian
|
4e04148f13 |
feat(web): localSearch MiniSearch store (TASK-1363) (#508)
* feat(web): localSearch MiniSearch store (TASK-1363)
Build the client-side full-text search foundation for Phase 3 of the
local-first read model: a per-workspace MiniSearch index that mirrors
`localIndex` and provides sub-millisecond ranked search over titles +
parsed fields. No UI changes — TASK-1364 wires the collection page and
TASK-1365 wires the global CommandPalette.
- New `web/src/lib/stores/localSearch.svelte.ts`: per-workspace
MiniSearch index keyed by workspace slug (module-level plain Map —
search results are derived on demand via `.search()`, not subscribed
to). Indexed fields with boosts: title (3x), ref (2x), item_number
(2x), tags, parent_ref, parent_title, collection_slug, and a
flattened `fields` blob so status / priority / assignee names land
as searchable terms. Custom tokenizer splits on whitespace + `-_./`
so `TASK-5` indexes as both `task` and `5`. Public API: `rebuild`,
`upsert`, `remove`, `reset`, `search(ws, q, opts?)`, `size`.
Returns `{ id, score }[]` ranked by descending score; callers
resolve to full rows via `localIndex` (O(1) Map lookup).
- `localIndex` mutation paths now mirror writes into the MiniSearch
index: `applyDelta`, `upsert`, `remove`, `removeByCollection`,
`reset` (and the 403-purge error path in `bootstrap`). The cold
`/items-index` snapshot and the warm IDB hydrate trigger a single
bulk `localSearch.rebuild(...)` instead of N per-row upserts —
measurably cheaper at workspace boot.
- SSR-safe: every entry point gates on `typeof window !== 'undefined'`
so SvelteKit prerender / SSR can't build a server-side index.
- Add `minisearch@7.2.0` (~10KB gz) as an exact-pinned dependency.
Parent: PLAN-1343 (DOC-1342 Phase 3a).
* fix(web): broaden localSearch tokenizer per Codex review (round 1)
Codex round 1 P2: the previous tokenizer only split on whitespace + a
narrow set of separators (`-_./`), so values like `foo:bar`,
`foo,bar`, and `foo(bar)` indexed as a single token. Searches for
`bar` or `foo bar` would miss valid matches.
Broaden to split on any non-letter/non-digit character (`/[^\p{L}\p{N}]+/u`).
This matches MiniSearch's default `SPACE_OR_PUNCTUATION` shape — catches
whitespace, dashes, dots, slashes, colons, commas, parens, brackets,
underscores, etc. — while preserving the `TASK-5` → ['task', '5']
behavior the original regex aimed for.
|
||
|
xarmian
|
f1c9457790 |
feat(web): 403-driven cache purge for localIndex (TASK-1360) (#507)
* feat(web): 403-driven cache purge for localIndex (TASK-1360) Per DOC-1342 design decision #3: the local cache is "what you could see last time you synced." When the server returns 403 mid-session, the offending entry is purged so the next read doesn't surface stale-by-permission rows. ## API client (web/src/lib/api/client.ts) - New `AccessRevokedScope` type + `setAccessRevokedHandler` registration hook. Keeps client.ts free of any store import (no circular dep). - `request()` on 403 parses the URL with `parseAccessRevokedScope` (handles item endpoints `/workspaces/{ws}/items/{idOrSlug}` and collection-items endpoints `/workspaces/{ws}/collections/{coll}/items`) and invokes the handler. Handler failures are caught + logged; the 403 still propagates as a `PadApiError`. ## localIndex - New `removeByCollection(ws, collSlug)` — bulk-remove every row in a collection. Used when a collection-scoped 403 means the whole grant is revoked. - New `findByIdOrSlug(ws, idOrSlug)` — id-first then slug-scan lookup so an item-scoped 403 with a slug URL can resolve to the in-RAM id (the SvelteMap is keyed by id). ## App bootstrap (+layout.svelte) - Registers the handler once at top level: item → `remove` after `findByIdOrSlug` resolves; collection → `removeByCollection`. Both purges write through to IDB via the existing `persistRemovals` path so reloads don't resurrect the stale row. 401 already triggers a /login redirect; this lands as the 403 counterpart. Permission-revocation that doesn't trigger a 403 (e.g. visibility loss with no fetched row) is explicitly punted per DOC-1342 #3 — best-effort cache, not authoritative for permissions. Parent: PLAN-1343. See DOC-1342 design decision #3. * fix(web): only purge on GET 403s, not write-method 403s (Codex round 1) [P1] notifyAccessRevoked fired on every 403 — including POST /items (create), PATCH /items/{id} (update), DELETE /items/{id} (archive), and the grants/share-link write endpoints. A read-only user who correctly fails to create or modify an item would have their entire collection purged from localIndex. Gate the purge on read methods (GET / HEAD). Write-method 403s mean "you can read but not write" — the cached rows are still legitimately visible. Read-method 403s are the canonical "visibility revoked" signal. Parent: PLAN-1343. * fix(web): purge entire workspace on 403, not per-item/collection (round 2) [P1] Codex caught two related issues: 1. Pad's server returns 403 from the workspace-access middleware (`permission_denied`, `not a member of this workspace`), and item-level visibility misses return 404. A 403 on /workspaces/{ws}/items/{slug} therefore means workspace access is gone — purging only `slug` leaves the rest of the workspace's cache stale. 2. The item URL parser matched `/items/{slug}/grants`, `/items/{slug}/share-links`, etc. — owner-only subroutes that can 403 after a role downgrade while the item itself is still readable. Purging the item on those was incorrect. Both fixed by collapsing the scope to "the entire workspace": AccessRevokedScope is now `{ kind: 'workspace'; workspace }`; the parser just extracts the workspace slug from any `/workspaces/{ws}/...` path; the handler in +layout.svelte calls `localIndex.reset(ws)`. The per-item `findByIdOrSlug` and `removeByCollection` helpers added in round 0 stay in localIndex for future per-item revocation paths (e.g. server-side `unauthorized` SSE events), but the 403 path no longer uses them. Parent: PLAN-1343. * fix(web): scope 403 purge to read-model endpoints only (Codex round 3) [P1] Codex caught that workspace-scoped 403s aren't all workspace-access-revoked signals. Grant-only guests legitimately get 403 on /workspaces/{ws}/members, /workspaces/{ws}/storage/usage, etc., while their item read access is fine. The previous "any workspace-scoped 403 → reset" handler would wipe the local index on every such 403, leaving guest views stuck loading. Restrict parseAccessRevokedScope to the explicit read-model endpoints the local-first store actually consumes: GET /workspaces/{ws}/items GET /workspaces/{ws}/items/{idOrSlug} GET /workspaces/{ws}/items-index GET /workspaces/{ws}/items-changes GET /workspaces/{ws}/collections/{coll}/items A 403 on any of these means the cache is stale-by-permission. A 403 on anything else stays opaque to the local index. Parent: PLAN-1343. |
||
|
xarmian
|
54203087d0 |
feat(web): leader-elected SSE + cross-tab BroadcastChannel (TASK-1359) (#506)
* feat(web): leader-elected SSE + cross-tab BroadcastChannel (TASK-1359) Multiple tabs of the same workspace now share a single SSE connection via navigator.locks. Per DOC-1342 design decision #2: one leader holds the EventSource, peer tabs receive deltas via BroadcastChannel. - Each connecting tab opens a BroadcastChannel `pad-sync-{ws}` and races for an exclusive Web Lock on `pad-sse-leader-{ws}`. - The lock-holder opens the EventSource and forwards every event to local callbacks AND broadcasts to peer tabs. The browser's own-message filter keeps the leader from re-dispatching its own messages on the way back. - Peer tabs subscribe to the channel and dispatch leader-forwarded events to their local callbacks. They don't open EventSources while a leader exists, so the browser sees ONE /api/v1/events connection per workspace per browser regardless of tab count. - On leader-tab close, navigator.locks releases the lock automatically and a queued peer takes over (opens its own EventSource) without manual intervention. - Connection status is also broadcast so peer tabs surface the same connected / reconnecting / unauthorized indicator the leader sees. Fallback: browsers without navigator.locks (very old / non-browser environments) skip the election and every tab opens its own EventSource — N× traffic but correct. The public API (onItemEvent, onSyncRequired, status, etc.) is unchanged, so existing callers — sync.svelte, collection page — just work. Parent: PLAN-1343. See DOC-1342 design decision #2. * fix(web): gate BroadcastChannel on navigator.locks support (Codex round 1) [P1] When BroadcastChannel is available but navigator.locks is not, every tab falls back to per-tab EventSource AND opens the same shared channel. Each tab handles its local SSE event AND receives its peers' broadcasts — item callbacks fire N times across N tabs, producing duplicate toasts and refetches. Gate BC opening on `leaderElectionSupported()` (which checks for navigator.locks). Without leader election, the per-tab EventSource still delivers events locally; we just don't fan out — correct and avoids the N× duplication. Parent: PLAN-1343. * fix(web): leader promotion sync + BC close on fallback (Codex round 2) - [P1] When a peer tab is promoted to leader (the old leader's tab closed), the new EventSource opens with no Last-Event-ID so the server can't replay events from the gap. Query navigator.locks BEFORE requesting the lock; if the slot is already held, classify ourselves as a future "promotion" and fire sync_required on grant so consumers (syncService, the collection page) backfill via /items-changes. First leaders on fresh page loads don't fire — bootstrap already runs a sync — and any mis-classification is idempotent + cheap. - [P2] The lock-rejection fallback path now closes the BroadcastChannel before opening per-tab SSE. Without this, two tabs that both reject the lock would each open their own EventSource AND keep receiving each other's broadcasts, firing callbacks N times. Also triggers sync_required since we may have missed events while the rejection landed. Parent: PLAN-1343. * fix(web): grant-delay fallback for promoted-leader classification (round 3) [P1] Two tabs starting simultaneously can both query the lock state and see "no holder", so both set queryPromoted=false. One wins the request, the other queues. When the queued tab eventually gets promoted, queryPromoted=false would skip the sync_required, missing events from the gap between the old leader's close and the new EventSource. Add a grant-delay signal: record `performance.now()` before requesting, and treat any callback that fires more than 100ms later as a promotion. Uncontested lock grants are sub-millisecond; a queued tab takes at least the previous leader's full session. Final `promoted` is `queryPromoted || grantDelay > 100`, catching both the "slot-already-held-at-query-time" case AND the "simultaneous-startup-race" case. Parent: PLAN-1343. * fix(web): defer promoted-leader sync until EventSource connects (round 4) [P1] Promoted/fallback leaders called dispatchSyncRequired immediately after `new EventSource(...)`, before the replacement SSE stream was actually subscribed server-side. A mutation between the /items-changes snapshot (triggered by the sync) and the new stream's first received event could be missed by BOTH paths. Defer via a `pendingSyncOnConnect` flag set in the promotion / fallback paths; the EventSource's onopen and `connected` listeners fire the sync only after the stream is live. Whichever event arrives first claims the pending flag. Parent: PLAN-1343. |
||
|
xarmian
|
bd8667eadf |
feat: seq-stamped SSE events + stale-event short-circuit (TASK-1358) (#505)
* feat: seq-stamped SSE events + stale-event short-circuit (TASK-1358)
## Server
- events.Event gains a `Seq int64` field (omitempty). Server populates
it on item lifecycle events so SSE consumers can reason about
ordering and contiguity against their /items-changes cursor.
- publishItemEventWithName takes seq as a parameter; all call sites
in handlers_items.go pass the item's current seq:
- item_created → item.Seq
- item_updated → updated.Seq
- item_archived → re-fetched via GetItemIncludeDeleted (DeleteItem
bumps seq but doesn't return the updated row)
- item_restored → restored.Seq
- move target → moved.Seq
## Web
- ItemEvent gains optional `seq` (matches the server's omitempty).
- localIndex.classifySSEEvent(ws, event) returns
'no-seq' | 'stale' | 'contiguous' | 'gap'. The collection page
uses this to short-circuit duplicate / replayed events the
server's replay buffer re-delivers after tab-resume. Non-stale
events still call deltaSync because the SSE wire payload only
carries metadata, not the row data; classify does NOT advance
the cursor (applyDelta with real row data is the only path that
does — preserving the IDB invariant from TASK-1356).
Parent: PLAN-1343.
* fix(events): version-restore item_updated event carries seq (Codex round 1)
Version-restore in handlers_item_versions.go was publishing the
item_updated event directly via events.Publish without Seq, bypassing
the new seq-stamped SSE contract from TASK-1358. localIndex's
classifySSEEvent would always return 'no-seq' for those events,
forcing a generic /items-changes refetch instead of allowing the
stale/gap fast paths.
Now passes updated.Seq from the store response.
Parent: PLAN-1343.
|
||
|
xarmian
|
5fb85534fd |
feat(web): collection page reads from localIndex (TASK-1357) (#504)
* feat(web): collection page reads from localIndex (TASK-1357)
Wire the collection page (web/src/routes/[username]/[workspace]/[collection]/+page.svelte) to the local-first read model.
- `items` is now `$derived` from `localIndex.getByCollection(ws, coll, { includeArchived: showArchived })`. The collection page no longer fires `/items-index` on every nav — bootstrap is idempotent and runs once per workspace per session.
- New `bootstrap` `$effect` calls `localIndex.bootstrap(ws, { userId })` when the workspace or signed-in user changes, picking up the warm-IDB / cold-/items-index flow from PLAN-1343 Phase 2.
- Mutations (`handleStatusChange`, `handleReorder`, `handleRestore`, `quickCreate`) call `localIndex.upsert(ws, item)` with the canonical post-API row; the derived `items` re-renders automatically.
- SSE handler now triggers `/items-changes` → `applyDelta` via a new `deltaSync` helper instead of refetching the whole collection. SSE merely says "something changed"; the local cursor pulls only the delta. TASK-1358 will refine this to per-event seq-stamped apply.
- `syncService.onSync` routes both incremental and full-refresh signals through the same `deltaSync` path. The legacy /changes payload is no longer threaded into the local store — the seq-cursor /items-changes is canonical.
- The plans cross-collection lookup for task relation labels reads directly from `localIndex.getByCollection(ws, 'plans')` — no extra request.
Parent: PLAN-1343. Depends on TASK-1355 + TASK-1356.
* fix(web): collection page loading + reactive plan labels (Codex round 1)
- [P2] deltaSync() now returns a boolean and the syncService.onSync
handler only calls markSynced() on a clean catch-up. A transient
/items-changes failure leaves the legacy cursor untouched so a
later tab-resume retries instead of pinning at "fresh".
- [P2] `loading` is now derived from BOTH the metadata fetch
(metaLoading) AND the localIndex bootstrap state. Without this,
non-empty collections briefly rendered the empty-state CTA while
items were still hydrating, and scroll-restore could be consumed
against an empty filteredItems list.
- [P3] `relationLabels` (plan-id → plan-title for task cards) is
now `$derived` over `localIndex.getByCollection(ws, 'plans')`
instead of a one-shot fetch in loadCollection. Plans flow into
the local store as they hydrate, so the badge stays correct
without a navigation refresh.
Parent: PLAN-1343.
* fix(web): always deltaSync + gate archive toast on success (round 2)
- [P2] syncService.onSync now runs deltaSync for ALL result types,
including 'caught_up'. SSE only delivers events, not delta data;
a previous incremental deltaSync failure won't recover without
a fresh fetch attempt. The localIndex cursor is independent of
syncService.lastSyncTime, and per-row seq guards make repeated
calls idempotent.
- [P3] handleBulkArchive now waits for deltaSync to succeed before
showing a definitive success toast. The server-side deletes are
already persisted; if the cache fetch fails, surface a softer
"queued / updating…" toast so the user knows the local view will
catch up. The deletes themselves are still real.
Parent: PLAN-1343.
* fix(web): optimistic local sort_order on reorder (Codex round 3)
[P2] handleReorder now upserts the row into the local index with
the new sort_order BEFORE awaiting the API. Otherwise ListView,
which calls onReorder without awaiting, resyncs its displayed groups
from the unchanged `items` prop the moment dragging ends and the
rows snap back to the old order until the network PATCH returns.
Clearing `seq: undefined` on the optimistic copy bypasses the
per-row seq guard so the real API response (with a higher seq)
wins on arrival without the guard rejecting it.
Parent: PLAN-1343.
* fix(web): deltaSync 401/403 + error state on bootstrap failure (round 4)
- [P1] deltaSync now resets the local index on 401/403, mirroring
the auth-error handling in localIndex.bootstrap. If workspace
access is revoked after the page is mounted, the cached rows
drop instead of staying visible until reload. Other errors stay
transient.
- [P2] localIndex.bootstrapState === 'error' is no longer
conflated with 'ready'. A new `indexError` derived gates a
dedicated error-state branch in the template with a Retry CTA;
the misleading "No items yet" empty state no longer fires on a
transient /items-index failure for a non-empty collection.
Parent: PLAN-1343.
* fix(web): deltaSync on page entry + error surface after revoke (round 5)
- [P1] The bootstrap effect now ALWAYS runs a deltaSync after the
bootstrap promise settles. Once localIndex is 'ready', bootstrap
itself no-ops — but an item the user created/updated elsewhere
(item detail page, dashboard, another tab) while this collection
was unmounted is still catchable via /items-changes. Without
this, returning to the collection page after creating an item
elsewhere could miss the new row until the next SSE event.
- [P2] After a 401/403 from /items-changes, deltaSync now sets a
`deltaSyncFailed` flag in addition to calling localIndex.reset.
The reset rolls bootstrapState back to 'cold' but the bootstrap
effect can't re-fire on the same wsSlug/userId, so without the
flag the page would pin at "Loading…" forever. The error-state
banner now triggers on EITHER indexError OR deltaSyncFailed and
the Retry CTA clears the flag before re-bootstrapping.
Parent: PLAN-1343.
|
||
|
xarmian
|
13fd9bbda7 |
feat(web): IndexedDB persistence for localIndex (TASK-1356) (#503)
* feat(web): IndexedDB persistence for localIndex (TASK-1356)
Adds `web/src/lib/stores/localIndexPersistence.ts` and wires it into
the existing localIndex store. Cold loads still hit /items-index;
warm loads paint from IDB before any network IO.
- New `idb` (8.0.3) dependency — small wrapper around IndexedDB.
- Per-workspace database `pad-local-index-{wsSlug}` with two object
stores (`items` keyed by id, `meta` keyed by 'key' for cursor +
schemaVersion).
- `LOCAL_INDEX_SCHEMA_VERSION = 1` — bump it on incompatible changes
to `ItemIndexRow` or the IDB layout; mismatches drop the store and
force a full /items-index resync. Same pattern as the Yjs
schemaVersion in `web/src/lib/collab/schemaVersion.ts`.
- `bootstrap` now hydrates from IDB FIRST (paints from cache, flips
state to 'ready'), then reconciles via /items-changes?since=cursor
in the background. Cold cache falls through to /items-index and
persists the result.
- Every mutation path (`upsert`, `applyDelta`, `remove`, `reset`)
writes through to IDB. Persistence failures degrade silently to
in-memory only — the read path is never blocked.
- SSR-safe (every IDB call gated on typeof indexedDB !== 'undefined').
- Best-effort: Safari private mode / quota / eviction all surface
as "empty cache, re-bootstrap from network".
Phase 2 acceptance: warm paint of a populated workspace should now
appear before any /api/v1 request completes.
Parent: PLAN-1343. See DOC-1342 design decision #4.
* fix(web): localIndex reconcile loop + atomic delta persist (Codex round 1)
- [P1] 403 from the warm-load reconcile is no longer swallowed.
When /items-changes returns `forbidden`, drop the cache and
re-throw so the registered access-revoked handler (TASK-1360)
sees it. Other network blips remain non-fatal — cache stands
and the next reconnect retries.
- [P2] /items-changes is paged at DefaultItemChangesLimit (5000)
per response. The previous one-shot reconcile would only catch
up by a single page on a long-offline cache, and bootstrapState
would pin at 'ready' forever with no later trigger to fetch the
rest. Loop until the cursor stops advancing; defensively cap at
50 iterations.
- [P2] New persistDelta() writes rows + meta cursor in a SINGLE
IDB transaction. The previous separate persistUpserts + persistCursor
could persist the cursor without the rows that produced it
(tx interrupt, eviction), leaving the next warm hydrate with a
cursor that skipped rows. applyDelta + the cold-path bootstrap
snapshot now both use persistDelta. The standalone persistCursor
helper is no longer used by localIndex but remains for callers
that explicitly only need a cursor write.
Parent: PLAN-1343.
* fix(web): cold-path snapshot persists post-merge rows (Codex round 2)
[P1] When the cold-path `/items-index` request is in flight, an SSE
or `applyDelta` write can overlap and stamp a newer row into the
in-RAM index. `mergeRow` correctly skips the stale response row for
that id, but the previous IDB write used `resp.items.map(toSkinny)`
— the unfiltered server response — so the cache got the stale row
under the newer cursor. On the next warm boot, /items-changes?since
would skip that row forever.
Persist the POST-merge in-memory state (state.items.values()) so
the on-disk rows match the in-RAM rows that won the seq guard, and
the cursor stays consistent with them. Uses the same atomic
persistDelta path applyDelta does.
Parent: PLAN-1343.
* fix(web): generation guard on bootstrap + user-scoped IDB cache (round 3)
- [P1] WorkspaceState now carries a `generation` counter. Each
`reset(ws)` bumps it on the prior state object before dropping
the workspace from the map. Any in-flight bootstrap captures the
generation at start and re-checks after every await — if the
generation has advanced, the bootstrap silently bails out before
reapplying rows or writing the snapshot to IDB. Without this, a
sign-out / 403 purge during a slow /items-index could let the
completed snapshot resurrect just-purged rows.
- [P1] IDB databases are now keyed by (userId, workspaceSlug)
instead of workspaceSlug alone. The cache is "what THIS user could
see last sync" — if a different user signs into the same browser,
their bootstrap opens a fresh per-user namespace and the previous
user's rows never surface. Anonymous callers (pre-auth) use the
`anon` namespace. `localIndex.bootstrap` takes an `{ userId }`
opt the caller passes in (the workspace state captures it on
first bootstrap and threads it through every persistence call).
Parent: PLAN-1343.
* fix(web): durable applyDelta + required userId opt (Codex round 4)
- applyDelta now ALWAYS includes the existing in-RAM row in the
persistDelta batch when it wins the seq guard. The previous version
advanced the IDB cursor past those rows on the assumption their
upsert()-fired persistUpserts had already landed — but that's a
fire-and-forget background write that can lose the race or get
aborted. Result: a row in RAM with seq S, no copy in IDB, and a
persisted cursor of N > S — warm boot would skip it forever. One
extra IDB put per redundant row trades cheaply against a missing-
row class of bug.
- localIndex.bootstrap's `opts.userId` is now REQUIRED (not optional
with `null` default). Authenticated callers that forget to pass
it would have silently landed their cache in the shared `anon`
namespace — a later account on the same browser could then read
the previous account's rows. TypeScript now enforces an explicit
choice; pre-auth callers pass null deliberately.
Parent: PLAN-1343.
* fix(web): user-mismatch reset + transient resync retry (Codex round 5)
- [P1] bootstrap() now resets the workspace state BEFORE the
early-return for 'ready' / pending-promise when the caller's
opts.userId doesn't match the cached state.userId. Without this,
a user switch in the same tab could inherit the previous user's
in-memory map and in-flight promise.
- [P2] WorkspaceState.pendingResync tracks transient delta-sync
failures on the warm path. When /items-changes fails (non-403)
after warm cache hydrate, bootstrapState stays 'ready' so the
UI keeps working off the cache, but pendingResync stays true and
the next bootstrap() call retries the reconcile instead of
no-opping. Cold path always finishes with pendingResync=false.
- Documented the permission-revocation-without-row-change limitation
inline: the cache can't see grants removed without a mutation,
per DOC-1342 design decision #3 — that's the 403-on-click purge
flow (TASK-1360), not this layer's job.
Parent: PLAN-1343.
* fix(web): only clear pendingResync when reconcile catches up (round 6)
[P2] The /items-changes reconcile loop has a 50-page safety cap to
prevent pathological tight loops. The previous code unconditionally
cleared `pendingResync` after the loop exited, even on cap-hit, so
a cache that's 50+ pages behind (250k+ rows) would record itself as
fresh and skip retries on future bootstraps. Now `pendingResync` is
only cleared when the loop exited because the server returned no new
rows AND no cursor advance — the genuine "caught up" signal. Cap-hit
leaves `pendingResync = true` so the next bootstrap call resumes.
The permission-revocation-without-row-change concern Codex re-raised
is the explicit DOC-1342 design decision #3 (best-effort cache; 403
purge handles stale-by-permission). The server emits grant-revocation
tombstones through /items-changes per internal/store/grants.go, so the
cache reconciles to-server-truth at the next reconnect. Anything that
slips past that is the 403-on-click purge path (TASK-1360). The
limitation is now explicitly noted inline.
Parent: PLAN-1343.
* fix(web): reentry order + identity-checked inflight cleanup (round 7)
- [P2] Capture `reentry` BEFORE flipping bootstrapState to 'loading'.
The previous version set state='loading' first, then checked
`state.bootstrapState === 'ready'` to decide if we're in a
pendingResync retry — that check always read false, so retries
re-read IDB instead of just rerunning the reconcile. With
fire-and-forget IDB writes, re-reading rows whose RAM copy was
just removed but whose IDB delete hadn't landed would resurrect
them. Reentry now also skips the 'loading' flip so the UI never
blanks during a retry.
- [P2] Inflight cleanup is now identity-checked. A `reset()` during
an in-flight bootstrap can let a fresh bootstrap call re-occupy
the inflight slot before the stale promise's `finally` runs;
deleting unconditionally would remove the new entry and let a
duplicate bootstrap start. We hold the promise in `slot.p` (a
shared object so the closure can see assignment without TDZ
issues), and only clear `inflight.delete(ws)` if `slot.p` is
still the registered promise.
Parent: PLAN-1343.
* fix(web): 401 + empty-cache warm-load (Codex round 8)
- [P1] 401 (unauthorized) from /items-changes reconcile is now
treated like 403 (forbidden): drop the cache, mark state=error,
re-throw. The api.items.changes path throws PadApiError with
code='unauthorized' on 401 (after the redirect-to-login is fired),
so the cache shouldn't keep showing private rows while the
redirect is in flight. Other network failures remain transient.
- [P2] A populated IDB cache is now defined as "has rows OR cursor
> 0", not "has rows". An empty workspace (or a guest with
item-level grants but no items granted yet) legitimately has zero
rows but a real meta cursor from the prior sync. The previous
check forced those workspaces through the cold /items-index path
on every page load, defeating the warm-load fast path.
Parent: PLAN-1343.
|
||
|
xarmian
|
979537e4bb |
feat(web): localIndex in-RAM canonical store (TASK-1355) (#495)
* feat(web): localIndex in-RAM canonical store (TASK-1355) New Svelte 5 module at web/src/lib/stores/localIndex.svelte.ts that owns the in-RAM truth for the local-first read model. Per DOC-1342 decision #4: the store is canonical; IndexedDB persistence (next task) is hydration + write-behind only. Per-workspace state in a Map keyed by workspace slug: - items: SvelteMap<itemId, ItemIndexRow> — keyed by item.id - cursor: monotonic seq cursor as opaque decimal string - bootstrapState: 'cold' | 'loading' | 'ready' | 'error' Public API: - bootstrap(ws): idempotent /items-index hydration (in-flight coalescing) - getByCollection(ws, collSlug): synchronous filtered read - applyDelta(ws, changes, cursor): batch upsert/remove + cursor advance - upsert(ws, row): single-item write for SSE/optimistic paths - remove(ws, id): single-item delete for SSE archive + 403 purge - cursorFor(ws) / bootstrapStateFor(ws): reactive getters - reset(ws): drop all state for a workspace Defensively strips Item.content on every ingest so a caller passing a full Item (e.g. from api.items.update) cannot leak the rich body into the local index — matches the destructure-by-rest pattern in api.items.listIndex / changes. Parent: PLAN-1343. * fix(web): localIndex reactivity + stale-batch guards per Codex review (round 1) - [P1] WorkspaceState is now a class with `$state` class fields for `cursor` and `bootstrapState`. Svelte 5 only permits `$state()` at variable-initializer / class-field / constructor-first-assign sites — the previous `state = $state({...})` inside `ensureState` silently produced a non-reactive object on first hydration, so `bootstrapStateFor` getters could stay 'cold' through 'loading' / 'ready' transitions. Class-field runes give us the same shape with reactivity intact. - [P1] Documented that the store intentionally holds both live and archived rows. `applyDelta` only removes on the soft-delete `deleted: true` tombstone — status='archived' rows stay so a later `showArchived` toggle on the consumer doesn't need a refetch. Intended consumer pattern (TASK-1357) filters on `fields.status` at render time. - [P2] `applyDelta` now drops the whole batch when `newCursor` does not strictly advance, AND skips individual rows whose `seq` is not greater than the cursor at the start of the call. In normal /items-changes flow the server filters to `seq > since`, but the per-row guard prevents test or future replay callers from overwriting newer state with older rows. Parent: PLAN-1343. * fix(web): localIndex archive filter + per-row seq guard (Codex round 2) - [P1] `Item.deleted_at` was missing from the TS interface even though the server populates it (`Item.DeletedAt *time.Time` with omitempty). Added the field to `Item`, which flows into `ItemIndexRow` via the existing `Omit<Item, 'content'>` mapping. - [P1] `getByCollection(ws, collSlug)` now filters soft-deleted rows out by default. The store still holds them (so a `showArchived` toggle doesn't need a refetch) but the default view is live-only, matching every other collection consumer in the codebase. Callers that want archived rows pass `{ includeArchived: true }`. Updated the module-level docstring: archived = `deleted_at` set (not `fields.status`), and clarified the upsert-vs-delete split on the change wire format (`deleted: true` = hard tombstone; soft deletes arrive as upserts with `deleted_at` populated). - [P2] `applyDelta` per-row check now compares against BOTH the cursor floor at start AND the existing row's `seq`. Without the second check, a delta that legitimately advances the cursor could still carry a row whose `seq` is older than what we already hold for that id (since `upsert` / SSE paths can store newer rows without touching the cursor). Parent: PLAN-1343. * fix(web): preserve soft-deleted rows + upsert seq guard (Codex round 3) - [P1] /items-changes sets `deleted: true` for soft-deleted rows (the server's derived view of `deleted_at != nil`), not for hard tombstones. The previous applyDelta removed those rows, defeating the store's stated invariant that archived items remain queryable via `getByCollection(..., { includeArchived: true })`. Now applyDelta always upserts on a change — the soft-deleted row keeps its skinny payload (with `deleted_at`) and falls out of the default filter but stays in the index. Hard deletes still flow through `remove()`. - [P2] `upsert` now mirrors `applyDelta`'s per-row seq guard: skip the write if the incoming row's `seq` is not strictly greater than the existing row's `seq`. Without this, a late SSE / out-of-order optimistic response could regress a row after a fresher version had already landed. Parent: PLAN-1343. * fix: items-index returns deleted_at + bootstrap merges instead of clears (round 4) - [P1] `/items-index` projection now selects `i.deleted_at` and `scanItemsIndex` populates `Item.DeletedAt`. Without this the local-first client could not distinguish archived rows from live ones, so `localIndex.getByCollection`'s default live-only filter would surface archived rows as live. Mirrors the projection of ListItemsChangesSince which has always carried this column. - [P2] `localIndex.bootstrap` now merges into the existing state using the same per-row `seq` guard as `upsert`/`applyDelta`, and the cursor only advances forward. The previous clear-and-replace could regress rows that an in-flight `upsert()` or SSE-driven write had landed during the bootstrap request, and could reset the cursor below an SSE delta that advanced it concurrently. Explicit "drop everything" still flows through `reset()`. Parent: PLAN-1343. * fix(web): localIndex.getByCollection sorts updated_at DESC, id ASC (round 5) [P2] SvelteMap iterates in insertion order; live upserts and applyDelta writes appended rows / kept stale positions, so consumers reading from getByCollection saw an order that drifted away from the server's /items-index documented `updated_at DESC, id ASC`. Sort on read so the collection page sees a stable, server-aligned order regardless of how recently a row arrived through the in-RAM index. The cost is O(n log n) per read; the consumer side is expected to memoize via $derived. Parent: PLAN-1343. |
||
|
xarmian
|
a5b93c17c9 |
feat(api): add /items-changes?since=<seq> delta endpoint (TASK-1354) (#494)
* feat(api): add /items-changes?since=<seq> delta endpoint (TASK-1354) Adds the delta-fetch sibling of /items-index for the local-first read model (PLAN-1343 / DOC-1342 design decision #1). Clients track the workspace-scoped monotonic seq cursor returned by /items-index (TASK-1353) and poll /items-changes?since=<cursor> to apply just the rows that have mutated — without re-downloading the entire workspace. ## Endpoint GET /api/v1/workspaces/{ws}/items-changes?since=<seq>&limit=<n> - `since`: exclusive seq lower bound (returns `seq > since`). Defaults to 0 → full delta == /items-index modulo ordering. Bad input → 400. - `limit`: cap on rows. Defaults to 5000, clamped to 50000. Bad input → 400. ## Response { "changes": [...skinny rows with `deleted: bool`...], "cursor": "<decimal MAX(seq) or unchanged since when empty>" } Soft-deleted rows propagate (no `deleted_at IS NULL` filter on the backing scan) so a delta consumer can remove them from its local index without a second roundtrip. Parent metadata enrichment matches /items-index: the underlying GetItem filters soft-deleted parents so we never leak parent title/ref for an archived parent. Cursor contract: - Sorted ASC by seq → re-passing the response's cursor as `since` on the next poll is no-overlap, no-gap (strictly monotonic seq invariant from TASK-1352). - Empty response preserves the caller's `since` so position isn't lost. - Truncated-by-limit responses set cursor to the last row's seq. ## Tests - FullDeltaFromZero — three creates, since=0, ascending seq, every row deleted=false, cursor=MAX(seq). - IncrementalUpdateAndDelete — typical resume flow: snapshot cursor, mutate, delta returns exactly the mutated + tombstoned rows with the right `deleted` flag. - CursorRoundtripsCleanly — empty-poll after consuming, cursor preserved. - LimitTruncatesAndCursorResumes — paging contract holds end to end with no overlap. - InvalidParams — bad since / limit values rejected with 400. - EmptyWorkspace — cursor round-trips caller's since unchanged. ## Web TypeScript: `ItemChangeRow = ItemIndexRow & { deleted: boolean }`, `ItemChangesResponse = { changes, cursor }`. API client gains `api.items.changes(ws, sinceCursor, opts?)` with the same defensive content-strip as listIndex so a stray `content: ""` key from a Go zero-value can never clobber the canonical store. Parent: PLAN-1343. Depends on TASK-1352 (seq column) and TASK-1353 (seq cursor on /items-index). Unblocks the future client-side localIndex.applyDelta integration task. * fix(api): surface tombstones for item-grant users in /items-changes per Codex review (round 1) Codex round 1 caught that handleListItemsChanges was building its ItemIDs filter from guestResourceFilter, which itself uses GuestVisibleResources whose item-grant query filters out soft-deleted items. The result: a guest or restricted member with an item-level grant on a single item would see that ID disappear from the lookup as soon as the item was soft-deleted — and /items-changes would never emit a `deleted:true` tombstone, so the client would keep the stale row in its local index forever. Fix: - New Store.GuestVisibleResourcesIncludeDeleted that drops the `i.deleted_at IS NULL` / `c.deleted_at IS NULL` filters on both collection and item grants so tombstone IDs flow through. - New Server.guestResourceFilterIncludeDeletedItems delegate pointing at the new store helper. Implementation is shared with the live variant via guestResourceFilterCore so the member-collection-access + system-collection merge logic stays in one place. - handleListItemsChanges swaps to the include-deleted variant. Test: TestGuestVisibleResourcesIncludeDeleted_SurfacesTombstones covers both variants side-by-side — live drops the soft-deleted grant, include-deleted preserves it. * fix(store): assign per-row unique seqs in MigrateItemFieldValues per Codex review (round 2) Codex round 2 caught that the bulk UPDATE inside MigrateItemFieldValues gave every affected row the SAME MAX(seq)+1. A /items-changes?limit=N poll that cut through that equal-seq group would advance the cursor to the shared seq, and the next `seq > cursor` poll would silently miss the rest of the group — the cursor contract requires strict monotonicity. Switched to a per-row loop inside the migration transaction so every UPDATE re-reads MAX(seq) and each affected row ends up with a strictly unique seq. The workspace advisory lock makes the read-modify-write race-free on Postgres; SQLite's single-writer rule handles it implicitly. Trade-off: O(N) statements instead of O(1) for the bulk path. Option-rename is an admin one-off so the cost is acceptable (~1s/1000 rows on a warm SQLite connection). If future use cases demand a larger row budget, a single-statement UPDATE..FROM with ROW_NUMBER() CTE assigning per-row seqs would also work. Test: TestMigrateItemFieldValues_PerRowUniqueSeq confirms 5 rows in a single migration step all get unique seqs. |
||
|
xarmian
|
974472799a |
feat(api): wire real workspace seq into /items-index cursor + rows (TASK-1353) (#493)
* feat(api): wire real workspace seq into /items-index cursor + rows (TASK-1353)
Replaces the placeholder `updated_at`-derived cursor on the
/items-index response with the real workspace-scoped MAX(seq)
introduced by TASK-1352. Each returned row carries its own `seq`
field so clients can reason about ordering without parsing the
cursor.
When the requested scope returns zero rows but the workspace has
items (e.g. ?collection=docs on a workspace whose docs collection
is empty but whose tasks/ideas are not), the cursor falls back to
the workspace's true MAX(seq) via a new Store.MaxItemSeq helper.
That way the client's next /items-changes?since=cursor poll starts
at the right floor instead of replaying every prior mutation from 0.
Empty workspaces collapse to "0".
Encoding: cursor is the decimal-encoded MAX(seq). Treated as opaque
on the wire (clients re-pass it as ?since=). String form leaves
room to switch to base32/etc later without an API break.
TypeScript: `ItemIndexRow` (via `Item`) adds optional `seq?: number`;
`ItemIndexResponse.cursor` docstring updated to reflect the real
seq cursor semantics. `api.items.listIndex` docstring updated.
Tests:
- TestListItemsIndex_SkinnyProjectionAndShape: cursor now asserts
decimal-encoded MAX(seq); per-row seq is non-zero.
- TestListItemsIndex_EmptyResultFallsBackToWorkspaceMax: new test
covering the cursor fallback on filtered-but-empty results.
- TestListItemsIndex_CursorMonotonicAcrossMutations: new test
confirming cursor advances after every mutation.
Parent: PLAN-1343. Depends on TASK-1352 (seq column). Unblocks
TASK-1354 (/items-changes endpoint).
* fix(api): snapshot workspace MAX(seq) before list to close cursor race per Codex review (round 1)
Codex round 1 caught a real race in /items-index cursor computation:
ListItemsIndex ran first, then MaxItemSeq ran in a separate query.
A concurrent INSERT visible to a future /items-changes call could
land between them — the response would be `items: []` with cursor =
the new seq, and a subsequent /items-changes?since=cursor poll
(seq > cursor) would never return that row.
Fix: capture MaxItemSeq BEFORE the list query. Per the workspace's
monotonic counter invariant (TASK-1352) any insert after that
snapshot has seq > captured M, so /items-changes?since=M will see
it. Rows the list DOES observe may have seq > M (a concurrent
insert the list query happened to commit-snapshot); MAX(rows.seq)
bumps the cursor for that case so the client never re-fetches what
was already in the response.
Long-form comment on the handler captures the race scenario and the
invariant that makes the snapshot order safe.
|
||
|
xarmian
|
7456b5aed6 |
feat(store): add workspace-scoped monotonic seq column to items (TASK-1352) (#492)
* feat(store): add workspace-scoped monotonic seq column to items (TASK-1352) Adds an `items.seq` column that bumps on every mutation (create/update/soft-delete/restore) as the cursor mechanic for the local-first read model's delta sync (PLAN-1343, DOC-1342 design decision #1). Each mutation stamps `MAX(seq) + 1 WHERE workspace_id = ?` inside the same transaction that performs the write, with a Postgres advisory lock keyed on the workspace serializing concurrent seq-bumping mutations. SQLite's single-writer rule covers the same guarantee there. Migration backfills existing rows with sequential per-workspace seqs in (updated_at, id) order so every workspace has a non-zero MAX(seq) floor immediately. Adds an idx_items_workspace_seq index supporting both the `/items-index` cursor read and the future `/items-changes` range scan. The Seq field is now populated through every items SELECT helper (GetItem, GetItemIncludeDeleted, ListItems, ListItemsIndex, listItemsFTS, SearchItems, ItemsModifiedSince, GetChildItems, ListStarredItems, ResolveItemIncludeDeleted, GetItemBySlugIncludeDeleted) and the workspace import path stamps it via the same MAX+1 subquery so imported rows don't all collapse to seq=0. Parent: PLAN-1343. Foundation for TASK-1353 (wire seq into /items-index cursor) and TASK-1354 (/items-changes delta endpoint). * fix(store): bump items.seq on role reorder, MoveItem, and field migrations per Codex review (round 1) Codex round 1 flagged that UpdateRoleSortOrder was rewriting items.role_sort_order without bumping the new workspace-scoped seq column — delta-sync clients would miss role-board reorders until a full refresh. The same gap applied to MoveItem (collection change) and MigrateItemFieldValues (bulk select-option rename), which are also user-visible mutations the cursor must surface. Each path now: - acquires the workspace seq advisory lock (no-op on SQLite) - stamps seq = MAX(seq)+1 inside the same transaction The bulk rename gives all rows affected by a single statement the same seq value (MAX+1 at statement start). That preserves the "no overlap, no gap" cursor contract — a client at cursor < MAX sees them all in one batch, at cursor >= MAX sees none. |
||
|
xarmian
|
d6894def4f |
feat(web): collection page fetches via skinny /items-index endpoint (TASK-1349) (#491)
* feat(web): collection page fetches via skinny /items-index endpoint (TASK-1349)
Replaces every \`api.items.listByCollection(ws, coll)\` call in the
collection page with \`fetchSkinnyItems(ws, coll, includeArchived)\`,
which calls the local-first \`/items-index\` endpoint (TASK-1344)
through the typed client wrapper (TASK-1345). Items now ship
without the rich-text \`content\` body — the bulk of the per-row
wire size — until the user opens an item detail page, which still
goes through its existing full-item fetch.
Call sites updated:
- loadCollection — primary load + plans-names lookup
- SSE handler for item_created / item_archived / item_restored / item_updated
- Sync coordinator's full-refresh fallback
The skinny rows are widened to \`Item[]\` at the boundary by setting
\`content: ''\` on each row. This keeps the existing view component
type contract unchanged and means existing call sites that read
\`item.content\` see an empty string — already a "nothing to do"
sentinel in the markdown-checklist progress branch.
Documented regression — out of scope for this task: non-plans
collections used to display checklist progress derived from item
content's markdown checkboxes. With \`content\` no longer fetched
for the list view, that progress no longer appears. Plans
progress is unaffected (uses /plans-progress, not content
parsing). Re-introducing the feature requires either server-side
progress on the index endpoint or a separate lazy fetch — a
follow-up rather than a blocker for the bandwidth win.
In-scope behavior preserved:
- Item create/update flow: server still returns full items, dropped
into the array as-is; sync coordinator's incremental updates
similarly use the full-item type from the changes feed
- Server-side FTS search via \`searchResultIds\`: still id-keyed,
works against skinny rows
- List / Board / Table view components: already only read fields
present on the skinny row (title, fields, tags, sort_order…)
- Detail page fetch: unchanged — still goes through
\`api.items.get\` which returns the full Item with content
Parent: PLAN-1343.
* fix(api+web): add /collections/{coll}/checkbox-progress endpoint to preserve list-view checklist progress per Codex review (round 1)
Codex round 1 [P2] flagged that the original PR shipped a real
regression: non-plans collections used to compute markdown-checkbox
progress client-side from `item.content`, and the skinny
`/items-index` endpoint dropped `content` from the payload — so
list/board/table progress badges silently stopped appearing on
docs/tasks/custom collections.
This commit closes that gap with a new server endpoint that
computes the same `{item_id, total, done}` counts via
LENGTH/REPLACE arithmetic on the stored content, returning only
the small derived counts. No item bodies cross the wire.
Server (Go):
- `store.CollectionCheckboxProgress(workspaceID, collectionID)` —
SQL: `(LENGTH(content) - LENGTH(REPLACE(content, '- [ ]', '')))
/ 5 + (LENGTH(content) - LENGTH(REPLACE(content, '- [x]', '')))
/ 5` for total, the second clause alone for done. Same trick on
SQLite and PostgreSQL.
- `handleCollectionCheckboxProgress` — collection-visibility +
item-grant filter so guests / restricted members can't enumerate
items they shouldn't see. Mirrors `guestResourceFilter` exactly.
- Route: `GET /api/v1/workspaces/{ws}/collections/{coll}/checkbox-progress`.
- Test `TestCollectionCheckboxProgress` covers the math (open +
done counts), zero-result rows are filtered, unknown collection
→ 404, empty result → 200 + `[]`.
Web:
- `api.items.collectionCheckboxProgress(ws, coll)`
- Both call sites in `+page.svelte` (initial `loadCollection`
non-plans branch + `refreshProgress` non-plans branch) now
pull from the endpoint instead of parsing `item.content`.
- Drops the previous "documented regression" comment — the
feature is fully preserved.
Sub-100-byte response per item (vs. the full content body) so the
bandwidth win from `/items-index` is preserved. The endpoint scans
content server-side, but doesn't transmit it — the original
listByCollection call both scanned AND transmitted content.
Parent: PLAN-1343.
* fix(api+web): plumb include_archived through checkbox-progress per Codex review (round 2)
Codex round 2 [P2] caught that the Archived toggle path lost
checklist progress badges: `CollectionCheckboxProgress` hard-coded
`deleted_at IS NULL`, but the page-side fetch is called with the
same `showArchived` flag that toggles whether archived items
render. With the toggle on, archived non-plan items appeared in
the list but had no `itemProgress` row — the old client-side parse
would have counted them.
Fix: thread `includeArchived` through the call chain.
- store.CollectionCheckboxProgress(workspaceID, collectionID,
includeArchived bool) — appends `AND deleted_at IS NULL` only
when includeArchived is false. Default match the original
archived-off behavior.
- handleCollectionCheckboxProgress reads
?include_archived=true and forwards.
- api.items.collectionCheckboxProgress(ws, coll, { includeArchived })
on the client.
- +page.svelte's two call sites pass `showArchived` /
`includeArchived` exactly.
TestCollectionCheckboxProgress now archives one of the seeded
items and asserts:
- default response excludes the archived item (1 row)
- ?include_archived=true response includes it (2 rows)
Also clarified the const-doc on `checkboxCountSQL` to reflect the
dynamic deleted-at clause.
|
||
|
xarmian
|
f699d3480e |
feat(web): virtualize TableView rows via content-visibility (TASK-1348) (#490)
* feat(web): virtualize TableView rows via content-visibility (TASK-1348)
Flat-row variant of the approach landed for ListView in TASK-1346
and BoardView in TASK-1347. Adds
\`content-visibility: auto; contain-intrinsic-size: auto 36px;\`
to \`tbody tr\` so the browser skips layout/style/paint work for
rows that have scrolled out of the table's viewport.
CSS Containment L2 §4.4 historically treated layout/paint
containment as a no-op on table-row elements, but Chrome 122+
(March 2024) and follow-on Firefox / Safari releases lifted that
limitation for content-visibility specifically. The rule is
therefore opportunistic: modern browsers get virtualization,
older engines treat it as a no-op and render unchanged.
Preserved behavior:
- Sticky thead header (position: sticky; top: 0; on <th>) lives
in <thead> and is unaffected by per-row paint skipping.
- Column sort (toggleSort()) lives entirely in <thead> button
handlers — also untouched.
- No DnD on this view, so no drop-target preservation work.
- No protruding badges or absolute-positioned overflow content
inside rows, so no overflow-clip-margin escape hatch needed
(cf. PR #489's pr-badge handling on BoardView).
\`contain-intrinsic-size: auto 36px\` matches the actual data-row
height (var(--space-2) padding × 2 + ~20px line-height). The
\`auto\` keyword caches measured heights so rows with progress
bars or wrapped titles keep their natural sizing on re-entry.
Parent: PLAN-1343.
* fix(web): refactor TableView to CSS Grid so content-visibility actually applies per Codex review (round 1)
Codex round 1 [P2] correctly flagged that `content-visibility: auto`
on `<tr>` is a no-op: CSS Containment L2 §4.4 makes layout/paint
containment inactive on internal table boxes, and content-visibility
depends on size containment which is also no-op for table rows. So
the original PR shipped CSS that did nothing — the table-row branch
of CSS Containment defeats the trick that worked for ListView (PR
#488) and BoardView (PR #489).
Refactor: replace `<table>/<tr>/<td>` with `<div role="table">` /
`<div role="row">` / `<div role="cell">` and lay them out with CSS
Grid + subgrid. ARIA roles preserve assistive-tech semantics. Each
row is no longer an "internal table box," so content-visibility +
size containment apply normally.
The grid template is built dynamically because visibleFields depends
on the collection schema:
grid-template-columns: 70px minmax(200px, 1fr) auto* 90px
Each row sets `grid-template-columns: subgrid; grid-column: 1 / -1;`
so cells align across rows perfectly. Subgrid lands in Chrome 117+ /
Firefox 71+ / Safari 16+; an `@supports not (subgrid)` fallback
inherits the parent's grid template instead.
Preserved behavior:
- Sticky header — `.table-header { position: sticky; top: 0; }` is
on the first row (no `<thead>` anymore, but the role is the same).
- Column sort — `toggleSort()` logic unchanged.
- Column widths — fixed Ref (70px) / Updated (90px) bracket
minmax title + auto fields, matching the pre-refactor layout.
- Progress bar inside title cell — `.col-title` uses
`flex-direction: column` to stack title + progress.
- Hover state — `.table-row:not(.table-header):hover` keeps the
row-level hover behavior.
Virtualization rule (the actual point of the PR):
.table-row:not(.table-header) {
content-visibility: auto;
contain-intrinsic-size: auto 36px;
}
The header is excluded so sticky positioning isn't fought by paint
skipping.
Parent: PLAN-1343.
|
||
|
xarmian
|
9768655103 |
feat(web): virtualize BoardView cards via content-visibility (TASK-1347) (#489)
* feat(web): virtualize BoardView cards via content-visibility (TASK-1347)
Per-column virtualization for the kanban board, mirroring the
approach landed for ListView in TASK-1346. Adds
`content-visibility: auto; contain-intrinsic-size: auto 80px;` to
`.card-wrapper` so the browser skips layout/style/paint work for
cards that have scrolled out of their column's viewport.
`.column-cards` is itself `overflow-y: auto`, so content-visibility's
near-viewport check uses the column as its frame — naturally
per-column. Cards stay mounted so:
- svelte-dnd-action keeps every drop target in the DOM for
drag-between-columns + drop-into-empty-column hit-testing
- column horizontal scroll + column reorder (native HTML5 DnD on
`.kanban-column`) are unaffected
- keyboard focus on an off-screen card still resolves via
querySelector and scrollIntoView rehydrates paint
Intrinsic size is `80px` (vs. ListView's `60px`) because board cards
render with `compact={true}` — they stack status + tags taller than
the list row's single-line layout. The `auto` keyword caches the
real measured height after first paint so subsequent scrolls don't
reflow.
Parent: PLAN-1343.
* fix(web): explicit overflow:visible + spec citation on board card-wrapper per Codex review (round 1)
Codex round 1 [P2] worried that `content-visibility: auto` on
`.card-wrapper` would apply paint containment that clips ItemCard's
`.pr-badge` (positioned at `right: -6px`, deliberately protruding
past the card's right edge).
Per CSS Containment Module Level 2 §4 ("content-visibility"),
`content-visibility: auto` applies paint containment ONLY when the
element is "not relevant to the user" — off-screen, when the badge
isn't being painted anyway. On-screen elements receive only layout
containment, which does not clip ink overflow.
Even so, declaring `overflow: visible` explicitly is the cheapest
defense against a future style sweep that might silently add
`overflow: hidden` to wrappers, and pairs naturally with the
in-source comment citing the spec. Codex's concern is now
documented, addressed, and auditable.
No behavior change for spec-compliant browsers — the badge
already rendered correctly on-screen. The explicit declaration
makes the contract self-describing.
* fix(web): use overflow-clip-margin:6px to preserve PR badge protrusion per Codex review (round 2)
Codex round 2 [P2] correctly pushed back on round 1's spec reading:
per CSS Containment L2 §3.4 / §4, `content-visibility: auto` applies
paint containment continuously (including on-screen), and paint
containment clips ink overflow regardless of an explicit
`overflow: visible` declaration (overflow:visible is treated like
overflow:clip at used-value time when paint containment is active).
The proper fix is `overflow-clip-margin: 6px` — a CSS property
specifically designed to extend the paint-clip rectangle past the
element's content box by a fixed margin, without affecting layout.
6px matches `.pr-badge`'s outward offset (`right: -6px`) exactly,
so the badge renders unchanged from the pre-virtualization layout.
Browser support for overflow-clip-margin is identical to
content-visibility:auto (Chrome 90+, Firefox 102+, Safari 16.4+) —
every browser that ships the virtualization also ships the escape
hatch. Older browsers ignore both properties and render
unvirtualized (which is also correct).
Comment in the file now cites the actual spec section and the
correct mental model — no more "auto applies paint only off-screen"
misreading.
* fix(web): grow overflow-clip-margin to 12px for badge shadow + hover per Codex review (round 3)
Codex round 3 [P3] caught that 6px covered the badge's border-box
offset (`right: -6px`) but not the ink overflow from
`box-shadow: 0 1px 3px` (~3px blur) and the hover
`transform: scale(1.05)` (~2px growth at typical badge widths).
12px covers offset + shadow + hover with a small safety margin.
|
||
|
xarmian
|
624cc27866 |
feat(web): virtualize ListView rows via content-visibility (TASK-1346) (#488)
Adds `content-visibility: auto; contain-intrinsic-size: auto 60px;`
to `.list-row` so the browser skips layout, style, and paint work
for any off-screen row in a collection page. Achieves the 5k-items
@ 60fps scroll target from the task acceptance without rewriting
three preservation-critical call sites.
Why content-visibility, not @tanstack/svelte-virtual or
IntersectionObserver windowing:
- svelte-dnd-action operates on the DOM. Removing off-screen
rows breaks reorder/drop when the drag distance crosses the
visible window.
- Scroll restoration in [collection]/+page.svelte targets
window.scrollTo(y) where y is the previously-saved offset.
A windowing layer that unmounts rows shrinks the document
height and the saved y lands on the wrong group.
- Keyboard navigation calls scrollIntoView on
`.item-card.focused`. A row the windowing layer has
unmounted is not queryable; the focus jump silently no-ops.
content-visibility keeps every row mounted (so DnD, window scroll,
and querySelector all work unchanged) while letting the engine
short-circuit the per-frame work that dominates 5k-item scroll
profiles. `contain-intrinsic-size: auto 60px` gives the engine a
size placeholder so the scrollbar is correct on first paint and
caches the actual measured height on Chrome 99+ / Firefox 125+ /
Safari 18+ — eliminating layout shift when rows enter visibility.
Parent: PLAN-1343.
|
||
|
xarmian
|
f13e299d44 |
feat(web): typed client wrapper for /items-index (TASK-1345) (#487)
* feat(web): typed client wrapper + ItemIndexResponse for /items-index (TASK-1345)
Adds the TypeScript surface for the local-first read model bootstrap
endpoint shipped in TASK-1344:
- `ItemIndexRow` — `Omit<Item, 'content'>` so adding a column to `Item`
flows into the skinny row shape automatically.
- `ItemIndexResponse` — `{ items, total, cursor }` wrapper. `cursor` is
documented as opaque since Phase 2 swaps the placeholder for a `seq`
cursor.
- `api.items.listIndex(ws, { collection?, includeArchived? })` — mirrors
`listByCollection`'s shape, hits `/workspaces/{ws}/items-index`
(workspace-level, not `/items/index`, to avoid colliding with an item
whose slug is `"index"` — see PR #486 round 1).
No callers wired yet — that's TASK-1346 (ListView virtualization) and
the IDB sync layer in Phase 2.
Parent: PLAN-1343.
* fix(web): strip empty content field in listIndex per Codex review (round 1)
Codex round 1 [P2] flagged that the server still ships `content: ""`
on every row of /items-index because `models.Item.Content` is tagged
`json:"content"` without `omitempty`. TypeScript's `Omit<Item, 'content'>`
hides the field from downstream consumers, so naive spread/cache code
could silently overwrite real item bodies with the empty string.
Enforce the typed contract at the wrapper boundary: parse the raw
response with `content` typed as optional, then destructure it out of
each row before returning. The returned object has no `content` key,
matching `ItemIndexRow`'s shape both at compile time AND runtime.
Future cleanup option (separate task): add a Go DTO struct so the
server doesn't put the empty field on the wire in the first place.
The client-side strip is the minimal fix that addresses the
correctness risk without touching the server contract.
|
||
|
xarmian
|
e83e7ef413 |
feat(api): add /items/index skinny-projection endpoint (TASK-1344) (#486)
* feat(api): add /items/index skinny-projection endpoint (TASK-1344)
Foundation for PLAN-1343 (local-first read model). Adds a new
GET /api/v1/workspaces/{ws}/items/index endpoint that returns
every item in a workspace minus the rich-text `content` body,
so the client can hydrate an in-memory + IndexedDB index from a
single request and render every collection page from local
state without re-fetching.
Response: {items, total, cursor}. The cursor placeholder is the
max(updated_at) across the result set — Phase 2 replaces it with
a monotonic `seq` cursor. Sort is updated_at DESC, id ASC for
deterministic, cursor-friendly ordering.
Auth uses the same collection-visibility + item-grant filter as
handleListItems. Optional ?collection=<slug> filter for use by
collection pages. ?include_archived=true mirrors the existing
list behavior.
Parent: PLAN-1343.
* fix(api): move skinny-projection endpoint to /items-index per Codex review (round 1)
Codex round 1 [P2] flagged that the original `/items/index` path
shadowed the detail URL of any item whose slug is `index` — slugify
emits `index` for a title of "Index", and chi's static-over-wildcard
preference would route `GET /items/index` to the new index handler
instead of the existing `/items/{itemSlug}` detail handler.
Move the endpoint up to the workspace level as `/items-index`, sibling
to the existing `/plans-progress` route. Slugs cannot contain hyphens
adjacent to identifiers in a way that would collide with a static
workspace-level path, so this URL space is permanently safe.
New test `TestListItemsIndex_DoesNotShadowItemSlug` locks in the
contract: a real item titled "Index" still resolves through
`/items/{itemSlug}`, while `/items-index` returns the index wrapper.
|
||
|
xarmian
|
bdcb62e902 |
fix(api): accept nested object/array for PATCH items fields/tags (BUG-1144) (#485)
The PATCH /api/v1/workspaces/{ws}/items/{ref} endpoint previously
demanded `fields` and `tags` arrive as JSON-encoded strings, because
models.ItemUpdate declares them as *string to mirror the storage shape.
Sending the natural nested-object shape any reasonable HTTP client
would produce returned HTTP 400 with a leaked Go unmarshal error
naming the internal struct field — confusing for anyone integrating
against Pad over HTTP (webhook reactors, custom dashboards, non-CLI
agents, third-party MCP bridges).
This is the symmetric input-side counterpart to BUG-991, which was
fixed at the MCP boundary in PR #364 with dual-emit normalization
rather than the full Plan-sized models.Item migration.
Fix: add a custom ItemUpdate.UnmarshalJSON that accepts either shape
on the wire and normalizes to the canonical string internally. The
struct field type stays *string, so the validation/storage/web/CLI
pipeline is untouched. All in-process Go callers construct ItemUpdate
literals (15 grepped call sites) and never hit UnmarshalJSON, so the
change is invisible to them.
Wrong shapes (e.g. `{"fields":42}`, `{"tags":{"x":1}}`) now return a
domain-level 400 — `"fields" must be a JSON object or a JSON-encoded
string` — surfaced via sentinel errors (ErrInvalidFieldsType /
ErrInvalidTagsType) that the handler unwraps from decodeJSON's
"invalid JSON: %w" wrapper.
Coverage:
- models/item_test.go: 10 sub-tests covering object, array, string,
null, absent, and wrong-type cases for both fields and tags.
- server/handlers_items_test.go: 6 PATCH integration sub-tests
asserting back-compat, the BUG-1144 repro now returns 200, and
that error responses no longer leak Go struct field names.
Smoke-tested against the live server with the exact repro curl from
BUG-1144 (HTTP 200), plus malformed (HTTP 400 with clean message)
and stringified-string back-compat (HTTP 200).
|
||
|
xarmian
|
9fecb82e82 |
docs: surface --schema flag in SKILL.md + CLAUDE.md (TASK-1336) (#484)
PR #482 / TASK-1334 added the --schema flag to `pad collection create`. PR #483 / TASK-1335 wired it through MCP. Agents reading the live SKILL.md (embedded in the binary) need to see the new path or they'll keep reaching for --fields DSL and hit the original BUG-1284 symptom. Updates: - skills/pad/SKILL.md "Collections" section: now shows both --fields DSL and --schema JSON forms side-by-side. Calls out exactly when --schema is required (terminal_options et al.), and notes the label-from-key auto-fill so agents know empty labels are safe. - CLAUDE.md command list: adds the --schema variant alongside the existing --fields example so the project-level cheat sheet stays accurate. The CLI Long help (`pad collection create --help`) and the MCP tool description were already updated in TASK-1334 / TASK-1335. Parent: PLAN-1333. |
||
|
xarmian
|
a28927c51e |
feat(mcp): add typed schema param to pad_collection.create (TASK-1335) (#483)
* feat(mcp): add typed schema param to pad_collection.create (TASK-1335)
TASK-1334 shipped the CLI --schema flag for BUG-1284. This wires the
same capability through the MCP surface so agents calling pad_collection
get a structured object parameter rather than a stringified DSL —
matching how every other Pad-native data structure (items, comments,
etc.) flows through MCP.
Three coordinated changes:
1. internal/mcp/catalog.go — add "object" Type to ParamDef, mapped to
mcp-go's WithObject. Generic addition; the schema param is the first
consumer.
2. internal/mcp/catalog_collection.go — declare schema (object) param
on pad_collection.create alongside the existing fields (string)
param. Tool description now points agents at schema for
terminal_options, defaults, computed fields, suffixes, and relation
collections (everything the DSL cannot express), and notes the
fields/schema mutual exclusion.
3a. internal/mcp/dispatch.go — BuildCLIArgs now JSON-encodes
non-string structured values via a new encodeFlagValue helper. The
ExecDispatcher path (`pad mcp serve`) hands MCP input through to
the CLI's --schema flag as inline JSON; previously fmt.Sprint(map)
produced garbage Go-syntax. Generic — applies to any future flag
that accepts JSON.
3b. internal/mcp/dispatch_http_routes.go — mapCollectionCreate accepts
`schema` (object or stringified JSON) alongside `fields`, errors
if both are set, and backfills missing field labels using
titleCaseLabel so HTTPHandlerDispatcher MCP clients see the same
rendering as CLI users.
Tests cover: object→JSON encoding in BuildCLIArgs, array→JSON encoding,
structured schema input round-trips through mapCollectionCreate with
terminal_options preserved, missing-label backfill, stringified-JSON
fallback shape, both-flags rejection, malformed-JSON rejection.
Parent: PLAN-1333.
* fix(mcp): normalize empty/null schema input to absent per Codex round 1
Codex flagged a transport mismatch: the CLI treats `--schema ""` as
absent (falls through to --fields), but the HTTP dispatcher treated
schema=null or schema="" as set, triggering the mutually-exclusive
guard against fields or an invalid-JSON error from unmarshal.
Fix: in mapCollectionCreate, normalize nil and empty/whitespace strings
to "schema absent" before the mutex check. Keeps both transports
symmetric so MCP clients sending optional-arg defaults (null/empty) get
identical behavior to CLI users passing --schema "".
Test: TestMapCollectionCreate_EmptySchemaFallsThroughToFields covers
nil, "", and whitespace-only inputs in subtests, asserting the request
body uses the --fields-parsed schema.
Parent: PLAN-1333 / TASK-1335.
|
||
|
xarmian
|
25a21184a2 |
feat(cli): add --schema flag to pad collection create (TASK-1334) (#482)
* feat(cli): add --schema flag to pad collection create (TASK-1334) The existing --fields DSL (key:type[:options]) had no syntax for terminal_options, default, required, computed, suffix, or relation collection — every CLI-created collection lost those FieldDef properties even though the model already supports them. Symptom from BUG-1284: dashboard "active" counts treat published/archived items as in-progress because the persisted schema has no terminal_options. Adds a new --schema flag that accepts the full CollectionSchema JSON, which captures every current and future FieldDef property automatically. Three input modes: --schema '<json>' inline literal (agent-natural; CLI is agent-first) --schema @./path.json file path --schema - stdin --fields and --schema are mutually exclusive; --fields keeps working unchanged for backward compat (no deprecation). Refactors the inline parser into three testable helpers in main.go: collectionSchemaJSONFromFlags (orchestrator), readSchemaInputBytes (input resolver), and parseFieldsDSL (legacy DSL parser preserving the "first status select gets required+default" heuristic). Tests: 9 table-style cases in collection_create_schema_test.go covering all three input modes, the mutually-exclusive guard, malformed JSON, missing file, fallthrough-to-DSL, both-empty, and a regression test that verifies terminal_options + computed + suffix + relation.collection all round-trip through --schema. Parent: PLAN-1333. * fix(cli): backfill missing labels in --schema fields per Codex review (round 2) Codex flagged that the --schema example omitted "label", which the parser preserved as label:"" — agents constructing JSON could create collections that render blank field headers in the web UI. Fix: after unmarshaling --schema input, backfill any FieldDef with an empty Label using the same Title-Case-of-key heuristic the legacy --fields DSL applies (e.g. "due_date" → "Due Date"). Explicit labels are preserved. Also updated the help-text example to include "label" on the status field so the canonical shape is visible, plus a tip line documenting the auto-fill behavior so users know it's safe to omit labels. Test: TestCollectionSchemaJSONFromFlags_BackfillsMissingLabels covers auto-fill, multi-word key normalization, and the explicit-label-not- clobbered case. Parent: PLAN-1333 / TASK-1334. |
||
|
xarmian
|
e01c1581d0 |
fix(web): wire New Collection dashboard card to CreateCollectionModal (BUG-1332) (#481)
* fix(web): wire New Collection dashboard card to CreateCollectionModal (BUG-1332)
The "+ New Collection" tile in the workspace dashboard's Collections grid
was a plain <a href=".../settings"> link, so clicking it navigated to the
workspace Settings page instead of starting the create-collection flow.
Swapped the anchor for a <button> that opens the existing
CreateCollectionModal (same pattern Sidebar.svelte already uses for its
"+" affordance). The oncreated handler refreshes the dashboard so the new
collection card appears immediately without a full reload. Added a small
scoped reset on button.coll-card-new (font, text-align, cursor, width) so
it's visually indistinguishable from the sibling <a> cards.
* fix(web): gate dashboard New Collection on isOwner; refresh collectionStore per Codex review (round 1)
Codex review of round 0 surfaced two findings, both addressed here:
P2 — Owner gating: the New Collection dashboard trigger and modal were
shown to all viewers. The server requires owner role for collection
create (handlers_collections.go:48), and the settings page already gates
this modal behind isOwner. Without the gate, non-owners could open the
full create flow and only learn it's forbidden on submit. Wrapped both
the trigger button and the modal mount in {#if isOwner}, mirroring the
settings page's pattern.
P3 — Sidebar staleness: oncreated only refreshed dashboard-local state
via load(), so the Sidebar and quick-add (which read from
collectionStore) didn't see new collections until another refresh path
ran. Added a collectionStore.loadCollections(wsSlug) call alongside the
existing load(), matching Sidebar.svelte's own create-flow pattern.
* fix(web): keep CreateCollectionModal mounted across isOwner flicker per Codex review (round 2)
Round 1 wrapped the dashboard's CreateCollectionModal in {#if isOwner},
which Codex round-2 caught as a regression: the dashboard's 30s poll
(and any sync signal) calls load() → workspaceStore.setCurrent(), which
transiently clears currentMembership before /me resolves. isOwner flips
false during that window, unmounting the modal mid-edit and dropping any
form state the owner had typed.
Switched the modal-level gate from {#if isOwner} to {#if wsSlug} — the
same pattern Sidebar.svelte uses. The trigger button stays owner-gated
(UX boundary), and handlers_collections.go:48 remains the security
boundary, so this regression-free path preserves both protections.
* fix(web): cache isOwner so New Collection trigger doesn't flicker per Codex review (round 3)
Round 2 fixed the modal unmount race but left the trigger button reading
workspaceStore.isOwner directly, which Codex round-3 caught as a related
regression: the button itself disappears/reappears every 30s as the
dashboard's silent poll calls workspaceStore.setCurrent() → clears
currentMembership before /me resolves. Drops focus on the CTA every
refresh.
Cached the page-local isOwner via two effects per CONVE-606 (split
reactive-state sync from route-change effects):
1. On wsSlug change → reset cached isOwner to false (so we never leak
the prior workspace's owner status into the new workspace's CTA,
and never flash owner-only UI before /me confirms).
2. On workspaceStore.currentMembership change → only update when
non-null. The transient null windows during silent refreshes are
ignored, so the cached value (and the trigger's visibility) stays
stable.
Server-side enforcement (handlers_collections.go:48) remains the
security boundary; this is purely a UX stability fix.
|
||
|
xarmian
|
671ecabc41 |
fix(console): mobile hamburger toggles correctly on tap (BUG-1330) (#480)
The /console mobile hamburger button never opened its dropdown on tap.
A tap on the closed-state SVG inside the button triggered the toggle's
onclick (`mobileMenuOpen = true`), but Svelte 5 then synced the
{#if mobileMenuOpen}{:else}{/if} swap *before* the bubbled
`<svelte:window onclick={handleWindowClick}>` listener ran. By that
point the original `<rect>` click target was detached from the DOM
(`event.target.isConnected === false`); `target.closest('.console-nav')`
walked an orphaned subtree and returned null, the outside-click branch
fired, and the menu was reset to closed in the same tick — visually
"never opened."
Verified the timing in a Svelte 5 playground that mirrors the pattern;
the window handler logged `target=rect, isConnected=false,
closest(.nav)=NULL` for every tap.
Two-layer fix in web/src/routes/console/+layout.svelte:
1. Add `pointer-events: none` to `.mobile-hamburger svg` and its
children so the click target is always the button itself, which is
never re-rendered/detached when `mobileMenuOpen` flips. This is the
primary fix and matches the standard "icons inside buttons should
not capture pointer events" pattern.
2. Stop propagation on the toggle button's onclick so the click cannot
reach `handleWindowClick` even if a future change adds an inner
element without the same guard. Belt-and-braces.
Inline comments record the BUG-1330 root cause so the next person to
touch this nav doesn't reintroduce the SVG-swap.
TopBar.svelte's mobile hamburger is unaffected — its SVG content
doesn't swap on toggle (same icon regardless of sidebar state), so the
detach race never fires there.
|
||
|
xarmian
|
4b887c77db |
fix(editor): block drag handle picks up atom block-level nodes (TASK-1329) (#479)
* fix(editor): block drag handle picks up atom block-level nodes (TASK-1329) `BlockDragHandle.blockAtPos` rejected `depth === 0` outright, so atom block-level nodes (e.g. the new htmlBlock from PLAN-1322) never got a drag handle. When the cursor hovers over an atom block, posAtCoords returns a position at the doc boundary between top-level children; that resolves to depth 0, which the existing logic treats as "no enclosing block." Fix: when depth is 0, look at the node AT `pos` (after the boundary) and the node immediately before `pos` (walking doc children to find the sibling whose end matches `pos`). If either is an atom block-level node, return its block info so the handle shows up. Non-atom or inline content still returns null — same as before. The "Turn into" context menu still doesn't apply to atom blocks (they don't map to any TURN_INTO_ITEMS), so tapping the handle on an htmlBlock opens an empty / no-op menu in v1. Drag-to-reorder is the primary use case and is what users asked for here. Atom-block "Turn into" (e.g. convert htmlBlock ↔ codeBlock) is a follow-up. Parent: PLAN-1322. * fix(editor): hide Turn-into entries for atom blocks per Codex review (round 1) * fix(editor): measure menu height after visibility toggle (Codex round 2) |
||
|
xarmian
|
08904c9a60 |
feat(versions): collapse HTML blocks as semantic units in diff view (TASK-1328) (#478)
* feat(versions): collapse HTML blocks as semantic units in diff view (TASK-1328)
Adds a chunker that groups ` ```html ` fenced-block contents into a single
collapsed summary row in DiffView.svelte when the block contains any
added/removed lines. Avoids tag-soup diffs in the audit trail —
reviewing a marketing item or styled email with one HTML island no
longer floods the diff with N raw markup lines.
## How it works
The pipeline becomes:
diffLines(old, new) → buildDiffLines → chunkHtmlBlocks → collapseContext → render
chunkHtmlBlocks walks the line list looking for `^(\`{3,})html$` openers
and exact closing fence matches. Within a fenced range:
- Any added/removed line inside → emit one `{ kind: 'htmlBlock' }`
entry with counts of added/removed/unchanged lines
- All lines unchanged → pass through as individual lines (so the
existing context-collapse can compress them as ordinary unchanged
context — entirely-unchanged blocks don't get a special UI)
- Unbalanced fence (open with no close) → fall through, emit opener
as a plain line. Graceful degradation.
collapseContext is updated to operate on the new MidEntry[] shape
(discriminated union of line / htmlBlock). An htmlBlock entry counts
as a "change" for visibility-window purposes; hidden runs only tally
plain line entries when reporting skipped count.
## UX
The summary row is a <button> showing:
▶ HTML block changed +3 -2 (5 unchanged) click to expand
Click toggles a per-entry expansion state (SvelteSet keyed by display
index, so reactivity propagates without reassignment). When expanded,
the inner lines render using the existing .diff-line markup — no new
diff algorithm; we reuse the line-level diff already computed by
buildDiffLines.
The header stats badge still reads from diffLineEntries (the line-level
list), so total +/- counts across the whole diff include changes
inside chunks.
## Out of scope (future work)
- Semantic HTML diff (attribute moved, child reordered) — line-level
diff with collapse is enough; semantic HTML diffing is much harder
- Server-side diff storage changes — internal/diff/ unchanged; this
is purely a presentation concern
- Diff-view changes for any other content type (code blocks,
attachments, etc.) — htmlBlock-specific
Parent: PLAN-1322. Closes the plan: htmlBlock node, markdown round-
trip, render-time sanitization, source-view editor, insertion UX,
hidden-content authoring warning, and now diff collapse — all merged.
* fix(versions): chunk html blocks by per-side line ranges (Codex round 1)
|
||
|
xarmian
|
2bbd9c9e36 |
feat(editor): hidden-content detector + non-blocking authoring warning (TASK-1327) (#477)
* feat(editor): hidden-content detector + non-blocking authoring warning (TASK-1327)
Adds an authoring-honesty feature for HTML blocks: when the user pastes
or types raw HTML containing content that's invisible (or
near-invisible) to humans but readable by LLMs / agents, surface a
non-blocking warning pill above the block. Click the pill to expand an
inspector listing each hidden segment with the rule that flagged it
plus a snippet for context. "Dismiss for this block" button persists
acknowledgement via a sentinel HTML comment marker so the warning
doesn't re-fire after a doc reload.
NOT a security control. Render-time sanitization (sanitizeHtmlBlock,
TASK-1323) protects browsers from XSS. This protects authors from
unconsciously shipping content that looks one way and reads another
— common with copy-pasted HTML blobs that contain steganography
channels (display:none divs, white-on-white text, font-size:0,
hidden HTML comments, off-screen positioning, suspiciously long
aria-label / alt / title values).
## Detector
`web/src/lib/utils/hiddenContentDetector.ts` (NEW). Pure function
`detectHiddenContent(html: string): HiddenSegment[]`. Uses DOMParser
to walk the HTML tree:
- Inline-style heuristics:
- display:none, visibility:hidden, opacity:0/0%/0.0
- font-size with px value < 6
- color matches background-color (exact normalised match)
- position absolute/fixed with left/right/top/bottom <= -9000px
- transform translate to off-screen (heuristic regex on -9XXX or
-1XXXX values inside translateX/Y/translate)
- width:0 AND height:0
- clip:rect(0,0,0,0) — intentionally flagged; the user can dismiss
if it's deliberate sr-only positioning, but it's also a known
steganography channel
- HTML comments — every <!-- ... --> flags, EXCEPT the Pad-internal
ack marker (PAD_ACK_HIDDEN_MARKER) which the detector skips
- aria-label / alt / title attributes longer than 200 chars OR
containing newlines
Class-based hiding (e.g. .sr-only) is NOT flagged: resolving it
requires the page's stylesheet context, which we don't have, and the
false-positive rate is too high.
## NodeView UX
Warning pill ⚠ "N hidden segment(s) — click to inspect" appears above
the preview pane when segments > 0 AND the user hasn't dismissed for
this block. Click toggles the inspector panel below the source pane,
which lists each segment as `<code>tag</code> — rule` with the snippet
in a quoted monospace box. Dismiss button prepends
`<!-- pad:ack-hidden -->` to attrs.html via setNodeMarkup; the marker
survives markdown round-trip (it's a valid HTML comment) and the
detector skips it on subsequent runs.
The wrapper gets `.html-block--has-hidden` while the warning is
showing, in case any caller wants to react.
`updateWarning()` runs on every renderPreview call, so the warning
follows attrs.html changes (e.g. the user edits the block in source
mode and removes the hidden content — warning disappears immediately).
## Out of scope (future work)
- Class-based hiding detection (requires stylesheet resolution)
- Auto-stripping hidden content (this is an authoring honesty
feature, not a sanitizer; the user decides)
- Detection in non-HTML-block content (markdown comments, raw HTML
in markdown surface)
Parent: PLAN-1322.
* fix(detector): walk comments at doc root + normalize style values (Codex round 1)
* fix(detector): use CSSStyleDeclaration for browser-correct parsing (Codex round 2)
|
||
|
xarmian
|
781fe86f0e |
feat(editor): slash menu, toolbar, markdown shortcut to insert htmlBlock (TASK-1326) (#476)
* feat(editor): slash menu, toolbar, markdown shortcut to insert htmlBlock (TASK-1326) Three insertion paths for the htmlBlock node, all landing the user in source mode so they can immediately type HTML: 1. **Slash menu** — block-types.ts gets a new SLASH_ITEMS entry (id=htmlBlock, icon='HTML', label='HTML Block', insertOnly=true). execSlash dispatches setHtmlBlock + a requestAnimationFrame click on the empty-state placeholder to enter source mode. 2. **Toolbar** — EditorToolbar.svelte gets an 'HTML' button in the 'blocks' group, after the table button. Same setHtmlBlock + auto-flip pattern. 3. **Markdown shortcut** — htmlBlock.ts adds a new ProseMirror InputRule matching `^```html[\s\n]$`. Replaces the typed text with an empty htmlBlock node. The extension's priority is bumped to 1000 (default 100) so this rule wins against CodeBlock's broader `^```([a-z]+)?[\s\n]$` rule — without this, typing ``` ```html ``` + Enter would create a code block with language=html, not an htmlBlock. The auto-flip-to-source heuristic queries `.html-block:not(.html-block--editing) .html-block-empty` and clicks the preview pane on the next frame after insertion. This works because a freshly inserted block is always empty (empty placeholder visible) and never in --editing mode (--editing is only set when the user explicitly clicks). Multiple new empty blocks would in principle race-flip, but realistically only one is inserted at a time. Out of scope (TASK-1327, TASK-1328 follow): - Hidden-content authoring warning - Diff view collapse Parent: PLAN-1322. * fix(editor): target just-inserted htmlBlock by position + flip from input rule (Codex round 1) * fix(editor): scan for just-inserted htmlBlock + capture editor in input rule (Codex round 2) * fix(editor): capture insertion point before insert + walk forward to find new htmlBlock (Codex round 3) * fix(editor): disambiguate new htmlBlock via before-position snapshot (Codex round 4) * fix(editor): attrs-aware htmlBlock snapshot — handle replace + adjacent cases (Codex round 5) |
||
|
xarmian
|
c02aedd4ac |
feat(editor): source-view toggle for htmlBlock nodes (TASK-1325) (#475)
* feat(editor): source-view toggle for htmlBlock nodes (TASK-1325) Extends the htmlBlock NodeView with a click-to-edit source pane. The block renders sanitized live HTML by default; clicking the preview flips to a raw HTML textarea bound to attrs.html. Blur, Escape, or Cmd/Ctrl+Enter commits via setNodeMarkup and flips back to preview. Behavior: - Click anywhere in the rendered preview → flip to source mode and focus the textarea with caret at end. Clicks on interactive descendants (a, button, iframe, input, textarea, select, video, audio) pass through normally so embedded controls stay clickable. - Escape: preventDefault + commitAndFlip. Per task spec, Escape commits rather than cancelling — matches the project's existing block UX where edits aren't undone by escape. - Cmd/Ctrl+Enter: same as Escape — one-shot commit-and-flip. - Blur: also commits. The handler is idempotent (commit early-returns when textarea.value === lastHtml) so the Done-button click path doesn't double-commit when blur fires after the click. - Done button: mousedown.preventDefault keeps focus on the textarea so the click handler runs in the same selection context. Without that, the button would steal focus → blur → commitAndFlip → click on a hidden element no-op. - Empty block: shows "Empty HTML block — click to edit" placeholder so the atom node remains discoverable when attrs.html is empty. NodeView's update() handler re-renders only the preview when external attrs.html changes (e.g. via collab transactions). The textarea isn't auto-synced — if the user is mid-edit when a remote change lands, their in-progress text wins on the next commit. Last-write-wins is fine for v1; collab-aware merge would be its own task. CSS lives in Editor.svelte's <style> block immediately after the mermaid-source rule, using the same .editor-content :global(...) pattern as every other block-level element. The wrapper toggles between preview and source via the .html-block--editing class. Out of scope (separate tasks): - TASK-1326 — slash menu / toolbar / markdown shortcut to insert - TASK-1327 — hidden-content authoring warning - TASK-1328 — diff view collapse Parent: PLAN-1322. * fix(editor): isolate htmlBlock textarea events + gate edit on isEditable per Codex review (round 1) |
||
|
xarmian
|
40c9ec2a89 |
feat(editor): add htmlBlock node + markdown round-trip (TASK-1324) (#474)
Defines the foundation Node for PLAN-1322: an atomic block (atom: true) that round-trips ` ```html ` fenced blocks ↔ an editor node, rendered as sanitized live HTML in the WYSIWYG view via a NodeView. How the round-trip works: - markdown → node: tiptap-markdown's parse pipeline runs each fenced block through markdown-it's renderer.rules.fence. The new extension installs a wrapper that intercepts info === 'html' and emits a <div data-pad-html-block data-html="…escaped…"></div>. Other languages pass through to the original fence renderer (MermaidCodeBlock and syntax-highlighted code blocks unaffected). - node → markdown: addStorage().markdown.serialize emits ` ```html ` with a fence one backtick longer than the longest run inside the body, so a literal triple-backtick in the user's HTML can't close the fence early. Body always ends in a trailing newline before the closing fence. Sanitization is render-time only — attrs.html stores the raw user input verbatim (sanitizeHtmlBlock from TASK-1323 runs in the NodeView's update path, not at write time). That keeps the storage lossless for a future source-view editor (TASK-1325) and version-diff view (TASK-1328) to show what was actually typed. NodeView mirrors MermaidCodeBlock's pattern: contenteditable=false wrapper, ignoreMutation true, update() re-runs sanitizeHtmlBlock when attrs.html changes. Wired into Editor.svelte's extensions array immediately after MermaidCodeBlock so the markdown parser override runs after the default fence handler is in place. No insertion UX in this PR (no slash menu, no toolbar button, no keyboard shortcut, no source-view editor). Those are TASK-1325 (source view), TASK-1326 (insert UX), TASK-1327 (hidden-content warning), and TASK-1328 (diff collapse). Parent: PLAN-1322. |
||
|
xarmian
|
ac09035d01 |
feat(web): add sanitizeHtmlBlock with iframe-host allowlist (TASK-1323) (#473)
Introduces a second client-side sanitizer alongside sanitizeMarkdownHtml, intended for the future htmlBlock node type (TASK-1324). Differences from the markdown sanitizer: - Permits <iframe>, but only when src matches one of four embed hosts (YouTube, Vimeo, Loom, CodeSandbox). Enforced via a one-shot uponSanitizeElement DOMPurify hook scoped to the call. - Permits inline `style` attributes (styled callouts are the use case). - Adds structural tags (section, article, aside, header, footer, main, nav, figure, figcaption, picture, source) and media tags (video, audio) plus their relevant attributes. - Otherwise identical: strips <script>, on*/event handlers, javascript: and data: URLs. The markdown surface (comments + rendered item bodies) is unchanged — sanitizeMarkdownHtml keeps its tighter allowlist. autoplay is intentionally not permitted; embed providers handle autoplay via query parameters when intentional. Also exports isAllowedIframeSrc for use by future detectors / UX hints. Parent: PLAN-1322. |
||
|
xarmian
|
18087463ce |
feat(collab): op-log cursor protocol — force-refresh + watermark advance (TASK-1319) (#472)
* feat(collab): op-log cursor protocol — force-refresh + watermark advance (TASK-1319)
Closes both holes left by TASK-1309:
1. Long-disconnected tab + external-write race. A reconnecting client
announces its highest applied item_yjs_updates.id via `?since=<id>`.
If that id is below MIN(id) for the item, rows it expected to
replay have been pruned and the server sends a `force_refresh`
control frame and closes the conn. Client recreates the Y.Doc
and lazy-seeds from items.content. Without this, Tab A's stale
state would silently overwrite an external CLI/MCP write on the
next 5s flush.
2. Browser-only-edited items never GC'd. Browser collab-snapshot
PATCHes now carry an op_log_cursor body field. The store advances
items.content_flushed_op_log_id only when the cursor matches the
current MAX(op-log.id) — proving the markdown captures every
persisted op. SQL CASE clause re-evaluates MAX at COMMIT time so
a peer op landing between client-side cursor capture and the
UPDATE leaves the watermark untouched (no over-advancement).
Combined cursor mechanism:
- Server attaches op_log_cursor JSON control frames after replay,
after every successful AppendYjsUpdate (originator), and to every
peer's binary fan-out (so all peers stay in lockstep without a
round trip).
- Client persists per-tab in sessionStorage (NOT localStorage —
avoids cross-tab cursor leakage that would force-refresh stable
sessions).
- Server's MIN(id) check + force_refresh fires only when a non-zero
`since` is below MIN; `since=0` is treated as a fresh client.
New store methods: MinOpLogID, MaxOpLogID. New ItemUpdate field:
OpLogCursor *int64. New control message types: op_log_cursor,
force_refresh. New OpEvent.OpLogID for cursor piggyback. Existing
collab tests updated to drain TextMessage cursor frames.
Tests cover: initial cursor frame after replay (populated + empty
op-log), force_refresh fires when since<MIN, delta replay when
since>=MIN, cursor broadcast to originator + peers on append, and
watermark advancement gated on cursor==MAX.
Parent: PLAN-1248. Builds on TASK-1309.
* fix(collab): skip stale-Ydoc flush on force_refresh teardown per Codex review (round 1)
A force_refresh tear-down means the local Y.Doc cursor is below the
server's MIN(item_yjs_updates.id) — its derived markdown is stale.
Without this guard the collab $effect cleanup runs flushCollabNow
on the way out and silently PATCHes that stale markdown back to
items.content, overwriting the canonical content the fresh provider
is supposed to lazy-seed from. Per Codex round 1 [P1] of TASK-1319.
* fix(collab): force_refresh on empty op-log + cancel pending flush per Codex review (round 2)
Two P1 fixes:
1. Manager.Join now force_refreshes when since>0 and the op-log is
empty (hasMin==false), not just when since<MIN. After
PruneAndApply wipes the entire op-log, MIN is undefined; the
original predicate would have admitted the stale tab and let its
on-open Y.encodeStateAsUpdate write resurrect the pre-prune
document.
2. The +page.svelte onForceRefresh handler now also clears
collabFlushTimer. Without this a 5s timer that armed before the
force_refresh frame arrived can still fire AFTER the cleanup
ran, PATCHing stale Y.Doc-derived markdown to items.content.
New test: TestRoomManagerForceRefreshOnEmptyOpLogWithSince covers
the empty-op-log branch.
Per Codex round 2 [P1] of TASK-1319.
* fix(collab): include forceRefreshNonce in Editor key so it remounts on force_refresh per Codex review (round 3)
The collab $effect cleanup runs on forceRefreshNonce bump, but the
<Editor> {#key} was `${item.id}:true` — itemID doesn't change, so
the keyed Editor wasn't unmounting. The Tiptap Collaboration
extension only binds in onMount, so the editor stayed wired to the
stale (destroyed) Y.Doc while a fresh provider+doc were set up
in parallel. Edits would either be unsynced or eventually flush
stale markdown again.
Adding forceRefreshNonce to the key forces the Editor to remount
in lockstep with the doc swap. Per Codex round 3 [P1] of TASK-1319.
* fix(collab): refetch item.content before lazy-seed on force_refresh per Codex review (round 4)
After force_refresh the collab $effect rebuilds the Y.Doc and the
lazy-seed (TASK-1261) seeds it from item.content. But item.content
was the cached page-state copy — possibly stale relative to the
server (the WS force_refresh can beat the SSE/visibility refresh
that would otherwise update it). Lazy-seeding stale content into
a fresh op-log re-introduces exactly the staleness force_refresh
was supposed to clear: the next 5s flush PATCHes that stale view
back to canonical items.content.
onForceRefresh now does an api.items.get() before bumping the
nonce so the rebuild's lazy seed reads server-fresh content. A
failed fetch falls through to the bump anyway (an editor on
possibly-stale content is still better than a broken editor).
Per Codex round 4 [P1] of TASK-1319.
* fix(collab): suppress cursor during replay + move force_refresh check before getOrCreate per Codex review (round 5)
Two more findings:
1. [P1] writeLoop sends op_log_cursor frames for live ops broadcast
during the replay window. A client disconnecting after one of
those cursors lands but BEFORE the rest of replay completes
would persist a cursor pointing past unreplayed rows. On
reconnect with since=that-cursor, server replays nothing — the
client's Y.Doc would be missing causally-required ops.
Fix: per-roomConn replayDone atomic.Bool. writeLoop suppresses
cursor frames while it's false. runConn flips it after the
post-replay initial cursor is on the wire. Live binary frames
continue to flow during replay (Yjs CRDT commutativity); only
the cursor metadata is gated.
2. [P2] Force-refresh path leaked an empty room. getOrCreate
inserted into m.rooms before the force_refresh bail-out left
an orphan entry that PruneSweep would later treat as 'active'
and skip indefinitely.
Fix: schema-rebuild + force_refresh checks now run BEFORE
getOrCreate. Both are store-only mutations and the per-item
lock is held throughout, so concurrency is unchanged.
New test: TestRoomManagerCursorSuppressedDuringReplay regression-
guards the cursor-suppression behaviour.
Per Codex round 5 [P1+P2] of TASK-1319.
* fix(collab): tighten initial cursor + sync-destroy provider on force_refresh per Codex review (round 6)
Two more P1 fixes:
1. runConn's empty-replay fallback used MaxOpLogID() to anchor
the initial cursor. A live op landing between replayTo
returning and the cursor write would be reflected in MAX
but its binary frame might not have flowed through this
conn's writeLoop yet — the cursor would advertise an id
the client hasn't received. Initial cursor is now strictly
max(highestReplayed, since); MaxOpLogID is removed from
the opLogStore interface.
2. Provider.handleControlMessage's force_refresh branch now
calls this.destroy() SYNCHRONOUSLY before invoking the
onForceRefresh callback. Previously the consumer's recovery
path (async items.get refetch) would race the provider's
own onClose-triggered reconnect, which would re-open with
since=0 and push Y.encodeStateAsUpdate of the stale Y.Doc
— recreating the corruption force_refresh was meant to
prevent. destroy() sets destroyed=true so scheduleReconnect
short-circuits.
Per Codex round 6 [P1] of TASK-1319.
* fix(collab): block flush scheduling during force_refresh recovery per Codex review (round 7)
Previously, after onForceRefresh fires:
1. Provider is destroyed synchronously.
2. Async items.get refetch is in flight.
3. forceRefreshNonce bumps after refetch resolves.
4. $effect cleanup runs, then rebuild.
But during steps 2-3 the editor component is still mounted with
the stale Y.Doc, and a local edit fires handleContentUpdate which
calls scheduleCollabFlush. clearTimeout earlier in onForceRefresh
only canceled the timer at THAT moment; a new edit during the
refetch window arms a fresh timer that fires before cleanup. That
PATCHes stale Y.Doc-derived markdown back to canonical content,
recreating the corruption force_refresh was meant to prevent.
Fix: forceRefreshInFlight flag set in onForceRefresh, blocks
scheduleCollabFlush, resets after the fresh provider is wired
(end of $effect run). Per Codex round 7 [P1].
* fix(collab): gate runCollabFlush itself on force_refresh in-flight per Codex review (round 8)
scheduleCollabFlush blocked the 5s timer path, but direct callers
of flushCollabNow / runCollabFlush (beforeunload handler,
rich-to-raw toggle) bypassed the guard. A page reload or raw
toggle DURING the force_refresh recovery window still PATCHed
stale Y.Doc-derived markdown to canonical items.content.
Pulling the guard into runCollabFlush covers every caller in one
spot and returns 'deduped' so the result-shape contract holds.
Per Codex round 8 [P1] of TASK-1319.
* fix(collab): distinct 'skipped' result for force_refresh path; raw-toggle aborts per Codex review (round 9)
runCollabFlush returning 'deduped' on the force_refresh-blocked
path was indistinguishable from a legitimate same-content dedupe.
The rich→raw toggle treats 'deduped' as 'server already has this
markdown' and seeds rawSeedMarkdown from it — letting the user's
next raw edit overwrite canonical items.content with content
derived from the stale Y.Doc.
Add a distinct 'skipped' result for the force_refresh path. Raw
toggle aborts on it (with a 'try again in a moment' toast); other
callers fall through unchanged because no other call site
behaviorally depends on 'deduped' vs 'skipped'.
Per Codex round 9 [P1] of TASK-1319.
* fix(collab): server-side gate + post-await client guard against stale collab-snapshot per Codex review (round 10)
A force_refresh frame can arrive WHILE a collab-snapshot PATCH is
already mid-flight to the server. The client-side
forceRefreshInFlight check at PATCH-start can't catch this race;
the request lands at the server with stale Y.Doc-derived markdown.
Two-pronged fix:
1. Server: handler now checks op_log_cursor against MIN(op-log.id)
for collab-snapshot PATCHes and returns 409 Conflict when
cursor < MIN. Such cursors prove the flushing tab's Y.Doc was
built on rows that have been pruned (PruneAndApply, schema
rebuild, dormant GC). The markdown is, by construction, stale.
2. Client: post-await check on forceRefreshInFlight returns
'skipped' instead of 'flushed' so saveStatus / lastFlushedContent
don't seed from a known-stale base even if the server happened
to accept the PATCH (e.g. MIN advanced after handler validation).
New tests: TestCollabSnapshotRejectsCursorBelowMin (gate fires),
TestCollabSnapshotAcceptsCursorAtOrAboveMin (negative path).
Also de-leak an unused slice in the round-5 cursor-suppression test
so staticcheck stays clean.
Per Codex round 10 [P1] of TASK-1319.
* fix(collab): reject collab-snapshot when cursor>0 and op-log empty per Codex review (round 11)
The HTTP-layer gate I added in round 10 mirrored only PART of the
WS-upgrade force_refresh predicate. Round 5 had already taught us
that 'op-log entirely pruned' is a separate stale path from
'cursor below MIN' (PruneAndApply, schema rebuild, dormant GC all
leave hasMin=false), and the WS check now uses
`since > 0 && (!hasMin || since < minID)`. The HTTP gate had
only the second clause.
Mirror the WS predicate at the handler so a stale collab-snapshot
PATCH against an empty op-log gets a 409 too. New regression:
TestCollabSnapshotRejectsCursorOnEmptyOpLog.
Per Codex round 11 [P1] of TASK-1319.
* fix(collab): reject collab-snapshot cursor=0 on non-empty op-log per Codex review (round 12)
Round-11 gate accepted cursor=0 unconditionally. But a stateful tab
whose previous session disconnected BEFORE receiving the
post-replay cursor frame (network blip during the writeMu burst
between replay binaries and the cursor) ends up with sessionStorage
cursor=0 + a non-empty Y.Doc populated by prior replay binaries.
On reconnect with since=0 the server treats it as fresh, replays
nothing if the op-log was meanwhile pruned, and the client's
on-open Y.encodeStateAsUpdate resurrects pre-prune ops. The next
flush carries cursor=0 + stale-derived markdown.
The gate now refuses any incompatible cursor:
- cursor>0 + empty op-log (prior rule)
- cursor<MIN + non-empty op-log (prior rule, now naturally
catches cursor=0 too because 0 < any positive MIN)
The WS replay path is unchanged — full replay from since=0 is
the recovery for clients that genuinely lost their cursor; the
corruption manifested through the flush PATCH which we now gate.
New test: TestCollabSnapshotRejectsCursorZeroOnNonEmptyOpLog.
Per Codex round 12 [P1] of TASK-1319.
* fix(collab): close cursor=0 client/server gaps + lock validation+write atomically per Codex review (round 13)
Four P1 issues addressed:
1. Client always sends op_log_cursor (including 0) so the server
gate sees the field. Previously cursor=0 was omitted, which
silently bypassed the server's stale-snapshot rejection.
2. Provider construction now resets sessionStorage cursor to 0
when the Y.Doc is empty. The Y.Doc isn't persisted across
page reload, so a stored cursor=N + fresh empty Y.Doc would
announce since=N to the server and miss rows 1..N from
replay (server only replays id > N).
3. onOpen skips Y.encodeStateAsUpdate when lastOpLogID === 0.
A populated Y.Doc + cursor=0 is the network-blip-during-cursor-
write failure mode; pushing that state can resurrect ops the
server has pruned. Server replay + lazy-seed handle recovery
without our push.
4. Server gate now runs INSIDE the per-item collab setup lock
(new RoomManager.UnderItemLock helper) so a concurrent prune
(PruneAndApply, schema rebuild, dormant GC) cannot land
between the MIN check and the items.content write. Without
this, a tight race let stale snapshots overwrite canonical
content the prune just installed.
Per Codex round 13 [P1] of TASK-1319.
* fix(collab): gate handleDocUpdate on cursorAnchored to close stale-Ydoc edit path per Codex review (round 14)
Round 13 fix skipped on-open send for lastOpLogID===0, but local
edits via handleDocUpdate still propagated. A populated Y.Doc +
no-cursor-yet client could type, the edit would land in the
op-log with id N, server would send originator cursor=N, and
the next 5s flush would carry an 'anchored' cursor that passed
the server's MIN check — overwriting items.content with stale-
Y.Doc-derived markdown.
Add a cursorAnchored boolean. Set on first op_log_cursor frame
receipt (including cursor=0 against an empty op-log — that's a
legitimate 'server has nothing' signal). handleDocUpdate refuses
to send before this. Local edits buffer in the editor; once the
cursor arrives (or force_refresh rebuilds the provider), the
existing reconnect/edit paths catch them up.
Per Codex round 14 [P1] of TASK-1319.
* fix(collab): buffer + flush pre-anchor local updates per Codex review (round 15)
Round 14 silently dropped local Yjs updates fired before the
first op_log_cursor frame anchored the session. Yjs updates are
incremental: a dropped keystroke leaves later ops referencing
structs no peer can resolve, breaking convergence.
Buffer pre-anchor updates in a Uint8Array[] (capped at 1000 to
prevent unbounded growth in pathological 'anchor never arrives'
scenarios — overflow triggers force_refresh-style recovery).
On the first cursor frame, flush the buffer in order so the
server gets every causally-required struct before any post-
anchor updates land.
Per Codex round 15 [P1] of TASK-1319.
* fix(collab): destroy provider before force_refresh on pre-anchor buffer overflow per Codex review (round 16)
Round 15 overflow path called onForceRefresh but didn't destroy
the provider synchronously. A late op_log_cursor arriving before
the page-level rebuild (the recovery callback is async — refetches
items.content) would flip cursorAnchored=true, the partially-
populated buffer would flush, but the DROPPED prefix (the
overflowed entries) would leave server-side ops causally
incomplete — exactly the bug the buffer was supposed to prevent.
destroy() sets destroyed=true, removes message listener,
short-circuits scheduleReconnect, closes the socket. Late cursor
frames can no longer anchor a doomed provider.
Per Codex round 16 [P2] of TASK-1319.
* fix(collab): refuse rebuild on refetch fail + broaden on-open gate to cursorAnchored per Codex review (round 17)
Two findings:
[P1] force_refresh recovery bumped forceRefreshNonce in finally
even when the item.content refetch failed. The rebuild then
lazy-seeded from the cached (possibly-stale) item.content, and
the next flush would PATCH that stale view back to the server.
Move the bump into .then() so a failed refetch surfaces a
'please reload' toast and leaves the editor effectively
read-only (forceRefreshInFlight stays true, blocking flushes).
[P2] Send-on-open gate was lastOpLogID > 0, which silently
dropped local edits made during a brief offline window after a
legitimate 'cursor=0' anchor (empty op-log session). Switch to
cursorAnchored — the boolean specifically distinguishes
'unanchored' (stale Y.Doc + no server confirmation) from
'anchored at cursor=0' (legitimate empty op-log).
Per Codex round 17 [P1+P2] of TASK-1319.
* fix(collab): force_refresh on cursor=0 against non-empty Y.Doc per Codex review (round 18)
cursor=0 means the server's op-log is currently empty. A
non-empty Y.Doc at first-cursor receipt implies the ops came
from an earlier connection within this provider's life that
never reached its post-replay cursor frame, followed by a
server-side prune (PruneAndApply, schema rebuild, dormant GC)
during our disconnect. Anchoring at cursor=0 in that state
would mark a stale Y.Doc as authoritative; the next on-open
state push or flush would resurrect pre-prune state and
overwrite canonical items.content.
Detect the configuration via Y.encodeStateVector length and
invoke the same force_refresh-style recovery the explicit
server frame triggers: destroy provider, clear sessionStorage,
fire onForceRefresh so the page rebuilds from items.content.
Per Codex round 18 [P1] of TASK-1319.
* fix(collab): gate cursor=0 force_refresh on remoteSyncApplied per Codex review (round 19)
Round 18 force_refreshed the provider whenever cursor=0 arrived
against a non-empty Y.Doc. But local pre-anchor edits (user typed
before the initial cursor=0 of a legitimate empty-op-log session
arrived) ALSO populate Y.Doc — yet those edits live in
preAnchorUpdates and were supposed to flush on anchor. The
predicate spuriously triggered force_refresh, dropping the
buffered local edits.
Track remoteSyncApplied (set when readSyncMessage applies
anything to Y.Doc — replay binary or live peer op). Only force_
refresh on cursor=0 when remoteSyncApplied is true: that's the
true 'remote replay landed but server now reports empty op-log
=> mid-session prune' signature.
Per Codex round 19 [P1] of TASK-1319.
* fix(collab): repair brace mis-merge in wsProvider cursor=0 guard
The round-19 patch overlapped the round-18 inner block, producing
an extra brace + over-indented body. Collapsing into a single
clean block restores parseability without changing semantics
beyond what round 19 already documented.
* fix(collab): gate syncStep2 reply on cursorAnchored per Codex review (round 20)
readSyncMessage writes an inline syncStep2 reply when it receives
a peer's syncStep1. That reply embeds our current Y.Doc state.
If a peer's syncStep1 arrives before our first op_log_cursor
(pre-anchor window), the reply path bypasses handleDocUpdate's
cursorAnchored gate and lets potentially-stale Y.Doc state reach
the server before the cursor=0 + remoteSyncApplied force_refresh
recovery has a chance to fire.
Suppress the reply while unanchored. Peer state propagation
still works: the buffered preAnchorUpdates flush on anchor, and
the lazy-seed rebuild after a force_refresh seeds canonical
content from items.content.
Per Codex round 20 [P1] of TASK-1319.
* fix(collab): fold mid-replay live op ids into post-replay cursor + remoteSyncApplied only on apply per Codex review (round 21)
Two more findings:
[P1 server] writeLoop suppresses cursor frames during replay to
prevent the client persisting a cursor past unreplayed rows.
But binary frames for those live ops still go through
(commutativity), so the client APPLIES them to its Y.Doc. The
post-replay initial cursor only covered max(highestReplayed,
since), leaving the cursor below the highest applied op. On
empty-replay sessions this trips the client's
'cursor=0 + remoteSyncApplied' force_refresh path and discards
buffered pre-anchor edits.
Track maxLiveOpLogIDDuringReplay on the roomConn (atomic
compare-and-swap) and fold it into the post-replay cursor.
[P1 client] remoteSyncApplied was set on every MESSAGE_SYNC,
including syncStep1 (which only carries a state vector — it
doesn't apply state). A peer's syncStep1 arriving pre-anchor
would falsely flag remote-sync-applied and trip the cursor=0
force_refresh on legitimate empty-op-log sessions. Set the
flag only after readSyncMessage returns, and only for
syncStep2 / update subtypes.
Per Codex round 21 [P1] of TASK-1319.
* fix(collab): widen writeMu critical section + drop omitempty on op_log_id per Codex review (round 22)
Two more P1s:
[P1 server] writeLoop's mid-replay record-max happened OUTSIDE
writeMu, so runConn's post-replay read could race the record:
runConn loads → writeLoop's atomic store of higher value →
runConn sends cursor below the live id. Move the entire
per-event sequence (binary write + replayDone observation +
record-or-send) inside writeMu, and have runConn acquire
writeMu around its read+cursor-write+replayDone-flip. The lock
serializes the two paths cleanly: writeLoop events that ran
first have already recorded; events that arrive after replayDone
flips emit their own cursor frames.
[P1 protocol] OpLogID had `omitempty` JSON tag — a legitimate
cursor=0 (empty op-log session) serialized as
`{"type":"op_log_cursor"}` with no op_log_id field. The
client's strict-type check then rejected it as malformed,
leaving the session unanchored and local edits buffered
forever. Drop omitempty so 0 is wire-visible. Other control
types (applier_request/ack) carry an extra op_log_id:0 in
their JSON, which their client dispatches ignore.
Per Codex round 22 [P1] of TASK-1319.
* fix(collab): route originator cursor through writeLoop FIFO per Codex review (round 23)
readLoop sent the originator's op_log_cursor directly via
sendOpLogCursor right after AppendYjsUpdate, bypassing the bus/
writeLoop ordering. With a peer op already queued in rc.bus, the
sequence on the wire could be:
1. originator cursor=N (newer local op)
2. peer binary (older op)
3. peer cursor=M < N (rejected by client's max-take logic)
Client persists cursor=N. If the client then disconnects before
applying the peer binary, reconnect with since=N replays nothing
(server has nothing > N) and the older peer op is lost forever
to this client's Y.Doc.
Fix: writeLoop now processes self events too — skipping the
binary echo (the originator already has Y.Doc state) but routing
the cursor frame through the same FIFO bus channel as peer ops.
The originator's cursor=N now arrives strictly AFTER all
older-id peer events on the same channel.
Per Codex round 23 [P1] of TASK-1319.
|
||
|
xarmian
|
028db39217 |
feat(collab): periodic op-log GC sweeper for dormant items (TASK-1309) (#471)
The Yjs collab dumb-relay accumulates op-log rows indefinitely in item_yjs_updates. DOC-1307 surfaced 45-second p50 cold-reconnect latency on a single item with 5000 accumulated rows. Without GC, busy items keep growing. This adds a periodic background sweeper that prunes the entire op-log for items that are both DORMANT (no recent activity) AND FULLY FLUSHED (items.content has captured every op-log row). Whole-log only — Yjs op streams are causally linked, prefix-pruning corrupts replay; future cold connects lazy-seed from items.content. Components: - Store.ListDormantOpLogItemsBefore (joins items, filters watermark) - Store.PruneItemOpLogIfDormantBefore (atomic conditional DELETE) - Store.GetItemContentFlushedOpLogID (per-item watermark getter) - RoomManager.PruneSweep (per-item-locked, active-room-skip) - Server.StartOpLogGC / stopOpLogGC (mirrors orphan_gc.go pattern) - cmd/pad/main.go env vars PAD_OPLOG_GC_INTERVAL / PAD_OPLOG_GC_MIN_AGE - New (item_id, created_at) index for the dormancy query - New items.content_flushed_op_log_id column (id-based watermark, monotonic, no clock-skew or second-granularity false positives) + content_flushed_at (informational timestamp) Watermark policy: - Server-driven full-content writes (CLI / MCP / version restore / PruneAndApply) advance content_flushed_op_log_id to MAX(op-log.id) via subquery, atomic with the content UPDATE - Browser collab-snapshot 5s flushes do NOT advance the watermark — they can't prove their markdown captured every peer's ops, so letting them stamp would risk later GC-pruning unsynced peer edits - Schema-mismatch rebuild (TASK-1268) logs a WARN when it drops unflushed ops (data loss is unavoidable on schema bumps but visible) Stop ordering: collab.Close() now runs BEFORE bg.Wait() so a GC goroutine waiting on an itemLock behind an active Join can drain. Migration backfill: items WITH existing op-log rows keep NULL watermark (don't certify); items WITHOUT op-log rows get a synthetic 0 watermark (vacuous, harmless — no rows to compare against). Tests: - 6 RoomManager.PruneSweep tests (dormant prune / default minAge / empty / bails-on-Close / skips-active-room / skips-row-added-mid- sweep via fakeOpLog hook) - 5 Server.OpLogGC tests (prunes-dormant / start-idempotent / preserves-unflushed / backfill-doesnt-certify-unflushed / no-collab-noop) - TestCollabSnapshotDoesNotAdvanceOpLogWatermark in store - TestCollabSnapshotQueryOverridesBodyVersionSource in server (regression for body-attacker bypass) Seven rounds of Codex review — caught 5 P1s and 4 P2s I would have shipped under self-review: 1. Prefix-prune corrupts Yjs replay 2. Stop ordering deadlock 3. Missing index 4. Best-effort flush ⇒ data loss 5. Backfill over-certifies via metadata-PATCH 6. Second-granularity timestamp comparison 7. Schema-mismatch path drops unflushed silently 8. Browser flush stamps watermark beyond Y.Doc 9. Body version_source bypasses server policy |
||
|
xarmian
|
66aa6f5197 |
fix(loadtest): codex catch-up review fixes (TASK-1270) (#470)
PR #468 shipped without a codex review because codex was unavailable that day. This is the catch-up; codex found 2 P2s and 5 NITs. [P2] readSendTimestamp recorded latencies for prior-run replay frames. The original guard only checked nonzero ts, not session recency, so any stale op-log row inflated p95/p99. Fixed: runContext captures startedAt; recv path filters frames whose embedded ts predates this run. Verified: against an item with 90 stale rows the test counts received frames (630) but only records latencies for the 180 live ones. [P2] Shutdown deadlock. The writer goroutine could be blocked in conn.WriteMessage when rc.done closed; the only path to conn.Close was that same goroutine's select-case, so wg.Wait() could hang forever under server backpressure. Fixed: per-client watchdog goroutine closes the conn from outside the writer when done fires, plus a 5s SetWriteDeadline per send as defence-in-depth. A 5s duration test now exits in exactly 5.008s. NITs (also fixed): - buildFrame docstring corrected (minimum is 16 metadata bytes, buffer is frameBytes+1). - buildFrame returns (bytes, error) instead of log.Fatalf-ing on rand.Read; caller logs detail and increments errors counter. - -cookie / -token flag help now states both can be set together. - Watchdog-induced WriteMessage errors are no longer counted as real errors (isClosedDone check). - buildFrame error path now logs the actual error detail. Two rounds of codex review: round 1 found the items above; round 2 returned CLEAN. |
||
|
xarmian
|
f44e592554 |
docs: confirm collab adds no container deps; close PLAN-1248 cleanup (TASK-1272) (#469)
Documents that the dumb-relay design preserves the single-Go-binary self-hosted shape — no Yjs Go port to vendor, no separate sync server, no Redis (multi-instance fanout is a separate deferred IDEA). Op-log lives in the existing SQLite/Postgres; relay is part of the main HTTP listener. The original task scope also called for removing `web/src/routes/dev/yjs-sandbox/`, but that route only existed on the `feat/yjs-tiptap-spike` branch and was deliberately not cherry-picked into PLAN-1248's productionization work — so there's nothing to delete on `main`. The spike branch can now be deleted. Audited web/package.json for spike-only deps: every yjs / tiptap / y-protocols dep is consumed by production code paths. Nothing to clean up. |
||
|
xarmian
|
287fa545fb |
feat(loadtest): add cmd/loadtest-collab + DOC-1307 findings (TASK-1270) (#468)
Synthetic Go load-test for the Yjs collab dumb-relay. Each simulated client opens a WebSocket, sends tagged sync frames at a configurable rate, consumes inbound frames, and computes broadcast fanout latency. Doesn't depend on a real Yjs port — the dumb-relay's first-byte discriminator (yMessageSync=0) is enough to exercise the persist + broadcast path with synthetic payloads. Each frame embeds a unix-nano timestamp + client ID so receivers can compute round- trip latency without out-of-band coordination. Findings (in DOC-1307): - N=5, N=25: clean, p95 < 10ms, fanout matches expected (N-1)x - N=100: 38/100 dial failures (consistent), but the 62 successful see p95=27ms — server rejects ~38% of simultaneous dials at this level. Filed BUG-1308 to investigate the ceiling. - Op-log grows unbounded without compaction; old runs replay on reconnect causing latency blow-up. Filed TASK-1309 to wire a periodic prune sweeper. Self-reviewed only; codex was unresponsive today after multiple hour-long retries. |
||
|
xarmian
|
680cfbd879 |
docs: collab endpoint + Tiptap coordinated-bump rule (TASK-1269) (#467)
CLAUDE.md changes (no behaviour change):
- API Reference: GET /api/v1/collab/{itemID}?schema_version=N
- New "Real-time collaboration (Yjs / Tiptap)" section under Common
Tasks, documenting:
- Where the collab code lives
- The three-package coordinated-bump rule (@tiptap/core +
@tiptap/extension-collaboration + @tiptap/y-tiptap must move
together with exact pins)
- When the schema version (web/src/lib/collab/schemaVersion.ts +
internal/collab/manager.go::DefaultSchemaVersion) must bump
|
||
|
xarmian
|
9b46be915a |
feat(collab): schema-version handshake + mismatch rebuild (TASK-1268) (#466)
Adds a client→server schema-version handshake on every WS connect and a per-item op-log rebuild path for the case where the server ships a new SCHEMA_VERSION and finds older rows persisted in the op-log. Client side - New web/src/lib/collab/schemaVersion.ts exporting `SCHEMA_VERSION` (currently '1') with a documented bump rule covering Tiptap extension changes, coordinated multi-package bumps, and Y.Doc fragment-shape changes. - wsProvider's defaultCollabUrl appends ?schema_version=... Server side - handlers_collab.go validates ?schema_version against RoomManager.SchemaVersion() BEFORE upgrading the WS; mismatch returns HTTP 400 with code "schema_mismatch". An empty query is treated as legacy '1' for graceful deploys; once the server bumps past v1, missing query becomes a 400 too. - New RoomManager.SchemaVersion() getter. - RoomManager.Join's setup-phase (under itemLock) now calls maybeRebuildOnSchemaMismatch: if the latest persisted op-log row's schema_version disagrees with the manager's current version, the entire item op-log is pruned via PruneYjsUpdatesBefore. items.content is canonical and untouched, so the lazy-seed path (TASK-1261) re-encodes it into ops at the new schema on the next idle tick. - New store method LatestYjsUpdateSchemaVersion. Tests - internal/collab/manager_test.go: three new tests (mismatch prunes, clean version preserves op-log, post-rebuild connects are clean) + fakeOpLog gets LatestYjsUpdateSchemaVersion + PruneYjsUpdatesBefore. - internal/server/handlers_collab_test.go: rejects-schema-mismatch (400), accepts-explicit-match (101). One round of Codex review (CLEAN with two NITs, both fixed). |
||
|
xarmian
|
191b887e23 |
feat(versions): VersionSource attribution + collab coexistence (TASK-1267) (#465)
The collab 5s-flush PATCH (TASK-1260) sends
`?source=collab-snapshot` with a body of just `{ content }`. Without
a handler-side stamp, `Store.UpdateItem`'s default coerced empty
input.Source to "web" on the version row and the per-(actor, source)
throttle suppressed every collab-driven snapshot following the user's
last manual web edit — version-diff effectively went silent during
co-edit sessions.
Adds:
- ItemUpdate.VersionSource: overrides per-version-row Source
attribution WITHOUT mutating items.source. The latter feeds
WorkspaceHasAgentActivity's `source IN ('cli', 'mcp')` filter,
so a CLI/MCP-created item the user opens in the editor would
otherwise silently flip out of the agent-activity tally on every
auto-flush.
- Store.UpdateItem prefers VersionSource over Source for version
row creation; falls back to Source then "web" if neither set.
- handlers_items.go stamps `input.VersionSource = "collab-snapshot"`
for `?source=collab-snapshot` PATCHes (when not already set).
Tests:
- internal/store/items_collab_versions_test.go: store-level
reverse-patch reconstruction over a CLI→web→collab-snapshot
edit sequence; verifies IsDiff=true on at least one row.
- internal/server/handlers_items_collab_versions_test.go: full
HTTP-level test of the route; asserts a collab-snapshot version
row is created AND that items.source stays "cli".
Four rounds of Codex review.
|
||
|
xarmian
|
fdc6b221d0 |
test(collab): regression guard for share-page collab isolation (TASK-1266) (#464)
The public /s/{token} share page must keep rendering markdown via
marked + DOMPurify and never open a WebSocket to /api/v1/collab —
anonymous viewers can't authenticate, and exposing per-item Y.Doc
traffic to the public internet would be a security regression.
This is a verification task: zero production code changes. Adds
TestSharePageDoesNotImportCollab in internal/server which:
- Walks the import closure rooted at the share-page +page.svelte
(resolves $lib/ aliases, relative paths, conventional extensions,
static AND dynamic `import(...)` forms; skips external packages)
- Asserts no file in the closure contains forbidden tokens:
wsProvider, CollabProvider, @tiptap/extension-collaboration,
@tiptap/y-tiptap, 'yjs' / "yjs", y-protocols, Editor.svelte
(suffix), WebSocket, /api/v1/collab
- Asserts the route file still imports + invokes marked and
DOMPurify.sanitize (catches a renderer swap)
- Strips comments before all checks so leftover commented-out
imports can't bypass
Verified by injection: direct AND transitive AND dynamic-import
regressions all fail the test.
Four rounds of Codex review.
|
||
|
xarmian
|
27b048eef3 |
feat(collab): presence carets with deterministic user colors (TASK-1263) (#463)
Renders remote peers' carets and selection highlights in the collab-mode Tiptap editor via @tiptap/extension-collaboration-caret (the v3 rename of extension-collaboration-cursor — pinned at 3.22.5 to match the rest of the Tiptap suite). - New cursorColor.ts: djb2 hash → HSL → #rrggbb (hex required because y-tiptap's selectionRender appends an alpha-hex byte and only validates hex) - Editor.svelte: optional `awareness` + `collabUser` props; only registers CollaborationCaret when ydoc + awareness + user are all present - +page.svelte: derives `collabUserState` from authStore.user and threads it + collabProvider.awareness into <Editor> - CSS for .collaboration-carets__caret + label + selection (label always visible — caret is too thin to hover, so the original hover-reveal was unreachable) Three rounds of Codex review. |
||
|
xarmian
|
1ad9ce6c1f |
feat(collab): mobile WS reconnect handling (TASK-1265) (#462)
Adds visibility / online / offline event listeners to CollabProvider so iOS Safari (and other mobile suspends) recover the WS without waiting for the 30s backoff ceiling. - visibilitychange→'visible' / online: forceReconnect — closes any existing socket (even apparently-OPEN ones, since iOS can silently suspend the transport while leaving readyState OPEN) and reconnects from a clean slate. - offline: demote state='offline' immediately + tear down the socket so a queued syncStep2 can't flip back to 'synced'. Backoff timer keeps trying so we recover even when 'online' never fires. - handleControlMessage now pins the source socket (e.currentTarget) so an applier_ack after force-reconnect doesn't land on a new socket the server doesn't recognize. - Extracted runDisconnectCleanup() helper to keep all teardown paths in lockstep. Six rounds of Codex review. |
||
|
xarmian
|
22b4030057 |
feat(collab): connection-state badge on item editor (TASK-1264) (#461)
Surfaces the WS connection state ('connecting' | 'synced' |
'reconnecting' | 'offline') as a small badge in the item-detail
meta-info row. Visible only when the WS provider exists (i.e.
canEdit && !rawMode), so share-page / read-only / raw mode don't
render it.
Adds CollabProvider.state $state field with transitions on the
real-sync edges only. reconnectAttempts is now reset in the
syncStep2 branch and the grace timer (not on raw open/close), so a
flaky proxy that OPEN→CLOSE-before-sync still reaches the
OFFLINE_THRESHOLD. State preserves 'offline' across retries to
avoid flicker; pre-first-sync failures stay 'connecting',
post-sync drops become 'reconnecting'.
Three rounds of Codex review.
|
||
|
xarmian
|
483e338a54 |
feat(collab): drop conservative content-skip when collab active + applier toast (TASK-1262) (#460)
## Drop TASK-1243's content-skip when collab is active
The conservative `item = { ...updated, content: item.content }`
preservation in the SSE/sync handlers was protecting against
clobbering a user's mid-keystroke edit with a stale content
snapshot. Under collab the editor reads from Y.Doc — NOT the
content prop — so the Editor.svelte $effect's `if (ydoc) return`
gate at line 810 makes adopting `updated.content` harmless to
the live editor while keeping `item.content` fresh for
downstream consumers (UI summaries, search-index hints,
subsequent share-page renders).
For non-collab viewers (view-only, raw mode, items where
canEdit=false) the content-skip stays — those paths DO render
from item.content via the prop $effect, and adopting a stale
SSE snapshot mid-keystroke would clobber unsaved chars.
Applied to all four adoption sites:
- SSE item_updated
- SSE item_restored
- syncService incremental update
- syncService full-refresh fallback
## Applier-success toast
The applier handler (wired in TASK-1259's absorption of TASK-1262
scope) silently called setContent. Users would see their editor
change under them with no UI hint. Adds a brief
toastStore.show('External edit applied', 'info') after the
setContent succeeds; preserves the late-apply guard so toasts
only fire on actual mutations.
## Acceptance criteria
- [x] `pad item update REF --stdin < new.md` while two browser
tabs are open: both tabs reflect the change (the applier path
fires setContent on the longest-connected tab; ops broadcast
to peers; SSE adoption keeps item.content fresh).
- [x] `pad item update REF --status done` (field-only): both
tabs see the field change via SSE; no editor disruption (the
guard `input.Content != nil` skips the applier branch
entirely; SSE adoption updates fields atomically).
- [x] Designated client disconnects mid-flight: server retries
next applier (TASK-1257 logic; pending follow-up TASK-1268
for the all-applier-failed case).
- [x] Toast: "External edit applied" surfaced.
Parent: PLAN-1248
|