fix(sidebar): converge channel sidebar state across devices

[wpfleger96]

Problem

Channel sidebar state — sections, sort preference, stars, and mutes — syncs per identity as kind 30078 relay events. That state diverged between a user's devices and sometimes never self-healed. The main gaps: a local edit that lost a conflict was silently republished as remote content while the UI kept showing the edit; edits made inside the publish debounce were dropped on quit or community switch; a skewed remote head could push published createdAt past the relay's future-drift window and wedge all later publishes; and stale-at-open state waited for a reconnect a healthy socket never fires.

What this changes

Convergence. Sections and sort resolve whole-blob conflicts via LWW: when a local edit loses, the manager adopts the winning remote head (writes it through to state, storage, and the watermark) instead of republishing it, unifying with the relay's OK-false conflict path as one mechanism. Stars and mutes move to a per-entry Lamport rev with a single max-merge on every path (ordered by updatedAt, then rev, then the starred/muted-true leaf); a click strictly dominates every state its replica has observed, so it cannot lose to a same-second remote write. Payloads stay version: 1 so older builds keep parsing.

Durability. Every edit is persisted synchronously to a durable per-identity localStorage outbox and resumed on next mount, so edits survive quit, community switch, and the debounce window. Outbox records carry write-once ownership tokens, so one window's completing publish cannot clear another window's still-unpublished edit.

Publish correctness. The pre-publish no-op guard compares the pending store only against the freshly fetched readable relay head, so matching a stale previously-published value never suppresses a publish. Published createdAt is clamped inside the relay's drift window. Publishes use generation-CAS ownership with single-flight and completion re-drive, retained-head confirmation on ACK, bounded 2s→30s retry backoff, and a reconciliation loop (steady 60s, backoff on failure, refresh on window visibility).

Stale-outbox replay guard (P1). A durable outbox entry is only replayed if the relay head it was authored against is still the relay head at resume time. When the relay has advanced, the stale entry is discarded and the current remote state is adopted. This closes the path where a previously-lost edit was re-published above a newer relay head on next mount.

Bootstrap/live-head ordering (P2b). The pre-publish fetch baseline is derived from the bootstrap fetch result directly, not from the mutable lastRemoteHead field that subscribeLive can update during an in-flight bootstrap. A live peer head arriving after the click is held as a genuine remote advance rather than folding into the publish baseline.

Queue-until-bootstrap (P2a). When a publish is debounced while bootstrap is still in flight, the edit is queued rather than fired immediately. On bootstrap completion, releaseDeferred publishes the queued edit using canonicalMax(queueTimeBaseline, bootstrapResultHead) as the baseline — preserving the queue-time baseline when bootstrap returns an older head, and never regressing a click that was authored from a live peer head that fully applied before the click. Three additional invariants: the absent-bootstrap seed call is a no-op when a pending edit is already queued (pendingStore === null guard); failed-bootstrap base establishment uses canonicalMax against {0,""} so the first observed head always wins; and a bootstrapFailedExternalHeadObserved flag disarms the pre-publish exception once any peer head has been independently observed, allowing a genuine post-click peer advance to be adopted rather than published over.

Merge-lane key threading (P3). Stars and mutes pre-publish fetch, reconnect fetch, and restart fetch all thread the retained event ID through the merge, so the key used for the incoming-payload merge is always consistent with the key retained in the watermark after publish.

Structure. All four lanes share two engines: wholeBlobSyncManager.ts (sections, sort) and mergeLaneSyncManager.ts (stars, mutes); each lane manager is a thin config wrapper. Legacy preference migration proves ownership of the legacy key at read time, and retained legacy whole-blob replay is one-shot per value.

Tests. Lane test suites are consolidated into shared parameterized suites (wholeBlobSync.shared, wholeBlobSyncP2a.shared, mergeLane*.shared) plus helper modules, covering the new invariants (durable resume, multi-window outbox, cross-lane isolation, tie-breaks, migration ownership, bootstrap/live-head ordering, queue-until-bootstrap state machine, failed-bootstrap recovery) while shrinking the sidebar test surface from 6,303 to 5,860 lines. Each production defect has a named causal regression (T1–T6) executed through both concrete lane managers (ChannelSectionSyncManager, ChannelSortSyncManager).

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at exact head 0d797b550e405eea9557e053f884533ad3dd891a. The relay ordering and the per-entry max-merge are coherent, but the lifecycle still has reproducible edit-loss paths.

[P1] The durable outboxes are not safe across desktop windows

Sections, stars, and mutes share one localStorage outbox key per identity + relay, but each window owns only an in-memory generation. A write in one window therefore replaces another window's pending payload, and a completion in either window removes the shared key without proving that it still owns the persisted value.

