Skip to content

feat(desktop): merge sidebar sections and sort per item across devices - #7805

Merged
wpfleger96 merged 10 commits into
mainfrom
wpfleger/sidebar-stale-reader-recovery
Sep 28, 2026
Merged

wpfleger96 merged 10 commits into
mainfrom
wpfleger/sidebar-stale-reader-recovery

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Channel sections and sidebar sort used to be published as one whole encrypted blob each (kind 30078, d-tags channel-sections / channel-sort), so a device with a stale copy overwrote newer edits made elsewhere. This PR merges them per item. It also adds stale-reader recovery to channel stars and mutes, which already merged per entry.

Sections and sort: per-field registers

  • Each field is a last-writer-wins register [v, dev, value]. v is a millisecond version, max(now, lastIssued + 1, maxSeen + 1), and dev is a random per-install id. Registers are ordered by v, then dev, then tombstone over value, then value bytes, so every device that has seen the same registers holds identical state (sidebarLwwMap.ts).
  • Sections store name, icon, order and live per id. Assignments store a section id, or null for an explicit unassign. Sort groups store alpha, recent, or null for a reset. Unknown group keys are kept as they are.
  • A delete sets live = false and is kept (no GC). A concurrent rename cannot bring a deleted section back.
  • New sections append after the highest live order. Reorder, move up and move down resolve against the current live list inside the transaction (dead ids dropped, missing live ids appended) and write an order register for every live section.
  • Dynamic keys are read as own properties, so group keys like constructor merge like any other key.
  • The wire format stays version: 1. The legacy sections/assignments/groups fields are always emitted, projected from the registers: live sections only, dense integer order, and assignments whose target is live. Registers travel under meta. Older desktop and mobile builds keep reading normally.
  • A payload without meta from an older writer is imported only into fields that have no register yet. Its additions land, but its deletes and renames of existing items are ignored. Existing local caches are imported once at a stable stamp (v = 1, legacy dev); any remote register, including one read from a meta-less relay payload, replaces such a placeholder, while registers authored on this device still win.

Store and reconciler

  • LaneStore (sidebarLaneStore.ts) is the single authoritative tree per pubkey + relay. Bootstrap, live events, reconnects, recovery reads, pre-publish reads and other tabs all merge into it; nothing replaces it. An unchanged transaction has no side effects. A failed localStorage write is retried later. The old pubkey-only key is still migrated once; it is removed only after a scoped write is durable, retried on any later successful persist.
  • LaneReconciler (sidebarLaneReconciler.ts) publishes only when the decoded relay head's canonical bytes differ from the local tree.
    • It never publishes over a head that hasn't been decoded yet or can't be read.
    • It never publishes over an empty read once this scope has seen a head, which is main's watermark guard. An empty read on a running reconciler demotes a previously decoded head and holds until a readable head returns; an empty read that raced a newer observation is ignored.
    • Every publish attempt waits for one not-before deadline: the 2 s edit debounce, or after a failed publish the full 5/10/30/60 s backoff. Recovery reads, live events and reconnects keep reading and merging during the wait but cannot start an attempt early or shorten it. The deadline is also enforced after the attempt's pre-publish read and right before the socket send; a held attempt re-arms at the deadline without adding backoff, while a real preflight, crypto or publish failure always backs off, even when an edit hold is active.
    • The attempt is dropped if the tree or the head changed or the manager was destroyed. publishEvent takes an optional isCurrent predicate, checked right before the first socket send and before the reconnect retry send, so a stale snapshot is not sent after a rate-limit or reconnect wait. A declined send rejects with a typed PublishCanceledError, so relay rejection text can never be mistaken for a cancellation. Other callers are unchanged.
    • An ACK schedules a verification read.
    • A decode result updates the head's status only if that event is still the head and no newer observation (event or empty read) arrived meanwhile. A stale decode still merges its content.
  • useStaleReaderRecovery re-reads the relay on a steady 60 s cadence while the app is in the foreground, and immediately when it becomes visible again. For sections and sort, every read goes to the reconciler, including while a local edit is pending.

