fix: require Copilot-specific markers for .github/ auto-detection#1069
fix: require Copilot-specific markers for .github/ auto-detection#1069Giggitycountless wants to merge 4 commits intomicrosoft:mainfrom
Conversation
Previously, the mere existence of a .github/ directory was used to infer the project uses GitHub Copilot. This is too broad — nearly every repo has .github/ for CI workflows, CODEOWNERS, issue templates, etc. A Cursor user with standard CI would get skills installed to .github/skills/ instead of .cursor/. Now .github/ is only detected as a Copilot target when it contains Copilot-specific markers: - .github/copilot-instructions.md - .github/skills/ - .github/agents/ - .github/prompts/ Fixes microsoft#805
There was a problem hiding this comment.
Pull request overview
Tightens APM's target auto-detection so a repo is only treated as a GitHub Copilot (VS Code) integration target when .github/ contains Copilot-specific markers, avoiding false positives for repos that only use .github/ for CI/workflows.
Changes:
- Updated
detect_target()to require specific Copilot markers under.github/rather than treating.github/existence alone as a Copilot signal. - Updated unit tests to create Copilot marker folders for
.github/detection and added a new test ensuring a bare.github/workflows/falls through tominimal.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/apm_cli/core/target_detection.py | Replaces .github existence check with a Copilot marker-based check for VS Code/Copilot auto-detection. |
| tests/unit/core/test_target_detection.py | Updates existing detection tests to use markers and adds coverage for the “bare .github” case. |
| # For .github/, require Copilot-specific markers (not just CI workflows). | ||
| # A bare .github/ with only workflows/CODEOWNERS/etc. is NOT a Copilot signal. | ||
| github_copilot_markers = [ | ||
| ".github/copilot-instructions.md", | ||
| ".github/skills", | ||
| ".github/agents", | ||
| ".github/prompts", | ||
| ] | ||
| github_exists = any( | ||
| (project_root / marker).exists() for marker in github_copilot_markers | ||
| ) |
There was a problem hiding this comment.
detect_target() now treats .github/ as present only if one of the listed markers exists, but the marker set looks incomplete relative to the repo's own Copilot/VS Code integration surface. In particular, APM supports .github/instructions/ (and also .github/hooks/, plus legacy .github/chatmodes/) as first-class Copilot integration directories, so projects that only have modular instructions (or hooks) will now fall through to minimal and skip Copilot integration.
Consider expanding the marker list to include the other Copilot-specific subdirectories APM reads/writes (at least .github/instructions, and likely .github/hooks / .github/chatmodes) so auto-detection doesn't regress for those setups.
There was a problem hiding this comment.
Thanks for catching this! ✅ Fixed: added , , and to the marker list. APM reads/writes to all of these directories for Copilot integration, so they should all trigger auto-detection.
| # For .github/, require Copilot-specific markers (not just CI workflows). | ||
| # A bare .github/ with only workflows/CODEOWNERS/etc. is NOT a Copilot signal. | ||
| github_copilot_markers = [ |
There was a problem hiding this comment.
This change updates the semantics of VS Code/Copilot auto-detection (a bare .github/ should no longer trigger Copilot), but several docs pages still state that detection is based on .github/ directory existence alone (e.g. docs/src/content/docs/introduction/how-it-works.md, docs/src/content/docs/reference/cli-commands.md auto-detection table, and docs/src/content/docs/integrations/ide-tool-integration.md). Please update the docs to reflect the new "Copilot-specific markers" requirement, otherwise the documented behavior will be wrong for users.
There was a problem hiding this comment.
Great catch! ✅ Updated all three docs pages:
- how-it-works.md: Changed 'falling back to ' to 'falling back to minimal (AGENTS.md only)' and added note about marker-based detection
- cli-commands.md: Updated VSCode integration description to mention Copilot-specific markers
- ide-tool-integration.md: Updated auto-detection callout to explain marker requirement and that bare with only CI workflows won't trigger detection
|
|
||
| assert target == "minimal" | ||
| assert reason == REASON_NO_TARGET_FOLDER | ||
| assert "no target folder found" in reason |
There was a problem hiding this comment.
In test_auto_detect_bare_github_no_copilot_markers, reason is asserted equal to REASON_NO_TARGET_FOLDER and then also asserted to contain the same literal substring. The second assertion is redundant once equality is checked, and it makes future reason-message refactors noisier. Consider keeping only the constant-equality assertion (or only the substring assertion, but prefer the exported constant).
| assert "no target folder found" in reason |
There was a problem hiding this comment.
Good point! ✅ Removed the redundant substring assertion. The constant-equality check is sufficient and cleaner.
- Add .github/instructions, .github/hooks, .github/chatmodes to Copilot-specific marker list per Copilot review feedback - Update 3 docs pages to reflect marker-based .github/ detection: - how-it-works.md: clarify fallback to minimal, not .github/ - cli-commands.md: update VSCode integration detection description - ide-tool-integration.md: document marker requirement explicitly - Remove redundant substring assertion in bare-github test - Add 3 new tests for instructions/hooks/chatmodes markers Addresses Copilot review comments on microsoft#1069
- tests/unit/core/test_target_detection.py: rename unused 'reason' to
'_reason' in test_auto_detect_neither_folder (RUF059).
- src/apm_cli/core/target_detection.py: collapse multi-line generator
expression to satisfy 'ruff format'.
Verification:
uv run --extra dev ruff check src/ tests/ -> All checks passed!
uv run --extra dev ruff format --check src/ tests/ -> 623 files already formatted
uv run --extra dev pytest tests/unit/core/test_target_detection.py
-> 101 passed
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Not everyone has those markers yet use Copilot, and this would make it worse UX than the other vendors. We need to look at another solution. Ideally, APM should encourage users to use |
Description
Previously, the mere existence of a
.github/directory was used to infer the project uses GitHub Copilot. This is too broad — nearly every repo has.github/for CI workflows, CODEOWNERS, issue templates, etc. A Cursor user with standard CI would get skills installed to.github/skills/instead of.cursor/.Fix
.github/is now only detected as a Copilot target when it contains Copilot-specific markers:.github/copilot-instructions.md.github/skills/.github/agents/.github/prompts/A bare
.github/with only workflows/CODEOWNERS is no longer treated as a Copilot signal.Changes
src/apm_cli/core/target_detection.py— Replace.githubexistence check with Copilot marker detectiontests/unit/core/test_target_detection.py— Update existing tests to use Copilot markers, add new test verifying bare.github/falls through tominimalFixes #805
How Has This Been Tested?
test_auto_detect_bare_github_no_copilot_markersverifies bare.github/workflows/does NOT trigger Copilot detectionChecklist