feat: generate Copilot coding agent files on openspec init (github-copilot) - #1274
Conversation
…selected When `openspec init` or `openspec update` is run with the github-copilot tool selected, two additional files are now generated in the user's project: 1. `.github/workflows/copilot-setup-steps.yml` - A GitHub Actions workflow that pre-installs the OpenSpec CLI in the Copilot coding agent's ephemeral environment (required for the agent to use `openspec` commands). 2. `.github/agents/openspec.agent.md` - A custom agent definition that instructs the GitHub Copilot coding agent how to use the OpenSpec CLI, including all agent-compatible commands with `--json` output, workflow patterns, and best practices. These files are only written if they don't already exist (to preserve user customizations). The generation is non-fatal — if it fails, init/update still completes successfully. New module: src/core/github-copilot/cloud-agent.ts Tests: test/core/github-copilot-cloud-agent.test.ts
When github-copilot is not in the configured tools during update, remove the cloud agent files (copilot-setup-steps.yml and openspec.agent.md) if they exist.
|
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:
📝 WalkthroughWalkthroughAdds GitHub Copilot cloud-agent file generation and lifecycle management. Initialization creates the files when Copilot is selected. Updates create, reconcile, or remove them according to configured tools. Tests cover content, filesystem behavior, legacy handling, and command integration. ChangesCopilot Cloud Agent Files
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InitCommand
participant UpdateCommand
participant CloudAgent
participant ProjectFiles
InitCommand->>CloudAgent: Write Copilot cloud files during setup
UpdateCommand->>CloudAgent: Synchronize configured tool state
CloudAgent->>ProjectFiles: Create, preserve, reconcile, or remove managed files
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/core/github-copilot/cloud-agent.ts`:
- Around line 186-199: The cleanup in removeCopilotCloudFiles currently deletes
any existing file at the Copilot cloud paths, which can remove user-customized
files. Update removeCopilotCloudFiles to only unlink files that are known to be
generated/managed by the extension, and skip files that have been modified or
created by the user; use COPILOT_CLOUD_FILES and FileSystemUtils.fileExists as
the entry points for the check, and add a reliable ownership/safeguard before
calling fs.promises.unlink.
In `@src/core/update.ts`:
- Line 156: The no-tools early return in execute() is skipping Copilot file
cleanup, so stale cloud files can remain when github-copilot is removed. Move
the syncCopilotCloudFiles call in update.ts so it runs before the return path
that handles an empty tools list, and make sure both execute() and the related
sync flow around syncCopilotCloudFiles use the resolvedProjectPath and tool set
consistently even when there are no configured tools.
In `@test/core/github-copilot-cloud-agent.test.ts`:
- Around line 51-60: The test in generateCopilotAgentFile only verifies the
presence of execute, so it can miss regressions in the Copilot agent tool
aliases. Tighten the expectations in github-copilot-cloud-agent.test.ts to
assert all four aliases exposed by the updated contract are present: execute,
read, search, and edit, alongside the existing frontmatter and command checks.
- Around line 63-67: The COPILOT_CLOUD_FILES path assertions are too loose
because `toContain(...)` only checks filenames and can miss incorrect parent
directories or POSIX-only regressions. Update the
`github-copilot-cloud-agent.test.ts` checks to compare the full expected
relative paths using `path.join(...)`, referencing
`COPILOT_CLOUD_FILES.setupSteps` and `COPILOT_CLOUD_FILES.agent` so the test
validates separator behavior on Windows as well.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3078b54-83db-4187-acde-3e207441355f
📒 Files selected for processing (5)
src/core/github-copilot/cloud-agent.tssrc/core/init.tssrc/core/update.tstest/core/github-copilot-cloud-agent.test.tstest/core/update.test.ts
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks for restoring this. The current guard is much safer for customized files, but I think the cleanup is now too strict: removeCopilotCloudFiles() only deletes files whose content exactly matches the current generated template.
That means a user who generated these files with an older OpenSpec version, then removed github-copilot, would keep stale generated .github/workflows/copilot-setup-steps.yml / .github/agents/openspec.agent.md files forever. That is the path this PR is explicitly trying to clean up.
I’d change the ownership check to recognize OpenSpec-generated legacy/current variants, e.g. a managed marker in new files plus a conservative legacy recognizer for the two templates this PR introduced. Customized files should still be skipped, but previously generated files should be removable when github-copilot is no longer configured.
Verified locally: pnpm exec vitest run test/core/github-copilot-cloud-agent.test.ts test/core/update.test.ts passes, 70 tests.
Thanks for reviewing this, I changed the cleanup logic and added tests for edge cases. |
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 `@test/core/github-copilot-cloud-agent.test.ts`:
- Around line 17-45: The synthetic legacy fixture helpers currently use silent
string replacements, so template drift can go unnoticed and the legacy-removal
path may stop being exercised. Harden removeManagedMarker and
generateLegacyCopilotAgentFile by asserting each expected replace target exists
before transforming the content, and fail the test immediately if any marker or
legacy text is missing. This should make the fixture generation loud when
generateCopilotAgentFile changes and keep the markerless/legacy tests
meaningful.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 863f6c2d-366b-4903-b8e9-a310e29e3a28
📒 Files selected for processing (2)
src/core/github-copilot/cloud-agent.tstest/core/github-copilot-cloud-agent.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/github-copilot/cloud-agent.ts
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Looks good. The latest commits address the legacy cleanup path without deleting customized files, and the fixture hardening makes the markerless legacy tests fail loudly if the generated templates drift.\n\nVerified locally: pnpm exec vitest run test/core/github-copilot-cloud-agent.test.ts test/core/update.test.ts, pnpm run lint, and pnpm run build.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/core/init.ts (1)
615-623: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWarn when Copilot file generation fails.
Keeping initialization non-fatal is reasonable, but the empty
catchmakes the command report success while the Copilot files may be missing. Emit a warning with the error details before continuing so users can diagnose the incomplete setup.Suggested adjustment
- } catch { + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.warn(`Warning: failed to generate Copilot cloud agent files: ${message}`); // Non-fatal: don't block init if cloud agent files fail }🤖 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 `@src/core/init.ts` around lines 615 - 623, Update the catch block around writeCopilotCloudFiles in the GitHub Copilot initialization path to emit a warning containing the caught error details, while preserving the non-fatal behavior so initialization continues.
🤖 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.
Nitpick comments:
In `@src/core/init.ts`:
- Around line 615-623: Update the catch block around writeCopilotCloudFiles in
the GitHub Copilot initialization path to emit a warning containing the caught
error details, while preserving the non-fatal behavior so initialization
continues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 334da9d5-0d71-4164-9b3c-82c167408e3c
📒 Files selected for processing (3)
src/core/init.tssrc/core/update.tstest/core/update.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/update.ts
- test/core/update.test.ts
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Blocking: the new Copilot cloud file writer and remover never apply the project-boundary containment guard used by other managed artifacts. On this exact head, with project/.github symlinked to an external directory, writeCopilotCloudFiles wrote both managed files outside the project and removeCopilotCloudFiles then deleted them there; all 188 focused tests still pass. Please validate containment immediately before the relevant read, write, and unlink boundaries, and add regressions for a symlinked .github directory or nested workflows/agents directory while preserving the existing customized-file and legacy-file behavior.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/core/github-copilot/cloud-agent.ts (2)
184-258: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider storing legacy content as frozen snapshots.
generateLegacyCopilotAgentFileBody()derives historical content by applying seven reverse edits to the current template. Every future edit togenerateCopilotAgentFileBody()can invalidate one of these search literals.replaceRequiredmakes that failure loud, which is good, but it also means routine template edits break unrelated code paths and force authors to re-derive history.A frozen string constant per historical revision removes the coupling. Each snapshot then stays valid forever, and new revisions only append a constant.
This is a design preference, not a defect. The current code is correct.
🤖 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 `@src/core/github-copilot/cloud-agent.ts` around lines 184 - 258, Refactor generateLegacyCopilotAgentFileBody() to return a frozen string snapshot of the historical legacy template instead of deriving it through chained replaceRequired calls. Preserve the exact legacy content currently produced, and structure future historical revisions as additional immutable snapshot constants rather than reverse-editing generateCopilotAgentFileBody().
445-452: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider reporting the managed agent removal to the caller.
When
agentReconciliation === 'remove-managed', this code deletes.github/agents/openspec.agent.mdand returnsagentWritten: false. The caller cannot distinguish that deletion from "nothing to do".src/core/init.tsandsrc/core/update.tstherefore print no message, and the user sees a generated file disappear without explanation after adding.github/agents/openspec.md.Add a third field, for example
agentRemoved: boolean, so callers can log the change. The deletion itself is correct and is limited to recognized generated content.♻️ Suggested signal for the caller
let agentWritten = false; + let agentRemoved = false; if (agentReconciliation === 'reconcile') { agentWritten = await reconcileCopilotCloudFile(agentPath, COPILOT_CLOUD_FILES.agent); } else if (agentReconciliation === 'remove-managed') { await fs.unlink(agentPath); + agentRemoved = true; } - return { setupStepsWritten, agentWritten }; + return { setupStepsWritten, agentWritten, agentRemoved };Update the return type on line 417 to match, and update the two
toEqual({ setupStepsWritten, agentWritten })assertions intest/core/github-copilot-cloud-agent.test.ts.🤖 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 `@src/core/github-copilot/cloud-agent.ts` around lines 445 - 452, Extend the return value and declared return type of the relevant cloud-agent setup function to include an agentRemoved boolean, set it true only when agentReconciliation is 'remove-managed', and preserve agentWritten for reconciliation writes. Update callers in init.ts and update.ts to report the removal, and adjust both matching test assertions to include the new field.test/core/github-copilot-cloud-agent.test.ts (1)
110-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCanonicalize
tempDiraftermkdtemp.
os.tmpdir()is itself a symlink on macOS, where/var/folders/...resolves to/private/var/folders/.... The code under test resolves managed paths throughFileSystemUtils.resolveProjectArtifactPath, and the symlink tests at Line 353 and Line 520 depend on that resolution rejecting paths outside the project root. A non-canonical project root makes those containment results platform-dependent.Canonicalize the root once in
beforeEachto remove that variable. The coding guidelines direct tests to canonicalize existing filesystem paths before comparing them.Based on coding guidelines: "When asserting existing filesystem paths as identities, canonicalize both actual and expected paths first using `FileSystemUtils.canonicalizeExistingPath()` in project code and `fs.realpathSync.native()` in test-only expectations".♻️ Suggested change
beforeEach(async () => { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-copilot-cloud-agent-')); + tempDir = await fs.realpath( + await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-copilot-cloud-agent-')) + ); });Apply the same treatment to the
outsideDirvalues created at Line 354, Line 495, and Line 521.🤖 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 `@test/core/github-copilot-cloud-agent.test.ts` around lines 110 - 112, Canonicalize the temporary project root assigned in the beforeEach setup using fs.realpathSync.native() after mkdtemp, and apply the same canonicalization to each outsideDir created in the symlink-related tests. Preserve the existing path assertions while ensuring both project-root and expected outside paths use canonical filesystem identities.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/core/github-copilot/cloud-agent.ts`:
- Around line 184-258: Refactor generateLegacyCopilotAgentFileBody() to return a
frozen string snapshot of the historical legacy template instead of deriving it
through chained replaceRequired calls. Preserve the exact legacy content
currently produced, and structure future historical revisions as additional
immutable snapshot constants rather than reverse-editing
generateCopilotAgentFileBody().
- Around line 445-452: Extend the return value and declared return type of the
relevant cloud-agent setup function to include an agentRemoved boolean, set it
true only when agentReconciliation is 'remove-managed', and preserve
agentWritten for reconciliation writes. Update callers in init.ts and update.ts
to report the removal, and adjust both matching test assertions to include the
new field.
In `@test/core/github-copilot-cloud-agent.test.ts`:
- Around line 110-112: Canonicalize the temporary project root assigned in the
beforeEach setup using fs.realpathSync.native() after mkdtemp, and apply the
same canonicalization to each outsideDir created in the symlink-related tests.
Preserve the existing path assertions while ensuring both project-root and
expected outside paths use canonical filesystem identities.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e5deda99-a271-455e-a306-7692fb495cfc
📒 Files selected for processing (7)
.changeset/add-copilot-cloud-agent-files.mdsrc/core/github-copilot/cloud-agent.tssrc/core/init.tssrc/core/update.tstest/core/github-copilot-cloud-agent.test.tstest/core/init.test.tstest/core/update.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/update.ts
Resolved in 1427cb7: all Copilot cloud read/write/unlink targets now use project-boundary resolution and all-target preflight; regressions cover linked .github and nested workflows/agents paths while preserving customized and legacy files. Final CI passes on Linux, macOS, and Windows.
clay-good
left a comment
clay-good
left a comment
There was a problem hiding this comment.
LGTM. The prior project-boundary blocker is fixed with guarded, preflighted cloud-file lifecycle handling and regressions for linked paths, partial writes/deletes, customization preservation, legacy refresh/removal, and same-identifier agent collisions. Verified 201 focused tests plus build/lint locally; all hosted Linux, macOS, Windows, audit, dependency, release, and review checks pass on the current head.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-reviewed the containment fix on the exact rebased head. The managed Copilot paths now resolve through the project boundary, all targets are preflighted before mutation, and the prior .github symlink write/delete reproduction now fails closed without touching outside files. Fresh build and 201 focused tests pass; the full hosted matrix is green.
3 participants
Restores and updates #858 on top of current
main.Summary
When a user runs
openspec initoropenspec updatewith thegithub-copilottool configured, OpenSpec generates the GitHub Copilot coding agent support files:.github/workflows/copilot-setup-steps.yml.github/agents/openspec.agent.mdThe generated files are only written when missing, so user customizations are preserved. If
github-copilotis no longer configured,openspec updateremoves the generated cloud-agent files when present.Drift handled while restoring
openspec init(github-copilot) #858 onto currentmain.execute,read,search,edit).openspec updatecloud-file sync so the files are created even whengithub-copilotis already up to date and smart update exits early.Testing
Restored from closed PR: #858
Summary by CodeRabbit