Stars and mutes

The codec and the per-entry mergeStores rule are unchanged. The hooks gain the same recovery cadence, which skips reads while an edit is pending and discards any read that started before a local edit. A live event decoded after the manager is destroyed is dropped.

  • A completing publish clears the pending edit only if it still owns it (ACK and identical-payload exits), so an older completion cannot strand a newer edit.
  • The bootstrap first-copy seed (local cache published to a relay that looked empty) is tracked separately from edits. Any observed relay head retires it: a queued seed is cancelled, and an acquired seed aborts after its pre-publish read, after signing, or before the socket send. The observed head is then applied by recovery instead of being overwritten.
  • Each manager retains every decoded relay head (fetch, recovery, preflight, live), merged per entry within the existing 500-entry bound. Every publish attempt merges the edit with that retained data after its pre-publish read settles, so an absent or failed read can no longer publish over relay entries already seen. If a head decoded before dispatch changes a retained winner, the attempt is dropped before dispatch and the still-pending edit is re-queued; an observation that contributes no changed winner cancels nothing. The UI and cache converge on the next successful recovery read after the pending edit clears.

Known limits

  • Per-field versions make payloads larger, so heavy users reach the ~64 KB event limit sooner. Over the existing caps (100 sections, 1,000 assignments, 104 sort groups, 65,535 B ciphertext), the full state stays local and is not published. Splitting lists across several relay events is a follow-up.
  • Two devices reordering at the same moment can interleave per section. Previously one would overwrite the other entirely.
  • Edits from older desktop and mobile builds are best-effort: added items sync, but deletes and renames of known items do not.
  • Downgrade is read-compatible but not lossless: an older desktop drops meta when it writes, and that cache re-imports at stamp 1 on re-upgrade.
  • Hardening against a peer that publishes a maximal version (2^53−1) is not addressed.
  • Mobile is unchanged.

@wpfleger96
wpfleger96 requested a review from a team as a code owner September 22, 2026 17:01
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Sep 22, 2026
@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is b65cff31a4c5f4a0af63952b60a21fd73195321a...7bd43bdcb0dc15f873fbbe0e85a23f9b526d25e0.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 7bd43bdcb0dc15f873fbbe0e85a23f9b526d25e0 to authorize a new review.
Any previous review applies only to its recorded range.

@github-actions github-actions Bot added codex-security-review-current The posted Codex security review matches its recorded range. and removed codex-security-review-current The posted Codex security review matches its recorded range. labels Sep 22, 2026

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Changes requested

Reviewed head d9223cfec4049864e9a797695632801d1bf4eb9f against exact base 77729abfb692b25a0f4ec4a69add86af2e32c0dd. The intended contract is reader-only convergence for desktop sections, sort, stars and mutes without clobbering local edits; no outbox or publish redesign is required.

P2: Fence reads that start while a local edit is already pending

useStaleReaderRecovery.ts:75–91 snapshots the edit revision, but checks pending only when applying the response. A retry that starts after the edit and before its publish completes can therefore pass both guards after publication:

  1. Open with stale cache and failed initial reads, leaving the hook’s applied head at zero. Make a local section/sort edit (revision becomes 1; publish is pending).
  2. Trigger visibility recovery or a scheduled tick. It snapshots revision 1 and reads the old relay blob H. Let the manager record H, but hold that read’s decryption/result. The publisher’s separate preflight also sees H; its watermark comparison therefore retains the local edit.
  3. Let publish succeed and clear pending; delay or miss its live echo. The manager updates its own remote-head watermark, not the hook’s applied-head refs.
  4. Release H. Pending is now false and revision is still 1, so H is accepted and the whole-blob sections/sort updater replaces both the just-edited UI state and its localStorage cache. A later live event/poll can repair it, but an intervening user edit starts from the rolled-back blob.

