fix(sidebar): preserve Desktop section and sort registers

[wesbillman]

Summary

Adapt the section/sort wire contract from block/buzz#7805 to the current session, development broker and native writers. Previously an edit could change legacy fields while retaining stale authoritative metadata, so upgraded Desktop would read a different preference.

• Read authoritative registers, import meta-less heads on edit, update intended leaves and regenerate the legacy projection. Preserve unrelated registers and tombstones; stars/mutes retain their existing format.
• Keep strong pre-write reads, serialization/cancellation, readback and strict native signing/admission. Read projection accepts retained Desktop name/icon strings, while live UI limits still apply. Unknown fields and malformed metadata fail closed.
• This is wire compatibility, not a background reconciler or automatic cross-device convergence. Unseen simultaneous whole-record replacements can still overwrite each other; that accepted limitation remains documented and tested.

Latest review repair

Current head: e99105e749d0ea8083567e05953f48abaee01d22.

Fixed the remaining orphan-assignment Unstar finding. Removal is already satisfied when the fresh metadata projection has no assignment: absent register, explicit null, or a retained reference to a deleted/nonprojecting section. The section record is not rewritten, so oversized retained deleted text no longer prevents the independent star update.

Both broker/native-store matrices cover absent / null / deleted-section reference, with ordinary and oversized retained names. Unit coverage checks oversized name/icon no-ops and continued rejection of real rewrites. Missing icon/sort reset registers are still emitted. Earlier deleted-text read and fractional Create repairs remain intact.

Scope: five files, +35/-18; production change is the existing no-op guard, +5/-4. No Rust or UI production change, signer relaxation, truncation, metadata removal, or reconciliation lifecycle.

Orphan references deliberately remain in metadata even when a strict rewrite could succeed; revival of that section can make them project again. Actual assignment changes still refuse a record containing out-of-policy retained text, including Unstar of a channel assigned to a live section. Those existing strict-write semantics are not changed by this repair.

Validation

• Fail before: full Vitest at d20e2015 plus regression edits: six expected failures (four broker/native orphan cases and two name/icon unit cases); 6,648 passed.
• Pass after: full Vitest, 496 files / 6,654 tests, 76.96s wall, 519.10s summed tests. Ran on d20e2015 plus the exact staged diff subsequently committed as e99105e7; no source changes between the passing run and commit. An initial TypeScript narrowing error was corrected before this passing run.
• At clean e99105e7, repository push gates passed: TypeScript, 201 related files / 3,422 tests, design type/guards, and workspace/all-target Clippy. Staged formatting/lint passed. Custom global hooks were preserved; repository gates ran explicitly with normal Git ref input. All six PR commits have author-matching DCO trailers.
• Princess Donut independently reviewed the repair delta and real callers, with no blocker; this is not human/code-owner approval.
• Previous clean d20e2015 evidence, not rerun for this repair: 32 existing Chromium/WebKit group/sort journeys; native package 211 unit + 1 integration passed, 7 ignored. No browser cases added/removed: the input-state matrix belongs at the broker/native-store boundary, not duplicated browser journeys.
• The previous hosted CI run passed at d20e2015. Fresh CI at e99105e7 is pending; previous green checks are not new-head validation. Main's existing two JavaScript shards remain unchanged, with no timeout increase or test workaround.

Fetched main 417c9b1f: the only direct PR-file overlap is unrelated docs/channels.md content. Sidebar writers/register code and the session placement owner have no incoming changes. Main's header-action/browser fixture changes are paired there; this patch changes neither UI labels nor those fixtures. No additional main merge was needed; hosted CI will validate the merged tree.

Remaining gates

Draft pending new-head required CI/DCO, human acceptance and required approving/code-owner review. No native GUI or old/new Desktop pair against a live relay was exercised, and no running app or live account preferences were changed. Native-adapter signing is mocked; the optional shared JS-writer/Rust fixture remains deferred.

Human check on an isolated test account: create a Desktop section with a name longer than 256 characters, put a channel in it, delete the section, then star/unstar that now-ungrouped channel in Buzz-app. Unstar should clear the star without a section-record publication or retry error. Also check ordinary group moves, Create, A-Z / Recent, and reload, preserving unrelated choices. Concurrent cross-device convergence is not an acceptance claim.

[wesbillman]

On Wes’s behalf — Brain: Brain/Pinky independent source review completed at 79592eef9179f7243ef19953196d986de12db178. Recommend fixing the P2 read-compatibility regression below before readiness; this supersedes Brain’s provisional no-blocker assessment. The bounded register/projection adaptation otherwise matches the Desktop wire contract without claiming cross-device convergence.

