Skip to content

feat(list_apps): unify running + installed apps across platforms - #1545

Merged
f-trycua merged 9 commits into
mainfrom
feat/cua-driver-rs-unified-list-apps
May 17, 2026
Merged

feat(list_apps): unify running + installed apps across platforms#1545
f-trycua merged 9 commits into
mainfrom
feat/cua-driver-rs-unified-list-apps

Conversation

@f-trycua

@f-trycua f-trycua commented May 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Extends list_apps so every platform returns a single flat array where each entry self-describes whether it's currently running or installed-only, and carries the metadata launch_app needs to start it cold.

  • Unified shape (all three platforms): { pid, name, bundle_id, running, active, kind, launch_path, last_used, windows }. Pre-change fields are unchanged in name, position, and type; the new fields are additive.
  • macOS: backfills launch_path and last_used onto running entries by matching bundle id against the existing /Applications scan; existing installed-not-running entries now carry kind: "desktop".
  • Windows: adds a installed_apps module that walks both Start-Menu shortcut roots and resolves each .lnk via IShellLinkW::GetPath for desktop apps, plus enumerates UWP packages via WinRT Management::Deployment::PackageManager::FindPackagesWithPackageTypes(Main). Running pids are merged against the desktop installed set by executable basename; UWP packages get launch_path = "shell:appsFolder\\{family}!App".
  • Linux: adds a installed_apps module that walks the XDG application directories ($XDG_DATA_HOME/applications + each $XDG_DATA_DIRS entry's applications/ subdir), parses each .desktop file's [Desktop Entry] section (filtering NoDisplay=true, Hidden=true, Type!=Application), and strips XDG field codes from Exec= to produce launch_path. Running pids matched against installed apps by exe basename.
  • Adds Management_Deployment, ApplicationModel, Foundation_Collections, Win32_Storage_FileSystem, Win32_UI_Shell_Common, Win32_UI_Shell_PropertiesSystem, Win32_System_Com_StructuredStorage to the windows crate features. No new third-party dependencies on any platform.
  • Backwards-compat: the legacy processes alias key remains on Linux/Windows for callers reading the old running-only shape.
  • Docs: PARITY.md gets the new shape, per-platform enumeration breakdown, and verification recipes; docs/.../mcp-tools.mdx gets a per-field rundown of which values apply to which platform.

Test plan

  • cargo build --release clean on macOS (after every commit).
  • cargo check --target x86_64-pc-windows-msvc -p platform-windows clean cross-target (Windows live run pending on a Windows host — see PARITY.md for the recipe).
  • python3 -m unittest test_api_parity.RustParityTests.test_*list_apps* — 5/5 pass on macOS.
  • Manual smoke: cua-driver call list_apps on macOS returns 105 entries (19 running, 86 installed-only), every entry has all 9 expected fields, installed-only entries carry launch_path + last_used + kind: "desktop".
  • Windows: live-run smoke per the PARITY.md recipe (cross-target check passes; needs a Windows host for the runtime call).
  • Linux: live-run smoke per the PARITY.md recipe (needs a Linux host with cargo + the Rust port to install).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • list_apps now returns a unified cross‑platform apps list (running + installed) with fields: pid, running, active, name, bundle_id, kind, launch_path, last_used, and a reserved windows array. Legacy processes alias retained for running-only callers.
    • Launch operations accept launch_path directly and resolve targets more reliably.
  • Documentation

    • Updated reference docs with cross‑platform contract, sources per platform, and guidance to use list_windows for per-window state.

Review Change Stack

@vercel

vercel Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 17, 2026 5:54pm

Request Review

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5b908b3-74bd-4d36-98af-6998d9450f14

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Extends list_apps across Linux, macOS, and Windows to emit a unified apps array combining running and installed discoveries with standardized per-app fields (pid, name, bundle_id, running, active, kind, launch_path, last_used, windows) and a legacy processes alias for running-only callers.

Changes

Cross-platform app enumeration and unified list_apps

Layer / File(s) Summary
API contract and documentation
docs/content/docs/cua-driver/reference/mcp-tools.mdx, libs/cua-driver-rs/PARITY.md
Defines unified apps schema and documents platform-specific enumeration sources (XDG .desktop, Start Menu .lnk, WinRT packages, /proc, osascript), verification recipes, and the legacy processes alias.
Linux installed app scanning
libs/cua-driver-rs/crates/platform-linux/src/installed_apps.rs
New XDG desktop scanner parsing [Desktop Entry], filtering NoDisplay/Hidden/Type, extracting Name/Exec, computing canonical bundle_id, normalizing Exec, optional last_used from mtime, de-duplication, sorting, and unit tests.
Linux list_apps tool integration
libs/cua-driver-rs/crates/platform-linux/src/lib.rs, libs/cua-driver-rs/crates/platform-linux/src/tools/impl_.rs
Exports installed_apps module; rewrites list_apps to merge /proc running processes with installed .desktop entries by normalized executable basename, adds exec_basename and disambiguate_installed_match, updates text/structured output and launch_app to accept launch_path.
macOS AppInfo structure and merge
libs/cua-driver-rs/crates/platform-macos/src/apps/mod.rs
Adds optional launch_path, kind, last_used to AppInfo; running-app parse sets kind; list_all_apps backfills running entries from installed scan keyed by bundle_id, deduplicates, and scan_installed_apps populates filesystem-derived last_used and launch_path.
macOS tool description and output
libs/cua-driver-rs/crates/platform-macos/src/tools/list_apps.rs
Expands tool description and structured invoke JSON to include launch_path, kind, last_used, and a reserved empty windows array while preserving backward-compatible fields.
Windows dependencies for installed apps
libs/cua-driver-rs/crates/platform-windows/Cargo.toml
Adds Win32 and WinRT feature flags required for Start Menu .lnk resolution and PackageManager enumeration.
Windows installed app scanning
libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs
New module aggregating Start Menu .lnk (resolved via COM IShellLinkW) and UWP/WinRT PackageManager results into InstalledApp entries with kind, bundle_id, launch_path, optional RFC3339 last_used, deduplication, and timestamp helpers.
Windows list_apps tool integration
libs/cua-driver-rs/crates/platform-windows/src/win32/mod.rs, libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs
Exports installed_apps API; rewrites Windows list_apps to build pid->exe map, match running pids to installed metadata by basename, merge metadata, append installed-not-running entries, update human and structured outputs, and add split_launchable_target + launch_path-aware launch_app behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • trycua/cua#1544: Overlaps with Windows launch_app validation/launch_path handling and UWP/AUMID launch behavior.

Poem

🐰 I hopped through .desktop, .lnk, and plist,

Merged PIDs and installs with a tiny twist,
One list to show running and those that rest,
Tidy fields and timestamps — now all in one nest,
Cheers from a rabbit, on cross-platform quest!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective: unifying running and installed apps enumeration across all platforms (macOS, Windows, Linux) into a single consolidated list.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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
  • Commit unit tests in branch feat/cua-driver-rs-unified-list-apps

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (3)
docs/content/docs/cua-driver/reference/mcp-tools.mdx (1)

525-530: 💤 Low value

Consider mentioning the legacy processes alias for backward compatibility.

The PARITY.md document (lines 372-374) indicates that a legacy processes key is maintained as an alias for the running subset on Linux/Windows for backward compatibility. User-facing documentation might benefit from mentioning this to help users migrating from older versions understand the relationship between the new unified apps array and the legacy shape.

📝 Suggested addition

Add a note after line 530:

 `list_apps` is not a prerequisite.
+
+**Backward compatibility note:** On Linux and Windows, the response includes a legacy `processes` 
+alias containing only the running subset for compatibility with older callers expecting the 
+pre-unification shape. New integrations should use the `apps` array directly.
🤖 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 `@docs/content/docs/cua-driver/reference/mcp-tools.mdx` around lines 525 - 530,
Add a short note after the paragraph that references list_apps and launch_app
explaining the legacy processes alias: state that the unified apps array
supersedes the old shape but on Linux/Windows a legacy "processes" key is still
provided as an alias for the running subset (see PARITY.md), to help users
migrating from older versions understand that processes maps to the running-apps
subset of list_apps results.
libs/cua-driver-rs/crates/platform-macos/src/tools/list_apps.rs (1)

54-66: 💤 Low value

Consider reducing duplication between AppInfo and JSON output.

The structured JSON is manually constructed rather than using serde to serialize AppInfo directly. This duplicates the field list, creating a maintenance burden: if AppInfo gains new fields, this manual construction must be updated in sync.

Additionally, there's a minor inconsistency: AppInfo uses #[serde(skip_serializing_if = "Option::is_none")] on optional fields, but the manual JSON includes them as null. While this may be intentional for explicit API contracts, it's worth noting.

♻️ Potential approach using serde directly

If the windows field could be added to AppInfo (or a wrapper struct), you could serialize directly and avoid duplication:

 let structured = serde_json::json!({
-    "apps": apps.iter().map(|a| serde_json::json!({
-        "pid": a.pid,
-        "name": a.name,
-        "bundle_id": a.bundle_id,
-        "active": a.active,
-        "running": a.running,
-        "launch_path": a.launch_path,
-        "kind": a.kind,
-        "last_used": a.last_used,
-        "windows": Vec::<serde_json::Value>::new(),
-    })).collect::<Vec<_>>()
+    "apps": apps.iter().map(|a| {
+        let mut val = serde_json::to_value(a).unwrap();
+        val["windows"] = serde_json::json!([]);
+        val
+    }).collect::<Vec<_>>()
 });

However, the current approach gives you explicit control over output shape, which may be preferable for API stability.

🤖 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/tools/list_apps.rs` around lines
54 - 66, The JSON construction in list_apps.rs duplicates AppInfo fields and
forces optional fields to null; instead serialize AppInfo (or a small wrapper
that adds the windows Vec) via serde to avoid manual mapping. Modify the code
that builds structured (currently mapping over apps.iter() and creating
serde_json::json! objects) to collect the apps into either Vec<AppInfo> or
Vec<WrapperWithWindows> and call serde_json::to_value or serde_json::json!(...)
on that collection so AppInfo's #[serde(skip_serializing_if =
"Option::is_none")] behavior is preserved and future field changes don't require
updating this manual mapping.
libs/cua-driver-rs/crates/platform-macos/src/apps/mod.rs (1)

438-463: ⚡ Quick win

The hand-rolled RFC3339 formatter is correctly implemented.

The unix_secs_to_rfc3339 function correctly implements Howard Hinnant's civil date algorithm. Testing confirms the output matches reference datetime calculations across a range of cases including epoch zero, negative timestamps, leap days, and future dates. The use of div_euclid and rem_euclid correctly handles negative timestamp semantics.

However, the function lacks unit tests in the codebase. While the implementation is sound, adding tests would prevent regressions and document expected behavior for future maintainers.

Alternative: Consider using a well-tested date/time crate like chrono or time instead of hand-rolling this formatter. While the current implementation is correct, reducing custom date arithmetic lowers long-term maintenance risk.

🤖 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/mod.rs` around lines 438 -
463, The code adds a hand-rolled RFC3339 formatter (unix_secs_to_rfc3339) but
has no unit tests; add tests that call unix_secs_to_rfc3339 covering epoch 0,
negative timestamps (pre-1970), leap-day examples (e.g., 1972-02-29), boundary
seconds (end/start of day), and a couple of future dates to assert exact
expected strings, and include edge-case assertions for rem_euclid/div_euclid
behavior; place these as #[cfg(test)] mod tests in the same module or a
companion tests module and use fixed known epoch-to-RFC3339 pairs to prevent
regressions.
🤖 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-linux/src/installed_apps.rs`:
- Around line 38-49: The code currently uses file_stem() to derive bundle IDs
which flattens nested .desktop files and can merge distinct launchers; implement
a desktop_file_id(root, path) helper that computes the ID as the path stripped
of the root prefix, removes the ".desktop" suffix and replaces path separators
with a safe character (e.g., '-'), then use that ID when inserting into seen
(replace the current app.bundle_id logic in the xdg_application_dirs scan and
the other branch around the parse_desktop_file usage at the 95–102 area) so
nested .desktop paths produce unique, path-derived bundle IDs and system/user
precedence still works.

In `@libs/cua-driver-rs/crates/platform-linux/src/tools/impl_.rs`:
- Around line 122-129: The current HashMap<String, usize> named by_exe built in
the loop that uses exec_basename(&app.launch_path) overwrites entries when
multiple installed apps share the same basename; change by_exe to map to
Vec<usize> (e.g., HashMap<String, Vec<usize>>) and push each index into the
vector (use entry().or_default().push(i)) so collisions are preserved, then
update the subsequent merge logic that looks up by_exe (the code that assigns
name/bundle_id/launch_path to running processes) to iterate over the candidate
indices and pick the best match (prefer exact launch_path or bundle_id match,
then fallback to first candidate) instead of assuming a single index. Ensure all
references to by_exe and the merge block are updated accordingly (exec_basename,
installed, and the merge routine).
- Around line 97-99: The docs for list_apps reference calling
launch_app(launch_path=...) but the launch_app input schema (properties name,
bundle_id, urls, additionalProperties: false) rejects launch_path; update the
contract by adding a launch_path string property to the launch_app input schema
(alongside name, bundle_id, urls) and ensure validation accepts it, or
alternatively change the list_apps documentation to instruct callers to pass
bundle_id or name instead of launch_path; locate the launch_app schema
definition and modify it to include "launch_path": { "type": "string" } (or
update list_apps text to remove the launch_path example) so examples and schema
remain consistent.

In `@libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs`:
- Around line 185-196: The code builds a HashMap named by_exe mapping executable
basename -> single usize index which silently overwrites earlier entries for
duplicate basenames (created in the loop over installed.iter() using basename
derived from app.launch_path), causing wrong installed-app merges; change by_exe
to map basename -> Vec<usize> (or Vec<InstalledApp> indices) and push each index
instead of insert, then update the merge logic (the code later that looks up
by_exe.get(basename)) to handle multiple candidates by checking full launch_path
or other disambiguating fields (e.g., exact launch_path match or additional
metadata) before selecting the correct installed app; apply the same change to
the other occurrence mentioned (lines ~204-221).
- Around line 126-127: Update the UWP launch_path documentation in impl_.rs so
it accurately reflects what the enumeration emits: change or extend the doc
string that currently says `uwp: shell:appsFolder\\{PackageFamilyName}!{AppId}`
to indicate the observed fallback token form (e.g., `!App`) or list both
possibilities (accurate AppId and fallback `!App`), ensuring the doc for the
`launch_path` field (the UWP case in the Windows enumeration code) matches the
actual emitted token.

In `@libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs`:
- Around line 113-137: The resolve_lnk path resolution currently uses
read_lnk_target which calls IShellLinkW::GetPath but ignores
IShellLinkW::GetArguments, causing dropped shortcut arguments; modify
read_lnk_target (and thus resolve_lnk usage) to also call GetArguments, append
the arguments to the resolved target (preserving quoting/whitespace as
appropriate) and return the full commandline so resolve_lnk sets both bundle_id
and launch_path to the target+arguments string (and keeps the .exe-only check
based on the executable path portion), ensuring argument-driven shortcuts are
preserved in metadata.

---

Nitpick comments:
In `@docs/content/docs/cua-driver/reference/mcp-tools.mdx`:
- Around line 525-530: Add a short note after the paragraph that references
list_apps and launch_app explaining the legacy processes alias: state that the
unified apps array supersedes the old shape but on Linux/Windows a legacy
"processes" key is still provided as an alias for the running subset (see
PARITY.md), to help users migrating from older versions understand that
processes maps to the running-apps subset of list_apps results.

In `@libs/cua-driver-rs/crates/platform-macos/src/apps/mod.rs`:
- Around line 438-463: The code adds a hand-rolled RFC3339 formatter
(unix_secs_to_rfc3339) but has no unit tests; add tests that call
unix_secs_to_rfc3339 covering epoch 0, negative timestamps (pre-1970), leap-day
examples (e.g., 1972-02-29), boundary seconds (end/start of day), and a couple
of future dates to assert exact expected strings, and include edge-case
assertions for rem_euclid/div_euclid behavior; place these as #[cfg(test)] mod
tests in the same module or a companion tests module and use fixed known
epoch-to-RFC3339 pairs to prevent regressions.

In `@libs/cua-driver-rs/crates/platform-macos/src/tools/list_apps.rs`:
- Around line 54-66: The JSON construction in list_apps.rs duplicates AppInfo
fields and forces optional fields to null; instead serialize AppInfo (or a small
wrapper that adds the windows Vec) via serde to avoid manual mapping. Modify the
code that builds structured (currently mapping over apps.iter() and creating
serde_json::json! objects) to collect the apps into either Vec<AppInfo> or
Vec<WrapperWithWindows> and call serde_json::to_value or serde_json::json!(...)
on that collection so AppInfo's #[serde(skip_serializing_if =
"Option::is_none")] behavior is preserved and future field changes don't require
updating this manual mapping.
🪄 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: b2b4b760-cafe-4866-9b71-03401d14e488

📥 Commits

Reviewing files that changed from the base of the PR and between b4536a9 and b7b5074.

⛔ Files ignored due to path filters (1)
  • libs/cua-driver-rs/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • docs/content/docs/cua-driver/reference/mcp-tools.mdx
  • libs/cua-driver-rs/PARITY.md
  • libs/cua-driver-rs/crates/platform-linux/src/installed_apps.rs
  • libs/cua-driver-rs/crates/platform-linux/src/lib.rs
  • libs/cua-driver-rs/crates/platform-linux/src/tools/impl_.rs
  • libs/cua-driver-rs/crates/platform-macos/src/apps/mod.rs
  • libs/cua-driver-rs/crates/platform-macos/src/tools/list_apps.rs
  • libs/cua-driver-rs/crates/platform-windows/Cargo.toml
  • libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs
  • libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs
  • libs/cua-driver-rs/crates/platform-windows/src/win32/mod.rs

Comment on lines +38 to +49
for root in xdg_application_dirs() {
let Ok(entries) = fs::read_dir(&root) else { continue };
for entry in entries.flatten() {
let path = entry.path();
if path.extension().and_then(|e| e.to_str()) != Some("desktop") { continue }
let Some(app) = parse_desktop_file(&path) else { continue };
// Per the spec, user-scope files (XDG_DATA_HOME) override
// system-scope ones with the same basename. xdg_application_dirs
// already yields user-scope first, so insert-if-absent gives us
// the precedence right.
seen.entry(app.bundle_id.clone()).or_insert(app);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Desktop file ID handling is incomplete and can merge distinct apps incorrectly.

Line 38-Line 49 only scans one directory level, and Line 95-Line 102 uses file_stem() as bundle_id. Per XDG desktop file ID semantics, nested .desktop paths are valid and IDs are path-derived; current behavior can miss apps and collapse different launchers into one key.

Suggested direction
- for root in xdg_application_dirs() {
-     let Ok(entries) = fs::read_dir(&root) else { continue };
-     for entry in entries.flatten() {
-         let path = entry.path();
-         if path.extension().and_then(|e| e.to_str()) != Some("desktop") { continue }
-         let Some(app) = parse_desktop_file(&path) else { continue };
-         seen.entry(app.bundle_id.clone()).or_insert(app);
-     }
- }
+ for root in xdg_application_dirs() {
+     for path in walk_desktop_files(&root) {
+         let Some(mut app) = parse_desktop_file(&path) else { continue };
+         if let Some(id) = desktop_file_id(&root, &path) {
+             app.bundle_id = id;
+             seen.entry(app.bundle_id.clone()).or_insert(app);
+         }
+     }
+ }
fn desktop_file_id(root: &Path, path: &Path) -> Option<String> {
    let rel = path.strip_prefix(root).ok()?.to_string_lossy().to_string();
    let rel = rel.strip_suffix(".desktop")?.replace('/', "-");
    Some(rel)
}

Also applies to: 95-102

🤖 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-linux/src/installed_apps.rs` around lines
38 - 49, The code currently uses file_stem() to derive bundle IDs which flattens
nested .desktop files and can merge distinct launchers; implement a
desktop_file_id(root, path) helper that computes the ID as the path stripped of
the root prefix, removes the ".desktop" suffix and replaces path separators with
a safe character (e.g., '-'), then use that ID when inserting into seen (replace
the current app.bundle_id logic in the xdg_application_dirs scan and the other
branch around the parse_desktop_file usage at the 95–102 area) so nested
.desktop paths produce unique, path-derived bundle IDs and system/user
precedence still works.

Comment thread libs/cua-driver-rs/crates/platform-linux/src/tools/impl_.rs Outdated
Comment thread libs/cua-driver-rs/crates/platform-linux/src/tools/impl_.rs Outdated
Comment thread libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs Outdated
Comment thread libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs Outdated
@f-trycua

Copy link
Copy Markdown
Collaborator Author

Pushed 5 fixup commits addressing the review:

  • 66f29d19 fix(linux): use desktop file id (path-derived) for bundle_iddesktop_file_id(root, path) helper handles nested .desktop files so foo.desktop and category/foo.desktop no longer collide. New unit tests cover the flat / nested / outside-root cases.
  • 4abbc812 fix(merge): bucket installed-app indices by exe basename to avoid collisions — HashMap<String, Vec<usize>> on Linux + Windows with a disambiguate_installed_match tie-breaker that prefers an exact launch-path match against the running cmdline (Linux), then the most recent last_used.
  • d27dea89 fix(windows-installed): preserve IShellLinkW::GetArguments in the resolved launch_path — shortcuts that encode behavior in args (Chrome profile launchers, per-workspace VS Code) now round-trip. bundle_id stays as the exe path alone so the by-basename merge in list_apps still works.
  • 5b38be34 feat(launch_app): accept launch_path for round-trip from list_apps on Linux + Windows. Windows resolution precedence is now launch_path > path > bundle_id-with-! (AUMID) > bundle_id > name. A split_launchable_target helper turns a launch_path carrying shortcut args into ShellExecuteEx's file + args pair (AUMID / shell-launch tokens pass through unchanged).
  • 3d5deeaa docs+tests: UWP {AppId} fallback note (!App when manifest lacks Application.Id), legacy processes-alias paragraph in mcp-tools.mdx, and 7 new unix_secs_to_rfc3339 unit tests (epoch, pre-epoch, leap-day in a leap year, Feb 28 in a non-leap year, end-of-year wrap, an arbitrary recent timestamp, pre-2000).

Full workspace cargo build --release and cargo test --release are clean on macOS; cargo check --target x86_64-pc-windows-msvc -p platform-windows is clean. The lychee link-check failure is pre-existing (unrelated to these changes).

@coderabbitai please re-review.

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

@f-trycua Thanks for the detailed summary of the fixup commits! The fixes look well-targeted — the desktop_file_id collision fix, HashMap<String, Vec<usize>> bucketing with disambiguate_installed_match, IShellLinkW::GetArguments round-trip, the launch_path-first resolution chain in launch_app, and the additional unix_secs_to_rfc3339 unit tests all directly address the concerns raised in the prior review. Kicking off a re-review now.

✅ Actions performed

Full review triggered.

Extends the macOS `list_apps` response so each entry includes the
fields needed for cross-platform parity:

- `launch_path` — the `.app` bundle path `launch_app` would consume.
- `kind` — `"desktop"` on macOS for `.app` bundles; reserved for
  packaged-app discrimination on other platforms.
- `last_used` — RFC3339 mtime of the bundle, when readable.
- `windows` — empty array placeholder; per-window state stays in
  `list_windows` for cost reasons, but the key is present so callers
  can rely on the unified shape.

Backwards-compat: `pid`, `name`, `bundle_id`, `running`, `active`
remain in the same positions; new fields are additive. Running
entries get their `launch_path` and `last_used` backfilled from the
installed-app scan when the bundle id matches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f-trycua and others added 6 commits May 17, 2026 19:49
Adds a Windows installed-apps source so `list_apps` returns both
running and installed-but-not-running entries in one flat array.

Two enumeration backends:

- **Start-Menu shortcuts**: walks the machine-wide and per-user
  `Start Menu\Programs` directories, resolves each `.lnk` via
  `IShellLinkW::GetPath` (COM apartment-threaded init), and emits
  entries whose targets are `.exe` paths.

- **UWP / packaged apps**: queries WinRT
  `Management::Deployment::PackageManager::FindPackagesWithPackageTypes(Main)`
  for every Main package and builds the canonical
  `shell:appsFolder\{PackageFamilyName}!App` launch token from the
  package family name. Wrapped in `catch_unwind` so legacy Windows
  builds without `PackageManager` degrade to "no UWP entries"
  rather than a panic.

Running pids matched to a Start-Menu target by executable basename
are folded into a single entry with `running: true`; unmatched
installed entries are emitted with `running: false, pid: 0`. The
legacy `processes` alias is preserved for any pre-existing callers
that read the old shape.

Adds `Management_Deployment`, `ApplicationModel`,
`Foundation_Collections`, `Win32_Storage_FileSystem`,
`Win32_UI_Shell_Common`, `Win32_UI_Shell_PropertiesSystem`,
`Win32_System_Com_StructuredStorage` to the `windows` crate's
feature list — needed for the `IShellLinkW` and `PackageManager`
call surfaces. No new third-party dependencies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `installed_apps` module that walks every XDG application
directory ($XDG_DATA_HOME/applications, then each $XDG_DATA_DIRS
entry's applications/ subdir per the XDG Base Directory Spec) and
parses each `.desktop` file's `[Desktop Entry]` section for the
fields `list_apps` needs: `Name`, `Exec`, `Type`, `NoDisplay`,
`Hidden`. Entries with `NoDisplay=true`, `Hidden=true`, or
`Type!=Application` are filtered out.

The Exec value has its XDG field codes (`%U`, `%f`, `%i`, etc.)
stripped so the result is a clean launcher command callers can hand
to `launch_app(launch_path=...)`. User-scope entries override
system-scope entries with the same desktop file id, matching the
spec's precedence.

`list_apps` itself now merges running processes against the
installed-app index by executable basename — `Exec=firefox %u` and a
running `/usr/bin/firefox` collapse into one entry with `running:
true`. Unmatched installed entries are emitted with `running: false,
pid: 0`.

No new crate dependencies — XDG parsing is a small hand-rolled INI
walker (`extract_desktop_entry_section` + `string_key` + `bool_key`)
that doesn't try to model the full spec, just the keys this enumerator
uses. Unit tests cover the field-code stripper, the `NoDisplay`
filter, the minimal-entry parse path, and the RFC3339 formatter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates the parity audit and the public MCP-tools reference to
describe the new `list_apps` response shape: a single flat array
where every entry carries `pid`, `name`, `bundle_id`, `running`,
`active`, `kind`, `launch_path`, `last_used`, `windows`.

PARITY.md gains:
- the unified-shape JSON example,
- a per-platform enumeration breakdown (NSWorkspace + .app scan on
  macOS, Start-Menu .lnk + WinRT PackageManager on Windows, /proc +
  XDG .desktop on Linux),
- backwards-compatibility notes for callers reading the old running-
  only shape,
- verification recipes per platform.

mcp-tools.mdx gains a fuller per-field rundown so agents can reason
about which fields apply to which platform without reading the
parity audit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The XDG Desktop Entry spec defines the "desktop file id" as the path
relative to the applications/ root with separators replaced by `-` and
the `.desktop` suffix stripped — not just `file_stem()`. The previous
code collapsed nested launchers (`category/foo.desktop` and `foo.desktop`
both produced `"foo"`), causing one to overwrite the other in the dedup
HashMap.

Introduce a `desktop_file_id(root, path)` helper, derive the bundle_id
once at scan time, and pass it into `parse_desktop_file`. Update the
Linux list_apps tool description and the public docs to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lisions

When several installed launchers share the same executable basename
(common: multiple `firefox`/`code` shortcuts, snap/flatpak duplicates),
the previous `HashMap<String, usize>` overwrite-on-insert silently
dropped all but the last entry, so a running pid could be merged into
the wrong installed-app metadata.

Switch the bucket to `HashMap<String, Vec<usize>>` on Linux and Windows
and pick a winner via `disambiguate_installed_match`, which prefers an
exact launch-path match against the running process's cmdline (Linux),
then the most recently modified launcher.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…unch_path

`IShellLinkW::GetPath` returns only the target executable, so shortcuts
that encode behavior in their arguments (Chrome profile launchers,
per-workspace VS Code shortcuts, `code.exe path\\to\\workspace`) lost
that context — `launch_app(path=...)` could not reproduce the shortcut.

Also call `IShellLinkW::GetArguments` and append the result to the path
with a quoted exe when it contains whitespace. `bundle_id` keeps the
executable alone so the by-basename merge in list_apps still matches
the running pid's process-table name.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
libs/cua-driver-rs/crates/platform-linux/src/installed_apps.rs (1)

40-53: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Nested .desktop files in subdirectories are not discovered.

The scanning loop uses fs::read_dir(&root) which only lists immediate children. Files in subdirectories like kde4/konqbrowser.desktop or flatpak/ entries won't be found, even though desktop_file_id() correctly handles nested paths.

Consider adding recursive directory walking:

Sketch of recursive walking
 for root in xdg_application_dirs() {
-    let Ok(entries) = fs::read_dir(&root) else { continue };
-    for entry in entries.flatten() {
-        let path = entry.path();
-        if path.extension().and_then(|e| e.to_str()) != Some("desktop") { continue }
+    for path in walk_desktop_files(&root) {
         let id = desktop_file_id(&root, &path);
         // ...
     }
 }

+fn walk_desktop_files(root: &Path) -> impl Iterator<Item = PathBuf> {
+    walkdir::WalkDir::new(root)
+        .into_iter()
+        .filter_map(|e| e.ok())
+        .filter(|e| e.path().extension().map_or(false, |ext| ext == "desktop"))
+        .map(|e| e.path().to_path_buf())
+}

Alternatively, a simple recursive fs::read_dir would work without adding a dependency.

🤖 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-linux/src/installed_apps.rs` around lines
40 - 53, The loop that iterates xdg_application_dirs() currently calls
fs::read_dir(&root) which only lists immediate children so nested .desktop files
are missed; replace the direct read_dir call with a recursive directory walk
(either a small helper like walk_dir_recursive(root) that yields file paths by
recursing into subdirectories or an explicit stack/Vec loop) and feed each
discovered file path into the existing checks (path.extension(),
desktop_file_id(&root, &path), parse_desktop_file(&path, &id)) and finally
insert into seen via seen.entry(app.bundle_id.clone()).or_insert(app) so
precedence logic is unchanged; note desktop_file_id and parse_desktop_file can
remain as-is because they already handle nested paths.
🧹 Nitpick comments (1)
libs/cua-driver-rs/PARITY.md (1)

354-359: 💤 Low value

Consider clarifying Windows desktop bundle_id format.

Line 358 mentions "Merged by exe basename" but doesn't explicitly state what bundle_id contains for Windows desktop apps. The mcp-tools.mdx (line 492) states it's "the resolved .exe path." Consider adding this detail here for completeness.

📝 Suggested clarification
 - **Windows**: running set from visible top-level windows
   (`EnumWindows` → owner pids) + `CreateToolhelp32Snapshot` for the
   pid→exe table. Installed set is the union of Start-Menu
   `.lnk` shortcuts (resolved via `IShellLinkW::GetPath`) and WinRT
   `Management::Deployment::PackageManager::FindPackagesWithPackageTypes(Main)`.
-  Merged by exe basename. UWP entries carry
+  Desktop apps use the exe path as `bundle_id`; merged by exe basename. UWP entries carry
   `launch_path = "shell:appsFolder\\{PackageFamilyName}!App"`.
🤖 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/PARITY.md` around lines 354 - 359, Clarify the Windows
desktop bundle_id by editing the paragraph that currently reads "Merged by exe
basename" to explicitly state that for Windows desktop apps `bundle_id` is the
resolved full path to the .exe (the resolved `.exe` path), matching the
description in mcp-tools.mdx; mention that UWP entries instead use the shell
launch path format already shown (`launch_path =
"shell:appsFolder\\{PackageFamilyName}!App"`).
🤖 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/mod.rs`:
- Around line 363-366: The map construction for installed_by_bundle currently
collects all entries and lets later duplicates overwrite earlier ones; change
the dedup logic so you only insert the first (or otherwise deterministic) entry
per bundle_id instead of blindly collecting/overwriting. Replace the current
installed.iter().filter_map(...).collect() pattern used to build
installed_by_bundle with an explicit fold/for_each that uses the HashMap entry
API (or Iterator::fold) to insert only if entry.is_vacant() (or apply a clear
selection rule such as preferring non-empty launch_path/last_used), and apply
the same change to the other constructed installed_by_bundle occurrence
referenced around lines 385-391 so both merges deduplicate by bundle_id
deterministically.

In `@libs/cua-driver-rs/crates/platform-macos/src/tools/list_apps.rs`:
- Around line 16-20: The description is inconsistent: it tells callers to "Pass
this to `launch_app`" for `launch_path` but elsewhere shows calling
`launch_app({bundle_id: ...})`; update the text so it matches the macOS contract
by clearly stating which argument `launch_app` expects — e.g., change the
sentence about `launch_path` to: "`launch_path`: filesystem path to the `.app`
bundle, when known; the macOS contract expects callers to invoke `launch_app`
with the `bundle_id` (e.g., `launch_app({bundle_id: ...})`); `launch_path` is
provided for callers that need a cold-start path or for implementations that
accept a `launch_path` parameter." Ensure the same clarification is applied to
the other occurrence referenced (lines 28-29) and keep references to the
`launch_path`, `bundle_id`, and `launch_app` symbols so callers are unambiguous.

In `@libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs`:
- Around line 192-205: The current loop that builds by_exe using
Path::new(&app.launch_path).file_name() fails for launch_path strings that
include quoted executables plus arguments (e.g. `"C:\Path\app.exe" --flag`);
update the logic inside for (i, app) in installed.iter().enumerate() to first
try extracting the real executable name by parsing app.launch_path for a quoted
path (strip surrounding quotes and trim, then take the file_name) and, if that
yields empty/None, fall back to using app.bundle_id (or parse bundle_id's
file_name) before lowercasing and inserting into by_exe; ensure the variable
basename is computed from the parsed executable path (or bundle_id) so shortcuts
with arguments map correctly.

---

Duplicate comments:
In `@libs/cua-driver-rs/crates/platform-linux/src/installed_apps.rs`:
- Around line 40-53: The loop that iterates xdg_application_dirs() currently
calls fs::read_dir(&root) which only lists immediate children so nested .desktop
files are missed; replace the direct read_dir call with a recursive directory
walk (either a small helper like walk_dir_recursive(root) that yields file paths
by recursing into subdirectories or an explicit stack/Vec loop) and feed each
discovered file path into the existing checks (path.extension(),
desktop_file_id(&root, &path), parse_desktop_file(&path, &id)) and finally
insert into seen via seen.entry(app.bundle_id.clone()).or_insert(app) so
precedence logic is unchanged; note desktop_file_id and parse_desktop_file can
remain as-is because they already handle nested paths.

---

Nitpick comments:
In `@libs/cua-driver-rs/PARITY.md`:
- Around line 354-359: Clarify the Windows desktop bundle_id by editing the
paragraph that currently reads "Merged by exe basename" to explicitly state that
for Windows desktop apps `bundle_id` is the resolved full path to the .exe (the
resolved `.exe` path), matching the description in mcp-tools.mdx; mention that
UWP entries instead use the shell launch path format already shown (`launch_path
= "shell:appsFolder\\{PackageFamilyName}!App"`).
🪄 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: d6f4f8de-9944-4594-b9b4-ec8765d55dfb

📥 Commits

Reviewing files that changed from the base of the PR and between b4536a9 and 3d5deea.

⛔ Files ignored due to path filters (1)
  • libs/cua-driver-rs/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • docs/content/docs/cua-driver/reference/mcp-tools.mdx
  • libs/cua-driver-rs/PARITY.md
  • libs/cua-driver-rs/crates/platform-linux/src/installed_apps.rs
  • libs/cua-driver-rs/crates/platform-linux/src/lib.rs
  • libs/cua-driver-rs/crates/platform-linux/src/tools/impl_.rs
  • libs/cua-driver-rs/crates/platform-macos/src/apps/mod.rs
  • libs/cua-driver-rs/crates/platform-macos/src/tools/list_apps.rs
  • libs/cua-driver-rs/crates/platform-windows/Cargo.toml
  • libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs
  • libs/cua-driver-rs/crates/platform-windows/src/win32/installed_apps.rs
  • libs/cua-driver-rs/crates/platform-windows/src/win32/mod.rs

Comment on lines +363 to +366
let installed_by_bundle: std::collections::HashMap<String, (Option<String>, Option<String>)> =
installed.iter()
.filter_map(|a| a.bundle_id.clone().map(|b| (b, (a.launch_path.clone(), a.last_used.clone()))))
.collect();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Deduplicate installed entries by bundle_id before merge.

The installed scan can return multiple rows for the same bundle id (multiple roots/symlinks/copies). Current merge may emit duplicates, and backfill chooses whichever duplicate was last inserted into installed_by_bundle.

Suggested fix
-    let installed = scan_installed_apps();
+    let installed = scan_installed_apps();
+    let mut installed_unique: std::collections::HashMap<String, AppInfo> =
+        std::collections::HashMap::new();
+    for app in installed {
+        if let Some(bid) = app.bundle_id.clone() {
+            // Keep first-seen deterministically by scan order (or replace with your preferred tie-breaker).
+            installed_unique.entry(bid).or_insert(app);
+        }
+    }
+    let mut installed: Vec<AppInfo> = installed_unique.into_values().collect();

     // Lookup: bundle_id → (launch_path, last_used) from the installed scan.
     let installed_by_bundle: std::collections::HashMap<String, (Option<String>, Option<String>)> =
-        installed.iter()
+        installed.iter()
             .filter_map(|a| a.bundle_id.clone().map(|b| (b, (a.launch_path.clone(), a.last_used.clone()))))
             .collect();

Also applies to: 385-391

🤖 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/mod.rs` around lines 363 -
366, The map construction for installed_by_bundle currently collects all entries
and lets later duplicates overwrite earlier ones; change the dedup logic so you
only insert the first (or otherwise deterministic) entry per bundle_id instead
of blindly collecting/overwriting. Replace the current
installed.iter().filter_map(...).collect() pattern used to build
installed_by_bundle with an explicit fold/for_each that uses the HashMap entry
API (or Iterator::fold) to insert only if entry.is_vacant() (or apply a clear
selection rule such as preferring non-empty launch_path/last_used), and apply
the same change to the other constructed installed_by_bundle occurrence
referenced around lines 385-391 so both merges deduplicate by bundle_id
deterministically.

Comment on lines +16 to +20
- launch_path: filesystem path to the `.app` bundle, when known. \
Pass this to `launch_app` to start the app cold.\n\
- kind: `\"desktop\"` for `.app` bundles on macOS.\n\
- last_used: RFC3339 timestamp from the bundle's filesystem mtime, \
when readable; otherwise null.\n\n\

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Tool description is internally inconsistent about launch input.

The text says launch_path should be passed to launch_app, but later says to call launch_app({bundle_id: ...}). Please align this wording to the macOS contract to avoid misleading callers.

Suggested fix
-            - launch_path: filesystem path to the `.app` bundle, when known. \
-            Pass this to `launch_app` to start the app cold.\n\
+            - launch_path: filesystem path to the `.app` bundle, when known \
+            (metadata for parity/output; macOS launch uses `bundle_id`).\n\

Also applies to: 28-29

🤖 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/tools/list_apps.rs` around lines
16 - 20, The description is inconsistent: it tells callers to "Pass this to
`launch_app`" for `launch_path` but elsewhere shows calling
`launch_app({bundle_id: ...})`; update the text so it matches the macOS contract
by clearly stating which argument `launch_app` expects — e.g., change the
sentence about `launch_path` to: "`launch_path`: filesystem path to the `.app`
bundle, when known; the macOS contract expects callers to invoke `launch_app`
with the `bundle_id` (e.g., `launch_app({bundle_id: ...})`); `launch_path` is
provided for callers that need a cold-start path or for implementations that
accept a `launch_path` parameter." Ensure the same clarification is applied to
the other occurrence referenced (lines 28-29) and keep references to the
`launch_path`, `bundle_id`, and `launch_app` symbols so callers are unambiguous.

Comment on lines +192 to +205
let mut by_exe: std::collections::HashMap<String, Vec<usize>> =
std::collections::HashMap::new();
for (i, app) in installed.iter().enumerate() {
if app.kind == "desktop" {
let basename = std::path::Path::new(&app.launch_path)
.file_name()
.and_then(|s| s.to_str())
.unwrap_or("")
.to_ascii_lowercase();
if !basename.is_empty() {
by_exe.entry(basename).or_default().push(i);
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Basename extraction from launch_path may fail for shortcuts with arguments.

When launch_path contains a quoted exe with arguments (e.g. "C:\Path\chrome.exe" --profile), Path::new(&app.launch_path).file_name() won't correctly extract the executable basename. It will parse the entire string as a path, potentially returning None or an incorrect value like the last argument segment.

Consider extracting from bundle_id instead (which is the raw exe path without arguments) or parsing the quoted executable from launch_path:

Suggested fix
 for (i, app) in installed.iter().enumerate() {
     if app.kind == "desktop" {
-        let basename = std::path::Path::new(&app.launch_path)
+        // bundle_id is the raw exe path; launch_path may include arguments
+        let basename = std::path::Path::new(&app.bundle_id)
             .file_name()
             .and_then(|s| s.to_str())
             .unwrap_or("")
             .to_ascii_lowercase();
         if !basename.is_empty() {
             by_exe.entry(basename).or_default().push(i);
         }
     }
 }
🤖 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-windows/src/tools/impl_.rs` around lines
192 - 205, The current loop that builds by_exe using
Path::new(&app.launch_path).file_name() fails for launch_path strings that
include quoted executables plus arguments (e.g. `"C:\Path\app.exe" --flag`);
update the logic inside for (i, app) in installed.iter().enumerate() to first
try extracting the real executable name by parsing app.launch_path for a quoted
path (strip surrounding quotes and trim, then take the file_name) and, if that
yields empty/None, fall back to using app.bundle_id (or parse bundle_id's
file_name) before lowercasing and inserting into by_exe; ensure the variable
basename is computed from the parsed executable path (or bundle_id) so shortcuts
with arguments map correctly.

f-trycua and others added 2 commits May 17, 2026 19:52
…inux + windows)

list_apps surfaces a `launch_path` per entry; callers should be able to
hand that exact string back to launch_app to start the app cold. Add the
field to the input schema on Linux and Windows and wire it through.

Windows resolution precedence is now
`launch_path > path > bundle_id-with-! (AUMID) > bundle_id > name`, and
launch_path values that carry shortcut arguments (preserved by the
IShellLinkW::GetArguments fix) are split into ShellExecuteEx's
file token + args tail via `split_launchable_target`.

Linux launch_path goes through the same direct-exec path as `name`; AUMID
logic does not apply.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tests

- Document that Windows UWP `launch_path` uses
  `shell:appsFolder\{family}!{appId}` and that `{appId}` falls back to
  `App` when the manifest does not surface an Application.Id (matches
  the actual enumeration behavior).
- Note the legacy `processes` alias still emitted on Linux + Windows
  for pre-unification callers; new callers should read `apps` and
  filter on `running`.
- Add `unix_secs_to_rfc3339` unit tests on macOS covering epoch,
  pre-epoch negatives, leap-day in a leap year, Feb 28 in a non-leap
  year, end-of-year wrap, an arbitrary recent timestamp, and a
  known pre-2000 timestamp.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@f-trycua
f-trycua force-pushed the feat/cua-driver-rs-unified-list-apps branch from 3d5deea to 979f0af Compare May 17, 2026 17:52
@f-trycua
f-trycua merged commit d891621 into main May 17, 2026
4 of 7 checks passed
f-trycua added a commit that referenced this pull request May 18, 2026
…re (#1548)

* fix(windows): doctor COM lifecycle + Session 0 hardening for launch paths

Three fixes for Windows correctness in non-interactive sessions:

1) **doctor segfault (0xC0000005 ACCESS_VIOLATION).** The
   `ui_automation_available` probe was calling `CoUninitialize` while the
   `probe` binding still held the IUIAutomation interface. When the
   binding dropped at function return, `IUnknown::Release` ran against a
   torn-down apartment and segfaulted. Fix: flatten the probe result into
   a plain `Result<(), String>` first so the IUIAutomation drops before
   we tear down the apartment.

2) **launch_app hang in Session 0 via name-based UWP lookup.**
   `resolve_aumid_by_name` walked `shell:AppsFolder` via the interactive
   shell broker, which doesn't exist in services/SSH contexts and hangs
   the COM call forever. Detect Session 0 and short-circuit to None so
   the caller falls through to ShellExecuteEx's PATH lookup (which works
   in Session 0 for any app reachable via PATH).