The production seams are channelSectionsSync.ts:61–89,179–223, useChannelSections.ts:86–109, and the equivalent sort manager/updater. This is a new periodic/visibility-read race, not a request to redesign inherited publish arbitration. Stars/mutes use per-entry merging, so the concrete whole-blob rollback above is scoped to sections/sort.

Smallest exit: skip fetching while already pending (without stopping future scheduling), or remember that the fetch began pending and discard that response; retain the existing apply-time pending/revision fences. Add a regression that starts the held retry after the local edit, completes/acknowledges publication, and then releases the actual old retry with the live echo withheld. Assert both the visible local edit and persisted cache survive.

Also repair the existing C2 regression at useChannelRecovery.test.mjs:1314–1350: its single resolveRemote is overwritten by the publisher’s preflight fetch when the debounce fires as intended. It then releases that preflight, not the held recovery read. If the window timer is not advanced by the mocked global timer, publication never starts, which also misses the claimed prerequisite. The test neither stubs successful publishEvent nor verifies acknowledgement/pending clearance, so its current assertions do not establish the advertised post-publish revision fence. Give bootstrap, recovery and prepublish reads distinct responses/resolvers, assert successful publication, and retain a case where the retry starts before the edit to exercise the revision guard.

Non-blocking: observe the actual steady-state timer

useChannelRecovery.test.mjs:879–894 advances 60 seconds while the 30-second step is still pending, then checks only for any additional fetch. It never observes the newly scheduled 60-second interval. Advance the 30-second step explicitly, then assert exact fetch deltas across at least two 60-second intervals. This is a coverage gap, not an observed runtime cadence defect.

Scope and evidence

Source-only inspection covered all four adapters and their storage/manager contracts; initial/failed/absent/found reads, timer and visibility scheduling, local mutation and pending/published transitions, cache-write failure, live/reconnect interaction, and identity/community disposal. The production AppReady community/config key remounts the tree, so the stars/mutes helper’s lack of a standalone relay dependency is not a production blocker. The retry is reader-only and applied-head refs now advance after successful cache writes. The equal-timestamp comparator now correctly prefers the smaller ID, consistent with relay replacement/query ordering (crates/buzz-db/src/store/replaceable.rs:238–250, store/event.rs:690–700). All four kind-30078 d-tags and v1 payload schemas are unchanged. Unchanged mobile/web clients, schema migrations and publish/outbox redesign are outside this delta.

Existing exact-head CI snapshot at 2026-09-22T19:34:51Z: 50 successful and 23 skipped checks. No checkout, install, build, tests, mutation experiment or live reproduction was performed; the race and test gaps above are established by source tracing. CI was not rerun or monitored. Review does not constitute approval.

@github-actions github-actions Bot added codex-security-review-current The posted Codex security review matches its recorded range. and removed codex-security-review-current The posted Codex security review matches its recorded range. labels Sep 22, 2026

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Changes requested: original P2 remains

Bounded re-review of head ab785d4ff2a8a8b19dcdb9ebe48e3fab09426bb3, base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, and the delta from previously reviewed d9223cfec4049864e9a797695632801d1bf4eb9f. Contract remains reader-only sidebar convergence without rolling back local edits; no publish/outbox redesign.

P2: Fence recovery reads that begin while an edit is already pending

useStaleReaderRecovery.ts:89–108 still snapshots only the revision at fetch start and checks pending only at apply time. The production delta adds identity/relay lifetime dependencies, not the pending-at-fetch fence requested in the previous review.

The original schedule remains possible:

  1. Initial reads fail. Make a local section/sort edit, incrementing revision to 1 and setting pending.
  2. Start visibility/timer recovery after that edit. It snapshots revision 1 and reads old relay blob H. Let the manager record H, but hold this read’s decryption/result. The publisher’s separate preflight sees the same H and retains the local edit.
  3. Publication succeeds and clears pending; withhold/delay its live echo. Release the held recovery read.
  4. Pending is false and revision is still 1, so both guards pass. The hook’s applied head has not advanced with publication: the whole-blob updater replaces the just-edited UI and localStorage with H. Another user edit before repair starts from that rolled-back blob.

