From 78b7cda22513482a9d5443c56da62e2fbbd9687f Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Fri, 18 Sep 2026 16:25:26 -0400 Subject: [PATCH 1/6] fix(install): discard lockfile with git conflict markers, fail closed under --frozen LockFile.read raises LockfileConflictError, a LockfileFormatError, when apm.lock.yaml contains git merge conflict markers, so every command names the file and a next action instead of printing a YAML scanner error. A full apm install and apm lock warn, discard the conflicted file, and resolve from apm.yml. Partial installs (positional packages, --only, --mcp) and apm install --frozen fail closed with the named error, and the frozen tip no longer points at apm outdated when the lockfile cannot be read. Fixes #2979 --- CHANGELOG.md | 1 + .../src/content/docs/reference/cli/install.md | 2 +- docs/src/content/docs/reference/cli/lock.md | 1 + .../content/docs/reference/lockfile-spec.md | 11 ++ .../docs/troubleshooting/install-failures.md | 10 + src/apm_cli/commands/install.py | 21 +- src/apm_cli/commands/lock.py | 12 +- src/apm_cli/deps/lockfile.py | 35 +++- src/apm_cli/install/errors.py | 8 +- src/apm_cli/install/mcp/command.py | 11 +- src/apm_cli/install/presentation/dry_run.py | 5 +- src/apm_cli/install/service.py | 8 +- .../test_install_conflicted_lockfile_e2e.py | 183 ++++++++++++++++++ .../deps/test_lockfile_conflict_markers.py | 110 +++++++++++ tests/unit/install/test_frozen.py | 33 +++- 15 files changed, 437 insertions(+), 14 deletions(-) create mode 100644 tests/integration/test_install_conflicted_lockfile_e2e.py create mode 100644 tests/unit/deps/test_lockfile_conflict_markers.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 44f9f98b05..a4412e46f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A full `apm install` and `apm lock` now warn, discard `apm.lock.yaml`, and resolve from `apm.yml` when the lockfile still contains git merge conflict markers, instead of exiting with a YAML parse error. `apm install --frozen`, partial installs, and read-only commands such as `apm update` and `apm outdated` fail closed with an error that names the conflict and the next action, and the `--frozen` failure tip no longer points at commands that cannot read the lockfile. (#2979) - Autopilot maintainer canvas removes a Decide row as soon as GitHub confirms `status/accepted`, without waiting for a full issue/PR refetch. - Issue and PR triage no longer skip bot-authored items (Copilot, Dependabot, github-actions). They stay in the queue like any other contribution. (#3024) - PR-review scheduler no longer queues every open pull request. A fresh review requires the `panel-review` label (same trigger as the Agentic Workflow), `status/accepted` on the PR, or an explicit named PR list. The reviewing session also requires `status/accepted` on the PR or a linked issue; otherwise scheduler and review-worker stop with no comment. The worker may clear `panel-review`; the scheduler does not comment or change labels. Both also apply a CODEOWNERS last-comment gate: read the last CODEOWNER comment as conditions and evaluate them against later comments AND labels on the PR and linked issues. Drop or `noop` only when those conditions are unmet or unclear. Named list does not bypass that gate. diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index 7b092b560d..af855fa12f 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -150,7 +150,7 @@ in `apm.yml`, then run `apm install` again. - **Lockfile replay and Git ref freshness.** Plain and `--frozen` installs may trust `apm.lock.yaml` and the local Git cache, reusing the locked commit for unchanged Git dependencies across the full resolved graph. In contrast, `apm install --update`, `apm install --refresh`, [`apm update`](../update/) with or without `--force`, [`apm lock --update`](../lock/), and [`apm outdated`](../outdated/) establish mutable Git refs from upstream instead of accepting stale refs from a local bare Git cache. APM picks up upstream changes to a transitive package's `apm.yml` only when you regenerate the graph -- run `apm update` or `apm lock --update`. See the [lockfile specification](../../lockfile-spec/) for the replay contract. - **Semver ranges on git deps.** `ref:` accepts semver ranges (`^1.2.0`, `~1.4`, `>=2.0 <3`, `1.5.x`) for git-source deps, including positional virtual-subdirectory references. APM runs `git ls-remote` against the dep, picks the highest tag matching the range, and pins the resolved tag plus commit SHA, version, and original constraint in `apm.lock.yaml`. Subsequent installs replay the lockfile without network; use `--update` (or change the manifest constraint) to re-resolve. See [manage dependencies](../../../consumer/manage-dependencies/#pin-a-semver-range) for the supported syntax. - **No-op nudge.** When the lockfile is already satisfied and nothing needs deploying, install prints `[i] Run 'apm update' to check for newer versions.` so you know the silent success was not a missed refresh. -- **Frozen mode.** With `--frozen`, install resolves only what is in `apm.lock.yaml`. A missing lockfile, a direct dependency missing from it, or MCP config state that differs from `apm.yml` exits `1` before lockfile, target config, deployment, or cache mutation. Cold-cache installs (empty `apm_modules/`) with git `apm_package` deps are tolerated: MCP checks are skipped for absent package directories (the packages will be hydrated by the pipeline), and their MCP server configs are restored from the lockfile so no false drift is reported. Remote `claude_skill` dependencies declared at a repository root or subdirectory are also accepted from their locked type before materialization; once present, the lock type and detected skill shape must agree. Missing local paths still fail. See [`config-consistency`](../../baseline-checks/#config-consistency) for the full manifest rule. Run normal `apm install` to create or repair MCP-only lock state, then retry frozen mode. Add-style invocations (`apm install PACKAGE` and `apm install --mcp NAME`) are rejected because they mutate `apm.yml`. Orphan package lock entries are tolerated; local-path deps are skipped. This is a structural check, not a content check -- run `apm audit --ci` for hash verification. +- **Frozen mode.** With `--frozen`, install resolves only what is in `apm.lock.yaml`. A missing lockfile, a direct dependency missing from it, or MCP config state that differs from `apm.yml` exits `1` before lockfile, target config, deployment, or cache mutation. Cold-cache installs (empty `apm_modules/`) with git `apm_package` deps are tolerated: MCP checks are skipped for absent package directories (the packages will be hydrated by the pipeline), and their MCP server configs are restored from the lockfile so no false drift is reported. Remote `claude_skill` dependencies declared at a repository root or subdirectory are also accepted from their locked type before materialization; once present, the lock type and detected skill shape must agree. Missing local paths still fail. A lockfile that contains git merge conflict markers also exits `1` and is never rewritten under `--frozen`. See [`config-consistency`](../../baseline-checks/#config-consistency) for the full manifest rule. Run normal `apm install` to create or repair MCP-only lock state, or to discard a conflicted lockfile and resolve from `apm.yml`, then retry frozen mode. Add-style invocations (`apm install PACKAGE` and `apm install --mcp NAME`) are rejected because they mutate `apm.yml`. Orphan package lock entries are tolerated; local-path deps are skipped. This is a structural check, not a content check -- run `apm audit --ci` for hash verification. - **Local `.apm/` deployment.** After dependencies are integrated, primitives in the project's own `.apm/` directory are deployed to the same targets. Local files win on collision. Skipped at `--global` and with `--only mcp`. - **User-scope root context hint.** Compilation stays explicit. After `apm install -g`, targets with native user-scope instruction files pick up global instructions during install. Targets whose user-scope instruction surface is a root context file require [`apm compile --global`](../compile/#global-compilation); install prints a one-line `[i]` hint and writes no root context file. - **OpenCode user scope.** `apm install -g --target opencode` deploys skills to diff --git a/docs/src/content/docs/reference/cli/lock.md b/docs/src/content/docs/reference/cli/lock.md index c4314428bc..198db8e910 100644 --- a/docs/src/content/docs/reference/cli/lock.md +++ b/docs/src/content/docs/reference/cli/lock.md @@ -72,6 +72,7 @@ apm lock --verbose - **Write `apm.lock.yaml`.** The lockfile records every pinned ref, resolved commit, and content hash. Fresh lock-only runs add no deployed files, and existing deployed-file rows, hashes, and deployment-ledger entries stay recorded while those bytes remain on disk. - **No files deployed or deleted.** The targets, cleanup, post-deps-local, and audit phases are skipped. The integrate phase runs but deploys nothing because the target set is empty. Running `apm lock` is safe to run before you are ready to install. - **Idempotent.** If the lockfile already matches the resolution result, it is overwritten with the same content. +- **Recovers from merge conflicts.** If `apm.lock.yaml` still contains git conflict markers, `apm lock` warns, discards it, and resolves from `apm.yml`. See [lockfile versioning](../../lockfile-spec/#versioning). ## Export (SBOM inventory) diff --git a/docs/src/content/docs/reference/lockfile-spec.md b/docs/src/content/docs/reference/lockfile-spec.md index 91323296e0..20b35735b9 100644 --- a/docs/src/content/docs/reference/lockfile-spec.md +++ b/docs/src/content/docs/reference/lockfile-spec.md @@ -393,6 +393,17 @@ constructs lock state. Pre-versioned legacy files migrate as v1 inputs. Fix or remove other invalid files explicitly; APM does not silently replace them with an empty lockfile. +A lockfile that still contains git merge conflict markers (`<<<<<<<`, +`>>>>>>>`, or `|||||||` at the start of a line) is reported as a merge +conflict rather than a YAML error. Every command that reads the lockfile names +the file and the next action. A full `apm install` (no package arguments, no +`--only`) or `apm lock` warns, discards the file, and resolves from +`apm.yml`, the same input a non-frozen install resolves from anyway; +deployed-file records from the conflicted file are not carried over, and a +manifest with nothing to lock ends with no lockfile. Partial installs and +`apm install --frozen` never rewrite the file: resolve the conflict in git, or +run a full `apm install` without `--frozen`. + ## Example A small project with one remote APM package, one MCP server, and its own diff --git a/docs/src/content/docs/troubleshooting/install-failures.md b/docs/src/content/docs/troubleshooting/install-failures.md index e9c3a76c11..8285897a50 100644 --- a/docs/src/content/docs/troubleshooting/install-failures.md +++ b/docs/src/content/docs/troubleshooting/install-failures.md @@ -175,6 +175,16 @@ apm install This re-resolves and rewrites `apm.lock.yaml`. Commit the result. +### Merge conflict markers in the lockfile + +After a git merge that touched `apm.lock.yaml`, the file may still contain `<<<<<<<` / `>>>>>>>` markers. Commands that read the lockfile then report `apm.lock.yaml contains git merge conflict markers`. A full install discards the file and resolves from `apm.yml`: + +```bash +apm install +``` + +APM warns that it is discarding the conflicted file, then writes a fresh lockfile. Commit the result. `apm lock` does the same without deploying files. Partial installs (`apm install PACKAGE`, `--only apm`, `--only mcp`, `--mcp NAME`) and `apm install --frozen` refuse to rewrite the file; resolve the conflict in git first, or run a full `apm install` without `--frozen`. A lockfile that is invalid for any other reason still fails closed in every mode. + ### Drifted refs To force re-resolution to the latest version or Git ref allowed by `apm.yml`: diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 84b5a9efad..647cb6d377 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -224,9 +224,17 @@ class InstallContext: # APM Dependencies (conditional import for graceful degradation) APM_DEPS_AVAILABLE = False _APM_IMPORT_ERROR = None +_LOCKFILE_CONFLICT_DISCARDED = ( + "apm.lock.yaml contains git merge conflict markers; discarding it and resolving from apm.yml." +) try: from ..deps.apm_resolver import APMDependencyResolver - from ..deps.lockfile import LockFile, get_lockfile_path, migrate_lockfile_if_needed + from ..deps.lockfile import ( + LockFile, + discard_conflicted_lockfile, + get_lockfile_path, + migrate_lockfile_if_needed, + ) from ..integration.mcp_integrator import ( MCPIntegrator, # noqa: F401 -- re-exported; tests patch commands.install.MCPIntegrator ) @@ -1643,7 +1651,8 @@ def install( # noqa: PLR0913 logger.error(str(e)) for reason in e.reasons: logger.error_detail(reason) - logger.info(_frozen_install_tip(e)) + if tip := _frozen_install_tip(e): + logger.info(tip) command_result = ( transaction.fail(e) if transaction is not None @@ -1822,6 +1831,11 @@ def _install_apm_packages(ctx, outcome): # Migrate legacy apm.lock -> apm.lock.yaml if needed (one-time, transparent) migrate_lockfile_if_needed(ctx.apm_dir) + # Only a full install re-resolves every apm.yml entry; a partial add or + # --only run would write a lockfile missing the other entries. + full_install = not ctx.frozen and not ctx.packages and ctx.install_mode == InstallMode.ALL + if full_install and discard_conflicted_lockfile(get_lockfile_path(ctx.apm_dir)): + logger.warning(_LOCKFILE_CONFLICT_DISCARDED) # Capture old MCP servers and configs from lockfile BEFORE # _install_apm_dependencies regenerates it (which drops the fields). @@ -1928,7 +1942,8 @@ def _install_apm_packages(ctx, outcome): logger.error(str(e)) for reason in e.reasons: logger.error_detail(reason) - logger.info(_frozen_install_tip(e)) + if tip := _frozen_install_tip(e): + logger.info(tip) raise InstallFailureAlreadyRendered(str(e)) from e except InstallFailureAlreadyRendered: raise diff --git a/src/apm_cli/commands/lock.py b/src/apm_cli/commands/lock.py index 922316bd88..bde1ea1ac8 100644 --- a/src/apm_cli/commands/lock.py +++ b/src/apm_cli/commands/lock.py @@ -223,8 +223,14 @@ def _run_lock( logger = InstallLogger(verbose=verbose) try: - from apm_cli.commands.install import _install_apm_dependencies + from apm_cli.commands.install import ( + _LOCKFILE_CONFLICT_DISCARDED, + _install_apm_dependencies, + ) + from apm_cli.deps.lockfile import discard_conflicted_lockfile, get_lockfile_path + if discard_conflicted_lockfile(get_lockfile_path(project_root)): + logger.warning(_LOCKFILE_CONFLICT_DISCARDED) result = _install_apm_dependencies( apm_package, update_refs=update_refs, @@ -304,11 +310,11 @@ def lock_export(fmt: str, output: str | None, global_: bool, timestamp: str | No project_root = manifest_path.parent if manifest_path else Path.cwd().resolve() lockfile_path = get_lockfile_path(project_root) - if not lockfile_path.is_file(): + lockfile = LockFile.read(lockfile_path) + if lockfile is None: _rich_error(f"No lockfile found at {lockfile_path}. Run 'apm lock' to generate one first.") sys.exit(1) - lockfile = LockFile.from_yaml(lockfile_path.read_text(encoding="utf-8")) resolved_timestamp = _resolve_export_timestamp(timestamp, lockfile.generated_at) document = export_sbom(lockfile, fmt, timestamp=resolved_timestamp) diff --git a/src/apm_cli/deps/lockfile.py b/src/apm_cli/deps/lockfile.py index 7fb23cfdd5..ae6bdd654e 100644 --- a/src/apm_cli/deps/lockfile.py +++ b/src/apm_cli/deps/lockfile.py @@ -7,6 +7,7 @@ import logging import os +import re from dataclasses import dataclass, field from datetime import datetime, timezone from pathlib import Path @@ -77,6 +78,25 @@ class UnsupportedLockfileVersionError(LockfileFormatError): """Raised when a lockfile declares a version this client cannot read.""" +class LockfileConflictError(LockfileFormatError): + """Raised when a lockfile still contains git merge conflict markers.""" + + def __init__(self, path: Path) -> None: + self.path = path + super().__init__( + f"{path} contains git merge conflict markers. Resolve the merge " + "conflict, or run 'apm install' to regenerate it from apm.yml." + ) + + +_CONFLICT_MARKER_RE = re.compile(r"^(?:<{7}|>{7}|\|{7})(?: |$)", re.MULTILINE) + + +def has_conflict_markers(text: str) -> bool: + """Return True when *text* contains a git merge conflict marker at line start.""" + return _CONFLICT_MARKER_RE.search(text) is not None + + def require_supported_lockfile_version(data: object) -> str: """Return a supported declared version or fail closed.""" if not isinstance(data, dict): @@ -1034,11 +1054,14 @@ def write( @classmethod def read(cls, path: Path) -> LockFile | None: - """Read lock file from disk. Returns None if not exists or corrupt.""" + """Read lock file from disk. Returns None when the file does not exist.""" if not path.exists(): return None + text = path.read_text(encoding="utf-8") + if has_conflict_markers(text): + raise LockfileConflictError(path) try: - return cls.from_yaml(path.read_text(encoding="utf-8")) + return cls.from_yaml(text) except (LockfileFormatError, UnsupportedLockfileVersionError): raise except (yaml.YAMLError, ValueError, KeyError, TypeError) as exc: @@ -1264,6 +1287,14 @@ def migrate_lockfile_if_needed(project_root: Path) -> bool: return False +def discard_conflicted_lockfile(path: Path) -> bool: + """Delete a lockfile left with git merge conflict markers so install regenerates it.""" + if not path.exists() or not has_conflict_markers(path.read_text(encoding="utf-8")): + return False + path.unlink() + return True + + def get_lockfile_installed_paths(project_root: Path) -> list[str]: """Deprecated: use LockFile.installed_paths_for_project() instead.""" return LockFile.installed_paths_for_project(project_root) diff --git a/src/apm_cli/install/errors.py b/src/apm_cli/install/errors.py index cd635ad59a..755601002c 100644 --- a/src/apm_cli/install/errors.py +++ b/src/apm_cli/install/errors.py @@ -94,7 +94,13 @@ def __init__(self, message: str, *, reasons: list[str] | None = None): def frozen_install_tip(error: FrozenInstallError) -> str: - """Return recovery guidance tailored to package or MCP lock drift.""" + """Return recovery guidance tailored to package or MCP lock drift. + + Returns an empty string when the error carries no drift reasons, because + a missing or unreadable lockfile message already names its next action. + """ + if not error.reasons: + return "" has_mcp_drift = any("MCP server" in reason for reason in error.reasons) has_package_drift = any("MCP server" not in reason for reason in error.reasons) if has_mcp_drift and has_package_drift: diff --git a/src/apm_cli/install/mcp/command.py b/src/apm_cli/install/mcp/command.py index ec00d5c063..0ac53c2cfb 100644 --- a/src/apm_cli/install/mcp/command.py +++ b/src/apm_cli/install/mcp/command.py @@ -34,7 +34,12 @@ # code paths (package install vs. MCP install). APM_DEPS_AVAILABLE = False try: - from ...deps.lockfile import LockFile, get_lockfile_path, migrate_lockfile_if_needed + from ...deps.lockfile import ( + LockFile, + LockfileFormatError, + get_lockfile_path, + migrate_lockfile_if_needed, + ) from ...integration.mcp_integrator import MCPIntegrator APM_DEPS_AVAILABLE = True @@ -300,6 +305,10 @@ def run_mcp_install( # noqa: PLR0913 ) except InstallFailureAlreadyRendered: raise + except LockfileFormatError as exc: + logger.error(str(exc)) + logger.error("MCP server written to apm.yml but tool integration failed.") + raise click.ClickException(f"MCP integration failed for '{mcp_name}'") from exc except Exception as exc: # Keep the raw exception (which may contain internal paths, # credentials, or stack-trace fragments) at verbose level diff --git a/src/apm_cli/install/presentation/dry_run.py b/src/apm_cli/install/presentation/dry_run.py index bb7d13f433..e200502f4f 100644 --- a/src/apm_cli/install/presentation/dry_run.py +++ b/src/apm_cli/install/presentation/dry_run.py @@ -28,7 +28,7 @@ def render_and_exit( The caller is responsible for ``return``-ing after this function completes -- this function does NOT exit or return early on its own. """ - from apm_cli.deps.lockfile import LockFile, get_lockfile_path + from apm_cli.deps.lockfile import LockFile, LockfileConflictError, get_lockfile_path from apm_cli.drift import detect_orphans logger.progress("Dry run mode - showing what would change:") @@ -81,6 +81,9 @@ def render_and_exit( # required, accurate to compute. try: _dryrun_lock = LockFile.read(get_lockfile_path(apm_dir)) + except LockfileConflictError as exc: + logger.warning(str(exc)) + _dryrun_lock = None except Exception: _dryrun_lock = None if _dryrun_lock: diff --git a/src/apm_cli/install/service.py b/src/apm_cli/install/service.py index 9dd5699dfd..fb4ac3abff 100644 --- a/src/apm_cli/install/service.py +++ b/src/apm_cli/install/service.py @@ -263,7 +263,7 @@ def enforce_frozen(request: InstallRequest) -> None: """ from pathlib import Path - from apm_cli.deps.lockfile import LockFile + from apm_cli.deps.lockfile import LockFile, LockfileConflictError from apm_cli.install.errors import FrozenInstallError from apm_cli.install.plan import lockfile_satisfies_manifest @@ -284,6 +284,12 @@ def enforce_frozen(request: InstallRequest) -> None: try: lockfile = LockFile.read(lockfile_path) + except LockfileConflictError as e: + raise FrozenInstallError( + "--frozen cannot use apm.lock.yaml: it contains git merge conflict " + "markers. Resolve the merge conflict, or run 'apm install' without " + "--frozen to regenerate it from apm.yml.", + ) from e except Exception as e: raise FrozenInstallError( f"--frozen could not read apm.lock.yaml: {e}", diff --git a/tests/integration/test_install_conflicted_lockfile_e2e.py b/tests/integration/test_install_conflicted_lockfile_e2e.py new file mode 100644 index 0000000000..aa74b8fb8f --- /dev/null +++ b/tests/integration/test_install_conflicted_lockfile_e2e.py @@ -0,0 +1,183 @@ +"""End-to-end recovery from git merge conflict markers in ``apm.lock.yaml`` (#2979). + +A full ``apm install`` and ``apm lock`` discard the lockfile and resolve from +``apm.yml`` with a warning. ``--frozen``, partial installs, and read-only +commands fail closed with an error that names the file and a working next +action. +""" + +from __future__ import annotations + +import textwrap +from pathlib import Path +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + +from apm_cli.deps.lockfile import LockFile, LockfileConflictError +from apm_cli.models.apm_package import clear_apm_yml_cache + +_PATCH_UPDATES = "apm_cli.commands._helpers.check_for_updates" + +_CONFLICTED_LOCKFILE = textwrap.dedent("""\ + lockfile_version: '1' + <<<<<<< HEAD + dependencies: [] + ======= + dependencies: + - repo_url: example/x + >>>>>>> feature +""") + + +@pytest.fixture +def runner() -> CliRunner: + return CliRunner() + + +@pytest.fixture(autouse=True) +def _clear_cache() -> None: + clear_apm_yml_cache() + yield + clear_apm_yml_cache() + + +@pytest.fixture +def conflicted_project(tmp_path: Path, monkeypatch) -> Path: + monkeypatch.chdir(tmp_path) + (tmp_path / "apm.yml").write_text( + textwrap.dedent("""\ + name: test-project + version: '1.0.0' + targets: + - claude + """), + encoding="utf-8", + ) + instructions = tmp_path / ".apm" / "instructions" + instructions.mkdir(parents=True) + (instructions / "hi.instructions.md").write_text("hello\n", encoding="utf-8") + (tmp_path / "apm.lock.yaml").write_text(_CONFLICTED_LOCKFILE, encoding="utf-8") + return tmp_path + + +def _invoke(runner: CliRunner, args: list[str], *, catch_exceptions: bool = False): + from apm_cli.cli import cli + + with patch(_PATCH_UPDATES, return_value=None): + return runner.invoke(cli, args, catch_exceptions=catch_exceptions) + + +def _combined_output(result) -> str: + return " ".join(((result.output or "") + (result.stderr or "")).split()) + + +@pytest.mark.parametrize("args", [["install"], ["lock"]]) +def test_regenerates_lockfile_with_warning( + runner: CliRunner, conflicted_project: Path, args: list[str] +) -> None: + result = _invoke(runner, args) + + assert result.exit_code == 0, result.output + assert "conflict markers" in _combined_output(result) + assert "resolving from apm.yml" in _combined_output(result) + lockfile_path = conflicted_project / "apm.lock.yaml" + assert "<<<<<<<" not in lockfile_path.read_text(encoding="utf-8") + lock = LockFile.read(lockfile_path) + assert lock is not None + assert lock.get_package_dependencies() == [] + + +def test_install_records_local_content_after_regeneration( + runner: CliRunner, conflicted_project: Path +) -> None: + result = _invoke(runner, ["install"]) + + assert result.exit_code == 0, result.output + lock = LockFile.read(conflicted_project / "apm.lock.yaml") + assert lock is not None + assert lock.local_deployed_files == [".claude/rules/hi.md"] + + +def test_frozen_install_fails_closed_and_leaves_file_untouched( + runner: CliRunner, conflicted_project: Path +) -> None: + result = _invoke(runner, ["install", "--frozen"]) + + assert result.exit_code == 1 + output = _combined_output(result) + assert "conflict markers" in output + assert "without --frozen" in output + assert "apm outdated" not in output + lockfile_text = (conflicted_project / "apm.lock.yaml").read_text(encoding="utf-8") + assert lockfile_text == _CONFLICTED_LOCKFILE + + +@pytest.mark.parametrize("args", [["update"], ["outdated"], ["lock", "export"]]) +def test_read_only_commands_name_the_conflict( + runner: CliRunner, conflicted_project: Path, args: list[str] +) -> None: + """These commands let the error reach ``main()``, which prints ``Error: {exc}``.""" + result = _invoke(runner, args, catch_exceptions=True) + + assert result.exit_code == 1 + assert isinstance(result.exception, LockfileConflictError) + message = str(result.exception) + assert "apm.lock.yaml contains git merge conflict markers" in message + assert "run 'apm install'" in message + lockfile_text = (conflicted_project / "apm.lock.yaml").read_text(encoding="utf-8") + assert lockfile_text == _CONFLICTED_LOCKFILE + + +@pytest.mark.parametrize( + "args", + [ + ["install", "./pkg"], + ["install", "--only", "apm"], + ["install", "--only", "mcp"], + ["install", "--mcp", "foo", "--url", "http://127.0.0.1:1/mcp"], + ], +) +def test_partial_installs_fail_closed_and_name_the_conflict( + runner: CliRunner, conflicted_project: Path, args: list[str] +) -> None: + """A partial install cannot re-resolve every apm.yml entry, so it never discards.""" + pkg = conflicted_project / "pkg" / ".apm" / "instructions" + pkg.mkdir(parents=True) + (conflicted_project / "pkg" / "apm.yml").write_text("name: pkg\nversion: '1.0.0'\n") + (pkg / "p.instructions.md").write_text("pkg\n", encoding="utf-8") + + result = _invoke(runner, args) + + assert result.exit_code == 1, result.output + output = _combined_output(result) + assert "apm.lock.yaml contains git merge conflict markers" in output + assert "run 'apm install'" in output + lockfile_text = (conflicted_project / "apm.lock.yaml").read_text(encoding="utf-8") + assert lockfile_text == _CONFLICTED_LOCKFILE + + +def test_dry_run_names_the_conflict_without_touching_the_file( + runner: CliRunner, conflicted_project: Path +) -> None: + result = _invoke(runner, ["install", "--dry-run"]) + + assert result.exit_code == 0, result.output + assert "apm.lock.yaml contains git merge conflict markers" in _combined_output(result) + lockfile_text = (conflicted_project / "apm.lock.yaml").read_text(encoding="utf-8") + assert lockfile_text == _CONFLICTED_LOCKFILE + + +def test_corrupt_lockfile_without_markers_still_fails_closed( + runner: CliRunner, conflicted_project: Path +) -> None: + lockfile_path = conflicted_project / "apm.lock.yaml" + corrupt = "lockfile_version: '1'\ndependencies: [\n" + lockfile_path.write_text(corrupt, encoding="utf-8") + + result = _invoke(runner, ["install"]) + + assert result.exit_code == 1, result.output + assert "conflict markers" not in _combined_output(result) + assert lockfile_path.read_text(encoding="utf-8") == corrupt diff --git a/tests/unit/deps/test_lockfile_conflict_markers.py b/tests/unit/deps/test_lockfile_conflict_markers.py new file mode 100644 index 0000000000..46177d98a5 --- /dev/null +++ b/tests/unit/deps/test_lockfile_conflict_markers.py @@ -0,0 +1,110 @@ +"""Git merge conflict markers in ``apm.lock.yaml`` (#2979).""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from apm_cli.deps.lockfile import ( + LockFile, + LockfileConflictError, + LockfileFormatError, + discard_conflicted_lockfile, +) + +pytestmark = pytest.mark.unit + +_VALID = "lockfile_version: '1'\ndependencies: []\n" + +_CONFLICTED = ( + "lockfile_version: '1'\n" + "<<<<<<< HEAD\n" + "dependencies: []\n" + "=======\n" + "dependencies:\n" + "- repo_url: example/x\n" + ">>>>>>> feature\n" +) + +_CONFLICTED_DIFF3 = ( + "lockfile_version: '1'\n" + "<<<<<<< HEAD\n" + "dependencies: []\n" + "||||||| merged common ancestors\n" + "dependencies:\n" + "- repo_url: example/base\n" + "=======\n" + "dependencies:\n" + "- repo_url: example/x\n" + ">>>>>>> feature\n" +) + + +@pytest.mark.parametrize("text", [_CONFLICTED, _CONFLICTED_DIFF3]) +def test_read_names_conflict_markers_and_next_action(tmp_path: Path, text: str) -> None: + path = tmp_path / "apm.lock.yaml" + path.write_text(text, encoding="utf-8") + + with pytest.raises(LockfileConflictError) as exc_info: + LockFile.read(path) + + message = str(exc_info.value) + assert str(path) in message + assert "conflict markers" in message + assert "apm install" in message + assert exc_info.value.path == path + + +def test_conflict_error_is_a_format_error() -> None: + assert issubclass(LockfileConflictError, LockfileFormatError) + + +def test_read_corrupt_lockfile_without_markers_is_a_plain_format_error(tmp_path: Path) -> None: + path = tmp_path / "apm.lock.yaml" + path.write_text("lockfile_version: '1'\ndependencies: [\n", encoding="utf-8") + + with pytest.raises(LockfileFormatError) as exc_info: + LockFile.read(path) + + assert not isinstance(exc_info.value, LockfileConflictError) + + +def test_read_accepts_marker_text_that_is_not_at_line_start(tmp_path: Path) -> None: + path = tmp_path / "apm.lock.yaml" + path.write_text( + "lockfile_version: '1'\n" + "dependencies:\n" + "- repo_url: example/x\n" + " resolved_ref: 'tag <<<<<<< HEAD >>>>>>> end'\n", + encoding="utf-8", + ) + + lock = LockFile.read(path) + + assert lock is not None + assert lock.dependencies["example/x"].resolved_ref == "tag <<<<<<< HEAD >>>>>>> end" + + +def test_read_treats_separator_only_line_as_ordinary_format_error(tmp_path: Path) -> None: + path = tmp_path / "apm.lock.yaml" + path.write_text("lockfile_version: '1'\n=======\ndependencies: []\n", encoding="utf-8") + + with pytest.raises(LockfileFormatError) as exc_info: + LockFile.read(path) + + assert not isinstance(exc_info.value, LockfileConflictError) + + +def test_discard_removes_only_a_conflicted_lockfile(tmp_path: Path) -> None: + path = tmp_path / "apm.lock.yaml" + + assert discard_conflicted_lockfile(path) is False + + path.write_text(_VALID, encoding="utf-8") + assert discard_conflicted_lockfile(path) is False + assert path.read_text(encoding="utf-8") == _VALID + + path.write_text(_CONFLICTED, encoding="utf-8") + assert discard_conflicted_lockfile(path) is True + assert not path.exists() diff --git a/tests/unit/install/test_frozen.py b/tests/unit/install/test_frozen.py index 259f1dae21..3a41f35233 100644 --- a/tests/unit/install/test_frozen.py +++ b/tests/unit/install/test_frozen.py @@ -11,7 +11,7 @@ import pytest from apm_cli.deps.lockfile import LockedDependency, LockFile -from apm_cli.install.errors import FrozenInstallError +from apm_cli.install.errors import FrozenInstallError, frozen_install_tip from apm_cli.install.request import InstallRequest from apm_cli.install.service import InstallService from apm_cli.models.dependency.reference import DependencyReference @@ -77,6 +77,37 @@ def test_succeeds_when_lockfile_has_all_manifest_deps(self, tmp_path: Path): InstallService.enforce_frozen(req) + def test_conflict_markers_fail_closed_with_named_next_action(self, tmp_path: Path): + """#2979: --frozen never regenerates; the error names the conflict and the way out.""" + _write_apm_yml(tmp_path) + conflicted = ( + "lockfile_version: '1'\n" + "<<<<<<< HEAD\n" + "dependencies: []\n" + "=======\n" + "dependencies:\n" + "- repo_url: example/x\n" + ">>>>>>> feature\n" + ) + (tmp_path / "apm.lock.yaml").write_text(conflicted) + req = _make_request(project_dir=tmp_path, manifest_deps=[]) + + with pytest.raises(FrozenInstallError, match="conflict markers") as exc_info: + InstallService.enforce_frozen(req) + + assert "without --frozen" in str(exc_info.value) + assert "apm outdated" not in frozen_install_tip(exc_info.value) + assert (tmp_path / "apm.lock.yaml").read_text() == conflicted + + def test_missing_lockfile_tip_does_not_point_at_unreadable_commands(self, tmp_path: Path): + _write_apm_yml(tmp_path) + req = _make_request(project_dir=tmp_path, manifest_deps=[]) + + with pytest.raises(FrozenInstallError) as exc_info: + InstallService.enforce_frozen(req) + + assert frozen_install_tip(exc_info.value) == "" + def test_orphan_lockfile_entries_dont_fail(self, tmp_path: Path): """Mirrors npm ci: extra lock entries are tolerated; only direct deps must be present.""" _write_apm_yml(tmp_path) From 855d8eede4f40717e9b1b8408ed32a7781f375b1 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Fri, 18 Sep 2026 18:23:57 -0400 Subject: [PATCH 2/6] fix(install): normalise undecodable lockfiles and name frozen next actions Keep the conflict-marker read inside LockFile.read's normalising try so a non-UTF-8 lockfile still fails through LockfileFormatError, and leave such a file in place instead of probing it for discard. The generic --frozen unreadable-lockfile error now names the repair step. apm lock export resolves a legacy apm.lock like the other read-only consumers. Narrow the docs claim to commands that require the lockfile, update the apm-guide skill resources, mark the new test modules as component tests, and cite the PR number in the changelog. --- CHANGELOG.md | 2 +- .../content/docs/reference/lockfile-spec.md | 5 +++-- .../docs/troubleshooting/install-failures.md | 2 +- .../.apm/skills/apm-usage/commands.md | 10 ++++----- .../.apm/skills/apm-usage/troubleshooting.md | 1 + src/apm_cli/commands/lock.py | 4 ++-- src/apm_cli/deps/lockfile.py | 14 ++++++++---- src/apm_cli/install/service.py | 3 ++- .../test_install_conflicted_lockfile_e2e.py | 2 ++ .../deps/test_lockfile_conflict_markers.py | 22 ++++++++++++++++++- tests/unit/install/test_frozen.py | 11 ++++++++++ 11 files changed, 59 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4412e46f7..68efa8ab02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- A full `apm install` and `apm lock` now warn, discard `apm.lock.yaml`, and resolve from `apm.yml` when the lockfile still contains git merge conflict markers, instead of exiting with a YAML parse error. `apm install --frozen`, partial installs, and read-only commands such as `apm update` and `apm outdated` fail closed with an error that names the conflict and the next action, and the `--frozen` failure tip no longer points at commands that cannot read the lockfile. (#2979) +- A full `apm install` and `apm lock` now warn, discard `apm.lock.yaml`, and resolve from `apm.yml` when the lockfile still contains git merge conflict markers, instead of exiting with a YAML parse error. `apm install --frozen`, partial installs, and read-only commands such as `apm update` and `apm outdated` fail closed with an error that names the conflict and the next action, and the `--frozen` failure tip no longer points at commands that cannot read the lockfile. (closes #2979) (#3028) - Autopilot maintainer canvas removes a Decide row as soon as GitHub confirms `status/accepted`, without waiting for a full issue/PR refetch. - Issue and PR triage no longer skip bot-authored items (Copilot, Dependabot, github-actions). They stay in the queue like any other contribution. (#3024) - PR-review scheduler no longer queues every open pull request. A fresh review requires the `panel-review` label (same trigger as the Agentic Workflow), `status/accepted` on the PR, or an explicit named PR list. The reviewing session also requires `status/accepted` on the PR or a linked issue; otherwise scheduler and review-worker stop with no comment. The worker may clear `panel-review`; the scheduler does not comment or change labels. Both also apply a CODEOWNERS last-comment gate: read the last CODEOWNER comment as conditions and evaluate them against later comments AND labels on the PR and linked issues. Drop or `noop` only when those conditions are unmet or unclear. Named list does not bypass that gate. diff --git a/docs/src/content/docs/reference/lockfile-spec.md b/docs/src/content/docs/reference/lockfile-spec.md index 20b35735b9..81c17d1a02 100644 --- a/docs/src/content/docs/reference/lockfile-spec.md +++ b/docs/src/content/docs/reference/lockfile-spec.md @@ -395,8 +395,9 @@ an empty lockfile. A lockfile that still contains git merge conflict markers (`<<<<<<<`, `>>>>>>>`, or `|||||||` at the start of a line) is reported as a merge -conflict rather than a YAML error. Every command that reads the lockfile names -the file and the next action. A full `apm install` (no package arguments, no +conflict rather than a YAML error. Commands that require the lockfile name +the file and the next action; best-effort readers such as `apm view` continue +without lock metadata as before. A full `apm install` (no package arguments, no `--only`) or `apm lock` warns, discards the file, and resolves from `apm.yml`, the same input a non-frozen install resolves from anyway; deployed-file records from the conflicted file are not carried over, and a diff --git a/docs/src/content/docs/troubleshooting/install-failures.md b/docs/src/content/docs/troubleshooting/install-failures.md index 8285897a50..3d92f5ebf2 100644 --- a/docs/src/content/docs/troubleshooting/install-failures.md +++ b/docs/src/content/docs/troubleshooting/install-failures.md @@ -177,7 +177,7 @@ This re-resolves and rewrites `apm.lock.yaml`. Commit the result. ### Merge conflict markers in the lockfile -After a git merge that touched `apm.lock.yaml`, the file may still contain `<<<<<<<` / `>>>>>>>` markers. Commands that read the lockfile then report `apm.lock.yaml contains git merge conflict markers`. A full install discards the file and resolves from `apm.yml`: +After a git merge that touched `apm.lock.yaml`, the file may still contain `<<<<<<<` / `>>>>>>>` markers. Commands that require the lockfile then report `apm.lock.yaml contains git merge conflict markers`. A full install discards the file and resolves from `apm.yml`: ```bash apm install diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index ec8d5b5045..732186ac1d 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -12,11 +12,11 @@ | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing or out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; a newly bootstrapped manifest and explicit targets are kept), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use explicit target `hermes` to deploy skills and home-scoped MCP servers to `$HERMES_HOME/config.yaml` (or `~/.hermes/config.yaml` when unset or blank); run `apm compile` separately for `AGENTS.md`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when selected explicitly), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | -| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing or out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; a newly bootstrapped manifest and explicit targets are kept), `--verbose`, `--only [apm\|mcp]` (APM packages or MCP/LSP service dependencies), `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | -| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing or out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; project bootstrap keeps its new manifest and explicit targets, while absent global `-g` bootstrap uses temporary preview state and does not create `~/.apm`), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | -| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing or out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; project bootstrap keeps its new manifest and explicit targets, while absent global `-g` bootstrap uses temporary preview state and does not create `~/.apm`), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them, except absent global dry-runs only preview targets in temporary state; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | -| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing or out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; project bootstrap keeps its new manifest and explicit targets, while absent global `-g` bootstrap uses temporary preview state and does not create `~/.apm`), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them, except absent global dry-runs only preview targets in temporary state; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; direct `--mcp` additions are project-scoped and are not valid with `-g`/`--global`), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | +| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing, contains git merge conflict markers, or is out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; a newly bootstrapped manifest and explicit targets are kept), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use explicit target `hermes` to deploy skills and home-scoped MCP servers to `$HERMES_HOME/config.yaml` (or `~/.hermes/config.yaml` when unset or blank); run `apm compile` separately for `AGENTS.md`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when selected explicitly), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | +| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing, contains git merge conflict markers, or is out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; a newly bootstrapped manifest and explicit targets are kept), `--verbose`, `--only [apm\|mcp]` (APM packages or MCP/LSP service dependencies), `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | +| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing, contains git merge conflict markers, or is out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; project bootstrap keeps its new manifest and explicit targets, while absent global `-g` bootstrap uses temporary preview state and does not create `~/.apm`), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | +| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing, contains git merge conflict markers, or is out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; project bootstrap keeps its new manifest and explicit targets, while absent global `-g` bootstrap uses temporary preview state and does not create `~/.apm`), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them, except absent global dry-runs only preview targets in temporary state; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | +| `apm install [PKGS...]` | Install APM, MCP, and LSP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)); one effective target decision drives package, MCP, and LSP phases; plain/frozen installs replay locked refs and cache state, while `--update` and `--refresh` require current upstream mutable refs; a successful non-dry-run install also reconciles deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries for any target dropped from `targets:` | `--update` (deprecated; prefer `apm update`) refresh refs without accepting stale bare-cache answers, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite collisions and permit deployment after critical built-in scan findings (does NOT refresh refs by itself; `apm update --force` still requires upstream truth), `--frozen` CI-safe install that fails before any durable write when `apm.lock.yaml` is missing, contains git merge conflict markers, or is out of sync with `apm.yml`, including MCP config state (mutually exclusive with `--update`, package additions, and `--mcp`; use normal install to create or repair lock state, then `apm audit` for SHA integrity), `--dry-run` (no package/deployment writes; positional packages and ref changes are previewed without changing an existing `apm.yml`; project bootstrap keeps its new manifest and explicit targets, while absent global `-g` bootstrap uses temporary preview state and does not create `~/.apm`), `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; resolution chain `--target` > apm.yml `targets:` > `apm config set target ...` > auto-detect; this decision is reused by package, MCP, and LSP phases; unresolved required service work fails non-zero before manifest or package writes, and native MCP/LSP write failures also fail non-zero; `intellij` is MCP-only and writes JetBrains Copilot's user-scope config; explicit lists are exact, so `intellij,claude` writes those two MCP configs and `all,intellij` adds JetBrains to `all`; on auto-bootstrap when no `apm.yml` exists, recognized manifest target(s) are persisted to the new manifest's `targets:` field so a later bare `apm update` reuses them, except absent global dry-runs only preview targets in temporary state; `--target all` deprecated, see `apm compile --all`; use `kiro` for Kiro IDE; use `grok-build` for stable Grok Build rules, agents, commands, skills, and `AGENTS.md`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`; use `grok-cloud` after `apm experimental enable grok-cloud` to deploy skills only to `.grok/skills/`; use `hermes` after `apm experimental enable hermes` to deploy skills + `AGENTS.md` and, at `--global`, MCP servers to `~/.hermes/config.yaml`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Claude Code, Codex CLI, Gemini CLI, Antigravity CLI, Kiro, Windsurf, JetBrains Copilot, and Hermes when enabled), `--trust-transitive-mcp`, `--parallel-downloads N`, `--trust-bin` / `--no-trust-bin` (per-invocation consent for marketplace-plugin bin/ deployment: `--trust-bin` suppresses the trust-posture warning, `--no-trust-bin` skips bin/ even if policy allows; default deploys with a warning), `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skills from a dependency that exposes selectable skills (repeatable; plugin manifests accept a leaf name or source-relative path under skills/; a CLI name or path that matches no declared skill fails before manifest or lockfile commit with available names; a stale persisted `skills:` pin that no longer matches an available source skill warns with the package, declared request names, and available names, and directs the user to edit `skills:` in apm.yml; persisted in apm.yml only on a successful CLI match; additive across separate installs -- a later `--skill X` adds to the existing pin (union) rather than replacing it, so previously deployed skills are never silently removed; `'*'` resets to the full bundle; drop a single skill by editing the `skills:` list in apm.yml then re-running install), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry using that same effective target decision (the shared decision applies, so `apm install --mcp NAME --target intellij` writes only JetBrains Copilot's MCP config; compilation target policy applies to every explicitly selected target; direct `--mcp` additions are project-scoped and are not valid with `-g`/`--global`), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2). Explicit plugin component paths must resolve inside the plugin root; missing declarations fail before deployment and lockfile commit. | | `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; identifier selection is atomic. Accepts `owner/repo`, `name@marketplace`, exact declared local paths, or portable `_local/` keys for direct local declarations with matching lock metadata. A missing or ambiguous identifier exits nonzero before scripts or APM writes. MCP cleanup uses recorded target ownership, attempts every owning runtime, and exits nonzero if any target fails. | `--dry-run`, `-g` global | | `apm uninstall PKGS...` | Remove packages and reconcile their tracked files, MCP servers, and LSP servers; identifier selection is atomic. Accepts `owner/repo`, `name@marketplace`, exact declared local paths, or portable `_local/` keys for direct local declarations with matching lock metadata. A missing or ambiguous identifier exits nonzero before scripts or APM writes. If safe LSP cleanup fails after package removal, the command exits nonzero and preserves the conflicting config; repair it, then run `apm install` (or `apm install --global` for user scope). | `--dry-run`, `-g` global | diff --git a/packages/apm-guide/.apm/skills/apm-usage/troubleshooting.md b/packages/apm-guide/.apm/skills/apm-usage/troubleshooting.md index a91581f668..22fbaf5e9b 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/troubleshooting.md +++ b/packages/apm-guide/.apm/skills/apm-usage/troubleshooting.md @@ -11,6 +11,7 @@ | MCP path contains `.apm-resolution-staging` | Upgrade APM and retry the same install once. If it repeats, stop and report the redacted error and named MCP entry. Do not edit package files, delete the lockfile, or use `--refresh`/`--force` solely for this repair. | | TLS verification failed | Install your corporate CA into the OS trust store. For a per-shell override, set `REQUESTS_CA_BUNDLE=/path/to/ca-bundle.pem`; `SSL_CERT_FILE` alone is not a reliable requests override. | | Orphaned packages in lockfile | Run `apm prune` to remove packages no longer in apm.yml. | +| `apm.lock.yaml` contains git merge conflict markers | Run a full `apm install` (or `apm lock`): APM warns, discards the conflicted file, and resolves from apm.yml. `--frozen`, partial installs (`PACKAGE`, `--only`, `--mcp`), and read-only commands fail with the named error until then; resolve the conflict in git to keep the existing pins instead. | | Security findings block install | Run `apm audit` to review findings, then `apm install --force` if acceptable. | | Compilation not picking up changes | Run `apm compile --clean` to remove orphaned output, or `apm compile --watch` for auto-regeneration. | | Windows encoding / charmap errors | Ensure all source files and CLI output use printable ASCII only (U+0020-U+007E). No emojis or unicode symbols. | diff --git a/src/apm_cli/commands/lock.py b/src/apm_cli/commands/lock.py index bde1ea1ac8..fe00cb908b 100644 --- a/src/apm_cli/commands/lock.py +++ b/src/apm_cli/commands/lock.py @@ -300,7 +300,7 @@ def lock_export(fmt: str, output: str | None, global_: bool, timestamp: str | No re-resolves, re-hashes, or touches the network. """ from apm_cli.core.scope import InstallScope, get_apm_dir - from apm_cli.deps.lockfile import LockFile, get_lockfile_path + from apm_cli.deps.lockfile import LockFile, resolve_lockfile_path_for_read from apm_cli.export.sbom import export_sbom if global_: @@ -309,7 +309,7 @@ def lock_export(fmt: str, output: str | None, global_: bool, timestamp: str | No manifest_path = _find_apm_yml() project_root = manifest_path.parent if manifest_path else Path.cwd().resolve() - lockfile_path = get_lockfile_path(project_root) + lockfile_path = resolve_lockfile_path_for_read(project_root, read_only=True) lockfile = LockFile.read(lockfile_path) if lockfile is None: _rich_error(f"No lockfile found at {lockfile_path}. Run 'apm lock' to generate one first.") diff --git a/src/apm_cli/deps/lockfile.py b/src/apm_cli/deps/lockfile.py index ae6bdd654e..c670ed3783 100644 --- a/src/apm_cli/deps/lockfile.py +++ b/src/apm_cli/deps/lockfile.py @@ -1057,10 +1057,10 @@ def read(cls, path: Path) -> LockFile | None: """Read lock file from disk. Returns None when the file does not exist.""" if not path.exists(): return None - text = path.read_text(encoding="utf-8") - if has_conflict_markers(text): - raise LockfileConflictError(path) try: + text = path.read_text(encoding="utf-8") + if has_conflict_markers(text): + raise LockfileConflictError(path) return cls.from_yaml(text) except (LockfileFormatError, UnsupportedLockfileVersionError): raise @@ -1289,7 +1289,13 @@ def migrate_lockfile_if_needed(project_root: Path) -> bool: def discard_conflicted_lockfile(path: Path) -> bool: """Delete a lockfile left with git merge conflict markers so install regenerates it.""" - if not path.exists() or not has_conflict_markers(path.read_text(encoding="utf-8")): + if not path.exists(): + return False + try: + text = path.read_text(encoding="utf-8") + except UnicodeDecodeError: + return False + if not has_conflict_markers(text): return False path.unlink() return True diff --git a/src/apm_cli/install/service.py b/src/apm_cli/install/service.py index fb4ac3abff..1803d16f39 100644 --- a/src/apm_cli/install/service.py +++ b/src/apm_cli/install/service.py @@ -292,7 +292,8 @@ def enforce_frozen(request: InstallRequest) -> None: ) from e except Exception as e: raise FrozenInstallError( - f"--frozen could not read apm.lock.yaml: {e}", + f"--frozen could not read apm.lock.yaml: {e}. Fix or remove the file, " + "then run 'apm install' without --frozen to regenerate it.", ) from e if lockfile is None: diff --git a/tests/integration/test_install_conflicted_lockfile_e2e.py b/tests/integration/test_install_conflicted_lockfile_e2e.py index aa74b8fb8f..5875623a19 100644 --- a/tests/integration/test_install_conflicted_lockfile_e2e.py +++ b/tests/integration/test_install_conflicted_lockfile_e2e.py @@ -18,6 +18,8 @@ from apm_cli.deps.lockfile import LockFile, LockfileConflictError from apm_cli.models.apm_package import clear_apm_yml_cache +pytestmark = pytest.mark.component + _PATCH_UPDATES = "apm_cli.commands._helpers.check_for_updates" _CONFLICTED_LOCKFILE = textwrap.dedent("""\ diff --git a/tests/unit/deps/test_lockfile_conflict_markers.py b/tests/unit/deps/test_lockfile_conflict_markers.py index 46177d98a5..60cd4a60ac 100644 --- a/tests/unit/deps/test_lockfile_conflict_markers.py +++ b/tests/unit/deps/test_lockfile_conflict_markers.py @@ -13,7 +13,7 @@ discard_conflicted_lockfile, ) -pytestmark = pytest.mark.unit +pytestmark = pytest.mark.component _VALID = "lockfile_version: '1'\ndependencies: []\n" @@ -96,6 +96,26 @@ def test_read_treats_separator_only_line_as_ordinary_format_error(tmp_path: Path assert not isinstance(exc_info.value, LockfileConflictError) +def test_read_undecodable_lockfile_is_a_format_error(tmp_path: Path) -> None: + path = tmp_path / "apm.lock.yaml" + path.write_bytes(b"lockfile_version: '1'\n\xff\xfe<<<<<<< HEAD\n") + + with pytest.raises(LockfileFormatError) as exc_info: + LockFile.read(path) + + assert not isinstance(exc_info.value, LockfileConflictError) + assert str(path) in str(exc_info.value) + + +def test_discard_leaves_an_undecodable_lockfile_in_place(tmp_path: Path) -> None: + path = tmp_path / "apm.lock.yaml" + raw = b"\xff\xfe<<<<<<< HEAD\n" + path.write_bytes(raw) + + assert discard_conflicted_lockfile(path) is False + assert path.read_bytes() == raw + + def test_discard_removes_only_a_conflicted_lockfile(tmp_path: Path) -> None: path = tmp_path / "apm.lock.yaml" diff --git a/tests/unit/install/test_frozen.py b/tests/unit/install/test_frozen.py index 3a41f35233..7c5c228f08 100644 --- a/tests/unit/install/test_frozen.py +++ b/tests/unit/install/test_frozen.py @@ -99,6 +99,17 @@ def test_conflict_markers_fail_closed_with_named_next_action(self, tmp_path: Pat assert "apm outdated" not in frozen_install_tip(exc_info.value) assert (tmp_path / "apm.lock.yaml").read_text() == conflicted + def test_corrupt_lockfile_names_a_next_action(self, tmp_path: Path): + _write_apm_yml(tmp_path) + (tmp_path / "apm.lock.yaml").write_text("lockfile_version: '1'\ndependencies: [\n") + req = _make_request(project_dir=tmp_path, manifest_deps=[]) + + with pytest.raises(FrozenInstallError, match="could not read") as exc_info: + InstallService.enforce_frozen(req) + + assert "without --frozen" in str(exc_info.value) + assert frozen_install_tip(exc_info.value) == "" + def test_missing_lockfile_tip_does_not_point_at_unreadable_commands(self, tmp_path: Path): _write_apm_yml(tmp_path) req = _make_request(project_dir=tmp_path, manifest_deps=[]) From 1b1b506b995da6b0c44220a35c05d3efa3c7bac6 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Sun, 20 Sep 2026 19:22:15 -0400 Subject: [PATCH 3/6] fix(install): name only the lockfile conflict error on MCP add Other lockfile format errors keep the redacted, verbose-only handling that path had before #3028 widened the except clause. --- src/apm_cli/install/mcp/command.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apm_cli/install/mcp/command.py b/src/apm_cli/install/mcp/command.py index 0ac53c2cfb..56047d7901 100644 --- a/src/apm_cli/install/mcp/command.py +++ b/src/apm_cli/install/mcp/command.py @@ -36,7 +36,7 @@ try: from ...deps.lockfile import ( LockFile, - LockfileFormatError, + LockfileConflictError, get_lockfile_path, migrate_lockfile_if_needed, ) @@ -305,7 +305,7 @@ def run_mcp_install( # noqa: PLR0913 ) except InstallFailureAlreadyRendered: raise - except LockfileFormatError as exc: + except LockfileConflictError as exc: logger.error(str(exc)) logger.error("MCP server written to apm.yml but tool integration failed.") raise click.ClickException(f"MCP integration failed for '{mcp_name}'") from exc From 0bf3db49853c87684f7b7853edc056883a6f4440 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Sun, 20 Sep 2026 08:24:37 -0400 Subject: [PATCH 4/6] fix(install): restore a discarded conflicted lockfile when the run fails InstallTransaction now owns the conflicted-lockfile discard: it snapshots the bytes before unlinking and rollback puts the file back unless the attempt already wrote a new lockfile. apm lock runs under its own transaction so the same rule applies there. The module-level discard_conflicted_lockfile helper is removed. Follow-up to #3028 from the APM Review Panel. --- CHANGELOG.md | 2 +- .../src/content/docs/reference/cli/install.md | 2 +- docs/src/content/docs/reference/cli/lock.md | 2 +- .../content/docs/reference/lockfile-spec.md | 3 +- .../docs/troubleshooting/install-failures.md | 2 +- src/apm_cli/commands/install.py | 13 ++-- src/apm_cli/commands/lock.py | 19 ++++- src/apm_cli/deps/lockfile.py | 14 ---- src/apm_cli/install/transaction.py | 47 ++++++++++-- .../test_install_conflicted_lockfile_e2e.py | 27 +++++++ .../deps/test_lockfile_conflict_markers.py | 31 +------- .../unit/install/test_install_transaction.py | 72 +++++++++++++++++++ 12 files changed, 171 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68efa8ab02..3df634eb98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- A full `apm install` and `apm lock` now warn, discard `apm.lock.yaml`, and resolve from `apm.yml` when the lockfile still contains git merge conflict markers, instead of exiting with a YAML parse error. `apm install --frozen`, partial installs, and read-only commands such as `apm update` and `apm outdated` fail closed with an error that names the conflict and the next action, and the `--frozen` failure tip no longer points at commands that cannot read the lockfile. (closes #2979) (#3028) +- A full `apm install` and `apm lock` now warn, discard `apm.lock.yaml`, and resolve from `apm.yml` when the lockfile still contains git merge conflict markers, instead of exiting with a YAML parse error. `apm install --frozen`, partial installs, and read-only commands such as `apm update` and `apm outdated` fail closed with an error that names the conflict and the next action, and the `--frozen` failure tip no longer points at commands that cannot read the lockfile. A run that fails after discarding the file restores it. (closes #2979) (#3028, #3043) - Autopilot maintainer canvas removes a Decide row as soon as GitHub confirms `status/accepted`, without waiting for a full issue/PR refetch. - Issue and PR triage no longer skip bot-authored items (Copilot, Dependabot, github-actions). They stay in the queue like any other contribution. (#3024) - PR-review scheduler no longer queues every open pull request. A fresh review requires the `panel-review` label (same trigger as the Agentic Workflow), `status/accepted` on the PR, or an explicit named PR list. The reviewing session also requires `status/accepted` on the PR or a linked issue; otherwise scheduler and review-worker stop with no comment. The worker may clear `panel-review`; the scheduler does not comment or change labels. Both also apply a CODEOWNERS last-comment gate: read the last CODEOWNER comment as conditions and evaluate them against later comments AND labels on the PR and linked issues. Drop or `noop` only when those conditions are unmet or unclear. Named list does not bypass that gate. diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index af855fa12f..a58562b058 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -150,7 +150,7 @@ in `apm.yml`, then run `apm install` again. - **Lockfile replay and Git ref freshness.** Plain and `--frozen` installs may trust `apm.lock.yaml` and the local Git cache, reusing the locked commit for unchanged Git dependencies across the full resolved graph. In contrast, `apm install --update`, `apm install --refresh`, [`apm update`](../update/) with or without `--force`, [`apm lock --update`](../lock/), and [`apm outdated`](../outdated/) establish mutable Git refs from upstream instead of accepting stale refs from a local bare Git cache. APM picks up upstream changes to a transitive package's `apm.yml` only when you regenerate the graph -- run `apm update` or `apm lock --update`. See the [lockfile specification](../../lockfile-spec/) for the replay contract. - **Semver ranges on git deps.** `ref:` accepts semver ranges (`^1.2.0`, `~1.4`, `>=2.0 <3`, `1.5.x`) for git-source deps, including positional virtual-subdirectory references. APM runs `git ls-remote` against the dep, picks the highest tag matching the range, and pins the resolved tag plus commit SHA, version, and original constraint in `apm.lock.yaml`. Subsequent installs replay the lockfile without network; use `--update` (or change the manifest constraint) to re-resolve. See [manage dependencies](../../../consumer/manage-dependencies/#pin-a-semver-range) for the supported syntax. - **No-op nudge.** When the lockfile is already satisfied and nothing needs deploying, install prints `[i] Run 'apm update' to check for newer versions.` so you know the silent success was not a missed refresh. -- **Frozen mode.** With `--frozen`, install resolves only what is in `apm.lock.yaml`. A missing lockfile, a direct dependency missing from it, or MCP config state that differs from `apm.yml` exits `1` before lockfile, target config, deployment, or cache mutation. Cold-cache installs (empty `apm_modules/`) with git `apm_package` deps are tolerated: MCP checks are skipped for absent package directories (the packages will be hydrated by the pipeline), and their MCP server configs are restored from the lockfile so no false drift is reported. Remote `claude_skill` dependencies declared at a repository root or subdirectory are also accepted from their locked type before materialization; once present, the lock type and detected skill shape must agree. Missing local paths still fail. A lockfile that contains git merge conflict markers also exits `1` and is never rewritten under `--frozen`. See [`config-consistency`](../../baseline-checks/#config-consistency) for the full manifest rule. Run normal `apm install` to create or repair MCP-only lock state, or to discard a conflicted lockfile and resolve from `apm.yml`, then retry frozen mode. Add-style invocations (`apm install PACKAGE` and `apm install --mcp NAME`) are rejected because they mutate `apm.yml`. Orphan package lock entries are tolerated; local-path deps are skipped. This is a structural check, not a content check -- run `apm audit --ci` for hash verification. +- **Frozen mode.** With `--frozen`, install resolves only what is in `apm.lock.yaml`. A missing lockfile, a direct dependency missing from it, or MCP config state that differs from `apm.yml` exits `1` before lockfile, target config, deployment, or cache mutation. Cold-cache installs (empty `apm_modules/`) with git `apm_package` deps are tolerated: MCP checks are skipped for absent package directories (the packages will be hydrated by the pipeline), and their MCP server configs are restored from the lockfile so no false drift is reported. Remote `claude_skill` dependencies declared at a repository root or subdirectory are also accepted from their locked type before materialization; once present, the lock type and detected skill shape must agree. Missing local paths still fail. A lockfile that contains git merge conflict markers also exits `1` and is never rewritten under `--frozen`. See [`config-consistency`](../../baseline-checks/#config-consistency) for the full manifest rule. Run normal `apm install` to create or repair MCP-only lock state, or a full `apm install` (no package arguments, no `--only`) to discard a conflicted lockfile and resolve from `apm.yml`, then retry frozen mode. Add-style invocations (`apm install PACKAGE` and `apm install --mcp NAME`) are rejected because they mutate `apm.yml`. Orphan package lock entries are tolerated; local-path deps are skipped. This is a structural check, not a content check -- run `apm audit --ci` for hash verification. - **Local `.apm/` deployment.** After dependencies are integrated, primitives in the project's own `.apm/` directory are deployed to the same targets. Local files win on collision. Skipped at `--global` and with `--only mcp`. - **User-scope root context hint.** Compilation stays explicit. After `apm install -g`, targets with native user-scope instruction files pick up global instructions during install. Targets whose user-scope instruction surface is a root context file require [`apm compile --global`](../compile/#global-compilation); install prints a one-line `[i]` hint and writes no root context file. - **OpenCode user scope.** `apm install -g --target opencode` deploys skills to diff --git a/docs/src/content/docs/reference/cli/lock.md b/docs/src/content/docs/reference/cli/lock.md index 198db8e910..9d2682757f 100644 --- a/docs/src/content/docs/reference/cli/lock.md +++ b/docs/src/content/docs/reference/cli/lock.md @@ -72,7 +72,7 @@ apm lock --verbose - **Write `apm.lock.yaml`.** The lockfile records every pinned ref, resolved commit, and content hash. Fresh lock-only runs add no deployed files, and existing deployed-file rows, hashes, and deployment-ledger entries stay recorded while those bytes remain on disk. - **No files deployed or deleted.** The targets, cleanup, post-deps-local, and audit phases are skipped. The integrate phase runs but deploys nothing because the target set is empty. Running `apm lock` is safe to run before you are ready to install. - **Idempotent.** If the lockfile already matches the resolution result, it is overwritten with the same content. -- **Recovers from merge conflicts.** If `apm.lock.yaml` still contains git conflict markers, `apm lock` warns, discards it, and resolves from `apm.yml`. See [lockfile versioning](../../lockfile-spec/#versioning). +- **Recovers from merge conflicts.** If `apm.lock.yaml` still contains git conflict markers, `apm lock` warns, discards it, and resolves from `apm.yml`; a failed run restores the discarded file. See [lockfile versioning](../../lockfile-spec/#versioning). ## Export (SBOM inventory) diff --git a/docs/src/content/docs/reference/lockfile-spec.md b/docs/src/content/docs/reference/lockfile-spec.md index 81c17d1a02..f6ac6541a8 100644 --- a/docs/src/content/docs/reference/lockfile-spec.md +++ b/docs/src/content/docs/reference/lockfile-spec.md @@ -401,7 +401,8 @@ without lock metadata as before. A full `apm install` (no package arguments, no `--only`) or `apm lock` warns, discards the file, and resolves from `apm.yml`, the same input a non-frozen install resolves from anyway; deployed-file records from the conflicted file are not carried over, and a -manifest with nothing to lock ends with no lockfile. Partial installs and +manifest with nothing to lock ends with no lockfile. If that run fails before +writing a new lockfile, the conflicted file is put back. Partial installs and `apm install --frozen` never rewrite the file: resolve the conflict in git, or run a full `apm install` without `--frozen`. diff --git a/docs/src/content/docs/troubleshooting/install-failures.md b/docs/src/content/docs/troubleshooting/install-failures.md index 3d92f5ebf2..077d759783 100644 --- a/docs/src/content/docs/troubleshooting/install-failures.md +++ b/docs/src/content/docs/troubleshooting/install-failures.md @@ -183,7 +183,7 @@ After a git merge that touched `apm.lock.yaml`, the file may still contain `<<<< apm install ``` -APM warns that it is discarding the conflicted file, then writes a fresh lockfile. Commit the result. `apm lock` does the same without deploying files. Partial installs (`apm install PACKAGE`, `--only apm`, `--only mcp`, `--mcp NAME`) and `apm install --frozen` refuse to rewrite the file; resolve the conflict in git first, or run a full `apm install` without `--frozen`. A lockfile that is invalid for any other reason still fails closed in every mode. +APM warns that it is discarding the conflicted file, then writes a fresh lockfile. Commit the result. If the install fails before writing one, the conflicted file is restored. `apm lock` does the same without deploying files. Partial installs (`apm install PACKAGE`, `--only apm`, `--only mcp`, `--mcp NAME`) and `apm install --frozen` refuse to rewrite the file; resolve the conflict in git first, or run a full `apm install` without `--frozen`. A lockfile that is invalid for any other reason still fails closed in every mode. ### Drifted refs diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 647cb6d377..a625cd7da9 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -229,12 +229,7 @@ class InstallContext: ) try: from ..deps.apm_resolver import APMDependencyResolver - from ..deps.lockfile import ( - LockFile, - discard_conflicted_lockfile, - get_lockfile_path, - migrate_lockfile_if_needed, - ) + from ..deps.lockfile import LockFile, get_lockfile_path, migrate_lockfile_if_needed from ..integration.mcp_integrator import ( MCPIntegrator, # noqa: F401 -- re-exported; tests patch commands.install.MCPIntegrator ) @@ -1834,7 +1829,11 @@ def _install_apm_packages(ctx, outcome): # Only a full install re-resolves every apm.yml entry; a partial add or # --only run would write a lockfile missing the other entries. full_install = not ctx.frozen and not ctx.packages and ctx.install_mode == InstallMode.ALL - if full_install and discard_conflicted_lockfile(get_lockfile_path(ctx.apm_dir)): + if ( + full_install + and ctx.transaction is not None + and ctx.transaction.discard_conflicted_lockfile(get_lockfile_path(ctx.apm_dir)) + ): logger.warning(_LOCKFILE_CONFLICT_DISCARDED) # Capture old MCP servers and configs from lockfile BEFORE diff --git a/src/apm_cli/commands/lock.py b/src/apm_cli/commands/lock.py index fe00cb908b..c177ace6c9 100644 --- a/src/apm_cli/commands/lock.py +++ b/src/apm_cli/commands/lock.py @@ -221,15 +221,24 @@ def _run_lock( sys.exit(1) logger = InstallLogger(verbose=verbose) - + from apm_cli.core.scope import get_modules_dir + from apm_cli.deps.lockfile import get_lockfile_path + from apm_cli.install.transaction import InstallTransaction + + transaction = InstallTransaction( + manifest_path=project_root / "apm.yml", + apm_modules_dir=get_modules_dir(scope), + validation=None, + logger=logger, + acquire_lock=False, + ) try: from apm_cli.commands.install import ( _LOCKFILE_CONFLICT_DISCARDED, _install_apm_dependencies, ) - from apm_cli.deps.lockfile import discard_conflicted_lockfile, get_lockfile_path - if discard_conflicted_lockfile(get_lockfile_path(project_root)): + if transaction.discard_conflicted_lockfile(get_lockfile_path(project_root)): logger.warning(_LOCKFILE_CONFLICT_DISCARDED) result = _install_apm_dependencies( apm_package, @@ -241,10 +250,14 @@ def _run_lock( no_policy=no_policy, target=target, lockfile_only=True, + transaction=transaction, ) + result = transaction.complete(result) except click.UsageError: + transaction.rollback() raise except Exception as e: + transaction.fail(e) _handle_lock_error(e, verbose) from apm_cli.install.summary import exit_unless_install_result_allows_success diff --git a/src/apm_cli/deps/lockfile.py b/src/apm_cli/deps/lockfile.py index c670ed3783..dfb6230f52 100644 --- a/src/apm_cli/deps/lockfile.py +++ b/src/apm_cli/deps/lockfile.py @@ -1287,20 +1287,6 @@ def migrate_lockfile_if_needed(project_root: Path) -> bool: return False -def discard_conflicted_lockfile(path: Path) -> bool: - """Delete a lockfile left with git merge conflict markers so install regenerates it.""" - if not path.exists(): - return False - try: - text = path.read_text(encoding="utf-8") - except UnicodeDecodeError: - return False - if not has_conflict_markers(text): - return False - path.unlink() - return True - - def get_lockfile_installed_paths(project_root: Path) -> list[str]: """Deprecated: use LockFile.installed_paths_for_project() instead.""" return LockFile.installed_paths_for_project(project_root) diff --git a/src/apm_cli/install/transaction.py b/src/apm_cli/install/transaction.py index 02b3bcedba..cf2b7b84cd 100644 --- a/src/apm_cli/install/transaction.py +++ b/src/apm_cli/install/transaction.py @@ -10,6 +10,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Literal, cast +from apm_cli.deps.lockfile import has_conflict_markers from apm_cli.install.locking import acquire_lifecycle_lock from apm_cli.install.resolution_staging import ResolutionStagingSession from apm_cli.models.results import InstallDisposition, InstallResult @@ -18,16 +19,16 @@ from apm_cli.core.command_logger import InstallLogger, _ValidationOutcome -def _restore_manifest_from_snapshot(manifest_path: Path, snapshot: bytes) -> None: - """Atomically replace *manifest_path* with byte-exact *snapshot*.""" +def _restore_manifest_from_snapshot(path: Path, snapshot: bytes) -> None: + """Atomically replace *path* with byte-exact *snapshot*.""" fd, temporary_name = tempfile.mkstemp( prefix="apm-restore-", - dir=str(manifest_path.parent), + dir=str(path.parent), ) try: with os.fdopen(fd, "wb") as handle: handle.write(snapshot) - os.replace(temporary_name, manifest_path) + os.replace(temporary_name, path) except BaseException: with contextlib.suppress(OSError): os.unlink(temporary_name) @@ -108,6 +109,7 @@ def __init__( self._release_workspace_lock() raise self._lock = threading.RLock() + self._discarded_lockfile: tuple[Path, bytes] | None = None self.committed = False self._completed = False self._rolled_back = False @@ -117,6 +119,23 @@ def resolution(self) -> ResolutionStagingSession: """Return the single resolution journal owned by this attempt.""" return self._resolution + def discard_conflicted_lockfile(self, lockfile_path: Path) -> bool: + """Remove a lockfile left with git merge conflict markers so this attempt + resolves from the manifest; rollback restores it if the attempt fails.""" + if not lockfile_path.exists(): + return False + snapshot = lockfile_path.read_bytes() + try: + text = snapshot.decode("utf-8") + except UnicodeDecodeError: + return False + if not has_conflict_markers(text): + return False + with self._lock: + lockfile_path.unlink() + self._discarded_lockfile = (lockfile_path, snapshot) + return True + def record_validation(self, validation: _ValidationOutcome) -> None: """Attach the validation outcome produced after transaction creation.""" self._validation = validation @@ -210,6 +229,7 @@ def rollback(self) -> None: cleanup_issues = self._resolution.rollback() or [] self._report_resolution_cleanup_issues(cleanup_issues) self._restore_manifest() + self._restore_discarded_lockfile() self._rolled_back = True self._completed = True finally: @@ -243,6 +263,25 @@ def _restore_manifest(self) -> None: self._manifest_existed, ) + def _restore_discarded_lockfile(self) -> None: + """Put back the conflicted lockfile unless this attempt already wrote a new one.""" + if self._discarded_lockfile is None: + return + lockfile_path, snapshot = self._discarded_lockfile + if lockfile_path.exists(): + return + try: + _restore_manifest_from_snapshot(lockfile_path, snapshot) + if self._logger is not None: + self._logger.progress(f"{lockfile_path.name} restored to its previous state.") + except Exception as exc: + if self._logger is not None: + self._logger.warning( + f"Failed to restore {lockfile_path.name}. Recover it from version " + "control before retrying." + ) + self._logger.verbose_detail(f"Lockfile rollback error: {exc}") + def _release_workspace_lock(self) -> None: """Release this transaction's nested lifecycle acquisition once.""" if self._workspace_lock_finalizer is not None: diff --git a/tests/integration/test_install_conflicted_lockfile_e2e.py b/tests/integration/test_install_conflicted_lockfile_e2e.py index 5875623a19..fb0315f72c 100644 --- a/tests/integration/test_install_conflicted_lockfile_e2e.py +++ b/tests/integration/test_install_conflicted_lockfile_e2e.py @@ -183,3 +183,30 @@ def test_corrupt_lockfile_without_markers_still_fails_closed( assert result.exit_code == 1, result.output assert "conflict markers" not in _combined_output(result) assert lockfile_path.read_text(encoding="utf-8") == corrupt + + +@pytest.mark.parametrize("args", [["install"], ["lock"]]) +def test_failed_run_after_discard_restores_the_conflicted_lockfile( + runner: CliRunner, conflicted_project: Path, args: list[str] +) -> None: + (conflicted_project / "apm.yml").write_text( + textwrap.dedent("""\ + name: test-project + version: '1.0.0' + targets: + - claude + dependencies: + apm: + - ./does-not-exist + """), + encoding="utf-8", + ) + + result = _invoke(runner, args, catch_exceptions=True) + + assert result.exit_code == 1, result.output + output = _combined_output(result) + assert "resolving from apm.yml" in output + assert "apm.lock.yaml restored to its previous state" in output + lockfile_text = (conflicted_project / "apm.lock.yaml").read_text(encoding="utf-8") + assert lockfile_text == _CONFLICTED_LOCKFILE diff --git a/tests/unit/deps/test_lockfile_conflict_markers.py b/tests/unit/deps/test_lockfile_conflict_markers.py index 60cd4a60ac..b44beab582 100644 --- a/tests/unit/deps/test_lockfile_conflict_markers.py +++ b/tests/unit/deps/test_lockfile_conflict_markers.py @@ -6,16 +6,10 @@ import pytest -from apm_cli.deps.lockfile import ( - LockFile, - LockfileConflictError, - LockfileFormatError, - discard_conflicted_lockfile, -) +from apm_cli.deps.lockfile import LockFile, LockfileConflictError, LockfileFormatError pytestmark = pytest.mark.component -_VALID = "lockfile_version: '1'\ndependencies: []\n" _CONFLICTED = ( "lockfile_version: '1'\n" @@ -105,26 +99,3 @@ def test_read_undecodable_lockfile_is_a_format_error(tmp_path: Path) -> None: assert not isinstance(exc_info.value, LockfileConflictError) assert str(path) in str(exc_info.value) - - -def test_discard_leaves_an_undecodable_lockfile_in_place(tmp_path: Path) -> None: - path = tmp_path / "apm.lock.yaml" - raw = b"\xff\xfe<<<<<<< HEAD\n" - path.write_bytes(raw) - - assert discard_conflicted_lockfile(path) is False - assert path.read_bytes() == raw - - -def test_discard_removes_only_a_conflicted_lockfile(tmp_path: Path) -> None: - path = tmp_path / "apm.lock.yaml" - - assert discard_conflicted_lockfile(path) is False - - path.write_text(_VALID, encoding="utf-8") - assert discard_conflicted_lockfile(path) is False - assert path.read_text(encoding="utf-8") == _VALID - - path.write_text(_CONFLICTED, encoding="utf-8") - assert discard_conflicted_lockfile(path) is True - assert not path.exists() diff --git a/tests/unit/install/test_install_transaction.py b/tests/unit/install/test_install_transaction.py index 582f557783..9141846a29 100644 --- a/tests/unit/install/test_install_transaction.py +++ b/tests/unit/install/test_install_transaction.py @@ -763,3 +763,75 @@ def test_failed_first_install_removes_auto_created_manifest( assert result.exit_code == 1 assert not (tmp_path / "apm.yml").exists() + + +_CONFLICTED_LOCKFILE = ( + b"lockfile_version: '1'\n" + b"<<<<<<< HEAD\n" + b"dependencies: []\n" + b"=======\n" + b"dependencies:\n" + b"- repo_url: example/x\n" + b">>>>>>> feature\n" +) + + +def test_discard_conflicted_lockfile_removes_it_and_rollback_restores_it(tmp_path: Path) -> None: + transaction = _transaction(tmp_path) + lockfile = tmp_path / "apm.lock.yaml" + lockfile.write_bytes(_CONFLICTED_LOCKFILE) + + assert transaction.discard_conflicted_lockfile(lockfile) is True + assert not lockfile.exists() + + transaction.fail(RuntimeError("resolution failed")) + + assert lockfile.read_bytes() == _CONFLICTED_LOCKFILE + + +def test_rollback_keeps_a_lockfile_written_after_the_discard(tmp_path: Path) -> None: + transaction = _transaction(tmp_path) + lockfile = tmp_path / "apm.lock.yaml" + lockfile.write_bytes(_CONFLICTED_LOCKFILE) + transaction.discard_conflicted_lockfile(lockfile) + lockfile.write_bytes(b"lockfile_version: '1'\ndependencies: []\n") + + transaction.rollback() + + assert lockfile.read_bytes() == b"lockfile_version: '1'\ndependencies: []\n" + + +def test_commit_does_not_restore_a_discarded_lockfile(tmp_path: Path) -> None: + transaction = _transaction(tmp_path) + lockfile = tmp_path / "apm.lock.yaml" + lockfile.write_bytes(_CONFLICTED_LOCKFILE) + transaction.discard_conflicted_lockfile(lockfile) + + transaction.commit(InstallResult()) + + assert not lockfile.exists() + + +@pytest.mark.parametrize( + "content", + [ + pytest.param(b"lockfile_version: '1'\ndependencies: []\n", id="valid"), + pytest.param(b"lockfile_version: '1'\ndependencies: [\n", id="corrupt"), + pytest.param(b"\xff\xfe<<<<<<< HEAD\n", id="undecodable"), + ], +) +def test_discard_leaves_other_lockfiles_in_place(tmp_path: Path, content: bytes) -> None: + transaction = _transaction(tmp_path) + lockfile = tmp_path / "apm.lock.yaml" + lockfile.write_bytes(content) + + assert transaction.discard_conflicted_lockfile(lockfile) is False + assert lockfile.read_bytes() == content + transaction.rollback() + assert lockfile.read_bytes() == content + + +def test_discard_missing_lockfile_is_a_no_op(tmp_path: Path) -> None: + transaction = _transaction(tmp_path) + + assert transaction.discard_conflicted_lockfile(tmp_path / "apm.lock.yaml") is False From e5a87ced0b7f94031984049b804cca8a6febdab7 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Sun, 20 Sep 2026 12:00:23 -0400 Subject: [PATCH 5/6] fix(install): register the conflicted-lockfile discard owner and harden apm lock Record InstallTransaction.discard_conflicted_lockfile as the canonical owner in .apm/architecture/owners/install-deployment.json with the install-deployment-conflicted-lockfile-discard guard, its linter check, and a mutation-matrix case proving rollback must restore the file. apm lock now migrates a legacy apm.lock before the discard probe and runs the transaction as a context manager so an interrupt after the discard still restores the conflicted lockfile. The MCP add path names only the conflict error at default verbosity; other lockfile format errors keep their redacted handling. --- .../owners/install-deployment.json | 7 ++ CHANGELOG.md | 2 +- docs/src/content/docs/reference/cli/lock.md | 2 +- .../checks/install_deployment_analyzers.py | 7 ++ .../checks/install_frozen_and_audit.py | 72 +++++++++++++++++-- scripts/architecture_linter/diagnostics.py | 1 + src/apm_cli/commands/lock.py | 61 ++++++++-------- .../test_architecture_owner_rule_mutations.py | 8 +++ .../test_install_conflicted_lockfile_e2e.py | 28 ++++++++ .../unit/scripts/test_architecture_runner.py | 1 + 10 files changed, 152 insertions(+), 37 deletions(-) diff --git a/.apm/architecture/owners/install-deployment.json b/.apm/architecture/owners/install-deployment.json index a0872223aa..3bc8aac887 100644 --- a/.apm/architecture/owners/install-deployment.json +++ b/.apm/architecture/owners/install-deployment.json @@ -157,6 +157,13 @@ "owner": "models/dependency/selection.py (via DependencyReference)", "selectors": ["src/apm_cli/models/dependency/selection.py"], "guards": ["install-deployment-uninstall-selection"] + }, + { + "id": "conflicted-lockfile-discard", + "decision": "Conflicted lockfile discard and rollback restore", + "owner": "install/transaction.py (InstallTransaction.discard_conflicted_lockfile)", + "selectors": ["src/apm_cli/install/transaction.py"], + "guards": ["install-deployment-conflicted-lockfile-discard"] } ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index 3df634eb98..695c47f338 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- A full `apm install` and `apm lock` now warn, discard `apm.lock.yaml`, and resolve from `apm.yml` when the lockfile still contains git merge conflict markers, instead of exiting with a YAML parse error. `apm install --frozen`, partial installs, and read-only commands such as `apm update` and `apm outdated` fail closed with an error that names the conflict and the next action, and the `--frozen` failure tip no longer points at commands that cannot read the lockfile. A run that fails after discarding the file restores it. (closes #2979) (#3028, #3043) +- A full `apm install` and `apm lock` now warn, discard `apm.lock.yaml`, and resolve from `apm.yml` when the lockfile still contains git merge conflict markers, instead of exiting with a YAML parse error. `apm install --frozen`, partial installs, and read-only commands such as `apm update` and `apm outdated` fail closed with an error that names the conflict and the next action, and the `--frozen` failure tip no longer points at commands that cannot read the lockfile. A run that fails after discarding the file restores it. (closes #2979) (#3028) - Autopilot maintainer canvas removes a Decide row as soon as GitHub confirms `status/accepted`, without waiting for a full issue/PR refetch. - Issue and PR triage no longer skip bot-authored items (Copilot, Dependabot, github-actions). They stay in the queue like any other contribution. (#3024) - PR-review scheduler no longer queues every open pull request. A fresh review requires the `panel-review` label (same trigger as the Agentic Workflow), `status/accepted` on the PR, or an explicit named PR list. The reviewing session also requires `status/accepted` on the PR or a linked issue; otherwise scheduler and review-worker stop with no comment. The worker may clear `panel-review`; the scheduler does not comment or change labels. Both also apply a CODEOWNERS last-comment gate: read the last CODEOWNER comment as conditions and evaluate them against later comments AND labels on the PR and linked issues. Drop or `noop` only when those conditions are unmet or unclear. Named list does not bypass that gate. diff --git a/docs/src/content/docs/reference/cli/lock.md b/docs/src/content/docs/reference/cli/lock.md index 9d2682757f..5cb827ecd5 100644 --- a/docs/src/content/docs/reference/cli/lock.md +++ b/docs/src/content/docs/reference/cli/lock.md @@ -72,7 +72,7 @@ apm lock --verbose - **Write `apm.lock.yaml`.** The lockfile records every pinned ref, resolved commit, and content hash. Fresh lock-only runs add no deployed files, and existing deployed-file rows, hashes, and deployment-ledger entries stay recorded while those bytes remain on disk. - **No files deployed or deleted.** The targets, cleanup, post-deps-local, and audit phases are skipped. The integrate phase runs but deploys nothing because the target set is empty. Running `apm lock` is safe to run before you are ready to install. - **Idempotent.** If the lockfile already matches the resolution result, it is overwritten with the same content. -- **Recovers from merge conflicts.** If `apm.lock.yaml` still contains git conflict markers, `apm lock` warns, discards it, and resolves from `apm.yml`; a failed run restores the discarded file. See [lockfile versioning](../../lockfile-spec/#versioning). +- **Recovers from merge conflicts.** If `apm.lock.yaml` still contains git conflict markers, `apm lock` warns, discards it, and resolves from `apm.yml`; a run that fails before writing a replacement restores the discarded file. See [lockfile versioning](../../lockfile-spec/#versioning). ## Export (SBOM inventory) diff --git a/scripts/architecture_linter/checks/install_deployment_analyzers.py b/scripts/architecture_linter/checks/install_deployment_analyzers.py index d58844e0e7..fc7109fb0e 100644 --- a/scripts/architecture_linter/checks/install_deployment_analyzers.py +++ b/scripts/architecture_linter/checks/install_deployment_analyzers.py @@ -28,11 +28,13 @@ ) from scripts.architecture_linter.checks.install_frozen_and_audit import ( _GUARD_AUDIT_REPLAY, + _GUARD_CONFLICTED_LOCKFILE, _GUARD_FROZEN, _GUARD_LIFECYCLE_SERIALIZATION, _GUARD_MCP_OWNERSHIP, _GUARD_UNINSTALL_REACHABILITY, check_audit_replay, + check_conflicted_lockfile_discard, check_frozen, check_lifecycle_serialization, check_mcp_ownership_migration, @@ -167,6 +169,11 @@ "Lifecycle mutators route through install/locking.py.", check_lifecycle_serialization, ), + _rule( + _GUARD_CONFLICTED_LOCKFILE, + "Conflicted lockfile discard and restore route through install/transaction.py.", + check_conflicted_lockfile_discard, + ), _rule( _GUARD_UNINSTALL_SELECTION, "Dependency CLI parsing + uninstall selection route through dependency/selection.py.", diff --git a/scripts/architecture_linter/checks/install_frozen_and_audit.py b/scripts/architecture_linter/checks/install_frozen_and_audit.py index 4e550972e4..d37b87746b 100644 --- a/scripts/architecture_linter/checks/install_frozen_and_audit.py +++ b/scripts/architecture_linter/checks/install_frozen_and_audit.py @@ -1,12 +1,13 @@ -"""Frozen-mutation, MCP-ownership, uninstall-reachability, and audit-replay -install analyzers. +"""Frozen-mutation, MCP-ownership, uninstall-reachability, audit-replay, and +conflicted-lockfile install analyzers. -Ports four owner guards recorded in +Ports five owner guards recorded in ``.apm/architecture/owners/install-deployment.json``: ``install-deployment-frozen-mutation-eligibility``, ``install-deployment-mcp-ownership-migration``, -``install-deployment-uninstall-reachability``, and -``install-deployment-audit-replay``. +``install-deployment-uninstall-reachability``, +``install-deployment-audit-replay``, and +``install-deployment-conflicted-lockfile-discard``. """ from __future__ import annotations @@ -21,6 +22,7 @@ _body_has, _duplicate_definition_lines, _facts_for, + _line_findings, _lines, _name_calls_in, _present, @@ -43,6 +45,7 @@ _GUARD_AUDIT_REPLAY = "install-deployment-audit-replay" _GUARD_LIFECYCLE_SERIALIZATION = "install-deployment-lifecycle-serialization" +_GUARD_CONFLICTED_LOCKFILE = "install-deployment-conflicted-lockfile-discard" def _first_line(facts: object, needle: str) -> int | None: @@ -382,3 +385,62 @@ def check_lifecycle_serialization(provider: FactsProvider) -> tuple[Violation, . ) ) return tuple(findings) + + +_CONFLICT_DETECTION_OWNER = "src/apm_cli/deps/lockfile.py" +_CONFLICT_DISCARD_OWNER = "src/apm_cli/install/transaction.py" +_CONFLICT_DISCARD_CALLERS = ( + "src/apm_cli/commands/install.py", + "src/apm_cli/commands/lock.py", +) +_CONFLICT_MARKER_USE = re.compile(r"\bhas_conflict_markers\(") +_CONFLICT_DISCARD_DEF = re.compile(r"^\s*def discard_conflicted_lockfile\(") + + +def check_conflicted_lockfile_discard(provider: FactsProvider) -> tuple[Violation, ...]: + """Conflicted-lockfile discard and restore stay inside InstallTransaction.""" + rule_id = _GUARD_CONFLICTED_LOCKFILE + message = "Conflicted lockfile discard must route through InstallTransaction" + owner, owner_fail = _facts_for(provider, _CONFLICT_DISCARD_OWNER, rule_id) + if owner_fail: + return tuple(owner_fail) + + findings: list[Violation] = [] + owner_complete = ( + _present_re(owner, re.compile(r"^ def discard_conflicted_lockfile\(")) + and _present_re(owner, re.compile(r"^ def _restore_discarded_lockfile\(")) + and "_restore_discarded_lockfile" in _method_calls_in(owner, "rollback") + ) + if not owner_complete: + findings.append(_summary(rule_id, _CONFLICT_DISCARD_OWNER, message)) + for path in _CONFLICT_DISCARD_CALLERS: + caller, caller_fail = _facts_for(provider, path, rule_id) + if caller_fail: + findings.extend(caller_fail) + elif not _present(caller, ".discard_conflicted_lockfile("): + findings.append(_summary(rule_id, path, message)) + for path in _python_paths(provider, _SRC_PREFIX): + if path in (_CONFLICT_DETECTION_OWNER, _CONFLICT_DISCARD_OWNER): + continue + facts = provider.file_facts(path) + if facts.read_error is not None: + continue + for pattern in (_CONFLICT_MARKER_USE, _CONFLICT_DISCARD_DEF): + findings.extend( + _line_findings(facts, path, rule_id, pattern, message, respect_exempt=True) + ) + return tuple(findings) + + +def _method_calls_in(facts: object, function_name: str) -> set[str]: + """Return terminal attribute names called inside *function_name*.""" + ranges = [ + (definition.line, definition.end_line) + for definition in getattr(facts, "definitions", ()) + if definition.name == function_name and definition.kind in ("function", "async_function") + ] + names: set[str] = set() + for call in getattr(facts, "calls", ()): + if any(low <= call.line <= high for low, high in ranges): + names.add(call.qualname.rsplit(".", 1)[-1]) + return names diff --git a/scripts/architecture_linter/diagnostics.py b/scripts/architecture_linter/diagnostics.py index 917157f61a..842d811706 100644 --- a/scripts/architecture_linter/diagnostics.py +++ b/scripts/architecture_linter/diagnostics.py @@ -45,6 +45,7 @@ "install-deployment-cached-claude-skill-metadata": ("AC4",), "install-deployment-dependency-winner-selection": ("AC4",), "install-deployment-deployment-frame-projection": ("AC4",), + "install-deployment-conflicted-lockfile-discard": ("AC27",), "install-deployment-frozen-mutation-eligibility": ("AC27",), "install-deployment-git-object-field-authority": ("AC4",), "install-deployment-gitlab-facade-orchestration": ("AC3",), diff --git a/src/apm_cli/commands/lock.py b/src/apm_cli/commands/lock.py index c177ace6c9..137348d4d2 100644 --- a/src/apm_cli/commands/lock.py +++ b/src/apm_cli/commands/lock.py @@ -221,44 +221,45 @@ def _run_lock( sys.exit(1) logger = InstallLogger(verbose=verbose) + from apm_cli.commands.install import ( + _LOCKFILE_CONFLICT_DISCARDED, + _install_apm_dependencies, + ) from apm_cli.core.scope import get_modules_dir - from apm_cli.deps.lockfile import get_lockfile_path + from apm_cli.deps.lockfile import resolve_lockfile_path_for_read from apm_cli.install.transaction import InstallTransaction - transaction = InstallTransaction( + # The context manager rolls back on any exit that did not complete, so an + # interrupt after the discard still restores the conflicted lockfile. + with InstallTransaction( manifest_path=project_root / "apm.yml", apm_modules_dir=get_modules_dir(scope), validation=None, logger=logger, acquire_lock=False, - ) - try: - from apm_cli.commands.install import ( - _LOCKFILE_CONFLICT_DISCARDED, - _install_apm_dependencies, - ) - - if transaction.discard_conflicted_lockfile(get_lockfile_path(project_root)): - logger.warning(_LOCKFILE_CONFLICT_DISCARDED) - result = _install_apm_dependencies( - apm_package, - update_refs=update_refs, - verbose=verbose, - scope=scope, - parallel_downloads=parallel_downloads, - logger=logger, - no_policy=no_policy, - target=target, - lockfile_only=True, - transaction=transaction, - ) - result = transaction.complete(result) - except click.UsageError: - transaction.rollback() - raise - except Exception as e: - transaction.fail(e) - _handle_lock_error(e, verbose) + ) as transaction: + try: + lockfile_path = resolve_lockfile_path_for_read(project_root, read_only=False) + if transaction.discard_conflicted_lockfile(lockfile_path): + logger.warning(_LOCKFILE_CONFLICT_DISCARDED) + result = _install_apm_dependencies( + apm_package, + update_refs=update_refs, + verbose=verbose, + scope=scope, + parallel_downloads=parallel_downloads, + logger=logger, + no_policy=no_policy, + target=target, + lockfile_only=True, + transaction=transaction, + ) + result = transaction.complete(result) + except click.UsageError: + raise + except Exception as e: + transaction.fail(e) + _handle_lock_error(e, verbose) from apm_cli.install.summary import exit_unless_install_result_allows_success diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index 3c6599344f..d29a7f5e20 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -325,6 +325,14 @@ class MutationCase: new='if target.name == "copilot" and mapping is not None:', intent="Local bundle routing branches on target names instead of target primitives.", ), + MutationCase( + guard_id="install-deployment-conflicted-lockfile-discard", + rule_id="install-deployment-conflicted-lockfile-discard", + path="src/apm_cli/install/transaction.py", + old=" self._restore_discarded_lockfile()\n", + new="", + intent="Rollback stops restoring a discarded conflicted lockfile.", + ), MutationCase( guard_id="install-deployment-executable-trust-context", rule_id="install-deployment-executable-trust-context", diff --git a/tests/integration/test_install_conflicted_lockfile_e2e.py b/tests/integration/test_install_conflicted_lockfile_e2e.py index fb0315f72c..8af8c2bf03 100644 --- a/tests/integration/test_install_conflicted_lockfile_e2e.py +++ b/tests/integration/test_install_conflicted_lockfile_e2e.py @@ -210,3 +210,31 @@ def test_failed_run_after_discard_restores_the_conflicted_lockfile( assert "apm.lock.yaml restored to its previous state" in output lockfile_text = (conflicted_project / "apm.lock.yaml").read_text(encoding="utf-8") assert lockfile_text == _CONFLICTED_LOCKFILE + + +def test_lock_discards_a_conflicted_legacy_lockfile( + runner: CliRunner, conflicted_project: Path +) -> None: + (conflicted_project / "apm.lock.yaml").rename(conflicted_project / "apm.lock") + + result = _invoke(runner, ["lock"]) + + assert result.exit_code == 0, result.output + assert "resolving from apm.yml" in _combined_output(result) + assert not (conflicted_project / "apm.lock").exists() + lock = LockFile.read(conflicted_project / "apm.lock.yaml") + assert lock is not None + + +def test_lock_restores_the_conflicted_lockfile_when_interrupted( + runner: CliRunner, conflicted_project: Path +) -> None: + with patch( + "apm_cli.commands.install._install_apm_dependencies", + side_effect=KeyboardInterrupt, + ): + result = _invoke(runner, ["lock"], catch_exceptions=True) + + assert isinstance(result.exception, (KeyboardInterrupt, SystemExit)) + lockfile_text = (conflicted_project / "apm.lock.yaml").read_text(encoding="utf-8") + assert lockfile_text == _CONFLICTED_LOCKFILE diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index 1db5f7aafb..a278cbaec4 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -639,6 +639,7 @@ def exiting_import( install-deployment-manifest-inheritance-includes install-deployment-marketplace-mutation-lock install-deployment-lifecycle-serialization +install-deployment-conflicted-lockfile-discard install-deployment-mcp-ownership-migration install-deployment-mcp-registry-resolution install-deployment-outcome From c3e534ee6cd6bad14eb056fa37b07a923429f722 Mon Sep 17 00:00:00 2001 From: Lachlan Heywood Date: Wed, 23 Sep 2026 16:56:26 -0400 Subject: [PATCH 6/6] docs(spec): cite conflicted-lockfile recovery as req-lk-023 The Mode B detector flagged the conflicted-lockfile behaviour as a silent extension under OpenAPM critical paths. Add the normative citation rather than a waiver: the change is observable, so it needs a requirement, not an exemption. req-lk-023 (Section 5.4, consumer MUST) has three clauses: detection is centralised at lockfile load and names the path plus a next action executable while the markers are present; only an operation that re-resolves every declared dependency may discard the file, while replay, subset-scoped, and preview operations fail closed; and a discard is reverted when the operation writes no replacement. A lockfile unreadable for any other reason stays out of scope. Manifest, Appendix C, Section 5.7, Section 11.3.2, statement counts, and the revision history are updated, with three conformance oracles under tests/spec_conformance/ and regenerated CONFORMANCE.{md,json}. --- CONFORMANCE.json | 17 ++- CONFORMANCE.md | 3 +- .../manifests/openapm-v0.1.requirements.yml | 5 + docs/src/content/docs/specs/openapm-v0.1.md | 46 ++++++- tests/spec_conformance/test_lockfile_reqs.py | 127 +++++++++++++++++- 5 files changed, 191 insertions(+), 7 deletions(-) diff --git a/CONFORMANCE.json b/CONFORMANCE.json index 07fdca397f..98edd53810 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -312,6 +312,19 @@ "tests/spec_conformance/test_lockfile_reqs.py::test_materialization_spelling_migrates_one_case_variant_transactionally" ] }, + { + "conformance_class": "consumer", + "id": "req-lk-023", + "keyword": "MUST", + "section": "5.4", + "status": "active", + "test_count": 3, + "tests": [ + "tests/spec_conformance/test_lockfile_reqs.py::test_conflict_markers_are_named_at_lockfile_load", + "tests/spec_conformance/test_lockfile_reqs.py::test_discard_is_reverted_when_no_replacement_is_written", + "tests/spec_conformance/test_lockfile_reqs.py::test_full_re_resolution_discards_while_replay_and_subset_fail_closed" + ] + }, { "conformance_class": "producer", "id": "req-mf-001", @@ -1488,7 +1501,7 @@ "spec_version": "v0.1.1", "summary_by_class": { "consumer": { - "active": 91, + "active": 92, "skipped": 1, "unbound": 0, "xfail": 0 @@ -1512,5 +1525,5 @@ "xfail": 0 } }, - "total_requirements": 123 + "total_requirements": 124 } diff --git a/CONFORMANCE.md b/CONFORMANCE.md index f4bc15cc77..b9979a7581 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -29,7 +29,7 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | Class | Active | Skipped | Xfail | Unbound | |-------|-------:|--------:|------:|--------:| | Producer | 12 | 0 | 0 | 0 | -| Consumer | 91 | 1 | 0 | 0 | +| Consumer | 92 | 1 | 0 | 0 | | Registry | 1 | 0 | 0 | 0 | | Governance | 18 | 0 | 0 | 0 | @@ -63,6 +63,7 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | [req-lk-020](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-020) | MUST | 5.2 | consumer | active | 3 | - | | [req-lk-021](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-021) | MUST | 5.2 | consumer | active | 2 | - | | [req-lk-022](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-022) | MUST | 5.2 | consumer | active | 4 | - | +| [req-lk-023](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-023) | MUST | 5.4 | consumer | active | 3 | - | | [req-mf-001](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-001) | MUST | 4.1 | producer | active | 1 | - | | [req-mf-002](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-002) | MUST | 4.1 | producer | active | 1 | - | | [req-mf-003](docs/src/content/docs/specs/openapm-v0.1.md#req-mf-003) | MUST | 4.1 | producer | active | 1 | - | diff --git a/docs/public/specs/manifests/openapm-v0.1.requirements.yml b/docs/public/specs/manifests/openapm-v0.1.requirements.yml index 24576982dc..a8f460214b 100644 --- a/docs/public/specs/manifests/openapm-v0.1.requirements.yml +++ b/docs/public/specs/manifests/openapm-v0.1.requirements.yml @@ -226,6 +226,11 @@ requirements: section: "5.2" conformance_class: consumer notes: "source-cased materialization_repo_url remains non-identity metadata and drives rollback-safe, collision-closed materialization/link spelling" + - id: req-lk-023 + keyword: MUST + section: "5.4" + conformance_class: consumer + notes: "merge-conflict markers are detected at lockfile load; only a full re-resolution may discard the file, and the discard is reverted when no replacement is written" - id: req-pl-001 keyword: MUST section: "6.1" diff --git a/docs/src/content/docs/specs/openapm-v0.1.md b/docs/src/content/docs/specs/openapm-v0.1.md index 28ed2abd09..42dd478600 100644 --- a/docs/src/content/docs/specs/openapm-v0.1.md +++ b/docs/src/content/docs/specs/openapm-v0.1.md @@ -136,7 +136,7 @@ between the companion corpus and the implementation. ### 1.3 Document conventions -- OpenAPM v0.1 carries **123 normative statements (118 MUST, 5 SHOULD)** indexed in +- OpenAPM v0.1 carries **124 normative statements (119 MUST, 5 SHOULD)** indexed in [Appendix C](#appendix-c-index-of-normative-statements). - All on-disk files defined by this specification are **YAML 1.2** parsed under the safe subset defined in @@ -1169,6 +1169,42 @@ of the versions it recognises, with a diagnostic that explicitly offers the user a choice of either upgrading the consumer or regenerating the lockfile from the manifest. + +**[req-lk-023]** A conforming **consumer** implementation MUST treat +a lockfile carrying unresolved version-control merge conflict +markers as a distinct recoverable condition rather than a generic +parse failure, subject to all of the following: + +(a) **Detection is centralised.** The consumer MUST detect conflict +markers at the point where it loads the lockfile, before parsing, +and MUST report the condition with a diagnostic naming the lockfile +path and one next action that is executable while the markers are +present. A diagnostic that names only a serialization error, or that +directs the user to an operation which itself cannot read the +lockfile, does not satisfy this clause. + +(b) **Only a full re-resolution may discard it.** A consumer +performing an operation that re-resolves every dependency declared +in the manifest MAY discard the conflicted lockfile and resolve from +the manifest, provided it emits a diagnostic recording the discard. +Any other operation -- a frozen or lockfile-replay install +([req-lk-012](#req-lk-012)), an operation scoped to a subset of the +declared dependencies, or a preview that performs no durable write +-- MUST NOT remove or rewrite the lockfile and MUST fail closed with +the diagnostic from clause (a). + +(c) **The discard is revertible.** When a consumer discards a +conflicted lockfile under clause (b) and the operation then +terminates without writing a replacement lockfile, the consumer MUST +restore the conflicted file as it was on disk. A lockfile written by +the operation itself supersedes the discarded file and MUST NOT be +replaced by it. + +A lockfile that is unreadable for any reason other than conflict +markers is out of scope for this requirement and continues to fail +closed in every mode; this requirement never authorises replacing an +invalid lockfile with an empty one. + ### 5.5 Drift and integrity model The lockfile is the contract `apm audit` validates the workspace @@ -1333,7 +1369,8 @@ This section's normative statements are: [req-lk-014](#req-lk-014), [req-lk-015](#req-lk-015), [req-lk-016](#req-lk-016), [req-lk-017](#req-lk-017), [req-lk-019](#req-lk-019), [req-lk-020](#req-lk-020), - [req-lk-021](#req-lk-021), [req-lk-022](#req-lk-022). + [req-lk-021](#req-lk-021), [req-lk-022](#req-lk-022), + [req-lk-023](#req-lk-023). - Consumer (SHOULD): [req-lk-007](#req-lk-007), [req-lk-018](#req-lk-018). @@ -3509,6 +3546,7 @@ conformance statement identifying: [req-lk-017](#req-lk-017), [req-lk-018](#req-lk-018) (SHOULD), [req-lk-019](#req-lk-019), [req-lk-020](#req-lk-020), [req-lk-021](#req-lk-021), [req-lk-022](#req-lk-022), +[req-lk-023](#req-lk-023), [req-rs-001](#req-rs-001), [req-rs-002](#req-rs-002), [req-rs-003](#req-rs-003), [req-rs-004](#req-rs-004), [req-rs-005](#req-rs-005), [req-rs-006](#req-rs-006), @@ -3931,6 +3969,7 @@ renumbering of conformance classes. | [req-lk-020](#req-lk-020) | MUST | 5.2 | consumer | | [req-lk-021](#req-lk-021) | MUST | 5.2 | consumer | | [req-lk-022](#req-lk-022) | MUST | 5.2 | consumer | +| [req-lk-023](#req-lk-023) | MUST | 5.4 | consumer | | [req-pl-001](#req-pl-001) | MUST | 6.1 | governance | | [req-pl-002](#req-pl-002) | MUST | 6.2 | governance | | [req-pl-003](#req-pl-003) | MUST | 6.4 | governance | @@ -4006,7 +4045,7 @@ renumbering of conformance classes. | [req-cf-001](#req-cf-001) | MUST | 12.5 | consumer | | [req-cf-002](#req-cf-002) | MUST | 12.3 | consumer | -**Total normative statements: 123** (118 MUST, 5 SHOULD). +**Total normative statements: 124** (119 MUST, 5 SHOULD). --- @@ -4057,6 +4096,7 @@ renumbering of conformance classes. | 0.1.39 | 2026-09-01 | Spec-citation fold for user-scoped direct MCP target selection (closes #2548 Mode-B silent-extension gate). Added [req-tg-014] (Section 8.5.8, consumer MUST): explicit selection, the user-scope manifest, configured user default, and user-scope runtime discovery form one precedence chain; project-only signals cannot constrain final discovery; and a selected set with no user-capable runtime fails before user manifest, lockfile, or target-config mutation. Section 8.7, Section 11.3.2, and Appendix C updated. Statement count: 120 -> 121 (116 MUST, 5 SHOULD). | | 0.1.40 | 2026-09-07 | Spec-citation fold for dependency-policy identity casing in PR #2706. Added [req-pl-018] (Section 6.3.1, governance MUST) and extended [req-rs-016] clause (3): dependency allow, deny, and exact require operands use the documented per-host repository case rule, while registry-sourced repository coordinates are case-insensitive regardless of host; case normalization is ASCII-only, is bounded identically on both operands, stops at recursive-glob ambiguity, and does not cross virtual-path, ref, registry-name, MCP-name, unmanaged-path, or case-sensitive host/source boundaries; deny precedence is unchanged. Defined the policy glob grammar, documented byte-exact Section 6.4 merge behavior, and added the threat mapping. Classified this as a non-breaking correction of previously unspecified evaluation behavior under Section 9.2: existing lowercase workarounds remain matching; on registry sources and hosts documented as case-insensitive, case-variant allow entries can newly match, deny entries can newly enforce, and exact require entries can newly be satisfied, so those policies should be re-audited. Sections 1.3, 6.3.1, 6.3.5, 6.4, 6.5, 6.9, 7.2, 9.2, 10.8, 10.11, 11.2, and 11.3.4, Appendix C, and conformance coverage updated. Statement count: 121 -> 122 (117 MUST, 5 SHOULD). | | 0.1.41 | 2026-09-09 | Alias containment and lock-replay contract for PR #2901. Added [req-mf-025] (Section 4.3.2, consumer MUST), the optional lock-entry `alias` field, and conformance coverage. Under Section 9.2 this is an additive optional field and a defensive definition of previously unspecified alias behavior, not behavior-neutral errata: unsafe or reserved aliases can newly fail; valid dotted aliases remain accepted; surrounding whitespace is canonicalized; recorded aliases determine replay placement; absent aliases retain the unaliased layout. Source identity and permitted local source paths are unchanged. Older readers preserving the unknown field do not thereby implement placement support. Selects distinct 0.1.41 schema publication identities without changing published v0.1 URLs or bytes; Section 9.3 remains pending (see Appendix A). Sections 1.3, 4.9, 5.2, 10.7, 10.11, 11.3.2, and Appendix C updated. Statement count: 122 -> 123 (118 MUST, 5 SHOULD). | +| 0.1.42 | 2026-09-23 | Spec-citation fold for conflicted-lockfile recovery (closes the #2979 Mode-B silent-extension gate). Added [req-lk-023] (Section 5.4, consumer MUST): a lockfile carrying unresolved version-control merge conflict markers is a distinct recoverable condition, not a generic parse failure. Clause (a) centralises detection at lockfile load and requires a diagnostic naming the path and a next action executable while the markers are present; clause (b) permits discarding the file only for an operation that re-resolves every declared dependency, and requires frozen/replay, subset-scoped, and preview operations to fail closed without removing or rewriting it; clause (c) requires a discard to be reverted when the operation terminates without writing a replacement. A lockfile unreadable for any other reason stays out of scope and continues to fail closed. Section 5.7, Section 11.3.2, and Appendix C updated. Statement count: 123 -> 124 (119 MUST, 5 SHOULD). | Errata (none at publication). diff --git a/tests/spec_conformance/test_lockfile_reqs.py b/tests/spec_conformance/test_lockfile_reqs.py index 628fbae9ab..d4775837b0 100644 --- a/tests/spec_conformance/test_lockfile_reqs.py +++ b/tests/spec_conformance/test_lockfile_reqs.py @@ -1,6 +1,6 @@ """Lockfile (apm.lock.yaml) conformance tests -- sec.5. -Covers req-lk-001..022. The integrity sub-cluster (req-lk-012..017) +Covers req-lk-001..023. The integrity sub-cluster (req-lk-012..017) now drives REAL fail-closed oracles against the committed binary fixture pair under `integrity/`. """ @@ -758,3 +758,128 @@ def test_dropped_target_merge_hook_state_reconciled_fail_safe(tmp_path): "target while its ownership record remains", "MUST leave that document or record unmodified and\nemit an actionable diagnostic", ) + + +_CONFLICTED_LOCKFILE = ( + "lockfile_version: '1'\n" + "<<<<<<< HEAD\n" + "dependencies: []\n" + "=======\n" + "dependencies:\n" + "- repo_url: example/x\n" + ">>>>>>> feature\n" +) + + +def _conflicted_project(tmp_path: Path) -> Path: + """Write a manifest with local content plus a conflicted lockfile.""" + (tmp_path / "apm.yml").write_text( + "name: conformance\nversion: '1.0.0'\ntargets:\n - claude\n", + encoding="utf-8", + ) + instructions = tmp_path / ".apm" / "instructions" + instructions.mkdir(parents=True) + (instructions / "c.instructions.md").write_text("conformance\n", encoding="utf-8") + (tmp_path / "apm.lock.yaml").write_text(_CONFLICTED_LOCKFILE, encoding="utf-8") + return tmp_path + + +def _install_transaction(project: Path): + from apm_cli.install.transaction import InstallTransaction + + return InstallTransaction( + manifest_path=project / "apm.yml", + apm_modules_dir=project / "apm_modules", + validation=None, + logger=None, + acquire_lock=False, + ) + + +@pytest.mark.req("req-lk-023") +def test_conflict_markers_are_named_at_lockfile_load(tmp_path: Path) -> None: + """Clause (a): detection happens at load and names the path plus a next action.""" + from apm_cli.deps.lockfile import LockFile, LockfileConflictError, LockfileFormatError + + project = _conflicted_project(tmp_path) + lockfile_path = project / "apm.lock.yaml" + + with pytest.raises(LockfileConflictError) as conflict: + LockFile.read(lockfile_path) + + message = str(conflict.value) + assert str(lockfile_path) in message, "the diagnostic MUST name the lockfile" + assert "conflict markers" in message + assert "apm install" in message, ( + "the named next action MUST be executable while the markers are present" + ) + assert isinstance(conflict.value, LockfileFormatError) + + # A lockfile unreadable for any other reason stays out of scope. + lockfile_path.write_text("lockfile_version: '1'\ndependencies: [\n", encoding="utf-8") + with pytest.raises(LockfileFormatError) as generic: + LockFile.read(lockfile_path) + assert not isinstance(generic.value, LockfileConflictError) + + +@pytest.mark.req("req-lk-023") +def test_full_re_resolution_discards_while_replay_and_subset_fail_closed( + tmp_path: Path, +) -> None: + """Clause (b): only a full re-resolution may discard the conflicted lockfile.""" + from unittest.mock import MagicMock + + from apm_cli.install.errors import FrozenInstallError + from apm_cli.install.request import InstallRequest + from apm_cli.install.service import InstallService + + project = _conflicted_project(tmp_path) + lockfile_path = project / "apm.lock.yaml" + + package = MagicMock() + package.package_path = project / "apm.yml" + package.get_apm_dependencies.return_value = [] + package.get_dev_apm_dependencies.return_value = [] + + with pytest.raises(FrozenInstallError, match="conflict markers") as frozen: + InstallService.enforce_frozen(InstallRequest(apm_package=package, frozen=True)) + assert "without --frozen" in str(frozen.value) + assert lockfile_path.read_text(encoding="utf-8") == _CONFLICTED_LOCKFILE, ( + "a lockfile-replay install MUST NOT remove or rewrite the lockfile" + ) + + transaction = _install_transaction(project) + assert transaction.discard_conflicted_lockfile(lockfile_path) is True + assert not lockfile_path.exists() + + +@pytest.mark.req("req-lk-023") +def test_discard_is_reverted_when_no_replacement_is_written(tmp_path: Path) -> None: + """Clause (c): a terminated operation restores the conflicted lockfile.""" + from apm_cli.models.results import InstallResult + + project = _conflicted_project(tmp_path) + lockfile_path = project / "apm.lock.yaml" + + transaction = _install_transaction(project) + transaction.discard_conflicted_lockfile(lockfile_path) + transaction.fail(RuntimeError("resolution failed")) + + assert lockfile_path.read_text(encoding="utf-8") == _CONFLICTED_LOCKFILE + + # A replacement written by the operation itself supersedes the discarded file. + replacement = "lockfile_version: '1'\ndependencies: []\n" + transaction = _install_transaction(project) + transaction.discard_conflicted_lockfile(lockfile_path) + lockfile_path.write_text(replacement, encoding="utf-8") + transaction.commit(InstallResult()) + + assert lockfile_path.read_text(encoding="utf-8") == replacement + + assert_spec_contains( + "MUST treat\na lockfile carrying unresolved version-control merge conflict\nmarkers", + "MUST detect conflict\nmarkers at the point where it loads the lockfile, before parsing", + "MAY discard the conflicted lockfile and resolve from\nthe manifest", + "MUST NOT remove or rewrite the lockfile and MUST fail closed", + "MUST\nrestore the conflicted file as it was on disk", + )