For sections, window A can start publishing edit A, window B can replace the outbox with newer edit B, and A's ACK then clears B's outbox. If B closes before its debounce fires, the next bootstrap applies relay head A before discovering there is no outbox, so B is permanently lost. See channelSectionsStorage.ts:211-245, channelSectionsSync.ts:262-281,284-313,522, and useChannelSections.ts:138-156.

Stars and mutes can lose independent clicks even earlier: two windows starting from the same store can click different channels before receiving each other's asynchronous storage event. Their whole-store main/outbox writes race, teardown cancels both timers, and remount resumes only the last blob. The storage handler merges only into React state; it does not durably merge the main store or outbox. See channelStarsStorage.ts:206-242, useChannelStars.ts:57-72,95-113, and channelStarsSync.ts:256-260,383-393 (mutes mirror these paths).

Please give persisted attempts cross-window ownership, not only manager-local generations. A per-operation outbox whose owner deletes only its own record, or an actual cross-window serialization mechanism, would close both overwrite and stale-clear races. Add multi-window tests that interleave write, ACK, storage delivery, teardown, and remount.

[P1] Sort preferences still drop pending edits on ordinary lifecycle and failure paths

publishSortPrefs keeps intent only in memory (channelSortSync.ts:113-121). destroy() deliberately cancels and discards it (:252-262), while a failed publish only logs and never retries (:167-210). A live remote also cancels the debounce while leaving pendingStore stranded (useChannelSortPreference.ts:82-103).

Reproduction: change a sort mode and quit/switch communities within two seconds, or let one publish time out and remount. Bootstrap then whole-blob-replaces the local cache with the relay head (useChannelSortPreference.ts:108-122), visibly reverting the user's choice. Please carry the durable outbox, generation ownership, serialized retry, and pending-aware remote application used by sections over to sort, with lifecycle tests.

[P2] Future relay heads can wedge stars, mutes, and sort publishing

Sections clamps created_at inside the relay's future-drift window (channelSectionsSync.ts:463-472), but stars (channelStarsSync.ts:298-301), mutes (matching code), and sort (channelSortSync.ts:183-186) stamp lastRemoteCreatedAt + 1 without a cap. If a self-authored head was accepted near the relay's +900s boundary, a correctly clocked second device emits +901s; the relay rejects it, and retries continue deriving from the same head until wall time catches up. Apply the same bounded timestamp rule across all four sidebar sync surfaces.

CI is green at this head. I did not duplicate the CI-equivalent suite locally; these failures are source-reproduced interleavings absent from the current tests.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Re-reviewed exact current head 16aeab68b094baaba508a0f2fc91c06b0c430c1c against base ef0d2025683869418e8eee22ac5b5ac16c5198b7 after the force-rebase. Changes are still required.

All 19 files in this PR have the same Git blob IDs as at previously reviewed head 0d797b550e405eea9557e053f884533ad3dd891a, including all production sync files and tests. The only sidebar differences between the old and new repository trees are three UI files inherited from the new base; they do not touch the NIP-78 sync producers, consumers, or their direct dependencies. Consequently, the previous review's blockers survive unchanged:

1. P1: sections/stars/mutes still lack cross-window ownership for the shared durable outboxes. Manager-local generation fencing cannot stop one desktop window from overwriting another window's persisted payload or from unconditionally clearing the newer payload after its own older ACK/no-op/adopt completion. The existing tests remain single-manager and do not cover two windows sharing the key.
2. P1: sort still drops pending intent. It still has only an in-memory pendingStore, no durable outbox or failure retry, destroy() still cancels and nulls the edit, and a live remote can still cancel the timer while leaving the intent stranded.
3. P2: stars, mutes, and sort still derive created_at = max(now, lastRemoteCreatedAt + 1) without the future-drift clamp used by sections. A self-authored head accepted near the relay's +900s limit can therefore wedge subsequent writes until wall time catches up.

Please address the concrete reproductions and repair boundaries in the review on 0d797b550: #6525 (review)

Current CI is not green: Desktop Smoke E2E (2) failed at this head while several desktop jobs remain in progress. I am not using that still-unclassified failure as a separate code finding; the source blockers above independently require changes.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at exact head 9141fe292268e2681e65664bbeb708b732af2535.

[P1] The multi-window outbox operations are still racy

The new ownership token does not make the localStorage operations atomic. clearOutboxEntry reads and validates the stored token, then calls removeItem separately (sidebarSyncWatermark.ts:141-162). Window A can read token A, window B can write its newer {store, tokenB} envelope, and A can then remove B's value using the stale read. That still loses an unpublished edit on quit/remount.