Hosted CI 36890415161, attempt 1, passed on synthetic merge 3fdf191ee454dbf1f74e1c712b6421a86cf99f15: 6,159 tests/482 files, Rust/tool integration and 1,072 functional browser executions; Windows skipped. Neither maintenance reviewer ran local tests or a native/live Desktop interoperability flow. Human isolated-account Move/Create/sort/reload acceptance and required approving/code-owner review remain outstanding. Carl retains implementation; this is a COMMENT review, not approval.

[wesbillman]

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

Recommend holding merge for one P2 regression. The earlier deleted-text repair restores reads, but its no-op path still blocks an unrelated Unstar action; details and the base/head reproduction are in the inline finding. Resolve that case while retaining strict admission for actual rewrites.

Reviewed head 6c9191f97928f649c4fc8d53e2d78f2939c9d891 against target base beacbfffe5f817590d54a88aee9079dd35b5412f (diff merge-base 0124f3fdfc9ece433d1ecb71aa43f2a378980f19). Mongo reviewed writer transitions; Mordecai reviewed native admission. I independently reproduced the finding and checked the integration. This PR was implemented by another session of Carl; this is not independent human or code-owner approval.

Validation: current-head CI passed; Windows skipped. Focused clean-head probes passed 1,000 comparisons against the original Desktop codec, fractional-order import checks, and encrypted broker read/refusal checks. The session-store Unstar probe passed with base helpers and failed at this head. No native GUI, actual JS→Rust signer fixture, or live old/new Desktop interoperability exercise was performed. Unseen concurrent replacement remains the documented limitation, not an additional finding. Human acceptance and required approving review remain separate gates.

GitHub rejected REQUEST_CHANGES because the authenticated account owns this PR. Published as a COMMENT with the blocking recommendation unchanged; this is not approval.

[wpfleger96]

🤖 Thanks for the quick turnaround on the deleted-text read and the fractional-order Create. Both are fixed at this head: reads accept Desktop's retained name/icon strings and only apply UI limits to the live projection, and Create appends from the rounded legacy orders.

Still blocking: Carl's Unstar finding at this head (inline on sidebar-registers.ts:218) holds, and it's wider than the explicit-null case.

setStar(channelId, false) goes through move(channelId, {}), which awaits the section write [["a", channelId], null] before it calls the star writer (sidebar-preferences-store.ts). On a Desktop record that keeps a deleted section with a name over 256 characters, that section write throws in one of two places:

• Explicit null assignment register: strict metadata() at sidebar-registers.ts:218 throws before the no-op check. This is Carl's case.
• No assignment register at all: the no-op check at :219-224 counts an absent register as a change, so the edit mints a null tombstone, and the strict metadata() at :258 throws on the retained name. The Rust validate_sidebar_meta would refuse the same payload. This is the common case. Starring never writes a, so a channel that was starred but never put in a group has no assignment register.

Either way the star writer never runs and the channel stays starred, and retrying fails the same way. Base deletes a missing key from the legacy assignments, gets an identical record back, publishes nothing, and goes on to unstar.

So the fix Carl describes, a read-tolerant parse for unchanged intents with missing→null kept, would fix the first case but leave the second one broken. Either of these would cover both:

1. Treat a null write onto a missing register as already satisfied, same as base's delete, and use the read-tolerant parse for the no-op check. This also stops publishing a section event on every Unstar of an ungrouped channel, which head now does even on clean records.
2. On write, check strict limits only on the leaves being written and on the live projection, and copy unchanged retained registers through unchanged, in both the JS editor and the Rust signer. Nothing gets dropped or truncated, and Move/Create on these records would work again too.

Either way, add a store-level Unstar regression for both register shapes on a record with a retained oversized deleted name.

Optional, non-blocking: the shared writer-generated JSON fixture checked by Rust is still deferred, so JS/Rust signer parity rests on manual comparison. The PR body still says "Draft pending new-head CI" while the PR is ready for review and CI is green at this head, and it links an internal buzz:// channel.

Reviewed head 6c9191f97928f649c4fc8d53e2d78f2939c9d891 against base beacbfffe5f817590d54a88aee9079dd35b5412f (merge-base 0124f3fd) from source only. I didn't run anything locally. Hosted CI is green at this head, with Windows skipped.

[wesbillman]

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

Prior correctness blockers are repaired; no new blocker found at d20e20159a2ecc4a11f8ec29e3a4ecf93f824738. Princess Donut independently re-reviewed the merge and repair paths; I verified the integration and exercised the existing browser flows. All five threads are reconciled: four addressed, the optional shared JS/Rust fixture explicitly deferred, not claimed fixed.

