Skip to content

fix: resolve Firefox/Chromium profiles when the browser is configured by path#529

Open
xrl wants to merge 2 commits into
johnste:mainfrom
xrl:fix/firefox-profile-by-path
Open

fix: resolve Firefox/Chromium profiles when the browser is configured by path#529
xrl wants to merge 2 commits into
johnste:mainfrom
xrl:fix/firefox-profile-by-path

Conversation

@xrl

@xrl xrl commented Jun 6, 2026

Copy link
Copy Markdown

Problem

When a handler specifies the browser by path (appType: "path"), e.g.

browser: { name: "/Applications/Firefox.app", appType: "path", profile: "Work" }

the requested profile is silently ignored and the browser opens without it.

For Firefox the symptom is worse: with no profile argument, the launch falls into Firefox's default profile selection, which on recent versions (the new multi-profile feature — ShowSelector in profiles.ini) pops the profile picker window on every link instead of opening the chosen profile.

Root cause

resolveBrowserProfileArgs (and GetProfilesForBrowser) look up the browser in browsers.json by bundle ID or app name only:

if browser.ID == identifier || browser.AppName == identifier { ... }

With appType: "path" the identifier is the bundle path (/Applications/Firefox.app), which matches neither org.mozilla.firefox nor Firefox, so no match is found and no profile argument is emitted.

Fix

Add findBrowserInfo, which additionally matches an .app bundle path by its basename (minus .app) against the app name, and use it in both call sites. Profile detection now works for path-based configs for Firefox and Chromium alike. The usual -P <name> (Firefox) / --profile-directory= (Chromium) argument is emitted as before — which, for Firefox, also keeps the launch out of the default-selection path that triggers the picker.

Behavior for bundle-ID and app-name configs is unchanged.

Tests

Added launcher_test.go covering findBrowserInfo: match by app name, bundle ID, and .app path (including names with spaces such as "Firefox Developer Edition"), Chromium by path, plus negative cases (unknown app, unknown path, and no partial-prefix match).

Verified

Built and run locally on macOS (Apple Silicon, Firefox 151): links now open in the configured profile with no profile-picker window, routing into the running per-profile instance as a new tab.


The second commit is a small, unrelated build fix: the local build.sh path could mv a freshly built bundle into a stale build/Finicky.app and then abort under set -e before installing, silently leaving the old app in place. Happy to split it into its own PR if you'd prefer.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Resolved build process issue where stale app bundles could interfere with installation.
    • Enhanced browser profile detection to support app bundle paths in addition to existing identifier formats.
  • Tests

    • Added comprehensive test suite for browser identification logic, covering various identifier types and edge cases.

xrl and others added 2 commits June 6, 2026 14:46
resolveBrowserProfileArgs and GetProfilesForBrowser looked up the browser
in browsers.json by bundle ID or app name only. A handler that specifies
the browser by path (appType "path", e.g. name "/Applications/Firefox.app")
passes that path as the identifier, which matches neither, so no profile
argument was emitted and the browser opened without the requested profile.

For Firefox this had a worse symptom: with no profile argument the launch
fell into Firefox's default profile selection, which on recent versions
shows the new profile-selector ("ShowSelector") window. Passing -P/-profile
avoids that path entirely.

Add findBrowserInfo, which also matches an ".app" bundle path by its
basename (minus ".app") against the app name, and use it in both call
sites. Profile detection now works for path-based configs for Firefox and
Chromium alike.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The local build path ran `mv Finicky-arm64.app Finicky.app` without first
removing an existing build/Finicky.app. If one was left from a previous
build, mv nested the new bundle inside the old directory and, under
`set -e`, the script aborted before copying to /Applications -- silently
leaving the previously installed app in place.

Remove the destination bundle before the mv.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 316f3e13-69fb-4883-92a1-cd9087190b51

📥 Commits

Reviewing files that changed from the base of the PR and between 9313bca and fc6683b.

📒 Files selected for processing (3)
  • apps/finicky/src/browser/launcher.go
  • apps/finicky/src/browser/launcher_test.go
  • scripts/build.sh

📝 Walkthrough

Walkthrough

This PR consolidates browser matching logic into a single reusable helper and fixes a build-script race condition. The findBrowserInfo function unifies three matching strategies (bundle ID, app name, and .app path) and replaces two separate lookup loops, allowing profile resolution to recognize browsers specified by app paths. Build cleanup prevents nested bundle nesting from mv operations.

Changes

Browser lookup consolidation

Layer / File(s) Summary
Browser lookup helper and integration
apps/finicky/src/browser/launcher.go
findBrowserInfo helper matches browsers by bundle ID, app name, or .app path basename. resolveBrowserProfileArgs and GetProfilesForBrowser both replace their manual loops with calls to this helper.
Browser lookup test coverage
apps/finicky/src/browser/launcher_test.go
Table-driven test TestFindBrowserInfo validates matching across app name, bundle ID, and .app paths (including spaces), and confirms negative cases for unknown apps and partial-path matches.
Build script stale bundle cleanup
scripts/build.sh
Removes any existing Finicky.app bundle before renaming the freshly built bundle to prevent mv from nesting inside a stale one.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

Poem

🐰 A rabbit's helper hops the path,
Finds browsers by their bundle, name, or app,
No more loops that walk about,
Just findBrowserInfo does the route!
Build script cleanup clears the way,
Fresh apps bundle every day! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: fixing profile resolution for Firefox/Chromium when configured by path, which is the core issue addressed across launcher.go, launcher_test.go, and build.sh.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant