Skip to content

fix(gui): refresh the macOS device menu when inventory changes#456

Open
Phecda wants to merge 1 commit into
AprilNEA:masterfrom
Phecda:fix/macos-device-menu-refresh
Open

fix(gui): refresh the macOS device menu when inventory changes#456
Phecda wants to merge 1 commit into
AprilNEA:masterfrom
Phecda:fix/macos-device-menu-refresh

Conversation

@Phecda

@Phecda Phecda commented Jul 24, 2026

Copy link
Copy Markdown

Summary

Keep the native macOS Device menu synchronized with the device records shown by the GUI.

Changes

  • openlogi-gui: rebuild the application menu when an agent update changes the displayed state, without rebuilding it for unchanged polling snapshots.
  • Use NoAction for informational disabled menu entries instead of routing them through Settings actions.

Testing

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace
  • Hardware-tested on macOS 26.5.2 with an MX Master 3 over Unifying; after inventory loading, the native Device menu listed the active device records instead of remaining on No devices connected.

Fixes #455

@greptile-apps

greptile-apps Bot commented Jul 24, 2026

Copy link
Copy Markdown

Greptile Summary

This PR keeps the native macOS Device menu synchronized with the live device inventory by calling app_menu::rebuild alongside cx.refresh_windows() whenever the agent snapshot reports a state change, and switches disabled informational menu entries from OpenSettings to gpui::NoAction.

  • main.rs: adds app_menu::rebuild(cx) inside the if changed { … } guard in the IPC snapshot arm, so the Device submenu is rebuilt whenever the merged inventory (or agent-link status) changes — skipped for unchanged polling snapshots.
  • app_menu.rs: device list entries and the "No devices connected" placeholder are now backed by gpui::NoAction instead of OpenSettings, which is semantically correct since both items are always rendered .disabled(true) and should never route to a real action.

Confidence Score: 5/5

Safe to merge — the two-line change is narrowly scoped, the menu rebuild is guarded behind the existing changed flag, and replacing OpenSettings with gpui::NoAction on always-disabled entries is a straightforward correctness fix.

Both changes are small and well-contained. The app_menu::rebuild call reuses the already-tested menus() function and shares the same changed guard that already gates refresh_windows(). The NoAction substitution removes a misleading action association from items that are always disabled and never invoked. The only minor observation is that the menu is rebuilt on agent-link-only changes (not just inventory changes), but set_menus is inexpensive and this has no user-visible effect.

No files require special attention.

Important Files Changed

Filename Overview
crates/openlogi-gui/src/app_menu.rs Disabled informational menu items now use gpui::NoAction instead of OpenSettings — a correctness improvement since these items are never triggerable; no issues found.
crates/openlogi-gui/src/main.rs Calls app_menu::rebuild alongside cx.refresh_windows() when inventory state changes; the rebuild fires on both inventory merges and agent-link transitions (bitwise OR), which is slightly over-eager but harmless.

Sequence Diagram

sequenceDiagram
    participant Agent as openlogi-agent
    participant IPC as IPC Client
    participant SelectLoop as select loop
    participant State as AppState
    participant GPUI as GPUI runtime
    participant Menu as macOS menu bar

    Agent->>IPC: Snapshot (inventory + status)
    IPC->>SelectLoop: GuiUpdate::Snapshot
    SelectLoop->>State: refresh_inventories() + set_agent_link()
    State-->>SelectLoop: "changed = merged | link_changed"
    alt "changed == true"
        SelectLoop->>GPUI: cx.refresh_windows()
        SelectLoop->>Menu: app_menu::rebuild(cx)
        Menu->>State: read device_list
        State-->>Menu: Vec of DeviceRecord
        Menu-->>Menu: set_menus() with updated Device submenu
    else "changed == false"
        SelectLoop-->>SelectLoop: skip no-op
    end
Loading

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(gui): keep macos device menu current" | Re-trigger Greptile

Comment on lines 344 to 347
if changed {
cx.refresh_windows();
app_menu::rebuild(cx);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Menu rebuilt on agent-link-only changes

changed is the bitwise OR of merged (inventory change) and state.set_agent_link(...) (agent-link change). When only the agent link transitions (e.g. Ready → Ready with a new status but identical inventory), app_menu::rebuild fires even though the Device submenu content is identical. This is harmless — set_menus is cheap — but for clarity it could be worth gating the menu rebuild specifically on merged, similar to how the set_agent_link standalone helper only calls cx.refresh_windows() when the link itself changes. Not a bug, just a minor unnecessary call on non-inventory transitions.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code

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.

[Bug]: macOS Device menu does not refresh after devices load

1 participant