Skip to content

feat(app-menu,shortcuts): add native macOS menu bar with Open Recent - #177

Merged
theBGuy merged 5 commits into
masterfrom
feat/macos-file-menu
Aug 9, 2026
Merged

feat(app-menu,shortcuts): add native macOS menu bar with Open Recent#177
theBGuy merged 5 commits into
masterfrom
feat/macos-file-menu

Conversation

@theBGuy

@theBGuy theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Gives GitDesktop a proper macOS application menu so the repo entry points are reachable from the menu bar the way Mac users expect, instead of only through the in-window welcome screen and repo switcher. File now carries New / Open / Clone Repository… plus an Open Recent submenu of the last ten repos, and Settings… sits in the GitDesktop menu. Windows and Linux ship no app menu on purpose — the in-window repo dropdown remains the idiom there.

Native menu (Rust)

  • Adds src-tauri/src/app_menu.rs, which builds the full macOS menu explicitly (mirroring Menu::default's composition) with the app, File, Edit, View, Window, and Help submenus. The Edit submenu keeps every predefined item, since macOS routes undo/redo/cut/copy/paste/select-all in text inputs through them.
  • Routes clicks through handle_menu_event, which matches only gd-menu- ids (muda has one global event channel shared with tray items), calls crate::tray::show_main_window so a hidden window can't swallow a dialog, and emits app-menu-action / app-menu-open-recent.
  • Encodes each recent repo's path directly in the menu item id (ID_RECENT_PREFIX) so a click needs no lookup table and can't read a stale row after a concurrent rebuild.
  • Exposes set_recent_repos_menu, which drains and re-appends rows on the live Submenu handle held in AppMenuState, recovers from a poisoned lock, and falls back to a disabled "No Recent Repositories" row when the list is empty. A no-op twin is compiled for non-macOS so the command is registered on every platform.
  • Deliberately assigns no accelerators to our items: app hotkeys are rebindable in settings, and a static menu shortcut would both lie about and intercept a rebound chord.
  • Wires the module and command into src-tauri/src/lib.rs, calling app_menu::setup_app_menu under #[cfg(target_os = "macos")] during setup.

Frontend bridge

  • Adds src/features/app-menu/useMacAppMenu.ts: listens for the two menu events, validates payloads against a MENU_ACTIONS allow-list before dispatchAction, opens recents via useOpenRepoByPath, and pushes the recents list (capped at MAX_RECENT_ITEMS) into the native submenu whenever it changes. Entries that share a display name are all suffixed with their parent folder so no twin reads as canonical. The listener effect guards against the StrictMode double-mount race where listen resolves after cleanup.
  • Hoists ownership of add-local-repository, clone-repository, and new-repository into src/App.tsx, along with the CloneRepoDialog / CreateRepoDialog hosts, so the actions work from Settings, Help, and Explore — screens that mount neither the welcome list nor the repo switcher. The registrations are gated on gitInstalled.isSuccess because the dialogs render below the early returns.
  • Removes the now-duplicate useHotkeyAction registrations from src/features/repository/RepoSwitcher.tsx and src/features/welcome/WelcomeScreen.tsx, which would otherwise shadow by mount order.

Documentation

  • Documents the menu bar under Features in README.md and in the "getting started" section of the in-app guide, src/features/help/content.ts.
  • Adds the capability line to site/src/data/capabilities.ts and a changelog fragment at changelog.d/added-macos-file-menu.md.

Note for reviewers: New Window is intentionally absent — the app is single-window — and the explicit menu construction must be re-checked against tauri::menu::Menu::default on a Tauri major upgrade. Menu behavior still needs verification on a Mac.

Relates to #171

macOS shows a menu bar whether or not the app populates it, so GitDesktop
now ships a real one; Windows and Linux keep the in-window repo dropdown as
the idiom and build no menu.

- `src-tauri/src/app_menu.rs`: constructs the App/File/Edit/View/Window/Help
  menu (Settings… in the app submenu, New/Open/Clone Repository… plus an Open
  Recent submenu in File), routes clicks into `app-menu-action` /
  `app-menu-open-recent` events, and exposes `set_recent_repos_menu` to
  rebuild the recents rows in place. The Edit predefined items are kept —
  macOS routes undo/cut/copy/paste in text inputs through them. Items carry
  no accelerators, since the app's hotkeys are rebindable and a static menu
  shortcut would both lie about and intercept a rebound chord. A no-op
  command twin keeps the wire shape identical off macOS.
- `src/features/app-menu/useMacAppMenu.ts`: bridges the menu to the existing
  action dispatch, validating native payloads against the action registry,
  and pushes the last ten recents into Open Recent, suffixing repos that
  share a display name with their parent folder.
- `src/App.tsx`: takes ownership of add-local-repository, clone-repository,
  and new-repository along with the clone/create dialogs so the menu works
  from Settings, Help, and Explore; the duplicate registrations in
  `RepoSwitcher` and `WelcomeScreen` are removed, since they only shadowed by
  mount order.
- Docs: README highlight, site capability entry, help-guide paragraph, and a
  changelog fragment.
@theBGuy theBGuy added the enhancement New feature or request label Aug 9, 2026
@theBGuy theBGuy self-assigned this Aug 9, 2026
@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

PR context from the orchestrated build — deliberate calls and the verification record, on the record before review. Single-claim items; later rounds can cite by number.

  1. Scope: macOS-only by design (target_os = "macos"). The feat: File menu item is a little sparse compared to competitors. #171 reporter agreed the in-window repo dropdown is the right idiom on Windows/Linux, so no other platform builds an app menu. The only cross-platform code is the set_recent_repos_menu no-op twin, which exists solely so the command registers on every platform.

  2. Our menu items carry no accelerators — locked product decision, not an omission. The app's hotkeys are user-rebindable (settings hotkeys overrides); a macOS menu accelerator is static Rust-side and intercepts the chord before the webview ever sees it, so a hardcoded one would both lie about and steal a rebound shortcut. Predefined items (Close Window ⌘W, Quit ⌘Q, …) keep their built-ins. Accelerator sync (rebuild the menu on hotkey change) is a possible v2, out of scope here.

  3. No "New Window" — blocked, not deferred. Single-instance is an architectural commitment (tauri_plugin_single_instance is the first plugin registered; the store plugin caches app-data per process, so two instances can't safely share state). This answers that part of feat: File menu item is a little sparse compared to competitors. #171's ask on the record.

  4. Mirror-construction over mutating Menu::default() — the composition was verified line-for-line against the installed tauri-2.11.5 source (src/menu/menu.rs:142-241); Edit keeps all seven predefined items because macOS routes text editing through them. The deliberate trade (explicit layout vs. auto-inheriting upstream default changes) is recorded as a re-check-on-Tauri-major constraint comment in app_menu.rs.

  5. The gd-menu-* id namespace is load-bearing. muda has ONE global menu-event channel (tauri app.rs:2350, dispatch at :2588): our handler receives tray clicks (open/quit) and the tray's handler receives menubar clicks. Each matches only its own ids and falls through everything else; the disabled empty-recents row is excluded by the gd-menu-recent: colon prefix.

  6. App now owns add-local-repository / clone-repository / new-repository (registrations moved off WelcomeScreen/RepoSwitcher, which keep their buttons and dialogs). Menu items must work on every screen, and Settings/Help/Explore mount neither surface; duplicate registrations would shadow by mount order (child effects run before parent effects). Intended widening: the palette and hotkeys also reach these actions on every screen now. The registrations are gated on gitInstalled.isSuccess so a menu click during the launch spinner or git-missing screen can't stash a stale dialog-open flag that pops later.

  7. Runtime verification: confirmed on the owner's Mac — every File item dispatches and Open Recent populates and opens repos. Windows-side gates all green (cargo check / clippy -D warnings / full test suite; tsc -b; scoped biome plus byte-exact format parity for the CRLF-checkout files; site build). Cross-compiling the mac arm from Windows dies in objc2-exception-helper's build script (needs a mac cc) — environmental, not code — so this PR's macos-latest CI job is the mac arm's first compile.

  8. Disclosures (known, each with a home):
    a. CloneRepoDialog/CreateRepoDialog now mount twice on some screens (App fallback + the surface's own copy). Stacked-open was adversarially checked and judged unreachable (modal overlay + dialog chord-swallow); the tidy shape — surfaces dispatch the action and drop their local copies — is a deliberate follow-up, not this PR.
    b. Duplicate-name disambiguation in Open Recent is one level deep (parent folder). A three-way src/api tie or case-only twins still render alike; matches the macOS-convention level and degrades to duplicate rows, never a wrong open (the item id carries the full path).
    c. An empty Open Recent shows an enabled submenu with one disabled "No Recent Repositories" row — the macOS convention, chosen over disabling the parent.
    d. Docs: changelog fragment + README Features bullet + site capability line + in-app guide paragraph are synced; a site FeatureRow was deliberately skipped (platform parity, not a marketed feature).
    e. Live-testing surfaced a close-to-tray discoverability gap on macOS: with the window hidden, the menu-bar status item and any File menu item restore it, but the Dock-click path is unverified and may do nothing (RunEvent::Reopen exists in tauri 2.11.5 if a handler is needed). Tracked as a follow-up — verify on hardware first, then decide.


Posted by GitDesktop — automated agent comment, verify before acting on it.

@theBGuy
theBGuy marked this pull request as ready for review August 9, 2026 07:01
Copilot AI lite review requested due to automatic review settings August 9, 2026 07:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a native macOS application menu (including File → Open Recent) and wires it into GitDesktop’s existing hotkey/action dispatch so menu items work across screens, while keeping Windows/Linux intentionally menu-less.

Changes:

  • Implement a macOS-only native menu bar in Rust, including a dynamically updated Open Recent submenu and frontend event emission.
  • Add a frontend bridge hook (useMacAppMenu) to listen for native menu events, validate action payloads, open recents by path, and push recent repo entries to the native menu.
  • Hoist repository action ownership in App.tsx and remove duplicate hotkey registrations from welcome/switcher surfaces; update README/help/site capability + add changelog fragment.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/features/welcome/WelcomeScreen.tsx Removes repo-action hotkey registrations from the welcome screen.
src/features/repository/RepoSwitcher.tsx Removes duplicate repo-action hotkey registrations from the repo switcher.
src/features/help/content.ts Documents the macOS menu bar + Open Recent behavior in the in-app guide.
src/features/app-menu/useMacAppMenu.ts New hook bridging native macOS menu events to action dispatch + recents sync.
src/App.tsx Centralizes repo action handling and mounts clone/create dialogs at app root.
src-tauri/src/lib.rs Wires macOS-only menu setup + registers the recents menu IPC command.
src-tauri/src/app_menu.rs New Rust implementation of the macOS menu bar + Open Recent submenu rebuild command.
site/src/data/capabilities.ts Adds marketing-site capability entry for the macOS menu bar feature.
README.md Documents the macOS menu bar feature under app features.
changelog.d/added-macos-file-menu.md Adds a changelog fragment announcing the new macOS File menu + Open Recent.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src-tauri/src/app_menu.rs
Comment thread src/App.tsx
@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 GitDesktop AI review · opus · automated


Summary

Adds a macOS-only application menu (File + Open Recent + Settings) in src-tauri/src/app_menu.rs, bridges its clicks into the existing action dispatcher via useMacAppMenu, and moves the three repo-action registrations from WelcomeScreen/RepoSwitcher up to App so they're live on every screen. The design is sound — id namespacing, the Option-free-in-practice submenu handle, the StrictMode-safe listen teardown, and the empty-recents row are all handled carefully — and nothing here is merge-blocking; the main item is a stacked-dialog case that the recorded "unreachable" note doesn't cover for the native-menu path.

Recorded decisions I'm not re-raising: no accelerators on our items, no New Window, one-level duplicate-name disambiguation, the enabled-submenu-with-disabled-row for empty recents, the skipped site FeatureRow, and the Dock-reopen follow-up.

Correctness

  • should-fix — duplicate dialogs can now stack, via the entry point this PR adds. App.tsx:238-239 mounts CloneRepoDialog/CreateRepoDialog, and WelcomeScreen.tsx:196-197 / RepoSwitcher.tsx:155-156 still mount their own copies with independent cloneOpen/createOpen state (WelcomeScreen.tsx:38-39, RepoSwitcher.tsx:51-52). Note 8a judges stacked-open unreachable via "modal overlay + dialog chord-swallow" — but the native menu bar is outside the webview, so neither guard applies to it: in the repo view, open the switcher → Clone repository… (RepoSwitcher.tsx:127 sets its local cloneOpen), then click File ▸ Clone Repository…; handle_menu_event (app_menu.rs:199) emits, onAction calls dispatchAction("clone-repository"), and since RepoSwitcher no longer registers that action, App's handler fires and opens a second CloneRepoDialog on top of the first (same for Create, and same on the welcome screen with WelcomeScreen.tsx:72/:79). Two independent Base UI modals with two focus traps is at best confusing and isn't something the PR exercises. Fix: land the tidy now rather than as a follow-up — have RepoSwitcher's clone/create ActionRows and WelcomeScreen's clone/create buttons call dispatchAction("clone-repository") / dispatchAction("new-repository") (import dispatchAction from @/lib/hotkeys/hotkeys), then delete both files' cloneOpen/createOpen state, their <CloneRepoDialog>/<CreateRepoDialog> renders, and the now-unused CloneRepoDialog/CreateRepoDialog imports (plus the useState import in WelcomeScreen, whose only other state is gone with them). Both surfaces only render once gitInstalled.isSuccess, so the dispatch always finds App's enabled handler.

  • should-fix — the Settings… item is live before git resolves, and unlike the File items it isn't inert. app_menu.rs:90 builds an always-enabled Settings…, handle_menu_event maps it to "open-settings" (app_menu.rs:200), and App.tsx:140 registers that action with no enabled argument — while App.tsx:146-156 deliberately gates the three repo actions on gitInstalled.isSuccess for exactly this reason. Concrete case: git isn't found → App.tsx:221-223 returns GitMissingScreen, the user clicks GitDesktop ▸ Settings…, openSettings() flips view to "settings" in the ui store, nothing renders (the SettingsScreen branch is below the early return), and after Retry succeeds they land on Settings instead of the welcome screen — the same "stale state that pops later" the comment at App.tsx:144-145 warns about. Fix: useHotkeyAction("open-settings", openSettings, gitInstalled.isSuccess);. (The open-mcp-servers-settings / browse-mcp-registry / show-help / open-explore registrations have the same latent shape but aren't menu-exposed — optional to align in the same pass.)

  • should-fix — the app submenu omits Show All (app_menu.rs:83-99): it goes hidehide_others → separator → quit. The standard macOS Application menu is About / … / Hide / Hide Others / Show All / Quit, and PredefinedMenuItem::show_all exists for it; a user who ⌘H-hid other apps has no way back from our menu. I couldn't open the installed tauri-2.11.5 source from this checkout to diff Menu::default line-for-line, so also re-check the "Mirrors Menu::default's macOS composition" claim in the doc comment at app_menu.rs:67-70 while adding &PredefinedMenuItem::show_all(app, None)?, after hide_others.

Edge cases

  • nitset_recent_repos_menu (app_menu.rs:226-242) drains before it builds: if MenuItem::with_id or append errors mid-loop, it returns with the submenu holding zero rows — precisely the empty submenu the comment at app_menu.rs:169-170 rules out — and useMacAppMenu.ts:102-104 swallows the error, so it stays empty until recents next change. Build the Vec<MenuItem<Wry>> (or the empty-row fallback) first, then drain and append.

Readability / maintainability

  • nitAppMenuState(Mutex<Option<Submenu<Wry>>>) (app_menu.rs:54): nothing ever stores None, so the let Some(recent) = … else { return Ok(()) } arm at :221-223 is dead. Mutex<Submenu<Wry>> drops both the Option and the branch; the poison recovery at :220 stays as-is.

  • nit — the action strings at app_menu.rs:196-201 are an untyped second copy of ActionId values: rename an id in registry.ts and MENU_ACTIONS (useMacAppMenu.ts:13-18) fails to compile while the Rust arm keeps emitting the old string, which isMenuAction then drops in total silence. Emit the menu id itself (ID_NEW_REPO, …) and map id → ActionId in a Record<string, ActionId> in useMacAppMenu, where the compiler checks both sides.

Docs

  • should-fix — the four product surfaces are synced (README Features bullet, site/src/data/capabilities.ts, src/features/help/content.ts, changelog.d/added-macos-file-menu.md; the FeatureRow skip is a recorded call), but CLAUDE.md:126-129 is now false: "we rely on Tauri's Menu::default() (it ships the Edit submenu that powers undo/redo/cut/copy/paste…)". On macOS the app now installs its own menu from app_menu::build_menu. Reword that bullet to point at src-tauri/src/app_menu.rs and carry the constraint there — e.g. "the macOS app menu is built in app_menu.rs::build_menu; keep all seven Edit PredefinedMenuItems or macOS text editing breaks" — matching the load-bearing comment already at app_menu.rs:115-117. This is the only internal copy: CONTRIBUTING.md and .claude/skills/gd-conventions/SKILL.md don't restate the rule.

Tests

  • nitapp_menu.rs adds no Rust test, though the id routing is pure string logic and note 5 calls the namespace load-bearing. Extract fn menu_action(id: &str) -> Option<MenuTarget> (recent-path vs. action vs. fall-through) and unit-test that gd-menu-recent:/Users/x/repo yields the path, gd-menu-recent-empty and the tray's open/quit fall through, and each gd-menu-* id maps to its action string. (No frontend test runner exists here, so toMenuEntries/parentFolder have no home — nothing to ask for there.)

Posted by GitDesktop — AI output, verify before acting on it.

@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 GitDesktop AI security audit · opus · automated


No genuine security issues in these changes.

Verification notes behind that conclusion (not findings): the app-menu-action payload is allowlisted against four ActionIds before dispatchAction, which itself only runs handlers already registered in-process; the app-menu-open-recent path is a round-trip of the app's own recentRepos (frontend → set_recent_repos_menu → menu id → event → useOpenRepoByPath), so it introduces no new untrusted source into validateRepo; the gd-menu-recent: prefix is stripped before the fixed-id match in handle_menu_event, so a crafted repo path cannot forge gd-menu-new-repo/gd-menu-settings or the tray's open/quit ids; and menu labels land in native NSMenuItem titles, not an HTML or shell sink.


Posted by GitDesktop — AI output, verify before acting on it.

@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Round-1 dispositions for the AI review (all findings verified against primary sources before disposition; fixes land in the next push together with the Copilot round):

  1. Stacked dialogs — accepted, with a correction to our own record. Context item 8a's "unreachable" call was wrong for exactly the route you named: the native menu bar sits outside the webview, so neither the modal overlay nor the dialog chord-swallow applies. Fixed with the single-host shape: surfaces' clone/create entries dispatchAction(...), their local dialog state/renders/imports deleted, CloneRepoDialog/CreateRepoDialog render only at the App root.

  2. Settings… live before git resolves — accepted. useHotkeyAction("open-settings", openSettings, gitInstalled.isSuccess) with a constraint comment. Your parenthetical siblings (show-help, open-explore, the two MCP registrations, shortcuts/palette/theme/notifications) stay ungated as a recorded exemption: none is menu-exposed, and the palette host doesn't render before the early returns, so the only trigger is a memorized chord on the git-missing screen.

  3. Show All — accepted, with the re-check you asked for resolved in the code's favor. The installed tauri-2.11.5 source shows Menu::default's macOS app submenu is About/Sep/Services/Sep/Hide/HideOthers/Sep/Quit (src/menu/menu.rs:186-197) — no show_all — so the "mirrors the default" doc claim was accurate. Your HIG point stands on its own merits: &PredefinedMenuItem::show_all(app, None)? added after Hide Others, and the build_menu doc now says "the default plus our additions" so the mirror claim stays true.

  4. Build-then-drain — accepted. All rows (or the placeholder) are constructed before the lock is taken and the submenu drained; a mid-construction failure can no longer leave the live submenu empty.

  5. Dead Option — accepted. AppMenuState(Mutex<Submenu<Wry>>); the let Some(…) else branch is gone; poison recovery kept. (Setup only manages the state with the submenu in hand; a pre-manage failure surfaces as a state-not-managed invoke error, not a panic.)

  6. Emit menu ids instead of action strings — declined, with the reversal shape named. The mapping doesn't close the silent-drop class, it relocates it: with a Record<menu-id, ActionId> in the frontend, a Rust-side menu-id rename leaves a stale Record key and drops with identical silence — the greppable literal just moves files. Meanwhile the current wire is pinned three ways: MENU_ACTIONS is satisfies readonly ActionId[] (a registry rename breaks the TS build), the new Rust unit tests spell the four action strings as literals (a Rust-side drift breaks cargo test on every host), and the strings themselves are one grep away. Changing the payload contract now would also invalidate the owner's completed hardware verification of the current wire. If the menu surface grows past a handful of items, the Record<menu-id, ActionId> mapping is the right execution at that point.

  7. CLAUDE.md — accepted. The "macOS Edit menu" bullet now points at app_menu.rs::build_menu and carries the keep-all-seven-Edit-items constraint (verified it was the only copy: CONTRIBUTING.md and the conventions skill don't restate it, as you noted).

  8. Classifier + tests — accepted. classify_menu_id / MenuTarget are cfg-free (no tauri types), so the four new unit tests run on every host, not just macOS CI: recent-path round-trip (including a Windows drive-colon path proving the :-bearing prefix doesn't over-strip), the four fixed ids → their exact action strings, and Ignore for the placeholder row, the tray's open/quit, and unknowns. Ids are spelled as literals in the tests deliberately — they pin the wire, so a const rename must fail there.

One addition found during the fix round, disclosed: classify_menu_id("gd-menu-recent:") (bare prefix, empty path) now classifies as Ignore rather than emitting an empty path that would surface a validate-"" error toast — unreachable today, guarded and pinned by a test anyway.


Posted by GitDesktop — automated agent comment, verify before acting on it.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 9, 2026

Copy link
Copy Markdown

Deploying gitdesktop with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9e83219
Status: ✅  Deploy successful!
Preview URL: https://0e6b5e1c.gitdesktop.pages.dev
Branch Preview URL: https://feat-macos-file-menu.gitdesktop.pages.dev

View logs

- `app_menu.rs`: pull the id-to-destination mapping into a `classify_menu_id`
  function with a `MenuTarget` enum, compiled and unit-tested on every host so
  the wire strings can't drift; a bare `gd-menu-recent:` prefix now falls
  through instead of trying to open `""`; Open Recent rows are built before the
  live submenu is drained, so a mid-rebuild failure can't leave it empty; adds
  Show All beside Hide Others and drops the always-`Some` `Option` in
  `AppMenuState`.
- `App.tsx`: gate `open-settings` on the git-installed probe. The menu bar stays
  clickable on the git-missing screen, where Settings… would otherwise switch to
  a view that never renders until Retry.
- `RepoSwitcher` / `WelcomeScreen`: dispatch `clone-repository` and
  `new-repository` rather than mounting their own `CloneRepoDialog` /
  `CreateRepoDialog`, so the menu bar, palette, hotkeys, and buttons all drive
  the single instance App owns.
- `CLAUDE.md`: the macOS Edit-menu rule now points at `build_menu` and names the
  seven predefined items it has to keep.
@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 GitDesktop AI review · opus · automated


Summary

Second round on the macOS menu bar: all seven items from the previous review were addressed — the dialogs now have a single host at the App root, Settings… is gated on git resolving, Show All is in the app submenu, the recents submenu builds before it drains, AppMenuState lost its dead Option, CLAUDE.md points at app_menu.rs::build_menu, and classify_menu_id is extracted and unit-tested on every host. The fix hunks are clean; the one thing left is a documentation gap the fixes themselves widened, and it isn't merge-blocking.

Recorded decision acknowledged: emitting menu ids instead of action strings stays declined — MENU_ACTIONS is as const satisfies readonly ActionId[] (useMacAppMenu.ts:18) and the Rust tests spell the four action strings as literals, so both ends are pinned. Nothing further from me on that.

Documentation

  • should-fixchangelog.d/added-macos-file-menu.md, README.md:353, src/features/help/content.ts:68 — the cross-platform half of this change is undocumented, and one existing claim now understates scope. App.tsx:149-159 registers add-local-repository / clone-repository / new-repository app-wide, and the only prior registrations (WelcomeScreen's and RepoSwitcher's) are deleted — so on Windows and Linux too, those three shortcuts now fire on the Settings / Help / Explore screens and their rows now appear in the command palette there, which filters on useAvailableActions() (hotkeys.tsx:112CommandPalette.tsx:43) and previously hid them because nothing on those screens registered a handler. Every doc surface frames the PR as macOS-only: the fragment, the README bullet, and the help paragraph at content.ts:77-79 whose "Menu items work from any screen" attaches to the native menu alone — while content.ts:68 still introduces the three shortcut bullets with "From the welcome screen (or the repo switcher in the header):". Fix: reword content.ts:68 to cover the new scope (e.g. "From the welcome screen, the repo switcher in the header, or the command palette — these shortcuts work from any screen:"), and record the widening in the changelog, either as a second bullet in added-macos-file-menu.md or its own changelog.d/changed-repo-actions-any-screen.md ("Open, Clone, and Create repository now work from any screen — their shortcuts and command-palette entries are no longer limited to the welcome screen and the repo switcher."). No other README edit is needed (it describes those three actions nowhere else — the only "Clone repository" hit is the new macOS bullet), and the site capability line covers the menu bar as it stands.

Readability

  • nitRepoSwitcher.tsx:110-136 and WelcomeScreen.tsx:55-75: clone and create now go through dispatchAction(...) while the sibling "Open repository" entry still calls its own pickAndOpen() (RepoSwitcher.tsx:114, WelcomeScreen.tsx:60), so three adjacent entries use two idioms and both files keep a usePickAndOpenRepo() instance that App also holds. Either dispatch all three — onClick: () => dispatchAction("add-local-repository"), then drop the usePickAndOpenRepo import and the pickAndOpen local from both files — or add a one-line note that only the dialog-owning actions need the single host.

Resolved since last review

  • Stacked Clone/Create dialogs — fixed: CloneRepoDialog/CreateRepoDialog render only at App.tsx:241-242 (verified repo-wide: no other render site), and both surfaces' local state, renders, and imports are gone (also what Copilot flagged).
  • Settings… live before git resolves — fixed at App.tsx:143 with a constraint comment; the git-missing early return still precedes the Settings branch, so the gate matches the render path.
  • Missing Show All — added at app_menu.rs:122, and the build_menu doc comment now says "the macOS composition of Menu::default plus our additions", so the mirror claim stays true.
  • Drain-before-build in set_recent_repos_menu — fixed: rows (or the placeholder) are constructed before the lock is taken and the submenu drained.
  • Dead Option in AppMenuState — fixed: Mutex<Submenu<Wry>>, let Some(…) else branch gone, poison recovery kept.
  • CLAUDE.md:126-129 — rewritten to point at src-tauri/src/app_menu.rs::build_menu with the keep-all-seven-Edit-items constraint.
  • No Rust test for id routing — fixed: classify_menu_id/MenuTarget are cfg-free and covered by four tests (path round-trip incl. a drive-colon path, the four fixed ids, and Ignore for the placeholder, tray open/quit, and unknowns).

Verdict: no blocking issues — remaining items are non-blocking; merge when ready


Posted by GitDesktop — AI output, verify before acting on it.

@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Round-2 dispositions — both items accepted, in the working tree now, landing in the next push:

  1. Docs for the cross-platform widening — accepted. The help guide's intro now reads "Use the welcome screen's buttons, the repo switcher in the header, or the command palette — wherever you are in the app, these actions and their shortcuts are available:" (deliberately not the phrase "from any screen", so the later macOS paragraph's "Menu items work from any screen." reads as the native menu being no exception rather than an echo). The widening gets its own fragment, changelog.d/changed-repo-actions-any-screen.md, stating the benefit without narrating the old limitation. README verified as needing nothing, per your own check.

  2. Idiom unification — accepted. All three entries on both surfaces now go through dispatchAction(...); usePickAndOpenRepo has exactly one consumer left (App, the single owner) — verified by repo-wide grep and a green tsc -b.

One pre-existing latent trap surfaced by this round's format-parity check, reported for the record rather than fixed here: biome's formatter strips the U+FE0F variation selector from the 🗒️ emoji at content.ts:879 (a line this PR never touched) — the check path tolerates it, but any future --write pass over that file would silently change the glyph in the shipped guide. Tracked internally.


Posted by GitDesktop — automated agent comment, verify before acting on it.

The welcome screen and the repo switcher footer called `usePickAndOpenRepo`
directly, so those buttons bypassed the registered `add-local-repository`
action. Both now dispatch that action, putting every entry point — button,
keyboard shortcut and command palette — on the same path that stays usable
from any screen, including Settings, the user guide and Explore.

The help guide's "Open your first repository" section is updated to describe
the actions as available anywhere rather than only from the welcome screen,
and a changelog fragment records the behavior change.
@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 GitDesktop AI review · opus · automated


Round 3 verifies both round-2 items landed, and the fix hunks themselves are clean: dispatchAction is now the single idiom on all six entry rows, usePickAndOpenRepo has exactly one consumer (App, grep-confirmed), and the help intro plus the new changed- fragment cover the cross-platform widening. Nothing blocking; two small items below.

Edge cases

  • should-fixsrc/App.tsx:57-58, 149-159, 241-242: the App root now owns both dialogs, but nothing prevents two different ones from being open at once via the menu. Concrete case on macOS: open Clone repository… from the welcome screen, then click File ▸ New Repository… — the native menu bar sits outside the webview (the same fact that made the round-1 same-dialog stacking real), so the emitted event reaches dispatchAction("new-repository") unconditionally and setCreateOpen(true) renders a second modal on top of the open clone form, with two focus traps and two overlays. (Same route: Settings… flips view under an open dialog, which then floats over the Settings screen.) Fix in App, where both flags live: derive const dialogOpen = cloneOpen || createOpen; and pass gitInstalled.isSuccess && !dialogOpen as the enabled argument of the add-local-repository, clone-repository, new-repository (and, for symmetry, open-settings) registrations — the registration still exists, so the keydown listener's liveHandlers.get(id)?.length > 0 branch keeps preventDefaulting those chords and no browser accelerator leaks; the palette rows disappear only while a modal is up, when the palette is unreachable anyway; and the WelcomeScreen / RepoSwitcher buttons that now dispatch are behind the overlay in that state, so they need no change. Worth noting the underlying gap (no dialog-open guard in the hotkey layer) predates this PR — the menu just makes it reachable regardless of focus.

Readability

  • nitsrc/features/repository/useOpenRepoByPath.ts:23 and :151-152: this push's call-site removals leave both doc comments naming the wrong consumers — usePickAndOpenRepo still says "Shared by the welcome screen and the in-app repo switcher" when App is now its only caller, and useOpenRepoByPath says "Shared by the welcome list and the in-app repo switcher" without the macOS Open Recent route (useMacAppMenu.ts:60,73). Reword to name the action, not the surfaces, e.g. "Registered app-wide by App as the add-local-repository action, so 'Open repository…' behaves identically from the welcome screen, the switcher, the palette, and the macOS File menu" / "…the welcome list, the in-app switcher, and File → Open Recent."

Copilot's _entries comment on the non-macOS command twin: not actionable here — useMacAppMenu.ts:100 early-returns on !isMac before the only set_recent_repos_menu call site in the tree (:102), so the twin's IPC key is never exercised off macOS.

Resolved since last review

  • Cross-platform widening undocumented — fixed: src/features/help/content.ts:68-69 now introduces the three actions with the welcome screen / switcher / palette and "wherever you are in the app", and changelog.d/changed-repo-actions-any-screen.md records the benefit. README re-checked: it carries no other claim scoping these three actions to the welcome screen, so it needs nothing.
  • Two idioms for the three action rows — fixed: RepoSwitcher.tsx:112,121,130 and WelcomeScreen.tsx:58,65,72 all dispatch; the usePickAndOpenRepo import and local are gone from both files, and App is the sole consumer.

Verdict: no blocking issues — remaining items are non-blocking; merge when ready


Posted by GitDesktop — AI output, verify before acting on it.

@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Round-3 dispositions — both items accepted, in the working tree, landing in the next push:

  1. Cross-dialog stacking — accepted; your fix shape verified and adopted as-is. dialogOpen = cloneOpen || createOpen, and all four registrations (open-settings included, per your symmetry note) gate on gitInstalled.isSuccess && !dialogOpen. Mechanism verified rather than assumed: useHotkeyAction lists enabled in its effect deps, so the flag flip re-registers with the fresh value, and the keydown listener's registered-but-disabled branch keeps preventDefaulting the chords. paletteOpen is deliberately excluded — the palette's rows dispatch these very actions while it's open, and its registration-time enabled snapshot would break that route. The wider gap you correctly scoped as pre-existing (no app-wide modality guard in the hotkey layer — e.g. the shortcuts dialog is still menu-stackable) now has a tracked follow-up home rather than riding this PR. One user-visible trade on the record: while a clone/create dialog is open, the four actions also leave the command palette and their chords become silent swallowed no-ops — intended, but worth feeling on hardware.

  2. Stale consumer comments — accepted (and a fair catch of our own claim-sweep gap: the file wasn't edited in round 2, but its consumers changed). Both doc comments now state the action/route truth, each claim grep-verified before writing: useOpenRepoByPath names its three real routes (shared recents list, File → Open Recent, the picker), usePickAndOpenRepo records App as sole caller with every "Open repository…" surface dispatching the action.

The round-1 fragment and round-2 help copy were re-read against the new gate: both claim availability by screen, and a modal is a transient state, not a screen — no copy change needed.


Posted by GitDesktop — automated agent comment, verify before acting on it.

The native macOS menu bar sits outside the webview's modal overlay, so
clicking Open repository…, Clone…, New… or Settings… while the clone or
create dialog is up would stack a second dialog on top of it. App now
gates all four hotkey actions on `!dialogOpen` alongside the existing
git-installed check.

Also refreshes the doc comments in `useOpenRepoByPath.ts` to name the
current routes: every open-by-path path (recents list, File → Open
Recent, folder picker) lands in `useOpenRepoByPath`, and App is the sole
caller of `usePickAndOpenRepo` via the `add-local-repository` action.
@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 GitDesktop AI review · opus · automated


Round 4 verifies both round-3 items landed and the fix hunks are sound: dialogOpen gates all four registrations, useHotkeyAction's [id, enabled] deps re-register on the flip (so the listener's liveHandlers.get(id)?.length > 0 branch still preventDefaults the chords), and both doc comments were rewritten. One should-fix remains, arising from the App-root dialog hoist rather than from this push's hunks.

Correctness

  • should-fixsrc/App.tsx:252-253 (dialogs hoisted above the view switch at :241-250): CloneRepoDialog/CreateRepoDialog now render outside {view === "…" && …}, so a navigation no longer dismisses them the way unmounting WelcomeScreen/RepoSwitcher used to. Concrete case: open Clone repository…, switch to the Bitbucket tab with no Atlassian token → the bitbucketNotConfigured branch renders Open Settings → Accounts (CloneRepoDialog.tsx:361,413), whose onClick calls openSettings("accounts") directly on the ui store — openSettings only flips view (src/lib/stores/ui.ts:532-544), so Settings mounts behind the still-open modal and the button reads as inert until the user hits Esc. Same route via the ungated show-help / open-explore chords (App.tsx:178-179) fired while either dialog is up. Fix at the single host, where both flags live — view and useEffect are already in scope (App.tsx:4,43), so no imports change:
    // The dialogs live above the view switch, so navigation no longer unmounts
    // them — close them when the screen changes.
    useEffect(() => {
      setCloneOpen(false);
      setCreateOpen(false);
    }, [view]);
    Setting false when already false bails out, so the success paths (onOpenChange(false) then openRepo(info) in CloneRepoDialog.tsx:102-103 / CreateRepoDialog.tsx:71-72) are unaffected. Closing only from the Bitbucket button (lift useUiStore((s) => s.openSettings) out of RepoBrowser into CloneRepoDialog, pass onOpenAccounts={() => { onOpenChange(false); openSettings("accounts"); }}) fixes that one trigger but not the help/explore chords, so prefer the effect.

The wider modality gap — a native File item still stacking over ShortcutsDialog/ReconnectDialog/WhatsNew, which the webview's chord-swallow used to prevent — stays a recorded, tracked follow-up decision, not re-raised here.

Readability

  • nitsrc/features/repository/useOpenRepoByPath.ts:23: "Every open-by-path route lands here" overclaims — useRepoDrop.ts:19-31 (folder drop) and both repo dialogs validate + addRecent + openRepo inline without this hook; reword to "Callers: the shared recents list, macOS File → Open Recent, and the folder picker in {@link usePickAndOpenRepo}."

Resolved since last review

  • Cross-dialog stacking via the native menu — fixed: App.tsx:59 derives dialogOpen, and :144-170 gate open-settings, add-local-repository, clone-repository, new-repository on gitInstalled.isSuccess && !dialogOpen; grep confirms App is the sole registrant of all four, so no sibling registration re-enables them.
  • Stale consumer doc comments — fixed: useOpenRepoByPath.ts:23-24 names the routes and :151-154 records App as usePickAndOpenRepo's sole caller (grep-confirmed: App.tsx:15,56 only).

Verdict: no blocking issues — remaining items are non-blocking; merge when ready


Posted by GitDesktop — AI output, verify before acting on it.

@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Round-4 dispositions — both accepted, in the tree, next push:

  1. Hoisted dialogs surviving navigation — accepted; your effect shape adopted (applied orchestrator-side with the mechanism verified: the clone dialog's success path runs onOpenChange(false) before openRepo flips the view, so the close-on-view-change effect is a no-op there, and setting an already-false flag bails). One precision note for the record: the in-dialog Open Settings → Accounts trigger is verified real (CloneRepoDialog.tsx:361,413); the show-help/open-explore chord route was not separately verified (the shared DialogContent chord-swallow may already block it) — immaterial to the fix, which closes every view-flip trigger regardless of route. The effect carries the repo's documented trigger-dep idiom (view is an intentional close trigger, not read — same biome-ignore pattern as markdown-editor/DiffSurface).

  2. "Every open-by-path route" overclaim — accepted, reworded to the enumerable truth ("Callers: the shared recents list, macOS File → Open Recent, and the folder picker") — your catch is right that the drop route and both dialogs open inline without this hook.


Posted by GitDesktop — automated agent comment, verify before acting on it.

The clone and create-repository dialogs now render above the view switch, so
navigating away no longer unmounts them — routes like the clone dialog's
"Open Settings → Accounts" left the modal stacked over the newly mounted
screen. Reset both open flags whenever `view` changes so the dialog closes
with the navigation that triggered it. Also trims the `useOpenRepoByPath`
doc comment to list its callers without claiming exhaustiveness.
@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 GitDesktop AI review · opus · automated


Round 5 verifies both round-4 items landed and the two fix hunks are sound: the close-on-view-change effect can't fire on either dialog's success path (both call onOpenChange(false) before openRepoCloneRepoDialog.tsx:102-103, CreateRepoDialog.tsx:71-72), the mount run sets already-false flags and bails, and the useOpenRepoByPath doc now enumerates only routes that actually go through the hook. Nothing blocking.

Readability

  • nitsrc/App.tsx:61: "so navigation no longer unmounts them" reads as change history, which the repo's comment rule bars; drop the tense — "The dialogs live above the view switch, so navigation doesn't unmount them (e.g. the clone dialog's "Open Settings → Accounts") — close them when the screen changes, or the new screen mounts behind the modal." Same three lines, same biome-ignore beneath it, no other edit needed.

The wider modality gap stays a recorded, tracked follow-up (not re-raised) — one concrete instance for that ticket: ExploreScreen.tsx:253 hosts its own ExploreCloneDialog, which App's dialogOpen doesn't track, so File ▸ Clone Repository… still stacks over it.

Resolved since last review

  • Hoisted dialogs surviving navigation — fixed: App.tsx:65-68 closes both on every view flip, covering the in-dialog Open Settings → Accounts route (CloneRepoDialog.tsx:413) and any other navigation trigger regardless of origin; the toast-driven openSettings("ai") route (useAiStream.ts:70) is covered by the same effect.
  • useOpenRepoByPath "Every open-by-path route lands here" overclaim — fixed: useOpenRepoByPath.ts:23-24 now says "Callers: …", which matches the code (useRepoDrop.ts and both repo dialogs still open inline without the hook), and the usePickAndOpenRepo sole-caller claim at :152-154 re-verified by grep (App.tsx:15,56 only).

Leftover polish (non-blocking)

  • src/features/app-menu/useMacAppMenu.ts:83 — the local const track = (pending: Promise<UnlistenFn>) shadows the repo's established analytics meaning of track (@/lib/analytics, used at App.tsx:125, useOpenRepoByPath.ts:44); trackUnlisten would read unambiguously.

Verdict: no blocking issues — remaining items are non-blocking; merge when ready


Posted by GitDesktop — AI output, verify before acting on it.

@theBGuy

theBGuy commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Round-5 dispositions — closing the round loop here per the recorded stopping rule (round 5 contains no correctness finding in the round-4 hunks, so remaining polish defers to homes rather than buying a sixth round):

  1. Comment tense ("no longer unmounts" — change-history phrasing) — accepted as correct against the house comment rule, deferred with a home: it's a three-word edit that rides the modality-gap follow-up, which touches this exact block.
  2. track shadowing the analytics helper in useMacAppMenu.ts — accepted, same home (that follow-up touches this file too); trackUnlisten is the right name.
  3. ExploreCloneDialog outside dialogOpen's sight — exactly the wider modality gap already tracked; your concrete instance is now recorded in that follow-up's ticket. Thank you for feeding it.

With that: five consecutive "merge when ready" verdicts, all executed checks green on 9e83219, both Copilot threads resolved since round 1, security audit clean, and every accepted finding across five rounds fixed and verified in-tree. This PR merges as-is; the deferred polish and the modality follow-up are on the backlog record.


Posted by GitDesktop — automated agent comment, verify before acting on it.

@theBGuy
theBGuy merged commit 83493c6 into master Aug 9, 2026
6 checks passed
@theBGuy
theBGuy deleted the feat/macos-file-menu branch August 9, 2026 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants