diff --git a/CHANGELOG.md b/CHANGELOG.md index 674411a76..6cf05fa9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] - + +### Added + +- New `apm export-patch` command exports local edits to APM-managed files + as unified diffs against the packages that deployed them, one + `git apply`-ready `.patch` file per package with the locked base recorded + in the header. Verbatim-deployed files only; transformed deployments are + listed as skipped with the reason. (by @edenfunf; closes #2118) (#2162) + ### Fixed - Fresh checkouts with declared consumer targets no longer remain diff --git a/docs/src/content/docs/reference/cli/export-patch.md b/docs/src/content/docs/reference/cli/export-patch.md new file mode 100644 index 000000000..fc9a5de8b --- /dev/null +++ b/docs/src/content/docs/reference/cli/export-patch.md @@ -0,0 +1,69 @@ +--- +title: apm export-patch +description: Export local edits to APM-managed files as patches against their source packages. +sidebar: + order: 14 +--- + +## Synopsis + +```bash +apm export-patch [OPTIONS] +``` + +## Description + +`apm export-patch` turns local edits to APM-managed files into unified diffs against the packages that deployed them, so the edits can be contributed back upstream. + +It replays the locked install into a scratch tree (the same machinery as the [`apm audit`](../audit/) drift check) and, for every managed file that was modified locally, maps the change back to the package source file that produced it. Each package with exportable edits gets one `.patch` file, ready for `git apply` in a clone of the package repository. When two packages would sanitize to the same patch filename, the later one gets a short digest suffix so nothing is overwritten. + +Every patch file records its base in a leading comment block: the package key, its source, and the exact snapshot the diff applies to (`commit ` for git dependencies, `version ` for registry packages). Apply the patch from the package repository root, checked out at that base, then open your upstream pull request from the result: + +```bash +git -C path/to/package-clone checkout +git -C path/to/package-clone apply path/to/apm-patches/.patch +``` + +Only verbatim-deployed files can be exported: a local edit maps cleanly back to its source only when the deployment copied that source byte-for-byte. Everything else is listed as skipped with the reason instead of producing a patch that would not apply: + +- Deployments that transform their content (frontmatter rewrites for rule directories, compiled `AGENTS.md` output, aggregated `copilot-instructions.md` sections, files with resolved links). +- Source files that are not normalization-clean (CRLF line endings, a UTF-8 BOM, or a build-id header): the deployed copy matches them only after normalization, so an exported diff could not be applied to the raw file. +- Conflicting edits: when one source file is deployed to several targets and the copies were edited differently, the conflict is reported instead of exporting either version. Identical edits across copies export as a single diff. +- Local deletions of managed files are never exported, since deleting a deployed copy does not imply the file should be removed from the package. + +Findings that belong to local path dependencies or to the project's own `.apm/` content are also skipped: their source already lives on disk, so the edit belongs there directly. A lockfile containing only local dependencies short-circuits before the replay. + +The replay is cache-only and read-only: it needs a lockfile and a populated `apm_modules/` (run `apm install` first) and never mutates the project tree. Patch files are the only output, and `--out` may not point inside `apm_modules/`. + +## Options + +| Flag | Default | Description | +|---|---|---| +| `--out`, `-o DIR` | `apm-patches` | Directory to write per-package `.patch` files into. Created if missing; only created when there is something to export. Must not point inside `apm_modules/`. | +| `--dry-run` | off | List what would be exported (and what gets skipped, with reasons) without writing patch files. | +| `--verbose`, `-v` | off | Show replay progress and per-file skip details. | + +## Exit codes + +| Code | Meaning | +|---|---| +| 0 | Success, including "nothing to export" (skipped-only runs exit 0 with warnings). | +| 1 | Replay or export failed (missing or unparsable lockfile, cold cache, invalid `--out`). | + +## Examples + +```bash +# Export all local edits as per-package patches under ./apm-patches/ +apm export-patch + +# Preview what would be exported without writing anything +apm export-patch --dry-run + +# Write patches somewhere else +apm export-patch -o /tmp/spec-patches +``` + +## See also + +- [`apm audit`](../audit/) -- detect the drift this command exports. +- [Drift and secure by default](../../../consumer/drift-and-secure-by-default/) -- why local edits to managed files are overwritten on the next install. diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 152404620..69c36c7d5 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -14,6 +14,7 @@ | `apm targets` | Show resolved deployment targets for the current project (Click group; reads filesystem signals; works with or without `apm.yml`) | `--all` also include the `agent-skills` meta-target (only meaningful with `--json`), `--json` machine-readable output. No provenance line is printed (the table is the provenance). | | `apm uninstall PKGS...` | Remove packages (accepts `owner/repo` or `name@marketplace`) | `--dry-run`, `-g` global | | `apm prune` | Remove orphaned packages | `--dry-run` | +| `apm export-patch` | Export local edits to APM-managed files as `git apply`-ready patches against their source packages (one `.patch` per package, locked base in the header; verbatim-deployed files only -- transformed deployments are listed as skipped with the reason) | `-o/--out DIR` output dir (default `apm-patches`; must not point inside `apm_modules/`), `--dry-run` preview, `-v` verbose | | `apm deps list` | List installed packages | `-g` global, `--all` both scopes, `--insecure` | | `apm deps tree` | Show dependency tree | -- | | `apm deps why PKG` | Explain why a package is installed (walks lockfile bottom-up to direct deps; analogue of `npm why` / `yarn why`) | `-g` global, `--json` | diff --git a/src/apm_cli/cli.py b/src/apm_cli/cli.py index e2553cf51..15ee2eb3f 100644 --- a/src/apm_cli/cli.py +++ b/src/apm_cli/cli.py @@ -26,6 +26,7 @@ from apm_cli.commands.deps import deps from apm_cli.commands.doctor import doctor from apm_cli.commands.experimental import experimental +from apm_cli.commands.export_patch import export_patch from apm_cli.commands.find import find as find_cmd from apm_cli.commands.init import init from apm_cli.commands.install import install @@ -177,6 +178,7 @@ def cli(ctx, verbose: bool) -> None: cli.add_command(lock) cli.add_command(uninstall) cli.add_command(prune) +cli.add_command(export_patch, name="export-patch") cli.add_command(update) cli.add_command(self_update) cli.add_command(plugin_cmd, name="plugin") diff --git a/src/apm_cli/commands/export_patch.py b/src/apm_cli/commands/export_patch.py new file mode 100644 index 000000000..c505ffb5b --- /dev/null +++ b/src/apm_cli/commands/export_patch.py @@ -0,0 +1,654 @@ +"""APM export-patch command. + +Turns ``modified`` drift findings into patches against package sources. +``apm export-patch`` builds on the drift replay (``install/drift.py``): a +``modified`` finding means a deployed, APM-managed file differs from what +a clean install of the locked dependency graph would produce. For +deployments that copy their source file verbatim, that difference IS the +user's local edit -- so it can be re-expressed as a unified diff against +the package source file and applied upstream (``git apply`` in a clone +of the package repository, checked out at the locked base). + +Eligibility is decided by content, not by a per-format allowlist: a +finding is exportable iff the replayed (expected) content matches +exactly one file in the owning package's source tree AND that source +file is byte-identical to the replayed content (not merely equal after +drift normalization). The second condition guarantees the emitted diff +applies to the raw on-disk source: a CRLF-, BOM-, or Build-ID-bearing +source would digest-match in normalized space yet reject every hunk at +``git apply`` time, so those are skipped with an accurate reason. +Deployments that transform their source (frontmatter rewrites, +aggregated or compiled outputs, resolved links) fail the digest match +and are reported as skipped instead of producing a patch that would +not apply. + +The command consumes the replay/diff APIs read-only; it never mutates +the project tree, the cache, or the lockfile. Patch files are the only +output. +""" + +from __future__ import annotations + +import difflib +import hashlib +import re +import sys +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING + +import click + +from ..constants import APM_YML_FILENAME +from ..core.command_logger import CommandLogger +from ..deps.lockfile import LockFile, get_lockfile_path +from ..utils.normalization import _normalize + +if TYPE_CHECKING: + from ..deps.lockfile import LockedDependency + from ..install.drift import DriftFinding + from ..integration.targets import TargetProfile + +# Source files above this size are never indexed as reverse-mapping +# candidates. Managed primitives are small text files; a larger blob is +# noise at best and a memory hazard at worst. +_MAX_INDEXED_BYTES = 2 * 1024 * 1024 + +# Mirror of utils/content_hash.py walk exclusions: these can appear in a +# materialized package tree but are never package content. +_EXCLUDED_DIRS = {".git", "__pycache__"} + +# Windows reserved device names cannot be used as file stems. +_RESERVED_STEMS = ( + {"con", "prn", "aux", "nul"} + | {f"com{i}" for i in range(1, 10)} + | {f"lpt{i}" for i in range(1, 10)} +) + + +@dataclass(frozen=True) +class ExportedEdit: + """A local edit that was successfully mapped back to a source file.""" + + deployed_path: str # project-relative posix path of the edited file + source_path: str # package-source-relative posix path the diff applies to + package: str # lockfile dependency key + + +@dataclass(frozen=True) +class SkippedEdit: + """A ``modified`` finding that could not be exported, with the reason.""" + + deployed_path: str + package: str + reason: str + + +@dataclass(frozen=True) +class PatchExport: + """Result of a patch-export computation over drift findings.""" + + # dependency key -> full patch document (header + unified diffs) + patches: dict[str, str] + exported: list[ExportedEdit] + skipped: list[SkippedEdit] + + +@dataclass(frozen=True) +class _SourceIndex: + """Reverse-mapping index over one package's source tree.""" + + by_digest: dict[str, list[str]] # sha256(normalized bytes) -> source rel paths + unindexed: tuple[str, ...] # rel paths excluded by size cap or read errors + + +def patch_filename(dep_key: str) -> str: + """Map a lockfile dependency key to a filesystem-safe patch filename. + + Lossy (many keys can share one name); use :func:`patch_filenames` + when writing a batch so collisions are disambiguated. + """ + safe = re.sub(r"[^A-Za-z0-9._-]+", "-", dep_key).strip("-.") or "package" + if safe.lower() in _RESERVED_STEMS: + safe = f"pkg-{safe}" + return f"{safe}.patch" + + +def patch_filenames(dep_keys: list[str]) -> dict[str, str]: + """Assign a unique patch filename to every dependency key. + + The sanitizer is many-to-one, so colliding keys get a short digest + suffix; without this, one package's patch would silently overwrite + another's. + """ + by_name: dict[str, list[str]] = {} + for key in dep_keys: + by_name.setdefault(patch_filename(key), []).append(key) + result: dict[str, str] = {} + for name, keys in by_name.items(): + if len(keys) == 1: + result[keys[0]] = name + continue + stem = name[: -len(".patch")] + for key in keys: + suffix = hashlib.sha256(key.encode("utf-8")).hexdigest()[:8] + result[key] = f"{stem}-{suffix}.patch" + return result + + +def _index_source_tree(source_root: Path) -> _SourceIndex: + """Index a package source tree by normalized content digest. + + Multiple paths under one digest means the content alone cannot + identify the source file (ambiguous -- the caller must skip). + Symlinks and ``.git``/``__pycache__`` trees are never candidates + (mirroring ``utils/content_hash.py``), and the root cache-pin + marker is install metadata, not package content. Files excluded by + the size cap or by read errors are recorded so skip reasons can + stay accurate. + """ + from ..install.cache_pin import MARKER_FILENAME + + by_digest: dict[str, list[str]] = {} + unindexed: list[str] = [] + for path in sorted(source_root.rglob("*")): + if path.is_symlink(): + continue + if not path.is_file(): + continue + rel_parts = path.relative_to(source_root).parts + if any(part in _EXCLUDED_DIRS for part in rel_parts): + continue + if len(rel_parts) == 1 and path.name == MARKER_FILENAME: + continue + rel = "/".join(rel_parts) + try: + if path.stat().st_size > _MAX_INDEXED_BYTES: + unindexed.append(rel) + continue + digest = hashlib.sha256(_normalize(path.read_bytes())).hexdigest() + except OSError: + unindexed.append(rel) + continue + by_digest.setdefault(digest, []).append(rel) + return _SourceIndex(by_digest=by_digest, unindexed=tuple(unindexed)) + + +def _diff_lines(text: str) -> list[str]: + """Split ``text`` into lines using git's line model. + + ``str.splitlines`` also breaks on bare CR, form feed, U+2028, etc., + which git treats as ordinary bytes within a line -- using it would + emit hunk headers whose line counts disagree with ``git apply``. + Only ``\\n`` terminates a line here; a final unterminated line is + kept without its newline so the caller can mark it. + """ + parts = text.split("\n") + lines = [part + "\n" for part in parts[:-1]] + if parts[-1]: + lines.append(parts[-1]) + return lines + + +def _unified_diff(a_text: str, b_text: str, rel: str) -> str: + """Build a ``git apply``-compatible unified diff for one file. + + Emits the ``\\ No newline at end of file`` marker where either side + lacks a trailing newline, which :func:`difflib.unified_diff` does not + produce on its own. + """ + a_lines = _diff_lines(a_text) + b_lines = _diff_lines(b_text) + out: list[str] = [] + for line in difflib.unified_diff(a_lines, b_lines, fromfile=f"a/{rel}", tofile=f"b/{rel}"): + if line.endswith("\n"): + out.append(line) + else: + out.append(line + "\n") + out.append("\\ No newline at end of file\n") + return "".join(out) + + +def _header_value(value: object) -> str: + """Sanitize a lockfile-sourced value for a single-line patch header. + + Control characters (including newlines) are collapsed to a space: + header text is only inert to ``git apply`` while it stays on one + ``#`` comment line, and these fields originate from registries and + manifests, not from the user. + """ + return re.sub(r"[\x00-\x1f\x7f]+", " ", str(value)).strip() + + +def _strip_userinfo(url: str) -> str: + """Drop URL userinfo (``user:token@host``) before it reaches a header. + + Private-registry and authenticated-source URLs can legally embed + credentials; a patch file is made to be shared, so they must never + be written into it. + """ + if "://" not in url: + return url + return re.sub(r"(?<=//)[^/@]*@", "", url) + + +def _base_label(dep: LockedDependency) -> str: + """Human-readable description of the snapshot the patch applies to.""" + if dep.source == "registry": + parts = [f"version {_header_value(dep.version or 'unknown')}"] + if dep.resolved_url: + parts.append(f"({_header_value(_strip_userinfo(dep.resolved_url))})") + return " ".join(parts) + if dep.resolved_commit: + label = f"commit {_header_value(dep.resolved_commit)}" + if dep.resolved_ref: + label += f" ({_header_value(dep.resolved_ref)})" + return label + return f"version {_header_value(dep.version or 'unknown')}" + + +def _patch_header(dep_key: str, dep: LockedDependency) -> str: + """Leading comment block for a per-package patch document. + + ``git apply`` ignores text before the first ``---``/``+++`` header, + so the metadata rides inside the patch file itself. + """ + lines = [ + "# Exported by 'apm export-patch'", + f"# package: {_header_value(dep_key)}", + f"# source: {_header_value(_strip_userinfo(dep.source_url or dep.repo_url))}", + f"# base: {_base_label(dep)}", + "# Apply from the package repository root, checked out at the", + "# base above: git apply ", + ] + return "\n".join(lines) + "\n" + + +def _dir_prefix_table(lockfile: LockFile) -> list[tuple[str, str]]: + """Trailing-slash ``deployed_files`` entries as (prefix, key) pairs. + + Longest prefix first, so nested dir entries attribute to the most + specific owner. Covers legacy lockfiles that track a skill directory + rather than every file inside it; ``local_deployed_files`` dirs map + to the self key so project-local content keeps its accurate skip + reason. + """ + from ..deps.lockfile import _SELF_KEY + + pairs: list[tuple[str, str]] = [] + for key, dep in lockfile.dependencies.items(): + for tracked in dep.deployed_files or []: + if tracked.endswith("/"): + pairs.append((tracked, key)) + for tracked in lockfile.local_deployed_files or []: + if tracked.endswith("/"): + pairs.append((tracked, _SELF_KEY)) + pairs.sort(key=lambda pair: len(pair[0]), reverse=True) + return pairs + + +def _resolve_package_key( + finding_path: str, package: str, dir_prefixes: list[tuple[str, str]] +) -> str: + """Resolve the owning dependency key for a finding. + + Findings normally carry the key already; the fallback matches the + longest tracked directory prefix. + """ + if package: + return package + for prefix, key in dir_prefixes: + if finding_path.startswith(prefix): + return key + return "" + + +def build_patch_export( + project_root: Path, + scratch_root: Path, + lockfile: LockFile, + findings: list[DriftFinding], +) -> PatchExport: + """Compute exportable patches from drift findings. + + Only ``modified`` findings are considered: ``unintegrated`` (file + deleted locally) does not imply intent to delete upstream, and + ``orphaned`` is a lockfile-hygiene issue rather than a content edit. + """ + from ..deps.lockfile import _SELF_KEY + from ..install.drift import CacheMissError, _materialize_install_path + + result_exported: list[ExportedEdit] = [] + result_skipped: list[SkippedEdit] = [] + patches: dict[str, str] = {} + + dir_prefixes = _dir_prefix_table(lockfile) + modified = [f for f in findings if f.kind == "modified"] + by_package: dict[str, list[DriftFinding]] = {} + for finding in modified: + key = _resolve_package_key(finding.path, finding.package, dir_prefixes) + by_package.setdefault(key, []).append(finding) + + apm_modules_dir = project_root / "apm_modules" + + for key in sorted(by_package): + group = by_package[key] + + def skip_group(reason: str, *, key: str = key, group: list = group) -> None: + result_skipped.extend(SkippedEdit(f.path, key, reason) for f in group) + + if not key: + skip_group("not tracked to a package in the lockfile") + continue + + dep = lockfile.get_dependency(key) + if key == _SELF_KEY or dep is None: + skip_group("project-local content; the source already lives in this project") + continue + if dep.source == "local" or dep.local_path: + skip_group(f"local package ({dep.local_path}); edit the source file directly") + continue + + try: + source_root = _materialize_install_path( + dep, project_root, apm_modules_dir, cache_only=True, lockfile=lockfile + ) + except CacheMissError as exc: + skip_group(f"package cache unavailable: {exc}") + continue + + index = _index_source_tree(source_root) + # (deployed path, source rel, diff text) per successful mapping; + # grouped afterwards so several deployed copies of one source + # file cannot emit clashing hunks into a single patch. + successes: list[tuple[str, str, str]] = [] + for finding in sorted(group, key=lambda f: f.path): + edit = _export_one(finding, key, index, scratch_root, project_root, source_root) + if isinstance(edit, SkippedEdit): + result_skipped.append(edit) + continue + successes.append(edit) + + by_source: dict[str, list[tuple[str, str, str]]] = {} + for success in successes: + by_source.setdefault(success[1], []).append(success) + + file_diffs: list[str] = [] + for source_rel in sorted(by_source): + copies = by_source[source_rel] + unique_diffs = {diff for _, _, diff in copies} + if len(unique_diffs) > 1: + paths = ", ".join(deployed for deployed, _, _ in copies) + result_skipped.extend( + SkippedEdit( + deployed, + key, + f"conflicting edits across {len(copies)} deployed copies of " + f"{source_rel} ({paths}); reconcile them and re-run", + ) + for deployed, _, _ in copies + ) + continue + # Identical edits in every deployed copy map to one source + # change: emit the diff once, report every copy as exported. + file_diffs.append(copies[0][2]) + result_exported.extend( + ExportedEdit(deployed, source_rel, key) for deployed, _, _ in copies + ) + + if file_diffs: + patches[key] = _patch_header(key, dep) + "".join(file_diffs) + + return PatchExport(patches=patches, exported=result_exported, skipped=result_skipped) + + +def _export_one( + finding: DriftFinding, + dep_key: str, + index: _SourceIndex, + scratch_root: Path, + project_root: Path, + source_root: Path, +) -> SkippedEdit | tuple[str, str, str]: + """Map one ``modified`` finding to (deployed, source rel, diff text). + + Returns a :class:`SkippedEdit` with the reason when the finding + cannot be exported. + """ + scratch_path = scratch_root / finding.path + project_path = project_root / finding.path + try: + scratch_bytes = _normalize(scratch_path.read_bytes()) + project_bytes = _normalize(project_path.read_bytes()) + except OSError as exc: + return SkippedEdit(finding.path, dep_key, f"unreadable: {exc}") + + digest = hashlib.sha256(scratch_bytes).hexdigest() + candidates = index.by_digest.get(digest, []) + if not candidates: + reason = ( + "deployed content does not match any source file byte-for-byte " + "(transformed during deployment); port the change manually" + ) + if index.unindexed: + reason += ( + f" -- note: {len(index.unindexed)} source file(s) were not " + "indexed (size cap or unreadable)" + ) + return SkippedEdit(finding.path, dep_key, reason) + if len(candidates) > 1: + preview = ", ".join(candidates[:3]) + return SkippedEdit( + finding.path, + dep_key, + f"ambiguous source: {len(candidates)} identical files ({preview})", + ) + + source_rel = candidates[0] + try: + source_raw = (source_root / source_rel).read_bytes() + except OSError as exc: + return SkippedEdit(finding.path, dep_key, f"source unreadable: {exc}") + # The digest matched in normalized space; the patch must apply to the + # RAW source file. Only a normalization-clean source (no CRLF, BOM, or + # Build-ID header) guarantees the normalized diff context matches the + # on-disk bytes -- otherwise git apply would reject every hunk. + if _normalize(source_raw) != source_raw: + return SkippedEdit( + finding.path, + dep_key, + f"source file {source_rel} contains CRLF line endings, a BOM, or a " + "build-id header; the exported diff would not apply to it -- port " + "the change manually", + ) + + try: + a_text = scratch_bytes.decode("utf-8") + b_text = project_bytes.decode("utf-8") + except UnicodeDecodeError: + return SkippedEdit(finding.path, dep_key, "binary or non-UTF-8 content") + + diff_text = _unified_diff(a_text, b_text, source_rel) + return finding.path, source_rel, diff_text + + +# --------------------------------------------------------------------------- +# CLI command +# --------------------------------------------------------------------------- + + +def _resolve_diff_targets(project_root: Path) -> list[TargetProfile]: + """Resolve targets for the diff phase, honoring apm.yml ``target:``. + + Must match how the replay resolves targets (drift.py, #1924): + auto-detection alone would skip declared targets whose root + directory does not exist, silently hiding their findings. + """ + from ..install.drift import _read_apm_yml_target + from ..integration.targets import resolve_targets + + return resolve_targets(project_root, explicit_target=_read_apm_yml_target(project_root)) + + +@click.command( + name="export-patch", + help="Export local edits to APM-managed files as patches against their source packages", +) +@click.option( + "--out", + "-o", + "out_dir", + type=click.Path(file_okay=False), + default="apm-patches", + show_default=True, + help="Directory to write per-package .patch files into", +) +@click.option( + "--dry-run", + is_flag=True, + help="List what would be exported without writing patch files", +) +@click.option("--verbose", "-v", is_flag=True, help="Show replay progress and skip details") +def export_patch(out_dir, dry_run, verbose): + """Export local edits to APM-managed files as upstream patches. + + Replays the locked install into a scratch directory (the same + machinery as the 'apm audit' drift check), then re-expresses every + locally modified managed file as a unified diff against the source + file inside the package that deployed it. Each package with + exportable edits gets one .patch file, applicable with 'git apply' + from the package repository root at the base recorded in the patch + header. + + Only verbatim-deployed files can be exported. Deployments that + transform their source (frontmatter rewrites, compiled or aggregated + outputs, resolved links) are listed as skipped with a reason. + + \b + Examples: + apm export-patch # write patches to ./apm-patches/ + apm export-patch -o /tmp/out # write patches elsewhere + apm export-patch --dry-run # preview without writing + + \b + Exit codes: + 0 Success (including "nothing to export") + 1 Replay or export failed + """ + logger = CommandLogger("export-patch", verbose=verbose, dry_run=dry_run) + try: + _run_export_patch(logger, out_dir, dry_run, verbose) + except SystemExit: + raise + except Exception as exc: + logger.error(f"Error exporting patches: {exc}") + sys.exit(1) + + +def _run_export_patch(logger: CommandLogger, out_dir: str, dry_run: bool, verbose: bool) -> None: + project_root = Path.cwd() + + if not (project_root / APM_YML_FILENAME).exists(): + logger.error(f"No {APM_YML_FILENAME} found. Run 'apm init' first.") + sys.exit(1) + + lockfile_path = get_lockfile_path(project_root) + if not lockfile_path.exists(): + logger.error("No lockfile found. Run 'apm install' first.") + sys.exit(1) + lockfile = LockFile.read(lockfile_path) + if lockfile is None: + logger.error( + f"Lockfile at {lockfile_path} could not be parsed; fix it or re-run 'apm install'." + ) + sys.exit(1) + + out_path = Path(out_dir) + resolved_out = ( + out_path.resolve() if out_path.is_absolute() else (project_root / out_path).resolve() + ) + apm_modules_dir = (project_root / "apm_modules").resolve() + if resolved_out == apm_modules_dir or apm_modules_dir in resolved_out.parents: + logger.error("--out must not point inside apm_modules/ (the install cache).") + sys.exit(1) + + remote_deps = [ + dep + for dep in lockfile.dependencies.values() + if dep.source != "local" and not dep.local_path + ] + if not remote_deps: + logger.success( + "All dependencies are project-local or local-path packages; " + "their sources already live on disk, so there is nothing to " + "export upstream." + ) + return + + from ..deps.path_anchoring import LocalResolutionError + from ..install.drift import ( + CacheMissError, + CheckLogger, + ReplayConfig, + diff_scratch_against_project, + run_replay, + ) + + config = ReplayConfig(project_root=project_root, lockfile_path=lockfile_path) + check_logger = CheckLogger(verbose=verbose) + try: + scratch = run_replay(config, check_logger) + except CacheMissError as exc: + logger.error(f"Cannot replay the locked install: {exc}") + sys.exit(1) + except (LocalResolutionError, NotImplementedError) as exc: + logger.error(f"Drift replay failed: {exc}") + sys.exit(1) + + targets = _resolve_diff_targets(project_root) + findings = diff_scratch_against_project(scratch, project_root, lockfile, targets) + + export = build_patch_export(project_root, scratch, lockfile, findings) + + for skip in export.skipped: + logger.warning(f"skipped {skip.deployed_path}: {skip.reason}") + + if not export.exported and not export.skipped: + logger.success("No local edits to managed files detected; nothing to export.") + return + + if not export.exported: + logger.warning( + f"{len(export.skipped)} modified managed file(s) found, but none " + "could be exported as a patch (see reasons above)." + ) + return + + for edit in export.exported: + logger.progress(f"{edit.deployed_path} -> {edit.package}:{edit.source_path}") + + if dry_run: + logger.success( + f"Dry run: would write {len(export.patches)} patch file(s) " + f"covering {len(export.exported)} edit(s)." + ) + return + + from ..utils.atomic_io import atomic_write_text + + filenames = patch_filenames(sorted(export.patches)) + try: + resolved_out.mkdir(parents=True, exist_ok=True) + for dep_key in sorted(export.patches): + target = resolved_out / filenames[dep_key] + atomic_write_text(target, export.patches[dep_key]) + logger.progress(f"wrote {target}") + except OSError as exc: + logger.error(f"Failed to write patch files: {exc}") + sys.exit(1) + + logger.success( + f"Exported {len(export.exported)} edit(s) into {len(export.patches)} " + f"patch file(s) under {resolved_out}. Apply each with 'git apply' from " + "the package repository root at the base noted in the patch header." + ) diff --git a/tests/integration/test_export_patch_e2e.py b/tests/integration/test_export_patch_e2e.py new file mode 100644 index 000000000..663d590e4 --- /dev/null +++ b/tests/integration/test_export_patch_e2e.py @@ -0,0 +1,156 @@ +"""End-to-end tests for ``apm export-patch``. + +The core contract is the closed loop: + + edit a deployed managed file + -> ``apm export-patch`` + -> ``git apply`` the patch in the package source + -> the package source now carries the edit + -> a replay against the updated source reports nothing to export + +If any link in that chain breaks (reverse mapping, diff shape, +``git apply`` compatibility, header base), the loop does not converge +and the test fails. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path +from typing import Any + +import pytest +import yaml +from click.testing import CliRunner + +from apm_cli.cli import cli +from apm_cli.deps.lockfile import LockedDependency, LockFile, get_lockfile_path +from apm_cli.install.cache_pin import write_marker + +_COMMIT = "b" * 40 +_SOURCE_REL = ".apm/instructions/std.instructions.md" +_DEPLOYED = ".github/instructions/std.instructions.md" +_ORIGINAL = b'---\napplyTo: "**"\n---\n# Standard\n\nrule one\n' +_EDITED = b'---\napplyTo: "**"\n---\n# Standard\n\nrule one\nrule two\n' + + +def _run(args: list[str]) -> Any: + return CliRunner().invoke(cli, args, catch_exceptions=False) + + +def _write(path: Path, content: bytes) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(content) + + +def _make_project_with_remote_dep(tmp_path: Path) -> tuple[Path, Path]: + """Fabricate a project whose lockfile pins one cached remote package. + + Mirrors the state ``apm install`` leaves behind for a GitHub dep: + package snapshot under ``apm_modules//`` with a cache-pin + marker, the deployed file in the project tree, and a lockfile entry + tracking it. + """ + project = tmp_path / "proj" + (project / ".github").mkdir(parents=True) + (project / "apm.yml").write_bytes( + yaml.safe_dump({"name": "proj", "version": "1.0.0", "target": "copilot"}).encode() + ) + + pkg_src = project / "apm_modules" / "testorg" / "testpkg" + _write(pkg_src / "apm.yml", yaml.safe_dump({"name": "testpkg", "version": "1.0.0"}).encode()) + _write(pkg_src / _SOURCE_REL, _ORIGINAL) + write_marker(pkg_src, _COMMIT) + + _write(project / _DEPLOYED, _ORIGINAL) + + lock = LockFile() + lock.add_dependency( + LockedDependency( + repo_url="testorg/testpkg", + resolved_commit=_COMMIT, + resolved_ref="main", + version="1.0.0", + deployed_files=[_DEPLOYED], + ) + ) + lock.write(get_lockfile_path(project)) + return project, pkg_src + + +def _git(cwd: Path, *args: str) -> subprocess.CompletedProcess: + return subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True, check=False) + + +@pytest.fixture +def project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> tuple[Path, Path]: + proj, pkg_src = _make_project_with_remote_dep(tmp_path) + monkeypatch.chdir(proj) + return proj, pkg_src + + +class TestExportPatchE2E: + def test_clean_project_exports_nothing(self, project) -> None: + result = _run(["export-patch"]) + assert result.exit_code == 0 + assert "nothing to export" in result.output.lower() + assert not (project[0] / "apm-patches").exists() + + def test_closed_loop_edit_export_apply_converges(self, project, tmp_path: Path) -> None: + if shutil.which("git") is None: + pytest.skip("git not available") + proj, pkg_src = project + + # 1. A local edit to the deployed managed file. + (proj / _DEPLOYED).write_bytes(_EDITED) + + # 2. Export it. + result = _run(["export-patch", "-o", "patches"]) + assert result.exit_code == 0, result.output + patch_file = proj / "patches" / "testorg-testpkg.patch" + assert patch_file.exists() + patch_text = patch_file.read_text(encoding="utf-8") + assert "# package: testorg/testpkg" in patch_text + assert f"# base: commit {_COMMIT} (main)" in patch_text + assert f"--- a/{_SOURCE_REL}\n" in patch_text + + # 3. Apply the patch in a clone of the package repository. + upstream = tmp_path / "upstream" + shutil.copytree(pkg_src, upstream, ignore=shutil.ignore_patterns(".apm-pin")) + assert _git(upstream, "init", "-q").returncode == 0 + applied = _git(upstream, "apply", str(patch_file)) + assert applied.returncode == 0, applied.stderr + + # 4. The package source now carries the local edit. Compare after + # CRLF normalization: git on Windows may rewrite line endings + # on apply (core.autocrlf), which drift tolerates by design. + patched = (upstream / _SOURCE_REL).read_bytes().replace(b"\r\n", b"\n") + assert patched == _EDITED + + # 5. Convergence: ship the patched source (simulated by updating + # the cached snapshot) and the same command finds nothing left + # to export. + (pkg_src / _SOURCE_REL).write_bytes(_EDITED) + result = _run(["export-patch", "-o", "patches2"]) + assert result.exit_code == 0 + assert "nothing to export" in result.output.lower() + assert not (proj / "patches2").exists() + + def test_dry_run_writes_nothing(self, project) -> None: + proj, _pkg_src = project + (proj / _DEPLOYED).write_bytes(_EDITED) + + result = _run(["export-patch", "--dry-run", "-o", "patches"]) + assert result.exit_code == 0, result.output + assert "Dry run" in result.output + assert not (proj / "patches").exists() + + def test_missing_cache_fails_with_guidance(self, project) -> None: + proj, pkg_src = project + (proj / _DEPLOYED).write_bytes(_EDITED) + shutil.rmtree(pkg_src) + + result = _run(["export-patch"]) + assert result.exit_code == 1 + assert "apm install" in result.output diff --git a/tests/unit/commands/test_export_patch.py b/tests/unit/commands/test_export_patch.py new file mode 100644 index 000000000..d1ad8d257 --- /dev/null +++ b/tests/unit/commands/test_export_patch.py @@ -0,0 +1,596 @@ +"""Unit tests for the ``apm export-patch`` core logic. + +The closed-loop contract (edit -> export -> ``git apply`` -> reinstall +-> clean drift) is proven end-to-end in +``tests/integration/test_export_patch_e2e.py``; these tests pin the +unit-level behaviors: reverse mapping by normalized content, raw-bytes +applicability gating, skip reasons, diff shape, filename collision +handling, and header sanitization. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml +from click.testing import CliRunner + +from apm_cli.commands.export_patch import ( + ExportedEdit, + _base_label, + _index_source_tree, + _patch_header, + _resolve_diff_targets, + _unified_diff, + build_patch_export, + export_patch, + patch_filename, + patch_filenames, +) +from apm_cli.deps.lockfile import LockedDependency, LockFile +from apm_cli.install.drift import CacheMissError, DriftFinding + +_COMMIT = "a" * 40 + + +def _write(path: Path, content: bytes) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(content) + + +def _lockfile_with_remote(deployed: list[str]) -> LockFile: + lock = LockFile() + dep = LockedDependency( + repo_url="testorg/testpkg", + resolved_commit=_COMMIT, + resolved_ref="main", + deployed_files=list(deployed), + ) + lock.add_dependency(dep) + return lock + + +def _remote_key(lock: LockFile) -> str: + return next(iter(lock.dependencies)) + + +@pytest.fixture +def trees(tmp_path: Path) -> dict[str, Path]: + """Project / scratch / package-source roots for a fabricated remote dep.""" + roots = { + "project": tmp_path / "project", + "scratch": tmp_path / "scratch", + "source": tmp_path / "pkg-src", + } + for root in roots.values(): + root.mkdir() + return roots + + +def _patch_materialize(monkeypatch: pytest.MonkeyPatch, source_root: Path) -> None: + monkeypatch.setattr( + "apm_cli.install.drift._materialize_install_path", + lambda *args, **kwargs: source_root, + ) + + +# --------------------------------------------------------------------------- +# patch_filename / patch_filenames +# --------------------------------------------------------------------------- + + +def test_patch_filename_sanitizes_separators(): + assert patch_filename("testorg/testpkg") == "testorg-testpkg.patch" + assert patch_filename("gitlab.com/group/sub/repo") == "gitlab.com-group-sub-repo.patch" + + +def test_patch_filename_never_empty(): + assert patch_filename("///") == "package.patch" + + +def test_patch_filename_ascii_only_and_reserved_stems(): + # Unicode letters must not survive: repo sanitizers are ASCII-only (#1217). + assert all(ord(c) < 128 for c in patch_filename("caf\u00e9/p\u00e4ck")) + # Windows reserved device names cannot be file stems. + assert patch_filename("nul") == "pkg-nul.patch" + assert patch_filename("CON") == "pkg-CON.patch" + + +def test_patch_filenames_disambiguates_collisions(): + # Both keys sanitize to 'org-pkg.patch'; the batch mapping must keep + # them distinct or one package's patch silently overwrites the other. + names = patch_filenames(["org/pkg", "org-pkg"]) + assert len(set(names.values())) == 2 + assert all(name.endswith(".patch") for name in names.values()) + + +def test_patch_filenames_stable_without_collisions(): + names = patch_filenames(["testorg/testpkg"]) + assert names == {"testorg/testpkg": "testorg-testpkg.patch"} + + +# --------------------------------------------------------------------------- +# _unified_diff +# --------------------------------------------------------------------------- + + +def test_unified_diff_shape(): + diff = _unified_diff("line1\nline2\n", "line1\nline2 edited\n", ".apm/instructions/x.md") + assert diff.startswith("--- a/.apm/instructions/x.md\n") + assert "+++ b/.apm/instructions/x.md\n" in diff + assert "-line2\n" in diff + assert "+line2 edited\n" in diff + + +def test_unified_diff_marks_missing_trailing_newline(): + diff = _unified_diff("old\n", "new", "f.md") + assert "+new\n\\ No newline at end of file\n" in diff + + +def test_unified_diff_uses_git_line_model_for_bare_cr(): + # str.splitlines would split on the bare CR and desync hunk counts + # from git's \n-only line model, injecting a spurious no-newline + # marker mid-hunk. + diff = _unified_diff("x\rY\n", "z\rY\n", "f.md") + assert "-x\rY\n" in diff + assert "+z\rY\n" in diff + assert "\\ No newline at end of file" not in diff + assert "@@ -1 +1 @@" in diff + + +# --------------------------------------------------------------------------- +# _index_source_tree +# --------------------------------------------------------------------------- + + +def test_index_normalizes_line_endings(tmp_path: Path): + import hashlib + + _write(tmp_path / "a.md", b"one\r\ntwo\r\n") + _write(tmp_path / "b.md", b"unique\n") + index = _index_source_tree(tmp_path) + assert index.by_digest[hashlib.sha256(b"one\ntwo\n").hexdigest()] == ["a.md"] + assert index.by_digest[hashlib.sha256(b"unique\n").hexdigest()] == ["b.md"] + assert index.unindexed == () + + +def test_index_excludes_cache_pin_marker(tmp_path: Path): + from apm_cli.install.cache_pin import MARKER_FILENAME + + _write(tmp_path / MARKER_FILENAME, b'{"schema_version": 1}') + index = _index_source_tree(tmp_path) + assert index.by_digest == {} + + +def test_index_excludes_git_and_pycache_dirs(tmp_path: Path): + # A .git blob byte-identical to a real source file must not make the + # reverse mapping ambiguous (mirrors utils/content_hash exclusions). + _write(tmp_path / ".apm" / "instructions" / "x.md", b"same\n") + _write(tmp_path / ".git" / "objects" / "blob", b"same\n") + _write(tmp_path / "__pycache__" / "x.md", b"same\n") + index = _index_source_tree(tmp_path) + assert list(index.by_digest.values()) == [[".apm/instructions/x.md"]] + + +def test_index_records_oversized_files_as_unindexed(tmp_path: Path, monkeypatch): + monkeypatch.setattr("apm_cli.commands.export_patch._MAX_INDEXED_BYTES", 4) + _write(tmp_path / "big.md", b"0123456789\n") + index = _index_source_tree(tmp_path) + assert index.by_digest == {} + assert index.unindexed == ("big.md",) + + +def test_index_groups_identical_content(tmp_path: Path): + _write(tmp_path / "one.md", b"same\n") + _write(tmp_path / "sub" / "two.md", b"same\n") + index = _index_source_tree(tmp_path) + assert list(index.by_digest.values()) == [["one.md", "sub/two.md"]] + + +# --------------------------------------------------------------------------- +# header sanitization +# --------------------------------------------------------------------------- + + +def test_header_values_cannot_inject_diff_lines(): + # Registry-controlled fields must never break out of the '# ' comment + # line: an embedded newline could smuggle attacker hunks that + # 'git apply' would happily apply. + dep = LockedDependency( + repo_url="evil/pkg", + source="registry", + version="1.0.0", + resolved_url="https://x.example\n--- a/.github/workflows/ci.yml\n+++ b/evil", + ) + label = _base_label(dep) + assert "\n" not in label + header = _patch_header("evil/pkg", dep) + # The security property: every header line stays a '#' comment, so no + # injected content can start a line with diff syntax. + for line in header.strip().splitlines(): + assert line.startswith("#"), line + + +def test_header_strips_url_userinfo_credentials(): + # Private-registry URLs can embed credentials (user:token@host); a + # patch file is made to be shared, so they must never reach the header. + dep = LockedDependency( + repo_url="org/pkg", + source="registry", + version="1.0.0", + source_url="https://alice:s3cret@registry.example/org/pkg", + resolved_url="https://alice:s3cret@registry.example/org/pkg/1.0.0.tar.gz", + ) + header = _patch_header("org/pkg", dep) + assert "s3cret" not in header + assert "alice" not in header + assert "# source: https://registry.example/org/pkg" in header + assert "(https://registry.example/org/pkg/1.0.0.tar.gz)" in header + + +def test_header_leaves_urls_without_userinfo_intact(): + dep = LockedDependency( + repo_url="org/pkg", + source="registry", + version="1.0.0", + resolved_url="https://registry.example/org/pkg?ref=a@b", + ) + # The '@' in the query string is not userinfo and must survive. + assert "https://registry.example/org/pkg?ref=a@b" in _base_label(dep) + # Non-URL sources (plain owner/repo keys) pass through untouched. + plain = LockedDependency(repo_url="owner/repo", resolved_commit="c" * 40) + assert "# source: owner/repo" in _patch_header("owner/repo", plain) + + +# --------------------------------------------------------------------------- +# build_patch_export +# --------------------------------------------------------------------------- + +_DEPLOYED = ".github/instructions/std.instructions.md" +_SOURCE_REL = ".apm/instructions/std.instructions.md" +_ORIGINAL = b"# Standard\n\nrule one\n" +_EDITED = b"# Standard\n\nrule one\nrule two\n" + + +def _exportable_setup(trees: dict[str, Path]) -> LockFile: + _write(trees["source"] / _SOURCE_REL, _ORIGINAL) + _write(trees["scratch"] / _DEPLOYED, _ORIGINAL) + _write(trees["project"] / _DEPLOYED, _EDITED) + return _lockfile_with_remote([_DEPLOYED]) + + +def test_exports_verbatim_edit_as_source_diff(trees, monkeypatch): + lock = _exportable_setup(trees) + _patch_materialize(monkeypatch, trees["source"]) + key = _remote_key(lock) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=key)] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.skipped == [] + assert result.exported == [ExportedEdit(_DEPLOYED, _SOURCE_REL, key)] + patch = result.patches[key] + assert f"# package: {key}" in patch + assert f"# base: commit {_COMMIT} (main)" in patch + assert f"--- a/{_SOURCE_REL}\n" in patch + assert "+rule two\n" in patch + + +def test_crlf_source_is_skipped_not_exported(trees, monkeypatch): + """The digest matches in normalized space, but a CRLF source would + reject every hunk at git-apply time -- must skip, not export.""" + _write(trees["source"] / _SOURCE_REL, _ORIGINAL.replace(b"\n", b"\r\n")) + _write(trees["scratch"] / _DEPLOYED, _ORIGINAL) + _write(trees["project"] / _DEPLOYED, _EDITED) + lock = _lockfile_with_remote([_DEPLOYED]) + _patch_materialize(monkeypatch, trees["source"]) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=_remote_key(lock))] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert result.exported == [] + assert len(result.skipped) == 1 + assert "CRLF" in result.skipped[0].reason + + +def test_identical_edits_to_two_deployed_copies_emit_one_diff(trees, monkeypatch): + deployed2 = ".claude/commands/std.md" + lock = _exportable_setup(trees) + _write(trees["scratch"] / deployed2, _ORIGINAL) + _write(trees["project"] / deployed2, _EDITED) + _patch_materialize(monkeypatch, trees["source"]) + key = _remote_key(lock) + findings = [ + DriftFinding(path=_DEPLOYED, kind="modified", package=key), + DriftFinding(path=deployed2, kind="modified", package=key), + ] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.skipped == [] + assert {e.deployed_path for e in result.exported} == {_DEPLOYED, deployed2} + # One source change: the patch must contain the diff exactly once, + # or git apply fails on the second identical hunk set. + assert result.patches[key].count(f"--- a/{_SOURCE_REL}\n") == 1 + + +def test_conflicting_edits_to_two_deployed_copies_are_skipped(trees, monkeypatch): + deployed2 = ".claude/commands/std.md" + lock = _exportable_setup(trees) + _write(trees["scratch"] / deployed2, _ORIGINAL) + _write(trees["project"] / deployed2, _ORIGINAL + b"different edit\n") + _patch_materialize(monkeypatch, trees["source"]) + key = _remote_key(lock) + findings = [ + DriftFinding(path=_DEPLOYED, kind="modified", package=key), + DriftFinding(path=deployed2, kind="modified", package=key), + ] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert result.exported == [] + assert len(result.skipped) == 2 + assert all("conflicting edits" in s.reason for s in result.skipped) + + +def test_transformed_content_is_skipped_with_reason(trees, monkeypatch): + lock = _exportable_setup(trees) + # Simulate a format-transformed deployment: the replayed content no + # longer matches any source file byte-for-byte. + _write(trees["scratch"] / _DEPLOYED, b"---\nglobs: '**'\n---\nrule one\n") + _patch_materialize(monkeypatch, trees["source"]) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=_remote_key(lock))] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "transformed" in result.skipped[0].reason + + +def test_unmatched_finding_mentions_unindexed_sources(trees, monkeypatch): + """A size-capped source must not be misreported as 'transformed' + without a hint that indexing was incomplete.""" + monkeypatch.setattr("apm_cli.commands.export_patch._MAX_INDEXED_BYTES", 4) + lock = _exportable_setup(trees) + _patch_materialize(monkeypatch, trees["source"]) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=_remote_key(lock))] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "not indexed" in result.skipped[0].reason + + +def test_ambiguous_source_is_skipped(trees, monkeypatch): + lock = _exportable_setup(trees) + _write(trees["source"] / ".apm/instructions/copy.instructions.md", _ORIGINAL) + _patch_materialize(monkeypatch, trees["source"]) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=_remote_key(lock))] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "ambiguous" in result.skipped[0].reason + + +def test_local_package_is_skipped(trees, monkeypatch): + lock = LockFile() + lock.add_dependency( + LockedDependency( + repo_url="_local/mypkg", + source="local", + local_path="./packages/mypkg", + deployed_files=[_DEPLOYED], + ) + ) + key = next(iter(lock.dependencies)) + _write(trees["scratch"] / _DEPLOYED, _ORIGINAL) + _write(trees["project"] / _DEPLOYED, _EDITED) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=key)] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "local package" in result.skipped[0].reason + + +def test_project_self_content_is_skipped(trees): + lock = LockFile() + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=".")] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "project-local" in result.skipped[0].reason + + +def test_binary_content_is_skipped(trees, monkeypatch): + lock = _exportable_setup(trees) + blob = b"\xff\xfe\x00binary" + _write(trees["source"] / ".apm/instructions/bin.dat", blob) + _write(trees["scratch"] / _DEPLOYED, blob) + _write(trees["project"] / _DEPLOYED, blob + b"\x01") + _patch_materialize(monkeypatch, trees["source"]) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=_remote_key(lock))] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "binary" in result.skipped[0].reason + + +def test_cache_miss_is_skipped_with_reason(trees, monkeypatch): + lock = _exportable_setup(trees) + + def _raise(*args, **kwargs): + raise CacheMissError("cache miss for testorg/testpkg") + + monkeypatch.setattr("apm_cli.install.drift._materialize_install_path", _raise) + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package=_remote_key(lock))] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "cache unavailable" in result.skipped[0].reason + + +def test_non_modified_findings_are_ignored(trees, monkeypatch): + lock = _exportable_setup(trees) + _patch_materialize(monkeypatch, trees["source"]) + key = _remote_key(lock) + findings = [ + DriftFinding(path=".github/instructions/gone.md", kind="unintegrated", package=key), + DriftFinding(path=".github/instructions/extra.md", kind="orphaned", package=key), + ] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert result.exported == [] + assert result.skipped == [] + + +def test_untracked_finding_is_skipped(trees): + lock = LockFile() + findings = [DriftFinding(path=_DEPLOYED, kind="modified", package="")] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.patches == {} + assert len(result.skipped) == 1 + assert "not tracked" in result.skipped[0].reason + + +def test_legacy_dir_tracked_finding_resolves_package(trees, monkeypatch): + """Legacy lockfiles track skill dirs with a trailing slash; a modified + file under such a dir must still resolve to the owning package.""" + deployed_dir = ".github/skills/helper/" + deployed_file = ".github/skills/helper/SKILL.md" + source_rel = ".apm/skills/helper/SKILL.md" + lock = _lockfile_with_remote([deployed_dir]) + key = _remote_key(lock) + _write(trees["source"] / source_rel, _ORIGINAL) + _write(trees["scratch"] / deployed_file, _ORIGINAL) + _write(trees["project"] / deployed_file, _EDITED) + _patch_materialize(monkeypatch, trees["source"]) + # Diff engine could not attribute the file (only the dir is tracked). + findings = [DriftFinding(path=deployed_file, kind="modified", package="")] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert result.exported == [ExportedEdit(deployed_file, source_rel, key)] + assert key in result.patches + + +def test_legacy_dir_tracked_local_content_gets_local_reason(trees): + """A legacy dir-tracked local_deployed_files entry must resolve to the + self key, not fall through to 'not tracked'.""" + deployed_file = ".github/skills/mine/SKILL.md" + lock = LockFile() + lock.local_deployed_files = [".github/skills/mine/"] + findings = [DriftFinding(path=deployed_file, kind="modified", package="")] + + result = build_patch_export(trees["project"], trees["scratch"], lock, findings) + + assert len(result.skipped) == 1 + assert "project-local" in result.skipped[0].reason + + +# --------------------------------------------------------------------------- +# CLI-level regression tests +# --------------------------------------------------------------------------- + + +def _make_cli_project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + project = tmp_path / "cliproj" + project.mkdir() + (project / "apm.yml").write_bytes( + yaml.safe_dump({"name": "cliproj", "version": "1.0.0", "target": "copilot"}).encode() + ) + lock = LockFile() + lock.add_dependency( + LockedDependency( + repo_url="testorg/testpkg", + resolved_commit=_COMMIT, + deployed_files=[_DEPLOYED], + ) + ) + from apm_cli.deps.lockfile import get_lockfile_path + + lock.write(get_lockfile_path(project)) + monkeypatch.chdir(project) + return project + + +def test_cli_unexpected_replay_error_exits_1_with_message(tmp_path, monkeypatch): + """Exceptions outside the anticipated replay surface must hit the + outer net (exit 1 + message), not escape as a traceback.""" + _make_cli_project(tmp_path, monkeypatch) + + def _boom(*args, **kwargs): + raise RuntimeError("boom") + + monkeypatch.setattr("apm_cli.install.drift.run_replay", _boom) + result = CliRunner().invoke(export_patch, [], catch_exceptions=False) + assert result.exit_code == 1 + assert "Error exporting patches: boom" in result.output + + +def test_cli_rejects_out_dir_inside_apm_modules(tmp_path, monkeypatch): + _make_cli_project(tmp_path, monkeypatch) + result = CliRunner().invoke(export_patch, ["-o", "apm_modules/evil"], catch_exceptions=False) + assert result.exit_code == 1 + assert "apm_modules" in result.output + + +def test_cli_reports_unparsable_lockfile_distinctly(tmp_path, monkeypatch): + project = _make_cli_project(tmp_path, monkeypatch) + from apm_cli.deps.lockfile import get_lockfile_path + + get_lockfile_path(project).write_text(":\nnot valid yaml [", encoding="utf-8") + result = CliRunner().invoke(export_patch, [], catch_exceptions=False) + assert result.exit_code == 1 + assert "could not be parsed" in result.output + assert "No lockfile found" not in result.output + + +def test_cli_all_local_lockfile_short_circuits_without_replay(tmp_path, monkeypatch): + project = _make_cli_project(tmp_path, monkeypatch) + from apm_cli.deps.lockfile import get_lockfile_path + + lock = LockFile() + lock.add_dependency( + LockedDependency(repo_url="_local/mypkg", source="local", local_path="./pkg") + ) + lock.write(get_lockfile_path(project)) + + def _fail(*args, **kwargs): + raise AssertionError("replay must not run for an all-local lockfile") + + monkeypatch.setattr("apm_cli.install.drift.run_replay", _fail) + result = CliRunner().invoke(export_patch, [], catch_exceptions=False) + assert result.exit_code == 0 + assert "nothing to" in result.output.lower() + + +def test_resolve_diff_targets_honors_apm_yml_target(tmp_path, monkeypatch): + """A declared target whose root dir does not exist must still be part + of the diff target set, or its findings silently vanish (#1924).""" + project = tmp_path / "tproj" + project.mkdir() + (project / "apm.yml").write_bytes( + yaml.safe_dump({"name": "tproj", "version": "1.0.0", "target": "claude"}).encode() + ) + monkeypatch.chdir(project) + names = [t.name for t in _resolve_diff_targets(project)] + assert "claude" in names