Stars and mutes have the matching write-side race: writeChannelStarsOutbox reads the shared entry, merges in memory, then writes separately (channelStarsStorage.ts:226-237; mutes mirrors it). If two windows read before either writes, each computes a one-sided merge and the later setItem drops the other pending click. The main-store read/merge/write has the same shape (channelStarsStorage.ts:135-144).

The added multi-window tests execute whole operations sequentially, such as A write, B write, then A clear (multiWindowOutbox.test.mjs:62-167), so they cannot exercise either read/write or read/remove interleaving. localStorage provides no compare-and-delete or transactional read-modify-write primitive; the token proves what was read, not what is still stored at the destructive operation.

Please move this durability boundary to a design that does not depend on atomicity localStorage lacks, such as per-operation append-only records with owner-specific deletion, or an appropriately serialized cross-window store. Add tests that pause operations between their read and write/remove steps and verify teardown/remount preserves every unpublished intent.

The LWW comparator itself matches the relay's created_at DESC, id ASC rule; this review is blocking only on the durability race above. I reviewed read-only GitHub metadata and diff and did not check out or execute PR code.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes at exact head 3712e6fb2c71f03587a476c36081b1ac831a60c7 against base ef0d2025683869418e8eee22ac5b5ac16c5198b7.

Product direction: the relay is authoritative, while sidebar sections/sort use whole-blob LWW and stars/mutes use per-entry max-merge. The compatibility contract is that all four lanes converge across concurrently running builds/windows without dropping an unpublished user edit. I reviewed the four surfaces across bootstrap, live delivery, storage events, reconnect, periodic/visibility reconciliation, local edits, publish failure/ACK, destroy, remount, and community switch. I found these blockers:

[P1] Preserve the original whole-blob baseline when reconnect wakes a pending edit

useChannelSections.ts:264-276 and useChannelSortPreference.ts:257-269 fetch on reconnect and then call the public publish*() method for an already-pending edit. Those methods increment pendingGeneration and reset publishBaseline to the now-current lastRemoteHead (channelSectionsSync.ts:277-295, channelSortSync.ts:273-289).

Reproduction: window A queues an edit against H0; window B publishes H1 before A reconnects. A's reconnect fetch records H1, and applyRemote correctly defers because A is pending. The reconnect handler then re-queues A's old store, replacing its frozen H0 baseline with H1. The pre-publish fetch sees equality rather than remoteAdvancedSince, so A publishes above H1 instead of adopting the remote winner. This reverses the intended LWW decision on both whole-blob lanes.

Wake/retry the existing generation without resetting its baseline, and cover remote advancement plus reconnect while an edit is pending for both sections and sort.

[P1] Do not clear a merge-lane outbox merely because its EVENT received OK

channelStarsSync.ts:279-330 and channelMutesSync.ts:279-330 clear their own durable outbox after publishEvent resolves. But the relay returns accepted OK for a superseded NIP-33 write as a no-op, and two windows can also prefetch the same head and publish different whole blobs at the same second. Only one blob is retained.

The shared cache does not close this race: writeChannelStarsStore / writeChannelMutesStore at storage lines 140-151 overwrite the shared key without reading it, while the storage handlers at useChannelStars.ts:67-74 and useChannelMutes.ts:67-74 ignore e.newValue and reread whichever snapshot currently occupies that key. Two windows can therefore create different-channel snapshots from the same base, overwrite the cache before either storage event is processed, and independently receive successful OKs. The non-retained window then deletes the only durable record of its click. The relay, cache, and all outboxes can end with that edit absent.

After publication, clear an own outbox only after an authoritative retained-head fetch proves that head subsumes the attempted store; otherwise merge and retry. Add a two-window test where distinct edits race from the same head and both OK paths complete in either NIP-33 order.

[P1] A later old-build click can lose forever to an earlier new-build rev in the same second

channelStarsStorage.ts:59-77,188-195 and the identical mutes code normalize a missing rev from an old build to 0, then rank updatedAt before rev. If a new build publishes {updatedAt:T, rev:1, starred:true}, an old build observes it and the user unstars during the same second T, that later old-build payload has no rev and parses as rev:0. New builds deterministically retain the earlier true/rev:1; the user's later action cannot win until another click occurs in a later second.

Keeping payload version: 1 allows parsing but does not make the causal model backward-compatible. Use an explicit migration/version strategy that handles old writers, or define and test a product-accepted mixed-fleet limitation rather than claiming observed later intent cannot be lost.

[P1] An unreadable current head must block whole-blob overwrite, not fall through to publish

