feat(init): add shared agents skills target - #1303
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR enables ChangesGeneric agents installer target
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OpenSpecCLI
participant AIToolsConfig
participant ProjectFilesystem
User->>OpenSpecCLI: Run init --tools agents
OpenSpecCLI->>AIToolsConfig: Resolve agents target
AIToolsConfig-->>OpenSpecCLI: .agents skills path, no command adapter
OpenSpecCLI->>ProjectFilesystem: Write .agents/skills/openspec-explore/SKILL.md
OpenSpecCLI-->>User: Report skipped commands for agents
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
f247b43 to
de4b0ba
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/commands/artifact-workflow.test.ts (1)
1131-1139: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the skills-only contract in this regression test.
This test would still pass if
agentsalso generated command files. Assert the documentedCommands skipped for: agents (no adapter)output and/or the absence of generated command files.🤖 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 `@test/commands/artifact-workflow.test.ts` around lines 1131 - 1139, Update the “creates skills for the shared agents target” test to assert the skills-only contract: verify the CLI output includes “Commands skipped for: agents (no adapter)” and confirm no command files are generated for the agents target, while preserving the existing skill-file and successful-exit assertions.
🤖 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 `@docs/supported-tools.md`:
- Around line 114-118: Update the “Your tool has its own row above” guidance in
the supported-tools table to say that selecting the tool’s own ID provides its
integration, with generated slash commands only where supported; do not promise
command files for every tool-specific target.
In `@openspec/changes/add-init-agents-target/specs/ai-tool-paths/spec.md`:
- Around line 19-22: Update the “Windsurf paths defined” scenario to reflect the
alias contract: describe lookup of the windsurf alias resolving to devin and
require skillsDir to be .devin, unless a dedicated Windsurf tool target is
intentionally restored as the primary configuration.
---
Nitpick comments:
In `@test/commands/artifact-workflow.test.ts`:
- Around line 1131-1139: Update the “creates skills for the shared agents
target” test to assert the skills-only contract: verify the CLI output includes
“Commands skipped for: agents (no adapter)” and confirm no command files are
generated for the agents target, while preserving the existing skill-file and
successful-exit assertions.
🪄 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: 3c899815-a6ac-40ae-b8b1-7ea23fa57612
📒 Files selected for processing (14)
.changeset/add-agents-tool.mddocs/cli.mddocs/commands.mddocs/how-commands-work.mddocs/supported-tools.mddocs/troubleshooting.mdopenspec/changes/add-init-agents-target/.openspec.yamlopenspec/changes/add-init-agents-target/proposal.mdopenspec/changes/add-init-agents-target/specs/ai-tool-paths/spec.mdopenspec/changes/add-init-agents-target/specs/cli-init/spec.mdopenspec/changes/add-init-agents-target/tasks.mdsrc/core/config.tstest/cli-e2e/basic.test.tstest/commands/artifact-workflow.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- openspec/changes/add-init-agents-target/.openspec.yaml
- openspec/changes/add-init-agents-target/tasks.md
- test/cli-e2e/basic.test.ts
- openspec/changes/add-init-agents-target/proposal.md
| #### Scenario: Windsurf paths defined | ||
|
|
||
| - **WHEN** looking up the `windsurf` tool | ||
| - **THEN** `skillsDir` SHALL be `.windsurf` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align the Windsurf scenario with the alias contract.
src/core/config.ts no longer registers windsurf as a tool; TOOL_ID_ALIASES resolves it to devin, whose primary skills directory is .devin. Update this scenario to describe the windsurf alias and .devin path, or retain a dedicated Windsurf target if .windsurf must remain primary.
🤖 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 `@openspec/changes/add-init-agents-target/specs/ai-tool-paths/spec.md` around
lines 19 - 22, Update the “Windsurf paths defined” scenario to reflect the alias
contract: describe lookup of the windsurf alias resolving to devin and require
skillsDir to be .devin, unless a dedicated Windsurf tool target is intentionally
restored as the primary configuration.
de4b0ba to
d59fd4d
Compare
|
Addressed the CodeRabbit review:
|
d2fca19 to
c8500a9
Compare
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 `@docs/troubleshooting.md`:
- Line 62: Update the `.agents` entry in the “Confirm your tool supports command
files” guidance to avoid prescribing `/openspec-propose`; direct users to the
consuming tool’s documented skill invocation syntax instead. Keep the
vendor-specific invocation examples for Codex, Kimi Code, the other named tools,
and Amazon Q unchanged.
🪄 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: fb3a5621-16bc-4ece-81d5-0a8d6c5c5fb8
📒 Files selected for processing (18)
.changeset/add-agents-tool.mddocs/cli.mddocs/commands.mddocs/how-commands-work.mddocs/supported-tools.mddocs/troubleshooting.mdopenspec/changes/add-init-agents-target/.openspec.yamlopenspec/changes/add-init-agents-target/proposal.mdopenspec/changes/add-init-agents-target/specs/ai-tool-paths/spec.mdopenspec/changes/add-init-agents-target/specs/cli-init/spec.mdopenspec/changes/add-init-agents-target/tasks.mdsrc/core/config.tstest/cli-e2e/basic.test.tstest/commands/artifact-workflow.test.tstest/core/available-tools.test.tstest/core/init.test.tstest/core/shared/tool-detection.test.tstest/core/update.test.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- openspec/changes/add-init-agents-target/.openspec.yaml
- docs/commands.md
- openspec/changes/add-init-agents-target/specs/cli-init/spec.md
- test/cli-e2e/basic.test.ts
- test/commands/artifact-workflow.test.ts
- .changeset/add-agents-tool.md
- openspec/changes/add-init-agents-target/proposal.md
- openspec/changes/add-init-agents-target/specs/ai-tool-paths/spec.md
- openspec/changes/add-init-agents-target/tasks.md
- docs/how-commands-work.md
- docs/cli.md
- test/core/init.test.ts
- src/core/config.ts
c8500a9 to
5c24665
Compare
|
Addressed the latest CodeRabbit note. Valid, fixed. I kept |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/commands.md`:
- Line 676: Update the skills-only row in the command support table so the
shared `.agents` target is not presented as universally supporting `/openspec-*`
slash commands. Qualify command availability as assistant-dependent or direct
readers to each assistant’s skill-invocation documentation, while preserving the
existing vendor entries.
In `@test/core/init.test.ts`:
- Line 223: Remove the duplicate logCalls const declaration in the final test
callback, keeping one shared declaration for the existing assertions and
preserving its current computation.
🪄 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: 03aaf90c-66e3-413f-87a4-c250f265af5f
📒 Files selected for processing (18)
.changeset/add-agents-tool.mddocs/cli.mddocs/commands.mddocs/how-commands-work.mddocs/supported-tools.mddocs/troubleshooting.mdopenspec/changes/add-init-agents-target/.openspec.yamlopenspec/changes/add-init-agents-target/proposal.mdopenspec/changes/add-init-agents-target/specs/ai-tool-paths/spec.mdopenspec/changes/add-init-agents-target/specs/cli-init/spec.mdopenspec/changes/add-init-agents-target/tasks.mdsrc/core/config.tstest/cli-e2e/basic.test.tstest/commands/artifact-workflow.test.tstest/core/available-tools.test.tstest/core/init.test.tstest/core/shared/tool-detection.test.tstest/core/update.test.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- openspec/changes/add-init-agents-target/.openspec.yaml
- test/core/available-tools.test.ts
- test/core/shared/tool-detection.test.ts
- .changeset/add-agents-tool.md
- openspec/changes/add-init-agents-target/specs/cli-init/spec.md
- docs/cli.md
- openspec/changes/add-init-agents-target/tasks.md
- test/cli-e2e/basic.test.ts
- openspec/changes/add-init-agents-target/proposal.md
- test/commands/artifact-workflow.test.ts
- docs/troubleshooting.md
- src/core/config.ts
- docs/how-commands-work.md
| | `.../commands/opsx/<id>.*` | `/opsx:propose`, `/opsx:apply` | Claude Code, Gemini CLI, Crush | | ||
| | `.../opsx-<id>.*` | `/opsx-propose`, `/opsx-apply` | Cursor, Devin Desktop, Copilot (IDE), Trae, Oh My Pi | | ||
| | none — skills only | `/openspec-propose`, `/openspec-apply-change` | CodeArts, ForgeCode, Hermes, Mistral Vibe | | ||
| | none — skills only | `/openspec-propose`, `/openspec-apply-change` | CodeArts, ForgeCode, Hermes, Mistral Vibe, shared `.agents` | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not present shared .agents as universally supporting slash commands.
Because .agents is a vendor-neutral, skills-only target without a generic command adapter, this row may imply that /openspec-* syntax works in every assistant consuming .agents. Qualify the entry as assistant-dependent, or point readers to their assistant’s skill-invocation documentation.
🤖 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/commands.md` at line 676, Update the skills-only row in the command
support table so the shared `.agents` target is not presented as universally
supporting `/openspec-*` slash commands. Qualify command availability as
assistant-dependent or direct readers to each assistant’s skill-invocation
documentation, while preserving the existing vendor entries.
| const commandsDir = path.join(testDir, '.agents', 'commands'); | ||
| expect(await directoryExists(commandsDir)).toBe(false); | ||
|
|
||
| const logCalls = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls.flat().map(String); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Remove the duplicate logCalls declaration.
The final test contains two identical const logCalls declarations in the same callback scope, causing a TypeScript compilation error.
Proposed fix
const commandsDir = path.join(testDir, '.agents', 'commands');
expect(await directoryExists(commandsDir)).toBe(false);
- const logCalls = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls.flat().map(String);
const logCalls = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls.flat().map(String);📝 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.
| const logCalls = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls.flat().map(String); | |
| const commandsDir = path.join(testDir, '.agents', 'commands'); | |
| expect(await directoryExists(commandsDir)).toBe(false); | |
| const logCalls = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls.flat().map(String); |
🤖 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 `@test/core/init.test.ts` at line 223, Remove the duplicate logCalls const
declaration in the final test callback, keeping one shared declaration for the
existing assertions and preserving its current computation.
Source: Learnings
5c24665 to
a023ba5
Compare
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
The shared target is correctly scoped to .agents/skills, stays skills-only, preserves foreign content, and integrates cleanly with init, update, detection, and --tools all. Exact head a023ba5 passed the hosted CI/security matrix plus an isolated build and 316 focused tests.
3 participants
Status
Ready for review. Rebased onto current
main(was 108 commits behind), all conflicts resolved, four adversarial reviews run and their findings fixed. Full suite green: 115 files / 3391 tests, plustsc --noEmitandeslintclean.What was missing
AI_TOOLSalready carried anagentsentry, but withavailable: falseand noskillsDir— and every real gate keys offskillsDir, notavailable. So the option was unreachable everywhere: not in the picker, not via--tools, not in detection. Users wanting the shared.agents/skillsroot had to generate for some other tool and move files by hand.What it does
openspec init --tools agents(and the interactive picker) installs the workflow skills to.agents/skills/openspec-*/SKILL.md, the vendor-neutral root modern agent tools read. It is skills-only — there is no cross-vendor slash-command format — so it follows the existing Kimi/Hermes adapterless pattern and reportsCommands skipped for: agents (no adapter).Two things beyond flipping the flag, both from review:
.agents/skills, not a bare.agents/..agents/is a generic root other frameworks create; detecting on it alone made non-interactiveinitsilently install into a directory OpenSpec does not own, pre-checkedagentsin the picker, and produced a permanentDetected new toolnag on everyupdate.Shared .agents skills. The old label said "AGENTS.md", so the CLI printedCreated: AGENTS.md (works with Amp, VS Code, …)while creating no such file — and OpenSpec in fact strips its markers out of a rootAGENTS.md. That label was harmless while the entry was unreachable; this PR is what makes it user-visible.Proof it works
Real output,
openspec init --tools agentsin an empty repo:Verified in addition:
updatereportsTools: agentsand refreshes the skills; a stalegeneratedBycorrectly triggersagents (0.1.0 → 1.7.0); an unrelated.agents/my-framework/no longer triggers detection;--tools allandantigravity(.agent) coexist without collision;delivery: commandsdegrades with a clear message; init/update are idempotent.The change bundle also archives cleanly — verified end to end, including against a simulated main-side edit. It previously could not: every requirement sat under
## MODIFIED Requirementsbut none existed in the base specs, soopenspec archiveaborted. Restating the base requirements in full fixed that but introduced a worse problem — aMODIFIEDblock is a frozen copy, and the archive guard only detects dropped scenario names, not changed scenario bodies. I reproduced it: simulating a main-side correction to an unrelated scenario,validate --strictpassed, archive reported success, and the correction was silently reverted. The deltas are now## ADDED Requirementsthat copy no base text, so a main-side edit survives.Related
.agentsinstaller option for agentic tools #1104 — the generic.agentsinstaller request. All five acceptance criteria are met, including the last one ("docs explain when to choose the generic target versus a tool-specific integration"), now covered by a new When to pick the shared.agentstarget section indocs/supported-tools.md.available:falsewith noskillsDir). It also asks for.agents/commands, which is deliberately not implemented; the rationale is posted on the issue so the reporter sees why rather than having it buried by an auto-close..agent/skillsfolder for all agents that support it #689 — proposes the singular.agent; this uses plural.agents, which is the convention that won..agent→.agents) and feat(codex): migrate .codex skills to .agents #1157 (codex.codex→.agents). Those move vendor tools into the shared root; this adds the shared root itself. Neither blocks the other.Notes / nits
.agents/commands, despite Feedback: Please implement the 'agents' tool target (.agents/skills + .agents/commands). It is registered in the tool list as available:false with no skillsDir, so AGENTS.md-based setups have to generate for another tool and move files by hand. #1480 asking. Slash-command formats are per-vendor (frontmatter, folder namespacing, invocation prefix all differ), so a generic adapter would write files no tool reliably reads. Skills-only matches Kimi and Hermes..pi,.codex, or.agentinto.agents— that is fix(antigravity): update skill and workflow paths from .agent to .agents #830 / feat(codex): migrate .codex skills to .agents #1157's scope.--tools allnow includesagents, so it creates.agents/skills/where it previously did not. This is the only behavior change existing users get for free, and it is stated in the changeset..agents/skillspresent means this project keeps agent skills in the shared location, which is what makes the target a fit — the same signal.claude/gives for Claude Code. Verified that installing alongside a foreign skill is additive and non-destructive, matching how every existing tool root behaves.successLabelinAIToolOptionis dead config — declared but read nowhere. Set correctly here anyway; cleaning it up repo-wide is out of scope.Summary by CodeRabbit
New Features
agentstarget foropenspec init/updates that installs shared “AGENTS.md-compatible” workflow skills under.agents/skills.agentsis skills-only (no command adapters generated) and uses/openspec-<skill>syntax.Bug Fixes
.agents/skillsenables the target; a bare.agentsfolder won’t select it.experimental --tool agentsflow now succeeds and refreshes shared skills.Documentation
agents(withwindsurfkept as an alias).