3) **launch_uwp hang in Session 0 via explicit aumid.**
   `IApplicationActivationManager::ActivateApplication` needs the
   per-user AppX runtime which doesn't exist in Session 0. Same
   short-circuit: fail fast with a clear error pointing the caller at
   the workaround (interactive logon / scheduled task in user session).

Also adds best-effort foreground-window restoration after UWP
activation — the AppX runtime's default activation brings the spawned
app to foreground (same as a Start Menu click), which violates
cua-driver's "background launch, never steal focus" invariant. We
snapshot `GetForegroundWindow` before the call and re-assert it after
in a short retry loop. Best-effort because SetForegroundWindow is
subject to Windows' foreground-lock restrictions; visual confirmation
needs a Session 1+ run.

Plus: replace eprintln traces in list_apps / installed_apps with
tracing::debug (silent by default, enable via `RUST_LOG=...=debug`).

Verified on Windows VM (Session 0):
- doctor: exit 0, all probes report
- list_apps cold: 3.7s, returns 134 apps
- launch_app {"name":"notepad"}: pid returned, ShellExecuteEx path
- launch_app {"aumid":"..."}: fail-fast with Session-0 explanation,
  no hang

* test(windows): parity examples Session-0 aware + #1545 contract

- list_apps_parity: assert the unified #1545 contract (mixed running +
  installed, new fields kind/launch_path/last_used/windows, pid==0 for
  installed-not-running). Skip the active-app assertion in Session 0.
  Bump response timeout 4s -> 8s to absorb WinRT cold-enumeration.
