feat(copilot): make cloud coding-agent files opt-in - #1517
Conversation
Selecting the `github-copilot` tool auto-generated a GitHub Actions workflow (.github/workflows/copilot-setup-steps.yml) plus an agent file. Writing into a user's CI on init/update is invasive, benefits only the narrow set of Copilot *cloud* coding-agent users, and couples us to GitHub's externally-owned custom-agent format. Cloud files are now opt-in: - `openspec init` prompts before generating them (default No) and records the choice in openspec/config.yaml (`githubCopilot.cloudAgent`). - `--copilot-cloud` / `--no-copilot-cloud` decide non-interactively. - `openspec update` never prompts; it only refreshes files for projects that opted in, or that already have generated cloud files (so existing setups keep working — the migration path). The pre-existing content-matching guarantees are unchanged and now proven by regression tests: a user-customized cloud file is never overwritten or deleted. Opt-in state is persisted via the YAML document model so the user's hand-authored config comments and formatting survive untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughGitHub Copilot cloud-agent generation is now opt-in. Initialization supports CLI and interactive choices, persists explicit settings, and detects existing managed files. Updates synchronize cloud files only when enabled. ChangesCopilot cloud-agent opt-in
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant InitCommand
participant cloud-agent
participant ProjectConfig
participant UpdateCommand
CLI->>InitCommand: pass copilotCloud option
InitCommand->>cloud-agent: resolve enablement
cloud-agent->>ProjectConfig: read or persist cloudAgent
InitCommand->>InitCommand: generate cloud files conditionally
UpdateCommand->>cloud-agent: check enablement
cloud-agent-->>UpdateCommand: return enabled state
UpdateCommand->>UpdateCommand: synchronize cloud files conditionally
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 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 `@src/core/github-copilot/cloud-agent.ts`:
- Around line 563-566: Update the persistence logic in persistCopilotCloudOptIn
to detect when the existing COPILOT_CONFIG_KEY node is scalar and replace it
with a mapping before calling doc.setIn for COPILOT_CLOUD_AGENT_KEY; preserve
existing mappings and ensure the write remains non-fatal. Add a regression test
covering githubCopilot: false followed by persistCopilotCloudOptIn(tempDir,
true).
🪄 Autofix
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 Plus
Run ID: 35156a3b-6b85-4551-89cc-34b4bff3917b
📒 Files selected for processing (10)
.changeset/copilot-cloud-opt-in.mdsrc/cli/index.tssrc/core/completions/command-registry.tssrc/core/github-copilot/cloud-agent.tssrc/core/init.tssrc/core/project-config.tssrc/core/update.tstest/core/github-copilot-cloud-agent.test.tstest/core/init.test.tstest/core/update.test.ts
Deploying openspec-docs with
|
| Latest commit: |
c4d4332
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cdc62903.openspec-docs.pages.dev |
| Branch Preview URL: | https://fix-copilot-cloud-opt-in.openspec-docs.pages.dev |
Follow-up hardening driven by a five-agent review swarm over the opt-in. Correctness: - persistCopilotCloudOptIn no longer throws on a scalar/`null` config file (reproduced crash); it starts a fresh map while preserving comment-only and empty files. - Explicit opt-out (`--no-copilot-cloud` / `cloudAgent: false`) now removes OpenSpec-managed cloud files on both init and update, instead of orphaning them. Customized files are still never touched. - `--copilot-cloud` / `--no-copilot-cloud` warns when github-copilot isn't among the selected tools, instead of silently no-opping. UX / discoverability: - init prints whether cloud files were written or, when skipped for want of a signal, how to enable them (`--copilot-cloud`). - When the user opts in but already has their own copilot-setup-steps.yml or agent file, init/update say it was left untouched and that the OpenSpec install step must be added by hand — the direct answer to "will this affect my existing Copilot cloud agent?". - Clearer interactive prompt (names both files; distinguishes the GitHub-hosted cloud agent from Copilot in the editor); a dim, interactive-only, decision- gated hint on `openspec update`; tightened flag help text. Docs (the feature was undocumented): new "GitHub Copilot cloud coding agent" section in supported-tools.md; init flags in cli.md; the githubCopilot.cloudAgent key in customization.md. Tests: interactive prompt (accept/decline), opt-out removal + customized-file preservation, config.yml variant, scalar-config regression, collision reporting, flag-ignored warning, re-init honoring persisted opt-in, and the config parse/warn branches. 2763 tests pass; the only failures are pre-existing and unrelated (completion mocks, adapters loader, one config-profile PATH case, one experimental-alias case), verified identical on clean main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 @.changeset/copilot-cloud-opt-in.md:
- Line 5: Update the changeset description to state that non-interactive
openspec init without --copilot-cloud or --no-copilot-cloud skips GitHub Copilot
cloud-agent files and does not persist a configuration choice.
In `@docs/cli.md`:
- Around line 105-106: Update the --copilot-cloud and --no-copilot-cloud entries
in docs/cli.md lines 105-106 to state that they only apply when github-copilot
is selected, such as with --tools github-copilot. Add the same condition to the
corresponding behavior table entries in docs/supported-tools.md lines 128-130.
In `@docs/customization.md`:
- Around line 56-60: Update the githubCopilot.cloudAgent configuration example
in the customization documentation to clearly label false as an explicit
opt-out, and state that users should omit the optional block when they have made
no decision.
In `@docs/supported-tools.md`:
- Line 131: Update the `openspec update` documentation row to replace “the
project already has them” with wording that specifically refers to “existing
OpenSpec-managed files,” preserving the surrounding refresh and removal
behavior.
🪄 Autofix
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 Plus
Run ID: 6684227a-e210-4f7f-9d84-7aa23c22be42
📒 Files selected for processing (12)
.changeset/copilot-cloud-opt-in.mddocs/cli.mddocs/customization.mddocs/supported-tools.mdsrc/cli/index.tssrc/core/github-copilot/cloud-agent.tssrc/core/init.tssrc/core/update.tstest/core/github-copilot-cloud-agent.test.tstest/core/init.test.tstest/core/project-config.test.tstest/core/update.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/cli/index.ts
- src/core/update.ts
| "@fission-ai/openspec": minor | ||
| --- | ||
|
|
||
| Make GitHub Copilot cloud coding-agent files opt-in. Selecting the `github-copilot` tool no longer silently writes a GitHub Actions workflow into `.github/`; `openspec init` now asks first (default No) and remembers the choice in `openspec/config.yaml` (`githubCopilot.cloudAgent`). Use `--copilot-cloud` / `--no-copilot-cloud` to decide non-interactively. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe the non-interactive default.
openspec init does not always prompt. In non-interactive mode without either flag, it skips cloud files and does not persist a choice. State this behavior here to prevent incorrect automation assumptions.
Based on PR objectives: “Non-interactive initialization without a flag skips cloud files without persisting a choice.”
🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: The official name of this software platform is spelled with a capital “H”.
Context: ...y writes a GitHub Actions workflow into .github/; openspec init now asks first (defa...
(GITHUB)
🤖 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 @.changeset/copilot-cloud-opt-in.md at line 5, Update the changeset
description to state that non-interactive openspec init without --copilot-cloud
or --no-copilot-cloud skips GitHub Copilot cloud-agent files and does not
persist a configuration choice.
| | `--copilot-cloud` | Set up GitHub Copilot [cloud coding-agent files](supported-tools.md#github-copilot-cloud-coding-agent) without prompting | | ||
| | `--no-copilot-cloud` | Skip GitHub Copilot cloud coding-agent files without prompting | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document that the cloud flags require selecting github-copilot.
The runtime ignores --copilot-cloud and --no-copilot-cloud when github-copilot is absent from the selected tools. The documentation must state this condition.
docs/cli.md#L105-L106: explain that users must selectgithub-copilot, such as with--tools github-copilot.docs/supported-tools.md#L128-L130: add the same condition to the behavior table.
📍 Affects 2 files
docs/cli.md#L105-L106(this comment)docs/supported-tools.md#L128-L130
🤖 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 `@docs/cli.md` around lines 105 - 106, Update the --copilot-cloud and
--no-copilot-cloud entries in docs/cli.md lines 105-106 to state that they only
apply when github-copilot is selected, such as with --tools github-copilot. Add
the same condition to the corresponding behavior table entries in
docs/supported-tools.md lines 128-130.
|
|
||
| # Set by `openspec init` when you choose (or decline) the GitHub Copilot | ||
| # cloud coding agent; controls whether `init`/`update` generate its files. | ||
| githubCopilot: | ||
| cloudAgent: false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify that cloudAgent: false is an explicit opt-out.
This manual configuration example persists a decision. It prevents later prompting and causes updates to remove managed cloud-agent files. Tell users to omit the optional block when no decision is intended, or label this example as an explicit opt-out.
🤖 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 `@docs/customization.md` around lines 56 - 60, Update the
githubCopilot.cloudAgent configuration example in the customization
documentation to clearly label false as an explicit opt-out, and state that
users should omit the optional block when they have made no decision.
| | `openspec init` (interactive) | Asks whether to set up cloud files. Default is **No**. | | ||
| | `openspec init --copilot-cloud` | Sets them up without prompting (for scripts/CI). | | ||
| | `openspec init --no-copilot-cloud` | Skips them without prompting, and removes any previously generated ones. | | ||
| | `openspec update` | Never prompts. Refreshes the files only if you opted in (or the project already has them). If you opted out, it removes OpenSpec-managed cloud files. | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Refer specifically to existing OpenSpec-managed files.
“The project already has them” can include customized or user-owned files. Replace it with “existing OpenSpec-managed files” so this sentence matches the protection guarantee in lines 132-133.
🤖 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 `@docs/supported-tools.md` at line 131, Update the `openspec update`
documentation row to replace “the project already has them” with wording that
specifically refers to “existing OpenSpec-managed files,” preserving the
surrounding refresh and removal behavior.
Final hardening pass (adversarial review of the opt-in polish). - init's success line listed both cloud-file paths from the *decision* to write, not from what was written — so it claimed files that a write skipped (user already owns them) or that the alternate-agent path removed. It now lists only OpenSpec-managed files that actually exist after the write (listManagedCloudFiles), keeps the "left untouched" caveat for user-owned files, and reports opt-out removals in the normal output block. - persistCopilotCloudOptIn's non-map guard used isCollection, which is also true for sequences, so a YAML list at the config root still made setIn throw. Gate on isMap so scalars AND sequences fall back to a fresh document; empty/comment-only files still round-trip with comments intact. - Fixed a misleading catch comment on the opt-out removal path. Tests: success-line accuracy over a user-owned file, sequence-root config regression, and listManagedCloudFiles coverage. 318 tests pass across the touched suites; build + lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/core/github-copilot/cloud-agent.ts (1)
565-573: 🎯 Functional Correctness | 🟠 MajorHandle a non-map
githubCopilotnode beforesetIn.This is the same unresolved issue from the previous review. The guard checks only
parsed.contents. ForgithubCopilot: false, the root is a map, so Lines 571-573 keepparsedanddoc.setIn(...)still traverses through a scalar. The YAML API throws when an intermediatesetInnode is not a collection. (eemeli.org)Because
src/core/init.tscatches this persistence error, the explicit decision is silently not saved. Replace any existing non-mapgithubCopilotnode before callingsetIn. Add regression cases for scalar and sequence values.Proposed fix
const doc: Document = parsed.contents === null || isMap(parsed.contents) ? parsed : new Document(); + if (isMap(doc.contents)) { + const githubCopilot = doc.get(COPILOT_CONFIG_KEY, true); + if (githubCopilot !== undefined && !isMap(githubCopilot)) { + doc.set(COPILOT_CONFIG_KEY, {}); + } + } doc.setIn([COPILOT_CONFIG_KEY, COPILOT_CLOUD_AGENT_KEY], value);As per coding guidelines, run the focused test with
pnpm exec vitest run test/core/github-copilot-cloud-agent.test.ts -t "replaces a malformed githubCopilot scalar when persisting an explicit decision".🤖 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 565 - 573, Update the document preparation before setIn in the cloud-agent persistence flow to replace any existing non-map githubCopilot node, including scalar and sequence values, with a map while preserving valid mappings. Ensure doc.setIn([COPILOT_CONFIG_KEY, COPILOT_CLOUD_AGENT_KEY], value) no longer traverses a malformed intermediate node, and add regression coverage for both malformed scalar and sequence cases.Source: Coding guidelines
🧹 Nitpick comments (1)
test/core/init.test.ts (1)
1457-1474: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNormalize the cloud-file output assertions.
COPILOT_CLOUD_FILESis defined withpath.join(...), so Windows createscopilot.presentwith backslashes.listManagedCloudFiles()returns those same values, and the test assertions compare them directly againstpath.join(...)strings. Keep the filesystem paths frompath.join(...)but normalize both the captured output and the expected display paths before asserting.Proposed assertion fix
+ const displayOut = FileSystemUtils.toPosixPath(out); + const setupDisplay = FileSystemUtils.toPosixPath(setupRel); + const agentDisplay = FileSystemUtils.toPosixPath(agentRel); - expect(out).toContain(`GitHub Copilot cloud files: ${agentRel}`); - expect(out).not.toContain(`cloud files: ${setupRel}`); - expect(out).toContain(`Left your existing ${setupRel} untouched`); + expect(displayOut).toContain(`GitHub Copilot cloud files: ${agentDisplay}`); + expect(displayOut).not.toContain(`cloud files: ${setupDisplay}`); + expect(displayOut).toContain(`Left your existing ${setupDisplay} untouched`);🤖 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/init.test.ts` around lines 1457 - 1474, Update the cloud-file output assertions in the “opting in over a user-owned cloud file” test to normalize path separators on both the captured console output and expected display paths before comparing them. Keep filesystem setup and file reads using the existing path.join values, and apply normalization only to the string assertions involving agentRel, setupRel, and out.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.
Duplicate comments:
In `@src/core/github-copilot/cloud-agent.ts`:
- Around line 565-573: Update the document preparation before setIn in the
cloud-agent persistence flow to replace any existing non-map githubCopilot node,
including scalar and sequence values, with a map while preserving valid
mappings. Ensure doc.setIn([COPILOT_CONFIG_KEY, COPILOT_CLOUD_AGENT_KEY], value)
no longer traverses a malformed intermediate node, and add regression coverage
for both malformed scalar and sequence cases.
---
Nitpick comments:
In `@test/core/init.test.ts`:
- Around line 1457-1474: Update the cloud-file output assertions in the “opting
in over a user-owned cloud file” test to normalize path separators on both the
captured console output and expected display paths before comparing them. Keep
filesystem setup and file reads using the existing path.join values, and apply
normalization only to the string assertions involving agentRel, setupRel, and
out.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a30e37f-afd6-4ee5-8e64-f765ddf4e484
📒 Files selected for processing (4)
src/core/github-copilot/cloud-agent.tssrc/core/init.tstest/core/github-copilot-cloud-agent.test.tstest/core/init.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/init.ts
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Changes requested: persistCopilotCloudOptIn still throws when a valid top-level map contains a non-map githubCopilot value (false, null, or a sequence), so init swallows the error and doesn't save the explicit choice. Replace that intermediate node with a map before setIn and add a regression; I reproduced all three cases at b4113d5.
Addresses alfred review on #1517. The prior guard only fixed a non-map config *root*; a valid top-level map whose `githubCopilot` value is itself a scalar/null/sequence (`githubCopilot: false`, `null`, or a list) still made `setIn(['githubCopilot','cloudAgent'], ...)` throw, which init swallowed — so the explicit opt-in/out was never saved. Now the intermediate node is replaced with an empty map before descending, keeping the rest of the config and its comments intact. Regression covers all three reproduced cases (false/null/sequence). Full suite: 2770 pass; only the pre-existing unrelated failures remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deeper pass on persistCopilotCloudOptIn (the function alfred flagged), driven by an exhaustive input-shape check. Two malformed inputs still threw at toString(): a multi-document YAML stream and a tab-indented (syntactically invalid) file. Such a file can't be edited without corrupting it, so persist now detects parse errors and leaves it untouched (no throw, no clobber) — it is already invalid, so readProjectConfig ignores it regardless. With this the function is throw-free across every shape exercised: empty, comment-only, scalar/sequence root, a non-map githubCopilot value, anchors, CRLF, BOM, and the two malformed cases (now skipped byte-identical). Regression added for the multi-document case. Touched suites: 314 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
The config fix now handles scalar, null, and sequence values while preserving surrounding YAML; malformed YAML stays untouched. Fresh build, lint, 314 focused tests, runtime reproductions, and the full hosted matrix pass.
2 participants
Status
Follow-up to #1274 (merged). Everything stays merged; this hardens the one piece that carried a maintenance risk, then polishes it into a first-class, discoverable feature. Ready for review.
The issue with #1274
#1274 auto-generated GitHub Copilot cloud coding-agent files (
.github/workflows/copilot-setup-steps.yml+.github/agents/openspec.agent.md) whenever thegithub-copilottool was selected. Two problems:copilot-setup-steps; the agenttools:aliases are GitHub's schema), so they drift whenever GitHub changes the contract.What it does
Cloud files become opt-in:
openspec init(interactive)openspec/config.yaml→githubCopilot.cloudAgent.openspec init --copilot-cloud/--no-copilot-cloudopenspec init(non-interactive, no signal)--copilot-cloud).openspec updatePersistence uses the YAML document model, so the user's hand-authored config comments survive.
Hardening (two multi-agent review passes)
persistCopilotCloudOptInthrew on a scalar/sequence/nullconfig); explicit opt-out now removes orphaned managed files instead of stranding them;--copilot-cloudwarns whengithub-copilotisn't selected instead of silently no-opping.initreports cloud files from what is actually on disk after the write — it never claims a file a write skipped (you already own it) or that reconciliation removed; clearer prompt (names both files, distinguishes the GitHub-hosted agent from editor Copilot); a dim, decision-gated hint onupdate.copilot-setup-steps.yml,init/updatenever touch it and now say it was preserved and that you must add the OpenSpec install step by hand.supported-tools.md; init flags incli.md; the config key incustomization.md.Proof it works
config.ymlvariant, scalar- and sequence-root config regressions, collision reporting, flag-ignored warning, re-init honoring persisted opt-in, and config parse/warn branches.node build.js✓,eslint src/✓, 2769 tests pass.Notes / nits
minor(new flag + config field + changed default).completion.test.tsenv-sensitive Zsh,command-generation/adapters.test.tsmissingsmol-tomldep, oneconfig-profilecase, oneartifact-workflowcase) fail identically on cleanmain— pre-existing and unrelated, verified against a clean checkout.--copilot-cloud/--no-copilot-cloudflags are registered in the shell-completion command registry to keep the parity test green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation