fix(compile): remove stale APM-generated CLAUDE.md on --clean when .claude/rules/ is populated (#1747)

* fix(compile): remove stale APM-generated CLAUDE.md on --clean when .claude/rules/ is populated (#1729)

Follow-up to #1138: that PR prevented creating a new CLAUDE.md when
.claude/rules/ already contained instructions, but never removed a
pre-existing APM-generated CLAUDE.md. The duplicate-context problem
#1138 set out to eliminate was still present for any project that
compiled before .claude/rules/ was populated.

Changes:
- agents_compiler.py: in the `files_written==0 and skip_instructions`
  branch, inspect the root CLAUDE.md if it exists. When --clean
  (clean_orphaned=True) is set and the file carries the APM-generated
  marker (CLAUDE_HEADER), remove it and log the removal. When the
  marker is absent (hand-authored file), emit a warning instead of
  deleting. Plain `apm compile` (no --clean) remains strictly
  non-destructive.
- claude_formatter.py: promote CLAUDE_HEADER from a cosmetic/opt-in
  comment to a functional marker always written into generated CLAUDE.md
  files, matching the always-present _COPILOT_ROOT_GENERATED_MARKER
  pattern used by the Copilot path. This makes CLAUDE_HEADER available
  as the orphan-detection gate regardless of --source-attribution.
- test_token_overhead_opt_in.py: update the single test that asserted
  CLAUDE_HEADER was absent by default; document the promotion from
  cosmetic to functional marker.
- test_stale_claude_md_cleanup_1729.py: new test file with 10 tests
  covering all acceptance criteria from issue #1729.
- CHANGELOG.md: add ### Fixed entry under ## [Unreleased].

* fix(compile): review-panel follow-up for #1729 stale CLAUDE.md cleanup

- Gate hand-authored warning on clean_orphaned=True so plain `apm compile` is
  silent when a hand-authored CLAUDE.md coexists with .claude/rules/ (items 1, 6)
- Add dry-run preview line '(would remove stale CLAUDE.md ...)' when
  clean_orphaned=True so --dry-run --clean is self-explanatory (item 2)
- Use portable_relpath in both OS-error warnings for consistency with rest
  of file (item 4)
- Fix stale docstring in _generate_claude_content: CLAUDE_HEADER is always
  emitted as a functional marker regardless of source_attribution (item 5)
- Append adopt-via-prepend escape hatch to hand-authored CLAUDE.md warning,
  mirroring the Copilot-root equivalent (item 6)
- Update --clean docs in reference/cli/compile.md and producer/compile.md to
  mention stale CLAUDE.md removal and hand-authored file protection (item 3)
- Add tests: plain compile + hand-authored emits no warning; dry-run clean
  leaves file on disk; dry-run clean preview mentions removal; dry-run no-clean
  does not mention removal

* fix(compile): render dry-run stale-CLAUDE.md preview + align logging (#1729 review round 2)

Addresses the second-round review-panel findings on the stale-CLAUDE.md
cleanup:

- BLOCKING (cli-logging): the --dry-run --clean stale-removal preview was
  built into result.content but never reached the terminal on the
  distributed claude path (cli.py does `pass` on dry-run success). Emit the
  preview through self._log so the user actually sees it. New test exercises
  the real rendering path via a mock logger.
- Deletion-success message now logs at "success" level (green) instead of
  progress+symbol=success (rendered blue), matching the traffic-light rule
  for a destructive filesystem action.
- Extract a single `would_emit_no_claude_md` boolean used symmetrically by
  the dry-run preview and live-removal paths, replacing the implicit
  dual-guard (content_map emptiness vs files_written==0) and removing the
  dead `not config.dry_run` clause. The preview now correctly does NOT fire
  for a project that emits a CLAUDE.md (e.g. with a constitution).
- Update the apm-guide skill resource (packages/apm-guide/.apm/skills/
  apm-usage/commands.md) to document the --clean stale-CLAUDE.md removal.
- Add a CLI-runner integration test exercising the full dispatch ->
  clean_orphaned -> _compile_claude_md chain.
- Clarify the source_attribution docstring; use portable_relpath in the
  preview line.

* fix(compile): surface CLAUDE.md --clean behavior in --help, clarify warnings (#1729 review round 3)

- --clean --help text and compile docstring now document the stale
  CLAUDE.md removal for --target claude (discoverability for a
  file-deleting flag).
- Hand-authored warning now says 'to the top of the file' and makes the
  removal outcome explicit (mark as APM-managed, then --clean removes it).
- Dry-run preview uses the [dry-run] prefix convention.
- Note the intentional success() default symbol.

* refactor(agents_compiler): address PR #1747 review comments E/F/G

- (E) Add autouse _reset_constitution_cache fixture in test module so
  the module-level constitution cache is cleared before and after every
  test, preventing cross-test state leaks.

- (F) In the dry-run stale-CLAUDE.md preview block, surface an OSError
  as a warning in all_warnings instead of silently discarding it, so
  --dry-run --clean shows a visible failure when the file exists but
  cannot be read (mirrors the live-removal block's error handling).

- (G) Extract _detect_stale_claude_md() helper that computes the
  canonical Path, portable rel string, exists/has_marker booleans, and
  read_error for the root CLAUDE.md. Both the dry-run preview block and
  the live removal block now call this helper, eliminating the duplicated
  read + marker-detect + rel logic. All asserted message substrings
  (would remove stale, Removed stale, Skipped removal, hand-authored,
  Could not read) are byte-identical to before.

* refactor(agents_compiler): address PR #1747 review comments H/I/J/K/L

K – introduce StaleClaudeDetection NamedTuple for _detect_stale_claude_md;
    update both call sites (dry-run + live) to use attribute access.
I – add dry-run preview for the hand-authored CLAUDE.md case (was previously
    silent); surfaces a [dry-run] skip-preview line and a warning consistent
    with the live --clean hand-authored warning. Add TestDryRunHandAuthoredPreview
    class (3 tests) covering file integrity, warning emission, and content preview.
J – reword the "unreachable but explicit" comment in the live-removal block to
    accurately state that dry-run returns earlier, so no guard is needed.
H – add autouse _reset_constitution_cache fixture to the integration test module
    to prevent global cache leaking between tests (mirrors unit-test pattern).
L – split the dense single-paragraph compile dedup/clean/dry-run description in
    commands.md into shorter paragraphs with bold section headings for readability.

* fix(apm-compile): address review comments M/N/O for stale CLAUDE.md cleanup (#1729)

- Comment M: gate the live stale-file detection+removal block behind
  `config.clean_orphaned` so plain `apm compile` (no --clean) does no
  extra disk I/O and emits no stale-file warnings; the
  "CLAUDE.md not generated" INFO log still fires unconditionally.

- Comment N: emit dry-run read-error at `warning` level and dry-run
  hand-authored skip preview at `progress`/`info` level via `self._log`
  so both outcomes surface on the distributed terminal path (matching
  the existing marker-removal preview pattern).

- Comment O: reword `--clean` Click help= text and matching docstring
  bullet to reflect the actual trigger (deduplication suppresses
  CLAUDE.md generation) rather than the misleading "rules/ is already
  populated" phrasing.

Also adds three regression tests: plain compile with APM-generated file
emits no stale warnings; dry-run hand-authored case logs via logger;
dry-run unreadable file logs a warning via logger.

* fix(apm-review-panel): address PR #1747 review comments P/Q/R/S

P: catch UnicodeDecodeError alongside OSError in _detect_stale_claude_md
   so a non-UTF-8 CLAUDE.md surfaces a warning instead of crashing compilation.

R: add self._log("warning", det.read_error) in the live --clean read-error
   branch for parity with the dry-run path's existing logger call.

Q: replace flaky chmod(0o000) test with invalid-UTF-8 bytes approach
   (write_bytes(b"\xff\xfe...")) -- deterministic across all environments
   including containers running as root; exercises the new P code path.

S: extract module-level _CapturingLogger class, replacing four identical
   inline definitions; update all call sites to use logger.logged.

* fix(1729): bounded prefix read for marker check, dedupe dry-run message, tighten --clean docs

E: Replace full read_text() with bounded 4096-byte binary read in
   _detect_stale_claude_md; strict UTF-8 decode preserves UnicodeDecodeError
   -> read_error path (mirrors #1730 AGENTS.md side).

G: Build the dry-run removal-preview message once into removal_msg and
   reuse for both preview_lines (with leading indent) and self._log(),
   eliminating drift between the two sinks.

F: Reword --clean docs in reference/cli/compile.md and producer/compile.md
   to accurately describe the gate (deduplication suppresses all CLAUDE.md
   emission) rather than "when .claude/rules/ is already populated".

* fix(1729): apply panel follow-ups -- path guard, logger parity, helper dedup, docs, help text

Follow-up 1 (supply-chain security): route CLAUDE.md unlink through
ensure_path_within(det.path, self.base_dir) before unlink in the live
block, matching the .claude/rules/ guard convention (~line 72).
PathTraversalError is caught alongside OSError; both surface a warning
via all_warnings + self._log("warning") and do NOT crash compilation.

Follow-up 2 (CLI logging): add self._log("warning", msg) after each
all_warnings.append() in the live block's OSError and hand-authored
branches, matching the existing read-error branch pattern so warnings
reach the terminal on both the interactive and distributed dry-run paths.

Follow-up 4 (Python architect): extract _hand_authored_skip_message(rel)
as a module-level helper returning the shared guidance tail ("hand-authored
file will not be deleted. To remove the duplicate context..."). Both the
dry-run block and the live block now build their warning from the helper;
the dry-run preview line (in result.content) retains its shorter wording,
while the warning message uses the full canonical guidance from the helper.
All 22 test_stale_claude_md_cleanup_1729 asserted substrings preserved.

Follow-up 3 (docs): add a dry-run preview note to compile.md below the
--clean table row, with a code example showing
`apm compile --clean --dry-run --target claude` and an explanation of
what the preview output means. Mechanism detail (when dedup fires, what
a constitution does) retained in the table cell per existing style.

Follow-up 5 (DevX UX): shorten --clean Click help= to one actionable
sentence referencing --dry-run and the compile reference docs. Update
the compile() docstring bullet to be consistent: retains slightly more
mechanism detail than the one-liner help but no longer contradicts it.

* fix: harden stale Claude cleanup

Fold shepherd-panel follow-ups for issue #1729 by routing stale CLAUDE.md unlink through the path containment guard, emitting live cleanup warnings through the logger, shortening --clean help, and documenting dry-run preview in the CLI reference.

Co-authored-by: tillig <tillig@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: polish Claude cleanup follow-ups

Remove a stale hand-authored skip helper, keep dry-run skip wording consistent, and tighten CLI reference plus apm-guide notes after the final panel pass.

Co-authored-by: tillig <tillig@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: guard Claude cleanup detection reads

Fold terminal supply-chain feedback by checking CLAUDE.md containment before reading marker bytes as well as before unlinking, keeping symlinked paths fail-closed.

Co-authored-by: tillig <tillig@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Daniel Meppiel <51440732+danielmeppiel@users.noreply.github.com>

Author: 3 people

CHANGELOG.md

@@ -37,6 +37,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Optional-auth MCP registry servers now install without token prompts when values are unset; `apm install` also omits empty runtime config entries and preserves user-edited optional values on reinstall; refs #20. (#1734)
 - Dependencies with the same path on different git hosts no longer collide in `apm.lock.yaml`; `apm install` keeps GitHub/GitLab PATs off generic-host file downloads, routes bespoke GitLab hosts through `type: gitlab`, and surfaces non-404 download failures with host and endpoint context. Reading private files from a generic non-default host now requires a whole-repo git dependency or explicit backend signal; see the dependency and lockfile docs (closes #773). (#1735)
 - GitLab `path:` files now just work on self-hosted instances where the REST API is disabled or restricted -- APM fetches via git transport automatically. `GITLAB_APM_PAT` / `GITLAB_TOKEN` remain available as a thin REST API fallback; closes #1014. (#1740)
+- `apm compile --clean --target claude` now removes a stale APM-generated
+  `CLAUDE.md` when instructions have already been deployed to `.claude/rules/`
+  (the dedup path introduced in #1138), so the duplicate-context problem #1138
+  set out to eliminate is fully resolved even for projects that compiled before
+  `.claude/rules/` existed. Hand-authored `CLAUDE.md` files (those lacking the
+  `<!-- Generated by APM CLI -->` marker) are never deleted; a warning is
+  emitted instead. Plain `apm compile` (without `--clean`) remains
+  non-destructive. (closes #1729, follow-up to #1138)
 - `apm compile --target copilot` no longer writes empty `AGENTS.md`
   shell files when `.github/instructions/` already holds deployed
   instructions; `--clean` also removes stale APM-generated shells while

docs/src/content/docs/producer/compile.md

@@ -253,7 +253,13 @@ you can omit `start_marker` and `end_marker` if you use those verbatim.
   pin `targets:` in your manifest.
 - **Stale `AGENTS.md` after deleting an instruction.** Compile leaves
   previous output in place by default. Pass `--clean` to remove
-  orphaned files generated by earlier runs.
+  orphaned files generated by earlier runs. When compiling for the
+  `claude` target, `--clean` also removes a stale APM-generated
+  `CLAUDE.md` when deduplication suppresses `CLAUDE.md` entirely: all
+  instructions already live in `.claude/rules/`, and no constitution or
+  dependency content keeps `CLAUDE.md` active. Hand-authored `CLAUDE.md`
+  files (those without the `<!-- Generated by APM CLI -->` marker) are
+  never deleted.
 - **Hand-edited primitives skip the security scan.** `apm compile`
   does not run the install-time hidden-Unicode scan. After hand-edits,
   run `apm audit` before publishing. See

docs/src/content/docs/reference/cli/compile.md

@@ -77,7 +77,18 @@ use `apm install` or `apm deps update` when you want shared
 | `--root DIR` | Redirect generated artifacts (AGENTS.md and per-target files) under `DIR` while `apm.yml`, `.apm/`, and `apm_modules/` still resolve from the current working directory. Mirrors `pip install --target`. `DIR` is created if missing. Not valid with `--watch`. |
 | `-o, --output PATH` | Output file path. Only applies in single-file mode (`--single-agents`). Default: `AGENTS.md`. |
 | `--single-agents` | Force single-file compilation (legacy). Writes one combined file at `--output` instead of a distributed per-directory target-file tree. Applies to every target that uses distributed placement. |
-| `--clean` | Remove orphaned AGENTS.md files no longer produced by the current primitive set. |
+| `--clean` | Remove orphaned AGENTS.md files no longer produced by the current primitive set. For `--target claude`, also removes a stale APM-generated CLAUDE.md once instructions live in `.claude/rules/`. Hand-authored CLAUDE.md files (files without the `<!-- Generated by APM CLI -->` marker) are never deleted. |
+
+If a constitution or dependency imports still require `CLAUDE.md`, `--clean`
+regenerates/overwrites that file instead of removing it.
+
+`--clean` is a file-deleting operation. Before running it live, pass `--dry-run` to preview exactly what would be removed without touching the filesystem:
+
+```bash
+apm compile --clean --dry-run --target claude
+```
+
+The dry-run output shows `[dry-run] would remove stale CLAUDE.md -- instructions now live in .claude/rules/` when an APM-generated file would be deleted, or `[dry-run] would skip removal of CLAUDE.md: hand-authored file will not be deleted. Delete or rename it manually if duplicate context is unwanted.` when a hand-authored file is found. No files are written or deleted during dry-run.
 
 ### Content
 
@@ -146,10 +157,12 @@ Skip dependencies and compile only local primitives:
 apm compile --local-only
 ```
 
-Remove stale AGENTS.md files after deleting primitives:
+Remove stale AGENTS.md files after deleting primitives (also removes a stale APM-generated CLAUDE.md once `.claude/rules/` owns the instructions):
 
 ```bash
 apm compile --clean
+apm compile --clean --target claude
+apm compile --clean --target claude --dry-run
 ```
 
 Redirect generated files to a scratch directory (sources stay in `$PWD`):

packages/apm-guide/.apm/skills/apm-usage/commands.md

@@ -56,7 +56,13 @@ If no `--target`, no `targets:` in `apm.yml`, and no harness signal is present,
 
 `apm compile --watch` live-reloads `apm.yml`: editing `target:` / `targets:` mid-session takes effect on the next file event without restarting the watcher. The CLI `--target` flag, when passed to `apm compile --watch`, still outranks `apm.yml`. Re-resolution is gated on the changed file's basename being `apm.yml`, so `.instructions.md` edits do not pay an extra resolver round-trip and a stray `backup_apm.yml` cannot trigger a reload. `--clean` is ignored in watch mode and the watcher prints an explicit `[!]` warning at startup (`--clean is ignored in watch mode; run 'apm compile --clean' separately to remove orphaned outputs.`); run `apm compile --clean` separately between watch sessions to remove orphans.
 
-When `apm install` has already deployed instructions to `.claude/rules/`, `apm compile --target claude` omits the Project Standards section from `CLAUDE.md` to avoid Claude Code seeing every instruction twice. Detection is a simple glob (`.claude/rules/*.md`). `CLAUDE.md` is still generated when it carries a constitution block or dependency `@import` paths -- only the instructions section is suppressed. An informational log message is emitted when zero `CLAUDE.md` files are generated because all content was already deployed via rules. Pass `--no-dedup` (alias: `--force-instructions`) to override this behaviour and always include the instructions section in `CLAUDE.md`.
+When `apm install` has already deployed instructions to `.claude/rules/`, `apm compile --target claude` omits the Project Standards section from `CLAUDE.md` to avoid Claude Code seeing every instruction twice. Detection is a simple glob (`.claude/rules/*.md`). `CLAUDE.md` is still generated when it carries a constitution block or dependency `@import` paths -- only the instructions section is suppressed. An informational log message is emitted when zero `CLAUDE.md` files are generated because all content was already deployed via rules.
+
+**Deduplication override.** Pass `--no-dedup` (alias: `--force-instructions`) to always include the instructions section in `CLAUDE.md`, regardless of `.claude/rules/` contents.
+
+**Stale `CLAUDE.md` removal (`--clean`).** When `.claude/rules/` is populated and an APM-generated `CLAUDE.md` (identified by the `<!-- Generated by APM CLI -->` marker) is present, `apm compile --target claude --clean` removes that stale file if no constitution or dependency import keeps `CLAUDE.md` active; otherwise it regenerates the file. Hand-authored `CLAUDE.md` files (no marker) are never deleted; a warning is emitted instead so you know the duplicate context remains.
+
+**Dry-run preview.** Use `--dry-run` with `--clean` to preview what would be removed without touching any files. If a hand-authored `CLAUDE.md` would block deletion, the preview surfaces that outcome as well.
 
 When `apm install --target copilot` has already deployed instructions to `.github/instructions/`, `apm compile --target copilot` omits `AGENTS.md` entirely if its only content would be the duplicated instructions section. `AGENTS.md` is still generated when it carries non-instruction content such as a constitution. Pass `--no-dedup` (alias: `--force-instructions`) to force full `AGENTS.md` output.
 

src/apm_cli/commands/compile/cli.py

@@ -828,7 +828,10 @@ def _coerce_provenance_targets(value):
 @click.option(
     "--clean",
     is_flag=True,
-    help="Remove orphaned AGENTS.md files that are no longer generated",
+    help=(
+        "Remove orphaned output files (AGENTS.md, CLAUDE.md) no longer generated. "
+        "Hand-authored files are never deleted; use --dry-run to preview removals."
+    ),
 )
 @click.option(
     "--legacy-skill-paths",
@@ -917,7 +920,12 @@ def compile(  # noqa: PLR0913 -- Click handler
     * --dry-run: Preview compilation without writing files (shows placement decisions)
     * --verbose: Show detailed source attribution and optimizer analysis
     * --local-only: Ignore dependencies, compile only local .apm/ primitives
-    * --clean: Remove orphaned AGENTS.md files that are no longer generated
+    * --clean: Remove orphaned AGENTS.md files no longer generated; for
+      --target claude, also removes a stale APM-generated CLAUDE.md when
+      deduplication suppresses CLAUDE.md generation entirely (instructions
+      already in .claude/rules/ with no constitution or other keep-alive).
+      Hand-authored files are never deleted. Combine with --dry-run to
+      preview removals before they happen.
     """
     logger = CommandLogger("compile", verbose=verbose, dry_run=dry_run)