- list_windows_parity: skip non-empty-windows assertion in Session 0
  (EnumWindows is correctly empty there — no attached desktop).
- launch_app_parity: accept Session-0 error from UWP/aumid path the
  same way it accepts "not installed" — both are valid skip signals.
- overlay_dump.rs: fix PrintWindow + PRINT_WINDOW_FLAGS import (moved
  to Win32::Storage::Xps in windows-rs 0.58, was failing to compile).

Verified in Session 0 on Windows VM. Pass rate: 11/11 of the tests
that don't require a foreground window owner (the remaining ones
naturally require Session 1+ to exercise meaningfully).

* feat(serve): Session-0 warning banner on Windows daemon startup

Mirrors the warning `cua-driver doctor` already surfaces for the same
condition. When the daemon starts inside Session 0 (services /
SSH-launched processes), every window-driving tool — click, type_text,
screenshot, get_window_state, list_windows, and UWP launches — will
fail or return empty. Surfacing this at daemon startup saves users
hours of debugging tools that are working as designed.

Quiet in Session 1+ (the normal interactive logon case).

* docs(parity): update list_apps + launch_app Windows status with live evidence

list_apps: bump windows status from 'VERIFIED for Win32 path, live-run
pending' to 'VERIFIED live' with the actual measurements from the
Win11 24H2 VM run (~370ms cold for 150 apps, sub-100ms warm).

launch_app: distinguish the Win32 path (fully verified, SW_SHOWNOACTIVATE
+ no focus steal) from the UWP path (requires interactive session — now
fast-fails in Session 0 instead of hanging; foreground-restore is
best-effort due to Windows' foreground-lock restrictions and needs
visual confirmation from a Session 1+ run).

* feat(installed_apps): filter opaque-system-family UWP packages from list_apps

These are packages whose Package.DisplayName is empty / unresolved
ms-resource: token AND have no Properties/DisplayName in their
AppxManifest.xml, so they fall through #1547's normalize cascade and
end up reporting their FamilyName as the user-facing name. On a stock
Win11 image that's ~10-30 entries like:

  - 1527c705-839a-4832-9118-54d4Bd6a0c89_cw5n1h2txyewy
  - F46D4000-FD22-4DB4-AC8E-4E1DDDE828FE_cw5n1h2txyewy
  - Windows.Internal.ShellExperience_cw5n1h2txyewy

They're noise in list_apps output: agents can't usefully launch them
(typically unactivatable system Components) and they crowd out real
installed apps. Filter them out when:

  display_name == family_name
    AND (family looks GUID-prefixed OR starts with Windows.Internal)

Real user-facing apps survive: Microsoft.WindowsNotepad_..., Spotify...,
.MicrosoftEdge.Stable_..., etc.