channelSectionsSync.ts:353-359 and channelSortSync.ts:341-347 record an existing head, but if decryption or parsing fails they return publish. A pending local blob is then signed at lastRemoteCreatedAt + 1 and replaces the only current head, even though the client could not inspect it or determine whether it should be adopted. A future schema, transient keychain/decrypt fault, or malformed payload therefore becomes destructive data loss on the next local edit.

Treat an unreadable/future head as a failed precondition: retain the durable pending edit and retry/surface the incompatibility, but do not overwrite unknown authoritative state. Add pre-publish coverage for decryption failure and unsupported payload version in sections and sort.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

REQUEST CHANGES on exact head 6078943f3be3a3fea99e86d225b79246af53f599 against ef0d2025683869418e8eee22ac5b5ac16c5198b7. I reviewed GitHub metadata, diff, and exact-head source only; I did not execute PR code.

Product contract: the relay is authoritative. Sections/sort converge as whole-blob LWW; stars/mutes converge by per-entry max-merge. A client must not overwrite relay state it could not read, because that turns a transient fetch/decrypt failure into durable cross-device data loss.

P1 — Retain edits when the authoritative pre-publish read fails

All four lanes fall through to publishing local state when the pre-publish relay query throws. Sections does so at channelSectionsSync.ts:371-416 and sort has the matching branch; stars does so at channelStarsSync.ts:210-232 and mutes mirrors it. The client therefore cannot distinguish “no head exists” from timeout/auth/socket failure.

Reproduction: device B publishes a newer sections/sort blob or independent star/mute entries after device A’s last observation; A edits while its pre-publish fetch fails. A signs above its stale watermark and can replace the unseen authoritative head, erasing B’s data. Durable outboxes and retries do not recover content that A never merged.

Return a tri-state publish decision in every lane: only a successful, genuinely absent head may publish the local store directly; fetch failure must retain the outbox and retry. Add causal tests for an unseen newer head plus rejected pre-publish fetch across sections, sort, stars, and mutes.

P1 — Stars and mutes also overwrite an existing but unreadable head

channelStarsSync.ts:220-231 records an event but returns the local store when decryption, JSON parsing, or schema validation fails; doPublish then publishes it (:315-357). Mutes has the identical path. This can destroy entries in a temporarily undecryptable or future-schema head. Bootstrap already classifies this condition as failed, while sections/sort correctly return retain for it.

Give merge-lane pre-publish reads the same retain/retry behavior and cover decrypt failure, malformed JSON, and unsupported schema. A max-merge is safe only after both operands were actually read. Dungeon law is unhelpfully strict about this.

P2 — Reject numeric revisions that cannot advance

channelStarsStorage.ts:59-69 and the matching mutes parser accept any finite non-negative integer rev, including values beyond Number.MAX_SAFE_INTEGER. Local clicks mint maxRev + 1 (useChannelStars.ts:252-267, mirrored for mutes); at sufficiently large IEEE-754 values that no longer increases. With equal timestamps/revisions, true wins (channelStarsStorage.ts:199-207), so a malformed starred:true or muted:true entry can suppress later unstar/unmute attempts indefinitely.

Require safe, bounded integers for revision and timestamp inputs, define malformed-input handling, and add a regression proving a huge revision cannot wedge a later false toggle.

The prior blockers around multi-window ownership, reconnect baselines, unreadable whole-blob heads, publish retention confirmation, and timestamp clamping are materially addressed. Exact-head CI is broadly green; it does not cover the failed-read transitions above.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

REVIEW CLEAR: prior production blocker resolved

Reviewed head e3624ed22f826a83eb1be666e7a629ec1d3a94c3 against base ef0d2025683869418e8eee22ac5b5ac16c5198b7, focusing on the correction since 0f98440bea7dea8f73c78992e1fcfb4e7b6c2870. No remaining actionable production blocker found in this corrective review. This is a comment, not an approval.

