feat(agent)!: align skill install with the Agent Skills standard#271
Open
powxenv wants to merge 3 commits into
Open
feat(agent)!: align skill install with the Agent Skills standard#271powxenv wants to merge 3 commits into
powxenv wants to merge 3 commits into
Conversation
Replace the per-agent bespoke install formats with the Agent Skills open standard: one canonical .agents/skills/<skill>/SKILL.md per project, read directly by universal agents (Codex, Cursor, Cline, Gemini CLI, Copilot, …) and via a relative symlink by every other agent. The bundled registry grows from 8 hand-wired targets to the full standard set (72 agent ids). BREAKING CHANGE: agent install no longer writes per-agent bespoke formats. Codex no longer receives a managed section in AGENTS.md and Windsurf no longer receives a size-capped compact body — both now read the canonical .agents/skills/. The install result schema gains a `mode` (canonical | symlink) field and `path` reflects the new model. Skills written in the old format report stale/modified under `agent status` until refreshed (re-run `agent install`, add --force where needed). The legacy aliases (claude, kiro, copilot) and the claude-code default are preserved, so existing commands keep working.
WalkthroughChangesThe PR adopts canonical Agent Skills files under Canonical skills and target registry
Canonical installation and status flow
Setup defaults and command integration
Skill detection and doctor wiring
Documentation and contribution policy
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AgentInstall
participant CanonicalSkills
participant AgentLanding
User->>AgentInstall: agent install --target <id>
AgentInstall->>CanonicalSkills: write or validate canonical SKILL.md
AgentInstall->>AgentLanding: link target or copy fallback
AgentLanding-->>User: install action and path
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
✅ This PR is linked to an issue assigned to @powxenv — thanks! The |
github-actions
Bot
added
the
needs-issue
2 tasks
…ehavior - Revise CONTRIBUTING.md to describe universal vs symlinked agent targets and requirements for adding new targets - Update DOCUMENTATION.md to clarify agent install command usage and differentiate universal and symlinked agents - Expand README.md with a detailed supported agents table, including canonical ids, aliases, and skills folder locations - Update src/lib/agent-targets.ts with added agents, corrections to skillsDir and universal flags, and alias mappings - Modify unit tests and e2e tests to reflect renamed and updated agent targets, replacing deprecated aliases like gemini-cli with antigravity-cli and windsurf with devin-desktop - Ensure single source of truth for skills at `.agents/skills`, symlink strategy for non-universal agents, and consistency across docs and code
… chore/agentskills-standard
github-actions
Bot
removed
the
needs-issue
powxenv
marked this pull request as ready for review
July 24, 2026 12:09
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/agent-targets.test.ts (1)
131-165: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCoverage doesn't include adversarial alias tokens.
Tests confirm
resolveTargetrejects a plain unknown token (line 156-158), but don't cover prototype-chain keys (constructor,__proto__,toString) that expose the bug flagged inagent-targets.ts(resolveTarget, lines 168-172). Once that's fixed, add a case here.🤖 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 `@src/lib/agent-targets.test.ts` around lines 131 - 165, Add adversarial-token coverage to the resolveTarget tests, specifically asserting that constructor, __proto__, and toString are rejected as unknown tokens. Extend the existing “resolveTarget rejects an unknown token” test in the TARGET_ALIASES + resolveTarget suite, preserving the current null result expectation.
🤖 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 `@DOCUMENTATION.md`:
- Around line 111-118: Update the agent install examples in the documentation to
pass each agent name through the documented --target option instead of as a
positional argument. Preserve the existing install, list, and status examples
unchanged in purpose.
In `@src/commands/agent.ts`:
- Around line 714-728: Update runStatus around the TARGETS and DEFAULT_SKILLS
iteration to emit universal ok rows only for targets explicitly requested by the
user or targets with an installed actionable artifact. Preserve rows for
actionable states, while suppressing ok-only universal results for uninstalled,
unspecified targets.
In `@src/lib/agent-targets.ts`:
- Around line 168-172: Update resolveTarget’s alias lookup to verify raw is an
own key of TARGET_ALIASES before reading its value, returning null for inherited
keys such as constructor, __proto__, and toString. Preserve the existing TARGETS
lookup and canonical alias behavior, and add regression coverage in
agent-targets.test.ts for these tokens to ensure unknown-target validation is
reached instead of a crash.
---
Nitpick comments:
In `@src/lib/agent-targets.test.ts`:
- Around line 131-165: Add adversarial-token coverage to the resolveTarget
tests, specifically asserting that constructor, __proto__, and toString are
rejected as unknown tokens. Extend the existing “resolveTarget rejects an
unknown token” test in the TARGET_ALIASES + resolveTarget suite, preserving the
current null result expectation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 50a2a944-c182-480c-bf90-a91e921b1d6f
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.jsontest/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (18)
CONTRIBUTING.mdDOCUMENTATION.mdREADME.mdskills/testsprite-onboard.skill.mdskills/testsprite-verify.codex.mdskills/testsprite-verify.skill.mdsrc/commands/agent.test.tssrc/commands/agent.tssrc/commands/doctor.tssrc/commands/init.test.tssrc/commands/init.tssrc/lib/agent-targets.test.tssrc/lib/agent-targets.tssrc/lib/skill-nudge.test.tssrc/lib/skill-nudge.tstest/e2e/agent-install.e2e.test.tstest/e2e/setup.e2e.test.tstest/e2e/skill-nudge.e2e.test.ts
💤 Files with no reviewable changes (2)
- skills/testsprite-verify.codex.md
- src/commands/doctor.ts
Comment on lines
111
to
118
| ```bash | ||
| testsprite agent install claude # install the skill for Claude Code | ||
| testsprite agent install codex # install into AGENTS.md for Codex (managed-section) | ||
| testsprite agent install cursor # .cursor/rules/testsprite-verify.mdc | ||
| testsprite agent install cline # .clinerules/testsprite-verify.md | ||
| testsprite agent install windsurf # .windsurf/rules/testsprite-verify.md | ||
| testsprite agent install antigravity # .agents/skills/testsprite-verify/SKILL.md | ||
| testsprite agent install kiro # .kiro/skills/testsprite-verify/SKILL.md | ||
| testsprite agent install copilot # .github/instructions/testsprite-verify.instructions.md | ||
| testsprite agent list # list all 8 targets with status + mode + path | ||
| testsprite agent status # check installed skills against this CLI version | ||
| testsprite agent install claude-code | ||
| testsprite agent install codex | ||
| testsprite agent install cursor | ||
| testsprite agent install kiro-cli | ||
| testsprite agent list | ||
| testsprite agent status | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the documented --target option in install examples.
agent install accepts targets through --target; the supplied CLI definition does not declare a positional argument. These examples currently fail with an unexpected argument error.
Proposed fix
-testsprite agent install claude-code
-testsprite agent install codex
-testsprite agent install cursor
-testsprite agent install kiro-cli
+testsprite agent install --target claude-code
+testsprite agent install --target codex
+testsprite agent install --target cursor
+testsprite agent install --target kiro-cli📝 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
| ```bash | |
| testsprite agent install claude # install the skill for Claude Code | |
| testsprite agent install codex # install into AGENTS.md for Codex (managed-section) | |
| testsprite agent install cursor # .cursor/rules/testsprite-verify.mdc | |
| testsprite agent install cline # .clinerules/testsprite-verify.md | |
| testsprite agent install windsurf # .windsurf/rules/testsprite-verify.md | |
| testsprite agent install antigravity # .agents/skills/testsprite-verify/SKILL.md | |
| testsprite agent install kiro # .kiro/skills/testsprite-verify/SKILL.md | |
| testsprite agent install copilot # .github/instructions/testsprite-verify.instructions.md | |
| testsprite agent list # list all 8 targets with status + mode + path | |
| testsprite agent status # check installed skills against this CLI version | |
| testsprite agent install claude-code | |
| testsprite agent install codex | |
| testsprite agent install cursor | |
| testsprite agent install kiro-cli | |
| testsprite agent list | |
| testsprite agent status | |
| ``` |
🤖 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 `@DOCUMENTATION.md` around lines 111 - 118, Update the agent install examples
in the documentation to pass each agent name through the documented --target
option instead of as a positional argument. Preserve the existing install, list,
and status examples unchanged in purpose.
Comment on lines
+714
to
728
| for (const target of Object.keys(TARGETS) as AgentTarget[]) { | ||
| const spec = TARGETS[target]!; | ||
| for (const skill of DEFAULT_SKILLS) { | ||
| const relPath = pathFor(target, skill); | ||
| results.push({ | ||
| target, | ||
| skill, | ||
| path: relPath, | ||
| state: await classifyOwnFileState( | ||
| agentFs, | ||
| path.resolve(root, relPath), | ||
| target, | ||
| skill, | ||
| bodyForSkill, | ||
| ), | ||
| }); | ||
| const state = spec.universal | ||
| ? await classifySkillFile( | ||
| agentFs, | ||
| path.resolve(root, canonicalSkillFile(skill)), | ||
| skill, | ||
| contentForSkill, | ||
| ) | ||
| : await classifySymlinked(agentFs, root, target, skill, contentForSkill); | ||
| if (state === 'absent') continue; | ||
| results.push({ target, skill, path: pathFor(target, skill), state }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'universal' src/lib/agent-targets.ts | head -50
rg -nP "runStatus" -C15 src/commands/agent.tsRepository: TestSprite/testsprite-cli
Length of output: 8028
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Relevant agent-status implementation"
sed -n '637,750p' src/commands/agent.ts
echo
echo "## Agent target count and universal entries"
python3 - <<'PY'
from pathlib import Path
import re
text = Path('src/lib/agent-targets.ts').read_text()
entries = re.findall(r"([A-Za-z0-9-]+):\s*{\s*displayName:\s*'[A-Za-z0-9\s]+',\s*skillsDir:\s*[^,}]+,\s*universal:\s*(true|false)\s*},", text)
targets = [name for name, _ in entries]
universal = [name for name, flag in entries if flag == 'true']
print("targets:", len(targets), universal)
print("universal_count:", len(universal))
PY
echo
echo "## Search for runStatus result filtering/logging"
sed -n '748,798p' src/commands/agent.tsRepository: TestSprite/testsprite-cli
Length of output: 6601
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## locate install/status tests and agent command fixtures"
git ls-files | rg -n 'test|spec|jest|vitest|__tests__|agent|status|install' | sed -n '1,200p'
echo
echo "## focused search for agent status/install expectations"
rg -n "agent status|agent-install|install.*codex|runStatus|no TestSprite skill artifacts installed|Reports only agents that have an install|universal" -S --glob '*test*' --glob '!node_modules' --glob '!dist' --glob '!build' . | sed -n '1,240p'Repository: TestSprite/testsprite-cli
Length of output: 8678
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## src/commands/agent.test.ts runStatus tests"
sed -n '689,900p' src/commands/agent.test.ts | cat -n | sed -n '1,220p'
echo
echo "## relevant e2e/status expectations"
sed -n '1,120p' test/e2e/agent-install.e2e.test.ts
sed -n '316,360p' test/e2e/agent-install.e2e.test.ts
echo
echo "## deterministic status behavior from source: install order and target loops"
sed -n '450,535p' src/commands/agent.ts
sed -n '544,625p' src/commands/agent.ts
sed -n '220,260p' src/commands/agent.tsRepository: TestSprite/testsprite-cli
Length of output: 22347
Filter status output to installed artifacts only. After a single --target=codex install, runStatus currently prints 2 universal ok rows for every universal target in TARGETS, while the agent status contract says it reports only agents that have an install. Keep target rows that are actionable, but suppress ok-only universal rows that weren’t user-specified.
🤖 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 `@src/commands/agent.ts` around lines 714 - 728, Update runStatus around the
TARGETS and DEFAULT_SKILLS iteration to emit universal ok rows only for targets
explicitly requested by the user or targets with an installed actionable
artifact. Preserve rows for actionable states, while suppressing ok-only
universal results for uninstalled, unspecified targets.
Comment on lines
+168
to
172
| /** Resolve a `--target` token (id or alias) to a canonical id, or null if unknown. */ | ||
| export function resolveTarget(raw: string): AgentTarget | null { | ||
| if (Object.prototype.hasOwnProperty.call(TARGETS, raw)) return raw as AgentTarget; | ||
| return TARGET_ALIASES[raw] ?? null; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
resolveTarget alias lookup is vulnerable to prototype-chain keys.
The TARGETS branch correctly guards with hasOwnProperty, but the alias branch falls back to a plain bracket lookup: TARGET_ALIASES[raw] ?? null. Since TARGET_ALIASES is a plain object literal, tokens like constructor, __proto__, or toString resolve to inherited, truthy values (e.g. the Object constructor) instead of undefined, so ?? null never triggers. That non-string "target" then gets pushed into targets/dedupedTargets in runInstall, and TARGETS[t]!.universal (agent.ts) throws on undefined instead of surfacing the documented "unknown target" validation error — a crash instead of the intended exit-code path.
As per path instructions, src/** requires "Correctness and clear error handling" and error paths that "map to the documented exit code" — this bug produces an unhandled crash instead of the validation error for those tokens.
🐛 Proposed fix
export function resolveTarget(raw: string): AgentTarget | null {
if (Object.prototype.hasOwnProperty.call(TARGETS, raw)) return raw as AgentTarget;
- return TARGET_ALIASES[raw] ?? null;
+ return Object.prototype.hasOwnProperty.call(TARGET_ALIASES, raw) ? TARGET_ALIASES[raw]! : null;
}Consider adding a regression test in agent-targets.test.ts for tokens like 'constructor'/'__proto__'.
📝 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
| /** Resolve a `--target` token (id or alias) to a canonical id, or null if unknown. */ | |
| export function resolveTarget(raw: string): AgentTarget | null { | |
| if (Object.prototype.hasOwnProperty.call(TARGETS, raw)) return raw as AgentTarget; | |
| return TARGET_ALIASES[raw] ?? null; | |
| } | |
| /** Resolve a `--target` token (id or alias) to a canonical id, or null if unknown. */ | |
| export function resolveTarget(raw: string): AgentTarget | null { | |
| if (Object.prototype.hasOwnProperty.call(TARGETS, raw)) return raw as AgentTarget; | |
| return Object.prototype.hasOwnProperty.call(TARGET_ALIASES, raw) ? TARGET_ALIASES[raw]! : null; | |
| } |
🤖 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 `@src/lib/agent-targets.ts` around lines 168 - 172, Update resolveTarget’s
alias lookup to verify raw is an own key of TARGET_ALIASES before reading its
value, returning null for inherited keys such as constructor, __proto__, and
toString. Preserve the existing TARGETS lookup and canonical alias behavior, and
add regression coverage in agent-targets.test.ts for these tokens to ensure
unknown-target validation is reached instead of a crash.
Source: Path instructions
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.
What does this PR do?
Aligns
agent installwith the Agent Skills open standard. Instead of hand-wiring a few agents with bespoke formats, the CLI now writes one canonical skill per project at.agents/skills/<skill>/SKILL.md(the standard's shared directory), and each agent reads it directly (universal agents) or via a symlink back to it. The bundled registry grows from 8 targets to the full standard set (72 agent ids), so any skills-compatible agent works out of the box.This broadens adoption (works with 72 agents instead of 8), simplifies maintenance (one uniform code path instead of per-agent
wrap()/mode/compact-body machinery), and follows a vendor-neutral standard already adopted by the agents users run.Related issue
Closes #270
Type of change
Checklist
mainbranch.(
feat(...),fix(...),docs(...), …).npm run lintandnpm run format:checkpass.npm run typecheckpasses.npm testpasses and coverage stays at or above the 80% gate.credentials required).
README.md/DOCUMENTATION.mdwhererelevant.
Notes for reviewers
Breaking changes
This restructures the install model, so existing behavior changes for current users:
AGENTS.md; Windsurf no longer receives a size-capped compact body. Both now read the canonical.agents/skills/(codex directly, windsurf via symlink), like every other agent. Thewrap()/ own-file / managed-section machinery is gone in favor of one standardSKILL.md.agent installresults carry a newmode(canonical|symlink) field, andpathnow reflects the canonical-or-symlink model rather than each agent's bespoke location.stale/modifiedunderagent statusuntil refreshed — re-runningagent install(with--forcewhere needed) brings them in line.What's preserved: the
claude/kiro/copilotaliases still resolve, andagent installwith no--targetstill defaults toclaude-code— so the commands existing scripts use keep working; only the on-disk layout and output change.Implementation notes
.agents/skills/<skill>/SKILL.mdis the single source of truth. Universal agents (Codex, Cursor, Cline, Gemini CLI, Copilot, …) read it directly; every other agent gets a relative symlink from its own skills folder back to it — no drift between agents.lstatand refuse to traverse a planted symlink (exit 5), so a malicious symlink can't redirect a write outside--dir.agent.tsis ~92% lines, with the remainder being defensive error handling and real-fswrappers exercised by the e2e suite.Summary by CodeRabbit
New Features
.agents/skillslocation, with symlink or copy fallback support.claude-codeand documents API-key verification and CI usage.Documentation