Skip to content

feat(notifications): add a Show Dock Badge toggle (macOS) - #25191

Open
ggbdpq wants to merge 4 commits into
stablyai:mainfrom
ggbdpq:feat/dock-badge-toggle
Open

ggbdpq wants to merge 4 commits into
stablyai:mainfrom
ggbdpq:feat/dock-badge-toggle

Conversation

@ggbdpq

@ggbdpq ggbdpq commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

ELI5

Orca shows an unread-count badge on the macOS Dock icon. Some users want it gone — even an accurate "15" sitting on the Dock all day is noise when you check activity inside the app anyway. This adds a "Show Dock Badge" toggle in Settings → Notifications (on by default): off clears the badge immediately and keeps it cleared, on brings the current count back.

What Changed

  • Single write point gate (src/main/dock/unread-badge.ts): applyDockBadge() — the one place that writes the Dock badge — now checks a module-level dockBadgeVisible flag (default true, so existing behavior is byte-identical until the user opts out). Hidden mode writes '' while the underlying unread count keeps accumulating, so re-enabling restores the current count; setDockBadgeVisible early-returns on same-value writes to avoid redundant Dock calls at startup.
  • Settings plumbing: notifications.showDockBadge: boolean (default true) in the shared notification-settings types, defaults, and onboarding normalization (a legacy profile without the field falls back to on and persists once). src/main/ipc/notifications.ts reads the persisted value at handler registration and subscribes to settings changes, so the toggle applies immediately.
  • UI: a "Show Dock Badge" toggle in NotificationsPane after "Suppress While Focused", deliberately independent of the master notifications enabled gate (the issue asks for the badge specifically). en.json keys follow the localization extraction contract (verify:localization-extraction passes).
  • Platform guard: read/write paths early-return off darwin (app.dock is macOS-only), and the toggle renders only on darwin in settings (locked by tests).

User-visible before/after: toggle off → Dock badge clears and stays clear; toggle on → current unread count returns; in-app unread indicators never change.

Why

