parity(launch_app): port Swift #1492 bundle-ID + locale fallbacks to Rust - #1523
parity(launch_app): port Swift #1492 bundle-ID + locale fallbacks to Rust#1523f-trycua wants to merge 2 commits into
Conversation
…Rust Mirror `AppLauncher.locate(bundleId:name:)` from PR #1492 in the Rust port so `launch_app` on macOS accepts bundle identifiers as `name`, locale-specific display names, and case-insensitive variants — not just the exact on-disk bundle filename. The new `apps::locate_app_by_name` runs the same three passes as Swift: 1. Filesystem `<name>.app` lookup in the canonical roots. 2. LaunchServices bundle-ID lookup via `NSWorkspace.URLForApplicationWithBundleIdentifier:` (objc2). 3. Fuzzy scan, case-insensitive: locale-aware `localizedName` from `NSRunningApplication`, then `CFBundleDisplayName` / `CFBundleName` / stem from each candidate bundle's Info.plist. When a match resolves to a bundle ID, `launch_app_by_name` and `launch_with_urls_by_name` now delegate to the bundle-ID launch path (unambiguous, avoids a second LaunchServices lookup inside `open`). Resolver miss falls back to the previous raw `open -g -a <name>` behavior to preserve existing inputs LaunchServices already knows about. Linux and Windows backends are untouched — bundle ID / locale display name is a macOS concept. Verified end-to-end against the Rust binary: launch_app name="com.apple.calculator" → bundle_id=com.apple.calculator launch_app name="CALCULATOR" → bundle_id=com.apple.calculator launch_app name="Calculator" → unchanged (regression guard) launch_app name="no_such_app_xyzzy" → MCP error Adds three parametrized parity tests (`test_mcp_launch_app_by_name_accepts_bundle_id`, `test_mcp_launch_app_by_name_case_insensitive`, `test_mcp_launch_app_unknown_name_raises_error`) and flips the `launch_app` macOS row in PARITY.md to VERIFIED with a cross-reference to the Swift implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR enhances macOS ChangesmacOS launch_app name resolution with three-pass resolver
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@libs/cua-driver-rs/crates/platform-macos/src/apps.rs`:
- Around line 127-129: The equality check allows matching when both
app.bundle_id and resolved.bundle_id are None, causing false PID matches; update
the condition so bundle IDs only count as equal when both are Some and equal.
Replace the current bundle-id comparison (app.bundle_id.as_deref() ==
resolved.bundle_id.as_deref()) with a guarded check such as matching both Some
values and comparing them (e.g., if let (Some(a), Some(b)) =
(app.bundle_id.as_deref(), resolved.bundle_id.as_deref()) && a == b), keeping
the existing display-name case-insensitive check
(app.name.eq_ignore_ascii_case(&resolved.display_name)).
In `@libs/cua-driver-rs/PARITY.md`:
- Around line 471-476: The fenced code block containing the launch_app examples
is missing a language specifier and triggers markdownlint MD040; edit the block
that starts with the triple backticks before the lines beginning "launch_app
name=" and add a language token such as text (e.g., ```text) so the block
becomes fenced with a language and satisfies the linter while preserving the
four example lines.
🪄 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: 02e8ef59-0d93-44aa-a225-9c305408d3ad
⛔ Files ignored due to path filters (1)
libs/cua-driver-rs/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
libs/cua-driver-rs/PARITY.mdlibs/cua-driver-rs/crates/platform-macos/src/apps.rslibs/cua-driver-rs/crates/platform-macos/src/tools/launch_app.rslibs/cua-driver-rs/tests/integration/test_api_parity.py
| if app.name.eq_ignore_ascii_case(&resolved.display_name) | ||
| || app.bundle_id.as_deref() == resolved.bundle_id.as_deref() | ||
| { |
There was a problem hiding this comment.
Guard None bundle-ID equality to avoid false PID matches.
Line 128 can match unrelated running apps when both sides are None, so the function may return the wrong PID.
🔧 Proposed fix
- if app.name.eq_ignore_ascii_case(&resolved.display_name)
- || app.bundle_id.as_deref() == resolved.bundle_id.as_deref()
+ if app.name.eq_ignore_ascii_case(&resolved.display_name)
+ || resolved
+ .bundle_id
+ .as_deref()
+ .is_some_and(|bid| app.bundle_id.as_deref() == Some(bid))
{
return Ok(app.pid);
}🤖 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-rs/crates/platform-macos/src/apps.rs` around lines 127 - 129,
The equality check allows matching when both app.bundle_id and
resolved.bundle_id are None, causing false PID matches; update the condition so
bundle IDs only count as equal when both are Some and equal. Replace the current
bundle-id comparison (app.bundle_id.as_deref() == resolved.bundle_id.as_deref())
with a guarded check such as matching both Some values and comparing them (e.g.,
if let (Some(a), Some(b)) = (app.bundle_id.as_deref(),
resolved.bundle_id.as_deref()) && a == b), keeping the existing display-name
case-insensitive check (app.name.eq_ignore_ascii_case(&resolved.display_name)).
| ``` | ||
| 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=…, (regression guard) | ||
| launch_app name="no_such_app_xyzzy" → MCP error | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced code block.
This block is currently untyped and triggers markdownlint MD040.
📝 Proposed fix
-```
+```text
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=…, (regression guard)
launch_app name="no_such_app_xyzzy" → MCP error</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 471-471: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
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-rs/PARITY.md around lines 471 - 476, The fenced code block
containing the launch_app examples is missing a language specifier and triggers
markdownlint MD040; edit the block that starts with the triple backticks before
the lines beginning "launch_app name=" and add a language token such as text
(e.g., ```text) so the block becomes fenced with a language and satisfies the
linter while preserving the four example lines.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
CR #1523: - apps.rs:127-129: comparing two `None` bundle_ids would silently match an unrelated running app, returning the wrong pid for the launch we just performed. Require both-Some equality. - PARITY.md:471: add `text` language tag to the launch_app example fence to satisfy markdownlint MD040.
|
Implemented in PR #1576. Ported Swift
Pass 1 (exact filename) and Pass 2 (bundle-id-as-name) are unchanged. Three new integration tests added to |
2 participants
Summary
Ports Dylan's just-merged Swift fix from PR #1492 (
3b5c372d) to the Rust port.launch_appon macOS now accepts the same three input shapes the Swift binary does — bundle identifiers asname, locale-specific display names, and case-insensitive variants — not just the exact on-disk bundle filename.The Rust resolver
apps::locate_app_by_namemirrors SwiftAppLauncher.locate(bundleId:name:)with the same three-pass chain:<name>.applookup in the canonical roots (/Applications,/System/Applications[/Utilities],~/Applications,~/Applications/Chrome Apps.localized). Fast, locale-independent.NSWorkspace.URLForApplicationWithBundleIdentifier:(objc2). Letsname="com.apple.calculator"resolve without switching to thebundle_idparameter.localizedNamefromNSRunningApplication(covers e.g.計算機on JP macOS for Calculator).CFBundleDisplayName→CFBundleNamefrom each candidate bundle's Info.plist (viaplutil -extract, matching the existingscan_installed_appspattern in this file)..app).When a match resolves to a bundle ID,
launch_app_by_nameandlaunch_with_urls_by_namedelegate to the bundle-ID launch path (open -g -b) — unambiguous, avoids a second LaunchServices lookup insideopen. Falls back toopen <path>when no bundle ID is recovered, and finally to a rawopen -g -a <name>if the resolver misses (preserves the pre-fix behavior for unusual installs LaunchServices already knows about).Scope
Only
crates/platform-macos/and the parity test/doc files. Linux and Windows backends are untouched — bundle ID / locale display name is a macOS concept (the Windows port already documentsnameas a path/bundle alias).Before / After
Files touched
libs/cua-driver-rs/crates/platform-macos/src/apps.rs— newlocate_app_by_name+ResolvedAppand supporting helpers (url_for_application_with_bundle_identifier,find_running_app_by_localized_name,read_bundle_metadata,app_search_roots).libs/cua-driver-rs/crates/platform-macos/src/tools/launch_app.rs—launch_with_urls_by_namenow runs the resolver first; rawopen -g -aremains as a fallback.libs/cua-driver-rs/tests/integration/test_api_parity.py— three new parametrized parity tests:test_mcp_launch_app_by_name_accepts_bundle_idtest_mcp_launch_app_by_name_case_insensitivetest_mcp_launch_app_unknown_name_raises_errorlibs/cua-driver-rs/PARITY.md—launch_appmacOS row flipped fromOPEN→VERIFIEDwith cross-reference to SwiftAppLauncher.locate(PR cua-driver: fix #1481 app name resolution — bundle ID + locale fallbacks #1492). Notes block ported.libs/cua-driver-rs/Cargo.lock— workspace version bumps (cua-driver 0.1.2→0.1.3, cursor-overlay, mcp-server, focus-monitor-win) refreshed during build; matches the version numbers already committed inCargo.tomlfiles since commitc1731734.Test plan
cargo build -p platform-macos --releaseclean (no new warnings).cargo build --release(full workspace) clean.test_mcp_launch_app_by_bundle_idtest_mcp_launch_app_by_nametest_mcp_launch_app_by_name_accepts_bundle_id(new)test_mcp_launch_app_by_name_case_insensitive(new)test_mcp_launch_app_unknown_name_raises_error(new)~/cua/libs/cua-driver/.build/release/cua-driveris from before3b5c372d, sotest_mcp_launch_app_by_name_accepts_bundle_idcurrently fails on it pre-rebuild — expected, the Swift fix already landed onmain)../run_tests.sh --parityonce Swift binary in CI cache rebuilds against3b5c372d.References
3b5c372d)libs/cua-driver/Sources/CuaDriverCore/Apps/AppLauncher.swift::AppLauncher.locate(bundleId:name:)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests