fix(compile): resolve context links when folding instructions into CL… - #2918
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The formatter holds a stateful link resolver whose context registry can leak across repeated format_distributed() calls, potentially rewriting links to stale/incorrect context paths.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes apm compile --target claude so that when instruction bodies are folded into CLAUDE.md, any embedded relative links to .context.md / .memory.md fragments are rewritten to be correct relative to CLAUDE.md’s output location (mirroring the existing AGENTS.md pipeline behavior).
Changes:
- Add a
UnifiedLinkResolvertoClaudeFormatter, register contexts during compilation, and runresolve_links_for_compilation()on the assembledCLAUDE.mdcontent before returning it. - Add regression tests covering link rewriting for both local and dependency-sourced instruction/context pairs.
File summaries
| File | Description |
|---|---|
src/apm_cli/compilation/claude_formatter.py |
Registers context fragments and resolves context/memory links in generated CLAUDE.md content. |
tests/unit/compilation/test_claude_formatter.py |
Adds regression tests ensuring rewritten .context.md links resolve from CLAUDE.md’s directory, including dependency scenarios. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
5312569 to
90f519f
Compare
|
@microsoft-github-policy-service agree company="Etnetera" |
…AUDE.md claude_formatter.py never called into the link resolver, so any relative link embedded in an instruction body (in particular a link to a .apm/context/*.context.md fragment) was emitted verbatim from the source file when folded into CLAUDE.md -- correct only when CLAUDE.md happens to live in the same directory as the instruction that referenced it, and broken for any dependency-sourced or non-root placement. agents_compiler.py / distributed_compiler.py already call UnifiedLinkResolver.resolve_links_for_compilation() for the equivalent AGENTS.md fold; this mirrors that call in ClaudeFormatter: - construct a UnifiedLinkResolver once in __init__ - register_contexts() at the start of format_distributed() - resolve_links_for_compilation() on the assembled content before returning it from _generate_claude_content() Adds regression tests covering a local context fragment and a dependency-sourced one (apm_modules/_local/<repo>/... anchor), both of which reproduce the stale/broken link before this change and resolve correctly after it. All 35 existing + new unit tests in test_claude_formatter.py pass; full unit/compilation suite (1402 tests) unaffected.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
90f519f to
d26c2e1
Compare
Description
ClaudeFormatter(theCLAUDE.mdgenerator) never ran its assembled instruction content throughthe markdown-link resolver, so any relative link embedded in an instruction body -- most notably a
link to a
.apm/context/*.context.mdfragment -- was emitted byte-for-byte from the source file.That is correct only when
CLAUDE.mdhappens to land in the same directory as the instruction thatreferenced it, and silently broken otherwise: a global (no-
applyTo) instruction sourced from adependency, or placed anywhere but the project root, produces a link that no longer resolves from
CLAUDE.md's own location.The equivalent
AGENTS.mdpipeline (distributed_compiler.py/agents_compiler.py) does not havethis problem -- it explicitly calls
UnifiedLinkResolver.resolve_links_for_compilation()/resolve_markdown_links()before returning the generated content.claude_formatter.pyhad noequivalent call anywhere in the file.
This PR mirrors that call in
ClaudeFormatter:UnifiedLinkResolveronce in__init__register_contexts()at the start offormat_distributed()resolve_links_for_compilation()on the assembled content before returning it from_generate_claude_content()I found this while designing a monorepo/workspace layout where a root package depends on several
component-repo packages purely to pull in one "signpost" instruction each (an unconditional
instruction whose body links out to
.apm/context/*.context.mdfragments in the same package).AGENTS.mdresolved those links correctly out of the box;CLAUDE.mddid not.Fixes #2881
Type of change
Testing
Added
TestContextLinkResolutiontotests/unit/compilation/test_claude_formatter.py:test_context_link_rewritten_relative_to_claude_md-- local context fragment, link must resolvefrom
CLAUDE.md's own directory rather than the instruction's source directory.test_context_link_rewritten_for_dependency_sourced_instruction-- same, but the instruction andits linked context fragment are sourced from a dependency materialized under
apm_modules/, sincethat's the scenario where the bug actually bites (root instruction is never in the same directory
as a dependency-sourced context file).
Confirmed both regression tests fail on the pre-fix code (reverting just
src/apm_cli/compilation/claude_formatter.pywhile keeping the tests) and pass after the fix.Ran the full
tests/unit/compilation/suite (1402 tests) before and after: no regressions, allgreen. Ran
tests/unit/compilation/test_claude_formatter.pyspecifically (35 tests, up from 33):all pass.
Minimal manual repro (also embedded as the reproduction steps in the test fixtures):
<!-- .apm/context/conventions.context.md --> Real content lives here.Before:
[conventions](../context/conventions.context.md)(broken fromCLAUDE.md's owndirectory). After:
[conventions](.apm/context/conventions.context.md)(correct, matches whatapm compile --target opencodealready produces for the same source instruction).Spec conformance (OpenAPM v0.1)
This is an internal implementation bug fix (a missing function call) that brings
CLAUDE.md'slink-resolution behaviour in line with what
AGENTS.mdalready does and what the spec/docs alreadydescribe as the intended behaviour; it does not introduce, change, or remove any normative
req-XXXrequirement.