Skip to content

feat(init): add Amp skills support - #420

Merged
clay-good merged 5 commits into
Fission-AI:mainfrom
jeanduplessis:add-amp-support
Sep 29, 2026
Merged

clay-good merged 5 commits into
Fission-AI:mainfrom
jeanduplessis:add-amp-support

Conversation

@jeanduplessis

@jeanduplessis jeanduplessis commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Status

LGTM. Ready for human review; all GitHub CI and security checks pass on the refreshed branch.

What was missing / the motivation

Amp reads project Agent Skills from .agents/skills/, but OpenSpec did not list Amp in the init picker or accept --tools amp. The universal agents target worked only for users who already knew Amp's discovery path.

What it does

  • Adds Amp as a first-class, skills-only tool with the amp id.
  • Uses OpenSpec's current shared .agents/skills/ generator and ownership marker instead of maintaining Amp-specific workflow templates.
  • Detects Amp from .amp/ and refreshes Amp-owned skill trees during openspec update.
  • Documents Amp in the canonical docs-lab supported-tools reference.
  • Adds a minor changeset for the user-facing integration.

Proof it works

  • openspec validate add-amp-support --strict: passed.
  • Amp detection, init, and update coverage: passed as part of 344 targeted tests.
  • eslint src/: passed.
  • node build.js: passed.
  • Docs site production build: passed, including 70 statically generated pages.
  • Docs page checked at desktop and 390 px widths; the support matrix stays inside its horizontal-scroll container.

The full local suite reached unrelated environment-sensitive failures: this machine has a user-level MiniMax skill install that changes subprocess delivery behavior, and its terminal reports TERM=dumb, which suppresses two completion-tip assertions. The clean GitHub runners pass the full suite on Linux, macOS, and Windows.

Notes / nits

  • The issue is current: Amp's official docs specify .agents/skills/ for project skills and SKILL.md files with name and description frontmatter.
  • The original 2025 implementation no longer matched OpenSpec's architecture. This update removes the duplicate three-workflow template generator and uses the current profile-aware shared generator instead.
  • The unrelated configurator-renaming proposal was removed from this PR.
  • Security review found no dependency, install-script, workflow, executable, credential, network-call, or shell-execution changes in the final diff.

Jean du Plessis added 3 commits December 29, 2025 17:40
Add AmpSlashCommandConfigurator that generates Amp-native skill files
at .agents/skills/openspec-{proposal,apply,archive}/SKILL.md with YAML
frontmatter containing name and description fields.

- Register Amp in the native tool picker for init and update commands
- Include comprehensive test coverage for init and update scenarios
- Mark all add-amp-support tasks as complete

Amp-Thread-ID: https://ampcode.com/threads/T-019b6a90-6107-755b-8087-942d9b5460ac
Fix semantic mismatch with diverse tool terminology (skills, prompts,
commands). Old names kept as deprecated aliases for compatibility.

Amp-Thread-ID: https://ampcode.com/threads/T-019b6a90-6107-755b-8087-942d9b5460ac
@coderabbitai

coderabbitai Bot commented Dec 29, 2025 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: Fission-AI/OpenSpec/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c420799e-d141-4313-9b14-af3329c49cac

📥 Commits

Reviewing files that changed from the base of the PR and between 5ff0c92 and c11f332.

📒 Files selected for processing (10)
  • .changeset/calm-amps-work.md
  • docs-lab/reference/supported-tools.md
  • openspec/changes/add-amp-support/.openspec.yaml
  • openspec/changes/add-amp-support/proposal.md
  • openspec/changes/add-amp-support/specs/ai-tool-paths/spec.md
  • openspec/changes/add-amp-support/tasks.md
  • src/core/config.ts
  • test/core/available-tools.test.ts
  • test/core/init.test.ts
  • test/core/update.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • openspec/changes/add-amp-support/tasks.md
  • openspec/changes/add-amp-support/proposal.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

Amp is added as a skills-only tool that uses .agents/skills/. The change adds .amp/ project detection, shared-tree ownership detection, initialization and update tests, and supported-tools documentation.

Changes

Amp skills support

Layer / File(s) Summary
Register and document Amp skills support
openspec/changes/add-amp-support/*, src/core/config.ts, test/core/available-tools.test.ts, test/core/init.test.ts, test/core/update.test.ts, docs-lab/reference/supported-tools.md, .changeset/calm-amps-work.md
The OpenSpec change specifies Amp’s shared skills integration. The tool configuration and tests cover detection, initialization, and refreshing an Amp-owned skills tree. Documentation and a changeset describe Amp’s paths and invocation.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant InitCommand
  participant AI_TOOLS
  participant ProjectFiles
  participant CLIOutput
  InitCommand->>AI_TOOLS: Resolve Amp tool configuration
  InitCommand->>ProjectFiles: Write skills under .agents/skills/ and the amp target marker
  InitCommand->>CLIOutput: Report that command generation was skipped
Loading

Merge Risk: ⚪ Minimal · up to c11f3

Amp support adds skills under the shared .agents/skills directory, while preserving detection of projects owned by other tools. No material merge-blocking risk is established; the change appears ready after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c11f3

Amp support can cause unattended setup to write shared agent skills when an Amp directory is detected. Existing ownership controls limit the impact, but an unmarked shared tree may still be changed without an explicit tool selection.

Retained concerns

  • Low · security · inferred: In an Amp-detected project, unattended init can select Amp without an explicit tool flag and overwrite a same-named skill in an unmarked shared tree. If that file carried locally maintained agent instructions, those instructions can be lost. This requires the init process to run with write access; it is not an established privilege escalation.
Security review details

Security Blast Radius

  • inferred — The effective exposure is the invoking process's project-local .agents/skills tree and any tools consuming it. The reviewed path does not establish access to another tenant, service, credential store, or deployment environment.

Security Findings and Attack Paths

  • inferred — Someone able to affect project filesystem state can influence Amp detection through .amp. If a higher-trust process subsequently runs unattended init against that project, detection can lead to writes in the shared skills tree; harmful instruction loss additionally requires a conflicting, unmarked target file.

Trust Boundaries and Controls

  • observed — Explicit tool selection takes precedence over detection. Shared-skill reconciliation limits root-only detection; writer resolution favors established ownership; generation checks path containment before each skill write.

Resilience and Maintainability Implications

  • inferred — Existing sequential writes and post-write marker persistence permit partial state after failure. This is a pre-existing recovery limitation applied to a newly supported tool, not evidence that Amp bypasses the owner-selection control.

Hardening Proposals

  • proposed — Consider requiring explicit selection or checking for conflicting unmarked skills before unattended setup adopts a newly detected tool's shared root.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files. (6 skipped: 6 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding Amp skills support. It is specific and matches the pull request changes.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@vibe-kanban-cloud

Copy link
Copy Markdown

Review Complete

Your review story is ready!

View Story

Comment !reviewfast on this PR to re-generate the story.

@TabishB

TabishB commented Dec 30, 2025 •

Copy link
Copy Markdown
Contributor

@jeanduplessis Interesting, I would have thought the equivalent in AMP was the custom commands. But this is a pattern I'm seeing where some coding agents allow you to invoke skills directly vs others it's still on a more "conversational" trigger basis.

Claude Code also recently seems to have some internal instructions mapping slash commands to skills. (Can't remember the exact details)

Where it gets even trickier is when a coding agent supports both skills and slash commands...

I think the approach here is right in the sense we should be opinionated on how best to integrate OpenSpec with whatever Coding Agent (AMP in this case). I just need to figure out how to deal with the awkwardness thats emerging.

Instead of having a SlashCommand Generator (configurator/slash/*.ts)I think we'll need to instead have some sort of tool specific generator i.e. AMPConfigurator that sets it up in an Amp appropriate way.

I'll have to get back to you on this PR as I need to think through things a bit more. (There's also some changes in progress that might make this out of date very fast).

@jeanduplessis

Copy link
Copy Markdown
Contributor Author

@TabishB yeah, it seems like the agent harness authors are preferring skills as a more comprehensive solution to custom/slash commands. Invoking the skill via a slash command or not essentially becomes a UX choice. I'll wait to see what you come up with (re in progress changes, etc.) and adapt as needed.

@ppasieka

ppasieka commented Feb 2, 2026

Copy link
Copy Markdown

It would be great to have Amp supported as an agent. Until now, I was using the AGENT.md flow, but after updating to the latest version, it disappeared.

# Conflicts:
#	src/core/config.ts
#	src/core/configurators/slash/registry.ts
#	src/core/templates/index.ts
#	src/core/templates/skill-templates.ts
#	test/core/init.test.ts
#	test/core/update.test.ts
@clay-good
clay-good requested a review from a team as a code owner September 28, 2026 17:53
@clay-good clay-good changed the title Add Amp workflow support (using Agent Skills) feat(init): add Amp skills support Sep 28, 2026

@alfred-openspec alfred-openspec left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Amp project skills and workspace detection match the current vendor documentation. Shared-root ownership, skills-only generation, update behavior, release tracking, and CI are covered. The docs-lab change still requires final review from @TabishB.

@clay-good
clay-good added this pull request to the merge queue Sep 29, 2026
Merged via the queue into Fission-AI:main with commit 070de01 Sep 29, 2026
14 checks passed
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.

5 participants