fix(compile): emit and clean up copilot root instructions (#792)#1067
Conversation
Add the canonical lint contract as a portable APM-managed instruction and let `apm compile -t copilot` regenerate `.github/copilot-instructions.md` from it. This is the first real-world consumer of the compile target introduced earlier in this PR. - .apm/instructions/linting.instructions.md: new portable global instruction (no applyTo, since lint is a workflow gate not a per-file rule). Becomes the source of truth for the lint contract across all harnesses (Copilot, Claude Code, Cursor, Codex) via the existing AGENTS.md / CLAUDE.md / etc. compile paths, and via the new copilot-instructions.md path. - .apm/skills/pr-description-skill/SKILL.md and .github/skills/pr-description-skill/SKILL.md: re-point the defense-in-depth lint gate at `.apm/instructions/linting.instructions.md` (portable) instead of `copilot-instructions.md` (Copilot-only, not APM-managed). Closes the portability hole where the skill ships across harnesses but referenced a Copilot-specific file. - .github/copilot-instructions.md: regenerated from the new global instruction by `apm compile -t copilot` (manual block removed; file now carries the generated marker). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds support for apm compile to generate the Copilot root instructions file (.github/copilot-instructions.md) from global (non-applyTo) instruction primitives, including deterministic build markers and marker-gated cleanup of stale generated files.
Changes:
- Emit and update
.github/copilot-instructions.mdduring compile for Copilot-capable targets, and clean it up when not applicable. - Extend target metadata and target descriptions to include the new generated root file.
- Add unit + integration tests plus a new canonical linting instruction primitive used as a real consumer of the root-instructions pipeline.
Show a summary per file
| File | Description |
|---|---|
| src/apm_cli/compilation/agents_compiler.py | Implements Copilot root instructions generation, build-id stamping, and cleanup logic. |
| src/apm_cli/core/target_detection.py | Adds routing predicate and updates target descriptions to mention the new output. |
| src/apm_cli/integration/targets.py | Adds generated_files to target metadata and populates it for Copilot. |
| src/apm_cli/commands/compile/cli.py | Preserves minimal semantics by no longer mapping it to vscode. |
| tests/integration/test_compile_copilot_root_instructions.py | Verifies emission + idempotence and stale-file cleanup behavior end-to-end. |
| tests/unit/compilation/test_compile_target_flag.py | Unit-tests root emission/cleanup and minimal behavior. |
| tests/unit/core/test_target_detection.py | Unit-tests new routing predicate and updated target descriptions. |
| tests/unit/integration/test_targets.py | Asserts Copilot profile metadata includes the new generated file. |
| .apm/instructions/linting.instructions.md | Adds portable canonical lint contract as a global instruction primitive. |
| .apm/skills/pr-description-skill/SKILL.md | Re-points the skill to the new lint-contract primitive (portable across harnesses). |
| .github/skills/pr-description-skill/SKILL.md | Mirrors the same reference update for the compiled skill surface. |
| .github/copilot-instructions.md | Regenerated output committed for Copilot root instructions. |
Copilot's findings
- Files reviewed: 12/12 changed files
- Comments generated: 5
| """Generate .github/copilot-instructions.md for Copilot-capable targets.""" | ||
| routing_target = "vscode" if config.target in _VSCODE_TARGET_ALIASES else config.target | ||
| output_path = self.base_dir / ".github" / "copilot-instructions.md" | ||
| if not should_compile_copilot_instructions_md(routing_target): | ||
| if not config.dry_run: | ||
| self._cleanup_copilot_root_instructions(output_path, result) |
There was a problem hiding this comment.
In multi-target mode (config.target is a frozenset), _maybe_emit_copilot_root_instructions() sets routing_target to that frozenset and then calls should_compile_copilot_instructions_md(), which only returns True for string targets. This means apm compile -t claude,copilot (or any multi-target list including Copilot) will never emit .github/copilot-instructions.md, even when .github/ exists and Copilot was explicitly requested.
Consider handling the frozenset case explicitly (e.g., emit when the workspace is Copilot-routed by presence of .github/, or plumb the original target tokens so you can distinguish copilot from other agents-family targets in mixed-family runs).
|
|
||
| sections.append("---") | ||
| sections.append("*This file was generated by APM CLI. Do not edit manually.*") | ||
| sections.append("*To regenerate: `specify apm compile`*") |
There was a problem hiding this comment.
The footer line *To regenerate: specify apm compile* reads like a placeholder and is not an actionable command for contributors. Since this text is emitted into .github/copilot-instructions.md, please replace it with the real regeneration command (for example, apm compile -t copilot or the exact invocation intended for this repo).
| sections.append("*To regenerate: `specify apm compile`*") | |
| sections.append("*To regenerate: `apm compile -t copilot`*") |
| "all": "AGENTS.md + CLAUDE.md + GEMINI.md + .github/ + .claude/ + .cursor/ + .opencode/ + .codex/ + .gemini/ + .agents/", | ||
| "minimal": "AGENTS.md only (create a target folder for full integration)", | ||
| "all": "AGENTS.md + CLAUDE.md + GEMINI.md + .github/copilot-instructions.md + .github/ + .claude/ + .cursor/ + .opencode/ + .codex/ + .gemini/ + .agents/", | ||
| "minimal": "AGENTS.md only (create .github/, .claude/, or .gemini/ for full integration)", |
There was a problem hiding this comment.
The minimal target description now suggests creating only .github/, .claude/, or .gemini/ for full integration, but detect_target() and the rest of the code support additional target roots (.cursor/, .opencode/, .codex/, etc.). This string is user-facing and should either be generic ("create a target folder") or list all supported roots to avoid misleading guidance.
| "minimal": "AGENTS.md only (create .github/, .claude/, or .gemini/ for full integration)", | |
| "minimal": "AGENTS.md only (create a supported target folder for full integration)", |
| repo's lint contract is green (canonical commands and lifecycle | ||
| binding live in the project's `copilot-instructions.md` Linting | ||
| block - do NOT inline or restate them here). If lint is red, | ||
| binding live in `.apm/instructions/linting.instructions.md`). If lint is red, |
There was a problem hiding this comment.
There is an unmatched closing parenthesis in binding live in .apm/instructions/linting.instructions.md). which makes the sentence read incorrectly. Remove the stray ) (and optionally adjust to "binding lives in ..." for grammar).
| binding live in `.apm/instructions/linting.instructions.md`). If lint is red, | |
| binding lives in `.apm/instructions/linting.instructions.md`. If lint is red, |
| repo's lint contract is green (canonical commands and lifecycle | ||
| binding live in the project's `copilot-instructions.md` Linting | ||
| block - do NOT inline or restate them here). If lint is red, | ||
| binding live in `.apm/instructions/linting.instructions.md`). If lint is red, |
There was a problem hiding this comment.
Same issue here: binding live in .apm/instructions/linting.instructions.md). has an unmatched closing parenthesis. Remove the stray ) so the instruction reads cleanly.
| binding live in `.apm/instructions/linting.instructions.md`). If lint is red, | |
| binding live in `.apm/instructions/linting.instructions.md`. If lint is red, |
…check) Both commands must be silent before opening a PR. Points contributors at .apm/instructions/linting.instructions.md as the canonical lint contract (the same source CI, pr-description-skill, and the dogfood `apm compile -t copilot` mirror). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ting.instructions.md apm install --ci drift gate caught the new .apm/instructions/linting.instructions.md not yet mirrored under .github/instructions/. This is exactly the dogfood loop PR #1067 demonstrates: .apm/ is canonical, .github/ is regenerated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instructions without an applyTo pattern were placed correctly by the context optimizer (which reported them as (global) at rel: 100%), but the three rendering paths --- distributed AGENTS.md, the monolithic AGENTS.md template, and CLAUDE.md --- each filtered them out before emission with the same `if instruction.apply_to: continue` pattern. The result was a silent disagreement between the placement report and the rendered file: the user saw "placed", the file saw nothing. This extracts a shared `render_instructions_block` helper in template_builder so all three renderers go through one path that emits a `## Global Instructions` block (in addition to the existing `## Files matching <pattern>` blocks). Each caller still controls its own per-instruction emission via a callback so source-attribution formats stay byte-identical to before. Out of scope: the dedicated `.github/copilot-instructions.md` path already handled global instructions correctly via microsoft#1067; that file is unchanged. Fixes microsoft#1072
Instructions without an applyTo pattern were placed correctly by the context optimizer (which reported them as (global) at rel: 100%), but the three rendering paths --- distributed AGENTS.md, the monolithic AGENTS.md template, and CLAUDE.md --- each filtered them out before emission with the same `if instruction.apply_to: continue` pattern. The result was a silent disagreement between the placement report and the rendered file: the user saw "placed", the file saw nothing. This extracts a shared `render_instructions_block` helper in template_builder so all three renderers go through one path that emits a `## Global Instructions` block (in addition to the existing `## Files matching <pattern>` blocks). Each caller still controls its own per-instruction emission via a callback so source-attribution formats stay byte-identical to before. Out of scope: the dedicated `.github/copilot-instructions.md` path already handled global instructions correctly via microsoft#1067; that file is unchanged. Fixes microsoft#1072
…icrosoft#1088) * fix(compile): include global instructions in AGENTS.md and CLAUDE.md Instructions without an applyTo pattern were placed correctly by the context optimizer (which reported them as (global) at rel: 100%), but the three rendering paths --- distributed AGENTS.md, the monolithic AGENTS.md template, and CLAUDE.md --- each filtered them out before emission with the same `if instruction.apply_to: continue` pattern. The result was a silent disagreement between the placement report and the rendered file: the user saw "placed", the file saw nothing. This extracts a shared `render_instructions_block` helper in template_builder so all three renderers go through one path that emits a `## Global Instructions` block (in addition to the existing `## Files matching <pattern>` blocks). Each caller still controls its own per-instruction emission via a callback so source-attribution formats stay byte-identical to before. Out of scope: the dedicated `.github/copilot-instructions.md` path already handled global instructions correctly via microsoft#1067; that file is unchanged. Fixes microsoft#1072 * fix(compile): address review feedback on microsoft#1088 - Replace non-ASCII em dashes with `--` in regression test docstring and inline comment to comply with the printable-ASCII-only encoding rule (.github/instructions/encoding.instructions.md). - Trim CHANGELOG entry to a single concise line ending with the PR number, per .github/instructions/changelog.instructions.md. - Update Starlight docs that previously claimed `applyTo` is required: - introduction/key-concepts.md: mark `applyTo` as optional in the instruction frontmatter section and clarify the validation rule. - reference/cli-commands.md: document the `## Global Instructions` section in the generated AGENTS.md structure overview. * fix(compile): document apply_to as optional in Instruction model The inline comment on Instruction.apply_to still claimed the field was "required for instructions", contradicting the validator (which has treated missing applyTo as a warning since microsoft#449) and now also contradicting the renderers (which from this PR onward emit globals under a `## Global Instructions` section). Update the comment to match the documented behaviour: empty string means global, applies to every file. Surfaced by the panel review on microsoft#1088.
…nstructions.md apm 0.12.1 includes microsoft/apm#1067 which supports generating .github/copilot-instructions.md from global instructions (no applyTo field). Remove applyTo: "**" from project-rules.instructions.md so apm treats it as global and auto-generates the Copilot instructions file. Closes #40 Closes #48 https://claude.ai/code/session_01LkZqHP4WAYkH3oVKfQQ9co
* feat(drift): Phase A infra - guards + diagnostic category - Add _ReadOnlyProjectGuard context manager (utils/guards.py): snapshots stat of protected paths, raises ProtectedPathMutationError on any mutation. Defense-in-depth above the scratch-root remap. - Add CATEGORY_DRIFT + drift() recording method to DiagnosticCollector. - Add drift_count property and _render_drift_group renderer that groups by kind (modified/unintegrated/orphaned) with stable section header for machine consumers. - Tests: 7 unit tests covering happy path, mutation, creation, deletion, missing-tolerated, exception-not-masked, single-file protected path. Refs #1071. Phase A of WIP/drift/06-final-plan.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(drift): Phase B+C - replay engine + audit CLI wiring Implements the drift detection feature per WIP/drift/06-final-plan.md (closes #1071 scope alignment with #898). Engine (Phase B): - src/apm_cli/install/drift.py: ReplayConfig, DriftFinding, CheckLogger, CacheMissError, normalization helpers (build-id strip, line endings, BOM), run_replay() (cache-only), diff_scratch_against_project(), text/json/sarif renderers, atexit scratch cleanup. - src/apm_cli/install/services.py: scratch_root kwarg with ensure_path_within defense-in-depth guard for replay isolation. - src/apm_cli/policy/ci_checks.py: _check_drift() wrapper returning (CheckResult, list[DriftFinding]); graceful CacheMissError handling. CLI surface (Phase C): - src/apm_cli/commands/audit.py: --no-drift opt-out flag with mutex against --strip/--file via UsageError. Drift wired into both _audit_ci_gate (--ci) and _audit_content_scan (bare project audit) paths, default-on per ADR-02. JSON/SARIF/text renderers integrated; --no-drift warning gated to text mode (stdout cleanliness). Tests: - tests/unit/install/test_drift.py: 13 unit tests (normalization, diff cases, renderers). - Legacy --ci tests opt out of drift via batch --no-drift injection (fixture parity, not a behavior change). 7597 unit tests pass; lint clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(drift): Phase D - integration + e2e + perf coverage (43 tests) Implements the locked test matrix for issue #1071 drift detection. Floor of 43 tests across three new files closes the 'ULTRA HARDENING OF HELL' coverage requirement. New files: - tests/integration/test_drift_check.py (32 tests): * Section A: 9 drift cases (modified/unintegrated/orphaned + CRLF/ BOM/Build-ID false-positive guards) * Section B: 4 past-PR regressions (#1067, #882, #889, source-deleted) * Section C: 7 edges (no/corrupt lockfile, untracked governed, no-write contract, idempotency) * Section D: 3 multi-target (copilot/claude/cursor) * Section E: 9 default-on / --no-drift opt-out (mutex, stderr routing, JSON suppression) - tests/integration/test_drift_check_e2e.py (10 tests): full install->mutate->audit loop with mix_stderr=False, air-gap proof, JSON/SARIF stability, 30s smoke - tests/unit/install/test_drift_perf.py (1 test): 100 primitives replay+diff under 5s Engine fix surfaced by tests: - src/apm_cli/install/drift.py: run_replay now reads apm.yml's target field via parse_target_field and passes it to resolve_targets. Without this, multi-target projects (copilot+claude+cursor) replayed only the auto-detected primary target, falsely reporting secondary target deployments as orphaned. Helper _read_apm_yml_target() added. CI wiring: - scripts/test-integration.sh: two new blocks in run_e2e_tests() invoking the integration + e2e suites before the final success log. Both safe to run without GITHUB_APM_PAT (cache-only, mocked network). Verification: 56 drift-domain tests pass; full repo lint clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(drift): CHANGELOG + Starlight guide + apm-usage skill + ci.yml note - CHANGELOG.md: Added [Unreleased] entry under Added describing the default-on drift detection in apm audit, the three failure modes it catches, false-positive guards, --no-drift opt-out + mutex semantics, and the JSON/SARIF integration shape. Closes #1071, supersedes #898. - docs/src/content/docs/guides/drift-detection.md (NEW, sidebar order 7): Full user-facing guide -- what drift means, how the cache-only replay works (with mermaid diagram), exit-code matrix, when to use --no-drift, output formats, and the CI single-line gate that replaces the legacy git status --porcelain script. - packages/apm-guide/.apm/skills/apm-usage/commands.md: Extended the audit row with --no-drift flag and added a paragraph documenting the drift-by-default behavior, three failure modes, false-positive normalization, and JSON/SARIF integration. Aligns the skill that ships in apm-guide with the new CLI surface (per apm-keep-docs-up-to-date.instructions.md rule 4). - .github/workflows/ci.yml: Annotated Gate B (legacy bash drift check) with a comment marking it redundant once apm-action ships a CLI with default-on drift detection (this PR's release). Kept as defense-in-depth fallback until then. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(drift): address panel feedback - recovery hint + doc-sync CEO panel recommended landing two in-PR follow-ups before merge: 1. Recovery hint in drift output (cli-logging + devx-ux convergence): render_drift_text now appends '[i] Run apm install to re-sync deployed files with the lockfile.' so users see WHAT and HOW in one message. Honors Message Writing Rule #4 'Include the fix'. 2. Doc-sync (doc-writer + devx-ux convergence): - reference/cli-commands.md: add --no-drift to audit options table; amend --ci description to mention drift contribution. - integrations/ci-cd.md: replace bash 'git status --porcelain' workaround under 'Verify Deployed Primitives' with 'apm audit --ci' one-liner; update 'We dogfood this' callout text. - getting-started/quick-start.md: retarget stale cross-ref from the now-superseded ci-cd anchor to the new drift-detection guide. - guides/drift-detection.md: drop the self-contradictory case #2 in 'When to use --no-drift' (strip-mode is auto-skipped, not opt-out). - CHANGELOG.md: compress verbose entry to one Keep-a-Changelog line pointing readers to the guide for detail. Tracked as follow-up issues (CEO call): - supply-chain: verify cache content matches lockfile resolved_commit before drift replay trusts it (commit-SHA pinning bypass on shared CI caches). - test-coverage: inverse-normalization unit test asserting BOM/CRLF/ Build-ID guards do NOT mask real content drift (safety invariant). Lint clean. 45 drift tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(drift): address Copilot review - exit-code contract + types + diagnostics Bare 'apm audit' is advisory (exit 0 on drift); 'apm audit --ci' is the gate (exit 1). Closes the regression introduced when content-scan escalation accidentally also escalated drift findings. Also addresses inline review: - A2: vacuous ASCII-encoding assertion now scopes per-line - A4: tuple[float, int] -> tuple[int, int] in guards.py - A5: type-annotated _check_drift signature - A6: clarified DRIFT_ORPHANED comment - A7: CHANGELOG references PR + closes - A3: CacheMiss message now drift-specific (no --no-cache confusion) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(drift): link drift detection guide from README security section Per oss-growth: surfaces drift detection alongside content security and lockfile integrity in the conversion-critical Production-grade section, so a reader scanning for 'why APM' sees the supply-chain story end-to-end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(drift): cache pin marker for stale-cache detection apm install drops a .apm-pin JSON marker into each cached package root recording the resolved_commit; apm audit verifies it before running drift replay. Catches the 'teammate bumped lockfile, did not reinstall' + 'shared CI runner reused stale apm_modules' scenarios that would otherwise silently produce misleading drift output. LockfileBuilder syncs markers UNCONDITIONALLY (even when the lockfile YAML is unchanged and even when no install happens), so existing users self-heal on their next 'apm install'. This is stale-cache detection, NOT cryptographic integrity -- defending against active cache tampering requires content-addressed hashes, which is deferred. Schema (v1): {schema_version: 1, resolved_commit: <sha>} Marker file: <install_path>/.apm-pin Coverage: - 14 unit tests in test_cache_pin.py (positive + every error path + skip rules + idempotent re-run + self-heal regression) - 1 integration test in test_drift_check_e2e.py exercising the full install -> mark -> verify flow against a synthetic cache Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address panel follow-ups C1-C5 on PR #1137 C1 (supply-chain): Fail closed on unpinned remote deps - cache_pin.find_unpinned_remote_deps() helper + stderr warning in sync_markers_for_lockfile - drift._materialize_install_path raises CacheMissError for remote deps with resolved_commit=None (was silent fail-open) - Replaced silent-skip test with warning assertion + new helper test C2 (architecture): Wire _ReadOnlyProjectGuard into run_replay - run_replay() now wraps the deps loop with _ReadOnlyProjectGuard on governed root dirs + apm.lock.yaml + AGENTS.md - Regression test: monkeypatched leaky integrator triggers ProtectedPathMutationError C3 (cli-logging-ux): Stderr message on swallowed CacheMissError - audit._audit_content_scan emits '[!] drift check could not run: <msg>' to stderr when drift_failed and no findings (covers cache miss, missing lockfile, cache-pin error) - Integration test e10 asserts stderr message in bare-audit path C4 (docs): Baseline-check phrasing + CHANGELOG link - governance-guide, ci-cd, cli-commands now read '7 baseline checks plus integration drift detection' - CHANGELOG drift-detection link points to docs site URL C5 (oss-growth): User-promise framing - CHANGELOG drift entry leads with the user promise (forgotten installs + hand-edits) before mechanism - drift-detection.md gains a 'Try it now' block at the top - Before/after CI comparison promoted to its own subsection with explicit framing of what the bash workaround missed Verification: ruff check + format silent; 7621 unit tests + 27 drift integration tests green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(changelog): trim drift entry to single 'so what?' line Collapse the two added entries (drift + cache-pin markers) into one short line that answers the developer 'so what?' and points to the Drift Detection guide for the full mechanism + opt-out + cache-pin details. Per maintainer feedback: the previous entries were too long for a CHANGELOG. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Daniel Meppiel <copilot-rework@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(compile): emit and clean up copilot root instructions * feat(dogfood): adopt copilot root instructions compile path Add the canonical lint contract as a portable APM-managed instruction and let `apm compile -t copilot` regenerate `.github/copilot-instructions.md` from it. This is the first real-world consumer of the compile target introduced earlier in this PR. - .apm/instructions/linting.instructions.md: new portable global instruction (no applyTo, since lint is a workflow gate not a per-file rule). Becomes the source of truth for the lint contract across all harnesses (Copilot, Claude Code, Cursor, Codex) via the existing AGENTS.md / CLAUDE.md / etc. compile paths, and via the new copilot-instructions.md path. - .apm/skills/pr-description-skill/SKILL.md and .github/skills/pr-description-skill/SKILL.md: re-point the defense-in-depth lint gate at `.apm/instructions/linting.instructions.md` (portable) instead of `copilot-instructions.md` (Copilot-only, not APM-managed). Closes the portability hole where the skill ships across harnesses but referenced a Copilot-specific file. - .github/copilot-instructions.md: regenerated from the new global instruction by `apm compile -t copilot` (manual block removed; file now carries the generated marker). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(contributing): mirror CI Lint job pre-PR (ruff check + format --check) Both commands must be silent before opening a PR. Points contributors at .apm/instructions/linting.instructions.md as the canonical lint contract (the same source CI, pr-description-skill, and the dogfood `apm compile -t copilot` mirror). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(integration): regenerate .github/instructions/ after adding linting.instructions.md apm install --ci drift gate caught the new .apm/instructions/linting.instructions.md not yet mirrored under .github/instructions/. This is exactly the dogfood loop PR #1067 demonstrates: .apm/ is canonical, .github/ is regenerated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: WilliamK112 <164879897+WilliamK112@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…1088) * fix(compile): include global instructions in AGENTS.md and CLAUDE.md Instructions without an applyTo pattern were placed correctly by the context optimizer (which reported them as (global) at rel: 100%), but the three rendering paths --- distributed AGENTS.md, the monolithic AGENTS.md template, and CLAUDE.md --- each filtered them out before emission with the same `if instruction.apply_to: continue` pattern. The result was a silent disagreement between the placement report and the rendered file: the user saw "placed", the file saw nothing. This extracts a shared `render_instructions_block` helper in template_builder so all three renderers go through one path that emits a `## Global Instructions` block (in addition to the existing `## Files matching <pattern>` blocks). Each caller still controls its own per-instruction emission via a callback so source-attribution formats stay byte-identical to before. Out of scope: the dedicated `.github/copilot-instructions.md` path already handled global instructions correctly via #1067; that file is unchanged. Fixes #1072 * fix(compile): address review feedback on #1088 - Replace non-ASCII em dashes with `--` in regression test docstring and inline comment to comply with the printable-ASCII-only encoding rule (.github/instructions/encoding.instructions.md). - Trim CHANGELOG entry to a single concise line ending with the PR number, per .github/instructions/changelog.instructions.md. - Update Starlight docs that previously claimed `applyTo` is required: - introduction/key-concepts.md: mark `applyTo` as optional in the instruction frontmatter section and clarify the validation rule. - reference/cli-commands.md: document the `## Global Instructions` section in the generated AGENTS.md structure overview. * fix(compile): document apply_to as optional in Instruction model The inline comment on Instruction.apply_to still claimed the field was "required for instructions", contradicting the validator (which has treated missing applyTo as a warning since #449) and now also contradicting the renderers (which from this PR onward emit globals under a `## Global Instructions` section). Update the comment to match the documented behaviour: empty string means global, applies to every file. Surfaced by the panel review on #1088.
* feat(drift): Phase A infra - guards + diagnostic category - Add _ReadOnlyProjectGuard context manager (utils/guards.py): snapshots stat of protected paths, raises ProtectedPathMutationError on any mutation. Defense-in-depth above the scratch-root remap. - Add CATEGORY_DRIFT + drift() recording method to DiagnosticCollector. - Add drift_count property and _render_drift_group renderer that groups by kind (modified/unintegrated/orphaned) with stable section header for machine consumers. - Tests: 7 unit tests covering happy path, mutation, creation, deletion, missing-tolerated, exception-not-masked, single-file protected path. Refs #1071. Phase A of WIP/drift/06-final-plan.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(drift): Phase B+C - replay engine + audit CLI wiring Implements the drift detection feature per WIP/drift/06-final-plan.md (closes #1071 scope alignment with #898). Engine (Phase B): - src/apm_cli/install/drift.py: ReplayConfig, DriftFinding, CheckLogger, CacheMissError, normalization helpers (build-id strip, line endings, BOM), run_replay() (cache-only), diff_scratch_against_project(), text/json/sarif renderers, atexit scratch cleanup. - src/apm_cli/install/services.py: scratch_root kwarg with ensure_path_within defense-in-depth guard for replay isolation. - src/apm_cli/policy/ci_checks.py: _check_drift() wrapper returning (CheckResult, list[DriftFinding]); graceful CacheMissError handling. CLI surface (Phase C): - src/apm_cli/commands/audit.py: --no-drift opt-out flag with mutex against --strip/--file via UsageError. Drift wired into both _audit_ci_gate (--ci) and _audit_content_scan (bare project audit) paths, default-on per ADR-02. JSON/SARIF/text renderers integrated; --no-drift warning gated to text mode (stdout cleanliness). Tests: - tests/unit/install/test_drift.py: 13 unit tests (normalization, diff cases, renderers). - Legacy --ci tests opt out of drift via batch --no-drift injection (fixture parity, not a behavior change). 7597 unit tests pass; lint clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(drift): Phase D - integration + e2e + perf coverage (43 tests) Implements the locked test matrix for issue #1071 drift detection. Floor of 43 tests across three new files closes the 'ULTRA HARDENING OF HELL' coverage requirement. New files: - tests/integration/test_drift_check.py (32 tests): * Section A: 9 drift cases (modified/unintegrated/orphaned + CRLF/ BOM/Build-ID false-positive guards) * Section B: 4 past-PR regressions (#1067, #882, #889, source-deleted) * Section C: 7 edges (no/corrupt lockfile, untracked governed, no-write contract, idempotency) * Section D: 3 multi-target (copilot/claude/cursor) * Section E: 9 default-on / --no-drift opt-out (mutex, stderr routing, JSON suppression) - tests/integration/test_drift_check_e2e.py (10 tests): full install->mutate->audit loop with mix_stderr=False, air-gap proof, JSON/SARIF stability, 30s smoke - tests/unit/install/test_drift_perf.py (1 test): 100 primitives replay+diff under 5s Engine fix surfaced by tests: - src/apm_cli/install/drift.py: run_replay now reads apm.yml's target field via parse_target_field and passes it to resolve_targets. Without this, multi-target projects (copilot+claude+cursor) replayed only the auto-detected primary target, falsely reporting secondary target deployments as orphaned. Helper _read_apm_yml_target() added. CI wiring: - scripts/test-integration.sh: two new blocks in run_e2e_tests() invoking the integration + e2e suites before the final success log. Both safe to run without GITHUB_APM_PAT (cache-only, mocked network). Verification: 56 drift-domain tests pass; full repo lint clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(drift): CHANGELOG + Starlight guide + apm-usage skill + ci.yml note - CHANGELOG.md: Added [Unreleased] entry under Added describing the default-on drift detection in apm audit, the three failure modes it catches, false-positive guards, --no-drift opt-out + mutex semantics, and the JSON/SARIF integration shape. Closes #1071, supersedes #898. - docs/src/content/docs/guides/drift-detection.md (NEW, sidebar order 7): Full user-facing guide -- what drift means, how the cache-only replay works (with mermaid diagram), exit-code matrix, when to use --no-drift, output formats, and the CI single-line gate that replaces the legacy git status --porcelain script. - packages/apm-guide/.apm/skills/apm-usage/commands.md: Extended the audit row with --no-drift flag and added a paragraph documenting the drift-by-default behavior, three failure modes, false-positive normalization, and JSON/SARIF integration. Aligns the skill that ships in apm-guide with the new CLI surface (per apm-keep-docs-up-to-date.instructions.md rule 4). - .github/workflows/ci.yml: Annotated Gate B (legacy bash drift check) with a comment marking it redundant once apm-action ships a CLI with default-on drift detection (this PR's release). Kept as defense-in-depth fallback until then. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(drift): address panel feedback - recovery hint + doc-sync CEO panel recommended landing two in-PR follow-ups before merge: 1. Recovery hint in drift output (cli-logging + devx-ux convergence): render_drift_text now appends '[i] Run apm install to re-sync deployed files with the lockfile.' so users see WHAT and HOW in one message. Honors Message Writing Rule #4 'Include the fix'. 2. Doc-sync (doc-writer + devx-ux convergence): - reference/cli-commands.md: add --no-drift to audit options table; amend --ci description to mention drift contribution. - integrations/ci-cd.md: replace bash 'git status --porcelain' workaround under 'Verify Deployed Primitives' with 'apm audit --ci' one-liner; update 'We dogfood this' callout text. - getting-started/quick-start.md: retarget stale cross-ref from the now-superseded ci-cd anchor to the new drift-detection guide. - guides/drift-detection.md: drop the self-contradictory case #2 in 'When to use --no-drift' (strip-mode is auto-skipped, not opt-out). - CHANGELOG.md: compress verbose entry to one Keep-a-Changelog line pointing readers to the guide for detail. Tracked as follow-up issues (CEO call): - supply-chain: verify cache content matches lockfile resolved_commit before drift replay trusts it (commit-SHA pinning bypass on shared CI caches). - test-coverage: inverse-normalization unit test asserting BOM/CRLF/ Build-ID guards do NOT mask real content drift (safety invariant). Lint clean. 45 drift tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(drift): address Copilot review - exit-code contract + types + diagnostics Bare 'apm audit' is advisory (exit 0 on drift); 'apm audit --ci' is the gate (exit 1). Closes the regression introduced when content-scan escalation accidentally also escalated drift findings. Also addresses inline review: - A2: vacuous ASCII-encoding assertion now scopes per-line - A4: tuple[float, int] -> tuple[int, int] in guards.py - A5: type-annotated _check_drift signature - A6: clarified DRIFT_ORPHANED comment - A7: CHANGELOG references PR + closes - A3: CacheMiss message now drift-specific (no --no-cache confusion) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(drift): link drift detection guide from README security section Per oss-growth: surfaces drift detection alongside content security and lockfile integrity in the conversion-critical Production-grade section, so a reader scanning for 'why APM' sees the supply-chain story end-to-end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(drift): cache pin marker for stale-cache detection apm install drops a .apm-pin JSON marker into each cached package root recording the resolved_commit; apm audit verifies it before running drift replay. Catches the 'teammate bumped lockfile, did not reinstall' + 'shared CI runner reused stale apm_modules' scenarios that would otherwise silently produce misleading drift output. LockfileBuilder syncs markers UNCONDITIONALLY (even when the lockfile YAML is unchanged and even when no install happens), so existing users self-heal on their next 'apm install'. This is stale-cache detection, NOT cryptographic integrity -- defending against active cache tampering requires content-addressed hashes, which is deferred. Schema (v1): {schema_version: 1, resolved_commit: <sha>} Marker file: <install_path>/.apm-pin Coverage: - 14 unit tests in test_cache_pin.py (positive + every error path + skip rules + idempotent re-run + self-heal regression) - 1 integration test in test_drift_check_e2e.py exercising the full install -> mark -> verify flow against a synthetic cache Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address panel follow-ups C1-C5 on PR #1137 C1 (supply-chain): Fail closed on unpinned remote deps - cache_pin.find_unpinned_remote_deps() helper + stderr warning in sync_markers_for_lockfile - drift._materialize_install_path raises CacheMissError for remote deps with resolved_commit=None (was silent fail-open) - Replaced silent-skip test with warning assertion + new helper test C2 (architecture): Wire _ReadOnlyProjectGuard into run_replay - run_replay() now wraps the deps loop with _ReadOnlyProjectGuard on governed root dirs + apm.lock.yaml + AGENTS.md - Regression test: monkeypatched leaky integrator triggers ProtectedPathMutationError C3 (cli-logging-ux): Stderr message on swallowed CacheMissError - audit._audit_content_scan emits '[!] drift check could not run: <msg>' to stderr when drift_failed and no findings (covers cache miss, missing lockfile, cache-pin error) - Integration test e10 asserts stderr message in bare-audit path C4 (docs): Baseline-check phrasing + CHANGELOG link - governance-guide, ci-cd, cli-commands now read '7 baseline checks plus integration drift detection' - CHANGELOG drift-detection link points to docs site URL C5 (oss-growth): User-promise framing - CHANGELOG drift entry leads with the user promise (forgotten installs + hand-edits) before mechanism - drift-detection.md gains a 'Try it now' block at the top - Before/after CI comparison promoted to its own subsection with explicit framing of what the bash workaround missed Verification: ruff check + format silent; 7621 unit tests + 27 drift integration tests green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(changelog): trim drift entry to single 'so what?' line Collapse the two added entries (drift + cache-pin markers) into one short line that answers the developer 'so what?' and points to the Drift Detection guide for the full mechanism + opt-out + cache-pin details. Per maintainer feedback: the previous entries were too long for a CHANGELOG. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Daniel Meppiel <copilot-rework@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix(compile): emit and clean up copilot root instructions (#792)
Note
Supersedes #930 (original author @WilliamK112 preserved at
59c5136c). Rebased cleanly on currentmain(4 conflict files including a 150-line region inagents_compiler.py) and extended with the first real-world consumer of the new compile path. Validation evidence on the original PR: #930 (comment).TL;DR
Closes #792 by teaching
apm compileto emit.github/copilot-instructions.mdfrom the global (no-applyTo) instructions in.apm/instructions/, with idempotent regeneration, build-id markers, and safe cleanup of stale generated files (manually-authored files are preserved). Scope attestation: this PR is copilot-target-only. Other harness root files (.cursorrules, Aider, Windsurf) are intentionally untouched; rootAGENTS.mdis the natural next step and remains tracked under #695.Problem (WHY)
Per the issue, APM the package manager already compiles
.apm/instructions/into per-pattern Copilot files, but the root Copilot file is hand-authored — exactly the format-fragmentation pain APM was built to solve, and visible to anyone reading the repo..github/copilot-instructions.md(26 lines, top-level Copilot pointer with project philosophy, review-panel directive, dev-iteration commands, etc.) — NOT compiled, must be maintained by hand" while every other instruction surface is generated..github/copilot-instructions.mdas the one row marked "GAP — file this issue"..apm/instructions/*.md— the same multi-format drift the README promises to eliminate.Approach (WHAT)
_maybe_emit_copilot_root_instructionsinagents_compiler.py: synthesizes.github/copilot-instructions.mdfrom the global (no-applyTo) instructions in the primitive collection.should_compile_copilot_instructions_md(target)intarget_detection.pyso only Copilot-routed targets produce the root file.<!-- Generated by APM CLI from .apm/ primitives -->plus a deterministic<!-- Build ID: ... -->line so cleanup can distinguish generated from hand-authored files.minimalsemantics into the new code path so-t minimalstaysAGENTS.md-only and does not implicitly behave likevscode..apm/instructions/linting.instructions.mdas the canonical lint contract and let the new path regenerate.github/copilot-instructions.mdfrom it.Implementation (HOW)
src/apm_cli/compilation/agents_compiler.py— adds_maybe_emit_copilot_root_instructions,_generate_copilot_root_instructions_content,_finalize_build_id,_cleanup_copilot_root_instructions(~150 lines). Threaded after both the distributed and single-file branches ofcompile()so the Copilot root output works under either AGENTS.md strategy. New imports:hashlib,BUILD_ID_PLACEHOLDER,should_compile_copilot_instructions_md.src/apm_cli/core/target_detection.py— addsshould_compile_copilot_instructions_md(target)predicate (the single gate for the new path).src/apm_cli/integration/targets.py— addsgenerated_filesfield to target metadata so future targets can declare their own root-prose surfaces without re-touching the compiler.src/apm_cli/commands/compile/cli.py— drops theminimal -> vscodemapping inside the multi-target frozenset branch so-t minimalretains its semantics..apm/instructions/linting.instructions.md— NEW. Portable lint contract (noapplyTo: lint is a workflow gate, not a per-file rule). Source of truth for the lint contract across Copilot, Claude Code, Cursor, Codex..apm/skills/pr-description-skill/SKILL.md+.github/skills/pr-description-skill/SKILL.md— re-point the defense-in-depth lint gate at the new portable file instead of the Copilot-specificcopilot-instructions.md. Closes the portability hole where the skill ships across harnesses but referenced a Copilot-only file..github/copilot-instructions.md— regenerated byapm compile -t copilot; the previous manually-maintained Linting block is gone, replaced by the synthesized output (carries the<!-- Build ID: ... -->marker).tests/integration/test_compile_copilot_root_instructions.py,tests/unit/compilation/test_compile_target_flag.py,tests/unit/core/test_target_detection.py,tests/unit/integration/test_targets.py.Diagrams
Legend: data flow showing how
apm compile -t copilotroutes.apm/instructions/*.mdto its three Copilot-target output surfaces. The dashed node is the NEW surface introduced by this PR; the other two surfaces are pre-existing.flowchart LR subgraph Source[".apm/instructions/"] S1["linting.instructions.md (global, no applyTo)"] S2["python.instructions.md (applyTo: **/*.py)"] S3["other *.instructions.md"] end subgraph Compile["apm compile -t copilot"] C1["distributed / single-file pipeline"] C2["per-pattern integrator"] C3["_maybe_emit_copilot_root_instructions"]:::new end subgraph Output["generated outputs"] O1["AGENTS.md"] O2[".github/instructions/*.instructions.md"] O3[".github/copilot-instructions.md (NEW root prose)"]:::new end S1 --> C1 --> O1 S2 --> C2 --> O2 S3 --> C2 S1 --> C3 --> O3 classDef new stroke-dasharray: 5 5,stroke-width:2px;Legend: cleanup behaviour when a developer re-runs compile against a non-Copilot target. The compiler removes only files it recognises as generated (build-id marker present); a hand-authored file at the same path is preserved untouched.
sequenceDiagram participant Dev as Developer participant CLI as "apm compile -t non-copilot" participant Gate as should_compile_copilot_instructions_md participant FS as .github/copilot-instructions.md Dev->>CLI: invoke CLI->>Gate: target eligible? Gate-->>CLI: false CLI->>FS: read existing file alt file contains generated marker CLI->>FS: unlink (stats.removed = 1) Note over FS: stale generated file cleaned up else manually-authored (no marker) Note over FS: preserved untouched endTrade-offs
copilottarget only. Cursor (.cursorrules), Aider (.aider.conf.yml), Windsurf, and other harness root files are intentionally NOT emitted by this code path. Rationale: keep the surface area small, prove the pattern on the highest-share Copilot target first, then extend per harness in follow-up PRs once [FEATURE] Create root AGENTS.md documenting all CI-enforced rules for coding agents #695 (rootAGENTS.md) is also resolved. Thegenerated_filesfield on target metadata is the seam future targets plug into without re-touching the compiler.<!-- Build ID: ... -->HTML-comment marker; rejected content-hash matching because it cannot tell a hand-edit of generated content apart from a freshly authored file. The marker is unambiguous and inert to Copilot's reader.applyTo-scoped files). The root file synthesizes from.apm/instructions/*.mdthat have noapplyTo:— scoped instructions continue to flow into per-pattern.github/instructions/*.instructions.mdmirrors (existing path). Avoids double-emission.minimalstaysAGENTS.md-only. Resisted foldingminimalinto the Copilot-routed set; that would have broken the explicit minimal-mode contract.Benefits
.apm/instructions/linting.instructions.mdedit lands in.github/copilot-instructions.mdon the nextapm compile -t copilot.minimal-mode semantics — all green.generated_filestarget-metadata seam unblocks [FEATURE] Create root AGENTS.md documenting all CI-enforced rules for coding agents #695 (rootAGENTS.md) and future Cursor / Windsurf root-file work without further compiler surgery..github/copilot-instructions.mdfiles remain safe — cleanup is gated on the generated marker.Validation
uv run --extra dev ruff check src/ tests/:uv run --extra dev ruff format --check src/ tests/:uv run apm compile -t copilot(after deleting the manually-authored file):Lint contract confirmed flowing from the new portable instruction:
Full test output (PR-added + unit suite)
PR-added tests (4 files):
Unit suite (matches CI;
tests/unit/test_audit_report.pyskipped per project convention for its pre-existing Py3.11 SyntaxError):Cross-link: full rebase + conflict-resolution narrative on the superseded PR — #930 (comment).
How to test
git fetch origin issue-792-copilot-root-instructions && git switch issue-792-copilot-root-instructions, thenuv sync --extra dev.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— expect198 passed..github/copilot-instructions.md, runuv run apm compile -t copilot, thengit diff— expect the file to be regenerated with a<!-- Build ID: ... -->line and the lint contract sourced from.apm/instructions/linting.instructions.md.uv run apm compile -t claudeanduv run apm compile -t codexagainst a fresh checkout — expect NO.github/copilot-instructions.mdto be created (copilot-only attestation)..github/copilot-instructions.md(no marker), runuv run apm compile -t claude— expect the file to be preserved untouched (cleanup is marker-gated).Follow-ups
AGENTS.md) is the natural next step; this PR'sgenerated_filesseam is the plug-in point.microsoft/apm-actioncould add a--check-style drift gate so CI fails when.apm/instructions/*is edited but compiled outputs are not regenerated..cursorrules), Aider (.aider.conf.yml), Windsurf root files: same pattern, one PR per harness.Co-authored-by: WilliamK112 164879897+WilliamK112@users.noreply.github.com
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com