Verified seams: channelSectionsSync.ts:61–89,122–150,179–223, useChannelSections.ts:86–109,215–223, and the equivalent sort manager/updater. Stars/mutes merge per entry; this concrete rollback remains scoped to sections/sort.

Unchanged smallest exit: skip reads while already pending without stopping future scheduling, or snapshot pending at fetch start and discard that response. Keep the apply-time pending/revision fences. Add a production-bound regression that begins the held retry after the edit, verifies successful publication/pending clearance, then releases the old response with its live echo withheld. Assert both UI and cache survive.

C2 repair is partial

The distinct recovery resolver and successful publishEvent stub fix the previous resolver collision. However, useChannelRecovery.test.mjs:1478–1525 still starts recovery before the edit. Its “publish succeeded” check asserts only the already-optimistic section, not publication or pending clearance. If publishing never runs or leaves pending set, those assertions can still pass through the pending guard. Retain this revision-fence case, but establish its publish prerequisite explicitly; it does not replace the missing pending-at-fetch case.

Scope and validation

The separate resume-after-pending test does exercise later ticks after a local edit and witnesses recovery resuming, but it does not hold a read that begins pending across successful publication. The identity/relay lifetime wiring is coherent across all four callers. Backoff coverage now advances the 30-second step and two 60-second cycles; exact cadence/later-head assertions remain non-blocking test improvements, not a new runtime finding. No additional production blocker found in this bounded delta.

Exact-head CI run 35795746678 includes successful Desktop Core and Desktop Domain jobs. This review is source-only on the pinned Blox host: no checkout, dependency install, build, test, mutation experiment, or live reproduction. Unchanged clients, schemas and inherited publish arbitration were not reopened.

@github-actions github-actions Bot added codex-security-review-current The posted Codex security review matches its recorded range. and removed codex-security-review-current The posted Codex security review matches its recorded range. labels Sep 23, 2026
@github-actions github-actions Bot added codex-security-review-current The posted Codex security review matches its recorded range. and removed codex-security-review-current The posted Codex security review matches its recorded range. labels Sep 23, 2026

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Changes requested: one P2

Reviewed the complete six-file feature at 4fdfeb000a70c8e09d397b89982ba5de2396c92c against base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, all intervening changes, and the unchanged manager/cache/publish boundaries. Contract: sections, sort, stars and mutes recover without another edit/reconnect, lost local edits, or crossing account/relay lifetimes.

  • Original finding fixed. Pending-at-fetch, post-publication revision and bootstrap-seed guards now have production-bound witnesses. A different two-edit schedule still defeats the manager’s pending predicate; see the inline finding. Merge criterion: an older publish cannot clear a newer pending edit, with a real-hook regression proving preservation and eventual publication.
  • Scope is proportionate: +305/−12 production lines across five files, including one shared 158-line scheduler; +710 test lines. All four adapters/managers, bootstrap, polling/visibility, mutation/publication, cache failure, live/reconnect and disposal interactions were traced. No wire/schema or outbox redesign is needed. Additional cadence/coverage improvements are non-blocking.
  • Validation: source-only on pinned Blox; no tests or live reproduction executed. Existing exact-head CI showed 57 successful and 22 skipped checks, including Desktop Core. CI was not rerun. This is not approval.

Comment thread desktop/src/features/sidebar/lib/useStaleReaderRecovery.ts
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Sep 23, 2026
@github-actions github-actions Bot added codex-security-review-current The posted Codex security review matches its recorded range. and removed codex-security-review-current The posted Codex security review matches its recorded range. labels Sep 23, 2026
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Sep 23, 2026
@wpfleger96
wpfleger96 enabled auto-merge (squash) September 24, 2026 15:11

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Changes requested: one P2 startup data-loss race, detailed inline. Reviewed head e1fbb791d534d74614f459f25c5bc539103a0894 against base 77729abfb692b25a0f4ec4a69add86af2e32c0dd, including the delta since 4fdfeb0.