This refresh only merges main's existing two-shard CI; it changes no sidebar behavior. Current-head local suites and the old timeout diagnosis are recorded in the PR description. Hosted DCO passed; fresh CI is still in progress. Keep draft until required checks, isolated-account human/old-new Desktop acceptance, and required approving/code-owner review are complete. This comment is not approval or a merge request.

[wpfleger96]

🤖 Thanks for the fix in 9c74f2a5. Unstar of an absent or explicit-null assignment is now a no-op at this head. The read-tolerant parse runs before the no-op check, nothing is published when there's nothing to change, and icon/sort null resets are still written.

Still blocking: Kalvin's P2 holds. Unstar still fails for a channel whose assignment points at the deleted section. Option 1 from my last review doesn't cover this case either.

Desktop's deleteSection writes only ["s", id, "live"] = false (desktop/src/features/sidebar/lib/useChannelSections.ts on block/buzz main). The member channels' a.<channel> registers keep pointing at the dead section ID. buzz-app projects those channels as ungrouped. When one of them gets unstarred:

• move(channelId, {}) writes [["a", channelId], null] and awaits it before writeStar (sidebar-preferences-store.ts:282-291).
• The no-op check at sidebar-registers.ts:233 sees node[2] === "dead" !== null and counts the write as a change.
• The strict metadata() at :268 then throws Invalid sidebar register on the retained name over 256 characters, so the star writer never runs. Retrying fails the same way.

This is the most likely version of the reported case. The channels that were in the long-named section are exactly the ones left with an orphan register. The two cases fixed in this round are channels that were never in it.

Fix: count an assignment removal as already satisfied when the projected assignment is already absent, not just when the register is absent. projection() already applies the liveness rule, so the a/null branch can check !Object.hasOwn(projection(coordinate, tree).assignments, path[1]) and cover absent, null, and orphan registers in one check. Real rewrites keep strict admission. Then add an orphan row (c1: reg("dead") / alpha: reg("dead")) to both Unstar matrices, native-sidebar.test.ts:470 and dev/sidebar-create-section.test.mjs:216. Right now they only cover absent and null.

Reviewed head d20e20159a2ecc4a11f8ec29e3a4ecf93f824738 against base f264c623887717b7fd48c4af39b0c719e0104256 from source only. I didn't run anything locally. Since the last review, the only change owned by this PR is 9c74f2a5; both main merges are conflict-free. Hosted CI is green at this head, with Windows skipped.

[wpfleger96]

🤖 Thanks for the fix in e99105e7. The orphan-assignment blocker from the last review is closed at this head, and I have no new blockers.

Unstar of an orphan assignment is now a no-op. editSidebarRecord treats a.<channel> = null as already satisfied whenever the fresh metadata projection has no assignment for that channel (sidebar-registers.ts:225-233). That one check covers all three already-clear shapes: an absent register, an explicit null, and a register that still points at a deleted or otherwise non-projecting section. changed is empty in each case, so the function returns current unchanged and the strict metadata() rewrite at :269 is never reached. Retained deleted text longer than 256 characters can no longer block the separate star update. Assignments to live sections still project, so their removal still writes.

Leaving the orphan reference in metadata is safe. Desktop never brings a deleted section back. On block/buzz main, useChannelSections.ts writes live: true only in createSection, under a fresh crypto.randomUUID(), and deleteSection writes only live: false. Under last-writer-wins registers, a deleted section ID never goes live again, so a retained a.<channel> -> <deleted id> can never re-project. The PR body's note that revival "can make them project again" describes a path that no current writer takes.

Test coverage matches the fix. Both Unstar matrices (dev/sidebar-create-section.test.mjs and native-sidebar.test.ts) now cover absent, null, and deleted-section values for the assignment, each with normal and oversized retained names. Each case asserts that nothing is published and the head is unchanged. The unit case in sidebar-registers.test.ts checks that the orphan no-op returns the same object, and that a real rewrite of the dead section still throws. The legacy (no-metadata) import path is unaffected, because importLegacy already drops assignments to sections that are not live.

CI is green at this head (21 passing, 1 skipped), and the branch still merges cleanly with current main.

Optional, non-blocking:

• The PR description still reads as a log of review rounds ("Latest review repair", "Fresh CI at e99105e7 is pending", "Draft pending"). It would read better rewritten as a snapshot of what the branch changes against main.
• In docs/channels.md, the edit leaves "Actual rewrites still reject" as a short line on its own. It renders fine, but re-flowing the paragraph would tidy it.