fix(desktop): register macOS badges for new and existing installs - #7783
Conversation
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
🔐 Codex Security Review
|
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking code findings for the native badge-registration contract.
Reviewed head 8ab8cf6d66d1bbeb2b7665c92f1ea58388a87999 against GitHub base a61239f0d8036aff58176f5c0ce7f080c66e21b7 (merge-base 5079c770fe30bb3d8204822ce6c2431eacac6d4b). Checked bundled startup, unbundled development fallback, permission-request callers, native status mapping, failure/relaunch behavior, and the unchanged unread-to-Dock setter boundary.
The repair is limited to Authorized + NotSupported, and ordinary native requests include Alert | Sound | Badge. The existing suite covers all 15 authorization/badge combinations and settings/request errors; consumed push logs show the complete Tauri workspace run passing (main unit target: 3,208 passed, 19 ignored) plus default/mesh clippy. Recorded startup-task and Badge-option mutations fail their respective regressions. I did not rerun broad suites or modify installed Buzz.
Release validation remains open: the isolated native probe demonstrates registration changing from NotSupported to Enabled, not recovery of the reported blank Dock badge in patched bundled Buzz. Before release, exercise an existing Alert/Sound-only installation through launch and visible unread Dock display, and verify an explicitly disabled badge stays disabled. This is a disclosed runtime validation gap, not a reproduced code defect.
Non-blocking metadata: the commit currently names Carl as author and sign-off rather than the usual Wes author/sign-off plus implementing-agent co-author convention. This can be corrected or overridden by Wes; it is not a reason to block the code.
CI/process status: at the review snapshot, hosted checks had no reported failures but several lanes, including macOS, were still running. The automated security-review comment still requested authorization for this exact range. Neither is represented as a completed gate here.
This is a review comment, not a GitHub approval.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Verdict: APPROVE
Reviewed: a61239f0d8036aff58176f5c0ce7f080c66e21b7..8ab8cf6d66d1bbeb2b7665c92f1ea58388a87999 (exact head 8ab8cf6d66d1bbeb2b7665c92f1ea58388a87999)
Risk: high — bundled macOS startup and notification authorization behavior.
Behavior/contracts traced: bundled vs unbundled startup; native authorization state mapping; new and existing-install badge registration; disabled/denied/provisional preservation; request failure and relaunch recovery; unread-to-Dock setter boundary; macOS cfg/build boundary.
Findings: no blocking code or product defect. Repair is limited to Authorized + NotSupported; explicit requests add Alert | Sound | Badge; other native states remain untouched. Startup failure is logged and retried on relaunch.
Author action: none.
Verification owner:
- Release/native smoke owner: before release, exercise a patched bundled app from an existing Alert/Sound-only installation through a visible unread Dock badge, plus an explicitly disabled-badge negative control. This journey was not independently witnessed; it is a confidence gap, not an established defect.
- CI owner: triage
Desktop Smoke E2E (4). Its persistent failure issidebar-snapshot.spec.tsexpecting 14 snapshot channels but receiving 0 within 500 ms (plus one explicitly flaky workflow timeout); 354 tests passed. This native-Rust-only diff does not touch that path, so the red gate is not attributed to the PR. The gate still owns merge readiness. - Repository/security gate owner: the exact-range Codex security review was skipped and still reports review required.
Validation at matching clean HEAD: full Tauri suite passed (3,309 passed, 20 ignored across 16 targets); desktop-tauri-fmt-check, desktop-tauri-check, and desktop-tauri-clippy passed; causal mutations proved both startup wiring and the Badge option regressions fail when removed. An independent focused macOS notification run also passed 10/10 after creating the repo-supported sidecar stubs. Hosted macOS build, Rust, Windows, relay/integration, Semgrep, zizmor, and three sibling smoke shards passed.
Manual/native evidence: isolated native probing establishes registration capability, but no exact patched packaged-app journey was run.
Residual risk: packaged existing-install recovery remains release-owned; the unrelated sidebar snapshot gate and skipped security review remain externally owned merge gates.
jedwards27
left a comment
There was a problem hiding this comment.
Review result: COMMENT — no author-actionable defect found
Reviewed base a61239f0d8036aff58176f5c0ce7f080c66e21b7 → exact head 8ab8cf6d66d1bbeb2b7665c92f1ea58388a87999 as :bot: Jude’s code review agent, integrating systems/integration and product/adversarial review.
Findings
No concrete defect found.
The change preserves the intended native state boundary:
- Existing bundled installs request registration only for
Authorized + NotSupported;NotDetermined,Denied,Provisional,Ephemeral,Disabled, andEnabledremain untouched (desktop/src-tauri/src/macos_notifications.rs:148-205). - Explicit permission requests now include
Alert | Sound | Badge(desktop/src-tauri/src/macos_notifications.rs:215-240). - Startup runs one non-blocking repair attempt per bundled process; failures are logged and naturally retry on a later launch, while unbundled development exits before invoking
UNUserNotificationCenter(desktop/src-tauri/src/macos_notifications.rs:124-159). - The macOS implementation and setup call remain platform-gated (
desktop/src-tauri/src/lib.rs:19-20,237-243). - Unread truth and Dock count/dot/none projection are unchanged (
desktop/src/app/useAppShellLifecycleEffects.ts:73-90;desktop/src/features/notifications/lib/desktop.ts:319-345).
Validation at exact head
- PASS: full
just desktop-tauri-teston a clean tree (all targets green; local run observed 3,209 passed / 20 ignored across the workspace; independent lanes also ran the complete suite successfully). - PASS: independent
just desktop-tauri-fmt-check,just desktop-tauri-check, andjust desktop-tauri-clippy(including mesh-llm in the systems lane). - PASS: causal mutation checks: removing startup wiring fails
bundled_startup_wires_native_badge_registration; removingBadgefailsrequests_all_interactions_used_by_buzz. - PASS:
git diff --check; one changed file; clean tree and live PR head confirmed as8ab8cf6d66d1bbeb2b7665c92f1ea58388a87999after validation. - PASS: hosted macOS build and macOS aggregate.
Confidence gaps and ownership
- Packaged release journey remains unobserved. Source and isolated native evidence do not prove that patched bundled Buzz upgrades an Alert/Sound-only existing install, renders the real unread Dock badge, and preserves a user-disabled badge as absent. Author action: none for this review. Verification owner: release/native smoke owner before release, using the packaged artifact and both positive and disabled negative controls.
- Required CI is red, but the failure does not intersect this native-only diff.
Desktop Smoke E2E (4)failed after retries insidebar-snapshot.spec.ts(expected 14 snapshot channels, received 0) and reported a separate workflow test as flaky; 354 tests passed. The aggregateDesktop Domain / DesktopandDesktopchecks consequently failed. Nothing in this one-file Objective-C/Rust authorization change reaches those browser fixtures, but a passing rerun or established baseline is still needed before merge. Author action: none unless CI triage establishes causality. Verification owner: CI/gate owner. Run: https://github.com/block/buzz/actions/runs/35622812209/job/106410013304 - The authorized security workflow skipped the actual Codex review. Author action: none from code review. Verification owner: repository security-gate owner.
Residual risk: setDesktopAppBadge still treats native setter failure as best-effort (desktop/src/features/notifications/lib/desktop.ts:331-344), which predates this PR. The packaged release smoke is therefore the decisive end-to-end evidence.
Verdict: COMMENT rather than REQUEST_CHANGES: no PR-caused defect or PR-caused required-gate failure was identified. Do not merge while the required red CI aggregate remains unresolved.
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed base a61239f0d8036aff58176f5c0ce7f080c66e21b7 through exact head f9a6ff2b0f7368497e5e74f0d9a8fd77bccd68af. The fresh delta from the previously cleared head is test-only and resolves the startup race without weakening the behavior under test.
No author-actionable defects remain.
Why the repaired test is meaningful
desktop/tests/e2e/sidebar-snapshot.spec.ts:303-330waits for the boot query to settle, arms the existing one-shot channel-response gate, starts realchannelsquery invalidation, and waits until that response is observably pending before applying the display mutation. It then proves the optimistic row is visible while revalidation is blocked and that exactly one held response is released.desktop/src/testing/e2eBridge.ts:11745-11760,13564-13576consumes the one-shot atomically, installs the pending resolver before exposing the pending state, and clears the gate before release. The test does not rely on an arbitrary delay or leave a deferred read behind.sidebar-snapshot.spec.ts:331-355waits for React Query settlement, requires the boot and revalidation requests with matching hashes, and independently asserts both user-visible continuity and persistence isolation: the optimistic row remains displayed but is absent from the authoritative persisted snapshot.- This still exercises the production not-modified path in
desktop/src/features/channels/hooks.ts:398-430,466-486; it does not manufacture a pass inside the mock.
Verification
At the exact head above:
- Focused Playwright test repeated serially: 20/20 passed.
- Full
sidebar-snapshot.spec.ts: 10/10 passed. - Causal mutation replacing the not-modified display merge with authoritative snapshot replacement failed at the optimistic-row assertion on every attempt.
- A separate blank/stale-row mutation failed the existing 14-row assertion.
git diff --checkpassed; the review worktrees were clean after mutation restoration.- GitHub reported the PR head/base unchanged and mergeable at submission time. Completed exact-head checks had no failures.
Confidence gap and ownership
Several exact-head CI jobs remained in progress at review submission time. This is not author rework and does not undermine the locally demonstrated synchronization contract. The named CI gates own terminal CI verification; any new head invalidates this approval.
Author action: none.
…n-surface * origin/main: feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…ecurity * origin/main: (22 commits) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) ... Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com>
…ness-overload * origin/main: (74 commits) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) ... Signed-off-by: tornquist <tornquist@squareup.com>
…rcement * origin/main: (87 commits) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) fix(desktop): keep managed agent avatars usable across communities (#7732) fix(mobile): fail open when age checks are unavailable (#7714) fix(ci): don't run desktop tests for purely mobile client changes (#7709) fix(mobile): temporarily disable age gating (#7708) feat(db): expose connection setup metrics (#7286) Isolate S3 storage metrics from the relay (#7543) ... Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> # Conflicts: # crates/buzz-auth/src/nip98.rs # crates/buzz-relay/Cargo.toml
…-history * origin/main: feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…-dev * origin/main: fix(desktop): bound startup request bursts and recover quota refusals (#7790) fix(audit): frame hash inputs with TLV (#7492) fix(admin): allow cold storage worker DB startup (#7770) feat(relay): add admin HTTP routes for member restriction management (#7302) fix(relay): fire kick live side effects at convergence; persist target; fence re-add race with held lock (#7298) feat(relay): add atomic complete read-state snapshots (#7572) fix(desktop): register macOS badges for new and existing installs (#7783) fix(mobile): avoid opening empty threads on message tap (#7756) fix(workflows): make deletion persistent and retryable (#7735) fix(mobile): preserve thread replies through refresh failures (#7757) fix(mobile): keep iOS message menu actions responsive after rebuilds (#7758) fix(relay): exclude ephemeral activity from message quota (#7736) release: push gateway chart 0.3.1 (#7749) fix(push): label plaintext push gateway service as HTTP (#7717) Replace personal and internal data in desktop test fixtures (#7748) Add mobile VISION (#7710) fix(mobile): keep relay sessions stable during push lease updates (#7745) Signed-off-by: Tom Brow <tomb@block.xyz>
Summary
AuthorizedandbadgeSettingisNotSupported(the omitted/unregistered interaction). Leave user-disabled badges, denied notifications, new users, and provisional/ephemeral authorization untouched.Validation
just cipassed at implementation heade6c4bc640, before the test-seam extraction and additional regression coverage.8ab8cf6d6: normal pre-commit/pre-push hooks passed, including macOS Tauri clippy (default and mesh-llm), complete Tauri test suite (3,208 passed, 19 ignored), and repository file-size gate.CI repair (
f9a6ff2b0)f9a6ff2b0; the repaired case also passed 3/3 CPU-throttled runs before commit. Temporarily removing display preservation or persisting the display mutation each caused the corresponding assertion to fail; both mutants were restored.Runtime limitation
This fixes a verified missing authorization registration and provides an automatic existing-install path. It is not yet proof that the reported blank Dock badge in the released Buzz app is fully resolved: patched bundled Buzz startup and unread-to-Dock recovery have not been exercised. The isolated app is native API evidence, not a replacement for that release smoke test. Installed Buzz and its read state were not modified.
Before release, verify an existing Alert/Sound-only install gains the badge setting on launch and renders an unread Dock badge; verify a user-disabled badge remains disabled.