Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 44 additions & 6 deletions src/apm_cli/primitives/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,13 +287,51 @@ def get_dependency_declaration_order(base_dir: str) -> list[str]:
# This matches our org-namespaced directory structure
dependency_names.append(dep.repo_url)

# Include transitive dependencies from apm.lock
# Direct deps from apm.yml have priority; transitive deps are appended
lockfile_paths = LockFile.installed_paths_for_project(Path(base_dir))
# Include transitive dependencies + local-bundle slugs from the
# lockfile. Read it once and reuse the parsed object for both
# the transitive-paths walk and the ``local_deployed_files``
# slug derivation (issue #1363) to avoid duplicate YAML parses
# on every compile.
project_root = Path(base_dir)
lockfile_path = project_root / "apm.lock.yaml"
if not lockfile_path.exists():
legacy = project_root / "apm.lock"
if legacy.exists():
lockfile_path = legacy
lock = LockFile.read(lockfile_path) if lockfile_path.exists() else None

direct_set = set(dependency_names)
for path in lockfile_paths:
if path not in direct_set:
dependency_names.append(path)
if lock is not None:
for path in lock.get_installed_paths(project_root / "apm_modules"):
if path not in direct_set:
dependency_names.append(path)

# Local-bundle install stages instructions under
# ``apm_modules/<slug>/.apm/...`` but intentionally does NOT
# mutate ``apm.yml`` (services.py:489-490), so the scan loop
# would otherwise never visit those staged dirs and
# ``apm compile`` would produce no output for compile-only
# targets (opencode, codex, gemini).
#
# Provenance is anchored to the lockfile record -- a stray
# directory under ``apm_modules/`` without a lockfile entry must
# not be discovered (defends against phantom-content injection
# and stale-debris drift).
if lock is not None:
local_slugs: set[str] = set()
for deployed in lock.local_deployed_files:
# Match ``apm_modules/<slug>/.apm/...`` only. Other
# deployed files (``.github/instructions/...``,
# ``.agents/skills/...``) are not bundle staging
# markers and must not produce phantom slugs.
parts = Path(deployed).parts
if len(parts) >= 3 and parts[0] == "apm_modules" and parts[2] == ".apm":
local_slugs.add(parts[1])
seen = set(dependency_names)
for slug in sorted(local_slugs):
if slug not in seen:
dependency_names.append(slug)
seen.add(slug)

return dependency_names

Expand Down
122 changes: 122 additions & 0 deletions tests/integration/test_install_local_bundle_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import subprocess
import tarfile
from pathlib import Path
from typing import ClassVar
from unittest.mock import patch

