Rewrite agents md for integration architecture#2119
Rewrite agents md for integration architecture#2119dhilipkumars wants to merge 2 commits intogithub:mainfrom
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.
| "requires_cli": True, # True if CLI tool required, False for IDE-based agents | ||
| }, | ||
| } | ||
| Each AI agent is a self-contained **integration subpackage** under `src/specify_cli/integrations/<key>/`. The package exposes a single class that declares all metadata, inherits setup/teardown logic from a base class, and registers itself in the global `INTEGRATION_REGISTRY` at import time. |
There was a problem hiding this comment.
Line 15 says each integration "registers itself" in INTEGRATION_REGISTRY at import time. In the current implementation, integrations are registered explicitly by src/specify_cli/integrations/__init__.py:_register_builtins() calling _register(<Integration>()); individual integration modules (e.g. src/specify_cli/integrations/windsurf/__init__.py) do not self-register. Suggest rewording to describe the _register_builtins() mechanism to avoid misleading contributors.
| Each AI agent is a self-contained **integration subpackage** under `src/specify_cli/integrations/<key>/`. The package exposes a single class that declares all metadata, inherits setup/teardown logic from a base class, and registers itself in the global `INTEGRATION_REGISTRY` at import time. | |
| Each AI agent is a self-contained **integration subpackage** under `src/specify_cli/integrations/<key>/`. The subpackage exposes a single class that declares all metadata and inherits setup/teardown logic from a base class. Built-in integrations are then instantiated and added to the global `INTEGRATION_REGISTRY` by `src/specify_cli/integrations/__init__.py` via `_register_builtins()`. |
| ### 2. Create the subpackage | ||
|
|
||
| Update the `--ai` parameter help text in the `init()` command to include the new agent: | ||
| Create `src/specify_cli/integrations/<key>/__init__.py`. The `key` **must match the actual CLI tool name** (the executable users install and run). Use a Python-safe directory name if the key contains hyphens (e.g., `kiro_cli/` for key `"kiro-cli"`). |
There was a problem hiding this comment.
This section states the integration key must match the executable users install/run. That’s only required when requires_cli is true (because CLI checks use shutil.which(key)); IDE-based integrations like copilot/windsurf don’t have an executable. Suggest clarifying the rule as “CLI-based integrations should use the actual executable name as key” to avoid confusion.
| Create `src/specify_cli/integrations/<key>/__init__.py`. The `key` **must match the actual CLI tool name** (the executable users install and run). Use a Python-safe directory name if the key contains hyphens (e.g., `kiro_cli/` for key `"kiro-cli"`). | |
| Create `src/specify_cli/integrations/<key>/__init__.py`. For CLI-based integrations (`requires_cli: True`), the `key` should match the actual CLI tool name (the executable users install and run) so CLI checks can resolve it correctly. For IDE-based integrations (`requires_cli: False`), use the canonical integration identifier instead. Use a Python-safe directory name if the key contains hyphens (e.g., `kiro_cli/` for key `"kiro-cli"`). |
| # Verify files were created | ||
| ls -R my-project/<commands_dir>/ |
There was a problem hiding this comment.
ls -R my-project/<commands_dir>/ uses a placeholder that isn’t defined elsewhere in the doc. Suggest replacing it with the concrete path derived from config["folder"] + config["commands_subdir"] (or an explicit example like .windsurf/workflows/) so readers can verify the correct output location.
| # Verify files were created | |
| ls -R my-project/<commands_dir>/ | |
| # Verify files were created in the commands directory configured by | |
| # config["folder"] + config["commands_subdir"] (for example, .windsurf/workflows/) | |
| ls -R my-project/.windsurf/workflows/ |
| Each integration also has a dedicated test file at `tests/integrations/test_integration_<key>.py`. Run it with: | ||
|
|
||
| # Then you need special cases everywhere: | ||
| cli_tool = agent_key | ||
| if agent_key == "cursor": | ||
| cli_tool = "cursor-agent" # Map to the real tool name | ||
| ```bash | ||
| pytest tests/integrations/test_integration_<key>.py -v | ||
| ``` |
There was a problem hiding this comment.
The doc claims each integration has a test at tests/integrations/test_integration_<key>.py, but keys containing hyphens use underscore filenames (e.g. key cursor-agent → test_integration_cursor_agent.py, key kiro-cli → test_integration_kiro_cli.py). Suggest documenting the Python-safe filename mapping or pointing contributors to the existing per-agent test naming convention.
| ## Argument Patterns | ||
|
|
||
| Different agents use different argument placeholders: | ||
|
|
||
| - **Markdown/prompt-based**: `$ARGUMENTS` | ||
| - **TOML-based**: `{{args}}` | ||
| - **Forge-specific**: `{{parameters}}` (uses custom parameter syntax) | ||
| - **Script placeholders**: `{SCRIPT}` (replaced with actual script path) |
There was a problem hiding this comment.
Argument Patterns only lists $ARGUMENTS and {{args}}, but the repo has at least one integration with a different placeholder (forge uses {{parameters}} via registrar_config["args"] in src/specify_cli/integrations/forge/__init__.py). Suggest either adding the Forge placeholder here or noting that the placeholder is integration-specific and should be taken from registrar_config["args"].
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
Description
Testing
uv run specify --helpuv sync && uv run pytestAI Disclosure