• The stale no-op defect is fixed. lastPublishedStore is removed. The preflight carries its freshly fetched readable head, and the no-op branch compares only against that head. Therefore S1 published → S2 observed → explicit S1 publishes again rather than silently dropping the edit. Missing heads cannot prove a no-op; failed/unreadable reads retain the durable pending edit. Generation checks and retained-head confirmation remain intact.
• Nonblocking test follow-up: the causal S1→S2→S1 witness exercises the production shared engine using SyntheticWholeBlobManager, not both concrete adapters. Its ID-only synthetic equality and outbox counters do not prove same-ID section rename/assignment restoration, sort-mode restoration, or actual persisted contents through delayed confirmation. Add that scenario through both concrete managers with their real outboxes. The current witness catches the original engine defect; the unchanged concrete equality functions compare the relevant fields correctly, so this coverage limitation is not evidence of another production P2. The merge-hook retention phase now drives actual timers; the earlier click-before-older-opposite-live recommendation remains nonblocking. Restore the deterministic reclamation read→delete-gap witness as well: the new stale/fresh-key coexistence case no longer injects an arrival during reclamation, although the unchanged implementation still deletes only the proven immutable key.
• Scope and validation: retained the established sections/sort whole-blob LWW and stars/mutes per-entry merge contract, scoped outbox ownership, community/signer teardown, and accepted compatibility residuals. Rechecked current adapters/consumers and protocol premises against prior integrated coverage; reviewed the consolidated tests with independent bounded lanes. Read-only exact-ref source review only: no checkout, build, test execution, or live multi-device validation. Native/runtime behavior and other client implementations are not newly validated here.

[kalvinnchau]

Changes requested

Reviewed exact head e3624ed22f826a83eb1be666e7a629ec1d3a94c3 against base ef0d2025683869418e8eee22ac5b5ac16c5198b7.

[P1] Stale whole-blob outboxes are replayed above newer relay state — desktop/src/features/sidebar/lib/useChannelSections.ts:164, desktop/src/features/sidebar/lib/useChannelSortPreference.ts:161

On bootstrap, both hooks apply the fetched relay head and then unconditionally pass the selected outbox record to publishSections / publishSortPrefs. That call freezes the just-fetched head as its baseline, and the manager stamps the replay above it.

A reachable sequence is: window A queues an edit at t=100 and closes during the debounce; another device publishes a newer head at t=200; A later opens. The t=100 outbox is promoted to t=201 and overwrites the newer state, even though the reclamation contract immediately below says records with queuedAt < head.createdAt lost whole-blob LWW. Reclamation runs after replay and never touches this window's newly copied own key, so it cannot prevent the overwrite.

Preserve queuedAt in the resume result and, when bootstrap fetched a head, replay only records that are not strictly older; clear/reclaim a stale selected record instead. Add this sequence to both whole-blob lane tests.

[P2] The whole-blob baseline is not the state the user edited — desktop/src/features/sidebar/lib/wholeBlobSyncManager.ts:421-433

publish() freezes lastRemoteHead, but that value means “head observed by the manager,” not “head incorporated into React/localStorage.” This loses data in both directions:

• If bootstrap failed or is still unresolved, lastRemoteHead is {0, ""}. A sections/sort edit made after mount freezes that empty baseline. When the pre-publish fetch later finds an older, pre-existing relay head, remoteAdvancedSince() classifies it as post-edit, adoptRemote() clears the pending store/outbox, and the fresh click is never published.
• A live callback does the reverse at wholeBlobSyncManager.ts:762-770: it records the head before async decrypt/application. A click in that gap freezes the unapplied head as its baseline; the subsequent pre-publish fetch sees equality and publishes a stale whole blob over remote-only changes.

Base arbitration on the exact head successfully applied to the editable store. Queue/replay mutations until bootstrap establishes that base, and do not advance the editable baseline merely because a raw event arrived. Add deterministic coverage for both deferred/failed bootstrap and the live decrypt/application gap.

[P2] The pre-publish merge can evict the click that the hook explicitly preserved — desktop/src/features/sidebar/lib/mergeLaneSyncManager.ts:293-299

Stars/mutes preserve the clicked channel while locally bounding to 500 entries (useChannelStars.ts:269-288, mirrored by mutes). The manager then calls mergeWithRemote() without that preserved key. mergeStores() re-bounds at channelStarsStorage.ts:238-253 using only updatedAt and channel ID for eviction.

With 500 remote entries in the same second plus a lexicographically smaller clicked channel, the hook keeps the click, the pre-publish merge drops it despite its higher rev, and retained-head confirmation clears the outbox. Carry the mutated key through the publish merge, or make eviction ordering preserve the newest mutation tuple. Add a 501-entry regression for both stars and mutes.

Assessment

• Minimalness: 6/10 — shared engines help, but 8,501 additions plus four duplicated hook orchestration loops create a large change surface.
• Elegance: 7/10 — the shared lane split is directionally sound, but the abstractions omit two load-bearing facts: which remote head the UI actually contains and which entry must survive capacity bounding.
• Correctness: 6/10 — the reproduced paths silently discard user intent, violating the core contract of this change.

Validation

• Desktop unit suite: 5,518/5,518 passed at the exact head.
• Desktop typecheck: passed at the exact head.
• GitHub Desktop, Unit Tests, and Desktop E2E checks are green.
• The findings are uncovered ordering/capacity cases rather than failures in the existing suite.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

