P1/C1-stale: after adopt completes, assert (a) lane storage equals the adopted
remote store, and (c) readOutbox returns null. Two new mutations now trip these
tests: removing clearOutbox from clearPendingState and removing the
onRemoteAdopted callback each leave the two stale tests red.
P2a-1 hook: after H102 adopt completes, assert lane storage equals H102's store
and the click outbox is cleared. The dropped-callback mutation turns the two
hook tests red.
runWholeBlobP2a1HookSuite signature extended with storageKey and readOutbox;
callers (sections, sort) pass readOutbox from their respective lane modules.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
IMPORTANT 1 — add C1-stale actual-hook test (failed-bootstrap stale replay must
adopt, not publish). Causality: drop !pendingIsRestoredReplay guard → exception
fires → publishes stale edit over newer head → 2 reds (sections + sort).
IMPORTANT 2 — assert original queuedAt persists in transferred v2 outbox before
debounce fires. Causality: replace restoredQueuedAt with undefined in writeOutbox
call → stamps wall-clock value → 2 reds (sections + sort).
IMPORTANT 3 — add runWholeBlobP2a1HookSuite driving blocked-bootstrap sequence
through the real hook: bootstrap parked → click via hook API → H102 live → release
bootstrap → real .then() reads/replays outbox naturally → debounce → H102 adopted
as genuine advance. Causality: publishBaseline = lastRemoteHead in publish(_, true)
→ H102 folds into baseline → publishes over H102 → 2 reds (hook: sections + sort).
MINOR — extract clearPendingState() helper shared by discardPending() and
adoptRemote(), so adoptRemote's gen===pendingGeneration branch also clears
pendingIsRestoredReplay and pendingRestoredQueuedAt on adopt.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
CRITICAL 1 (C1): restored replay must carry original queuedAt age.
- WholeBlobSyncManager.publish() now takes restoredQueuedAt param;
pendingRestoredQueuedAt drives the adopt-guard in
fetchOwnBlobBeforePublish (publish when remote.createdAt <=
restoredQueuedAt; adopt when strictly newer). writeOutbox called
with original queuedAt so on-disk stamp is never reminted.
- channelSectionsSync/channelSortSync publishSections/publishSortPrefs
accept and thread restoredQueuedAt through to publish().
- channelSectionsStorage/channelSortPreference writeOutbox signatures
updated to accept optional nowSecs override.
- Hook-layer tests in runWholeBlobCarlSuite use the actual React hook
(renderHook), driving the real bootstrap .then() callback, outbox
read, queuedAt, and shouldReplay guard.
CRITICAL 2 (C2): successful-bootstrap replay needs a baseline.
- bootstrap() snapshots the fetched head to this.bootstrapResultHead
before returning. publish(_, true) uses canonicalMax(current,
bootstrapResultHead) so a no-prior-pending restored replay gets the
correct baseline from the immutable bootstrap snapshot rather than
the mutable lastRemoteHead (which may carry a suppressed live peer
head H102 that must remain a genuine advance).
- New runWholeBlobC2Suite (manager layer) drives publish(_, true,
undefined) so C1's queuedAt guard is disabled and only C2's
bootstrapResultHead mechanism can prevent the adopt.
CRITICAL 3 (C3): confirmRetainedHead decrypt gap.
- For a foreign winner, lastRemoteHead is advanced only AFTER
decryptAndParse succeeds (mirrors fetchRemoteBlob fix). A click
arriving during the confirm decrypt gap now sees lastRemoteHead as
{0,""} (or the pre-confirm value) and correctly adopts the foreign
winner rather than publishing over it.
- New runWholeBlobC3Suite (manager layer) gates the first decrypt call
and injects a click during the gap; asserts publishCalls === 1.
CRITICAL 4 (C4 / P3-reclaim): reclaimSubsumedOutbox drops preservedKey.
- mergeLaneStorage.shared.ts reclaimSubsumedOutbox signature updated
to accept optional preservedKey; calls isSubsumedBy(record.store,
head, record.preservedKey) so a capacity-bounded proof can never
evict the reserved channel and falsely certify retention.
- New P3-reclaim tests (already in mergeLaneSyncCarl.shared.test.mjs)
exercise 500-entry relay head + foreign outbox with clicked channel
absent; mutation (drop preservedKey) → record deleted → test red.
IMPORTANT: test coverage.
- P1/C1 hook tests now drive renderHook with the real outbox read,
queuedAt threading, shouldReplay guard, and bootstrap .then()
callback (not manager.publishReplay directly).
- P3-ack fixture clock set to t=1s so clicked channel is the
lexicographically evicted entry at 501-entry capacity, making the
pendingPreservedKey-drop mutation genuinely causal (2 red).
- All 6 named mutations verified red on exactly their tests, green
on fixed source: C1 (2r), C2 (2r), C3 (2r), C4/P3-reclaim (2r),
P3-ack (2r), P2a-1 (2r), P2b (2r). 57/57 pass on fixed source.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
P1 — restored-outbox replay re-mints queuedAt and triggers the
failed-bootstrap exception designed only for fresh in-session clicks.
Hook-replayed outbox calls now pass isRestoredReplay=true through
publishSections/publishSortPrefs → publish(), which suppresses the
exception (pendingIsRestoredReplay) and does not re-freeze publishBaseline
from mutable lastRemoteHead.
P2a-1 — hook .then() replay after releaseDeferred establishes the correct
baseline called publish() unconditionally, overwriting the baseline with
the mutable lastRemoteHead (which by then could carry a suppressed live
peer head). The isRestoredReplay flag added for P1 also closes this path:
restored replays skip the publishBaseline re-freeze so the baseline
releaseDeferred established is preserved through the callback.
P2b — fetchRemoteBlob (periodic/reconnect path) called recordRemoteHead
before decryptAndParse, leaving a decrypt-gap window where a concurrent
click could freeze its publishBaseline against the pre-decrypt head.
Moved recordRemoteHead to after decryptAndParse succeeds, mirroring the
existing subscribeLive fix.
P3 — isStarsStoreSubsumedBy / isMutesStoreSubsumedBy proved subsumption
via a capacity-bounded mergeStores(head, candidate) without a preservedKey.
At the 500-entry cap the clicked channel (lexicographically smallest) could
be evicted during the proof merge, certifying retention of a click the relay
never kept. Two consumers affected: bootstrap suppression (hook files pass
outboxMeta.preservedKey to the subsumption check) and ACK confirmation
(mergeLaneSyncManager passes this.pendingPreservedKey to config.isSubsumedBy).
Both storage functions now take an optional preservedKey and use a direct
membership check when present: head must contain the preserved channel with
rev >= candidate's before the rest of the store is verified.
Regressions — hook-layer tests in new shared suites:
- wholeBlobSyncCarl.shared.test.mjs: P1 (failed-bootstrap replay adopts
newer retained head), P2a-1 (H102 survives hook replay as genuine advance),
P2b (fetchRemoteBlob decrypt gap). Wired from useChannelSections and
useChannelSortPreference test files.
- mergeLaneSyncCarl.shared.test.mjs: P3-bootstrap (500-entry head does not
subsume click via eviction) and P3-ack (confirmRetainedHeadSubsumes retains
outbox when relay head omits clicked channel). Wired from useChannelStars
and useChannelMutes test files.
All 47 targeted tests pass (43 pre-existing + 4 new Carl-round P3 tests
plus 6 new P1/P2a-1/P2b tests). T1-T6 manager-level contracts unchanged.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Thufir's focused pass (890de2d90) identified three executable loss/overwrite
paths and two test-gate gaps. This commit addresses all five findings.
Production fixes:
1. Absent-bootstrap seed must not clobber a pending edit.
When bootstrap resolves absent and local state is non-empty, runBootstrap
calls publishFn(localStore) to seed the relay. If a click was already
pending (pendingStore !== null), that click IS the seed intent; calling
publish(localStore) again bumps the generation and overwrites the outbox
with the stale mount snapshot. Fix: guard the publishFn lambda so
publish(localStore) is a no-op when a pending edit already exists.
2. releaseDeferred must use the bootstrap-result head, not mutable lastRemoteHead.
subscribeLive can advance lastRemoteHead with a live peer head while bootstrap
is in-flight. A live head arriving after the click but before bootstrap
resolves must remain a genuine remote advance — if releaseDeferred re-freezes
publishBaseline from lastRemoteHead, the live head folds into the baseline,
the pre-publish check sees equality, and the edit publishes over the peer.
Fix: derive bootstrapResultHead from fetchResult directly (not lastRemoteHead),
and pass it into releaseDeferred. fetchResult carries exactly the head the
bootstrap operation itself fetched; subscribeLive's passive observations
are invisible to it.
3. Failed-bootstrap exception must be disarmed once any head is independently
observed (live or reconnect/periodic fetch).
bootstrapFailed+empty-baseline correctly identifies the "first unknown base"
case, but after a transient pre-publish fetch failure and a subsequent live
peer head arrival, the exception re-fires on retry and treats a genuine peer
advance as a base-establishment, publishing over it. Fix: add
bootstrapFailedExternalHeadObserved flag; set it from subscribeLive decode
completion and from fetchRemoteBlob when it finds a head while
bootstrapFailed is true. The exception checks !bootstrapFailedExternalHeadObserved.
Test-gate fixes:
- P2a cases moved to a new wholeBlobSyncP2a.shared.test.mjs (449 lines) and
exported as runWholeBlobP2aSuite; called from both useChannelSections.test.mjs
and useChannelSortPreference.test.mjs for two-lane coverage. Removes the P2a
block from wholeBlobSync.shared.test.mjs (841 lines, down from 1080).
- Five causal regressions, each named by its exact production mutation:
T1: drop bootstrapStarted guard → baseline {0,""} at debounce → adopt fires.
T2: releaseDeferred uses mutable lastRemoteHead → live head folds into baseline
→ pre-publish sees equality → publish over peer (load-bearing for defect 2).
T3: drop pendingStore===null guard → seed clobbers edit → stale mount published.
T4: drop !bootstrapFailedExternalHeadObserved → exception fires after live
observation → publish over genuine peer advance.
T5 (Kalvin's original): remove bootstrapFailed exception block → hard-adopt
against {0,""} → click lost (load-bearing for defect 3).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
P1: Gate bootstrap outbox replay on queuedAt > appliedHead.createdAt in
both whole-blob hooks (useChannelSections, useChannelSortPreference).
Stale records (queuedAt <= head.createdAt) are left for reclaimSuperseded*
cleanup. Legacy records (queuedAt=0) under a non-zero head are skipped.
Exposes queuedAt from resumeWholeBlobOutbox and the two outbox read helpers.
P2b: Split subscribeLive in wholeBlobSyncManager — watermark+lastRemoteCreatedAt
advance stays synchronous (preserves undecryptable-event seed-publish guard),
lastRemoteHead tuple update moved into the .then() after successful decrypt.
Prevents publishing pre-decrypt content over remote-only changes.
P3: Thread preservedKey through publish(store, preservedKey?) ->
pendingPreservedKey field -> fetchOwnBlobBeforePublish -> mergeWithRemote
in MergeLaneSyncManager. Both channelStarsSync/channelMutesSync wrappers
and both useChannelStars/useChannelMutes click handlers updated to forward
the channelId as preservedKey.
Regression tests:
- wholeBlobSync.shared.test.mjs: P1 stale-replay (manager level, both lanes
via SyntheticWholeBlobManager) + P2b decrypt-gap sequence
- wholeBlobHook.shared.test.mjs: P1 stale outbox not replayed test added to
runWholeBlobHookSuite; useChannelSections + useChannelSortPreference opt in
- channelStarsSync.test.mjs: P3 501-entry merge with preservedKey retains
the clicked channel
P2a (publish-baseline contract for failed bootstrap) held pending Will's
ruling on Option A (narrow guard) vs Option B (queue-until-bootstrap).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wholeBlobSyncManager: no-op guard now compares only against the freshly
fetched readable head (decision.fetchedRemote.store). Absent, failed,
unreadable, and foreign-author pre-publish paths can never no-op; the
stale lastPublishedStore comparison is removed entirely.
useChannelMutes/useChannelStars: comment-only, removing obsolete field
reference that no longer exists.
Test consolidation (22 files, 2,570 ins / 3,008 del):
- 17 changed test files: 4,458 lines; all 24 head sidebar lib test
files: 4,786 lines; caller files 6 lines each
- Helper sidebarSyncTestHelpers.mjs: 487 lines; sibling
sidebarAdapterContractTestHelpers.mjs (new): 587 lines; both under
the 1,000-line repo ceiling (AGENTS.md:590)
- Whole-surface total (24 tests + 2 helpers): 5,860 lines (was 6,303,
base 23 tests + 1 helper; -443)
- 5,518/5,518 desktop tests passing at this commit; no new biome
warnings in changed files (pre-existing: channelMutesStorage
.test.mjs:115-116 useLiteralKeys, SetupStep.acpForcedGate.test.mjs
:423 useOptionalChain, wholeBlobSync.shared.test.mjs:554
noUnusedFunctionParameters, terminal.css:276 noImportantStyles)
Adapter-contract refactor: _runAdapterContract (15-knob generic)
deleted; runSectionsAdapterContract and runSortAdapterContract are
genuinely concrete, each using typed lane APIs and lane-local constants
inline. Repeated test infrastructure (assert, test, mock, relayClient,
relay URL) hoisted into _adapterCtx() cache + ADAPTER_RELAY constant;
phases 2/3 use _runDurableResumeTest / _runUnsupportedVersionTest with
5 lane-specific params each (laneLabel, Manager, publishTo, readOutbox/
badVersionPayload, makeStore). Adapter section split into
sidebarAdapterContractTestHelpers.mjs (587 lines) to satisfy the
1,000-line ceiling; re-exported from main helper so callers are
unaffected. grep confirms zero old descriptor knob names in either file.
Test coverage added:
- wholeBlobSync: S1->S2->S1 folded lifecycle - S2 delivery observed via
subscription callback; S1 pending and second durable outbox write
asserted before confirmation; clear on exact retained-ACK only
- mergeLaneHook: click-before-opposite-peer-arrival with causal mutations
(a) drop listener -> idsField hook-membership assertion fails; (b)
suppress fetchRemote* inside tick -> fetch-count assertion fails; (c)
drop fetched result -> stored entry stays at arrival tuple (900,12)
not relay head (950,13)
- Adapter runner (both lanes): opposite-lane sentinel deep-equal before
own-outbox-cleared; fetch and live filters exact-object deep-equal;
fetched and callback stores exact deep-equal (sections includes
assignments; sort has groups.p === 'recent'); unsupported-version
manager.destroy() in finally
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Six categories of test bugs introduced during the compression pass:
1. Unbound restore() in finally blocks (makeTimerBed pattern) — fixed
to t.restore() in mergeLaneSync.shared.test.mjs.
2. Stale key-ID snapshot in mergeLaneHook v2 own-key test — writeOwnOutbox
replaces the bootstrap key on click; test now enumerates current keys.
3. Missing assertionsAfterRestore in mergeLaneStorage destructuring — fixed.
4. Legacy-delete no-op: second readChannelStarsStore call moved from
assertions (mock still active) to assertionsAfterRestore.
5. channelStarsSync / channelMutesSync writeOutbox arg order — production
signature is (pubkey, store, relayUrl); args 2-3 were swapped.
6. cancelPendingPublish does not clear pendingStore (by design); removed
incorrect getPendingStore() === null assertions in typed-API tests.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Extract makeTimerBed() helpers in wholeBlobSync.shared.test.mjs and
channelSectionsSync.test.mjs, eliminating 18-22 line inline timer
factory blocks repeated across 9 tests.
Collapse the A-succeeds/A-fails CAS variants in mergeLaneSync.shared
and the 4 bootstrap seed-guard tests in both mergeLaneSync.shared and
wholeBlobSync.shared into parameterized for-loops.
No behavioral coverage removed; only structural duplication eliminated.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
P1.1: Both whole-blob storage event handlers (sections + sort) replaced React
state unconditionally from the shared scoped cache, bypassing the
hasPendingEdit() guard that applyRemote uses for relay arrivals. A peer window
writing B1 while A has a pending edit (A1) caused the updater for A2 to derive
from B1, stranding A1's outbox record (Carl r8).
Fix: check hasPendingEdit() in both storage handlers; defer the peer cache
write until the pending edit's debounced publish-or-adopt cycle completes.
When nothing is pending the peer write applies immediately, preserving
cross-window state mirroring for the idle case.
P1.2: parseChannelSectionPayload did not check version; relay decryptAndParse
called it bare, so a future {version: 2} relay head parsed as v1 state instead
of triggering the retain/retry path. Sort, stars, and mutes all reject non-v1
in their parsers; sections was the odd one out.
Fix: add version !== 1 guard at the top of parseChannelSectionPayload. The
local-storage parse path (parseRaw) already checked version before calling the
parser, so no local-storage read is affected.
Tests: two new hook regression tests (sections + sort) for P1.1 using
StorageEvent dispatch; one new manager regression test and one new storage unit
test for P1.2; existing parser tests updated to include version: 1 in
previously version-less test fixtures.
Co-authored-by: Will Pfleger <wpfleger96@gmail.com>
Signed-off-by: Will Pfleger <wpfleger96@gmail.com>
Close the pass-3 review blocker and Carl's four P1s on the channel
sections/sort/stars/mutes relay sync, all in the sidebar lib:
- Gate the legacy-consumed marker on a proven v2 transfer. writeOwnOutbox
now returns whether the fresh key's setItem succeeded; the whole-blob
hooks write the marker only when durable, so a quota failure leaves the
legacy record replayable instead of silently suppressing the only copy.
- Reconnect wakes the existing pending edit (retryPendingPublish) rather
than re-queueing via publish*(), which reset the frozen baseline and
published a stale edit over an advanced remote.
- Merge lanes clear an own outbox only after an authoritative retained-head
fetch proves the head subsumes the attempted store; a bare EVENT OK on a
superseded NIP-33 write no longer drops the loser's durable click.
- An unreadable/unsupported pre-publish head retains the pending edit and
retries rather than overwriting state the client could not inspect.
- Document the accepted mixed-fleet residual (a same-second old-build click
reads rev 0 and loses to an earlier new-build rev, healing on the next
later-second click) and pin it with a test; no protocol change.
Regressions added for each item, mutation-verified where the fix lives in
the hook.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
localStorage has no atomic compare-and-delete or transactional
read-modify-write, so a single outbox key shared across every window
could not be mutated safely: one window's read-then-write or
read-then-remove races a peer's write in the gap and drops its
still-unpublished edit. A per-write ownership token narrowed that window
but could not close it — the token proves what was read, not what is
still stored at the destructive op.
Key the outbox per window instead: <prefix>:<pubkey>:<relay>:<nonce>,
where the nonce is minted once and parked in sessionStorage. Each window
is the sole writer of its own key, so a hot-path write is one
unconditional setItem — the write race is designed out, not guarded.
Resume enumerates every window's key: merge lanes (stars/mutes) fold all
records order-independently; whole-blob lanes (sort/sections) replay the
max-queuedAt record with a nonce tiebreak. Redundant foreign keys are
reclaimed at boot, gated on durable relay evidence (merge: head subsumes;
whole-blob: head created_at supersedes) and re-read immediately before
removal so a live peer's fresh write in the recheck gap survives.
Reclamation runs only on a successful head fetch, never on a failed one.
The token contract is deleted entirely — ownership is the key.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Carl's re-review found three edit-loss paths remaining after the
stars/mutes rev-merge landed.
[P2] Stars, mutes, and sort stamped createdAt = lastRemoteCreatedAt + 1
uncapped, so a self-authored head accepted near the relay's +900s drift
boundary wedged every later publish until wall time caught up. Extract
the sections clamp into a shared clampPublishCreatedAt in
sidebarSyncWatermark.ts (all four surfaces already import that module)
and wire sections/stars/mutes/sort to it.
[P1] Sort preferences never got the durable lane: doPublish only logged
on failure, destroy() discarded the pending edit, and a live remote
cancelled the debounce with the edit stranded. Port the sections lane —
durable outbox + bootstrap resume, generation/CAS ownership, single-
flight + completion re-drive, 2s->30s backoff, 60s reconcile loop, and
pending-aware remote application. Sort stays whole-blob LWW; a lost head
is adopted at pre-publish rather than republished.
Tests: a clamp test per surface, and sort lifecycle coverage — outbox
resume after destroy-inside-debounce, retry without a later edit,
overlapping-generation safety, live-remote-during-debounce adopt, and a
hook-level pending-defer test.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
useChannelStars, useChannelMutes, and useChannelSortPreference carried the
same inverted equal-timestamp comparator as channel sections: applyRemote kept
the largest event id, opposite the relay/database canonical order (created_at
DESC, id ASC -> lowest id wins). Two devices writing the same second could
leave the UI showing an event the relay did not store.
Apply a strictly-lower id and ignore ids >= the last applied, matching the
sections fix and the relay winner across all four 30078 sidebar surfaces. Each
hook gains a regression test: larger-then-lower id delivery at equal timestamp,
lower-id store wins (mutation-checked - reverting >= to <= fails each).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