import pytest
Expand Down Expand Up @@ -1084,3 +1085,124 @@ def test_bundle_mcp_dry_run_does_not_call_integrator(

assert result.exit_code == 0, result.output
mock_install.assert_not_called()


# ---------------------------------------------------------------------------
# Regression for issue #1363 -- local-bundle compile round-trip
# ---------------------------------------------------------------------------


class TestLocalBundleCompileRoundTrip:
"""Regression coverage for issue #1363.

Before the fix, ``apm install <local-bundle> -t <compile-only-target>``
staged instructions under ``apm_modules/<slug>/.apm/instructions/`` but
``apm compile`` never discovered them, because the discovery scan only
walked paths declared in ``apm.yml`` deps + ``apm.lock.yaml``
``dependencies[]``. Local-bundle install intentionally does NOT mutate
``apm.yml`` (services.py:489-490), so the staged content was invisible
and compile produced no ``AGENTS.md`` / ``GEMINI.md``.

The fix derives bundle slugs from the lockfile's top-level
``local_deployed_files`` field (already written by
``local_bundle_handler.py:194-199``) and surfaces them to the discovery
scan. This test exercises the full install -> compile pipeline for
every compile-only target the bug affects.
"""

OUTPUT_FILE_BY_TARGET: ClassVar[dict[str, str]] = {
"opencode": "AGENTS.md",
"codex": "AGENTS.md",
"gemini": "GEMINI.md",
}

@pytest.mark.parametrize("target", ["opencode", "codex", "gemini"])
def test_install_then_compile_produces_output_with_staged_instruction(
self,
tmp_path: Path,
monkeypatch: pytest.MonkeyPatch,
target: str,
) -> None:
"""For every compile-only target, install a local bundle whose
only payload is an instruction, then run ``apm compile`` and
assert the target's output file is produced with the staged
instruction content embedded.
"""
from apm_cli.integration.targets import KNOWN_TARGETS

# Bundle with a single distinctive instruction so we can assert
# the body is present in the compiled output.
marker = "PEP8-MARKER-issue-1363"
files = {
"instructions/style.instructions.md": (
"---\n"
"description: Style guide for the bundle\n"
"applyTo: '**/*.py'\n"
"---\n\n"
f"# Style Guide\nFollow PEP8. {marker}\n"
),
}
bundle = _make_plugin_bundle(
tmp_path / "src",
plugin_id="bundle-1363",
pack_target="all",
files=files,
)
project = _make_project(tmp_path / "dst")
# Pre-create the target's root_dir so auto-detect resolves to it
# (mirrors a real project configured for that IDE).
(project / KNOWN_TARGETS[target].root_dir).mkdir(parents=True, exist_ok=True)

install_result = _invoke_install(project, str(bundle), monkeypatch=monkeypatch)
assert install_result.exit_code == 0, (
f"install failed for target={target}: {install_result.output!r}"
)
Comment on lines +1156 to +1159
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in e084a6a — cwd is now pinned explicitly via monkeypatch.chdir(project) immediately before runner.invoke(["compile", ...]), so the E2E no longer depends on _invoke_install's side-effect.

# Sanity: the staging path the fix relies on must exist on disk.
staged = (
project
/ "apm_modules"
/ "bundle-1363"
/ ".apm"
/ "instructions"
/ "style.instructions.md"
)
assert staged.is_file(), (
f"local-bundle install did not stage at {staged} for target={target}"
)

# Now run `apm compile --target <target>` in the project dir.
# Pin cwd explicitly rather than rely on a side-effect from
# ``_invoke_install`` (which monkeypatch.chdir'd into the
# project): future refactors of the install helper must not
# silently break this E2E.
monkeypatch.chdir(project)
runner = CliRunner()
compile_result = runner.invoke(
cli,
["compile", "--target", target],
catch_exceptions=False,
)
Comment on lines +1173 to +1184
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in e084a6a — same fix as above (monkeypatch.chdir(project) before runner.invoke(["compile", ...])).

assert compile_result.exit_code == 0, (
f"compile failed for target={target}: {compile_result.output!r}"
)

# The target's compile output file must exist AND contain the
# marker from the staged instruction. This is the regression
# signal: before the fix, the output file was not produced at
# all ("Compilation completed but produced no output files").
out_name = self.OUTPUT_FILE_BY_TARGET[target]
out_path = project / out_name
assert out_path.is_file(), (
f"compile did not produce {out_name} for target={target}; "
f"output: {compile_result.output!r}"
)
body = out_path.read_text(encoding="utf-8")
# For gemini, GEMINI.md is a thin pointer to AGENTS.md
# (``@./AGENTS.md``), so the staged content lands in AGENTS.md
# itself. Walk that pointer when present.
if target == "gemini" and "@./AGENTS.md" in body:
body = (project / "AGENTS.md").read_text(encoding="utf-8")
assert marker in body, (
f"compile did not include staged instruction marker in {out_name} "
f"for target={target}; body head: {body[:400]!r}"
)
188 changes: 179 additions & 9 deletions tests/unit/primitives/test_discovery_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,26 +593,196 @@ def test_transitive_deps_appended_deduped(self):
"""Transitive deps from lockfile are appended but not duplicated."""
apm_yml = Path(self.tmp) / "apm.yml"
apm_yml.write_text("name: test\n")
# Write a real lockfile so discovery's single LockFile.read() call
# picks up the transitive entry. apm_modules dir doesn't need to
# actually exist for get_installed_paths to return the entries.
(Path(self.tmp) / "apm.lock.yaml").write_text(
"version: 1\n"
"dependencies:\n"
" - repo_url: owner/direct-dep\n"
" resolved_ref: HEAD\n"
" resolved_commit: 0\n"
" version: 0.0.0\n"
" depth: 0\n"
" source: registry\n"
" - repo_url: owner/transitive-dep\n"
" resolved_ref: HEAD\n"
" resolved_commit: 0\n"
" version: 0.0.0\n"
" depth: 1\n"
" source: registry\n"
)
mock_dep = MagicMock()
mock_dep.alias = None
mock_dep.is_virtual = False
mock_dep.repo_url = "owner/direct-dep"
mock_package = MagicMock()
mock_package.get_apm_dependencies.return_value = [mock_dep]
with (
patch(
"apm_cli.primitives.discovery.APMPackage.from_apm_yml",
return_value=mock_package,
),
patch(
"apm_cli.primitives.discovery.LockFile.installed_paths_for_project",
return_value=["owner/direct-dep", "owner/transitive-dep"],
),
with patch(
"apm_cli.primitives.discovery.APMPackage.from_apm_yml",
return_value=mock_package,
):
result = get_dependency_declaration_order(self.tmp)
self.assertEqual(result, ["owner/direct-dep", "owner/transitive-dep"])


class TestLocalBundleStagedSlugs(unittest.TestCase):
"""Regression tests for issue #1363.

Local-bundle install stages instructions under
``apm_modules/<slug>/.apm/instructions/`` and records the deployed paths
in the lockfile's top-level ``local_deployed_files`` field. The install
intentionally does NOT mutate ``apm.yml`` (services.py:489-490), so
``scan_dependency_primitives`` previously could not see the staged
primitives, and ``apm compile`` produced no output for compile-only
targets (opencode, codex, gemini).

The fix derives bundle slugs from ``local_deployed_files`` entries that
match ``apm_modules/<slug>/.apm/...`` and feeds them into the discovery
scan list, so compile picks them up while the apm.yml contract stays
untouched. Provenance is anchored to the lockfile record, not to a blind
walk of ``apm_modules/`` -- a stray directory under ``apm_modules/``
without a lockfile record must NOT be discovered.
"""

def setUp(self):
self.tmp = tempfile.mkdtemp()

def tearDown(self):
import shutil

shutil.rmtree(self.tmp, ignore_errors=True)

def _write_local_bundle_lockfile(self, deployed_files: list[str]) -> None:
"""Write an apm.lock.yaml with ``local_deployed_files`` populated.

Mirrors what ``install_local_bundle`` writes in
``local_bundle_handler.py:194-199``.
"""
import yaml

data: dict = {
"lockfile_version": "1",
"generated_at": "2025-01-01T00:00:00+00:00",
}
if deployed_files:
data["local_deployed_files"] = sorted(deployed_files)
(Path(self.tmp) / "apm.lock.yaml").write_text(
yaml.dump(data, default_flow_style=False), encoding="utf-8"
)

def test_staged_bundle_slug_appended_when_apm_yml_empty(self):
"""A local bundle staged under apm_modules/<slug>/.apm/ appears in
the declaration order even with no apm.yml dependency entry.
"""
apm_yml = Path(self.tmp) / "apm.yml"
apm_yml.write_text("name: test\nversion: 1.0.0\n")
self._write_local_bundle_lockfile(
[
"apm_modules/my-bundle/.apm/instructions/style.md",
"apm_modules/my-bundle/.apm/instructions/style.instructions.md",
]
)
result = get_dependency_declaration_order(self.tmp)
self.assertIn("my-bundle", result)

def test_no_lockfile_record_no_slug(self):
"""A stray apm_modules/<slug>/.apm/ directory with NO lockfile
record must NOT be surfaced. This defends the security invariant
that discovery is gated by lockfile provenance, not by raw
on-disk presence (otherwise a hostile actor or stale debris
could inject content into compile output).
"""
apm_yml = Path(self.tmp) / "apm.yml"
apm_yml.write_text("name: test\nversion: 1.0.0\n")
# No lockfile written at all.
# Even if we physically stage files on disk, discovery must not
# surface them without a lockfile record.
staged = Path(self.tmp) / "apm_modules" / "phantom" / ".apm" / "instructions"
staged.mkdir(parents=True)
(staged / "evil.instructions.md").write_text(INSTRUCTION_CONTENT)
result = get_dependency_declaration_order(self.tmp)
self.assertNotIn("phantom", result)

def test_declared_dep_wins_over_local_bundle_with_same_slug(self):
"""When both a declared dep and a local-bundle slug share a name,
the declared dep appears first (priority). The local-bundle slug
is deduplicated against the existing entry.
"""
apm_yml = Path(self.tmp) / "apm.yml"
apm_yml.write_text("name: test\n")
mock_dep = MagicMock()
mock_dep.alias = "shared-name"
mock_dep.is_virtual = False
mock_package = MagicMock()
mock_package.get_apm_dependencies.return_value = [mock_dep]
self._write_local_bundle_lockfile(["apm_modules/shared-name/.apm/instructions/x.md"])
with patch(
"apm_cli.primitives.discovery.APMPackage.from_apm_yml",
return_value=mock_package,
):
result = get_dependency_declaration_order(self.tmp)
# "shared-name" appears exactly once, and the alias (declared)
# comes first.
self.assertEqual(result.count("shared-name"), 1)
self.assertEqual(result[0], "shared-name")

def test_multiple_local_bundles_sorted_deterministically(self):
"""Two local bundles surface in deterministic order (alphabetical
by slug) so compile output is reproducible across runs.
"""
apm_yml = Path(self.tmp) / "apm.yml"
apm_yml.write_text("name: test\nversion: 1.0.0\n")
self._write_local_bundle_lockfile(
[
"apm_modules/bundle-b/.apm/instructions/b.md",
"apm_modules/bundle-a/.apm/instructions/a.md",
]
)
result = get_dependency_declaration_order(self.tmp)
# bundle-a appears before bundle-b regardless of file insertion
# order in the lockfile list.
idx_a = result.index("bundle-a")
idx_b = result.index("bundle-b")
self.assertLess(idx_a, idx_b)

def test_non_apm_modules_paths_ignored(self):
"""``local_deployed_files`` also records files outside
apm_modules/ (e.g. .github/instructions/x.md from the same bundle
install). Those must NOT produce phantom slugs.
"""
apm_yml = Path(self.tmp) / "apm.yml"
apm_yml.write_text("name: test\nversion: 1.0.0\n")
self._write_local_bundle_lockfile(
[
".github/instructions/style.md",
".agents/skills/coding/SKILL.md",
"apm_modules/real-bundle/.apm/instructions/x.md",
]
)
result = get_dependency_declaration_order(self.tmp)
# The legitimate apm_modules entry must produce its slug.
self.assertIn("real-bundle", result)
# The non-apm_modules entries must NOT produce phantom slugs --
# check absence of plausible spoofs derived from their paths
# (filenames or first-segment dir names) without forbidding
# unrelated dependency entries from coexisting.
for phantom in ("style", ".github", "coding", ".agents"):
self.assertNotIn(phantom, result)

def test_non_instructions_apm_subdirs_also_recognized(self):
"""Future-proof: bundles may stage other primitive types under
apm_modules/<slug>/.apm/ (e.g. context/, agents/). The slug
derivation must trigger on any apm_modules/<slug>/.apm/* path,
not only instructions/.
"""
apm_yml = Path(self.tmp) / "apm.yml"
apm_yml.write_text("name: test\nversion: 1.0.0\n")
self._write_local_bundle_lockfile(["apm_modules/ctx-bundle/.apm/context/notes.context.md"])
result = get_dependency_declaration_order(self.tmp)
self.assertIn("ctx-bundle", result)


class TestScanLocalPrimitives(unittest.TestCase):
"""Tests for scan_local_primitives."""

Expand Down
Loading