Also filter out Package.IsFramework() = true entries — those are
shared libraries (.NET, Visual C++ runtime, language packs) that
list_apps callers should never see in the list of launchable apps.

Plus: unit tests for the family-name classifier + a dev-loop
reference doc (DEV_LOOP_WINDOWS_VM.md) capturing all the PowerShell
gotchas the autonomous session hit + a session status report
(WINDOWS_SESSION0_STATUS.md) documenting what was fixed, what needs
visual confirmation, and the one-liner to bring the local fixes into
a PR when ready.

* fix(installed_apps): drop Package.IsFramework() probe — hangs in Session 0

The IsFramework() WinRT call observed to hang indefinitely (~0 CPU,
no progress) when scan_uwp_packages runs in Session 0 / services
context. Removing it; the opaque-family-name filter below still
catches most of the entries this would have removed (Visual C++
runtime, .NET shared, language packs all tend to have ms-resource:
display names that normalize to family names).

Unit tests pass in either case because they exercise the pure-string
classifier, not the WinRT path.

* fix(launch_uwp): claim 'last input' before SetForegroundWindow restore

Windows restricts SetForegroundWindow to processes that meet specific
conditions; the daemon meets none of them when launch_app is invoked
via MCP from a backgrounded UI. Without a workaround the
foreground-restore added previously was almost always silently
dropped, leaving the freshly-activated UWP app stealing focus from
whatever the human was doing.

