Hide Window on start config option#528
Conversation
When Finicky is cold-launched to handle a URL, the GetURL/openFile Apple Event may be delivered after applicationDidFinishLaunching: runs. Deciding to auto-open the config window synchronously raced that event, briefly flashing the window before the target browser opened. Defer the auto-open decision by a short, named interval so a pending URL event has a chance to set receivedURL first. An explicit --window request still shows the window immediately.
Adds a new boolean config option, suppressWindow (default false), that prevents the config window from opening automatically on launch when no URL is being handled. The menu-bar "Show Window" item and the --window flag still open it on demand, and error windows are unaffected, so the user is never locked out. Plumbed through the config schema, the JS option reader, the JSON rules options (so it works from the UI too), the Objective-C app delegate, and the finicky-ui options panel. Also bumps the bundle version label.
The suppressWindow value was omitted from the config message sent to the webview, so the new "Don't open window" toggle always rendered off even when the option was enabled in the config. Send it like the other options.
…apps When the config window was already open in the background and a URL was opened from another app, delivering the GetURL Apple Event activated Finicky and pulled the open window to the foreground, flashing it before the target browser opened. When Finicky wasn't the active app, hide it again on the next runloop turn so it stays out of the way.
A second local build failed with "Directory not empty" because `mv Finicky-arm64.app Finicky.app` nests the new bundle inside the existing Finicky.app directory instead of replacing it. Clean the previous build's .app bundles before building.
… launch deferral Rename the config option to hideWindowOnStart, which describes precisely what it does: the window is only ever auto-opened on launch (routing a URL never opens it), so the option simply controls the launch window. Updated across the schema, option reader, JSON rules, Objective-C delegate, and the UI (now labelled "Hide window on start"). Also remove the cold-launch auto-open deferral. The window auto-opens only on launch, so the synchronous check is sufficient; the timed deferral guarded a launch-time race that isn't worth the added complexity. The background-hide on URL routing is unaffected.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughAdds a hideWindowOnStart option and wires it from schema/types through UI, Go VM config and JS propagation, Objective‑C app startup, and tests; also adds a pre-build cleanup step for stale .app bundles. ChangesHide window on start feature
Sequence DiagramsequenceDiagram
participant UserUI as StartPage UI
participant VM as WebView/VM
participant GoMain as main.go
participant ObjC as BrowseAppDelegate
participant AppKit as NSApp
participant Browser as DefaultBrowser
UserUI->>VM: saveRules (hideWindowOnStart)
VM->>GoMain: GetAllConfigOptions (reads hideWindowOnStart)
GoMain->>ObjC: RunApp(hideWindowOnStart)
ObjC->>ObjC: applicationDidFinishLaunching
alt hideWindowOnStart == true and no URL
ObjC->>ObjC: skip opening window
else
ObjC->>AppKit: orderFront window
end
ObjC->>Browser: route URL
alt Finicky not frontmost
ObjC->>AppKit: dispatch hide on next main queue (NSApp hide:nil)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
| -o ../build/${APP_NAME}/Contents/MacOS/Finicky | ||
| } | ||
|
|
||
| # Remove stale .app bundles from previous builds. Without this, a second local |
There was a problem hiding this comment.
not sure that is needed that but locally I've stuck on rebuild because of some files left from previous build run
I can remove or create a separate PR for that code change
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/finicky/assets/Info.plist`:
- Around line 20-22: The Info.plist currently sets CFBundleShortVersionString
and CFBundleVersion to "4.4.0-alpha+suppress-window", which is invalid for App
Store validation; update CFBundleShortVersionString to a digits-and-dots-only
version like "4.4.0" and set CFBundleVersion to a numeric build identifier with
1–3 dot-separated non-negative integers (e.g., "4.4.0" or "440" depending on
your build scheme) and move the "alpha+suppress-window" pre-release/branch
metadata out of these keys (e.g., into a custom key or CI/build metadata) so
CFBundleShortVersionString and CFBundleVersion contain only numeric values.
In `@apps/finicky/src/main.go`:
- Around line 252-258: The startup path currently skips applying options from
rules-only files because setupVM only constructs a VM when defaultBrowser or
rules exist, leaving newVM nil and therefore vm.GetAllConfigOptions() never
consulted; update setupVM (and the startup logic that reads newVM/vm) to build
or at least parse options when a rules file contains only options (i.e. detect
presence of options even without defaultBrowser/rules), ensure newVM is non-nil
or that you extract config via the same GetAllConfigOptions flow so
hideWindowOnStart (and other option flags) are respected during startup; change
the logic around setupVM/newVM and the call site that reads
vm.GetAllConfigOptions() so options-only rule files set hideWindowOnStart
consistently (also apply same fix to the other occurrences noted around the
other call sites).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71d957ae-a666-4269-a3ce-885598887eb8
📒 Files selected for processing (10)
apps/finicky/assets/Info.plistapps/finicky/src/config/vm.goapps/finicky/src/main.goapps/finicky/src/main.happs/finicky/src/main.mapps/finicky/src/rules/rules.gopackages/config-api/src/configSchema.tspackages/finicky-ui/src/pages/StartPage.sveltepackages/finicky-ui/src/types.tsscripts/build.sh
| <string>4.4.0-alpha+suppress-window</string> | ||
| <key>CFBundleVersion</key> | ||
| <string>4.4.0-alpha</string> | ||
| <string>4.4.0-alpha+suppress-window</string> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the allowed formats for CFBundleShortVersionString and CFBundleVersion in Apple's Info.plist? Is "4.4.0-alpha+suppress-window" valid for both keys?
💡 Result:
The string "4.4.0-alpha+suppress-window" is not a valid format for either CFBundleShortVersionString or CFBundleVersion [1][2]. According to Apple's current technical documentation, both keys must adhere to the following requirements: CFBundleShortVersionString This key represents the user-visible release version number of the bundle [1]. It must be a string consisting only of numeric characters (0-9) and periods, typically in a [Major].[Minor].[Patch] format (e.g., "10.14.1") [1]. CFBundleVersion This key represents the machine-readable build version number of the bundle [2]. It must be a string composed of one to three period-separated integers (e.g., "10.14.1") [2]. The string can only contain numeric characters (0-9) and periods [2]. While some older, legacy documentation mentioned support for specific alpha/beta suffixes, modern App Store submission requirements strictly enforce the numeric-only, period-separated integer format [2][3][4]. Using characters such as hyphens (-), plus signs (+), or alphabetic characters will cause validation errors during the submission process to App Store Connect [5][4]. To comply, you should use a purely numeric format for both keys, such as "4.4.0" [1][2].
Citations:
- 1: https://developer.apple.com/documentation/bundleresources/information-property-list/cfbundleshortversionstring
- 2: https://developer.apple.com/documentation/bundleresources/information-property-list/cfbundleversion
- 3: https://developer.apple.com/forums/thread/101592
- 4: https://stackoverflow.com/questions/25418798/cfbundleversion-must-be-a-period-separated-list-of-at-most-three-non-negative-in
- 5: https://stackoverflow.com/questions/66990121/what-is-the-accepted-string-format-for-cfbundleversion-and-cfbundleshortversions
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact surrounding contents of the referenced plist
FILE="apps/finicky/assets/Info.plist"
nl -ba "$FILE" | sed -n '1,60p'Repository: johnste/finicky
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/finicky/assets/Info.plist"
echo "=== lines 15-30 ==="
sed -n '15,30p' "$FILE"
echo "=== occurrences of 4.4.0-alpha+suppress-window ==="
rg -n "4\.4\.0-alpha\+suppress-window" "$FILE" || trueRepository: johnste/finicky
Length of output: 721
Fix CFBundleShortVersionString/CFBundleVersion to Apple-compliant numeric formats
apps/finicky/assets/Info.plist sets both keys to 4.4.0-alpha+suppress-window (lines 20-22). Apple requires:
CFBundleShortVersionString: digits and periods only (e.g.,4.4.0)CFBundleVersion: 1–3 period-separated non-negative integers only
This value contains - and +, which will fail Info.plist/App Store validation. Use numeric-only values for these keys and move the alpha/branch metadata elsewhere.
<string>4.4.0-alpha+suppress-window</string>
<key>CFBundleVersion</key>
<string>4.4.0-alpha+suppress-window</string>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/finicky/assets/Info.plist` around lines 20 - 22, The Info.plist
currently sets CFBundleShortVersionString and CFBundleVersion to
"4.4.0-alpha+suppress-window", which is invalid for App Store validation; update
CFBundleShortVersionString to a digits-and-dots-only version like "4.4.0" and
set CFBundleVersion to a numeric build identifier with 1–3 dot-separated
non-negative integers (e.g., "4.4.0" or "440" depending on your build scheme)
and move the "alpha+suppress-window" pre-release/branch metadata out of these
keys (e.g., into a custom key or CI/build metadata) so
CFBundleShortVersionString and CFBundleVersion contain only numeric values.
The version label was bumped only to distinguish local fork test builds; revert it so this PR doesn't change versioning, which is the maintainer's call. This also removes the non-Apple-compliant +suppress-window suffix from CFBundleShortVersionString/CFBundleVersion.
setupVM only constructed a VM when a rules file had a defaultBrowser or rules, so a rules file with only an options block (e.g. hideWindowOnStart) left the VM nil and the options fell back to defaults at startup. Build the VM when options are present too, matching the SaveRulesHandler path.
Guards the startup fix: a rules file with only an options block must still serialize its options (e.g. hideWindowOnStart) into the generated config script, falling back to the default browser.
This unconditional [NSApp hide:nil] when routing a URL from another app was out of scope for the hideWindowOnStart feature: it changed behavior for all users regardless of the option and could hide a window the user deliberately left open. With hideWindowOnStart the window no longer auto-opens, so there's nothing to flash; the manual-open edge case, if it matters, belongs in its own focused change.
This should help address the issue mentioned in #495
If you close the Finicky window, it doesn't appear when opening URLs. So if we also avoid opening the window on app start, the window stops being a problem
This PR adds a hideWindowOnStart option that does exactly that: when enabled, Finicky won't open its window on launch. The default behavior is unchanged.
Tested it locally, looks good
Fixes #495
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores