feat(cua-driver-rs)(skills): default to host-only platform docs (--all-platforms to keep the full set) - #1686
Merged
Merged
Conversation
…l-platforms to keep the full set) The skill pack ships docs for all three platforms (WINDOWS.md / MACOS.md / LINUX.md) in one tarball but only one of them is relevant to any given host. User on Windows complained about LINUX.md sitting in their skill install. Default install now filters: WINDOWS.md kept on Windows, MACOS.md on macOS, LINUX.md on Linux. Other platforms' .md files are skipped during extraction (release tarball path) and during per-file fetch (--from main path). README, SKILL, RECORDING, WEB_APPS, TESTS are always kept. `--all-platforms` opt-in restores the previous behaviour for users who help across OSes from one machine and want every deep-dive present in their agent prompt context. Wiring: - new `excluded_platform_docs(all_platforms)` returns the basenames to skip per #[cfg(target_os = "...")] - `is_excluded_platform_doc(basename, all_platforms)` is the predicate - threaded through `fetch_into` (per-file branch + tarball branch) - extract_tar_gz now takes `all_platforms: bool` Also: added MACOS.md to SKILL_FILES — was missing from the --from main list, so macOS users on bleeding-edge dev validation were missing their own platform doc. 2 new unit tests: - extract_per_host_filter_drops_other_platform_docs - extract_all_platforms_flag_keeps_every_platform_doc 6/6 skill tests pass on Windows. End-to-end install on a Windows host produces a skill dir with WINDOWS.md only, no LINUX.md / MACOS.md.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
📝 WalkthroughWalkthrough
ChangesSkill-pack platform-scoped installation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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/rust/crates/cua-driver/src/skills.rs`:
- Around line 22-25: The module docstring in skills.rs incorrectly says "Default
install drops only the host platform's deep-dive .md" while the implementation
actually keeps the host doc and drops the non-host docs; update the top-of-file
module docs (the module-level comment in skills.rs) to state that the default
install keeps the host platform's deep-dive .md and drops the other platform .md
files (and note the --all-platforms flag keeps all three), making the wording
consistent with the implementation.
🪄 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: e3c43d59-8d71-4207-b157-86b25b1e43e4
📒 Files selected for processing (1)
libs/cua-driver/rust/crates/cua-driver/src/skills.rs
Comment on lines
+22
to
+25
| //! Default install drops only the host platform's deep-dive .md | ||
| //! (WINDOWS.md / MACOS.md / LINUX.md — whichever matches). Pass | ||
| //! `--all-platforms` to keep all three (useful when assisting users | ||
| //! across OSes from one machine). |
Contributor
There was a problem hiding this comment.
Correct the default-behavior wording in the module docs.
This says default install drops the host platform doc, but the implementation keeps the host doc and drops the non-host docs.
Suggested patch
-//! Default install drops only the host platform's deep-dive .md
+//! Default install keeps only the host platform's deep-dive .md📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| //! Default install drops only the host platform's deep-dive .md | |
| //! (WINDOWS.md / MACOS.md / LINUX.md — whichever matches). Pass | |
| //! `--all-platforms` to keep all three (useful when assisting users | |
| //! across OSes from one machine). | |
| //! Default install keeps only the host platform's deep-dive .md | |
| //! (WINDOWS.md / MACOS.md / LINUX.md — whichever matches). Pass | |
| //! `--all-platforms` to keep all three (useful when assisting users | |
| //! across OSes from one machine). |
🤖 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/rust/crates/cua-driver/src/skills.rs` around lines 22 - 25,
The module docstring in skills.rs incorrectly says "Default install drops only
the host platform's deep-dive .md" while the implementation actually keeps the
host doc and drops the non-host docs; update the top-of-file module docs (the
module-level comment in skills.rs) to state that the default install keeps the
host platform's deep-dive .md and drops the other platform .md files (and note
the --all-platforms flag keeps all three), making the wording consistent with
the implementation.
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
User on Windows reported `LINUX.md` sitting in their installed skill pack. The pack ships `WINDOWS.md`, `MACOS.md`, and `LINUX.md` all in one tarball, but a single host only needs one of those.
Default install now filters per host. README + SKILL + cross-platform docs (RECORDING, WEB_APPS, TESTS) are always kept; only the platform-specific deep-dive matches the host.
`--all-platforms` opt-in keeps the full set for users who assist across OSes from one machine.
Result on Windows
Before:
```
LINUX.md MACOS.md README.md RECORDING.md SKILL.md TESTS.md WEB_APPS.md WINDOWS.md
```
After (default `cua-driver skills install`):
```
README.md RECORDING.md SKILL.md TESTS.md WEB_APPS.md WINDOWS.md
```
After `cua-driver skills install --all-platforms`:
```
LINUX.md MACOS.md README.md RECORDING.md SKILL.md TESTS.md WEB_APPS.md WINDOWS.md
```
Bonus
Found `MACOS.md` was missing from `SKILL_FILES` — the `--from main` per-file fetch path skipped it. Added.
Tests
2 new unit tests in `skills::tests`:
6/6 skill tests pass on Windows. Smoke-tested with real `cua-driver skills install --from main --force` — produces a Windows-only doc set.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--all-platformsflag for skill-pack installation, allowing control over whether platform-specific documentation is installed for all platforms or just the host platformDocumentation
Tests