CHANGES REQUESTED

Reviewed exact head 8148ead9cf657e6e489e466644c4b64f8e405083 against base ef0d2025683869418e8eee22ac5b5ac16c5198b7, focusing on the repairs since e3624ed22f826a83eb1be666e7a629ec1d3a94c3. Three unresolved data-loss paths remain in the contracts this revision repairs.

[P1] Failed bootstrap promotes an old outbox into a fresh edit

The new failed-bootstrap exception cannot distinguish a fresh click from a restored outbox. Both hooks replay unconditionally on hold (sort: lines 161–199).

Reproduction: A queues sections/sort at t=100 and quits before debounce; another device retains B at t=200; A reopens, but its bootstrap fetch throws. hold calls publish(A), freezing {0,""} and replacing the outbox's original queuedAt with the current time. The first successful preflight reads B before any live/reconcile observation, so the exception folds B into the baseline and publishes A above it. B's newer layout/preferences are overwritten. The successful-bootstrap age guard never runs in this sequence; retained confirmation then legitimately confirms the wrong replacement.

Repair: retain restored-edit provenance/age through a failed bootstrap and resolve its freshness when a head can actually be read. Do not grant a restored outbox the fresh-click exception or remint its age before that decision. Add failed-bootstrap → recovered newer head coverage through both actual hooks, asserting no stale publication and correct durable-outbox disposition.

[P2] Hook replay and refresh still freeze unapplied heads into the editable baseline

publish() still copies lastRemoteHead, which is not necessarily the head reflected in the editable store. Two concrete paths bypass the new baseline fixes:

1. Bootstrap is blocked with H100; a click at wall time 101 queues A; live H102 decrypts while A is pending, so applyRemote refuses to apply it. Bootstrap then resolves H100. releaseDeferred correctly retains H100 as the baseline, but the hook immediately reads A from its own outbox and calls publishSections again (sort: 175–189). This resets the baseline to H102. Preflight sees equality and publishes the pre-H102 blob over H102's remote-only changes instead of adopting H102. T2 exercises the concrete manager without this hook callback, so it does not catch the production sequence.
2. After normal bootstrap, a periodic/reconnect fetch receives B and fetchRemoteBlob records its tuple before awaiting decrypt. A click during that await freezes B while still editing A. The eventual hook application is suppressed by hasPendingEdit; preflight B again sees equality and publishes stale content over B. Moving tuple advancement after decrypt only in subscribeLive leaves this path unchanged.

Repair: preserve the pending generation/baseline when bootstrap finds the current session's already-queued edit, and keep the editable baseline tied to successfully incorporated state across live and fetch paths. Add actual-hook ordering regressions for both sequences, including durable state and the final publish/adopt outcome.

[P2] Capacity-bounded subsumption can certify a click the relay does not contain

The preserved key reaches the publish merge, but isStarsStoreSubsumedBy and the mutes equivalent still prove retention using mergeStores(head, candidate) with no preserved key. That merge bounds to 500 entries before testing equality.

Take a full head R with 500 entries at the same updatedAt. A candidate contains 499 of those entries plus a lexicographically smaller clicked channel X (with a higher rev), and its durable preservedKey is X. R does not contain X. The proof merges 501 entries, evicts X using timestamp/ID ordering, gets R back, and returns true. On restart, the bootstrap subsumption gate skips replay and reclamation deletes the foreign outbox. In a same-second publish collision, retained-head confirmation can likewise clear the own outbox even though the winning head omits X. Both lanes lose the explicitly protected click.

Repair: do not use lossy capacity eviction as evidence that the retained head contains the mutation. Preserve the clicked-entry requirement through bootstrap suppression, reclamation and ACK confirmation, or use an appropriate non-lossy dominance proof. Add found-full-head restart and ACK-but-unretained full-head regressions for stars and mutes. The new restart test intentionally returns an absent bootstrap head (mergeLaneHook.shared.test.mjs:694–704), and the new manager tests return an empty confirmation read, so neither exercises this boundary.

Scope and validation

Read-only exact-ref source/metadata review with independent whole-blob and merge/storage lanes. No checkout, build, test execution or live multi-device validation. Reused and verified unchanged consumer/protocol premises from the prior integrated review: community/identity isolation, all four kind-30078 lane tags, canonical replacement order and ACK-versus-retention semantics. The fresh-readable-head no-op correction and write-once ownership remain intact. Existing mixed-version, clock and general capacity residuals are not reopened; the findings above directly violate this revision's stale-resume, applied-baseline and preserved-click contracts.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Source-only re-review of 31510eabb18207dce052c3fbe3afdf904c01d35c against base ef0d2025683869418e8eee22ac5b5ac16c5198b7, focused on the repairs since 8148ead9cf657e6e489e466644c4b64f8e405083. No PR code was checked out, built, tested, imported, or executed; no runtime/CI pass is claimed.

The exact prior failed-bootstrap stale-replay, bootstrap/replay baseline-refreeze, and fetch/decrypt-gap witnesses are repaired. The preserved-click proof is also repaired for bootstrap suppression and ACK confirmation. Four actionable cases remain:

1. [P1] Successful reopen discards a valid newer sections/sort outbox

wholeBlobSyncManager.ts:530-544, with releaseDeferred at 394-410 and hook replay at useChannelSections.ts:178-195 / useChannelSortPreference.ts:175-192.

Relay holds H100; the user edits A at 101 and reloads before debounce, then makes no new click. Bootstrap successfully finds unchanged H100. Because the new manager has no pendingStore, releaseDeferred() never initializes publishBaseline. The hook correctly accepts A's age, but now calls publish(A, true), preserving {0,""}. Preflight therefore treats H100 as an advance and adopts it (628-680) without publishing A. On same-window reload, the replay transfer replaces the original own outbox, and adoption deletes that replacement too. An ordinary successful resume loses the saved edit.

Establish a bootstrap-result baseline for a genuinely restored, eligible outbox; do not assume a pending generation already established one. Keep post-click live heads out of that baseline. Add actual-hook outcome tests for both sections and sort with a distinct newer outbox, unchanged readable head, fired debounce, and retained publication/outbox assertions, not just a scheduled timer.

2. [P2] A second click still freezes a head the UI never incorporated

wholeBlobSyncManager.ts:530-532, reached from the sections/sort mutation callbacks; observed-head updates are at 292-309 and 908-921, while hook application is suppressed at useChannelSections.ts:115 / useChannelSortPreference.ts:114.

Start with H100 applied, queue A1, then deliver/decrypt live or periodic H102 while A1 is pending. The manager records H102, but applyRemote deliberately keeps A1 visible. A second user edit A2 is authored from A1, not H102. Its normal publish(false) nevertheless freezes lastRemoteHead=H102. Preflight sees equality and publishes A2 over H102-only changes. Generation guards and retained-head confirmation do not prevent that incorrect replacement. Moving observation after decrypt fixes the previous gap, but successfully decrypted still does not mean incorporated.

Tie the editable baseline to the state actually incorporated into the edit (or preserve the pending baseline across edits that still exclude that head). Cover the two-click/suppressed-head sequence through both production hooks.

3. [P2] Failed-bootstrap replay misclassifies an in-session click as restored

useChannelSections.ts:164-195 / useChannelSortPreference.ts:161-192, and wholeBlobSyncManager.ts:533-536,667-680.

With no prior outbox, start bootstrap, click before it resolves, then let bootstrap fail. releaseDeferred schedules the fresh pending edit with its zero baseline. The hook immediately reads the outbox just written by that same click and republishes it with true, relabeling it as restored. If the 2s preflight recovers the pre-existing H100 before the initial reconciliation timer, the new flag disables the intended first-unknown-base exception and adopts H100, deleting the fresh click even though no competing peer wrote anything.

Do not reclassify/requeue the already-pending current-session generation as an old outbox. Preserve its provenance separately from a genuine restore. Test click-before-bootstrap-failure through both hooks and drive publication to completion; a manager-only call or clicking after failure misses this path.

4. [P2] Foreign-outbox reclamation still omits the preserved click from its proof

mergeLaneStorage.shared.ts:254-268, invoked by channelStarsStorage.ts:393-405 / channelMutesStorage.ts:383-395 after hook bootstrap.

Reclamation still calls isSubsumedBy(record.store, head) without record.preservedKey, so it selects the old lossy bounded-merge fallback. Use a full 500-entry head R lacking X and a foreign record containing 499 R entries plus reserved X. All entries have the same updatedAt, X has a higher rev but a lexicographically smaller ID than every R entry, so the unkeyed capacity bound evicts X and returns R, falsely proving subsumption. For a concrete durability failure, let bootstrap replay find this foreign record but fail to write its new own outbox (quota); writeOwnOutbox returns false, but the merge wrapper discards that result. Reclamation then deletes the foreign record even though the relay never retained X and no durable replacement exists. Quit before the pending timer and the click is gone. A foreign write between the replay snapshot and reclamation's separate enumeration is another path with no transfer at all.