The gate lives at the single Dock write point rather than at the unread-count source: the count must keep accumulating while hidden (that's what makes "toggle back on restores the current count" true), and one gate can't drift out of sync with the multiple paths that push counts. The toggle is independent of the notifications master switch because the badge is ambient OS chrome, not a notification delivery channel.

Linked Issue

Fixes #25160

Visual Proof

N/A for attached media in this environment (headless run, macOS Dock chrome). Behavior is pinned by tests: hidden mode writes '' on count changes, re-enabling rewrites the current count, default-on preserves existing behavior, and the toggle renders only on darwin.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

New tests: unread-badge.test.ts (hidden writes '' and keeps accumulating, re-enable restores, same-value no-op) and notifications-dock-badge-setting.test.ts (setting default, persistence normalization, IPC wiring, darwin-only rendering) — 7 failed against the pre-fix code, 9/9 pass after. Adjacent suites: 15 files / 198 tests green (dock family, notifications IPC, delivery service, normalization, persistence defaults, NotificationsPane, onboarding flow); pnpm tc green (two pre-existing test literals needed the new required field); oxlint/oxfmt clean; localization extraction/catalogs verified. Not run: the e2e notification-settings spec (requires a built app + display; this change is purely additive to that spec's switch queries) and visual Dock confirmation on a real Mac.

AI Disclosure

Implemented by GLM-5.3-Flash (ZCode agent) under direction of ggbdpq, who reviewed the diff and re-ran the touched suites.

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Cross-platform: the Dock API is macOS-only and both the write path and the settings UI are platform-guarded; Windows/Linux behavior is unchanged (no Dock badge exists there). No persistence format migration (additive settings field with on-fallback). One known pre-existing quirk left alone: multiple renderers can each push counts (the main-process gate converges them); not in scope.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@ggbdpq
ggbdpq requested a review from brennanb2025 as a code owner October 4, 2026 08:54
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

Adds a Show Dock Badge setting to notification preferences. The setting defaults to enabled, is normalized as a boolean, and appears only on macOS. On macOS, changing the setting clears or restores the Dock badge without changing the unread count. Notification settings initialize badge visibility and update it when they change.

Merge Risk: Minimal · previous review 87397

The Dock badge setting and its search entry have no identified merge-blocking issue. The remaining import suggestion is stylistic.

Security architecture risk: Low · up to b8340

The change controls a local display without deleting unread state or adding permissions. No expanded access was identified. Whether a newly selected preference survives interrupted or unsuccessful saves remains partly unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The newly added control affects the running application's macOS Dock unread-count display. The inspected path does not add notification-content access, service authority, or another sensitive sink.

Trust Boundaries and Controls

  • observed — The existing unread-count IPC input is checked for finite values and normalized by the badge module. Once visibility is false, subsequent count inputs still produce an empty Dock label rather than bypassing the preference.
  • inferred — The inspected desktop startup path attaches core handlers and applies badge visibility before loading the renderer. This counters a first-renderer-update race, although the complete persisted-store initialization chain was not established.

Resilience and Maintainability Implications

  • inferred — The session can apply hidden mode before the preference becomes durable because settings saves are debounced. Normal shutdown attempts a final flush, but interruption or unsuccessful persistence may leave the prior saved visibility for restart. This is inherited persistence behavior, not a demonstrated new bypass; durable recovery for this preference remains partly unverified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 14 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)

Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding a Show Dock Badge toggle for macOS notifications.
Description check ✅ Passed The description is complete and covers the user impact, implementation, rationale, linked issue, visual-proof status, testing, platform considerations, AI disclosure, and checklist. It explicitly note…
Linked Issues check ✅ Passed Issue #25160 coding requirements are met. showDockBadge defaults to true, is normalized and persisted, and is read by notification IPC registration. setDockBadgeVisible gates the single Dock bad…
Out of Scope Changes check ✅ Passed The changes stay within issue #25160. The type, default, normalization, IPC wiring, Dock writer gate, macOS UI, localization, settings search entry, and supporting tests implement or verify the reques…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 14 files. (1 skipped: 1 unsupported.)

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
ℹ️ Recent review info

⚙️ Run configuration

  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dc0368db-f55c-4253-af7a-a42ec9f7d35c
📥 Commits

Reviewing files that changed from the base of the PR and between b834057c98048c85096669a26c1a29a6e58e1b0e and 8739747f878604074c836108a683c0cc515d80ee.

📒 Files selected for processing (3)

  • src/renderer/src/components/settings/notifications-search.test.ts
  • src/renderer/src/components/settings/notifications-search.ts
  • src/renderer/src/i18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (1)

  • src/renderer/src/i18n/locales/en.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes

  • Dock badge write gate — applyDockBadge() is still the single Dock write point, now suppressed when dockBadgeVisible is false while unreadCount keeps accumulating; setDockBadgeVisible() applies the flag, no-ops on non-darwin, and early-returns on same-value writes.
  • Settings plumbing — notifications.showDockBadge (default true) flows through the shared type, defaults, and normalizeNotificationSettings, and src/main/ipc/notifications.ts applies the persisted value at registration plus on settings changes.
  • UI — a darwin-only "Show Dock Badge" toggle in NotificationsPane, deliberately independent of the notifications master gate, with matching en.json keys.
  • Tests — unread-badge.test.ts (hidden writes '', re-enable restores, off-macOS no-op) and notifications-dock-badge-setting.test.ts (registration + change wiring + missing-listener fallback); both pass locally (9/9). Assertions are exact rather than loose.

Verified against the codebase: registerCoreHandlers guards with a module-level registered flag, so the new onSettingsChanged listener registers once per process and does not accumulate across window reopen; and the notifications settings section is desktop-only, so the web client never renders this pane and the getRendererAppPlatform() === 'darwin' guard has no browser false positive.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@ggbdpq
ggbdpq force-pushed the feat/dock-badge-toggle branch from 8739747 to 3f2d2d1 Compare October 25, 2026 13:01
@ggbdpq

ggbdpq commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto 7d18e19907 today. The one review thread (missing settings-search entry) is resolved in f9f334df8a with its own darwin-gated catalog. The rebase also surfaced five full-NotificationSettings fixtures outside the original sweep that needed the new required showDockBadge field — seeded in 3f2d2d17c4, and pnpm run typecheck now exits clean. Verification on the rebased head: the targeted suites pass (unread-badge + notifications-dock-badge-setting + NotificationsPane + notifications-search 21/21, notification-delivery-service + onboarding persistence 23/23), the fixture-touched runtime suites pass 30/30, and the ipc-events hook directory passes 266/267 (1 by-design expected failure).

@nwparker gentle ping — happy to adjust anything.

@ggbdpq
ggbdpq force-pushed the feat/dock-badge-toggle branch from 3f2d2d1 to ba8e32a Compare October 26, 2026 21:46
@ggbdpq

ggbdpq commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (2e67ba5) and force-pushed. One adaptation: upstream's test-inventory cleanup (#33796) removed src/main/dock/unread-badge.test.ts, which this PR's original commit had extended — the inventory removal is accepted as-is, and the one behavior test this feature adds (hiding the badge clears it immediately and keeps it cleared as unread counts change) moved to a focused new unread-badge-visible.test.ts so the module change stays red-capable. Wiring suites unchanged: 14/14 across the badge setting/delivery tests, typecheck:node clean, oxlint clean.

@nwparker gentle ping whenever you get a chance — happy to adjust anything.

@ggbdpq
ggbdpq force-pushed the feat/dock-badge-toggle branch 2 times, most recently from d6f3209 to 14d03cb Compare November 5, 2026 16:02
@ggbdpq

ggbdpq commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and force-pushed. Upstream moved the notifications pane into settings/notifications/ — this PR's two platform-gated regression cases follow into a new sibling test file (noted in its header), the search-keyword test moved with it, and the dock-badge init points land cleanly after upstream's delivery-service changes. Fixture seeding follows upstream's file moves.

Verified after the rebase: dock/notifications/settings suites 57 tests, typecheck:node, typecheck:web, format check, and the max-lines ratchet all clean.

@nwparker gentle ping — happy to adjust anything.

The unread badge on the macOS Dock icon is now gated by a
notifications.showDockBadge setting (default on) with a toggle in
Settings -> Notifications. Turning it off clears the badge immediately
and keeps it cleared while the unread count keeps accumulating
underneath; turning it on rewrites the current count. The gate lives at
applyDockBadge, the single Dock write point, so the count source and
in-app unread indicators are untouched. Non-darwin platforms early-
return on both the write and the toggle rendering.

Fixes stablyai#25160

Generated-by: GLM-5.3-Flash (ZCode)
The toggle was the only control in the Notifications pane missing from
getNotificationsPaneSearchEntries, so searching settings for "dock" or
"badge" never surfaced it. Index it in its own catalog gated on darwin,
matching the pane's darwin-only render (same pattern as the tray and
menu-bar-icon appearance entries).