Inject a single keybd_event of VK_NONAME (0xFC, reserved no-name
virtual key) just before the restore loop — that's enough to give us
"owner of last input" status, one of the conditions that lifts the
foreground lock. VK_NONAME doesn't map to any UI action so no app
sees a keystroke.

The Session-0 short-circuit higher up bypasses this entire path, so
this code only executes on interactive logons where focus matters in
the first place. Visual confirmation needs a Session 1+ run.

* docs(status): update Windows Session-0 status report with final fix list

All 7 commits accounted for, including the keybd_event upgrade to
the UWP foreground-restore (much stronger than the initial best-effort
because it claims 'owner of last input' before SetForegroundWindow,
lifting Windows' foreground-lock restriction in practice).

Also documents the IsFramework hang as a found-and-fixed surprise so
future debug sessions know to avoid that WinRT call in Session 0.

* docs(status): add good-morning TL;DR with merge one-liner and visual-validation pointer

* docs(faq): add Windows Session-0 troubleshooting entries

Two new entries under a new 'Windows (cua-driver-rs)' section:
- Why click/type_text/screenshot/list_windows return empty (Session-0
  detection + doctor surfacing) — explains the WindowStation+Desktop
  scoping that catches users debugging non-bugs.
- Why launch_app of a UWP app fails fast in Session-0 — points at the
  Win32 fallback path that works in services context.

Mirrors the in-binary doctor warning and serve startup banner so the
docs and the runtime tell the same story.

* feat(install.ps1)+docs: auto-start cua-driver serve at logon via Scheduled Task

The macOS install.sh registers a LaunchAgent so cua-driver-rs is up
and serving without the user pasting an env-var + path one-liner
every login. The Windows-native equivalent is a Scheduled Task
triggered at logon with `LogonType: Interactive` so it lands in a
Session 1+ user context (Session 0 would defeat the entire point —
window-driving tools need an attached interactive desktop).

This commit doesn't auto-register the task (that's a bigger choice
to make — bake it in with a flag, or leave opt-in). Instead:

