Rewrite AGENTS.md for integration architecture#2119
Conversation
Replaces the old AGENT_CONFIG dict-based 7-step process with documentation reflecting the integration subpackage architecture shipped in github#1924. Removed: Supported Agents table, old step-by-step guide referencing AGENT_CONFIG/release scripts/case statements, Agent Categories lists, Directory Conventions section, Important Design Decisions section. Kept: About Spec Kit and Specify, Command File Formats, Argument Patterns, Devcontainer section. Added: Architecture overview, decision tree for base class selection, configure/register/scripts/test/override steps with real code examples from existing integrations (Windsurf, Gemini, Codex, Copilot). Agent-Logs-Url: https://github.com/github/spec-kit/sessions/71b25c53-7d0c-492a-9503-f40a437d5ece Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/spec-kit/sessions/71b25c53-7d0c-492a-9503-f40a437d5ece Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates AGENTS.md to document the new integration-based architecture (integration subpackages + registry) and provides a revised, step-by-step guide for adding new agent integrations.
Changes:
- Replaces the old “add a new agent via config tables/scripts” guidance with an “integration subpackage + registry” architecture overview.
- Adds concrete examples for Markdown/TOML/Skills integrations, along with registration + wrapper-script steps.
- Refreshes related guidance sections (devcontainer notes, command formats, common pitfalls) to align with the integration model.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
- Clarify that integrations are registered by _register_builtins() in
__init__.py, not self-registered at import time
- Scope the key-must-match-executable rule to CLI-based integrations
(requires_cli: True); IDE-based integrations use canonical identifiers
- Replace <commands_dir> placeholder in test snippet with a concrete
example path (.windsurf/workflows/)
- Document that hyphens in keys become underscores in test filenames
(e.g. cursor-agent -> test_integration_cursor_agent.py)
- Note that the argument placeholder is integration-specific
(registrar_config["args"]); add Forge's {{parameters}} as an example
- Apply consistency fixes to Required fields table, Key design rule
callout, and Common Pitfalls #1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t key The scripts step previously referenced src/specify_cli/integrations/<key>/scripts/ but for hyphenated keys the actual directory is underscored (e.g. kiro-cli -> kiro_cli/). Rename the placeholder to <package_dir> and add a note explaining: - <package_dir> matches <key> for non-hyphenated keys - <package_dir> uses underscores for hyphenated keys (e.g. kiro-cli -> kiro_cli/) - IntegrationBase.key always retains the original hyphenated value Addresses: github#2119 (comment)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The pytest command previously used <key> as a placeholder, but test filenames always use underscores even for hyphenated keys. This was internally inconsistent since the preceding sentence already explained the hyphen→underscore mapping. Switch to <key_with_underscores> to match the actual filename on disk. Addresses: github#2119 (comment)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@mnriem ready for review. |
dhilipkumars
changed the title
|
@mnriem is this what you were looking for for this PR? and happy to pick up the next tickets for this |
There was a problem hiding this comment.
Pull request overview
Updates AGENTS.md to document the current “integration subpackage + registry” architecture for AI assistants, replacing the older “add to AGENT_CONFIG table” guidance so contributors follow the actual implementation model in src/specify_cli/integrations/.
Changes:
- Replaces the legacy “Adding New Agent Support” guide with an “Integration Architecture” overview aligned to the
INTEGRATION_REGISTRYsystem. - Adds a step-by-step “Adding a New Integration” workflow (base class selection, package layout, registry registration, wrapper scripts, testing).
- Updates argument placeholder guidance to point readers to
registrar_config["args"](including non-$ARGUMENTSplaceholders).
Show a summary per file
| File | Description |
|---|---|
| AGENTS.md | Rewrites the contributor guide to match the integrations registry architecture and current integration implementation patterns. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
|
@dhilipkumars Please address Copilot feedback |
The path src/specify_cli/integrations/<key>/__init__.py was inaccurate for hyphenated keys (e.g. kiro-cli lives in kiro_cli/, not kiro-cli/). Rename the placeholder to <package_dir>, define it inline (hyphens become underscores), and note that IntegrationBase.key always retains the original hyphenated value. Addresses: github#2119 (comment)
There was a problem hiding this comment.
Pull request overview
Updates AGENTS.md to reflect the current “integration subpackage + registry” architecture for AI assistant support, replacing the older “add to AGENT_CONFIG” style guidance.
Changes:
- Documented the integration package layout (
src/specify_cli/integrations/<package_dir>/) and registry-based discovery (INTEGRATION_REGISTRY/_register_builtins()). - Added a step-by-step guide for adding a new integration (choose base class, create subpackage, register, add wrapper scripts, update shared context-update scripts, test).
- Refreshed guidance around argument placeholders to point readers to
registrar_config["args"](including non-$ARGUMENTSplaceholders).
Show a summary per file
| File | Description |
|---|---|
| AGENTS.md | Rewrites agent-support documentation to match the integrations subsystem (base classes, registry, scripts, and testing workflow). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
…only The registry is only authoritative for Python integration metadata. Context-update dispatcher scripts (bash + PowerShell) still require explicit per-agent cases and maintain their own supported-agent lists until they are migrated to registry-based dispatch. Tighten the claim to avoid misleading contributors into skipping the script updates. Addresses: github#2119 (review)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…step The update-agent-context.ps1 script has a [ValidateSet(...)] on the AgentType parameter. Without adding the new key to that list, the script rejects the argument before reaching Update-SpecificAgent. Add this as an explicit step alongside the switch case and Update-AllExistingAgents. Addresses: github#2119 (review)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Both the import list and the _register() call list had codex before codebuddy, violating the alphabetical ordering that AGENTS.md documents. Swap them so the file matches the documented convention. Addresses: github#2119 (review)
There was a problem hiding this comment.
Pull request overview
Updates the project’s contributor-facing documentation to reflect the current “integration subpackage + registry” architecture used by Specify CLI for AI assistants, with a small ordering tweak in the built-in integration registry.
Changes:
- Rewrites
AGENTS.mdto document integration base classes, subpackage layout, registration steps, wrapper scripts, and testing guidance. - Clarifies argument placeholder selection as integration-driven (
registrar_config["args"]). - Adjusts import/registration ordering for
CodexIntegrationin the built-ins registry to keep lists alphabetical.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/integrations/__init__.py |
Keeps built-in integration imports/registration alphabetical (moves Codex accordingly). |
AGENTS.md |
Replaces the older “adding new agent support” guide with updated integration architecture and contributor instructions. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
|
Thank you! |
|
@mnriem thanks for the review feel free to assign any new tickets, happy to pick and contribute. :-) |
Description
Testing
uv run specify --helpuv sync && uv run pytestAI Disclosure