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>
Plain replacement `publishBaseline = { ...bootstrapResultHead }` discarded the
queue-time baseline when bootstrap resolved with an older head. If a live peer
head B fully applied before the user clicked, publish() froze publishBaseline=B,
but releaseDeferred then regressed it to the bootstrap-result head A. The
pre-publish fetch saw B as an advance and adopted the click away (0 publishes,
1 adopt).
Fix: use canonicalMax(this.publishBaseline, bootstrapResultHead), preserving
whichever head is canonically greater. If the click was authored from B and
bootstrap returned older A, canonicalMax keeps B as the baseline and the edit
publishes above B correctly. For a failed bootstrap, bootstrapResultHead is
{0,""} and canonicalMax leaves the queue-time baseline unchanged.
Add T6 regression to the P2a shared suite (runs through both
ChannelSectionSyncManager and ChannelSortSyncManager): blocked bootstrap on A
→ live B applies → click from B → bootstrap resolves A → edit publishes above
B. Failing mutation: revert to plain replacement → baseline regresses B→A →
pre-publish adopts B (0 publishes, 1 adopt).
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>
No publish debounce completes until bootstrap resolves. Edits queued
during an unresolved bootstrap store their intent durably (outbox write +
in-memory pending state) but the debounce timer is deferred. On
resolution, releaseDeferred() re-freezes publishBaseline from the
now-established lastRemoteHead before scheduling the timer, so the
pre-publish fetch sees the bootstrap head as 'not advanced' and
publishes above it rather than adopting it away.
Failed-bootstrap corner: when bootstrap fails, lastRemoteHead stays
{0,""} and bootstrapFailed=true is recorded. The empty-baseline
exception in fetchOwnBlobBeforePublish fires only when both flags
indicate a failed bootstrap with no known head, treating the first
fetched head as the new baseline rather than a competing remote.
Three causal regressions in wholeBlobSync.shared.test.mjs:
- P2a test 1 (load-bearing): click during unresolved bootstrap on a
fresh device publishes above the relay head; failing mutation: remove
bootstrapStarted guard from publish() → {0,""} baseline → adopt.
- P2a test 2: genuinely newer peer head that arrives after bootstrap
is adopted via normal LWW (publishBaseline re-frozen to bootstrap
head, peer head at higher ts is a genuine advance).
- P2a test 3 (Kalvin's original sequence): click after failed bootstrap
publishes above the relay head; failing mutation: remove bootstrapFailed
exception → hard-adopt against {0,""} baseline.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Without bestKey in the per-record reduce, folding two durable outbox records
totaling 501 entries evicts the clicked channel before the final bound can
protect it. Thread bestKey through merge(acc, r.store, bestKey) on every step
so the reservation is upheld at each intermediate capacity-bound merge.
Update the merge callback signature from (a, b) to (a, b, preservedKey?) —
both mergeStores implementations already accept the optional third arg.
Also fix two test gaps from Thufir pass 3:
reconnect test: both the reconnect-callback fetch and the subsequent
pre-publish fetch must return the remote 500-entry head so that reverting
the hook reconnect to publish*(pending) triggers the 501-entry merge and
fails. The previous single-return-on-call-1 mock left the pre-publish fetch
returning [] — no 501-merge occurred and the wrong wiring stayed green.
restart test: upgrade from one to two foreign-nonce outbox records. Record 1
carries the clicked channel + 499 entries with preservedKey; record 2 carries
the 501st entry with no key. This exercises the per-record fold eviction
(mutation c) directly: without bestKey in the reduce the fold evicts the click
at the record-2 merge, before the final bound runs. The single-record test
could not catch this class of defect.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Bootstrap fetch returns [] (hold) so publishStars is called unconditionally —
the subsumed guard only runs on apply-remote, which would require extra async
ticks to verify. The 501-entry merge still exercises at the production seam:
when the 2s debounce fires and doPublish calls fetchRemoteStars (fetchCalls=2),
the 500-entry remote head triggers mergeWithRemote producing 501 entries. Without
preservedKey forwarded from the foreign record, boundStarStore evicts the clicked
channel there. The test structure now matches the existing bootstrap replay test
(hold bootstrap → timer fires → publish cycle exercises the production path).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Thufir pass-2 findings — all four corrections applied:
1. P3 restart durability: replace two-function readOutbox+readOutboxPreservedKey
with readOutboxWithMeta<S> that enumerates records once and returns
{ store, preservedKey }. Key selected by max queuedAt across ALL records (own
and foreign); after quit, the prior window's nonce is gone so its record is
foreign — the old isOwn filter returned undefined, losing the reservation.
Both hooks forward outboxMeta.preservedKey directly to publish* on bootstrap.
2. Two-read snapshot/provenance split closed: the capacity bound now applies
inside readOutboxWithMeta using the selected preservedKey, so the clicked
channel is never evicted before it reaches the manager.
3. discardPending() clears pendingPreservedKey: minor correctness fix so a
confirmed publish resets the field alongside pendingStore and clearOutbox.
4. Test consolidation: remove non-causal reconnect/remount tests from
channelStarsSync.test.mjs (+120) and channelMutesSync.test.mjs (+192).
Replace with two parameterized regressions in mergeLaneHook.shared.test.mjs:
- P3 reconnect: triggers the actual subscribeToReconnects callback registered
by the hook, asserts clicked channel survives 501-entry merge.
Failing mutation: revert hook reconnect to publish*(pending).
- P3 restart: seeds a foreign-nonce outbox envelope with preservedKey,
mounts hook (bootstrap reads foreign record → recovers key → publishes),
fires 2s debounce, asserts clicked channel survives 501-entry merge.
Failing mutation: revert to readOutboxPreservedKey (isOwn filter) or drop
bootstrap preservedKey forward.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
P1 guard shape fix (both whole-blob hooks):
- Was: reduce all non-apply-remote paths to headCreatedAt=0, gate on queuedAt>0
- Now: branch on bootstrap result explicitly
- hold/absent/failed (result.action !== 'apply-remote'): always replay,
including legacy queuedAt=0 records (old 0>0 guard silently stranded them)
- apply-remote: suppress only on strict queuedAt < appliedHead.createdAt;
equality replays (aligns with reclamation's strict-lt, prevents stranding
a same-second edit neither consumed nor published)
- Documents clock-skew residual: queuedAt vs relay created_at are independent
wall clocks — accepted tradeoff of the existing LWW max-merge strategy
P3 reconnect preservation (MergeLaneSyncManager + both stars/mutes hooks):
- Add retryReconnectPublish() to MergeLaneSyncManager: re-drives the existing
generation without calling publish(), so pendingPreservedKey is not reset
- Expose retryReconnectStarsPublish() / retryReconnectMutesPublish() on adapters
- Switch both hooks' reconnect effects to retryReconnectPublish() instead of
publishStars/Mutes(pending), which would open a new generation and clear key
- Clear pendingPreservedKey in destroy()
P3 remount preservation (storage layer + both hooks):
- writeOwnOutbox: accept optional preservedKey, include in envelope JSON
- parseEnvelope: read preservedKey from envelope; OutboxRecord gains field
- enumerateOutbox: propagate preservedKey into OutboxRecord
- mergeLaneStorage.shared: writeOutbox forwards preservedKey; add
readOutboxPreservedKey to read own-window record's key for bootstrap
- channelStarsStorage / channelMutesStorage: accept+forward preservedKey in
write, expose readChannelStarsOutboxPreservedKey / readChannelMutesOutboxPreservedKey
- MergeLaneSyncManager: writeOutbox config type includes preservedKey param;
publish() forwards it to config.writeOutbox
- useChannelStars / useChannelMutes: import and call readPreservedKey on
bootstrap outbox replay; pass to publishStars/Mutes alongside the store
Causal test coverage:
- wholeBlobHook.shared: replace non-causal stale-replay test with four-case
P1 regression using makeHookTimerBed.hasDelay(2000) as the synchronous
causal signal (timer present iff publish() called): strict-lt suppresses,
equality replays, hold+v2 replays, hold+legacy-queuedAt=0 replays
- channelStarsSync.test: add P3 reconnect (retryReconnectStarsPublish keeps
key) and P3 remount (storage round-trip write+read preservedKey)
- channelMutesSync.test: add matching P3 501-entry, reconnect, and remount
regressions parameterizing coverage across both lanes
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The P2b decrypt-gap test assumed fetchEvents would be called twice
(once for bootstrap, once for pre-publish fetch), but the test never
calls bootstrap(). The first and only fetchEvents call goes to
fetchOwnBlobBeforePublish; returning [] there produces a 'publish'
decision and the test fails.
Remove the incorrect two-branch mock. The single fetchEvents call is
the pre-publish fetch; it returns the live event as decryptable
(good-cipher) so fetchOwnBlobBeforePublish sees remoteAdvancedSince
true → adopt, verifying the P2b fix.
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>
The field was documented but runMergeLaneStorageSuite() never accepted or
used it. Remove the orphaned JSDoc line.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The shared mergeLaneStorage suite was missing two tests that existed in the
original per-lane files: idsFromStore returns the set of IDs where the value
field is true, and idsFromStore returns an empty set for all-false/empty
stores. The idsFromStore helper was advertised in the usage comment but never
added to the config destructure or exercised by the suite.
Add idsFromStore to the suite's config parameter, add the two parameterized
tests, pass starredChannelIdsFromStore / mutedChannelIdsFromStore from the
respective wrappers, and remove the stale isSubsumedBy line from the usage
comment (it was never wired).
Stars and mutes storage suites now each register 47 tests, matching the
original per-lane count.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The shared wholeBlobSync suite's "second edit queued during confirmation
survives" test minted the peer winner with an auto-generated peer-N id,
which sorts lexicographically above evt-N. remoteAdvancedSince therefore
returned false and edit-2 published without either foldSupersedingAttemptWinner
repair path firing — both could be removed without failing the test.
Restore the explicit "0-peer-winner" id (lexicographically < evt-N), matching
the original channelSectionsSync.test.mjs fixture and the other two collision
tests in the same shared file. This makes remoteAdvancedSince return true so
the stale-gen adoptRemote repair path (line 401) fires on confirmation, and
the pre-publish repair path (line 538) acts as the safety net for the retry
case. Removing both fold call sites now fails the regression as required.
Updated inline comments to describe the corrected execution path. No change
to assertions (publishCalls=2, adopted=0, pendingStore=null, outbox=null)
or to the fetch sequence.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The consolidated runWholeBlobSyncSuite had the 'second edit queued during
confirmation survives' test mock inverted from the original: it returned
empty for both fetchCalls 1 and 2, then hung at fetchCalls 3 (the retry
pre-pub). The original hung at fetchCalls 2 (the debounce confirmation),
matching the invariant's intent of queuing edit-2 while edit-1's
confirmation is paused.
The broken mock caused an infinite while-loop spin (releaseConfirmation
stayed null because fc=3 was never called — it required a second
fireDelay to fire the retry timer). Node's test runner detected the
event loop draining with the Promise still pending and reported
'Promise resolution is still pending but the event loop has already
resolved'.
Fix: restore the original call sequence (fc=1 empty pre-pub, fc=2
confirmation hangs) and the publishEvent mock (publishCalls=1 stores
winner without explicit id so peer-N > evt-N — remoteAdvancedSince
returns false for edit-2; publishCalls=2 stores the actual signed event
so confirmRetainedHead returns confirmed and discardPending fires rather
than adoptRemote). Expand assertions to match the original: stale-gen
adopt check, pendingStore check, and outbox clearance on confirmation.
All 315 sidebar tests pass.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Replace channels["toggled"] bracket notation with channels.toggled
dot notation on the write assignment — biome complexity/useLiteralKeys
flagged the single-word string key.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The undecryptable-live-event test fires a live-event callback that spawns
a dangling async chain (void decryptAndParse().then(...)). When placed
mid-suite, trailing microtasks from that chain could interfere with
subsequent timer-based tests. Moving it last keeps the suite hermetic.
Also removes the console.log debug calls that were left in from development.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Remove unused isSubsumedBy/idsFromStore imports from stars/mutes storage
wrappers (not consumed by runMergeLaneStorageSuite), drop RELAY_KEY from
sections/sort sync wrappers (not consumed by the suite), remove unused
makeStore function from sections sync wrapper (superseded by
makeSectionsStore), and remove the makeStore parameter from
runWholeBlobSyncSuite (declared but never accessed in the body).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Extract merge-lane storage helpers to mergeLaneStorage.shared.ts:
- channelStarsStorage.ts + channelMutesStorage.ts now delegate
readStore, writeOutbox, readOutbox, clearOutbox, reclaimSubsumedOutbox,
makeStorageKey, and claimLegacy to the shared module.
- channelSectionsStorage.ts imports claimLegacy from the shared module,
removing the inline duplicate (sections' readChannelSectionsStore and
outbox keep whole-blob watermark wiring).
Parameterize remaining test mirrors:
- mergeLaneStorage.shared.test.mjs: 47-invariant storage suite covering
all stars/mutes storage behaviors. channelStarsStorage.test.mjs and
channelMutesStorage.test.mjs become 32-line wrappers.
- mergeLaneHook.shared.test.mjs: shared hook suite. useChannelStars and
useChannelMutes tests become 22-line wrappers.
- wholeBlobSync.shared.test.mjs: 17-invariant shared whole-blob sync
suite. channelSectionsSync.test.mjs and channelSortSync.test.mjs
call runWholeBlobSyncSuite + retain their lane-specific tests
(serialized generations, ambiguous ACK, malformed payload,
unsupported-version, reconnect adopts, durable outbox, failed publish).
Every named regression from all prior review rounds survives identically,
relocated to the shared suite or kept in the lane file.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Extract mergeLaneSync.shared.test.mjs as a runMergeLaneSyncSuite()
parameterized function covering all 14 merge-lane invariants (observe
high-water, destroy cancel, generation CAS A-succeeds/A-fails, bounded
backoff, retention-confirmation loser, boot-seed guard suite, failed
fetch retain, unreadable head retain, unsupported schema retain, and
timestamp clamp). Replace the 668-line channelStarsSync.test.mjs and
channelMutesSync.test.mjs with thin 14-line wrappers that call the
shared suite with lane-specific config (Manager, outbox reader,
watermark key, entry factory, publish/getPending/fetchRemote adapters).
Every named regression survives identically — test titles are prefixed
with the lane label so each pass/fail maps to its originating invariant.
Total: 316 pass (was 315; one test was added by the shared teardown
loop). Net test-file delta: -1300 lines.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The four sidebar sync engines (sections, sort, stars, mutes) shared
nearly all implementation. The two whole-blob lanes (sections + sort)
carried the same generation-CAS, outbox, watermark, retained-head
confirmation, prior-gen fold, and bounded-retry machinery; the two
merge lanes (stars + mutes) differed only in store type and the
merge/subsumption functions.
Extract:
- wholeBlobSyncManager.ts: parameterized by WholeBlobLaneConfig<S>
(kind, dTag, parse, serializePayload, writeOutbox, clearOutbox,
storesEqual, isLocalNonEmpty). Preserves all pass-2/pass-3/r7
invariants. ChannelSectionSyncManager and ChannelSortSyncManager
are now thin wrappers (~75 lines each) that pass lane config and
expose their named API methods.
- mergeLaneSyncManager.ts: parameterized by MergeLaneConfig<S>
(kind, dTag, parse, serializePayload, mergeWithRemote, isSubsumedBy,
storesEqual, observe, writeOutbox, clearOutbox, isLocalNonEmpty).
ChannelStarSyncManager and ChannelMuteSyncManager are thin wrappers
(~90 lines each).
All 315 sidebar lib tests pass at this commit with no behavior change.
Each lane wrapper still exposes its named public API (publishSections,
publishSortPrefs, publishStars, publishMutes, fetchRemoteSections,
subscribeToSections, etc.) so no hook changes are required.
Co-authored-by: Will Pfleger <wpfleger96@gmail.com>
Signed-off-by: Will Pfleger <wpfleger96@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>
Track attempt id → generation in ambiguousAttemptIds (Map<string,number>)
instead of a plain Set. foldSupersedingAttemptWinner now requires that the
mapped generation is strictly less than pendingGeneration, making it
explicit that the fold only applies when a newer edit's baseline was
poisoned by an older generation's unconfirmed attempt — not when the
current generation's own in-flight attempt is the baseline.
The generation invariant is structurally guaranteed (publishBaseline is
frozen before the attempt id is known), but the explicit guard provides
defense in depth against future code changes and makes the constraint
self-documenting.
Add two regressions per lane (sections + sort):
- Single-edit confirmation-retain path: ACK resolves, confirmation
returns retain, retry sees peer winner → adopt, do not republish.
Companion test proving correct adopt behavior in the no-second-edit path.
- Queued-during-confirmation: second edit queued after ACK but before
confirmRetainedHead resolves; stale-gen adoptRemote folds winner into
baseline; second edit's pre-publish check publishes above true head.
Mutation: reverting adoptRemote fold fails this test.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
When a second edit is queued after a publish ACK but before
confirmRetainedHead resolves, pendingGeneration bumps while the first
edit's confirmation fetch is in flight. The second edit's publishBaseline
is frozen against the attempted (non-retained) event id. When confirmation
returns the peer's same-second lower-id winner via adoptRemote with stale
gen, the phantom baseline causes the second edit's pre-publish check to see
the winner as having advanced past our attempt and adopt it away — the
user's second edit is silently lost (Carl r6 P1).
Extract foldSupersedingAttemptWinner (shared helper, both managers): when
publishBaseline.eventId is one of our own attempted ids and the remote is
the same-second lower-id peer winner, fold the winner into the baseline.
The pre-publish check then sees equality and publishes above the true
retained head. Scoped to same-second lower-id only — a strictly-later
remote is a genuine advance that the pending edit must still adopt
(pass-2 invariant). Invoke from adoptRemote stale-gen path (confirmation
variant) and from fetchOwnBlobBeforePublish (confirmation-retain/retry
variant). Both managers (sections, sort) mirrored.
Add one regression per manager: pauses the confirmation fetch, queues the
second edit during the pause, then releases with the peer winner. Asserts
the second edit publishes above the winner and is confirmed retained rather
than adopted away. Mutation: reverting the adoptRemote fold fails the test.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The pass-3 catch rolled back the just-written scoped copy on any failure.
If the legacy delete succeeded but the confirmation read then threw, legacy
was already gone, so the rollback destroyed the only surviving copy —
permanent preference loss.
Roll back only when the legacy key is provably still importable (keeping the
scoped copy then would let a second relay scope double-import it). If legacy
is gone, the scoped copy is the sole copy and is kept; if the probe itself
throws, keep it too, favoring no-data-loss. The read-time ownership gate
keeps a kept-but-unproven copy hidden while legacy remains, so it never seeds
early. Residual: legacy-delete and catch-probe both throw, then a relay
switch after partial recovery can let both scopes carry the value — bounded
and deliberately preferred over data loss.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The migration wrote the scoped key before deleting the legacy key. If the
legacy delete threw and the rollback delete also threw, the scoped copy
survived and the next read gave it precedence, exposing legacy prefs to
seed-publish while the legacy key was still importable by another relay.
Move the ownership check into the reader: a scoped copy is exposed only
while no importable legacy key remains, so a half-migrated copy can never
be read as owned. The claim completes inline once storage recovers.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Thufir's r6 review found the legacy→scoped migration exposed legacy data
to bootstrap even when the scoped write threw or the legacy delete did not
take, wrapped in a swallowed try/catch. On any storage failure the legacy
key survived while the relay still received the legacy preferences and
could seed-publish them; a second relay later read the same legacy key and
seeded it too — so "globally one-time" held only on the happy path.
migrateLegacyStore now exposes migrated data only when ownership is proven:
write the scoped key, delete the legacy key, and confirm the delete took.
On a scoped-write throw, a legacy-delete throw, or a delete that silently
does not take, roll back the partial scoped copy and return DEFAULT_STORE,
leaving the migration retryable. An empty store cannot seed-publish
(bootstrap's non-empty-local gate), so DEFAULT is safe.
The identical flaw existed in all three lanes with a legacy primary-cache
migration — sections (pre-existing on main), stars, and mutes. Sort has no
such migration and is untouched. Concurrent claimants are safe: every
window resolves the same app-wide active community, so at any instant they
scope to the same relay and any race migrates the identical value to the
identical key; a second relay only enters play after a community switch,
which is strictly after the first scoped read consumed the legacy key.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Carl r6 raised two P1 correctness gaps.
Whole-blob lanes (sections, sort) treated a resolved publishEvent as proof
of retention, but the relay OKs a superseded NIP-33 write as a no-op
(Duplicate). Two windows stamping the same created_at both get OK while
only the lower event id is retained, so the loser recorded a nonexistent
head as its baseline and its next edit adopted the true winner away. After
ACK the lanes now fetch the authoritative head and only clear/fold/discard
on an exact event-id match; a different readable head is adopted, an
unprovable/unreadable/own-prior head is retained and retried.
The stars/mutes primary cache was pubkey-only while its outbox was
relay-scoped, so a first visit to relay B seed-published relay A's
preferences. The cache key is now relay-scoped like sections/sort, with a
one-time legacy migration that deletes the pubkey-only key so no other
relay can import it.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The rev parse bound used Number.isSafeInteger, which accepts
Number.MAX_SAFE_INTEGER itself. The click path mints rev = maxRev + 1,
so an accepted boundary rev overflows to an unsafe value that never
advances again — recreating the same-second toggle wedge the bound
exists to prevent. Reject rev >= Number.MAX_SAFE_INTEGER (exclusive,
normalize to 0) so maxRev + 1 always stays safe. updatedAt keeps its
inclusive bound: the click path takes Math.max(now, observed), never
increments, so it cannot overflow.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Extends the retain-on-unreadable pattern to the two remaining overwrite
paths and hardens the rev/timestamp parse boundary.
- All four lanes tri-state the pre-publish read: a THROWN fetch (timeout /
auth / socket) now retains and retries instead of falling through to
publish, so an edit during a transient outage can no longer sign above a
stale watermark and erase an unseen newer head.
- The merge lanes (stars/mutes) retain when the existing head fails
decryption/JSON/schema parsing rather than publishing the local store
over it — a max-merge is only safe once both operands were read, matching
the whole-blob lanes.
- The rev/timestamp parsers require Number.isSafeInteger: an unsafe rev is
normalized to 0 and an unsafe timestamp entry is dropped, so a malformed
huge-rev true entry can no longer wedge later false toggles forever.
Retain reuses the existing bounded-backoff retry and generation CAS, so a
later readable/successful head resumes normal resolution. Regressions cover
all four lanes for the failed-fetch and unreadable-head paths and the
rev-wedge P2 case in both storage suites.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@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>
The legacy v1 shared outbox key is never deleted (it is mutable and a
concurrently-live old build may still rewrite it), so the whole-blob
resume path re-read it on every boot and republished the stale blob above
the current relay head forever. A found relay head never stopped it: a
fresh manager has no lastPublishedStore and queueing the replay freezes
publishBaseline to the just-fetched head.
Distinguish compatibility replay from permanently pending intent with a
durable per-value consumption marker, whole-blob lanes only. resumeWholeBlobOutbox
excludes the legacy record when its exact raw matches the stored marker;
a live old build rewriting the key stores a different raw and is replayed
again. The hook transfers the intent into its own v2 key (synchronous
publish) BEFORE writing the marker, so a crash in that gap replays the
blob once more rather than losing it.
Merge lanes (stars/mutes) need no marker but gained a head-subsumed gate
so a lingering legacy key does not re-drive an identical boot-time publish.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The per-window token/recheck reclamation still performed a non-atomic
compare-then-delete on a mutable foreign key: a live owner could rewrite
its key between the reclaim decision-read and the removeItem, and the
whole-blob `queuedAt <= head.created_at` gate dropped same-second and
legacy `queuedAt=0` records that had not provably lost LWW.
Records are now write-once: a key is `<prefix>:<pubkey>:<relay>:<nonce>:<seq>`
and is never rewritten. A new edit writes a new key (next zero-padded seq)
then deletes its own older keys (write-before-delete, so a crash leaves at
least one record). Foreign reclamation reads an immutable record, proves it
reclaimable against durable relay evidence, and deletes it with no recheck.
Whole-blob supersession is strict (`queuedAt < head.created_at`); replay runs
before reclamation in every hook so a same-second record is consumed into
pending first; the legacy v1 shared key is only ever replayed, never deleted.
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>
Sections, stars, mutes, and sort each persist an unpublished edit under one
localStorage outbox key per identity+relay, but generation ownership is only
in-memory per window. Without cross-window ownership, one window's completing
publish could clear a peer window's still-unpublished edit, and (on the merge
lanes) a peer's write could overwrite an edit before it published.
Every outbox write now mints an ownership token and stores a {store, token}
envelope; a completing publish compare-and-clears only when the stored token
still matches its own, so a peer's newer write survives an older window's ACK.
Stars and mutes additionally read-merge-write both the durable outbox and the
main store via their per-entry mergeStores, so two windows editing different
channels both survive; sort and sections replace whole-blob with LWW resolution
matching the relay. Sort also gains the durable outbox + bounded retry it
previously lacked, and stars/mutes/sort now clamp publish created_at inside the
relay's future-drift window like sections. The envelope reader tolerates a
legacy token-less entry so an outbox written by a prior build still resumes.
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>
The hard-eviction-branch test asserted only the rev tuple outcome on two
one-entry stores while its comment claimed the >500-entry eviction/remount
setup. Build the real fixture: >500 equal-updatedAt entries so the bound
evicts the target by the id tiebreak, then a remounted rev-1 click merged
against the retained rev-100 remote, asserting the deterministic rev-100
outcome in both merge orders. Stars and mutes.
The unobserved-future mixed-fleet residual was stated but never exercised
directly: the fast-clock suites cover only the observed-future fix. Add a
click with an empty high-water at t, then a genuinely unobserved
opposite-value head at t+300 that wins on the primary updatedAt key. Both
hook suites.
Test-only; no production source changes.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Replace the LWW register plus ownership/dirty-set/canonical-supersession
machinery with a per-entry Lamport `rev` and a single max-merge on every
path, mirroring the read-state data model. Each entry carries an additive
optional `rev` (missing implies 0; payload stays `version: 1` so older
builds keep parsing our blobs). One `mergeStores` orders by
updatedAt then rev then the starred/muted-true leaf, and ends in the
500-entry bound.
Clicks stamp `updatedAt = max(now, localEntry?.updatedAt ?? 0,
maxUpdatedAtSeen(id))` and mint `rev = max(localEntry.rev, maxRevSeen(id))
+ 1`, so a click strictly dominates every state its replica has observed
and cannot lose to a same-second remote. The sync managers hold a
per-channel two-field high-water map fed by a single `observe()` on every
ingest path. Stars sync keeps the generation-CAS + single-flight lane and
bounded-backoff retry plus a durable outbox so an in-flight publish can
never clear a newer pending edit; mutes sync mirrors it. Remote ingestion
never touches the pending lane. Deleted: mergeApplyingRemote,
mergeStoresWithTie, mergeCanonicalSupersession, dirtyChannelIds,
lastAppliedRemoteTs/lastAppliedEventId, and the event-clock branch.
Sections and sort are untouched.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Round-4 client-side convergence fixes for channel-sections/stars/mutes sync,
closing two silent edit-loss variants that survived publish serialization.
Ambiguous-ACK fold: a publish whose ACK is lost may still have been accepted
by the relay. Retain each attempt's signed id; when a later cycle's pre-publish
fetch returns a head whose id matches a prior attempt, fold it forward as our
own accepted predecessor and publish above it instead of adopting it away and
erasing the queued edit. A head the relay never accepted can never surface by
id, so the fold is proof-gated on an exact id match.
Canonical-supersession dirty overlay (stars + mutes): a lower-id canonical
correction that arrives after a same-second local click must not clobber the
click. Apply the correction to the prior remote layer, then overlay entries
changed locally since that layer; never cancel a pending publish merely because
a correction arrived.
Client-only: loss discovery relies on the existing pre-publish fetch, live
subscription, and reconcile loop rather than a relay conflict signal.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Each prior round patched one cross-generation interleaving and opened
another a layer deeper. Kill the race class structurally instead.
Sections: serialize publish cycles (one in-flight at a time; a newer edit
queued mid-cycle defers and the completion re-drives it). The per-edit
pre-publish baseline is frozen at queue time, so a genuine remote observed
during the debounce window still adopts, while our own accepted head is
folded forward via canonicalMax so a stale generation's own write is never
mistaken for a competing remote and adopted away. Dual generation guards in
doPublish (post-fetch and pre-publish) stop a stale generation signing or
publishing after a newer edit exists.
Stars/mutes: scope mergeApplyingRemote (remote-wins on entry-tie) and the
pending-publish cancel to fire only on a canonical supersession of an
already-applied same-timestamp larger-id head. Every other application
(bootstrap/live/newer-timestamp) keeps local-wins mergeStores and does not
cancel the pending publish, so a later same-second local click is no longer
clobbered by an older remote entry that decrypts late.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Two convergence holes one layer under the pass-1 fixes:
Sections: the pre-publish head check compared the fetched head against
the mutable lastRemoteCreatedAt, which a live event observed during the
debounce window already advanced to that same head — equality fell
through to publish and the local blob overwrote a remote that became
head after the edit was queued. Freeze a canonical head baseline
(created_at, id) at publishSections and compare the fetched head against
that generation baseline instead, adopting when the head advanced.
Stars/mutes: applyRemote admits the canonical lower-id winner but then
mergeStores resolved equal per-entry updatedAt as local/prev-wins, so a
stale larger-id value delivered first survived and undid the winner. Add
mergeApplyingRemote which resolves an entry-timestamp tie toward the
canonical incoming blob while keeping strictly-newer local entries.
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>
Three cross-layer races defeated the one-convergence-mechanism design:
- An older in-flight publish unconditionally cleared pending state on
completion, erasing a newer edit queued mid-flight. Each pending edit now
carries a monotonic generation; a completion clears pending/outbox/retry only
via compare-and-swap on the generation it published.
- Hook-level remote application (bootstrap/live/periodic) cancelled the pending
publish's timers without deciding supersession, stranding the durable outbox
and clobbering the optimistic edit. applyRemote now defers entirely to a
pending edit, whose own debounced publish converges via publish-or-adopt; the
manager's adopt path clears pending before write-through so the winning remote
still applies.
- The equal-timestamp tie-break kept the largest event id, opposite the
relay/database canonical order (created_at DESC, id ASC → lowest id wins).
applyRemote now applies a strictly-lower id and ignores ids >= the last
applied, so the UI converges on the event the relay actually stored.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Channel-section sidebar state diverged between a user's devices and
sometimes never self-healed. Four client-side gaps fed the divergence:
- A local edit that lost whole-blob LWW was silently republished as remote
content while the UI kept showing the edit. Now the manager adopts the
winning remote head (writes it through to state + storage, advances the
watermark) and skips publishing, unifying with the relay's OK-false
conflict path as one convergence mechanism.
- Edits made inside the 2s publish debounce were dropped on quit or
community switch. A durable localStorage outbox persists every edit
synchronously and resumes it on next mount; adopt clears the outbox so a
superseded edit can never be replayed.
- A skewed remote head could push the published createdAt past the relay's
future-drift window and wedge all later publishes. createdAt is now
clamped inside that window.
- Stale-at-open state waited for a reconnect that a healthy socket never
fires. A reconciliation loop periodically refetches the head (steady 60s,
backoff on failure) and refreshes on window visibility.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
## Why
Finish the replaceable-event slice of [tracker
#2](https://github.com/TheSentinel454/buzz/issues/2) and [domain issue
#6](https://github.com/TheSentinel454/buzz/issues/6) without disturbing
the runtime/store boundary established by #6660 and #6668. PR #6700 has
merged; this PR now targets current main containing its replacement lock
and transaction observability.
## What
- Move `Db::replace_addressable_event`, its SQL, lock/transaction
instrumentation, and focused addressable/parameterized replacement tests
from `lib.rs` to `replaceable.rs`
- Preserve the transaction-required parameterized API, replacement
ordering, rollback semantics, mention indexing, and exactly one
datastore span per public operation
- Keep implementation and focused PostgreSQL tests co-located; follow-up
review removed the dedicated replaceable ownership source guard as low
value
## Stack
- Exact base: main at f249710331
- Exact head: codex/issue-6-finish-replaceable-store at
da018405cc
- Tracker: https://github.com/TheSentinel454/buzz/issues/2
- Domain: https://github.com/TheSentinel454/buzz/issues/6
- Test/span acceptance: https://github.com/TheSentinel454/buzz/issues/17
and https://github.com/TheSentinel454/buzz/issues/19
## Non-goals
- No SQL, schema, retry, timeout, lock ordering, transaction boundary,
or client-visible behavior changes
- No store traits, domain-handle redesign, `PgExecutor` migration, raw
pool accessor, new crate, or directory reorganization
- No changes to, retargeting of, or merge action on PR #6700
## Risk Assessment
Low. This is a mechanical ownership move with unchanged signatures and
SQL. The primary review risk is losing or nesting instrumentation,
covered by exact moved PostgreSQL tests and the cumulative
production-diff audit.
## Blox Verification
Author workstation: `buzz-tornquist-issue-2-store-stack` (`2046520`),
exact head `2de5444e14606ce6912d1bb932bc88bffa379a77`.
- `cargo fmt --all --check` — passed
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings` —
passed
- `cargo test -p buzz-db --quiet` — 108 passed, 200 ignored; both
integration source guards passed
- Native PostgreSQL: `cargo test -p buzz-db replaceable::tests:: --
--ignored --test-threads=1` — 11 passed
- `cargo test -p buzz-relay --lib
api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo --
--exact --test-threads=1` — passed
- Full serial relay suite was also exercised: 908 tests passed but the
existing load-sensitive mesh demo test returned 504 under suite load,
matching PR #6700's documented baseline; it passed in isolation and this
diff does not touch that subsystem
Independent exact-head Blox review: `buzz-tornquist-pr-6777-review`
(`2047422`) at `2de5444e14606ce6912d1bb932bc88bffa379a77`.
- Review findings — none (critical, important, or minor)
- `cargo fmt --all --check` and the ownership guard — passed
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings` —
passed
- `cargo test -p buzz-db --quiet` — 108 passed, 200 ignored; source
guards passed
- Native PostgreSQL moved suite — 11 passed
- `cargo test -p buzz-relay --lib` — 909 passed, 48 ignored
Generated with Codex
## Superseded pre-comment restack verification
PR #6700 merged before publication completed. This layer was restacked
onto current main through the exact parent named above; the final
cumulative tip is 2ddcc8a29f95c8c8c9cb87bb6cf59335f2190fae. Cumulative
author gates passed: formatting and diff checks; buzz-db and buzz-relay
all-target clippy with -D warnings; DB lib 111 passed / 200 ignored;
ownership 22/22; observability 1/1; the full isolated PostgreSQL domain
matrix; and relay lib 910 passed / 49 ignored.
- Workstation: `buzz-tornquist-pr-6777-final-review` (`2057617`), fresh
shallow checkout
- Base: `f24971033178926153b49d320bd876d15d9cb2bf`
- Head: `ffbeaaf00810aa359ab85818ff4820f92263e45f`
- Findings: none
Reviewed `base..head` for the replaceable-store extraction. The
addressable replacement SQL, stale/duplicate outcomes, advisory-lock
ordering, transaction rollback/commit boundaries, mention-index
atomicity, and public `Db` signature are retained in `replaceable.rs`.
The PR #6700 transaction timer remains around the same logical
transaction through `TransactionTimer::observe`, and the public
operation has exactly one datastore span. Focused addressable and
parameterized tests moved with the implementation; the ownership guard
excludes duplicates from `lib.rs`.
Verification: format and diff checks passed; `buzz-db --all-targets`
clippy passed with `-D warnings`; DB lib tests passed (111 passed, 200
PostgreSQL tests ignored); ownership (1/1) and observability (1/1)
guards passed; all 11 replaceable PostgreSQL tests passed on native
PostgreSQL 17 with migrations 1-32 successful; relay lib test target
compiled successfully. Final worktree was detached at the exact head and
clean.
Complete evidence archive SHA-256:
`e37f92bbde4e2343196085095b3093b1bc78196be58bc86826325e0e1639d7ea`.
## Comment-addressed restack
Review follow-up removed the dedicated
`replaceable_store_has_single_ownership` source test as requested. No
production code changed.
- Exact base: `f24971033178926153b49d320bd876d15d9cb2bf`
- Exact head: `da018405cc83605362125c2d5e5a3f91492431ac`
- Final cumulative tip: `6fa2f104d42c6ba85bdf62e7ccb74ceaf4a84f67`
- Focused PR verification: formatting and diff checks passed; strict
`buzz-db`/`buzz-relay` Clippy passed; DB lib 111 passed / 200 ignored;
all 11 replaceable PostgreSQL tests passed.
- Cumulative Blox gate: formatting and diff checks; strict
`buzz-db`/`buzz-relay` Clippy; DB lib 111 passed / 200 ignored;
ownership 21/21; observability 1/1; every moved PostgreSQL test; relay
lib 910 passed / 49 ignored.
- Independent re-review at this exact head: no findings; fresh
exact-parent/head Blox review passed fmt/diff, strict `buzz-db` Clippy,
DB lib 111 passed / 200 ignored, manual one-owner/one-span replacement
checks, observability, 11 replaceable PostgreSQL tests, and relay
compilation.
---------
Signed-off-by: tornquist <tornquist@squareup.com>