1. install.ps1 prints the exact registration command at the end of a
   successful install, including the binary path it resolved to.
   Copy-paste once, done forever.
2. docs/installation.mdx gets a new sub-section under
   "Windows: interactive-session requirements" with the same recipe,
   the schtasks follow-up commands (Run / Query / Delete), and the
   two load-bearing parameter notes (LogonType=Interactive +
   ExecutionTimeLimit=0).

Validated live: registered the task on the test Windows VM, signed
out + back in via RDP, `cua-driver serve` was already listening when
the shell opened. Same UX as macOS.

* feat(install): -AutoStart flag + install-local.{ps1,sh} mirroring Swift

Three additions to bring cua-driver-rs install UX closer to the Swift
cua-driver pattern + the macOS LaunchAgent ergonomics on Windows:

1. install.ps1 -AutoStart (default $false)
   When set, registers the cua-driver-serve Scheduled Task at the
   end of install (LogonType=Interactive, lands in Session 1+). The
   factored helper Register-CuaDriverAutostart is also called from
   install-local.ps1 so both scripts share the same behavior. When
   the flag is OFF the post-install message still prints the manual
   recipe so users can opt in later.

2. install-local.ps1 (new) — Windows dev-loop installer
   Builds from the current source via cargo build [--release]
   -p cua-driver, drops the resulting cua-driver.exe into the same
   versioned-dirs + junction layout install.ps1 produces, retargets
   the `current` junction at the new dir. Accepts -Release and
   -AutoStart. Version tag carries '-local-debug' / '-local-release'
   so a local install can coexist with a release install in the
   same packages/releases tree and the user can flip `current`
   between them.

