Skip to content

fix(hook): fall back to executable path for a bundle-less foreground app - #1469

Open
4ni1ak wants to merge 1 commit into
AprilNEA:masterfrom
4ni1ak:fix/macos-bundle-less-foreground-app
Open

4ni1ak wants to merge 1 commit into
AprilNEA:masterfrom
4ni1ak:fix/macos-bundle-less-foreground-app

Conversation

@4ni1ak

@4ni1ak 4ni1ak commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • A frontmost process with no app bundle (a bare Mach-O launched from a shell script, a QEMU front-end, etc.) has NSRunningApplication.bundleIdentifier() return nil, so foreground_app_from_running_application dropped it entirely instead of returning a ForegroundApp.
  • That meant such a process's window could never match a per-app profile and it never showed up in the GUI's Recent picker.

Changes

  • crates/openlogi-hook/src/macos.rs: when bundleIdentifier() is nil, fall back to app.executableURL().path() as the ForegroundApp::id. ForegroundApp::id's own doc already documents the executable path as a legitimate identity namespace (it's what Windows always uses), so this reuses an existing contract instead of inventing a new one.

Testing

  • cargo check -p openlogi-hook --target x86_64-apple-darwin — clean
  • RUSTFLAGS="-D warnings" cargo clippy -p openlogi-hook --target x86_64-apple-darwin --all-targets -- -D warnings — clean
  • cargo fmt --all -- --check — clean
  • Affected-package tier (Linux host): RUSTFLAGS="-D warnings" cargo clippy -p openlogi-hook -p openlogi-agent -p openlogi-agent-core -p openlogi-desktop -p openlogi-overlay --all-targets -- -D warnings — clean
  • RUSTFLAGS="-D warnings" cargo test -p openlogi-hook -p openlogi-agent -p openlogi-agent-core -p openlogi-desktop -p openlogi-overlay — all green
  • Not runtime-tested on macOS hardware. The cross-target check/clippy above verify the objc2 FFI usage compiles and is unsafe-sound, but linking the Apple SDK and exercising real NSWorkspace notifications was not possible on this host.

Fixes #1404

…und app

foreground_app_from_running_application() dropped the frontmost app
entirely when app.bundleIdentifier() was nil — a real state for a bare
Mach-O launched from a shell script or a QEMU front-end, which still
becomes NSWorkspace's frontmost application normally. Its window could
therefore never match a per-app profile, and it never appeared in the
GUI's Recent picker section.

Falls back to app.executableURL().path() in that case.
ForegroundApp::id's own doc already documents the executable path as a
legitimate identity namespace — it's what Windows always uses — so
this reuses an existing contract instead of inventing a new one.

Fixes AprilNEA#1404
@4ni1ak
4ni1ak requested a review from AprilNEA as a code owner September 17, 2026 08:54
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

The functional change appears safe to merge, with non-blocking documentation and regression-test gaps around the new macOS path-identity branch.

Fix All in CodexFindings

  1. P2 Stale macOS identity contract
  2. P2 Fallback branch lacks coverage

Summary

This PR allows macOS foreground applications without bundle identifiers to participate in per-app profiles and the Recent picker by using their executable path as the fallback identity.

  • Preserves bundle identifiers as the preferred macOS identity.
  • Routes path-based identities through the existing foreground observation and Safari snapshot flow.
  • Leaves the expanded public identity contract undocumented and the new fallback branch without regression coverage.

Reviews (1) · Last reviewed commit: "fix(hook): fall back to the executable p..."

Comment on lines +174 to +176
let id_string = match app.bundleIdentifier() {
Some(bundle_id) => bundle_id,
None => app.executableURL().and_then(|url| url.path())?,

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 Stale macOS identity contract

This fallback makes executable paths valid macOS application IDs, but the shared ForegroundApp::id documentation still describes macOS IDs only as bundle identifiers, while the macOS backend documentation still says an app without a bundle identifier returns None. These conflicting contracts could lead callers or future refactors to treat the new path identity as unsupported. Please update both API descriptions to document the fallback.

Knowledge Base Used: Global input hooks

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

Comment on lines +174 to +176
let id_string = match app.bundleIdentifier() {
Some(bundle_id) => bundle_id,
None => app.executableURL().and_then(|url| url.path())?,

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 Fallback branch lacks coverage

This adds bundle-identifier precedence and an executable-path fallback, but no test exercises either branch. The decision is embedded in the AppKit calls even though comparable platform identity logic is extracted into testable code. Without focused coverage, a later refactor could restore the original bundle-less-app bug or reverse the precedence without CI detecting it.

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 windows owned by a bundle-less process can never match a per-app profile

1 participant