Generated-by: GLM-5.3-Flash (ZCode)
…gs fixtures

Upstream-facing fixtures outside the PR's original sweep construct full
NotificationSettings literals; the required field broke their typing.

Generated-by: GLM-5.3-Flash (ZCode)
…new translate idiom

Upstream moved terminal-agent-phone-push.test.ts under runtime/push/notifications/
and deleted notification-delivery-service.test.ts in its test purge; the moved
copy needs the new setting, and the settings pane now reads translations through
useTranslate().

Generated-by: GLM-5.3-Flash (ZCode)
@ggbdpq
ggbdpq force-pushed the feat/dock-badge-toggle branch from 14d03cb to 0357752 Compare November 8, 2026 09:22
@ggbdpq

ggbdpq commented Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main at 0da741a22f.

Adapters for the upstream reshuffle:

  • terminal-agent-phone-push.test.ts moved to src/main/runtime/push/notifications/; the moved copy gets the showDockBadge: true fixture seed.
  • notification-delivery-service.test.ts was deleted by the upstream test purge; its fixture-only edit goes with it.
  • The settings pane now reads translations through useTranslate(), so the new toggle uses t(...).

Verification on the new head 03577526e7:

  • pnpm test on the PR face + the moved phone-push suite → 22/22 passed
  • pnpm run tc:node → no type errors in 18,126 files
  • pnpm run tc:web → no type errors in 15,432 files

This branch has not been deployed

No deployments
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.

[Feature]: Option to hide the macOS Dock unread badge

1 participant