cua-driver: fix #1481 app name resolution — bundle ID + locale fallbacks#1492
Open
ddupont808 wants to merge 1 commit into
Open
cua-driver: fix #1481 app name resolution — bundle ID + locale fallbacks#1492ddupont808 wants to merge 1 commit into
ddupont808 wants to merge 1 commit into
Conversation
AppLauncher.locate() now tries three passes when only `name` is given:
1. Filesystem lookup by bundle filename (existing — fast, locale-independent
for English names whose on-disk bundle name matches the display name).
2. LaunchServices bundle-ID lookup — lets callers pass a bundle identifier
string as `name` (e.g. "com.apple.calculator") without switching to the
`bundle_id` parameter.
3. Fuzzy scan:
a) Locale-aware localizedName from NSRunningApplication (covers JP/FR/…
locales where the running app's display name differs from the bundle
filename, e.g. "計算機" on JP macOS for Calculator).
b) CFBundleDisplayName / CFBundleName from Info.plist (English; catches
case-insensitive variants like "calculator" or "CALCULATOR").
c) Bundle URL stem (filename minus .app) as a final fallback.
All matching is case-insensitive throughout.
Before (old binary):
launch_app name="com.apple.calculator" → Error: Could not locate app
launch_app name="calculator" → Error: Could not locate app
After (new binary):
launch_app name="com.apple.calculator" → pid=…, bundle_id=com.apple.calculator
launch_app name="calculator" → pid=…, bundle_id=com.apple.calculator
launch_app name="CALCULATOR" → pid=…, bundle_id=com.apple.calculator
launch_app name="Calculator" → pid=…, (unchanged, regression guard)
Adds 5 integration tests covering all new paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
📝 WalkthroughWalkthroughThis PR adds fallback name resolution to ChangesApp Name Fallback Resolution
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@libs/cua-driver/Sources/CuaDriverCore/Apps/AppLauncher.swift`:
- Around line 293-299: The current logic builds a single displayName by picking
the first non-nil of CFBundleDisplayName, CFBundleName, or the file stem and
then compares it to needle, which skips checking subsequent candidates if the
first exists but doesn't match; update the check in AppLauncher.swift so you
compare needle against each candidate separately (CFBundleDisplayName,
CFBundleName, and the stem from URL(fileURLWithPath:
path).deletingPathExtension().lastPathComponent) — e.g. gather the three
candidate strings and test if any.lowercased() == needle (or otherwise normalize
and compare each) instead of using the single displayName variable for the
match.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a79cdd5-bb1a-4c12-b620-1db19da693a9
📒 Files selected for processing (2)
libs/cua-driver/Sources/CuaDriverCore/Apps/AppLauncher.swiftlibs/cua-driver/Tests/integration/test_app_name_locale_fallback.py
Comment on lines
+293
to
+299
| // CFBundleDisplayName > CFBundleName > stem | ||
| let displayName = | ||
| (bundle.infoDictionary?["CFBundleDisplayName"] as? String) | ||
| ?? (bundle.infoDictionary?["CFBundleName"] as? String) | ||
| ?? URL(fileURLWithPath: path) | ||
| .deletingPathExtension().lastPathComponent | ||
| if displayName.lowercased() == needle { |
Contributor
There was a problem hiding this comment.
CFBundle fallback order is implemented incorrectly
On Line 294-Line 299, CFBundleDisplayName ?? CFBundleName ?? stem only tests one value. If CFBundleDisplayName exists but doesn’t match, CFBundleName is never compared, so valid names can still fail resolution.
Suggested fix
- // CFBundleDisplayName > CFBundleName > stem
- let displayName =
- (bundle.infoDictionary?["CFBundleDisplayName"] as? String)
- ?? (bundle.infoDictionary?["CFBundleName"] as? String)
- ?? URL(fileURLWithPath: path)
- .deletingPathExtension().lastPathComponent
- if displayName.lowercased() == needle {
- return URL(fileURLWithPath: path)
- }
- // Also match against the raw stem ("Calculator" → "Calculator.app")
- let stem = URL(fileURLWithPath: path)
- .deletingPathExtension().lastPathComponent
- if stem.lowercased() == needle {
+ let displayName =
+ bundle.infoDictionary?["CFBundleDisplayName"] as? String
+ let bundleName =
+ bundle.infoDictionary?["CFBundleName"] as? String
+ let stem = URL(fileURLWithPath: path)
+ .deletingPathExtension().lastPathComponent
+
+ if displayName?.lowercased() == needle
+ || bundleName?.lowercased() == needle
+ || stem.lowercased() == needle
+ {
return URL(fileURLWithPath: path)
}🤖 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 `@libs/cua-driver/Sources/CuaDriverCore/Apps/AppLauncher.swift` around lines
293 - 299, The current logic builds a single displayName by picking the first
non-nil of CFBundleDisplayName, CFBundleName, or the file stem and then compares
it to needle, which skips checking subsequent candidates if the first exists but
doesn't match; update the check in AppLauncher.swift so you compare needle
against each candidate separately (CFBundleDisplayName, CFBundleName, and the
stem from URL(fileURLWithPath: path).deletingPathExtension().lastPathComponent)
— e.g. gather the three candidate strings and test if any.lowercased() == needle
(or otherwise normalize and compare each) instead of using the single
displayName variable for the match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1481 —
launch_app name=now resolves beyond the exact bundle filename.Root cause:
AppLauncher.locate()only did a filesystem lookup for<name>.app. Callers who passed a bundle ID string ("com.apple.calculator") or a locale-specific display name ("計算機"on JP macOS) got a silent "not found" error.Fix: Three-pass fallback chain in
AppLauncher.locate():<name>.appin standard directoriesNSWorkspace.urlForApplication(withBundleIdentifier: name)for callers who pass"com.apple.calculator"asnameNSRunningApplication.localizedName(locale-aware — matches"計算機"on JP macOS)CFBundleDisplayName/CFBundleNamefrom Info.plist (English; case-insensitive).app)Before / After
Issue #1491 (mcp TCC)
Already fixed in PR #1479 (
41c6afdf).cua-driver mcp --helpconfirms--no-daemon-relaunchand the auto-daemon-launch are present on current main. No code changes needed.Test plan
test_app_name_locale_fallback.py— all passlaunch_app name="ThisAppDoesNotExist_xyzzy"correctly returnsisError: true🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests