Skip to content

Rewrite AGENTS.md for integration architecture#2119

Open
dhilipkumars wants to merge 5 commits intogithub:mainfrom
dhilipkumars:dhilip/rewrite-agents-md-for-integration-architecture
Open

Rewrite AGENTS.md for integration architecture#2119
dhilipkumars wants to merge 5 commits intogithub:mainfrom
dhilipkumars:dhilip/rewrite-agents-md-for-integration-architecture

Conversation

@dhilipkumars
Copy link
Copy Markdown
Contributor

@dhilipkumars dhilipkumars commented Apr 8, 2026 •
edited
Loading

Description

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
tests/test_timestamp_branches.py::TestDryRun::test_dry_run_with_timestamp PASSED     [ 99%]
tests/test_timestamp_branches.py::TestDryRun::test_dry_run_with_number PASSED        [ 99%]
tests/test_timestamp_branches.py::TestDryRun::test_dry_run_no_git PASSED             [ 99%]
tests/test_timestamp_branches.py::TestPowerShellDryRun::test_ps_dry_run_outputs_name SKIPPED [ 99%]
tests/test_timestamp_branches.py::TestPowerShellDryRun::test_ps_dry_run_no_branch_created SKIPPED [ 99%]
tests/test_timestamp_branches.py::TestPowerShellDryRun::test_ps_dry_run_no_spec_dir_created SKIPPED [ 99%]
tests/test_timestamp_branches.py::TestPowerShellDryRun::test_ps_dry_run_json_includes_field SKIPPED [ 99%]
tests/test_timestamp_branches.py::TestPowerShellDryRun::test_ps_dry_run_json_absent_without_flag SKIPPED [100%]

============================ 1183 passed, 12 skipped in 12.30s =============================
  • Tested with a sample project (if applicable)

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

Copilot AI and others added 2 commits April 7, 2026 21:54
@mnriem @dhilipkumars
Rewrite AGENTS.md for integration subpackage architecture
55a3326 
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>
@mnriem @dhilipkumars
Fix JSONC comment syntax in devcontainer example
7677ffe 
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>
@dhilipkumars dhilipkumars requested a review from mnriem as a code owner April 8, 2026 01:59
Copilot AI review requested due to automatic review settings April 8, 2026 01:59
@dhilipkumars dhilipkumars marked this pull request as draft April 8, 2026 02:00
@dhilipkumars dhilipkumars mentioned this pull request Apr 8, 2026
Closed
Copilot AI reviewed Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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
mnriem requested changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@mnriem mnriem left a comment

Choose a reason for hiding this comment

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

Please address Copilot feedback. If not applicable, please explain why

@dhilipkumars
docs(AGENTS.md): address Copilot PR review comments
9ac4cb5 
- 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
@dhilipkumars dhilipkumars marked this pull request as ready for review April 9, 2026 00:51
Copilot AI review requested due to automatic review settings April 9, 2026 00:51
Copilot AI reviewed Apr 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

@dhilipkumars
docs(AGENTS.md): clarify scripts path uses Python-safe package_dir no…
e1001f5 
…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)
@dhilipkumars dhilipkumars requested a review from Copilot April 9, 2026 00:59
Copilot AI reviewed Apr 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

@dhilipkumars
docs(AGENTS.md): use <key_with_underscores> in pytest example command
347f2c1 
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)
@dhilipkumars dhilipkumars requested a review from Copilot April 9, 2026 01:11
Copilot AI reviewed Apr 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

@dhilipkumars
Copy link
Copy Markdown
Contributor Author

dhilipkumars commented Apr 9, 2026

@mnriem ready for review.

@dhilipkumars dhilipkumars changed the title Rewrite agents md for integration architecture Rewrite AGENTS.md for integration architecture Apr 9, 2026
@dhilipkumars
Copy link
Copy Markdown
Contributor Author

dhilipkumars commented Apr 9, 2026

@mnriem is this what you were looking for for this PR? and happy to pick up the next tickets for this integration enhancement i --skills as an integration option is yet to be implemented is that correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@mnriem mnriem mnriem requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dhilipkumars @mnriem