The previous pending-ownership blocker is fixed across all four managers, including both completion paths. The separate cross-window allegation is not a merge blocker for the current desktop topology: sections/sort live in the main sidebar; the supported Huddle companion excludes that sidebar and duplicate app instances focus main.

Merge criterion: an absent-response bootstrap seed must not overwrite a remote sections/sort head subsequently observed by recovery. Add real-hook regressions for that ordering while retaining protection for actual pending user edits. No outbox or general arbitration redesign requested.

Validation: exact-head CI is green; Desktop Core reports 6,653 passing tests, including all eight pending-ownership cases. A focused local real-hook reproduction with stubbed relay/Tauri boundaries fails for both sections and sort; matching controls where recovery does not observe the head pass. Production source was unchanged. No live two-device/native workflow run. Merge-state BLOCKED is separate from this code finding.

Comment thread desktop/src/features/sidebar/lib/useStaleReaderRecovery.ts
…tirement

A stale decode could restore a head demoted by a newer absent read, an
edit during preflight could escape its debounce, and a same-store
persist retry left the unscoped legacy key importable by another relay.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Changes requested: three P2s

Reviewed eabb8bbe7ddceb3ecc9060b1e0d2a663363057d5 against exact base 8f8c4dfadecb2298a9140af5d323487aaad844ab. The per-field sections/sort direction and documented add-only legacy-writer limit are retained; full mixed-writer support is not a merge requirement.

  • New migration defect: first upgrade can republish stale cached sections/sort over a newer meta-less relay head without a local edit. Detailed inline.
  • Two prior stars/mutes findings are back in this head: pending ownership and observed bootstrap payload loss. Both managers again clear pending unconditionally (channelStarsSync.ts / channelMutesSync.ts:161–163,189–191) and return the seed on absent/failed prepublish (:114–134). The new recovery hook still consumes that false-negative pending state or discards the already-observed seed response (useStaleReaderRecovery.ts:94–110). I re-traced both original schedules at this head; these are not new requirements or findings against inherited code in isolation. The latest canonical-head dedup finding is removed: the first losing edit now republishes its resolved payload as a newer event.

Merge criteria: preserve newer pending edits through older completion, retain or yield to the payload observed during bootstrap, and prevent cache-only migration from replacing the current relay values. Add production-hook regressions asserting UI/cache plus the final replaceable head; preserve legitimate local edits and future-timestamp remote winners.

Source-only review covered all four desktop preference lanes, per-field codecs/projections, persistence/migration, bootstrap/live/recovery/reconnect, edit/ACK races, teardown and publisher currentness. Mobile was inspected for wire compatibility, not changed. Declared size/tombstone/maximal-version limitations and bounded decode-delay hardening are not additional merge gates.

Validation: no checkout, install, tests, builds or live reproduction. Head-associated CI36071691579 tested synthetic merge c370585a4614bcb0c704a86c50738f9c310cb925, not the isolated reviewed head. Desktop Core reported 6,689 JS passes; desktop smoke passed. Overall CI failed one PostgreSQL replica-fence test with MaskedActivity { masked: 1 } outside the sidebar diff; cause remains unassigned. Security review was cancelled. No reruns or monitoring.

