Repository navigation
Conversation
A typed --agent value that is an Orca agent id (claude) now maps onto the skills CLI key (claude-code) through the existing agent-key table, the same way detected agents already do. Unknown values still pass through for the skills CLI to reject. trae stays trae because it is also the skills CLI's own TRAE key; the detected-agent mapping to trae-cn is unchanged. Fixes stablyai#20831
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe skills install handler now converts explicit Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Explicit agent IDs map to install targets without redirecting accepted CLI keys. No concrete issue blocks merging after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
ELI5
orca skills install --agent claudenow installs into Claude Code instead of failing. Orca calls Claude Codeclaudeeverywhere else, but the outsideskillstool it runs only knows the nameclaude-code. Orca already had a translation table for this. It just wasn't used for names you type yourself.What Changed
Before, the reporter of #20831 saw this:
After, the same command runs
... --global --agent claude-code -yand the install goes through.--dry-runshows the translated name too.Mechanism.
src/shared/skills-cli-agent-keys.tsalready maps Orca's agent ids to the skills CLI's names (claude→claude-code,gemini→gemini-cli,copilot→github-copilot,rovo→rovodev, and so on). Until now that map was applied only to agents Orca detects on the host. A value passed with--agentonly got a shape check and went through unchanged. A newtoRequestedSkillsCliAgentKeysends a typed value through the same map when it is an Orca agent id.resolveInstallAgentKeyscalls it on each comma-separated value before de-duplicating, soclaude,claude-codebecomes a singleclaude-code.Unchanged:
universal,*orinference-sh, pass through. The skills CLI still judges them, as the existing comment intends.aideroromp, also pass through. The CLI still rejects them loudly, as it does today.-ystill runs, now after translation.Deliberate exception:
trae. Orca maps its detectedtraeagent totrae-cn, because Orca detects it through a binary only TRAE CN ships. Buttraeis also the skills CLI's own key for TRAE international, according to the valid-key list pinned inskills-cli-agent-keys.test.ts. If Orca rewrote a typed--agent traetotrae-cn, it would silently install into the wrong product. So a typedtraekeeps its skills CLI meaning. It is the only Orca id that is also a different valid skills key. A new test checks every entry of the map against the pinned list, so a future collision fails CI instead of slipping through.Why
The issue already names the cause and the fix: the map exists and is applied a few lines away on the detection path. Reusing it keeps one source of truth instead of adding a second alias table. I considered two alternatives. Rejecting Orca ids with a "did you mean claude-code?" error would still fail the most common command. Translating every known id blindly would hijack an explicit
--agent trae.Linked Issue
Fixes #20831
Visual Proof
N/A. This is a CLI argument fix with no UI or interaction change. The before and after argv are quoted above and pinned by the tests.
Testing
The existing handler test for an explicit
--agentlist now passescodex, claude-code ,codex,claude,trae. It asserts that the spawned argv containscodex,claude-codeandtraein that order. A new shared test checks the translation of every Orca agent id against the pinned skills-CLI key list and checks that unknown values pass through. Both tests fail without the fix and pass with it. I tested on macOS only. The change is pure string mapping with no path, shell or platform branches.vitest run src/cli/skills.test.ts src/shared/skills-cli-agent-keys.test.ts src/shared/agent-feature-install-commands.test.ts: 3 files, 69 tests passed.tsc --noEmit -p config/tsconfig.tc.cli.json(typecheck:cli): clean.tsc --noEmit -p config/tsconfig.node.json(typecheck:node): clean.oxlintandoxfmt --checkon the 4 changed files: clean.AI Disclosure
Implemented with Claude (Anthropic) as a coding agent. Reviewed by OpenAI Codex (
codex exec, read-only).Review
Codex reviewed the commit read-only and reported no actionable findings. It specifically checked these points:
traeis the only conflict between the map and the pinned valid-key set. Unknown and null-mapped values pass through.*.isTuiAgentfromtui-agent-configadds no import cycle and no browser-incompatible dependency.Compatibility notes:
orca skills installstill refuses to run through a forwarded shell, and that path is untouched.--agentvalues that are Orca ids change meaning, and each maps to the key Orca already uses for that agent on detection.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.The change reuses Orca's existing agent-key map and its already-pinned list of valid skills-CLI keys. It adds no upstream material.
Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Backwards compatibility: a typed
--agentthat already worked, meaning a skills-CLI key, behaves exactly as before. The only values whose output changes are Orca ids that the skills CLI rejected outright. Mobile is not affected.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)