Thread each enumerated record.preservedKey through the reclamation predicate as well. The existing new predicate already supplies the narrow proof needed. Add a full-capacity foreign-reclamation regression for both lanes, including failed transfer or a newly enumerated record. Bootstrap and ACK tests do not establish reclamation safety.

Bounded exit criteria

Preserve the existing version-1 payloads, whole-blob LWW policy, community/identity scoping, write-once ownership, and canonical relay ordering. Resolve the four traces above with production-hook/reclamation outcome coverage. Unchanged AppSidebar/AppShell consumers, signer/community remount boundaries, kind constants, and relay ACK/retention premises were checked against the previous reviewed source by blob identity. Native multi-device runtime and new mobile/web/CLI implementations are not validated by this review. No broader protocol migration, unbounded capacity retention, or unrelated hardening is requested. The new whole-blob suite manually invokes managers despite its “hook-layer” labels; the existing actual-hook replay check only tests timer presence. Those gaps support the defects above, not a separate testing blocker.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: two remaining P2 data-loss paths

Reviewed head ffda3579a5e61af7073f747e257ed2a4b5509755 against base ef0d2025683869418e8eee22ac5b5ac16c5198b7, with corrective comparison to 31510eabb18207dce052c3fbe3afdf904c01d35c. This is a read-only source review; I did not check out or execute PR code, run tests, or claim runtime validation.

1. P2: Replay age must not override an observed post-click remote advance

Anchor: wholeBlobSyncManager.ts:655–667.

The new restored-replay branch executes even after a successful bootstrap and ignores whether a competing head was independently observed. A concrete synchronized-clock sequence through either production hook:

1. Bootstrap is waiting to return H100 (created_at=100). The user edits at wall second 101; its own outbox records queuedAt=101.
2. Later in that same second, peer H101 is retained and delivered live. It decrypts successfully, but the hook suppresses applying it while the edit is pending.
3. Bootstrap returns H100. The hook rereads its current-session outbox and calls publishSections(store,true,101) / publishSortPrefs(store,true,101). The repaired baseline correctly remains H100.
4. Preflight finds H101 and correctly detects an advance, but 101 <= pendingRestoredQueuedAt overrides adoption. The client signs above H101 and publishes a blob that lacks H101’s peer-only changes.

This regresses the previously repaired bootstrap/live-head contract without clock skew. The override also affects a genuinely restored record: resume an outbox queued at 101 against H100, then observe peer H101 later in the same second while that replay is pending. Preserving fresh-session provenance alone does not protect that replay. Preserve genuine observed-advance arbitration; age-based replay eligibility cannot establish that a head was incorporated. The actual-hook P2a-1 regression currently uses queuedAt=50 and a live head at 200 (wholeBlobSyncCarl.shared.test.mjs:638–659,719–721), which avoids this branch. Add the equal-second witness through both real hooks and assert the adopted UI/cache and no overwrite.

2. P2: Bootstrap completion still relabels a fresh pending edit as restored

Anchors: useChannelSections.ts:178–196, mirrored in useChannelSortPreference.ts:175–193; engine wholeBlobSyncManager.ts:529–532,642–670.

Prior finding #3 is only masked when the newly fetched head’s timestamp is at or below the click’s wall-clock stamp. Start with no old outbox and no observed baseline. While bootstrap is pending, make a fresh click at wall second 100, then fail bootstrap. Its callback rereads that click’s own outbox and republishes it as (store,true,100), disabling the fresh first-unknown-base exception. If preflight discovers already-retained H101, the age branch fails (101 > 100), so adoption deletes the fresh pending edit and outbox without publishing it.

H101 can predate this click on synchronized clocks: another window can legitimately publish clampPublishCreatedAt(100,100)=101 (sidebarSyncWatermark.ts:45–52). This is the implementation’s normal timestamp bump, not the accepted clock-skew residual. Leaving the already-pending current-session generation intact preserves its first-unknown-base behavior; turning it into a restored generation does not. Preserve that provenance rather than compensating with timestamps. Cover a fresh click before bootstrap failure through both hooks, through preflight and completion; a preseeded old outbox or a manager click after bootstrap failure does not exercise this seam.

Credited repairs and stable exit criteria

The successful restored-outbox bootstrap baseline initialization, ordinary second-click baseline preservation, and per-record preservedKey forwarding during foreign merge-outbox reclamation are repaired in the inspected source. The confirmation decrypt ordering is also improved. Keep those repairs and the existing version-1, whole-blob/per-entry conflict contracts. Resolve only the two causal sequences above with production-hook regression coverage; no general clock, mixed-fleet, legacy, or capacity redesign is requested.