Comment thread desktop/src/features/sidebar/lib/sidebarLaneReconciler.ts
Only a deliberate deadline cancel (including the publisher's explicit cancel) keeps the edit deadline without failure backoff; preflight, crypto, rejection and timeout errors always back off. Bind the isCurrent forwarding through the real RelayClient.publishEvent.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Changes requested. Source-only re-review at head e154ea47a687f8d1dbd643cb76f241f448c3e477, base 8f8c4dfadecb2298a9140af5d323487aaad844ab.

Since eabb8bbe, the delta separates deliberate debounce holds from failure backoff and strengthens cancellation/quiescence tests. It leaves all three previous P2 blockers unchanged; I re-traced each at this head:

  • Stale sections/sort cache can overwrite the current relay state on upgrade. Finding and required regression. Current anchors: sidebarLaneStore.ts:77–84, sidebarLaneReconciler.ts:116–117,244, sidebarLwwMap.ts:57–67.
  • An older stars/mutes publication can clear a newer pending edit, allowing recovery to cancel its queued publication. Finding and required regression, now limited to stars/mutes. Both managers still clear pending unconditionally at :161–163,189–191; both hooks cancel the debounce at :85.
  • A stars/mutes bootstrap seed can overwrite a remote payload already observed by recovery. Finding and required regression. The apply-time pending guard remains at useStaleReaderRecovery.ts:108; both prepublish helpers still return the seed on absence/failure at :114–134 and publish above the observed timestamp at :170–173.

Merge criteria remain unchanged: resolve these three paths and cover UI/cache plus authoritative relay/fresh-reader outcomes through the real hooks/managers. Preserve the agreed per-field architecture and add-only legacy-writer limitation; full mixed-writer support and unrelated hardening are not requirements.

Validation: source-derived schedules only, not runtime reproduction. No checkout, tests, builds, or PR-code execution on any host. The captured exact-head CI snapshot had 31 successful checks, 23 skipped and 9 in progress, including Desktop Core and smoke tests (run); completed CI is not claimed.

A legacy relay read now replaces stamp-1 cache placeholders, so a first
upgrade no longer republishes a stale local copy over a newer relay value.
Stars/mutes clear pending only from the owning attempt, and the bootstrap
seed yields to any observed head, including while acquired. Publisher
cancellation is a typed error so relay rejection text stays a failure.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…seed retirement

Pins pending ownership, seed retirement at every publish stage, and hold-cancel cause capture with a relay fixture that follows the real replacement rule.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Key-only cache checks passed persisted false entries, and the identical-exit case never proved A's repeat reached its gated preflight.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Changes requested: one P2 remains. Source-only re-review at HEAD 5503afa83de7c5f6c7f1fbdff58b7fde68bfe857, BASE 8f8c4dfadecb2298a9140af5d323487aaad844ab.

The cache-placeholder precedence and pending-edit ownership fixes close those two findings. Seed-only cancellation is also fixed. The remaining inline finding continues the observed-remote-payload issue: a genuine edit can still overwrite data already observed by recovery when its later preflight is absent or fails.

Merge criterion: preserve that observed data through one and successive genuine edits, with real-hook regressions covering absent/failed preflight and UI, cache, replacement-aware relay, and fresh-reader outcomes. Preserve per-entry LWW semantics, including future-dated entry winners. No per-field architecture change, expanded legacy-writer support, or outbox redesign is required.

Validation: source-derived schedules, not runtime reproduction; no builds/tests or PR-code execution. The existing CI snapshot tested synthetic merge 6427a2abf301481cd3c1fa7604a587d15d884070 (main 2013148… + this HEAD), not the pinned base/head pair; Desktop Core and security were still running at the snapshot (run).

Comment thread desktop/src/features/sidebar/lib/channelStarsSync.ts
A genuine edit that took over from the startup seed dropped the relay payload recovery had already decoded, then published above it when its preflight came back absent or failed, erasing entries that only existed on the relay. Each manager now keeps an LWW accumulator of decoded heads, merges it into every attempt after the preflight settles, and fences attempts whose retained data changed mid-flight, requeueing the still-owned edit.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…tries by value

The conflict case passed even if the cache lost the future-dated tombstone or its timestamp, since only active IDs were compared. The accumulator's JSON equality also bumped the revision when the 500-entry bound merely reordered unchanged winners; it now shares the identical-payload exit's field comparison.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Clear bounded re-review; the remaining observed-data-loss blocker is resolved. Reviewed HEAD 7bd43bdcb0dc15f873fbbe0e85a23f9b526d25e0 against BASE 8f8c4dfadecb2298a9140af5d323487aaad844ab, concentrating on the three-file delta from 5503afa83de7c5f6c7f1fbdff58b7fde68bfe857.

Both stars/mutes managers now retain decoded per-entry winners and merge them after every preflight, including absent/failed reads and successive genuine edits. Changed winners fence stale dispatch and requeue the still-owned edit; unchanged observations do not cancel it. The real-hook regressions assert UI/cache, the replacement-aware relay head and a fresh reader, including a future-dated entry winner. Prior pending-ownership, seed cancellation and sections/sort cache-placeholder fixes remain intact. No new code blocker found in this repair; accepted legacy compatibility, size limits and inherited outbox behavior are unchanged.

Non-blocking coverage follow-up: add cases where preflight is the first observer of a new remote entry, and where a changed winner arrives through the live callback during crypto. The existing races exercise recovery-fetch observations; those two distinct feeds are correct in source but not independently pinned by the new tests. These are not additional merge requirements.

Validation caveat: source-only on pinned Blox; no checkout, builds, tests or live execution performed by this review. Existing Desktop Core CI passed, including the new stars/mutes regressions. Its checkout was synthetic merge fe9646cdee8cb7adfd23e6476bb9f0c39d735455 (this head + main b65cff31a4c5f4a0af63952b60a21fd73195321a), not the pinned base/head pair. Overall CI is red: Smoke E2E shard 2 failed empty-edit-delete.spec.ts:95 (message edit remains visible/old text remains). Cause not established or treated as a sidebar finding. Security-review automation was cancelled. No reruns or monitoring. Clear source review is not approval or a merge-readiness claim.

@wpfleger96
wpfleger96 merged commit 52c4c08 into main Sep 28, 2026
131 of 135 checks passed
@wpfleger96
wpfleger96 deleted the wpfleger/sidebar-stale-reader-recovery branch September 28, 2026 18:48
wpfleger96 pushed a commit that referenced this pull request Sep 28, 2026
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

* origin/main:
  fix(sidebar): converge stale-at-open state across devices (sections/sort/stars/mutes) (#7805)
  feat(nip-fi): harden Blossom kind-24242 verifier to NIP-FI spec (#7288)

Signed-off-by: Hayt <211b96e6a2b7f45fd4047988976c7bbbeeda0c15f3ae7b32eec20834b5a55118@buzz.block.builderlab.xyz>
wpfleger96 pushed a commit that referenced this pull request Sep 28, 2026
* origin/main:
  Add owner deletion admission control plane (#7818)
  fix(sidebar): converge stale-at-open state across devices (sections/sort/stars/mutes) (#7805)
  feat(nip-fi): harden Blossom kind-24242 verifier to NIP-FI spec (#7288)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
johnmatthewtennant pushed a commit that referenced this pull request Sep 28, 2026
…in-ui

* origin/main:
  test(desktop): wait for channel head refresh before paging thread summary test (#7955)
  🤖 perf: bound long-thread aux reads and make query deadlines terminal (#7854)
  Add native HPKE encryption for nsec backups (#7849)
  test(desktop): scope video menu e2e probes to emitted messages (#7953)
  Add owner deletion admission control plane (#7818)
  fix(sidebar): converge stale-at-open state across devices (sections/sort/stars/mutes) (#7805)
  feat(nip-fi): harden Blossom kind-24242 verifier to NIP-FI spec (#7288)
  Make relay readiness process-local (#7341)
  🤖 docs(nip-fi): remove implementation references from the spec (#7912)

Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz>

This branch was successfully deployed

No deployments
codex-review — 7bd43bdc Deployed Sep 25, 2026 by wpfleger96 via Run Codex Security Review #5747
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants