diff --git a/.apm/architecture/owners/contracts-tooling.json b/.apm/architecture/owners/contracts-tooling.json index d2745c583..cb21eb615 100644 --- a/.apm/architecture/owners/contracts-tooling.json +++ b/.apm/architecture/owners/contracts-tooling.json @@ -36,6 +36,13 @@ "selectors": ["src/apm_cli/utils/yaml_io.py"], "guards": ["contracts-tooling-frontmatter-yaml"] }, + { + "id": "read-only-lockfile-path", + "decision": "Read-only lockfile path resolution", + "owner": "deps/lockfile.py (resolve_lockfile_path_for_read)", + "selectors": ["src/apm_cli/deps/lockfile.py"], + "guards": ["contracts-tooling-lockfile-read"] + }, { "id": "generated-content-footer-wording", "decision": "Generated-content footer ownership wording", diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f0dd2347..f8db6e63d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `apm pack --check-clean` is now read-only and detects marketplace drift + without overwriting artifacts. Release pipelines that also produce artifacts + must run `apm pack` separately; see + [Releasing from any CI](docs/src/content/docs/producer/releasing-from-any-ci.md#the-canonical-sequence). + (by @danielmeppiel, closes #2727, #2730) - Distributed `apm compile` now reconciles existing managed-section `AGENTS.md` files without overwriting hand-authored content, generates new placements safely, and never discovers, writes, or cleans content across diff --git a/docs/src/content/docs/producer/releasing-from-any-ci.md b/docs/src/content/docs/producer/releasing-from-any-ci.md index da3868294..56a5de2c5 100644 --- a/docs/src/content/docs/producer/releasing-from-any-ci.md +++ b/docs/src/content/docs/producer/releasing-from-any-ci.md @@ -19,7 +19,8 @@ shell-script translation of these lines. set -euo pipefail VERSION="${VERSION:?VERSION must be set, e.g. v1.2.3}" -apm pack --check-versions --check-clean --json > pack-report.json +apm pack --check-versions --check-clean --json > gate-report.json +apm pack --json > pack-report.json for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue @@ -37,18 +38,27 @@ gh release create "$VERSION" \ What each command does: -- `apm pack --check-versions --check-clean --json` runs the pack with - the release gates enabled. `--check-versions` fails if per-package +- `apm pack --check-versions --check-clean --json` runs the read-only + release gates. `--check-versions` fails if per-package versions disagree with `marketplace.versioning.strategy`. `--check-clean` fails if the on-disk `marketplace.json` does not match what a fresh pack would produce. `--json` writes a machine-readable summary to stdout; human logs go to stderr. +- `apm pack --json` then writes the release artifacts after both gates + pass. - `sha256sum` produces one sidecar per artifact. Consumers verify with `sha256sum -c .sha256`. - `gh release create` uploads the bundle, the marketplace artifact, and the sidecars under one tag. Use whichever release API your forge exposes; the file set is what matters. +:::caution[Upgrading an existing release pipeline?] +`--check-clean` is now always read-only. If an older pipeline relied on one +`apm pack --check-clean` call to both validate and produce artifacts, split it +into the gate and pack calls shown above. Install one pinned apm-cli version +for the job so both calls use identical generation logic. +::: + Authenticate `gh` with a token that has `contents: write` on the repo. Substitute the equivalent verb for non-GitHub forges (`glab release create`, `az repos`, REST upload). @@ -76,11 +86,12 @@ jobs: [`microsoft/apm-action@v1`](https://github.com/microsoft/apm-action) with `mode: release` is a convenience wrapper for the canonical -sequence above. It installs the CLI, runs `apm pack ---check-versions --check-clean --json`, generates the sidecars, and -calls `gh release create` against the pushed tag. Use it when you +sequence above. It installs the CLI, runs the read-only gates, packs +the release artifacts separately, generates the sidecars, and calls +`gh release create` against the pushed tag. Use it when you want one less script to maintain; use the raw `run:` form below when -you need to customise any step. +you need to customise any step. The split gate-and-pack flow requires +apm-action `v1.10.0` or newer. > **Reference deployment.** [`DevExpGbb/zava-agent-config`](https://github.com/DevExpGbb/zava-agent-config) > runs this exact pipeline. The @@ -88,7 +99,8 @@ you need to customise any step. > attaches 7 per-plugin bundles + their `.sha256` companions + > `marketplace-6.1.2.json` (15 assets total) via the workflow in > [`.github/workflows/release.yml`](https://github.com/DevExpGbb/zava-agent-config/blob/main/.github/workflows/release.yml). -> APM `0.16.0` and apm-action `v1.9.1` or newer required. +> APM `0.16.0` or newer is required; use apm-action `v1.10.0` or newer +> for the split gate-and-pack flow documented here. :::caution[Migrating release workflows from `.tar.gz`?] The examples below assume the new `.zip` default from `apm pack --archive`. @@ -102,7 +114,8 @@ artifact format. with: { python-version: "3.12" } - run: pip install apm-cli - run: | - apm pack --check-versions --check-clean --json > pack-report.json + apm pack --check-versions --check-clean --json > gate-report.json + apm pack --json > pack-report.json for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue sha256sum "$f" > "${f}.sha256" @@ -125,7 +138,8 @@ release: - if: '$CI_COMMIT_TAG =~ /^v/' script: - pip install apm-cli - - apm pack --check-versions --check-clean --json > pack-report.json + - apm pack --check-versions --check-clean --json > gate-report.json + - apm pack --json > pack-report.json - | for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue @@ -149,7 +163,8 @@ pipeline { steps { sh ''' pip install apm-cli - apm pack --check-versions --check-clean --json > pack-report.json + apm pack --check-versions --check-clean --json > gate-report.json + apm pack --json > pack-report.json for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue sha256sum "$f" > "${f}.sha256" @@ -176,7 +191,8 @@ steps: - task: UsePythonVersion@0 inputs: { versionSpec: "3.12" } - script: pip install apm-cli - - script: apm pack --check-versions --check-clean --json > pack-report.json + - script: apm pack --check-versions --check-clean --json > gate-report.json + - script: apm pack --json > pack-report.json - script: | for f in build/*.zip .claude-plugin/marketplace.json; do [ -f "$f" ] || continue diff --git a/docs/src/content/docs/reference/cli/pack.md b/docs/src/content/docs/reference/cli/pack.md index c4cb31323..a74f62b68 100644 --- a/docs/src/content/docs/reference/cli/pack.md +++ b/docs/src/content/docs/reference/cli/pack.md @@ -43,7 +43,7 @@ Bundles are target-agnostic. The consumer's project decides where files land at | `--json` | off | Emit machine-readable JSON to stdout. All logs move to stderr. Shape: `{ok, dry_run, warnings, errors, marketplace: {outputs: [...]}}`. | | `--legacy-skill-paths` | off | Bundle skills under per-client paths (e.g. `.cursor/skills/`) instead of the converged `.agents/skills/`. Compatibility flag. | | `--check-versions` | off | Release gate: verify per-package versions agree with the configured `marketplace.versioning.strategy` (`lockstep`, `tag_pattern`, or `per_package`). Exits `3` on misalignment. Composes with `--check-clean` and `--dry-run`. | -| `--check-clean` | off | Release gate: regenerate every configured marketplace output to a temp representation and diff against the same effective path used by `apm pack`, including `--marketplace-path` overrides. Exits `4` for drift. Combine with `--dry-run` to compare without normal pack output generation. | +| `--check-clean` | off | Read-only release gate: regenerate every configured marketplace output to a temporary representation and diff against the same effective path used by `apm pack`, including `--marketplace-path` overrides. It never writes pack outputs and exits `4` for drift. | | `--target`, `-t VALUE` | auto-detect | **Deprecated.** Recorded as informational `pack.target` metadata only; ignored by `apm install`. Will be removed in a future release. | :::caution[Migrating automation from `.tar.gz`?] @@ -240,7 +240,7 @@ Plugin manifest generation runs after BUNDLE and MARKETPLACE phases so the gener | Code | Meaning | |---|---| -| `0` | Success. Requested artifacts written (or, with `--dry-run`, planned). | +| `0` | Success. Requested artifacts written, planned with `--dry-run`, or validated without writes by `--check-clean`. | | `1` | Build or runtime error: network failure, ref not found, no tag matches a marketplace range, lockfile read error, or unhandled packer exception. | | `2` | `apm.yml` schema validation error. | | `3` | `--check-versions` failed: per-package versions disagree with the configured marketplace versioning strategy. | diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 8c8ef7f3c..d496dfe36 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -198,7 +198,7 @@ Lifecycle scripts fire on six events: `pre-install`, `post-install`, `pre-update | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm pack` | Build distributable artifacts (bundle and/or marketplace.json -- driven by `apm.yml`). A `dependencies:` mapping, including `dependencies: {}`, produces a bundle of local package content; omitted or null `dependencies:` does not. Default output (no format flag) is a Claude Code plugin directory. Pass `--format agent-plugin` to opt into a portable Agent Plugins v1 bundle instead -- strict portable core only (root `plugin.json`, `skills/`, root `mcp.json` written even when empty; no `agents/`, `commands/`, `instructions/`, `extensions/`, `hooks/`, or LSP payload). That bundle build fails before any output is written if the source project has non-portable agents/commands/instructions/extensions/hooks/LSP, naming the surfaces and pointing to `--format claude-plugin` (and to configuring LSP in the target directly, since neither pack format carries it). A packed Agent Plugin installs through the declarative route: declare it as a dependency in `apm.yml` and run `apm install --target copilot`, which keeps the unit whole under `apm_modules/` and registers it without locating or executing Copilot; stable Copilot CLI 1.0.81 or newer is required when loading the projection. The imperative local-bundle route still fails closed for Agent Plugin bundles. Bundles are **target-agnostic**: `pack.target` is recorded in every bundle for diagnostic purposes (typically `"all"` for target-agnostic packs, or the project's detected target) and is not authoritative at install time; `pack.bundle_files` (path -> sha256) drives integrity verification. The consumer's project decides where files land. Dependency content is packed **exclusively** from lockfile-attested `deployed_files` (in every bundle format); the `apm_modules` cache is never packed. Each file is verified against its `deployed_file_hashes` SHA-256 before inclusion, so a file tampered after `apm install` (hash mismatch) or deleted (missing on disk) fails the pack with a message pointing at `apm install`; files with no recorded hash (older lockfiles) pack unverified. Dependency hooks-config / MCP-config is not attested, so it is not packed -- `apm pack` warns (`[!]`) and names the dependency (first-party root hooks/MCP are still packed). Marketplace-publishing projects (`marketplace:` block, no `dependencies:`) no longer emit the misleading "No plugin.json found" warning; after a successful build, a vendor-neutral catalog of artifact paths is appended together with a single docs pointer (`producer/publish-to-a-marketplace/#consume-from-any-assistant`) listing per-assistant install paths. Release-time gates `--check-versions` and `--check-clean` are opt-in: when present, they run after the build and exit non-zero on misalignment / drift (codes 3 and 4 respectively) so release pipelines can fail fast. The version gate reads a local package's `apm.yml` first; Plugin collections without `apm.yml` use `plugin.json`'s `version`. Invalid or versionless `apm.yml` fails closed, and the fallback likewise rejects malformed or non-object JSON and a missing or blank version. When `apm.yml` declares `target: claude` or `target: copilot` (or the plural `targets:` equivalent), `apm pack` also generates an ecosystem-specific `plugin.json`: `.claude-plugin/plugin.json` for Claude (includes `mcpServers` from `.mcp.json` if present) and `.github/plugin/plugin.json` for Copilot (omits `mcpServers`). An existing file at the target path is preserved (a warning is emitted and the write is skipped) unless `--force` is passed; `--dry-run` prevents writes. Credential-bearing keys and secret-shaped values in `.mcp.json` are stripped recursively at any depth from the Claude manifest before writing, so a committed manifest never leaks secrets (see the apm pack reference, `reference/cli/pack/#credential-stripping-claude-mcpservers`). | `-o PATH`, `--archive` (produce a `.zip` archive instead of a directory; changed from `.tar.gz`), `--archive-format [zip\|tar.gz]` (default `zip`; use `tar.gz` for smaller legacy CI artifacts; only active with `--archive`), `--dry-run`, `--format [plugin\|agent-plugin\|claude\|claude-plugin\|apm]` (`agent-plugin` is the sole selector for the Agent Plugin bundle; `plugin` is a compatibility alias for the Claude plugin bundle, not for `agent-plugin`; `claude`/`claude-plugin` also select the Claude plugin bundle; `apm` selects the legacy APM layout; default `claude-plugin`), `--claude-plugin` (shortcut for `--format claude-plugin`; passing more than one of `--claude-plugin`/`--format` is a usage error), `--force`, `--offline`, `--include-prerelease`, `--marketplace=FORMATS`, `--marketplace-path FORMAT=PATH`, `--json`, `--check-versions` (release gate: per-package versions match `marketplace.versioning.strategy`; exit 3 on failure), `--check-clean` (release gate: regenerate-and-diff against the effective marketplace path, including `--marketplace-path` overrides; exit 4 on drift; pair with `--dry-run` to avoid normal pack output generation). `-t/--target` is **deprecated** (warn only). Exit codes: `0` success, `1` build/runtime error, `2` schema validation error, `3` `--check-versions` misalignment, `4` `--check-clean` drift. | +| `apm pack` | Build distributable artifacts (bundle and/or marketplace.json -- driven by `apm.yml`). A `dependencies:` mapping, including `dependencies: {}`, produces a bundle of local package content; omitted or null `dependencies:` does not. Default output (no format flag) is a Claude Code plugin directory. Pass `--format agent-plugin` to opt into a portable Agent Plugins v1 bundle instead -- strict portable core only (root `plugin.json`, `skills/`, root `mcp.json` written even when empty; no `agents/`, `commands/`, `instructions/`, `extensions/`, `hooks/`, or LSP payload). That bundle build fails before any output is written if the source project has non-portable agents/commands/instructions/extensions/hooks/LSP, naming the surfaces and pointing to `--format claude-plugin` (and to configuring LSP in the target directly, since neither pack format carries it). A packed Agent Plugin installs through the declarative route: declare it as a dependency in `apm.yml` and run `apm install --target copilot`, which keeps the unit whole under `apm_modules/` and registers it without locating or executing Copilot; stable Copilot CLI 1.0.81 or newer is required when loading the projection. The imperative local-bundle route still fails closed for Agent Plugin bundles. Bundles are **target-agnostic**: `pack.target` is recorded in every bundle for diagnostic purposes (typically `"all"` for target-agnostic packs, or the project's detected target) and is not authoritative at install time; `pack.bundle_files` (path -> sha256) drives integrity verification. The consumer's project decides where files land. Dependency content is packed **exclusively** from lockfile-attested `deployed_files` (in every bundle format); the `apm_modules` cache is never packed. Each file is verified against its `deployed_file_hashes` SHA-256 before inclusion, so a file tampered after `apm install` (hash mismatch) or deleted (missing on disk) fails the pack with a message pointing at `apm install`; files with no recorded hash (older lockfiles) pack unverified. Dependency hooks-config / MCP-config is not attested, so it is not packed -- `apm pack` warns (`[!]`) and names the dependency (first-party root hooks/MCP are still packed). Marketplace-publishing projects (`marketplace:` block, no `dependencies:`) no longer emit the misleading "No plugin.json found" warning; after a successful build, a vendor-neutral catalog of artifact paths is appended together with a single docs pointer (`producer/publish-to-a-marketplace/#consume-from-any-assistant`) listing per-assistant install paths. Release-time gates `--check-versions` and `--check-clean` are opt-in and exit non-zero on misalignment / drift (codes 3 and 4 respectively) so release pipelines can fail fast; `--check-clean` is always read-only and never writes pack outputs. The version gate reads a local package's `apm.yml` first; Plugin collections without `apm.yml` use `plugin.json`'s `version`. Invalid or versionless `apm.yml` fails closed, and the fallback likewise rejects malformed or non-object JSON and a missing or blank version. When `apm.yml` declares `target: claude` or `target: copilot` (or the plural `targets:` equivalent), `apm pack` also generates an ecosystem-specific `plugin.json`: `.claude-plugin/plugin.json` for Claude (includes `mcpServers` from `.mcp.json` if present) and `.github/plugin/plugin.json` for Copilot (omits `mcpServers`). An existing file at the target path is preserved (a warning is emitted and the write is skipped) unless `--force` is passed; `--dry-run` prevents writes. Credential-bearing keys and secret-shaped values in `.mcp.json` are stripped recursively at any depth from the Claude manifest before writing, so a committed manifest never leaks secrets (see the apm pack reference, `reference/cli/pack/#credential-stripping-claude-mcpservers`). | `-o PATH`, `--archive` (produce a `.zip` archive instead of a directory; changed from `.tar.gz`), `--archive-format [zip\|tar.gz]` (default `zip`; use `tar.gz` for smaller legacy CI artifacts; only active with `--archive`), `--dry-run`, `--format [plugin\|agent-plugin\|claude\|claude-plugin\|apm]` (`agent-plugin` is the sole selector for the Agent Plugin bundle; `plugin` is a compatibility alias for the Claude plugin bundle, not for `agent-plugin`; `claude`/`claude-plugin` also select the Claude plugin bundle; `apm` selects the legacy APM layout; default `claude-plugin`), `--claude-plugin` (shortcut for `--format claude-plugin`; passing more than one of `--claude-plugin`/`--format` is a usage error), `--force`, `--offline`, `--include-prerelease`, `--marketplace=FORMATS`, `--marketplace-path FORMAT=PATH`, `--json`, `--check-versions` (release gate: per-package versions match `marketplace.versioning.strategy`; exit 3 on failure), `--check-clean` (read-only release gate: regenerate-and-diff against the effective marketplace path, including `--marketplace-path` overrides; never writes pack outputs; exit 4 on drift). `-t/--target` is **deprecated** (warn only). Exit codes: `0` success, `1` build/runtime error, `2` schema validation error, `3` `--check-versions` misalignment, `4` `--check-clean` drift. | | `apm unpack BUNDLE` | **[Deprecated]** Extract a bundle. Use `apm install ` instead -- it deploys directly with integrity verification and target resolution. | `-o PATH`, `--skip-verify`, `--force`, `--dry-run` | `apm install ` -- when the positional argument resolves to a directory containing `plugin.json` at its root, or to a `.zip` (or legacy `.tar.gz`/`.tgz`) archive whose extracted root contains `plugin.json`, install switches to local-bundle mode: the bundle is integrity-verified against its embedded `apm.lock.yaml` (`pack.bundle_files`) and deployed into the consumer's resolved target. Root `plugin.json` routing is exclusively schema-driven: only a `plugin.json` with **no** `$schema` key follows the legacy Claude/APM path described below. If `plugin.json` declares the exact recognized Agent Plugins v1 `$schema` (`1.0.0`), the imperative bundle route fails closed instead of dissecting it, with a message pointing at the declarative dependency route (`apm.yml` + `apm install --target copilot`) that registers the plugin natively with GitHub Copilot. Any other schema-bearing `plugin.json` hard-fails rather than falling back to legacy routing: a non-string `$schema`, an Agent Plugins schema at an unsupported version, or a foreign schema id from another tool entirely all raise before any dissection is attempted. Target resolution follows the same precedence as registry installs (`--target` > `apm.yml` > directory detection); the bundle itself carries no target binding. Targets without target-native instruction deployment (opencode, codex, gemini) receive instructions staged under `apm_modules//.apm/instructions/` and the install emits a hint to run `apm compile` to merge them. Grok Build deploys native instructions to `.grok/rules/`; run `apm compile` separately when you also want `AGENTS.md`. Other existing paths (e.g. a source-package directory without `plugin.json`) still flow through the normal local-path dependency-resolver pipeline. Files are recorded under `local_deployed_files` in the project lockfile -- `apm.yml` is **never** mutated. Honours `--target`, `--global`, `--force`, `--dry-run`, `--verbose`, plus `--as ALIAS` (log/display label only). Resolver/MCP/registry/policy flags (`--update`, `--mcp`, `--parallel-downloads`, `--allow-insecure-host`, `--skill`, ...) are rejected with a single consolidated error -- local-bundle install is an imperative deploy and bypasses those subsystems. diff --git a/scripts/architecture_linter/checks/contracts_test_taxonomy.py b/scripts/architecture_linter/checks/contracts_test_taxonomy.py index 898a61529..b62e19ad4 100644 --- a/scripts/architecture_linter/checks/contracts_test_taxonomy.py +++ b/scripts/architecture_linter/checks/contracts_test_taxonomy.py @@ -13,6 +13,7 @@ from __future__ import annotations +import ast import re from collections.abc import Sequence @@ -65,11 +66,22 @@ _GUARD_FRONTMATTER = "contracts-tooling-frontmatter-yaml" +_GUARD_LOCKFILE_READ = "contracts-tooling-lockfile-read" + + _GUARD_GENERATION_FOOTER = "contracts-tooling-generation-footer" _SRC_PREFIX = "src/apm_cli/" +_LOCKFILE_OWNER = "src/apm_cli/deps/lockfile.py" + +_LOCKFILE_CONSUMERS = ( + "src/apm_cli/bundle/packer.py", + "src/apm_cli/bundle/plugin_exporter.py", + "src/apm_cli/bundle/agent_plugin_exporter.py", +) + def _facts_for(provider: FactsProvider, path: str, rule_id: str): """Return ``(facts, failures)`` for one Python owner/consumer file.""" @@ -119,6 +131,127 @@ def _count_defs_across(provider: FactsProvider, prefix: str, pattern: re.Pattern return total +def _named_calls(nodes: Sequence[ast.AST], name: str) -> tuple[ast.Call, ...]: + """Return direct calls to one unqualified function name.""" + return tuple( + node + for node in nodes + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == name + ) + + +def _keyword_is_name(call: ast.Call, keyword_name: str, value_name: str) -> bool: + """Return whether a call has `keyword_name=value_name`.""" + return any( + keyword.arg == keyword_name + and isinstance(keyword.value, ast.Name) + and keyword.value.id == value_name + for keyword in call.keywords + ) + + +def check_lockfile_read_resolution(provider: FactsProvider) -> tuple[Violation, ...]: + """Read-only lockfile consumers must route through one non-mutating owner.""" + rule_id = _GUARD_LOCKFILE_READ + owner, owner_failures = _facts_for(provider, _LOCKFILE_OWNER, rule_id) + if owner_failures: + return tuple(owner_failures) + owner_index = owner.tree_index + if owner_index is None: + return (_summary(rule_id, _LOCKFILE_OWNER, "Lockfile owner has no Python syntax tree"),) + + findings: list[Violation] = [] + resolver = owner_index.function("resolve_lockfile_path_for_read") + if resolver is None: + findings.append( + _summary(rule_id, _LOCKFILE_OWNER, "Read-only lockfile resolver must have one owner") + ) + else: + read_only_guards = tuple( + node + for node in owner_index.children(resolver) + if isinstance(node, ast.If) + and isinstance(node.test, ast.Name) + and node.test.id == "read_only" + ) + migrate_calls = _named_calls( + owner_index.own_scope(resolver), + "migrate_lockfile_if_needed", + ) + migration_is_read_only = bool(read_only_guards) and any( + call in owner_index.walk(read_only_guards[0]) for call in migrate_calls + ) + if len(read_only_guards) != 1 or len(migrate_calls) != 1 or migration_is_read_only: + findings.append( + _summary( + rule_id, + _LOCKFILE_OWNER, + "Read-only lockfile resolution must guard migration", + ) + ) + + installed_paths = owner_index.function("LockFile.installed_paths_for_project") + if installed_paths is None: + findings.append( + _summary(rule_id, _LOCKFILE_OWNER, "LockFile installed-path reader must exist") + ) + else: + installed_nodes = owner_index.own_scope(installed_paths) + installed_calls = _named_calls(installed_nodes, "resolve_lockfile_path_for_read") + rederives_legacy = any( + isinstance(node, ast.Name) and node.id == "LEGACY_LOCKFILE_NAME" + for node in installed_nodes + ) + has_read_only_call = len(installed_calls) == 1 and any( + keyword.arg == "read_only" + and isinstance(keyword.value, ast.Constant) + and keyword.value.value is True + for keyword in installed_calls[0].keywords + ) + if not has_read_only_call or rederives_legacy: + findings.append( + _summary( + rule_id, + _LOCKFILE_OWNER, + "LockFile installed-path reads must delegate without re-deriving fallback", + ) + ) + + for consumer_path in _LOCKFILE_CONSUMERS: + consumer, consumer_failures = _facts_for(provider, consumer_path, rule_id) + findings.extend(consumer_failures) + if consumer_failures or consumer.tree_index is None: + continue + imported = { + alias.name + for node in consumer.tree_index.nodes + if isinstance(node, ast.ImportFrom) + for alias in node.names + } + calls = _named_calls( + consumer.tree_index.nodes, + "resolve_lockfile_path_for_read", + ) + routes_read_only = len(calls) == 1 and _keyword_is_name( + calls[0], + "read_only", + "dry_run", + ) + if ( + "resolve_lockfile_path_for_read" not in imported + or {"get_lockfile_path", "migrate_lockfile_if_needed"} & imported + or not routes_read_only + ): + findings.append( + _summary( + rule_id, + consumer_path, + "Bundle lockfile reads must route through the read-only owner", + ) + ) + return tuple(findings) + + _TAXONOMY_PLUGIN = "tests/quality/taxonomy_inventory_plugin.py" @@ -590,6 +723,11 @@ def _structural_rule(rule_id: str, description: str, check) -> Rule: "Frontmatter BOM decoding and bounded YAML parsing stay owned by utils/yaml_io.py.", check_frontmatter_yaml, ), + _owner_rule( + _GUARD_LOCKFILE_READ, + "Read-only lockfile path resolution stays owned by deps/lockfile.py.", + check_lockfile_read_resolution, + ), _owner_rule( _GUARD_GENERATION_FOOTER, "Generated-content footer wording stays owned by compilation/footer.py.", diff --git a/src/apm_cli/bundle/agent_plugin_exporter.py b/src/apm_cli/bundle/agent_plugin_exporter.py index 67caaa0b7..9492737de 100644 --- a/src/apm_cli/bundle/agent_plugin_exporter.py +++ b/src/apm_cli/bundle/agent_plugin_exporter.py @@ -20,7 +20,7 @@ load_agent_plugin, url_contains_literal_secret, ) -from ..deps.lockfile import LockFile, get_lockfile_path, migrate_lockfile_if_needed +from ..deps.lockfile import LockFile, resolve_lockfile_path_for_read from ..deps.plugin_parser import synthesize_plugin_json_from_apm_yml from ..models.apm_package import APMPackage from ..utils.archive import ( @@ -300,8 +300,7 @@ def export_agent_plugin_bundle( logger=None, ) -> PackResult: """Export the project as an Agent Plugin bundle.""" - migrate_lockfile_if_needed(project_root) - lockfile_path = get_lockfile_path(project_root) + lockfile_path = resolve_lockfile_path_for_read(project_root, read_only=dry_run) lockfile = LockFile.read(lockfile_path) if lockfile is None: raise FileNotFoundError( diff --git a/src/apm_cli/bundle/packer.py b/src/apm_cli/bundle/packer.py index 475525705..433ce7b4a 100644 --- a/src/apm_cli/bundle/packer.py +++ b/src/apm_cli/bundle/packer.py @@ -5,7 +5,7 @@ from pathlib import Path from ..core.target_detection import detect_target -from ..deps.lockfile import LockFile, get_lockfile_path, migrate_lockfile_if_needed +from ..deps.lockfile import LockFile, resolve_lockfile_path_for_read from ..models.apm_package import APMPackage from ..utils.archive import ( projected_archive_path, @@ -67,8 +67,6 @@ def pack_bundle( FileNotFoundError: If ``apm.lock.yaml`` is missing. ValueError: If deployed files referenced in the lockfile are missing on disk. """ - # 1. Read lockfile (migrate legacy apm.lock → apm.lock.yaml if needed) - migrate_lockfile_if_needed(project_root) bundle_format = coerce_bundle_format(fmt) if bundle_format is BundleFormat.AGENT_PLUGIN: @@ -98,7 +96,7 @@ def pack_bundle( logger=logger, ) - lockfile_path = get_lockfile_path(project_root) + lockfile_path = resolve_lockfile_path_for_read(project_root, read_only=dry_run) lockfile = LockFile.read(lockfile_path) if lockfile is None: raise FileNotFoundError( diff --git a/src/apm_cli/bundle/plugin_exporter.py b/src/apm_cli/bundle/plugin_exporter.py index f379aa460..7b1b5b1ea 100644 --- a/src/apm_cli/bundle/plugin_exporter.py +++ b/src/apm_cli/bundle/plugin_exporter.py @@ -18,8 +18,7 @@ from ..deps.lockfile import ( LockedDependency, LockFile, - get_lockfile_path, - migrate_lockfile_if_needed, + resolve_lockfile_path_for_read, ) from ..models.apm_package import APMPackage, DependencyReference from ..models.dependency.subsets import skill_subset_filter_tokens @@ -826,8 +825,7 @@ def export_plugin_bundle( :class:`PackResult` describing what was produced. """ # 1. Read lockfile - migrate_lockfile_if_needed(project_root) - lockfile_path = get_lockfile_path(project_root) + lockfile_path = resolve_lockfile_path_for_read(project_root, read_only=dry_run) lockfile = LockFile.read(lockfile_path) # 2. Read apm.yml diff --git a/src/apm_cli/commands/pack.py b/src/apm_cli/commands/pack.py index e316df21f..ace3cb989 100644 --- a/src/apm_cli/commands/pack.py +++ b/src/apm_cli/commands/pack.py @@ -250,8 +250,8 @@ def _parse_marketplace_filter( help=( "Release gate: regenerate every configured marketplace output to a " "temp representation and diff against the effective on-disk path, " - "including --marketplace-path overrides. Exits 4 for drift. Use " - "with --dry-run to check without normal pack output generation." + "including --marketplace-path overrides. This mode is read-only and " + "exits 4 for drift." ), ) @click.option( @@ -316,7 +316,9 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option check_clean, ): """Pack APM artifacts: bundle and/or marketplace.json.""" - logger = CommandLogger("pack", verbose=verbose, dry_run=dry_run) + effective_dry_run = dry_run or check_clean + implicit_check_clean_dry_run = check_clean and not dry_run + logger = CommandLogger("pack", verbose=verbose, dry_run=effective_dry_run) try: bundle_format = resolve_bundle_format( @@ -386,7 +388,7 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option marketplace_include_prerelease=include_prerelease, marketplace_formats=marketplace_formats, marketplace_path_overrides=path_overrides if path_overrides else None, - dry_run=dry_run, + dry_run=effective_dry_run, verbose=verbose, ) @@ -530,7 +532,7 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option if json_output: envelope = { "ok": True, - "dry_run": dry_run, + "dry_run": effective_dry_run, "warnings": list(result.warnings), "errors": [], "marketplace": {"outputs": []}, @@ -555,14 +557,19 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option ctx.exit(4) return + if implicit_check_clean_dry_run: + logger.dry_run_notice("--check-clean is read-only; no pack outputs were written.") + for sub in result.producer_results: + if implicit_check_clean_dry_run: + continue if sub.kind is OutputKind.BUNDLE: _render_bundle_result( logger, sub.payload, bundle_format, target, - dry_run, + effective_dry_run, show_zip_migration_notice=( archive and archive_format == "zip" @@ -571,7 +578,9 @@ def pack_cmd( # noqa: PLR0913 -- Click handler, one param per CLI option ), ) elif sub.kind is OutputKind.MARKETPLACE: - _render_marketplace_result(logger, sub.payload, dry_run, sub.warnings, sub.outputs) + _render_marketplace_result( + logger, sub.payload, effective_dry_run, sub.warnings, sub.outputs + ) # Gate exit codes (after non-JSON rendering above): 3 wins over 4. if version_gate_failed: diff --git a/src/apm_cli/deps/lockfile.py b/src/apm_cli/deps/lockfile.py index 95cffe50a..13d420755 100644 --- a/src/apm_cli/deps/lockfile.py +++ b/src/apm_cli/deps/lockfile.py @@ -1080,12 +1080,7 @@ def installed_paths_for_project(cls, project_root: Path) -> list[str]: ordered by depth then repo_url (no duplicates). """ try: - lockfile_path = get_lockfile_path(project_root) - if not lockfile_path.exists(): - # Fallback to legacy lockfile for pre-migration reads - legacy_path = project_root / LEGACY_LOCKFILE_NAME - if legacy_path.exists(): - lockfile_path = legacy_path + lockfile_path = resolve_lockfile_path_for_read(project_root, read_only=True) lockfile = cls.read(lockfile_path) if not lockfile: return [] @@ -1105,6 +1100,19 @@ def get_lockfile_path(project_root: Path) -> Path: return project_root / LOCKFILE_NAME +def resolve_lockfile_path_for_read(project_root: Path, *, read_only: bool) -> Path: + """Resolve the lockfile path, preserving legacy files for read-only callers.""" + if read_only: + new_path = get_lockfile_path(project_root) + legacy_path = project_root / LEGACY_LOCKFILE_NAME + if not new_path.exists() and legacy_path.exists(): + return legacy_path + return new_path + + migrate_lockfile_if_needed(project_root) + return get_lockfile_path(project_root) + + def migrate_lockfile_if_needed(project_root: Path) -> bool: """Migrate legacy apm.lock to apm.lock.yaml if needed. diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index f144e2f47..bd648c936 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -139,6 +139,14 @@ class MutationCase: new="def build_generation_footer_v2(", intent="Generated footer owner loses the one canonical builder definition.", ), + MutationCase( + guard_id="contracts-tooling-lockfile-read", + rule_id="contracts-tooling-lockfile-read", + path="src/apm_cli/deps/lockfile.py", + old=" if read_only:\n", + new=" if False and read_only:\n", + intent="Read-only lockfile resolution stops guarding the mutating migration path.", + ), MutationCase( guard_id="hooks-integrations-copilot-cli-mcp-paths", rule_id="mutation_writes.copilot_cli_mcp_paths", diff --git a/tests/integration/test_architecture_pack_lockfile_read.py b/tests/integration/test_architecture_pack_lockfile_read.py new file mode 100644 index 000000000..bc2cacce6 --- /dev/null +++ b/tests/integration/test_architecture_pack_lockfile_read.py @@ -0,0 +1,95 @@ +"""Architecture guard for read-only bundle lockfile resolution.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from scripts.architecture_linter.inventory import build_inventory +from scripts.architecture_linter.registry import load_registry +from scripts.architecture_linter.runner import registered_rules, run_selected_rules + +pytestmark = pytest.mark.component + +ROOT = Path(__file__).resolve().parents[2] +RULE_ID = "contracts-tooling-lockfile-read" +OWNER = "src/apm_cli/deps/lockfile.py" + + +def test_bundle_lockfile_reads_have_one_registered_owner() -> None: + """The registry and executable rule agree on the read-only path owner.""" + registry = load_registry( + ROOT / ".apm/architecture/owners", + build_inventory(ROOT).files, + ) + owner = next(entry for entry in registry.owners if entry.id == "read-only-lockfile-path") + rule = next(entry for entry in registered_rules() if entry.id == RULE_ID) + + report = run_selected_rules(ROOT, (RULE_ID,)) + + assert owner.selectors == (OWNER,) + assert owner.guards == (RULE_ID,) + assert rule.guard_ids == (RULE_ID,) + assert report.failures == () + assert report.violations == () + + +def test_lockfile_read_rule_rejects_disabled_read_only_guard() -> None: + """The registered rule catches a migration restored under read-only mode.""" + source = (ROOT / OWNER).read_text(encoding="utf-8") + mutated = source.replace(" if read_only:\n", " if False and read_only:\n", 1) + assert mutated != source + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={OWNER: mutated}, + ) + + assert report.failures == () + assert any(violation.rule_id == RULE_ID for violation in report.violations) + + +def test_lockfile_read_rule_rejects_installed_path_fallback_duplication() -> None: + """The installed-path reader cannot bypass and re-derive the owner.""" + source = (ROOT / OWNER).read_text(encoding="utf-8") + delegated = "lockfile_path = resolve_lockfile_path_for_read(project_root, read_only=True)" + duplicated = """lockfile_path = get_lockfile_path(project_root) + if not lockfile_path.exists(): + legacy_path = project_root / LEGACY_LOCKFILE_NAME + if legacy_path.exists(): + lockfile_path = legacy_path""" + mutated = source.replace(delegated, duplicated, 1) + assert mutated != source + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={OWNER: mutated}, + ) + + assert report.failures == () + assert any(violation.rule_id == RULE_ID for violation in report.violations) + + +def test_lockfile_read_rule_accepts_reformatted_consumer_call() -> None: + """AST routing survives a harmless multiline formatter change.""" + consumer = "src/apm_cli/bundle/packer.py" + source = (ROOT / consumer).read_text(encoding="utf-8") + one_line = "resolve_lockfile_path_for_read(project_root, read_only=dry_run)" + multiline = """resolve_lockfile_path_for_read( + project_root, + read_only=dry_run, + )""" + mutated = source.replace(one_line, multiline, 1) + assert mutated != source + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={consumer: mutated}, + ) + + assert report.failures == () + assert report.violations == () diff --git a/tests/integration/test_pack_unified.py b/tests/integration/test_pack_unified.py index a96b3b35a..a623ce9c8 100644 --- a/tests/integration/test_pack_unified.py +++ b/tests/integration/test_pack_unified.py @@ -625,6 +625,45 @@ def test_drift_recipe_preserves_marketplace_path_override(self, runner, tmp_path class TestCheckCleanEffectiveOutputPath: """Integration tests: --check-clean uses pack's effective output path.""" + def test_check_clean_preserves_legacy_lockfile_and_reports_read_only( + self, runner, tmp_path, monkeypatch + ): + """The read-only gate must preserve every existing project file.""" + monkeypatch.chdir(tmp_path) + _write_marketplace_block_yml(tmp_path) + manifest = tmp_path / "apm.yml" + manifest.write_text( + manifest.read_text(encoding="utf-8").replace( + "\nmarketplace:\n", "\ndependencies: {}\n\nmarketplace:\n" + ), + encoding="utf-8", + ) + _write_minimal_lockfile(tmp_path) + packed = runner.invoke(pack_cmd, ["--offline"]) + assert packed.exit_code == 0, packed.output + + canonical_lock = tmp_path / "apm.lock.yaml" + legacy_lock = tmp_path / "apm.lock" + canonical_lock.rename(legacy_lock) + before = { + path.relative_to(tmp_path).as_posix(): path.read_bytes() + for path in tmp_path.rglob("*") + if path.is_file() + } + + checked = runner.invoke(pack_cmd, ["--check-clean", "--offline", "--json"]) + + assert checked.exit_code == 0, checked.output + assert json.loads(checked.output)["dry_run"] is True + after = { + path.relative_to(tmp_path).as_posix(): path.read_bytes() + for path in tmp_path.rglob("*") + if path.is_file() + } + assert after == before + assert legacy_lock.is_file() + assert not canonical_lock.exists() + def test_check_clean_uses_marketplace_path_override(self, runner, tmp_path, monkeypatch): """The clean gate must compare the artifact selected by the CLI override.""" monkeypatch.chdir(tmp_path) diff --git a/tests/test_lockfile.py b/tests/test_lockfile.py index efa758243..032ed6bac 100644 --- a/tests/test_lockfile.py +++ b/tests/test_lockfile.py @@ -1,6 +1,6 @@ """Tests for the APM lock file module.""" -from pathlib import Path # noqa: F401 +from pathlib import Path from unittest.mock import Mock import pytest @@ -11,6 +11,7 @@ LockFile, get_lockfile_path, migrate_lockfile_if_needed, + resolve_lockfile_path_for_read, ) from apm_cli.models.apm_package import DependencyReference @@ -411,6 +412,46 @@ def test_get_lockfile_path(self, tmp_path): assert path == tmp_path / "apm.lock.yaml" +class TestResolveLockfilePathForRead: + def test_read_only_returns_legacy_path_without_migrating(self, tmp_path: Path) -> None: + legacy = tmp_path / "apm.lock" + legacy.write_text("legacy", encoding="utf-8") + + path = resolve_lockfile_path_for_read(tmp_path, read_only=True) + + assert path == legacy + assert legacy.read_text(encoding="utf-8") == "legacy" + assert not (tmp_path / "apm.lock.yaml").exists() + + def test_read_only_prefers_canonical_path_when_both_exist(self, tmp_path: Path) -> None: + canonical = tmp_path / "apm.lock.yaml" + canonical.write_text("canonical", encoding="utf-8") + legacy = tmp_path / "apm.lock" + legacy.write_text("legacy", encoding="utf-8") + + path = resolve_lockfile_path_for_read(tmp_path, read_only=True) + + assert path == canonical + assert canonical.read_text(encoding="utf-8") == "canonical" + assert legacy.read_text(encoding="utf-8") == "legacy" + + def test_read_only_returns_canonical_path_when_neither_exists(self, tmp_path: Path) -> None: + path = resolve_lockfile_path_for_read(tmp_path, read_only=True) + + assert path == tmp_path / "apm.lock.yaml" + assert list(tmp_path.iterdir()) == [] + + def test_writable_read_migrates_legacy_path(self, tmp_path: Path) -> None: + legacy = tmp_path / "apm.lock" + legacy.write_text("legacy", encoding="utf-8") + + path = resolve_lockfile_path_for_read(tmp_path, read_only=False) + + assert path == tmp_path / "apm.lock.yaml" + assert path.read_text(encoding="utf-8") == "legacy" + assert not legacy.exists() + + class TestMigrateLockfileIfNeeded: def test_migrates_legacy_lockfile(self, tmp_path): """apm.lock should be renamed to apm.lock.yaml when new file is absent.""" diff --git a/tests/unit/commands/test_pack_cli_flags.py b/tests/unit/commands/test_pack_cli_flags.py index a6ada3e92..2af43a3f0 100644 --- a/tests/unit/commands/test_pack_cli_flags.py +++ b/tests/unit/commands/test_pack_cli_flags.py @@ -114,6 +114,8 @@ def test_removed_flag_is_unknown_option(self) -> None: version: 1.0.0 """ +_APM_ALIGNED_WITH_BUNDLE = _APM_ALIGNED + "dependencies: {}\n" + _APM_MISALIGNED = """\ name: my-project description: A project. @@ -219,11 +221,16 @@ def test_flag_recognized(self) -> None: def test_skip_when_no_marketplace_block(self, tmp_path: _Path, monkeypatch) -> None: (tmp_path / "apm.yml").write_text( - "name: x\ndescription: y\nversion: 1.0.0\n", encoding="utf-8" + "name: x\ndescription: y\nversion: 1.0.0\ndependencies: {}\n", + encoding="utf-8", ) monkeypatch.chdir(tmp_path) - result = CliRunner().invoke(pack_cmd, ["--check-clean", "--dry-run"]) + result = CliRunner().invoke(pack_cmd, ["--check-clean"]) assert result.exit_code != 4 + assert "Marketplace drift check skipped" in result.output + assert ( + "[dry-run] --check-clean is read-only; no pack outputs were written." in result.output + ) def test_fails_when_on_disk_missing(self, tmp_path: _Path, monkeypatch) -> None: _write_project(tmp_path, _APM_ALIGNED) @@ -232,6 +239,64 @@ def test_fails_when_on_disk_missing(self, tmp_path: _Path, monkeypatch) -> None: # No marketplace.json on disk -> "missing" -> exit 4. assert result.exit_code == 4 + def test_detects_drift_without_mutating_existing_output( + self, tmp_path: _Path, monkeypatch + ) -> None: + _write_project(tmp_path, _APM_ALIGNED) + monkeypatch.chdir(tmp_path) + initial_pack = CliRunner().invoke(pack_cmd, ["--offline"]) + assert initial_pack.exit_code == 0, initial_pack.output + output = tmp_path / ".claude-plugin" / "marketplace.json" + initial_bytes = output.read_bytes() + + manifest = tmp_path / "apm.yml" + manifest.write_text( + manifest.read_text(encoding="utf-8").replace( + " version: 1.0.0", " version: 1.0.1" + ), + encoding="utf-8", + ) + + result = CliRunner().invoke(pack_cmd, ["--check-clean", "--offline"]) + + assert result.exit_code == 4, result.output + assert output.read_bytes() == initial_bytes + assert "[dry-run] Would write" not in result.output + + def test_reports_suppressed_bundle_output_as_read_only( + self, tmp_path: _Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + _write_project(tmp_path, _APM_ALIGNED_WITH_BUNDLE) + monkeypatch.chdir(tmp_path) + initial_pack = CliRunner().invoke(pack_cmd, ["--offline"]) + assert initial_pack.exit_code == 0, initial_pack.output + + result = CliRunner().invoke(pack_cmd, ["--check-clean", "--offline"]) + + assert result.exit_code == 0, result.output + assert ( + "[dry-run] --check-clean is read-only; no pack outputs were written." in result.output + ) + assert "Packed" not in result.output + + def test_explicit_dry_run_keeps_full_bundle_and_marketplace_preview( + self, tmp_path: _Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + _write_project(tmp_path, _APM_ALIGNED_WITH_BUNDLE) + monkeypatch.chdir(tmp_path) + initial_pack = CliRunner().invoke(pack_cmd, ["--offline"]) + assert initial_pack.exit_code == 0, initial_pack.output + + result = CliRunner().invoke( + pack_cmd, + ["--check-clean", "--dry-run", "--offline"], + ) + + assert result.exit_code == 0, result.output + assert "[dry-run] Would pack" in result.output + assert "[dry-run] Would write marketplace.json" in result.output + assert "[dry-run] --check-clean is read-only" not in result.output + def test_json_envelope_carries_drift(self, tmp_path: _Path, monkeypatch) -> None: _write_project(tmp_path, _APM_ALIGNED) monkeypatch.chdir(tmp_path) diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index a8fba7d66..b65e080b6 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -605,6 +605,7 @@ def exiting_import( contracts-tooling-dependency-identity contracts-tooling-frontmatter-yaml contracts-tooling-generation-footer +contracts-tooling-lockfile-read install-deployment-approval-outcome-routing install-deployment-audit-policy-discovery install-deployment-audit-replay diff --git a/tests/unit/test_agent_plugin_exporter.py b/tests/unit/test_agent_plugin_exporter.py index 85f4d963a..eca3a65dc 100644 --- a/tests/unit/test_agent_plugin_exporter.py +++ b/tests/unit/test_agent_plugin_exporter.py @@ -384,6 +384,20 @@ def test_agent_bundle_dry_run_does_not_claim_default_flip_before_t10( assert not any("defaults to Agent Plugin output" in warning for warning in result.warnings) +def test_agent_bundle_dry_run_reads_legacy_lockfile_without_migration(tmp_path: Path) -> None: + project = _write_agent_project(tmp_path / "project") + canonical = project / "apm.lock.yaml" + legacy = project / "apm.lock" + canonical.rename(legacy) + build = tmp_path / "build" + + export_agent_plugin_bundle(project, build, dry_run=True) + + assert legacy.is_file() + assert not canonical.exists() + assert not build.exists() + + def test_agent_bundle_dry_run_rejects_nonportable_components(tmp_path: Path) -> None: project = _write_agent_project(tmp_path / "project") _add_nonportable_components(project, include_lsp=False) diff --git a/tests/unit/test_packer.py b/tests/unit/test_packer.py index f643781ea..caf832821 100644 --- a/tests/unit/test_packer.py +++ b/tests/unit/test_packer.py @@ -203,6 +203,22 @@ def test_pack_apm_format_all(self, tmp_path): assert set(result.files) == set(deployed) + def test_apm_format_dry_run_reads_legacy_lockfile_without_migration( + self, tmp_path: Path + ) -> None: + project = _setup_project(tmp_path, [".github/agents/a.md"], target="vscode") + canonical = project / "apm.lock.yaml" + legacy = project / "apm.lock" + canonical.rename(legacy) + initial_bytes = legacy.read_bytes() + out = tmp_path / "build" + + pack_bundle(project, out, fmt="apm", dry_run=True) + + assert legacy.read_bytes() == initial_bytes + assert not canonical.exists() + assert not out.exists() + def test_pack_archive(self, tmp_path): deployed = [".github/agents/a.md"] project = _setup_project(tmp_path, deployed, target="vscode") diff --git a/tests/unit/test_plugin_exporter.py b/tests/unit/test_plugin_exporter.py index c3cbcffab..c46e1c35e 100644 --- a/tests/unit/test_plugin_exporter.py +++ b/tests/unit/test_plugin_exporter.py @@ -793,6 +793,20 @@ def test_dry_run_no_output(self, tmp_path): assert len(result.files) > 0 assert "plugin.json" in result.files + def test_dry_run_reads_legacy_lockfile_without_migration(self, tmp_path: Path) -> None: + project = _setup_plugin_project(tmp_path, agents=["a.agent.md"]) + canonical = project / "apm.lock.yaml" + legacy = project / "apm.lock" + canonical.rename(legacy) + initial_bytes = legacy.read_bytes() + out = tmp_path / "build" + + export_plugin_bundle(project, out, dry_run=True) + + assert legacy.read_bytes() == initial_bytes + assert not canonical.exists() + assert not out.exists() + def test_archive_dry_run_reports_projected_zip_path(self, tmp_path): project = _setup_plugin_project(tmp_path, agents=["a.agent.md"]) out = tmp_path / "build"