3. install-local.sh (new) — macOS + Linux dev-loop installer
   Mirrors install-local.ps1 in shape. Builds via cargo, stages into
   $HOME/.cua-driver-rs/packages/releases/0.0.0-local-<config>-<target>,
   swaps the `current` symlink. --autostart writes either a
   LaunchAgent plist (macOS) or a systemd --user unit (Linux) and
   loads/enables it. Both pointed at the visible-bin symlink so
   future install.sh upgrades take effect transparently.

Cross-platform shape parity with libs/cua-driver/scripts/install-local.sh
(Swift): same flag names where the OS allows (--release, --autostart vs
the Swift --release, --daemon), same prerequisite checks, same
"don't run with sudo" guard.

Out of scope for this commit: a `cua-driver autostart {enable|disable|
status}` CLI verb that would dedupe the OS-specific logic and let users
manage autostart without going back to the install script. Worth a
follow-up; design is straightforward (per-platform impl in
cua-driver-rs/crates/cua-driver/src/autostart/{macos,linux,windows}.rs
+ a dispatcher subcommand in cli.rs).

* fix(install-local.ps1): drop broken dot-source attempt, just inline helpers

* docs: sanitize VM hostname/IP/user/key/paths from runbook + status report

Replace concrete values (VM hostname, public IP, OS username, SSH key
file basename, local Mac filesystem paths) with placeholder tokens
(<vm-name>, <vm-host>, <user>, <ssh-key>, <repo-root>,
<rdp-file-dir>). Addresses CodeRabbit Major comments on PR #1548.

Content unchanged; same recipes, same examples, just with stub
identifiers a reader fills in from their own setup.
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