Skip to content

feat(copilot): warn before skills default rollout#3256

Open
WOLIKIMCHENG wants to merge 3 commits into
github:mainfrom
WOLIKIMCHENG:copilot-default-skills
Open

feat(copilot): warn before skills default rollout#3256
WOLIKIMCHENG wants to merge 3 commits into
github:mainfrom
WOLIKIMCHENG:copilot-default-skills

Conversation

@WOLIKIMCHENG

@WOLIKIMCHENG WOLIKIMCHENG commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a deprecation warning for Copilot's legacy markdown scaffold when users initialize Copilot without --skills.

This keeps the current .github/agents/*.agent.md + .github/prompts/ default behavior intact, while pointing users to opt in early with --integration-options "--skills" before skills mode becomes the default in a future release.

Scope of this change:

  • keep Copilot markdown mode as the default
  • warn when default Copilot setup uses the legacy markdown scaffold
  • keep explicit Copilot --skills installs warning-free
  • add direct setup and CLI init coverage for the warning behavior

Testing

  • Tested locally with uv run specify --help
  • Tested with a sample project (if applicable)

Focused validation run:

  • .venv/bin/python -m pytest tests/integrations/test_integration_copilot.py -q
  • .venv/bin/python -m pytest tests/integrations/test_cli.py -q
  • .venv/bin/python -m pytest tests/integrations/test_integration_subcommand.py -q
  • uvx ruff check src/specify_cli/integrations/copilot/__init__.py tests/integrations/test_integration_copilot.py
  • git diff --check

All focused checks above passed locally.

@WOLIKIMCHENG WOLIKIMCHENG requested a review from mnriem as a code owner June 30, 2026 13:47
@mnriem

mnriem commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks for tackling this, @WOLIKIMCHENG. Defaulting Copilot to skills mode is a reasonable long-term direction, but I'd like to push back on both the rollout and the justification.

1. Drop "Fixes #865" — this PR doesn't fix it.
Slash commands in Copilot do work; #865 isn't an actual defect in the markdown/.agent.md path, and nothing in this change addresses it. Please remove the Fixes #865 reference entirely (don't auto-close that issue on merge). The default flip needs to stand on its own merits, not on an unrelated, non-reproducing report.

2. The legacy mode becomes unreachable, not just non-default.
Flag options in _parse_integration_options (_helpers.py) can only ever be set to True when present — there's no negation, and IntegrationOption.default isn't seeded into parsed_options. Behavior is driven solely by parsed_options.get("skills", True) in setup(). After this change there's no CLI path that produces the .agent.md layout — it survives only because tests call setup(skills=False) directly. We don't want to remove that mode.

3. This needs a deprecation cycle.
Main is currently at 0.12.x (latest release v0.12.1). Proposed plan:

  • Now (0.12/0.13): keep markdown as the default but emit a deprecation warning on Copilot init telling users skills mode is becoming the default and how to opt in early. There's precedent in this same file — see the SPECKIT_ALLOW_ALL_TOOLS deprecation warning.
  • Flip the default 2 minor (Y) releases later (0.14.0), giving users a heads-up window.

So this PR shouldn't merge the flip yet — let's split it into "add deprecation warning now" and "flip default later."

4. Add an explicit --agents opt-out, and preserve existing users on flip.
When skills becomes the default, users need a way back to the legacy layout — please add --agents (negation support in the option parser) selecting the .github/agents/*.agent.md + .github/prompts/ + .vscode/settings.json scaffold. And the flip must respect .specify/init-options.json: markdown installs don't set ai_skills: true, so re-init/upgrade has to read and rewrite/normalize that file for existing --agents users so they aren't silently migrated onto skills.

@WOLIKIMCHENG WOLIKIMCHENG changed the title feat(copilot): default to skills mode feat(copilot): warn before skills default rollout Jun 30, 2026
@WOLIKIMCHENG

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed guidance. That makes sense.

I revised the PR to remove the default flip and keep the current markdown layout as the default. The PR now only adds a deprecation warning for default Copilot legacy markdown setup and points users to --integration-options "--skills" as the early opt-in path.

I also removed the Fixes #865 framing from the PR body. The default flip, explicit --agents opt-out, and .specify/init-options.json preservation/migration behavior can be handled in a later PR with the rollout window you described.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a user-visible warning when Copilot is initialized in the legacy “.agent.md + .prompt.md” default mode, nudging users to opt into --skills ahead of an eventual default switch.

Changes:

  • Emit a UserWarning during Copilot setup when --skills was not provided (legacy default path).
  • Add direct integration-level tests verifying the warning behavior (warns in default mode, no warning in skills mode).
  • Add CLI init tests verifying the same warning behavior through specify init.
Show a summary per file
File Description
tests/integrations/test_integration_copilot.py Adds unit + CLI coverage around the new deprecation warning behavior for default vs --skills Copilot setup.
src/specify_cli/integrations/copilot/init.py Introduces _warn_legacy_markdown_default() and triggers it only for default (non-skills) Copilot scaffolding.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines +54 to +59
with warnings.catch_warnings(record=True) as caught:
warnings.simplefilter("always")
created = copilot.setup(tmp_path, m, parsed_options={"skills": True})

assert len(caught) == 0
assert any(f.name == "SKILL.md" for f in created)

@mnriem mnriem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address Copilot feedback

@WOLIKIMCHENG

Copy link
Copy Markdown
Contributor Author

Addressed the Copilot feedback by changing the skills-mode warning assertion to check only that the legacy markdown deprecation warning is absent. The focused Copilot integration test file passes locally.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants