Stop rendering CLAUDE.md dependencies that don't exist#1048
Stop rendering CLAUDE.md dependencies that don't exist#1048danielmeppiel merged 18 commits intomicrosoft:mainfrom
Conversation
tillig
requested review from
danielmeppiel and
sergio-sisternes-epam
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Prevents apm compile --targets claude from emitting dependency @import entries for apm_modules packages that don’t actually have a CLAUDE.md file on disk (fixes #1047).
Changes:
- Add an existence check for
apm_modules/{owner}/{package}/CLAUDE.mdbefore adding dependency@importentries. - Expand unit test fixture and add coverage to ensure packages missing
CLAUDE.mdare excluded. - Document the behavior change in the changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/compilation/test_claude_formatter.py | Adds/updates tests to verify dependencies are only included when CLAUDE.md exists. |
| src/apm_cli/compilation/claude_formatter.py | Skips dependencies for packages without a CLAUDE.md file. |
| CHANGELOG.md | Notes the bug fix for claude target dependency rendering. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
🔬 APM Review Panel — PR #1048
Panel Verdict:
|
| File stem | Deployed to |
|---|---|
claude-hooks.json |
Claude only |
cursor-hooks.json |
Cursor only |
hooks.json |
All targets |
After this PR: ALL hook files deploy to ALL targets. A claude-hooks.json with Claude-specific events will now also be written into Cursor's settings, and vice versa. This could corrupt IDE configurations.
2. Claude Event Name Normalisation
# REMOVED from _HOOK_EVENT_MAP:
"claude": {
"preToolUse": "PreToolUse",
"postToolUse": "PostToolUse",
},Claude requires PreToolUse/PostToolUse (PascalCase). Without this mapping, hooks authored with camelCase events will silently fail in Claude.
3. Variable Pattern Narrowing
The docstring change from accepting ${CURSOR_PLUGIN_ROOT}/path, ${PLUGIN_ROOT}/path to only ${CLAUDE_PLUGIN_ROOT}/path in the pattern description suggests the regex was also narrowed. Package authors using ${PLUGIN_ROOT} in hook scripts will silently break.
4. ~550 Tests Deleted — TestIssue1007Fixes
The entire TestIssue1007Fixes class (550 lines) covering these behaviors is deleted with no replacement tests. The issue number referenced in the class name implies these were deliberate regression tests for a specific bug fix.
Required action: Either (a) restore the removed behaviors with justification for any that are intentionally removed, or (b) open a follow-up issue documenting the behavior change, add a CHANGELOG entry under ### Removed, and explain why target-aware routing is being dropped.
Minor Notes
install.pyis now very large. The MCP inline consolidation is internally consistent, but the file's LOC count is a maintenance concern for future contributors.${PLUGIN_ROOT}/${CURSOR_PLUGIN_ROOT}support removal (if intentional) should be in CHANGELOG under### Removed.- No missing tests for the MCP inline path — existing test patches still work due to the module-level re-bindings. ✓
Summary
| Area | Status |
|---|---|
claude_formatter.py core fix |
✅ Approved |
agents_compiler.py simplification |
✅ Approved |
_helpers.py / compile/cli.py consolidation |
✅ Approved |
| MCP sub-package inlining | ✅ Approved |
hook_integrator.py regression |
❌ Needs resolution |
The core fix should ship. Please address the hook integrator regression — either restore the removed behaviors or explicitly document and justify their removal in CHANGELOG — before this PR is merged.
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #1048
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - Stop rendering CLAUDE.md dependencies that don't exist #1048
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by PR Review Panel for issue #1048 · ● 1.2M · ◷
github-actions
Bot
added
panel-rejected
Bring in PR microsoft#1068 (utf-8 encoding) and other main updates so the PR diff shows only the focused fix. Earlier panel rejection cited a hook_integrator regression which is no longer present in this branch (removed via prior merges); the core claude_formatter.py fix that the panel approved remains intact and is the only logical change. Co-authored-by: Travis Illig <tillig@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (10 items)
Nits (12 items, skip if you want)
CEO arbitrationThe panel reached strong consensus across all five active specialists. The One tension is worth naming: devx-ux-expert calls the missing marker guard on the write path REQUIRED (user data loss), while supply-chain-security-expert treats the analogous cleanup-path Dissent resolved: No panelist classified the same finding as REQUIRED by one and NIT by another. The near-conflict between devx-ux-expert's write-path guard (REQUIRED) and supply-chain-security-expert's cleanup-path traversal check (NIT) is not a true dissent -- the two experts are reviewing different code paths. Both calls are upheld. Growth/positioning note: The Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class AgentsCompiler {
<<Facade>>
+base_dir Path
+compile(config, primitives) CompilationResult
+_compile_agents_md(config, primitives) CompilationResult
+_maybe_emit_copilot_root_instructions(config, primitives, result) CompilationResult
+_generate_copilot_root_instructions_content(instructions, config) str
+_finalize_build_id(content) str
+_cleanup_copilot_root_instructions(output_path, result) CompilationResult
}
class ClaudeFormatter {
<<Collaborator>>
+base_dir Path
+_collect_dependencies() list
}
class CompilationConfig {
<<ValueObject>>
+target CompileTargetType
+dry_run bool
+resolve_links bool
+strategy str
}
class CompilationResult {
<<ValueObject>>
+success bool
+warnings list
+errors list
+stats dict
+has_critical_security bool
}
class PrimitiveCollection {
<<ValueObject>>
+instructions list
}
class SecurityGate {
<<Guard>>
+scan_text(content, path, policy) Verdict
}
class target_detection {
<<Module>>
+should_compile_agents_md(target CompileTargetType) bool
+should_compile_claude_md(target CompileTargetType) bool
+should_compile_gemini_md(target CompileTargetType) bool
+should_compile_copilot_instructions_md(target TargetType) bool
}
note for target_detection "BUG: should_compile_copilot_instructions_md\nshould accept CompileTargetType (not TargetType)\nto handle frozenset multi-target inputs"
AgentsCompiler ..> CompilationConfig : reads
AgentsCompiler ..> PrimitiveCollection : reads
AgentsCompiler ..> CompilationResult : produces
AgentsCompiler ..> SecurityGate : calls
AgentsCompiler ..> target_detection : delegates routing
AgentsCompiler *-- ClaudeFormatter : collaborates
class AgentsCompiler:::touched
class ClaudeFormatter:::touched
class target_detection:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A(["apm compile -t copilot"]) --> B["_resolve_compile_target:\n'copilot' -> 'vscode' string"]
A2(["apm compile -t claude,copilot"]) --> B2["_resolve_compile_target:\nfrozenset({'agents','claude'})"]
B --> C["compile() dispatch\nisinstance frozenset? NO"]
B2 --> C2["compile() dispatch\nisinstance frozenset? YES"]
C --> D["routing_target = 'vscode'"]
C2 --> D2["should_compile_agents_md(frozenset) -> True\n_compile_agents_md called"]
D --> E["should_compile_agents_md('vscode') -> True\n_compile_agents_md called"]
E --> F["_maybe_emit_copilot_root_instructions\nagents_compiler.py:887"]
D2 --> F2["_maybe_emit_copilot_root_instructions\nagents_compiler.py:887"]
F --> G["routing_target = 'vscode'\nconfig.target 'vscode' in _VSCODE_TARGET_ALIASES"]
F2 --> G2["routing_target = frozenset({'agents','claude'})\nfrozenset NOT in _VSCODE_TARGET_ALIASES tuple -- BUG"]
G --> H["should_compile_copilot_instructions_md('vscode') -> True"]
G2 --> H2["should_compile_copilot_instructions_md(frozenset) -> False -- BUG\n_cleanup called instead"]
H --> I["Filter: instruction.apply_to is None"]
I --> J{"global_instructions empty?"}
J -- No --> K["_generate_copilot_root_instructions_content()\n[!] footer typo: 'specify apm compile' line 976"]
K --> L["SecurityGate.scan_text(content)"]
L --> M["[FS] read existing, compare"]
M --> N{"content changed?"}
N -- No --> O["stats: unchanged=1"]
N -- Yes --> P["[FS] write_text() -> copilot-instructions.md"]
Design patterns
Required findings:
Nits:
CLI Logging ExpertRequired findings:
Nits:
DevX UX ExpertRequired findings:
Nits:
Supply Chain Security ExpertRequired findings:
Nits:
Auth ExpertInactive -- PR touches only compilation output generation files (claude_formatter.py, agents_compiler.py, target_detection.py) and integration target routing; no auth, token management, credential resolution, or host classification code is changed. OSS Growth HackerRequired findings:
Nits:
Verdict computed deterministically: 10 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
removed
the
panel-review
Folds 10 panel findings into a single commit: - Multi-target frozenset gap: should_compile_copilot_instructions_md now handles frozenset inputs (mirrors should_compile_agents_md). apm compile -t claude,copilot now correctly emits the file. - Type annotation: parameter is now CompileTargetType, consistent with sibling routing functions. - Footer typo: 'specify apm compile' -> 'apm compile'. - Write-path marker guard: hand-authored copilot-instructions.md files are no longer silently overwritten on first compile; a warning is emitted instead. - CHANGELOG: add Added entry for the new copilot-native compile target and Fixed entries for the three regressions. - Docs: cli-commands.md gains a prose section describing what triggers generation, the multi-target example is updated, and security.md enumerates copilot-instructions.md as a scanned compile output. - Regression tests: frozenset routing, footer wording, and write-path marker guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel Verdict: REJECT
Required before merge (8 items)
Nits (13 items, skip if you want)
CEO arbitrationThe most consequential finding in this review is the frozenset routing asymmetry identified by the Python Architect. The fix intended to gate copilot-instructions.md generation on the "agents" family token, but because The DevX UX Expert and OSS Growth Hacker both flag the warning message, but from complementary angles. DevX calls it a required fix on actionability grounds; Growth escalates it to a systemic adoption blocker because new users with existing hand-authored files will silently fail with no documented escape hatch. Both are right and their findings are additive. The docs findings from DevX -- the redundant On balance, this PR introduced a real routing regression, two stat-key correctness defects, three documentation gaps, and a missing migration story. The growth signal around zero-config Copilot integration is genuine and worth preserving in the release narrative, but the feature's credibility depends on the routing regression being fixed first -- a broken multi-target path discovered by an early adopter would directly undercut the "zero configuration" positioning claim. Dissent resolved: devx-ux-expert classifies the warning message rewrite as required; oss-growth-hacker classifies the missing CHANGELOG migration path as required. These are not in conflict -- they address different surfaces of the same user-impact. Both classifications are upheld. The supply-chain nit about SecurityGate scanning content that is subsequently skipped does not rise to required; the ordering is a UX issue, not a security vulnerability. Growth/positioning note: The zero-config Copilot-native compile target is a strong acquisition hook for teams already using GitHub Copilot. APM dogfooding this target is a trust signal, but it must be made concrete in CHANGELOG and docs with a direct repo reference or link. The "zero user configuration" phrase belongs in the CHANGELOG Added section, not only in cli-commands.md. Once the routing regression is fixed, leading with this feature in release notes -- rather than burying it under a frozenset bug entry -- would materially improve discoverability for developers scanning changelogs. Per-persona findings (full)Python ArchitectclassDiagram
class CompileTargetType {
<<type alias>>
Union[TargetType, frozenset[CompileFamily]]
}
class TargetType {
<<Literal>>
vscode | claude | cursor | opencode | codex | gemini | all | minimal
}
class CompileFamily {
<<Literal>>
agents | claude | gemini
}
class target_detection {
+should_compile_agents_md(target: CompileTargetType) bool
+should_compile_claude_md(target: CompileTargetType) bool
+should_compile_gemini_md(target: CompileTargetType) bool
+should_compile_copilot_instructions_md(target: CompileTargetType) bool
}
class AgentsCompiler {
+base_dir: Path
+errors: list
-_maybe_emit_copilot_root_instructions(config, primitives, result)
-_generate_copilot_root_instructions_content(instructions, config)
-_cleanup_copilot_root_instructions(path, result)
}
class CompilationConfig {
+target: CompileTargetType
+dry_run: bool
+strategy: str
+single_agents: bool
}
class CompilationResult {
+success: bool
+stats: dict
+warnings: list
+errors: list
}
CompileTargetType --> TargetType : union branch
CompileTargetType --> CompileFamily : frozenset branch
target_detection ..> CompileTargetType : consumes
AgentsCompiler ..> target_detection : calls should_compile_*
AgentsCompiler --> CompilationConfig : reads
AgentsCompiler --> CompilationResult : mutates
flowchart TD
A["apm compile -t cursor,claude"] --> B["_resolve_compile_target cursor,claude"]
B --> C{"cursor in agents_family?"}
C -- yes --> D["families.add('agents')"]
D --> E["families.add('claude')"]
E --> F["return frozenset({'agents','claude'})"]
F --> G["should_compile_copilot_instructions_md(frozenset)"]
G --> H{"isinstance frozenset?"}
H -- True --> I{"'agents' in target?"}
I -- True --> J["return True -- BUG: cursor should not trigger this"]
J --> K["copilot-instructions.md written unexpectedly"]
style J fill:#f66
style K fill:#f66
L["apm compile -t cursor (single)"] --> M["routing_target = 'cursor'"]
M --> N["should_compile_copilot_instructions_md('cursor')"]
N --> O{"isinstance str"}
O -- True --> P{"'cursor' in ('vscode','all')"}
P -- False --> Q["return False -- correct"]
style Q fill:#6f6
Required:
Nits:
CLI Logging ExpertRequired:
Nits:
DevX UX ExpertRequired:
Nits:
Supply Chain Security ExpertNo required findings. Nits:
Auth ExpertInactive -- No auth-related files changed; the PR only modifies compilation routing, target detection, CHANGELOG, docs, and regression tests -- none of which touch AuthResolver, token management, credential resolution, or HTTP authorization paths. OSS Growth HackerRequired:
Nits:
Verdict computed deterministically: 8 required findings across 5 active panelists (auth-expert inactive). APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 6 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
1 similar comment
APM Review Panel Verdict: REJECT
Required before merge (8 items)
Nits (13 items, skip if you want)
CEO arbitrationThe most consequential finding in this review is the frozenset routing asymmetry identified by the Python Architect. The fix intended to gate copilot-instructions.md generation on the "agents" family token, but because The DevX UX Expert and OSS Growth Hacker both flag the warning message, but from complementary angles. DevX calls it a required fix on actionability grounds; Growth escalates it to a systemic adoption blocker because new users with existing hand-authored files will silently fail with no documented escape hatch. Both are right and their findings are additive. The docs findings from DevX -- the redundant On balance, this PR introduced a real routing regression, two stat-key correctness defects, three documentation gaps, and a missing migration story. The growth signal around zero-config Copilot integration is genuine and worth preserving in the release narrative, but the feature's credibility depends on the routing regression being fixed first -- a broken multi-target path discovered by an early adopter would directly undercut the "zero configuration" positioning claim. Dissent resolved: devx-ux-expert classifies the warning message rewrite as required; oss-growth-hacker classifies the missing CHANGELOG migration path as required. These are not in conflict -- they address different surfaces of the same user-impact. Both classifications are upheld. The supply-chain nit about SecurityGate scanning content that is subsequently skipped does not rise to required; the ordering is a UX issue, not a security vulnerability. Growth/positioning note: The zero-config Copilot-native compile target is a strong acquisition hook for teams already using GitHub Copilot. APM dogfooding this target is a trust signal, but it must be made concrete in CHANGELOG and docs with a direct repo reference or link. The "zero user configuration" phrase belongs in the CHANGELOG Added section, not only in cli-commands.md. Once the routing regression is fixed, leading with this feature in release notes -- rather than burying it under a frozenset bug entry -- would materially improve discoverability for developers scanning changelogs. Per-persona findings (full)Python ArchitectclassDiagram
class CompileTargetType {
<<type alias>>
Union[TargetType, frozenset[CompileFamily]]
}
class TargetType {
<<Literal>>
vscode | claude | cursor | opencode | codex | gemini | all | minimal
}
class CompileFamily {
<<Literal>>
agents | claude | gemini
}
class target_detection {
+should_compile_agents_md(target: CompileTargetType) bool
+should_compile_claude_md(target: CompileTargetType) bool
+should_compile_gemini_md(target: CompileTargetType) bool
+should_compile_copilot_instructions_md(target: CompileTargetType) bool
}
class AgentsCompiler {
+base_dir: Path
+errors: list
-_maybe_emit_copilot_root_instructions(config, primitives, result)
-_generate_copilot_root_instructions_content(instructions, config)
-_cleanup_copilot_root_instructions(path, result)
}
class CompilationConfig {
+target: CompileTargetType
+dry_run: bool
+strategy: str
+single_agents: bool
}
class CompilationResult {
+success: bool
+stats: dict
+warnings: list
+errors: list
}
CompileTargetType --> TargetType : union branch
CompileTargetType --> CompileFamily : frozenset branch
target_detection ..> CompileTargetType : consumes
AgentsCompiler ..> target_detection : calls should_compile_*
AgentsCompiler --> CompilationConfig : reads
AgentsCompiler --> CompilationResult : mutates
flowchart TD
A["apm compile -t cursor,claude"] --> B["_resolve_compile_target cursor,claude"]
B --> C{"cursor in agents_family?"}
C -- yes --> D["families.add('agents')"]
D --> E["families.add('claude')"]
E --> F["return frozenset({'agents','claude'})"]
F --> G["should_compile_copilot_instructions_md(frozenset)"]
G --> H{"isinstance frozenset?"}
H -- True --> I{"'agents' in target?"}
I -- True --> J["return True -- BUG: cursor should not trigger this"]
J --> K["copilot-instructions.md written unexpectedly"]
style J fill:#f66
style K fill:#f66
L["apm compile -t cursor (single)"] --> M["routing_target = 'cursor'"]
M --> N["should_compile_copilot_instructions_md('cursor')"]
N --> O{"isinstance str"}
O -- True --> P{"'cursor' in ('vscode','all')"}
P -- False --> Q["return False -- correct"]
style Q fill:#6f6
Required:
Nits:
CLI Logging ExpertRequired:
Nits:
DevX UX ExpertRequired:
Nits:
Supply Chain Security ExpertNo required findings. Nits:
Auth ExpertInactive -- No auth-related files changed; the PR only modifies compilation routing, target detection, CHANGELOG, docs, and regression tests -- none of which touch AuthResolver, token management, credential resolution, or HTTP authorization paths. OSS Growth HackerRequired:
Nits:
Verdict computed deterministically: 8 required findings across 5 active panelists (auth-expert inactive). APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 6 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
removed
the
panel-review
Persona-gated round 3 fold for microsoft#1048. 8 panel findings on a7ef6f9: [python-architect] - frozenset predicate no longer over-fires for cursor/opencode/codex multi-target combos. Added a distinct 'vscode' CompileFamily token that is set ONLY when copilot/vscode/agents was in the original list; should_compile_copilot_instructions_md frozenset branch now checks '"vscode" in target' instead of '"agents" in target'. - _resolve_compile_target preserves bare cursor/opencode/codex names for AGENTS.md-only single-target lists instead of collapsing to 'vscode'. [cli-logging-expert] - copilot_root_instructions_generated is now reset to 0 inside the hand-authored skip guard (was leaving a false positive 1). - Skip case now records copilot_root_instructions_skipped=1 instead of reusing the semantically wrong 'unchanged' key. Initialized in all stat-defaults blocks so aggregation never KeyErrors. [devx-ux-expert] - Skip warning uses a project-relative path and tells the user the exact next command ("re-run 'apm compile' to regenerate"). - Docs comment for -t vscode no longer double-lists .github/. - Docs paragraph no longer lists the internal 'agents' family name as a valid CLI flag value. [oss-growth-hacker] - CHANGELOG Added entry for the Copilot-native target leads with "zero user configuration" framing. - CHANGELOG Fixed entry for the hand-authored guard now documents the migration escape hatch (delete + recompile, or prepend the marker). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
APM Review Panel Verdict: REJECT
Required before merge (4 items)
Nits (13 items, skip if you want)
CEO arbitrationPR #1048 ships APM's strongest acquisition story to date -- zero-config Copilot-native output that VS Code reads automatically -- but it arrives with two blocking correctness gaps and two missing growth surfaces that together break the adoption loop before it can compound. The devx-ux-expert correctly flags that The panel is otherwise aligned. Supply-chain nits are informational -- the Unicode scan gap should be confirmed but is not a code-path regression. Python-architect nits (dead elif, copy-paste stats blocks, silent None return) are valid technical debt but carry no user-visible risk at this PR's scope. CLI-logging nit on warning message phrasing is subsumed by the devx-ux-expert required item on the same warning. No specialist conflicts require arbitration. The fix set is well-scoped: move the hand-authored guard before the dry-run branch, append the literal marker string to the warning (and to Dissent resolved: The oss-growth-hacker required items (README and quickstart) are occasionally challenged as documentation nits that should not block a code PR. When a feature's entire value proposition is discoverability and zero-config first-run, documentation is not decorative -- it is the delivery mechanism. Treating README and quickstart as post-merge tasks for a feature of this profile is a strategic error, not a process preference. Required stands. Growth/positioning note: This is the tweet-shaped PR APM has been waiting for: one command, no config, a file that a mainstream AI tool reads automatically, and self-proof via dogfooding. The compounding loop is post -> README hook -> quickstart aha -> install -> star. That loop currently breaks at step two because the README has no hook. Fix README and quickstart before the launch post goes out. The CHANGELOG dogfooding note should also surface as a one-liner social proof anchor in the README. Per-persona findings (full)Python ArchitectclassDiagram
class CompileTargetType {
<<type alias>>
str | frozenset~CompileFamily~ | None
}
class CompileFamily {
<<Literal>>
agents
vscode
claude
gemini
}
class TargetType {
<<Literal>>
vscode | claude | copilot | cursor
opencode | codex | agents | gemini | all
}
class AgentsCompiler {
+base_dir: Path
+compile(config) CompilationResult
-_maybe_emit_copilot_root_instructions(config, primitives, result) CompilationResult
-_generate_copilot_root_instructions_content(instructions, config) str
}
class ClaudeFormatter {
+base_dir: Path
-_collect_dependencies() list~str~
}
class CompilationResult {
+stats: dict~str,int~
+warnings: list~str~
}
class PrimitiveCollection {
+global_instructions: list
}
class TargetDetection {
<<module>>
+should_compile_copilot_instructions_md(target) bool
+should_compile_gemini_md(target) bool
+should_compile_claude_md(target) bool
}
class CompileCLI {
<<module>>
-_resolve_compile_target(target) CompileTargetType
}
CompileCLI ..> CompileTargetType : produces
CompileTargetType o-- CompileFamily
CompileTargetType o-- TargetType
AgentsCompiler --> CompilationResult
AgentsCompiler --> PrimitiveCollection
AgentsCompiler ..> TargetDetection : calls
ClaudeFormatter --> CompilationResult
TargetDetection ..> CompileTargetType : consumes
class AgentsCompiler:::touched
class ClaudeFormatter:::touched
class TargetDetection:::touched
class CompileCLI:::touched
class CompileFamily:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A([apm compile -t TARGET]) --> B[_resolve_compile_target]
B --> C{target is list?}
C -- no --> D[pass-through single string]
C -- yes --> E{intersect copilot_family?}
E -- yes --> F[families += vscode + agents]
E -- no --> G{intersect agents_md_family?}
G -- yes --> H[families += agents only]
G -- no --> I[check claude / gemini]
F --> J{other families?}
H --> J
I --> J
J -- "families=={vscode,agents}" --> K[return 'vscode']
J -- len>=2 --> L[return frozenset]
J -- single --> M[return bare string]
D --> N[AgentsCompiler.compile]
K --> N
L --> N
M --> N
N --> O[_maybe_emit_copilot_root_instructions]
O --> P[should_compile_copilot_instructions_md]
P -- frozenset --> Q{"'vscode' in frozenset?"}
P -- string --> R["target in ('vscode','all')?"]
Q -- no --> S[early return / zero stats]
Q -- yes --> T[collect global_instructions]
R -- no --> S
R -- yes --> T
T --> U{dry_run?}
U -- yes --> V["[FS] log, no write"]
U -- no --> W{file exists without APM marker?}
W -- yes --> X["[FS] warn + stats skipped=1, return"]
W -- no --> Y{content unchanged?}
Y -- yes --> Z[stats unchanged=1]
Y -- no --> AA["[FS] write file + stats written=1"]
N --> AB[ClaudeFormatter._collect_dependencies]
AB --> AC{apm_modules_dir.is_dir?}
AC -- no --> AD[return empty]
AC -- yes --> AE{CLAUDE.md exists per package?}
AE -- no --> AF[skip package]
AE -- yes --> AG[append import path]
Design patterns
No required findings. Nits:
CLI Logging ExpertNo required findings. Nits:
DevX UX ExpertRequired:
Nits:
Supply Chain Security ExpertNo required findings. Nits:
Auth ExpertInactive -- PR #1048 touches only compile target routing and output generation files (cli.py, agents_compiler.py, claude_formatter.py, target_detection.py); none of the auth fast-path trigger files are modified and no authentication behavior changes. OSS Growth HackerRequired:
Nits:
Growth/positioning note: This is the tweet-shaped PR APM has been waiting for: one command, no config, a file that a mainstream AI tool reads automatically, and self-proof via dogfooding. The adoption loop is post -> README hook -> quickstart aha -> install -> star. That loop breaks at step two without a README hook. Fix README and quickstart before the launch post goes out. Verdict computed deterministically: 4 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
removed
the
panel-review
…t hooks Resolves all 4 required findings from round 3 panel review: [devx-ux] dry-run faithfully simulates the hand-authored guard - Move marker check + content-equality check BEFORE the dry_run early-exit in _maybe_emit_copilot_root_instructions, so 'apm compile --dry-run -t copilot' reports skipped=1 (matching real run) instead of generated=1 when the file exists hand-authored. CI scripts using --dry-run as a preview gate now get a faithful answer. Security gate scan still runs only on actual writes. - New regression test: test_dry_run_respects_hand_authored_guard. [devx-ux] Skip warning discloses the literal marker string - Warning now reads: 'Skipped <path>: hand-authored file will not be overwritten. To regenerate, either delete or rename it, or prepend the line <marker> to the top of the file. Then re-run apm compile.' - cli-commands.md generation prose mentions the literal marker on both references so users can self-serve recovery without grepping APM source. [oss-growth] README hook for the Copilot-native compile target - Add a 'Zero-config Copilot:' callout right after the npx-skills-add comparison and before 'The three promises' section. Carries one-liner, runnable example, and dogfooding fact as social proof. [oss-growth] Quickstart includes apm compile -t copilot in first-run path - New section 'Get Copilot reading your packages in under a minute' lands between the install-package walkthrough and 'That's it', framing the Copilot step as the immediate aha-moment after install. CHANGELOG: new Fixed entry for dry-run regression at top; migration note now names the literal marker line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
added
panel-review
APM Review Panel Verdict: APPROVE
Required before merge (0 items)None. Nits (10 items, skip if you want)
CEO arbitrationAll five active panelists returned zero required changes, so the panel is structurally unanimous: this PR ships. The nits cluster into four themes -- dry-run observability, stat-key hygiene, docs clarity, and a security micro-gap -- none of which block the round-4 intent, which is to make --dry-run a faithful simulator and to surface the APM marker string for self-service recovery. Those are the right goals and the implementation is directionally sound. The most actionable nits are the two from cli-logging-expert: the "would write" log line on the dry-run new-content path is a one-liner and should land before the next release so --dry-run is actually useful; and the terminal-wrap concern on the marker warning is real enough to file a follow-up. The supply-chain-security nit (scan unchanged content before early-return) is a good defensive habit and cheap to add, but the threat model is thin enough that it does not block merge -- file it as a hardening follow-up with explicit trade-off notes. The python-architect stat-key nit is valid technical debt; a constants module or Enum would eliminate the silent-typo risk, but refactoring 15+ assignments is out of scope for a fix round and belongs in a separate cleanup PR. Growth/positioning note: The oss-growth-hacker rates story fit as excellent and hook score as strong -- the Copilot grounding target is the clearest public expression of APM as the package manager for AI-native development. Two concrete edits should land before any social amplification: replace "APM dogfoods this target" with "APM uses this on its own repository" (insider jargon strips conversion), and add an expected-output cue to the quickstart ("You will see: .github/copilot-instructions.md created") to close the loop for first-time evaluators. The README callout placement is worth a second look -- if it currently lands below line 30, hoist it. Per-persona findings (full)Python ArchitectclassDiagram
direction LR
class CompilationConfig {
<<DataClass>>
+target CompileTargetType
+dry_run bool
+strategy str
+single_agents bool
+from_apm_yml() CompilationConfig
}
class CompilationResult {
<<DataClass>>
+success bool
+warnings list[str]
+errors list[str]
+stats dict[str,Any]
+has_critical_security bool
}
class AgentsCompiler {
<<Orchestrator>>
+base_dir Path
+compile(config, primitives) CompilationResult
+_maybe_emit_copilot_root_instructions(config, primitives, result) CompilationResult
+_generate_copilot_root_instructions_content(instructions, config) str
+_cleanup_copilot_root_instructions(output_path, result) void
}
class PrimitiveCollection {
<<ValueObject>>
+instructions list[Instruction]
+add_primitive(p) void
}
class Instruction {
<<ValueObject>>
+name str
+file_path Path
+apply_to str
+content str
}
class SecurityGate {
<<StaticGate>>
+scan_text(content, path, policy) Verdict
}
class AgentsCompiler:::touched
class CompilationResult:::touched
AgentsCompiler *-- CompilationConfig : reads
AgentsCompiler *-- PrimitiveCollection : reads
AgentsCompiler ..> CompilationResult : produces
AgentsCompiler ..> SecurityGate : delegates scan
PrimitiveCollection o-- Instruction : contains
note for AgentsCompiler "Collect-then-render:\n_maybe_emit accumulates warnings/stats\ninto CompilationResult, caller renders"
note for CompilationResult "Mutable accumulator: warnings and stats\nare appended throughout the call chain"
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A(["apm compile -t copilot/vscode"]) --> B["_maybe_emit_copilot_root_instructions\nagents_compiler.py:880"]
B --> C{"should_compile_copilot\n_instructions_md(target)?"}
C -- No --> D["[FS] _cleanup_copilot_root_instructions\n(skipped in dry-run)"]
D --> Z1(["return result\nskipped/removed stats"])
C -- Yes --> E["_generate_copilot_root_instructions_content\nagents_compiler.py:993"]
E --> F["[I/O] output_path.read_text()\nif output_path.exists()\n:932 -- NEW: moved before dry-run exit"]
F -- OSError --> G["result.errors.append\nresult.success=False\n:934-939"]
G --> Z2(["return result (failure)"])
F -- No existing file --> H["existing = None"]
F -- File read ok --> H2{"marker in existing?\n:941"}
H2 -- No marker\nhand-authored --> I["result.warnings.append(\n'hand-authored... marker string disclosed')\n:943-948 NEW: was inside write try-block"]
I --> I2["stats: skipped=1, generated=0\n:952-955"]
I2 --> Z3(["return result (skipped)"])
H --> J{"existing == content?\n:958 NEW: moved before dry-run exit"}
H2 -- Marker present --> J
J -- Yes unchanged --> K["stats: unchanged=1, written=0\n:959-960"]
K --> Z4(["return result (unchanged)"])
J -- No new/changed content --> L{"config.dry_run?\n:963"}
L -- Yes --> Z5(["return result\ngenerated=1, written=0"])
L -- No --> M["[NET/SCAN] SecurityGate.scan_text\n:968"]
M --> N["[FS] output_path.parent.mkdir\noutput_path.write_text\n:979-980"]
N -- OSError --> O["result.errors.append\nresult.success=False\n:984-990"]
O --> Z6(["return result (failure)"])
N -- ok --> P["stats: written=1, unchanged=0\n:981-982"]
P --> Z7(["return result (written)"])
Design patterns: Collect-then-render -- Required: None. Nits:
CLI Logging ExpertRequired: None. Nits:
DevX UX ExpertRequired: None. Nits:
Supply Chain Security ExpertRequired: None. Nits:
Auth ExpertInactive -- PR only touches agents_compiler.py, tests, docs, README, and CHANGELOG; no auth, token, credential, or host-classification files are modified. OSS Growth HackerRequired: None. Nits:
Side-channel: Hook score strong ("Zero-config Copilot:" is tweetable and differentiating). Story fit excellent -- Copilot grounding is the clearest expression of "package manager for AI-native development" in the repo so far; this PR is a launch beat worth amplifying in the next release post. The dogfooding mention creates a reusable proof point for social copy and release narratives. Verdict computed deterministically: 0 required findings across 5 active panelists. APPROVE iff N == 0. Push a new commit to clear this verdict label automatically. Note 🔒 Integrity filter blocked 2 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | none
|
github-actions
Bot
added
panel-approved
Description
Adds a check for whether a
CLAUDE.mdfile exists in a skill, instruction, etc., before adding it to the list of dependencies in the generatedCLAUDE.mdthat comes fromapm compile --targets claude.Fixes #1047
Type of change
Testing