Skip to content

fix(compile): emit and clean up copilot root instructions#930

Closed
WilliamK112 wants to merge 1 commit intomicrosoft:mainfrom
WilliamK112:issue-792-copilot-root-instructions
Closed

fix(compile): emit and clean up copilot root instructions#930
WilliamK112 wants to merge 1 commit intomicrosoft:mainfrom
WilliamK112:issue-792-copilot-root-instructions

Conversation

@WilliamK112
Copy link
Copy Markdown
Contributor

Fixes #792.

Summary

  • generate .github/copilot-instructions.md from global .apm/instructions/*.md content during apm compile
  • preserve minimal semantics so it stays AGENTS.md-only instead of implicitly behaving like vscode
  • track Copilot root instructions in target metadata and target descriptions
  • clean up stale generated root instructions when switching away from Copilot output or when only scoped applyTo rules remain
  • preserve manually authored .github/copilot-instructions.md files during cleanup

Tests

  • PYTHONPATH=src uv run pytest tests/unit/compilation/test_compile_target_flag.py tests/integration/test_compile_copilot_root_instructions.py tests/unit/core/test_target_detection.py tests/unit/integration/test_targets.py -x
  • PYTHONPATH=src uv run pytest tests/unit tests/test_console.py -x

Copilot AI review requested due to automatic review settings April 25, 2026 06:37
@WilliamK112
Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends APM's compilation pipeline to generate GitHub Copilot root instructions (.github/copilot-instructions.md) from global .apm/instructions/*.instructions.md content, while preserving the minimal target's AGENTS.md-only behavior and cleaning up stale generated root instructions when they no longer apply.

Changes:

  • Emit .github/copilot-instructions.md during apm compile (for Copilot-capable targets) and remove stale generated copies when switching targets or when only scoped applyTo rules remain.
  • Preserve minimal semantics by no longer remapping minimal to vscode in the compile CLI.
  • Add target metadata/description updates and comprehensive unit + integration test coverage for the new behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/apm_cli/compilation/agents_compiler.py Implements generation + cleanup logic for Copilot root instructions and tracks related stats.
src/apm_cli/commands/compile/cli.py Preserves minimal as a first-class compilation target instead of mapping it to vscode.
src/apm_cli/core/target_detection.py Adds routing helper for Copilot root instructions and updates target descriptions.
src/apm_cli/integration/targets.py Tracks Copilot root generated file in target metadata (generated_files).
tests/unit/compilation/test_compile_target_flag.py Unit tests for emitting/removing/preserving Copilot root instructions across targets and scenarios.
tests/integration/test_compile_copilot_root_instructions.py CLI-level coverage for emission + idempotency + stale cleanup behavior.
tests/unit/core/test_target_detection.py Tests new target-routing helper and updated target descriptions.
tests/unit/integration/test_targets.py Ensures Copilot target profile lists the root generated file.

Comment on lines +398 to +400
# Keep the detected target intact so the compiler can preserve
# minimal-mode semantics (AGENTS.md only, no .github side outputs).
effective_target = detected_target
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

Now that effective_target can remain minimal, make sure every subsequent compile pass in this command uses the same target. In the legacy --single-agents path, an intermediate CompilationConfig(...) is constructed without target=... (so it defaults to all), which can unexpectedly route through CLAUDE compilation and potentially alter what gets injected/written to AGENTS.md. Propagating effective_target (or avoiding the second compile) would keep minimal-mode semantics intact.

Copilot uses AI. Check for mistakes.
Comment on lines +819 to +823
result.stats["copilot_root_instructions_unchanged"] = 1
return result

output_path.write_text(content, encoding="utf-8")
result.stats["copilot_root_instructions_written"] = 1
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

The write path for .github/copilot-instructions.md will overwrite an existing manually-authored file whenever global instructions exist (the only preservation logic is in cleanup, not generation). This risks clobbering user content and contradicts the repo/docs expectation that existing config files are not modified. Consider only writing when the file is missing or already marked as APM-generated (marker present); otherwise, warn and skip (or require an explicit force flag).

Copilot uses AI. Check for mistakes.
Comment on lines +835 to +839
def _generate_copilot_root_instructions_content(
self,
instructions,
config: CompilationConfig,
) -> str:
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

_generate_copilot_root_instructions_content() adds new public-ish helper logic but leaves the instructions parameter untyped. Please add an explicit type (e.g., a Sequence/List of Instruction primitives) to match the codebase's type-hinting guideline for new/changed code.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines 229 to 231
descriptions = {
"vscode": "AGENTS.md + .github/prompts/ + .github/agents/",
"vscode": "AGENTS.md + .github/copilot-instructions.md + .github/prompts/ + .github/agents/",
"claude": "CLAUDE.md + .claude/commands/ + .claude/agents/ + .claude/skills/",
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

get_target_description() is used by apm compile logging, but the vscode/all descriptions currently list .github/prompts/ and .github/agents/ as generated outputs. Those are installed/deployed by apm install integrators, not produced by apm compile (agents_compiler only emits AGENTS.md/CLAUDE.md and now copilot-instructions.md). Please adjust the description strings (or wording) so the compile command doesn't claim it is generating install-time directories.

Copilot uses AI. Check for mistakes.
Comment on lines +204 to +208
def should_compile_copilot_instructions_md(target: TargetType) -> bool:
"""Check if .github/copilot-instructions.md should be compiled.

Args:
target: The detected or configured target
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

This PR changes apm compile behavior by generating (and sometimes removing) .github/copilot-instructions.md. Per the docs-update rule, please update the Starlight docs (notably docs/src/content/docs/guides/compilation.md and the CLI reference) and the apm-guide skill resources (packages/apm-guide/.apm/skills/apm-usage/commands.md) to reflect the new output file and its cleanup/overwrite semantics.

Copilot generated this review using guidance from repository custom instructions.
@danielmeppiel
Copy link
Copy Markdown
Collaborator

Rebase + validation + dogfood for #930

Rebased the PR on top of ee410623 (current origin/main) and folded in the first real-world consumer of the new compile target. Branch pushed to microsoft/apm:issue-792-copilot-root-instructions (couldn't --force-with-lease directly to WilliamK112:issue-792-copilot-root-instructions since that's a fork; maintainer can re-target this PR or open a fresh one off the upstream branch).

Two commits on the branch:

  • 59c5136c -- the original fix(compile): emit and clean up copilot root instructions (preserved authorship: WilliamK112) re-applied on top of main with conflict resolutions.
  • 3f46ad88 -- feat(dogfood): adopt copilot root instructions compile path -- the demo content that exercises the new path.

Conflict resolution summary

Four files conflicted. All resolutions preserved BOTH sides' intent:

  • src/apm_cli/commands/compile/cli.py (1 region near L408): main added explicit-frozenset handling for multi-target --target a,b flags. PR removed the minimal -> vscode mapping so minimal-mode semantics survive into the compiler. Resolution kept main's frozenset branch structure and dropped the mapping inside the else branch (PR's intent), so effective_target = detected_target is preserved.
  • src/apm_cli/compilation/agents_compiler.py (5 regions): main introduced logging/_logger and template_builder imports; PR introduced hashlib, should_compile_copilot_instructions_md, BUILD_ID_PLACEHOLDER. Both sets of imports kept. The mid-file conflict around compile() reshape kept main's result = assignments and threaded the new _maybe_emit_copilot_root_instructions(...) call after both the distributed and single-file branches so Copilot root output works under both strategies. The 150-line PR-only block defining _maybe_emit_copilot_root_instructions, _generate_copilot_root_instructions_content, _finalize_build_id, _cleanup_copilot_root_instructions was preserved verbatim.
  • src/apm_cli/integration/targets.py (1 region near L99): main added three fields (user_root_resolver, resolved_deploy_root, requires_flag); PR added generated_files. Resolution kept all four fields with their docstrings.
  • tests/unit/core/test_target_detection.py (1 region near L3): PR's import block already covered by main's existing import block; only should_compile_copilot_instructions_md needed adding. Single coherent import group restored.

Test results

PR-added tests (4 files):

198 passed in 2.71s

Full unit suite (tests/unit tests/test_console.py, ignoring the pre-existing test_audit_report.py Py3.11 SyntaxError):

6848 passed, 1 warning, 30 subtests passed in 40.24s

Lint

$ uv run --extra dev ruff check src/ tests/
All checks passed!
$ uv run --extra dev ruff format --check src/ tests/
604 files already formatted

Both silent.

Dogfood proof

Deleted the manually-authored .github/copilot-instructions.md and ran the new compile target:

$ uv run apm compile -t copilot
[+] Compilation completed successfully!
[!]   .apm/instructions/linting.instructions.md: No 'applyTo' pattern specified -- instruction will apply globally

The "no applyTo" warning is the intended path: the new compile logic picks up only global instructions (no applyTo) for the Copilot root file. Verified the file was regenerated:

$ head -6 .github/copilot-instructions.md
<!-- Generated by APM CLI from .apm/ primitives -->
<!-- Build ID: 3a5991d7ca52 -->
<!-- APM Version: 0.11.0 -->

<!-- Source: .apm/instructions/linting.instructions.md -->
# Linting (canonical contract)

Lint contract content flowing from the new portable instruction confirmed:

$ grep -A 2 "CI .Lint. job runs:" .github/copilot-instructions.md
The CI `Lint` job runs:

- `uv run --extra dev ruff check src/ tests/`
- `uv run --extra dev ruff format --check src/ tests/`

Companion content

This branch also folds in the canonical .apm/instructions/linting.instructions.md, which is the first real-world consumer of the new compile target. The same instruction is now referenced by pr-description-skill's defense-in-depth gate (both .apm/skills/pr-description-skill/SKILL.md and .github/skills/pr-description-skill/SKILL.md), replacing the previous Copilot-only copilot-instructions.md reference. Net effect: the lint contract is now portable across Copilot, Claude Code, Cursor, and Codex via the .apm/ source-of-truth, and any harness with a compile target re-derives its harness-specific surface from it.

Note on the source of linting.instructions.md: the version landed here drops the applyTo: "**/*.py" line that existed in the draft from #1061's worktree, because the lint contract is a workflow gate (lifecycle binding for skills that produce green-CI claims), not a per-file rule. Dropping applyTo makes it qualify as a global instruction so the new Copilot root path picks it up. This is semantically correct for a workflow gate.

Follow-up (out of scope here)

microsoft/apm-action already exposes compile: true. A subsequent PR on microsoft/apm-action could add a --check-style drift gate so CI fails when .apm/instructions/* is edited but compiled outputs (.github/copilot-instructions.md, AGENTS.md, etc.) are not regenerated. Worth tracking as a separate issue once this lands.

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Superseded by #1067. The work in this PR has been rebased onto current main (4 conflict files including a 150-line region in agents_compiler.py resolved cleanly, original author commit preserved at 59c5136) and extended with a real-world dogfood demo (.apm/instructions/linting.instructions.md). Original author @WilliamK112 credited in the new PR body and Co-authored-by trailer. Validation evidence: see #930 (comment) and the new PR body.

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.

apm compile should emit .github/copilot-instructions.md (dogfood gap)

3 participants