feat(vscode): add native SDD agents#708
Conversation
|
Maintainer label request: please apply exactly one type label, |
d043669 to
079c30b
Compare
|
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:
📝 WalkthroughWalkthroughVS Code Copilot sub-agent support is enabled, embedded ChangesVS Code Copilot SDD Native Sub-Agent Support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/assets/assets_test.go (1)
125-282:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTestAllEmbeddedAssetsAreReadable() does not include the 10 new VSCode agent files; list is incomplete.
The
TestAllEmbeddedAssetsAreReadable()function defines anexpectedFilesslice that enumerates every embedded asset that should be readable. This is a safety net to catch missing/misnamed files at test time. The current list (lines 126-263) does NOT include any of the 10 new VSCode native agent files (vscode/agents/*.agent.md).This test should be updated to include:
vscode/agents/sdd-orchestrator.agent.mdvscode/agents/sdd-init.agent.mdvscode/agents/sdd-explore.agent.mdvscode/agents/sdd-propose.agent.mdvscode/agents/sdd-spec.agent.mdvscode/agents/sdd-design.agent.mdvscode/agents/sdd-tasks.agent.mdvscode/agents/sdd-apply.agent.mdvscode/agents/sdd-verify.agent.mdvscode/agents/sdd-archive.agent.mdvscode/agents/sdd-onboard.agent.mdWithout this update, accidental deletion or renaming of these files would not be caught by the test.
📝 Proposed addition to expectedFiles list
expectedFiles := []string{ // Claude agent files "claude/engram-protocol.md", ...existing entries... + // VSCode native agent files + "vscode/agents/sdd-orchestrator.agent.md", + "vscode/agents/sdd-init.agent.md", + "vscode/agents/sdd-explore.agent.md", + "vscode/agents/sdd-propose.agent.md", + "vscode/agents/sdd-spec.agent.md", + "vscode/agents/sdd-design.agent.md", + "vscode/agents/sdd-tasks.agent.md", + "vscode/agents/sdd-apply.agent.md", + "vscode/agents/sdd-verify.agent.md", + "vscode/agents/sdd-archive.agent.md", + "vscode/agents/sdd-onboard.agent.md", + // Hermes agent files "hermes/sdd-orchestrator.md", ...rest of existing entries... }🤖 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 `@internal/assets/assets_test.go` around lines 125 - 282, The expectedFiles slice in the TestAllEmbeddedAssetsAreReadable function is missing entries for 11 new VSCode agent files. Add the following file paths to the expectedFiles slice: vscode/agents/sdd-orchestrator.agent.md, vscode/agents/sdd-init.agent.md, vscode/agents/sdd-explore.agent.md, vscode/agents/sdd-propose.agent.md, vscode/agents/sdd-spec.agent.md, vscode/agents/sdd-design.agent.md, vscode/agents/sdd-tasks.agent.md, vscode/agents/sdd-apply.agent.md, vscode/agents/sdd-verify.agent.md, vscode/agents/sdd-archive.agent.md, and vscode/agents/sdd-onboard.agent.md. Insert these entries as a new commented section after the cursor agent files section to maintain the organized structure of the test.
🤖 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.
Outside diff comments:
In `@internal/assets/assets_test.go`:
- Around line 125-282: The expectedFiles slice in the
TestAllEmbeddedAssetsAreReadable function is missing entries for 11 new VSCode
agent files. Add the following file paths to the expectedFiles slice:
vscode/agents/sdd-orchestrator.agent.md, vscode/agents/sdd-init.agent.md,
vscode/agents/sdd-explore.agent.md, vscode/agents/sdd-propose.agent.md,
vscode/agents/sdd-spec.agent.md, vscode/agents/sdd-design.agent.md,
vscode/agents/sdd-tasks.agent.md, vscode/agents/sdd-apply.agent.md,
vscode/agents/sdd-verify.agent.md, vscode/agents/sdd-archive.agent.md, and
vscode/agents/sdd-onboard.agent.md. Insert these entries as a new commented
section after the cursor agent files section to maintain the organized structure
of the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ed726b4-2df2-4115-ab97-24dc36ab103a
📒 Files selected for processing (18)
internal/agents/vscode/adapter.gointernal/agents/vscode/adapter_test.gointernal/assets/assets.gointernal/assets/assets_test.gointernal/assets/vscode/agents/sdd-apply.agent.mdinternal/assets/vscode/agents/sdd-archive.agent.mdinternal/assets/vscode/agents/sdd-design.agent.mdinternal/assets/vscode/agents/sdd-explore.agent.mdinternal/assets/vscode/agents/sdd-init.agent.mdinternal/assets/vscode/agents/sdd-onboard.agent.mdinternal/assets/vscode/agents/sdd-orchestrator.agent.mdinternal/assets/vscode/agents/sdd-propose.agent.mdinternal/assets/vscode/agents/sdd-spec.agent.mdinternal/assets/vscode/agents/sdd-tasks.agent.mdinternal/assets/vscode/agents/sdd-verify.agent.mdinternal/components/sdd/inject.gointernal/components/sdd/inject_test.gointernal/components/uninstall/service_test.go
459c6b8 to
adab8d3
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 `@internal/components/uninstall/service_test.go`:
- Around line 342-349: The uninstall service test currently applies every
operation under the agents directory, which is too broad; update the loop in the
test around op.apply so it only runs when the operation is opRemoveFile,
matching the stricter behavior used by the Claude counterpart. Keep the existing
path-scope check, but add an op.typeID filter before applying to ensure only
file-removal operations are executed and no other operation types can be applied
accidentally.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 192db985-9691-4868-a69b-4373e2b7a669
📒 Files selected for processing (18)
internal/agents/vscode/adapter.gointernal/agents/vscode/adapter_test.gointernal/assets/assets.gointernal/assets/assets_test.gointernal/assets/vscode/agents/sdd-apply.agent.mdinternal/assets/vscode/agents/sdd-archive.agent.mdinternal/assets/vscode/agents/sdd-design.agent.mdinternal/assets/vscode/agents/sdd-explore.agent.mdinternal/assets/vscode/agents/sdd-init.agent.mdinternal/assets/vscode/agents/sdd-onboard.agent.mdinternal/assets/vscode/agents/sdd-orchestrator.agent.mdinternal/assets/vscode/agents/sdd-propose.agent.mdinternal/assets/vscode/agents/sdd-spec.agent.mdinternal/assets/vscode/agents/sdd-tasks.agent.mdinternal/assets/vscode/agents/sdd-verify.agent.mdinternal/components/sdd/inject.gointernal/components/sdd/inject_test.gointernal/components/uninstall/service_test.go
c7fffc5 to
f7ed70b
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 `@internal/assets/vscode/agents/sdd-orchestrator.agent.md`:
- Around line 177-182: The fenced dependency-graph block in the agent markdown
is missing a language identifier, triggering markdownlint MD040. Update the
fenced block around the dependency graph in the orchestrator agent doc to use a
language tag such as text so the block remains readable while satisfying lint
rules.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 55668796-4287-4447-b615-1678adf0ff45
📒 Files selected for processing (18)
internal/agents/vscode/adapter.gointernal/agents/vscode/adapter_test.gointernal/assets/assets.gointernal/assets/assets_test.gointernal/assets/vscode/agents/sdd-apply.agent.mdinternal/assets/vscode/agents/sdd-archive.agent.mdinternal/assets/vscode/agents/sdd-design.agent.mdinternal/assets/vscode/agents/sdd-explore.agent.mdinternal/assets/vscode/agents/sdd-init.agent.mdinternal/assets/vscode/agents/sdd-onboard.agent.mdinternal/assets/vscode/agents/sdd-orchestrator.agent.mdinternal/assets/vscode/agents/sdd-propose.agent.mdinternal/assets/vscode/agents/sdd-spec.agent.mdinternal/assets/vscode/agents/sdd-tasks.agent.mdinternal/assets/vscode/agents/sdd-verify.agent.mdinternal/components/sdd/inject.gointernal/components/sdd/inject_test.gointernal/components/uninstall/service_test.go
🔗 Linked Issue
Closes #504
Related context: #677
🏷️ PR Type
What kind of change does this PR introduce?
type:bug— Bug fix (non-breaking change that fixes an issue)type:feature— New feature (non-breaking change that adds functionality)type:docs— Documentation onlytype:refactor— Code refactoring (no functional changes)type:chore— Build, CI, or tooling changestype:breaking-change— Breaking change (fix or feature that changes existing behavior)📝 Summary
PR 1 of the VS Code Copilot SDD subagents stack. This adds the backend/native-agent foundation only: VS Code now supports native SDD
.agent.mdfiles in~/.copilot/agents, with embedded coordinator/phase assets, injection post-check support, idempotency coverage, and uninstall preservation for user-authored agents.Follow-up PRs will add VS Code model assignment foundation and TUI/docs. Those are intentionally out of scope here.
This PR requests
size:exception: it exceeds 400 changed lines because it ships substantive Markdown.agent.mdassets, not because the code surface is large.📂 Changes
internal/agents/vscode~/.copilot/agentsand added path testsinternal/assetsall:vscodeand added VS Code SDD.agent.mdassetsinternal/components/sdd.agent.mdin native sub-agent post-check and covered VS Code injection/idempotencyinternal/components/uninstall.agent.mdfiles🧪 Test Plan
Focused PR1 tests
Package smoke
go test ./internal/agents/vscode ./internal/assetsUnit Tests
go test ./...E2E Tests (Docker required)
go test ./...)cd e2e && ./docker-test.sh)Notes: focused PR1 tests passed locally. Full
go test ./...was not claimed here due known local Windows/CRLF/symlink/environment failures unrelated to this PR.🤖 Automated Checks
The following checks run automatically on this PR:
size:exception; most added lines are.agent.mdassetsCloses #504status:approvedstatus:approvedtype:*Labeltype:featurerequested (maintainer label needed)go test ./...✅ Contributor Checklist
status:approvedsize:exceptionwith rationale documented — maintainer label needed (size:exception)type:*label to this PR — maintainer label needed (type:feature)go test ./...)cd e2e && ./docker-test.sh)Co-Authored-Bytrailers💬 Notes for Reviewers
This is the backend/native-agent slice only. It deliberately does not include VS Code model assignment state, dynamic Copilot model catalog integration, TUI/profile UX, or final docs updates; those will come in follow-up PRs.
Summary by CodeRabbit
New Features
~/.copilot/agents).Bug Fixes
.agent.mdfiles.Tests