diff --git a/.apm/architecture/owners/contracts-tooling.json b/.apm/architecture/owners/contracts-tooling.json index 31ef286093..af718251b1 100644 --- a/.apm/architecture/owners/contracts-tooling.json +++ b/.apm/architecture/owners/contracts-tooling.json @@ -8,7 +8,10 @@ "selectors": [ "src/apm_cli/models/dependency/identity.py", "src/apm_cli/models/dependency/materialization.py", - "src/apm_cli/models/dependency/reference.py" + "src/apm_cli/models/dependency/reference.py", + "src/apm_cli/install/phases/download.py", + "src/apm_cli/install/phases/integrate.py", + "src/apm_cli/install/plan.py" ], "guards": ["contracts-tooling-dependency-identity", "contracts-tooling-policy-identity"] }, diff --git a/CHANGELOG.md b/CHANGELOG.md index fc6df1ff7f..cd157e47bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Global skill installs now deploy authorized files when `HOME` or `APM_HOME` uses a directory symlink alias, without changing environment settings or accepting symlinks inside packages. (by @DaveMeadAdjust, #2876) ### Added - gh-aw's shared APM import now supports `token-source: github-token`; after consumers re-vendor the workflow, its read-only current-repository identity can fetch same-repository private packages, while `cascade` remains the default and cross-repository packages still require a dedicated token or GitHub App. (#2706) diff --git a/CONFORMANCE.json b/CONFORMANCE.json index b2fbc74e4d..7aede305bd 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -86,8 +86,9 @@ "keyword": "MUST", "section": "5.2", "status": "active", - "test_count": 2, + "test_count": 3, "tests": [ + "tests/spec_conformance/test_lockfile_reqs.py::test_frozen_manifest_pin_requires_the_exact_locked_commit", "tests/spec_conformance/test_lockfile_reqs.py::test_full_sha_pin_audit_rejects_resolved_commit_mismatch", "tests/spec_conformance/test_lockfile_reqs.py::test_lockfile_carries_dependencies_block" ] diff --git a/CONFORMANCE.md b/CONFORMANCE.md index a6c0ce79fd..d301bd497e 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -43,7 +43,7 @@ Repository-coordinate segments are case-insensitive for `github.com`, GitHub Ent | [req-ext-002](docs/src/content/docs/specs/openapm-v0.1.md#req-ext-002) | MUST | 4.1 | producer | active | 1 | - | | [req-lk-001](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-001) | MUST | 5.1 | consumer | active | 1 | - | | [req-lk-002](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-002) | MUST | 5.4 | consumer | active | 1 | - | -| [req-lk-003](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-003) | MUST | 5.2 | consumer | active | 2 | - | +| [req-lk-003](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-003) | MUST | 5.2 | consumer | active | 3 | - | | [req-lk-004](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-004) | MUST | 5.4 | consumer | active | 1 | - | | [req-lk-005](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-005) | MUST | 5.5 | consumer | active | 2 | - | | [req-lk-006](docs/src/content/docs/specs/openapm-v0.1.md#req-lk-006) | MUST | 5.5 | consumer | active | 1 | - | diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index f691690a6e..87d21aded6 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -30,7 +30,7 @@ With no arguments it installs everything from `apm.yml`. With one or more `PACKA | Flag | Default | Description | |---|---|---| | `--update` | off | Re-resolve dependencies to the latest version or Git ref allowed by `apm.yml` and rewrite `apm.lock.yaml`. Mutable Git refs must resolve against upstream; APM does not fall back to stale refs from the local bare Git cache. Mutually exclusive with `--frozen`. For interactive use with a confirmation prompt, use [`apm update`](../update/) instead. | -| `--frozen` | off | Lockfile-only install: refuse to resolve anything new and fail before any project, config, deployment, or cache write if `apm.lock.yaml` is missing or out of sync with `apm.yml`, including MCP state. Mirrors `npm ci`. Mutually exclusive with `--update`, positional package additions, and `--mcp`. | +| `--frozen` | off | Lockfile-only install: refuse to resolve anything new and fail before any project, config, deployment, or cache write if `apm.lock.yaml` is missing or no longer matches the manifest-declared refs/pins in `apm.yml`, including MCP state. This is a structural lockfile check, not a network freshness check. Mirrors `npm ci`. Mutually exclusive with `--update`, positional package additions, and `--mcp`. | | `--dry-run` | off | Print the install plan without deployment writes. Positional packages and ref changes appear in the preview after validation but do not change an existing `apm.yml`. Project auto-bootstrap still keeps its new manifest and any explicit `--target` selection for the next run; global dry-run bootstrap uses temporary preview state and does not create `~/.apm`. The `-g --mcp` path creates no user manifest, lockfile, or runtime configuration. | | `--force` | off | Overwrite locally-authored files on collision **and** bypass the security scan's critical-finding block. Does **not** suppress general install errors (any reported error still exits `1`, matching npm / pip / cargo) or select ref freshness. Add `--update` or `--refresh` to resolve mutable refs upstream; [`apm update`](../update/) does so with or without `--force`. Use only after independent verification. | | `--verbose`, `-v` | off | Show per-file paths and full error context in the diagnostic summary. | @@ -147,11 +147,12 @@ in `apm.yml`, then run `apm install` again. installation. See [Author primitives](../../../producer/author-primitives/) for fence and UTF-8 BOM syntax. - **MCP-only lock state.** A normal project install creates or updates `apm.lock.yaml` when `apm.yml` declares only MCP dependencies, records the resolved MCP configs and targets, and migrates a legacy `apm.lock` first. Repeating the same install leaves the lockfile and target configs byte-identical. If initial lock creation fails, install exits nonzero and warns with writable-directory and rerun guidance. -- **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. +- **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. When no lockfile exists, a mutable branch install resolves current upstream on first materialization instead of reusing a stale bare-cache ref. 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. - **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 skill paths.** Global skill installation resolves the user-scope lockfile from the canonical `~/.apm/` root, even when `HOME` or `APM_HOME` points at a symlink alias. Root aliases work without extra flags or environment changes; symlinks inside the package remain rejected. - **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 `~/.config/opencode/skills/`. Run `apm compile -g` to refresh diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 1ec5ae5235..20ccccd617 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -11,7 +11,7 @@ | 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; dependency aliases materialize at the canonical install path, not duplicate noncanonical paths; plain/frozen installs replay locked refs and cache state, and a mutable branch install with no lockfile resolves current upstream instead of a stale bare-cache ref; 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 no longer matches the manifest-declared refs/pins in `apm.yml`, including MCP config state (structural lockfile check, not network freshness; 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. | diff --git a/scripts/architecture_linter/checks/contracts_test_taxonomy.py b/scripts/architecture_linter/checks/contracts_test_taxonomy.py index 785c8a4064..c98955dd4a 100644 --- a/scripts/architecture_linter/checks/contracts_test_taxonomy.py +++ b/scripts/architecture_linter/checks/contracts_test_taxonomy.py @@ -457,6 +457,15 @@ def check_taxonomy_classification(provider: FactsProvider) -> tuple[Violation, . _RESOLVE_PHASE = "src/apm_cli/install/phases/resolve.py" +_DOWNLOAD_PHASE = "src/apm_cli/install/phases/download.py" + + +_INTEGRATE_PHASE = "src/apm_cli/install/phases/integrate.py" + + +_INSTALL_PLAN = "src/apm_cli/install/plan.py" + + def check_dependency_identity(provider: FactsProvider) -> tuple[Violation, ...]: """Guard dependency identity, materialization, and embedded-subpath ownership.""" rule_id = _GUARD_DEPENDENCY_IDENTITY @@ -464,7 +473,18 @@ def check_dependency_identity(provider: FactsProvider) -> tuple[Violation, ...]: materialization, mat_fail = _facts_for(provider, _MATERIALIZATION_OWNER, rule_id) reference, ref_fail = _facts_for(provider, _REFERENCE_OWNER, rule_id) resolve, resolve_fail = _facts_for(provider, _RESOLVE_PHASE, rule_id) - failures = list(identity_fail) + list(mat_fail) + list(ref_fail) + list(resolve_fail) + download, download_fail = _facts_for(provider, _DOWNLOAD_PHASE, rule_id) + integrate, integrate_fail = _facts_for(provider, _INTEGRATE_PHASE, rule_id) + plan, plan_fail = _facts_for(provider, _INSTALL_PLAN, rule_id) + failures = ( + list(identity_fail) + + list(mat_fail) + + list(ref_fail) + + list(resolve_fail) + + list(download_fail) + + list(integrate_fail) + + list(plan_fail) + ) if failures: return tuple(failures) @@ -528,6 +548,33 @@ def check_dependency_identity(provider: FactsProvider) -> tuple[Violation, ...]: embedded_subpath_message, ) ) + install_path_message = "Install phase materialization paths must route through DependencyReference.get_install_path" + download_body = _awk_body(download, re.compile(r"^def run\("), re.compile(r"^def ")) + if not _body_has(download_body, "_pd_path = _pd_ref.get_install_path(apm_modules_dir)") or ( + _body_has(download_body, "apm_modules_dir / _pd_ref.alias") + or _body_has(download_body, "if _pd_ref.alias") + ): + findings.append(_summary(rule_id, _DOWNLOAD_PHASE, install_path_message)) + + integrate_body = _awk_body(integrate, re.compile(r"^def run\("), re.compile(r"^def ")) + if not _body_has( + integrate_body, "install_path = dep_ref.get_install_path(apm_modules_dir)" + ) or ( + _body_has(integrate_body, "apm_modules_dir / dep_ref.alias") + or _body_has(integrate_body, "if dep_ref.alias") + ): + findings.append(_summary(rule_id, _INTEGRATE_PHASE, install_path_message)) + + frozen_identity_message = "Frozen manifest drift checks must route through full-SHA comparison and drift.detect_ref_change" + plan_body = _awk_body( + plan, re.compile(r"^def lockfile_satisfies_manifest\("), re.compile(r"^def ") + ) + if ( + not _body_has(plan_body, "is_full_revision_pin(reference)") + or not _body_has(plan_body, "detect_ref_change(dep, locked_dep)") + or not _body_has(plan_body, "locked_dep.resolved_commit") + ): + findings.append(_summary(rule_id, _INSTALL_PLAN, frozen_identity_message)) primitive_dirs = re.compile(r"_APM_PRIMITIVE_DIRS") for path in _python_paths(provider, _SRC_PREFIX): if path == _REFERENCE_OWNER: diff --git a/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py b/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py index 59e0936ebe..169a80c5bf 100644 --- a/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py +++ b/scripts/architecture_linter/checks/install_base_integrator_and_contraction.py @@ -283,6 +283,9 @@ def check_target_file_contraction(provider: FactsProvider) -> tuple[Violation, . } +_SKILL_INTEGRATOR = "src/apm_cli/integration/skill_integrator.py" + + _OWNED_STATE_FIELDS = frozenset( { "deployed_files", @@ -417,6 +420,35 @@ def report(node: ast.AST, message: str) -> None: return findings +def _skill_lockfile_root_findings(provider: FactsProvider, rule_id: str) -> list[Violation]: + """User-scope skill ownership must read provenance from the canonical lockfile root.""" + facts, fail = _facts_for(provider, _SKILL_INTEGRATOR, rule_id) + if fail: + return list(fail) + + route_message = "user-scope skill ownership must route lockfile provenance through get_apm_dir(InstallScope.USER)" + forward_message = "skill ownership consumers must forward the canonical lockfile_root through every ownership-map integration path" + required_import = "from apm_cli.core.scope import InstallScope, get_apm_dir" + required_route = ( + "get_apm_dir(InstallScope.USER) if scope is InstallScope.USER else project_root" + ) + required_forward_count = 4 + findings: list[Violation] = [] + if not _present(facts, required_import) or not _present(facts, required_route): + findings.append(_summary(rule_id, _SKILL_INTEGRATOR, route_message)) + if not _present(facts, "LockFile.read(get_lockfile_path(lockfile_root))"): + findings.append( + _summary( + rule_id, + _SKILL_INTEGRATOR, + "skill ownership maps must resolve deployment provenance from the selected lockfile_root", + ) + ) + if sum("lockfile_root=lockfile_root" in line for line in facts.lines) != required_forward_count: + findings.append(_summary(rule_id, _SKILL_INTEGRATOR, forward_message)) + return findings + + def _owned_state_attr(node: ast.AST) -> bool: """Return whether one AST node references codec-owned deployment state.""" return isinstance(node, ast.Attribute) and node.attr in _OWNED_STATE_FIELDS @@ -727,6 +759,7 @@ def check_provenance_state(provider: FactsProvider) -> tuple[Violation, ...]: findings: list[Violation] = [] for path in _REQUIRED_OWNER_CALLS: findings.extend(_deployment_owner_findings(provider, path, rule_id)) + findings.extend(_skill_lockfile_root_findings(provider, rule_id)) findings.extend(_legacy_scope_findings(provider, rule_id)) findings.extend(_state_mutation_findings(provider, rule_id)) findings.extend(_local_bundle_findings(provider, rule_id)) diff --git a/src/apm_cli/deps/tiered_ref_resolver.py b/src/apm_cli/deps/tiered_ref_resolver.py index 35dd8df23c..756381eaa0 100644 --- a/src/apm_cli/deps/tiered_ref_resolver.py +++ b/src/apm_cli/deps/tiered_ref_resolver.py @@ -61,6 +61,7 @@ from ..cache.git_cache import GitCache from ..deps.git_reference_resolver import GitReferenceResolver from ..deps.github_downloader import GitHubPackageDownloader + from ..deps.lockfile import LockFile _log = logging.getLogger(__name__) @@ -107,6 +108,7 @@ class RefFreshnessContext(Protocol): ref_freshness_policy: RefFreshnessPolicy | None update_refs: bool refresh: bool + existing_lockfile: LockFile | None def ref_freshness_policy_for_install( @@ -118,6 +120,8 @@ def ref_freshness_policy_for_install( if not isinstance(configured, RefFreshnessPolicy): raise TypeError("ref_freshness_policy must be a RefFreshnessPolicy") return configured + if context.existing_lockfile is None: + return RefFreshnessPolicy.CURRENT_REMOTE return RefFreshnessPolicy.for_install_intent( update_refs=context.update_refs, refresh=context.refresh, diff --git a/src/apm_cli/install/phases/download.py b/src/apm_cli/install/phases/download.py index 5a171520fb..f58c1a00a6 100644 --- a/src/apm_cli/install/phases/download.py +++ b/src/apm_cli/install/phases/download.py @@ -54,11 +54,7 @@ def run(ctx: InstallContext) -> None: _need_download = [] for _pd_ref in deps_to_install: _pd_key = _pd_ref.get_unique_key() - _pd_path = ( - (apm_modules_dir / _pd_ref.alias) - if _pd_ref.alias - else _pd_ref.get_install_path(apm_modules_dir) - ) + _pd_path = _pd_ref.get_install_path(apm_modules_dir) # Skip local packages -- they are copied, not downloaded if _pd_ref.is_local: continue diff --git a/src/apm_cli/install/phases/integrate.py b/src/apm_cli/install/phases/integrate.py index b9632ef81c..b23ee2c7a5 100644 --- a/src/apm_cli/install/phases/integrate.py +++ b/src/apm_cli/install/phases/integrate.py @@ -614,12 +614,7 @@ def run(ctx: InstallContext) -> None: # e.g., microsoft/apm-sample-package -> apm_modules/microsoft/apm-sample-package/ # For virtual packages: owner/repo/prompts/file.prompt.md -> apm_modules/owner/repo-file/ # For subdirectory packages: owner/repo/subdir -> apm_modules/owner/repo/subdir/ - if dep_ref.alias: - # If alias is provided, use it directly (assume user handles namespacing) - install_path = apm_modules_dir / dep_ref.alias - else: - # Use the canonical install path from DependencyReference - install_path = dep_ref.get_install_path(apm_modules_dir) + install_path = dep_ref.get_install_path(apm_modules_dir) # Skip deps that already failed during BFS resolution callback # to avoid a duplicate error entry in diagnostics. diff --git a/src/apm_cli/install/plan.py b/src/apm_cli/install/plan.py index aa72727a9d..f692328b2e 100644 --- a/src/apm_cli/install/plan.py +++ b/src/apm_cli/install/plan.py @@ -457,9 +457,10 @@ def lockfile_satisfies_manifest( """Structural satisfaction check for ``apm install --frozen``. Verifies that every direct dependency declared in the manifest has - a corresponding entry in the lockfile. Does NOT perform any - resolution or compare resolved refs against the remote -- those are - ``apm update``'s job. + a corresponding entry in the lockfile and that the manifest still + points at the same locked identity. Does NOT perform any remote + resolution; it only compares manifest-declared refs against the + already-recorded lockfile state. Args: lockfile: The on-disk lockfile. @@ -473,6 +474,8 @@ def lockfile_satisfies_manifest( ``satisfied`` is True. """ from apm_cli.deps.lockfile import _SELF_KEY + from apm_cli.deps.revision_pins import is_full_revision_pin + from apm_cli.drift import detect_ref_change locked_keys = {key for key in lockfile.dependencies if key != _SELF_KEY} @@ -483,6 +486,23 @@ def lockfile_satisfies_manifest( key = _dep_ref_key(dep) if key not in locked_keys: reasons.append(f" - {key} is declared in apm.yml but missing from apm.lock.yaml") + continue + locked_dep = lockfile.dependencies[key] + reference = dep.reference + if ( + reference + and is_full_revision_pin(reference) + and (locked_dep.resolved_commit or "").lower() != reference.lower() + ): + reasons.append( + f" - {key}: manifest commit '{dep.reference}' != " + f"lockfile resolved_commit '{locked_dep.resolved_commit or '(missing)'}'" + ) + continue + if detect_ref_change(dep, locked_dep): + reasons.append( + f" - {key}: declared source, ref, or transport differs from apm.lock.yaml" + ) return (not reasons, reasons) diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index 7827adb388..1e621a5910 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/src/apm_cli/integration/skill_integrator.py @@ -7,6 +7,7 @@ import shutil import stat from collections.abc import Callable, Iterable +from copy import copy from dataclasses import replace from pathlib import Path from typing import TYPE_CHECKING @@ -863,28 +864,18 @@ def _promote_sub_skills( return promoted, deployed @staticmethod - def _build_ownership_maps(project_root: Path) -> tuple[dict[str, str], dict[str, str]]: + def _build_ownership_maps(lockfile_root: Path) -> tuple[dict[str, str], dict[str, str]]: """Read the lockfile once and build two ownership maps. - Returns a tuple of: - - owned_by: skill_name -> dep.get_unique_key(), for sub-skill self-overwrite detection. - - native_owners: skill_name -> dep.get_unique_key(), for native-skill cross-package - collision detection. Only paths under a ``/skills/`` prefix are included to avoid - false attribution from non-skill deployed_files entries (prompts, hooks, commands, etc.). - - Both maps key on the full unique dependency identity (owner/repo, or the - equivalent durable key for local/registry deps), NOT the last path - segment. Two different packages can share a repo/leaf name (e.g. two - orgs each publishing a "shared-skill" or "utils" repo); comparing only - the last segment would treat them as the same owner and silently - suppress the cross-package collision warning precisely when it matters - most -- an unrelated package overwriting another's skill undetected. + Both maps associate skill names with full dependency identities, never + just repository leaf names. The native-owner map includes only skill + paths, avoiding false attribution from prompts, hooks, or commands. """ from apm_cli.deps.lockfile import LockFile, get_lockfile_path owned_by: dict[str, str] = {} native_owners: dict[str, str] = {} - lockfile = LockFile.read(get_lockfile_path(project_root)) + lockfile = LockFile.read(get_lockfile_path(lockfile_root)) if not lockfile: return owned_by, native_owners for dep in lockfile.get_package_dependencies(): @@ -901,28 +892,23 @@ def _build_ownership_maps(project_root: Path) -> tuple[dict[str, str], dict[str, return owned_by, native_owners @staticmethod - def _build_skill_ownership_map(project_root: Path) -> dict[str, str]: - """Build a map of skill_name -> owner_package_name from the lockfile. - - Used to distinguish self-overwrites (no warning) from cross-package - conflicts (warning) when promoting sub-skills. - """ - owned_by, _ = SkillIntegrator._build_ownership_maps(project_root) + def _build_skill_ownership_map(lockfile_root: Path) -> dict[str, str]: + """Map skill names to durable owners for sub-skill collision detection.""" + owned_by, _ = SkillIntegrator._build_ownership_maps(lockfile_root) return owned_by @staticmethod - def _build_native_skill_owner_map(project_root: Path) -> dict[str, str]: - """Build a map of skill_name -> dep.get_unique_key() from the lockfile. - - Scoped to ``/skills/`` paths only -- see ``_build_ownership_maps`` for details. - """ - _, native_owners = SkillIntegrator._build_ownership_maps(project_root) + def _build_native_skill_owner_map(lockfile_root: Path) -> dict[str, str]: + """Map native skill names to durable owners, excluding non-skill paths.""" + _, native_owners = SkillIntegrator._build_ownership_maps(lockfile_root) return native_owners def _promote_sub_skills_standalone( self, package_info, project_root: Path, + *, + lockfile_root: Path | None = None, diagnostics=None, managed_files=None, force: bool = False, @@ -964,7 +950,7 @@ def _promote_sub_skills_standalone( # _build_ownership_maps). _dep_ref = getattr(package_info, "dependency_ref", None) parent_name = _dep_ref.get_unique_key() if _dep_ref is not None else package_path.name - owned_by = self._build_skill_ownership_map(project_root) + owned_by = self._build_skill_ownership_map(lockfile_root or project_root) name_filter = ( source_plan.selected_skill_names if source_plan is not None @@ -1033,6 +1019,8 @@ def _integrate_native_skill( package_info, project_root: Path, source_skill_md: Path, + *, + lockfile_root: Path | None = None, diagnostics=None, managed_files=None, force: bool = False, @@ -1115,7 +1103,7 @@ def _integrate_native_skill( primary_skill_md: Path | None = None # Read lockfile once and derive both maps in a single pass. - owned_by, lockfile_native_owners = self._build_ownership_maps(project_root) + owned_by, lockfile_native_owners = self._build_ownership_maps(lockfile_root or project_root) sub_skills_dir = package_path / ".apm" / "skills" # Full unique key of the package currently being installed. @@ -1274,6 +1262,8 @@ def _integrate_skill_bundle( package_info, project_root: Path, skills_dir: Path, + *, + lockfile_root: Path | None = None, diagnostics=None, managed_files=None, force: bool = False, @@ -1320,7 +1310,7 @@ def _integrate_skill_bundle( parent_name = ( _dep_ref.get_unique_key() if _dep_ref is not None else package_info.install_path.name ) - owned_by, lockfile_native_owners = self._build_ownership_maps(project_root) # noqa: RUF059 + owned_by, _ = self._build_ownership_maps(lockfile_root or project_root) total_promoted = 0 all_deployed: list[Path] = [] @@ -1458,16 +1448,22 @@ def integrate_package_skill( SkillIntegrationResult: Results of the integration operation """ enforce_agent_plugin_deployment_boundary(package_info) + from apm_cli.core.scope import InstallScope, get_apm_dir + + lockfile_root = ( + get_apm_dir(InstallScope.USER) if scope is InstallScope.USER else project_root + ) + + # Canonicalize only the root; preserve descendant links and caller metadata. + package_info = copy(package_info) + package_info.install_path = package_info.install_path.resolve() - # Check if package type allows skill installation (T4 routing) - # SKILL and HYBRID -> install as skill - # INSTRUCTIONS and PROMPTS -> skip skill installation if not should_install_skill(package_info): - # Even non-skill packages may ship sub-skills under .apm/skills/. - # Promote them so Copilot can discover them independently. + # Non-skill packages may still ship sub-skills under .apm/skills/. sub_skills_count, sub_deployed = self._promote_sub_skills_standalone( package_info, project_root, + lockfile_root=lockfile_root, diagnostics=diagnostics, managed_files=managed_files, force=force, @@ -1545,6 +1541,7 @@ def integrate_package_skill( package_info, project_root, source_skill_md, + lockfile_root=lockfile_root, diagnostics=diagnostics, managed_files=managed_files, force=force, @@ -1599,6 +1596,7 @@ def integrate_package_skill( package_info, project_root, package_path / ".apm" / "skills" if _is_plugin else root_skills_dir, + lockfile_root=lockfile_root, source_paths=_source_paths, diagnostics=diagnostics, managed_files=managed_files, @@ -1618,6 +1616,7 @@ def integrate_package_skill( sub_skills_count, sub_deployed = self._promote_sub_skills_standalone( package_info, project_root, + lockfile_root=lockfile_root, diagnostics=diagnostics, managed_files=managed_files, force=force, diff --git a/tests/integration/test_architecture_dependency_reference.py b/tests/integration/test_architecture_dependency_reference.py index e472905931..83ab2b38d1 100644 --- a/tests/integration/test_architecture_dependency_reference.py +++ b/tests/integration/test_architecture_dependency_reference.py @@ -11,6 +11,9 @@ ROOT = Path(__file__).parents[2] RULE_ID = "contracts-tooling-dependency-identity" REFERENCE = "src/apm_cli/models/dependency/reference.py" +DOWNLOAD = "src/apm_cli/install/phases/download.py" +INTEGRATE = "src/apm_cli/install/phases/integrate.py" +PLAN = "src/apm_cli/install/plan.py" def test_embedded_git_url_subpath_has_one_provider_aware_owner() -> None: @@ -20,12 +23,18 @@ def test_embedded_git_url_subpath_has_one_provider_aware_owner() -> None: encoding="utf-8" ) rule = next(rule for rule in registered_rules() if rule.id == RULE_ID) + report = run_selected_rules(ROOT, (RULE_ID,)) assert reference.count("def _check_no_embedded_subpath(") == 1 assert "classify_host_provider(host, host_type=host_type)" in reference assert 'provider.kind == "gitlab"' in reference + assert DOWNLOAD in owner_registry + assert INTEGRATE in owner_registry + assert PLAN in owner_registry assert "embedded git URL subpath validation" in owner_registry assert rule.guard_ids == (RULE_ID,) + assert report.failures == () + assert report.violations == () def test_embedded_git_url_subpath_guard_rejects_provider_bypass() -> None: @@ -44,3 +53,67 @@ def test_embedded_git_url_subpath_guard_rejects_provider_bypass() -> None: violation.rule_id == RULE_ID and "Embedded git URL subpath validation" in violation.message for violation in report.violations ) + + +@pytest.mark.parametrize( + ("path", "old", "new"), + [ + ( + DOWNLOAD, + "_pd_path = _pd_ref.get_install_path(apm_modules_dir)", + "_pd_path = (apm_modules_dir / _pd_ref.alias) if _pd_ref.alias else _pd_ref.get_install_path(apm_modules_dir)", + ), + ( + INTEGRATE, + "install_path = dep_ref.get_install_path(apm_modules_dir)", + "install_path = (apm_modules_dir / dep_ref.alias) if dep_ref.alias else dep_ref.get_install_path(apm_modules_dir)", + ), + ], +) +def test_install_phase_materialization_guard_rejects_alias_path_bypass( + path: str, old: str, new: str +) -> None: + """Download/integrate must not recompute materialization paths from alias branches.""" + source = (ROOT / path).read_text(encoding="utf-8") + mutated = source.replace(old, new, 1) + assert mutated != source + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={path: mutated}, + ) + + assert any( + violation.rule_id == RULE_ID + and "Install phase materialization paths must route through DependencyReference.get_install_path" + in violation.message + for violation in report.violations + ) + + +@pytest.mark.parametrize( + ("old", "new"), + [ + ("is_full_revision_pin(reference)", "False"), + ("detect_ref_change(dep, locked_dep)", "False"), + ], +) +def test_frozen_manifest_identity_guard_rejects_helper_bypass(old: str, new: str) -> None: + """Frozen plan drift checks must keep both the full-SHA and drift helper routes.""" + source = (ROOT / PLAN).read_text(encoding="utf-8") + mutated = source.replace(old, new, 1) + assert mutated != source + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={PLAN: mutated}, + ) + + assert any( + violation.rule_id == RULE_ID + and "Frozen manifest drift checks must route through full-SHA comparison and drift.detect_ref_change" + in violation.message + for violation in report.violations + ) diff --git a/tests/integration/test_architecture_skill_provenance_scope.py b/tests/integration/test_architecture_skill_provenance_scope.py new file mode 100644 index 0000000000..d826b9988e --- /dev/null +++ b/tests/integration/test_architecture_skill_provenance_scope.py @@ -0,0 +1,69 @@ +"""Architecture regression for user-scope skill provenance routing.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from scripts.architecture_linter.runner import registered_rules, run_selected_rules + +pytestmark = pytest.mark.component + +ROOT = Path(__file__).resolve().parents[2] +RULE_ID = "install-deployment-provenance-state" +SKILL_INTEGRATOR = "src/apm_cli/integration/skill_integrator.py" + + +def test_user_scope_skill_provenance_guard_is_registered_and_clean() -> None: + """The provenance guard includes skill_integrator's user-scope lockfile route.""" + rule = next(rule for rule in registered_rules() if rule.id == RULE_ID) + + assert rule.guard_ids == (RULE_ID,) + report = run_selected_rules(ROOT, (RULE_ID,)) + assert report.failures == () + assert report.violations == () + + +def test_user_scope_skill_provenance_guard_rejects_project_root_bypass() -> None: + """User-scope skill ownership must not read provenance from the project lockfile.""" + source = (ROOT / SKILL_INTEGRATOR).read_text(encoding="utf-8") + mutated = source.replace( + "get_apm_dir(InstallScope.USER) if scope is InstallScope.USER else project_root", + "project_root", + 1, + ) + assert mutated != source + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={SKILL_INTEGRATOR: mutated}, + ) + + assert any( + violation.rule_id == RULE_ID + and "user-scope skill ownership must route lockfile provenance through get_apm_dir(InstallScope.USER)" + in violation.message + for violation in report.violations + ) + + +def test_user_scope_skill_provenance_guard_rejects_missing_forwarding() -> None: + """Every ownership-map integration path must receive the selected lockfile_root.""" + source = (ROOT / SKILL_INTEGRATOR).read_text(encoding="utf-8") + mutated = source.replace("lockfile_root=lockfile_root,", "", 1) + assert mutated != source + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={SKILL_INTEGRATOR: mutated}, + ) + + assert any( + violation.rule_id == RULE_ID + and "skill ownership consumers must forward the canonical lockfile_root through every ownership-map integration path" + in violation.message + for violation in report.violations + ) diff --git a/tests/integration/test_frozen_host_qualified_git_e2e.py b/tests/integration/test_frozen_host_qualified_git_e2e.py index 80669519be..9f09842be5 100644 --- a/tests/integration/test_frozen_host_qualified_git_e2e.py +++ b/tests/integration/test_frozen_host_qualified_git_e2e.py @@ -53,6 +53,7 @@ def _cases() -> tuple[_FrozenGitCase, ...]: repo_url="org/private-skills", host="git.example.com", resolved_ref="2026.06.10", + constraint="2026.06.10", resolved_commit=_COMMIT, depth=1, name="private-skills", diff --git a/tests/integration/test_generated_lifecycle_state_machine.py b/tests/integration/test_generated_lifecycle_state_machine.py index 361f0ea354..adc7bb4a7a 100644 --- a/tests/integration/test_generated_lifecycle_state_machine.py +++ b/tests/integration/test_generated_lifecycle_state_machine.py @@ -9,35 +9,43 @@ import shutil import textwrap from dataclasses import dataclass -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest import yaml from hypothesis import HealthCheck, Phase, settings from hypothesis.stateful import ( RuleBasedStateMachine, + initialize, invariant, precondition, rule, run_state_machine_as_test, ) +from apm_cli.deps.lockfile import LockFile +from apm_cli.utils.yaml_io import dump_yaml, load_yaml from tests.integration.test_required_lifecycle_state_machine import ( _INSTALL_ARGS, - _audit, + _assert_same_state, + _audit_at, + _external_root_specs, _new_scenario, _publish, + _publish_revision, + _PublishedPackage, _result_evidence, - _run_success, _Scenario, _skill, ) +from tests.utils.apm_lifecycle_runner import CommandResult from tests.utils.artifact_snapshot import ( ArtifactSnapshotSet, assert_only_snapshot_paths_changed, assert_snapshot_changes_within, assert_snapshot_set_unchanged, ) +from tests.utils.lifecycle_state import LifecycleStateRoot, LifecycleStateSnapshot from tests.utils.local_package import LocalPackage pytestmark = [ @@ -52,6 +60,7 @@ _SKILL_NAME = "model-skill" _PACKAGE_NAME = "model-kit" _SKILL_BYTES = _skill(_SKILL_NAME).encode() +_TAMPER_BYTES = b"# user tamper\n" _PROJECT_WRITE_PATHS = frozenset( { ".agents", @@ -100,7 +109,22 @@ "remove_declaration": frozenset({"ownership.preserve_unowned"}), "repair": frozenset({"source.ref_cache_coherent"}), "tamper": frozenset({"outcome.status_matches_state"}), + "publish": frozenset({"source.ref_cache_coherent"}), + "outdated": frozenset({"source.ref_cache_coherent"}), + "update": frozenset({"source.ref_cache_coherent"}), + "compile": frozenset({"routing.authorized_targets_only"}), + "lock": frozenset({"ownership.preserve_unowned"}), + "frozen_replay": frozenset({"idempotency.byte_stable"}), + "frozen_refusal": frozenset({"transaction.failed_command_preserves_state"}), + "uninstall": frozenset({"ownership.preserve_unowned"}), + "audit_empty": frozenset({"outcome.status_matches_state"}), + "inspect_installed": frozenset({"filesystem.open_world_observation"}), + "export_lock": frozenset({"source.ref_cache_coherent"}), + "clean_sources": frozenset({"source.ref_cache_coherent", "ownership.preserve_unowned"}), + "clean_cache": frozenset({"filesystem.open_world_observation"}), + "legacy_update": frozenset({"source.ref_cache_coherent"}), } +_VARIANTS = ("project", "global-canonical", "global-aliased") def _phase_one_properties() -> frozenset[str]: @@ -117,11 +141,66 @@ class _ModelFixture: project: LocalPackage dependency: dict[str, object] environment: dict[str, str] + source: _PublishedPackage + variant: str + invoke_cwd: Path + audit_cwd: Path + skill_path: Path + external_roots: tuple[LifecycleStateRoot, ...] = () @classmethod - def create(cls, root: Path, apm_binary_path: Path) -> _ModelFixture: + def create(cls, root: Path, apm_binary_path: Path, variant: str = "project") -> _ModelFixture: scenario = _new_scenario(root, apm_binary_path) published = _publish(scenario, _PACKAGE_NAME, skill=_SKILL_NAME) + if variant != "project": + commit_a = _publish_revision(scenario, published, "a") + dependency = {**published.dependency, "ref": "main"} + environment = dict(published.environment) + target_root = scenario.isolated.root / "claude-home" + target_root.mkdir() + (target_root / "user-owned.txt").write_bytes(b"user-owned\n") + environment["CLAUDE_CONFIG_DIR"] = str(target_root) + if variant == "global-aliased": + alias = scenario.isolated.root / "home-alias" + alias.symlink_to(scenario.isolated.home, target_is_directory=True) + environment.update( + HOME=str(alias), USERPROFILE=str(alias), APM_HOME=str(alias / ".apm") + ) + state_root = scenario.isolated.config_root + state_root.mkdir(parents=True, exist_ok=True) + manifest_path = state_root / "apm.yml" + dump_yaml( + { + "name": "global-model-consumer", + "version": "0.1.0", + "dependencies": {"apm": [dependency]}, + "targets": ["claude"], + }, + manifest_path, + ) + source = _PublishedPackage( + published.package, + published.repository, + commit_a, + published.remote_url, + dependency, + environment, + ) + return cls( + scenario, + LocalPackage("global-model-consumer", state_root, manifest_path), + dependency, + environment, + source, + variant, + scenario.isolated.work_root, + state_root, + target_root / "skills" / _SKILL_NAME / "SKILL.md", + _external_root_specs( + {"claude": target_root}, + config_paths={"claude": (PurePosixPath("CLAUDE.md"),)}, + ), + ) project = scenario.consumers.create( "model-consumer", dependencies=(published.dependency,), @@ -132,8 +211,44 @@ def create(cls, root: Path, apm_binary_path: Path) -> _ModelFixture: project=project, dependency=published.dependency, environment=published.environment, + source=published, + variant=variant, + invoke_cwd=project.root, + audit_cwd=project.root, + skill_path=project.root / ".agents" / "skills" / _SKILL_NAME / "SKILL.md", ) + @property + def global_scope(self) -> bool: + return self.variant != "project" + + @property + def install_args(self) -> tuple[str, ...]: + return (*_INSTALL_ARGS, "--global") if self.global_scope else _INSTALL_ARGS + + @property + def exact_paths(self) -> dict[str, frozenset[str]]: + if not self.global_scope: + return _INSTALL_EXACT_PATHS + return { + "user": _USER_WRITE_PATHS, + "target": frozenset({"skills", "rules", "rules/revision.md", "CLAUDE.md"}), + } + + @property + def tree_prefixes(self) -> dict[str, frozenset[str]]: + if not self.global_scope: + return _INSTALL_TREE_PREFIXES + return { + "user": frozenset({".apm"}), + "target": frozenset({f"skills/{_SKILL_NAME}"}), + } + + def replace_dependencies(self, dependencies: tuple[dict[str, object], ...]) -> None: + manifest = load_yaml(self.project.manifest_path) + manifest.setdefault("dependencies", {})["apm"] = list(dependencies) + dump_yaml(manifest, self.project.manifest_path) + class _LifecycleReferenceModel(RuleBasedStateMachine): """Reference state independent of lockfile deployment records.""" @@ -145,7 +260,18 @@ def __init__(self, fixture: _ModelFixture) -> None: self.materialized = False self.clean = True self.locked = False + self.retained_user_file = False self.step = 0 + self.last_operation = "initial" + self.published_revision = "a" + self.installed_revision = "a" + self.published_commit = fixture.source.commit.sha + self.installed_commit = fixture.source.commit.sha + + @initialize() + def mandatory_spine(self) -> None: + """Guarantee connected commands inside every generated model execution.""" + _mandatory_replay(self) @property def scenario(self) -> _Scenario: @@ -157,89 +283,105 @@ def project(self) -> LocalPackage: @property def skill_path(self) -> Path: - return self.project.root / ".agents" / "skills" / _SKILL_NAME / "SKILL.md" + return self.fixture.skill_path + + def _run( + self, + args: tuple[str, ...], + operation: str, + expected_returncode: int = 0, + *, + command_cwd: Path | None = None, + ) -> CommandResult: + result = self.scenario.runner.run( + args, + cwd=command_cwd or self.fixture.invoke_cwd, + env=self.fixture.environment, + scenario_id=self._next_id(operation), + ) + assert result.returncode == expected_returncode, _result_evidence(result) + return result + + def _state(self) -> LifecycleStateSnapshot: + return LifecycleStateSnapshot.capture( + self.project.root, external_roots=self.fixture.external_roots + ) + + def _changed_path(self, relative_path: str) -> dict[str, set[str]]: + if self.fixture.global_scope: + return {"user": {f".apm/{relative_path}"}} + return {"project": {relative_path}} def _next_id(self, operation: str) -> str: self.step += 1 + self.last_operation = operation return f"generated-{self.step:02d}-{operation}" def _capture(self) -> ArtifactSnapshotSet: - return ArtifactSnapshotSet.capture( - { - "project": self.project.root, - "user": self.scenario.isolated.home, - } - ) + roots = {"project": self.fixture.invoke_cwd, "user": self.scenario.isolated.home} + if self.fixture.global_scope: + roots["target"] = self.fixture.external_roots[0].path + return ArtifactSnapshotSet.capture(roots) @rule() @precondition(lambda self: self.declared and not self.materialized) def install(self) -> None: before = self._capture() - _run_success( - self.scenario, - self.project, - _INSTALL_ARGS, - environment=self.fixture.environment, - scenario_id=self._next_id("install"), - ) + self._run(self.fixture.install_args, "install") assert_snapshot_changes_within( before, self._capture(), - exact_paths=_INSTALL_EXACT_PATHS, - tree_prefixes=_INSTALL_TREE_PREFIXES, + exact_paths=self.fixture.exact_paths, + tree_prefixes=self.fixture.tree_prefixes, ) self.materialized = True - self.clean = True + self.clean = not self.retained_user_file self.locked = True + self.installed_revision = self.published_revision + self.installed_commit = self.published_commit @rule() @precondition(lambda self: self.declared and self.materialized and self.clean) def reinstall(self) -> None: before = self._capture() - _run_success( - self.scenario, - self.project, - _INSTALL_ARGS, - environment=self.fixture.environment, - scenario_id=self._next_id("reinstall"), - ) + self._run(self.fixture.install_args, "reinstall") assert_snapshot_set_unchanged(before, self._capture()) @rule() @precondition(lambda self: self.declared) def dry_run(self) -> None: before = self._capture() - result = _run_success( - self.scenario, - self.project, - (*_INSTALL_ARGS, "--dry-run"), - environment=self.fixture.environment, - scenario_id=self._next_id("dry-run"), - ) - assert "[i] APM dependencies (1):" in result.stdout.splitlines(), ( - "dry-run must prove dependency resolution reached the fixture" - ) + result = self._run((*self.fixture.install_args, "--dry-run"), "dry-run") + assert "[i] APM dependencies (1):" in result.stdout.splitlines() assert_snapshot_set_unchanged(before, self._capture()) @rule() @precondition(lambda self: self.materialized and self.clean) def tamper(self) -> None: before = self._capture() - self.skill_path.write_bytes(b"# user tamper\n") + self.skill_path.write_bytes(_TAMPER_BYTES) assert_only_snapshot_paths_changed( before, self._capture(), - {"project": {f".agents/skills/{_SKILL_NAME}/SKILL.md"}}, + ( + {"target": {f"skills/{_SKILL_NAME}/SKILL.md"}} + if self.fixture.global_scope + else {"project": {f".agents/skills/{_SKILL_NAME}/SKILL.md"}} + ), ) self.clean = False @rule() - @precondition(lambda self: self.declared and self.materialized and not self.clean) + @precondition( + lambda self: ( + self.declared and self.materialized and not self.clean and not self.retained_user_file + ) + ) def audit_tampered(self) -> None: before = self._capture() - result, payload = _audit( + result, payload = _audit_at( self.scenario, - self.project, + self.fixture.audit_cwd, environment=self.fixture.environment, expected_returncode=1, scenario_id=self._next_id("audit-tampered"), @@ -251,28 +393,26 @@ def audit_tampered(self) -> None: @precondition(lambda self: self.declared and self.materialized and not self.clean) def repair(self) -> None: before = self._capture() - _run_success( - self.scenario, - self.project, - _INSTALL_ARGS, - environment=self.fixture.environment, - scenario_id=self._next_id("repair"), - ) + args = self.fixture.install_args + if self.retained_user_file: + args = (*args, "--force") + self._run(args, "repair") assert_snapshot_changes_within( before, self._capture(), - exact_paths=_INSTALL_EXACT_PATHS, - tree_prefixes=_INSTALL_TREE_PREFIXES, + exact_paths=self.fixture.exact_paths, + tree_prefixes=self.fixture.tree_prefixes, ) self.clean = True + self.retained_user_file = False @rule() @precondition(lambda self: self.declared and self.materialized and self.clean) def audit_clean(self) -> None: before = self._capture() - _result, payload = _audit( + _result, payload = _audit_at( self.scenario, - self.project, + self.fixture.audit_cwd, environment=self.fixture.environment, scenario_id=self._next_id("audit-clean"), ) @@ -283,14 +423,11 @@ def audit_clean(self) -> None: @precondition(lambda self: self.declared) def remove_declaration(self) -> None: before = self._capture() - assert self.scenario.consumers.remove_apm_dependency( - self.project, - self.fixture.dependency, - ) + self.fixture.replace_dependencies(()) assert_only_snapshot_paths_changed( before, self._capture(), - {"project": {"apm.yml"}}, + self._changed_path("apm.yml"), ) self.declared = False @@ -298,28 +435,22 @@ def remove_declaration(self) -> None: @precondition(lambda self: not self.declared) def readd_declaration(self) -> None: before = self._capture() - self.scenario.consumers.replace_apm_dependencies( - self.project, - (self.fixture.dependency,), - ) + self.fixture.replace_dependencies((self.fixture.dependency,)) assert_only_snapshot_paths_changed( before, self._capture(), - {"project": {"apm.yml"}}, + self._changed_path("apm.yml"), ) self.declared = True @rule() - @precondition(lambda self: not self.declared and self.materialized) + @precondition( + lambda self: not self.fixture.global_scope and not self.declared and self.materialized + ) def prune_removed(self) -> None: before = self._capture() - _run_success( - self.scenario, - self.project, - ("prune",), - environment=self.fixture.environment, - scenario_id=self._next_id("prune"), - ) + retained_bytes = self.skill_path.read_bytes() if not self.clean else None + self._run(("prune",), "prune") assert_snapshot_changes_within( before, self._capture(), @@ -327,18 +458,270 @@ def prune_removed(self) -> None: tree_prefixes=_INSTALL_TREE_PREFIXES, ) self.materialized = False + self.retained_user_file = retained_bytes is not None + if retained_bytes is not None: + assert self.skill_path.read_bytes() == retained_bytes self.clean = True self.locked = False + @rule() + @precondition(lambda self: self.fixture.global_scope and self.published_revision == "a") + def publish(self) -> None: + before = self._capture() + commit = _publish_revision(self.scenario, self.fixture.source, "b") + self.published_revision = "b" + self.published_commit = commit.sha + assert_snapshot_set_unchanged(before, self._capture()) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.declared and self.materialized) + def outdated(self) -> None: + before, state = self._capture(), self._state() + result = self._run( + ("outdated", "--global", "--parallel-checks", "0", "--verbose"), "outdated" + ) + if self.installed_commit != self.published_commit: + assert self.published_commit[:7] in result.stdout, _result_evidence(result) + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.materialized and self.clean) + def inspect_installed(self) -> None: + before, state = self._capture(), self._state() + for args in ( + ("deps", "list", "--global"), + ("deps", "tree", "--global"), + ("deps", "why", _PACKAGE_NAME, "--global", "--json"), + ("view", _PACKAGE_NAME, "--global"), + ("info", _PACKAGE_NAME, "--global"), + ): + result = self._run(args, f"inspect-{'-'.join(args[:2])}") + assert _PACKAGE_NAME in result.stdout, _result_evidence(result) + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + info = self._run( + ("deps", "info", _PACKAGE_NAME), "deps-info", command_cwd=self.fixture.audit_cwd + ) + assert _PACKAGE_NAME in info.stdout, _result_evidence(info) + self._run(("cache", "info"), "cache-info") + pruned = self._run(("cache", "prune", "--days", "30"), "cache-prune") + assert "Pruned 0 SHA group(s)" in pruned.stdout, _result_evidence(pruned) + targets = self._run(("targets", "--json"), "targets-project-only") + rows = json.loads(targets.stdout) + assert rows and all(row["status"] == "inactive" for row in rows) + found = self._run( + ("find", str(self.skill_path)), + "find-global-refusal", + 1, + command_cwd=self.fixture.audit_cwd, + ) + assert "is not tracked by any installed package" in " ".join(found.stdout.split()) + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.materialized and self.clean) + def export_lock(self) -> None: + before, state = self._capture(), self._state() + for format_name in ("cyclonedx", "spdx"): + result = self._run( + ("lock", "export", "--global", "--format", format_name), f"export-{format_name}" + ) + document = json.loads(result.stdout) + assert document.get("bomFormat") == "CycloneDX" or document.get("spdxVersion") + assert self.installed_commit in result.stdout, _result_evidence(result) + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.materialized and self.clean) + def clean_cache(self) -> None: + before, state = self._capture(), self._state() + self._run(("cache", "clean", "--yes"), "clean-cache") + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.materialized and self.clean) + def clean_sources(self) -> None: + before, state = self._capture(), self._state() + self._run( + ("deps", "clean", "--dry-run"), "clean-preview", command_cwd=self.fixture.audit_cwd + ) + assert_snapshot_set_unchanged(before, self._capture()) + self._run(("deps", "clean", "--yes"), "clean-sources", command_cwd=self.fixture.audit_cwd) + assert not (self.project.root / "apm_modules").exists() + assert self.skill_path.read_bytes() == ( + _SKILL_BYTES + f"\nrevision-{self.installed_revision}\n".encode("ascii") + ) + result = self._run( + ("audit", "--ci", "--no-policy", "--no-fail-fast", "--format", "json"), + "audit-missing-sources", + 1, + command_cwd=self.fixture.audit_cwd, + ) + assert json.loads(result.stdout)["passed"] is False + self._run(self.fixture.install_args, "rehydrate-sources") + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.materialized and self.clean) + def legacy_update(self) -> None: + before = self._capture() + self._run(("deps", "update", "--global", "--parallel-downloads", "0"), "legacy-update") + self.installed_revision = self.published_revision + self.installed_commit = self.published_commit + assert_snapshot_changes_within( + before, + self._capture(), + exact_paths=self.fixture.exact_paths, + tree_prefixes=self.fixture.tree_prefixes, + ) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.declared and self.materialized) + def update(self) -> None: + before = self._capture() + self._run(("update", "--global", "--yes", "--parallel-downloads", "0"), "update") + assert_snapshot_changes_within( + before, + self._capture(), + exact_paths=self.fixture.exact_paths, + tree_prefixes=self.fixture.tree_prefixes, + ) + self.installed_revision = self.published_revision + self.installed_commit = self.published_commit + self.clean = True + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.materialized) + def compile(self) -> None: + before = self._capture() + self._run(("compile", "--global"), "compile") + text = (self.fixture.external_roots[0].path / "CLAUDE.md").read_text(encoding="ascii") + assert f"# revision-{self.installed_revision}" in text + assert_snapshot_changes_within( + before, self._capture(), exact_paths={"target": {"CLAUDE.md"}}, tree_prefixes={} + ) + + @rule() + @precondition( + lambda self: ( + self.fixture.global_scope + and self.declared + and self.materialized + and self.clean + and self.installed_commit == self.published_commit + ) + ) + def lock(self) -> None: + before, state = self._capture(), self._state() + self._run(("lock", "--global", "--no-policy", "--parallel-downloads", "0"), "lock") + after = self._state() + assert after.deployment_records == state.deployment_records + assert after.files == state.files + assert_snapshot_changes_within( + before, + self._capture(), + exact_paths=self._changed_path("apm.lock.yaml"), + tree_prefixes={}, + ) + + @rule() + @precondition( + lambda self: ( + self.fixture.global_scope and self.declared and self.materialized and self.clean + ) + ) + def frozen_replay(self) -> None: + before, state = self._capture(), self._state() + self._run((*self.fixture.install_args, "--frozen"), "frozen-replay") + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + + @rule() + @precondition( + lambda self: ( + self.fixture.global_scope and self.declared and self.materialized and self.clean + ) + ) + def frozen_refusal(self) -> None: + manifest_bytes = self.project.manifest_path.read_bytes() + self.fixture.replace_dependencies(({**self.fixture.dependency, "ref": "missing-ref"},)) + before, state = self._capture(), self._state() + self._run((*self.fixture.install_args, "--frozen"), "frozen-refusal", 1) + _assert_same_state(state, self._state()) + assert_snapshot_set_unchanged(before, self._capture()) + self.project.manifest_path.write_bytes(manifest_bytes) + + @rule() + @precondition(lambda self: self.fixture.global_scope and self.materialized and self.clean) + def uninstall(self) -> None: + before = self._capture() + self._run(("uninstall", "--global", self.fixture.source.remote_url), "uninstall") + assert_snapshot_changes_within( + before, + self._capture(), + exact_paths=self.fixture.exact_paths, + tree_prefixes=self.fixture.tree_prefixes, + ) + self.materialized = False + self.declared = False + self.locked = False + + @rule() + @precondition( + lambda self: self.fixture.global_scope and not self.materialized and not self.declared + ) + def audit_empty(self) -> None: + before = self._capture() + _, payload = _audit_at( + self.scenario, + self.fixture.audit_cwd, + environment=self.fixture.environment, + scenario_id=self._next_id("audit-empty"), + ) + assert payload["passed"] is True + assert_snapshot_set_unchanged(before, self._capture()) + @invariant() def durable_state_matches_reference_model(self) -> None: manifest = yaml.safe_load(self.project.manifest_path.read_text(encoding="utf-8")) assert isinstance(manifest, dict) dependencies = manifest.get("dependencies", {}).get("apm", []) assert bool(dependencies) is self.declared - assert self.skill_path.exists() is self.materialized + assert self.skill_path.exists() is (self.materialized or self.retained_user_file) + if self.retained_user_file: + assert self.skill_path.read_bytes() == _TAMPER_BYTES if self.materialized and self.clean: - assert self.skill_path.read_bytes() == _SKILL_BYTES + expected = _SKILL_BYTES + if self.fixture.global_scope: + expected += f"\nrevision-{self.installed_revision}\n".encode() + assert self.skill_path.read_bytes() == expected, ( + f"after {self.last_operation} step={self.step}; expected revision " + f"{self.installed_revision} at {self.skill_path}" + ) + if self.materialized and self.fixture.global_scope: + lock = LockFile.read(self.project.root / "apm.lock.yaml") + assert lock is not None + dependencies = lock.get_package_dependencies() + assert len(dependencies) == 1 + assert dependencies[0].resolved_commit == self.installed_commit + modules = self.project.root / "apm_modules" + assert ( + modules / "apm-fixture-org" / _PACKAGE_NAME / "skills" / _SKILL_NAME / "SKILL.md" + ).is_file() + assert not (modules / _PACKAGE_NAME).exists() + assert self._state().deployment_records + assert ( + self.fixture.external_roots[0].path / "user-owned.txt" + ).read_bytes() == b"user-owned\n" + if self.fixture.variant == "global-aliased": + alias = self.scenario.isolated.root / "home-alias" + assert alias.is_symlink() + assert alias.readlink() == self.scenario.isolated.home assert (self.project.root / "apm.lock.yaml").exists() is self.locked def teardown(self) -> None: @@ -346,16 +729,18 @@ def teardown(self) -> None: shutil.rmtree(self.scenario.isolated.root) +@pytest.mark.parametrize("variant", _VARIANTS) def test_generated_lifecycle_sequences_preserve_reference_model( tmp_path: Path, apm_binary_path: Path, + variant: str, ) -> None: """Generate and shrink guarded transition sequences over a real CLI.""" sequence = itertools.count() def factory() -> _LifecycleReferenceModel: case_root = tmp_path / f"case-{next(sequence):03d}" - return _LifecycleReferenceModel(_ModelFixture.create(case_root, apm_binary_path)) + return _LifecycleReferenceModel(_ModelFixture.create(case_root, apm_binary_path, variant)) run_state_machine_as_test( factory, @@ -363,7 +748,7 @@ def factory() -> _LifecycleReferenceModel: database=None, deadline=None, derandomize=True, - max_examples=6, + max_examples=6 if variant == "project" else 3, phases=(Phase.generate, Phase.shrink), print_blob=True, stateful_step_count=8, @@ -372,6 +757,78 @@ def factory() -> _LifecycleReferenceModel: ) +@pytest.mark.parametrize("variant", _VARIANTS) +def test_generated_lifecycle_mandatory_replay( + tmp_path: Path, apm_binary_path: Path, variant: str +) -> None: + """Exercise the model's guarded commands without relying on random selection.""" + model = _LifecycleReferenceModel( + _ModelFixture.create(tmp_path / "mandatory", apm_binary_path, variant) + ) + _mandatory_replay(model) + + +def _mandatory_replay(model: _LifecycleReferenceModel) -> None: + operations = [ + model.dry_run, + model.install, + model.reinstall, + ] + if model.fixture.global_scope: + operations.extend( + [ + model.compile, + model.lock, + model.inspect_installed, + model.export_lock, + model.publish, + model.outdated, + model.frozen_replay, + model.update, + model.compile, + model.export_lock, + model.legacy_update, + model.frozen_replay, + model.frozen_refusal, + model.reinstall, + model.clean_cache, + model.clean_sources, + ] + ) + operations.extend( + [ + model.tamper, + model.audit_tampered, + model.repair, + model.audit_clean, + ] + ) + if model.fixture.global_scope: + operations.extend([model.uninstall, model.audit_empty]) + else: + operations.extend([model.remove_declaration, model.prune_removed]) + operations.extend([model.readd_declaration, model.install, model.audit_clean]) + if model.fixture.global_scope: + operations.extend([model.uninstall, model.audit_empty]) + else: + operations.extend( + [ + model.tamper, + model.remove_declaration, + model.prune_removed, + model.readd_declaration, + model.install, + model.repair, + model.audit_clean, + ] + ) + for operation in operations: + transition = operation.hypothesis_stateful_rule + assert all(condition(model) for condition in transition.preconditions) + operation() + model.durable_state_matches_reference_model() + + def test_generated_transition_catalog_covers_phase_one_properties() -> None: """Require every generated transition to invoke a filesystem oracle.""" rules = { diff --git a/tests/integration/test_global_audit_deploy_root.py b/tests/integration/test_global_audit_deploy_root.py index 08576024bf..ac5aae6464 100644 --- a/tests/integration/test_global_audit_deploy_root.py +++ b/tests/integration/test_global_audit_deploy_root.py @@ -47,12 +47,23 @@ def _tree_snapshot( return directories, files +@pytest.mark.parametrize("aliased_home", [False, True]) def test_global_install_audit_reads_home_deployment_root( + aliased_home: bool, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: isolated = IsolatedApmEnvironment.create(tmp_path / "global-audit", base_env=os.environ) environment = isolated.subprocess_env() + if aliased_home: + alias = isolated.root / "home-alias" + try: + alias.symlink_to(isolated.home, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("directory symlinks unavailable") + environment["HOME"] = str(alias) + environment["USERPROFILE"] = str(alias) + environment["APM_HOME"] = str(alias / ".apm") environment["APM_NO_CACHE"] = "1" for name, value in environment.items(): monkeypatch.setenv(name, value) diff --git a/tests/integration/test_lifecycle_state_snapshot_contract.py b/tests/integration/test_lifecycle_state_snapshot_contract.py index 80c5fc123f..c88e6aba3e 100644 --- a/tests/integration/test_lifecycle_state_snapshot_contract.py +++ b/tests/integration/test_lifecycle_state_snapshot_contract.py @@ -280,8 +280,10 @@ def test_capture_rejects_target_relative_deployment_without_bounded_root( LifecycleStateSnapshot.capture(workspace, targets=("claude",)) +@pytest.mark.parametrize("legacy_absolute", [False, True]) def test_capture_reads_two_bounded_roots_without_relative_path_collisions( tmp_path: Path, + legacy_absolute: bool, ) -> None: workspace = tmp_path / "workspace" workspace.mkdir() @@ -296,17 +298,19 @@ def test_capture_reads_two_bounded_roots_without_relative_path_collisions( claude_script.write_bytes(b"print('bounded')\n") records = ( _record( - kind=LocatorKind.TARGET_RELATIVE, + kind=LocatorKind.PROJECT_RELATIVE if legacy_absolute else LocatorKind.TARGET_RELATIVE, target="claude", - scope="user", - value="hooks/fixture/check.py", + scope="project" if legacy_absolute else "user", + value=claude_script.as_posix() if legacy_absolute else "hooks/fixture/check.py", content_hash="sha256:script", ), _record( - kind=LocatorKind.TARGET_RELATIVE, + kind=LocatorKind.PROJECT_RELATIVE if legacy_absolute else LocatorKind.TARGET_RELATIVE, target="cursor", - scope="user", - value="rules/missing.mdc", + scope="project" if legacy_absolute else "user", + value=(cursor_root / "rules/missing.mdc").as_posix() + if legacy_absolute + else "rules/missing.mdc", ), _record( kind=LocatorKind.URI, @@ -337,6 +341,8 @@ def test_capture_reads_two_bounded_roots_without_relative_path_collisions( ) snapshot = LifecycleStateSnapshot.capture(workspace, external_roots=roots) + assert set(snapshot.deployment_records) == set(records) + assert snapshot.lockfile_bytes == (workspace / "apm.lock.yaml").read_bytes() claude_config = snapshot.file("settings.json", root_id="claude-user") cursor_config = snapshot.file("settings.json", root_id="cursor-user") @@ -361,7 +367,10 @@ def test_capture_reads_two_bounded_roots_without_relative_path_collisions( snapshot.file("copilot-app-db://workflows/external", root_id="claude-user") -def test_bounded_roots_fail_closed_before_outside_reads(tmp_path: Path) -> None: +@pytest.mark.parametrize("legacy_absolute", [False, True]) +def test_bounded_roots_fail_closed_before_outside_reads( + tmp_path: Path, legacy_absolute: bool +) -> None: workspace = tmp_path / "workspace" workspace.mkdir() root = tmp_path / "user-root" @@ -375,10 +384,12 @@ def test_bounded_roots_fail_closed_before_outside_reads(tmp_path: Path) -> None: workspace, ( _record( - kind=LocatorKind.TARGET_RELATIVE, + kind=LocatorKind.PROJECT_RELATIVE + if legacy_absolute + else LocatorKind.TARGET_RELATIVE, target="claude", - scope="user", - value="linked.json", + scope="project" if legacy_absolute else "user", + value=(root / "linked.json").as_posix() if legacy_absolute else "linked.json", ), ), ) @@ -412,6 +423,41 @@ def test_bounded_roots_fail_closed_before_outside_reads(tmp_path: Path) -> None: assert outside_file.read_bytes() == b'{"secret":"unchanged"}\n' +@pytest.mark.parametrize("fault", ["unbounded", "wrong-target", "traversal", "linked-ancestor"]) +def test_legacy_absolute_deployments_require_matching_bounded_root( + tmp_path: Path, fault: str +) -> None: + workspace, root, outside = (tmp_path / name for name in ("workspace", "root", "outside")) + for directory in (workspace, root, outside): + directory.mkdir() + secret = outside / "secret.txt" + secret.write_bytes(b"outside-user-owned\n") + candidate = root / "missing.txt" + if fault == "unbounded": + candidate = secret + elif fault == "traversal": + candidate = root / ".." / "outside" / "secret.txt" + elif fault == "linked-ancestor": + (root / "link").symlink_to(outside, target_is_directory=True) + candidate = root / "link" / "secret.txt" + _write_lock( + workspace, + ( + _record( + kind=LocatorKind.PROJECT_RELATIVE, + target="cursor" if fault == "wrong-target" else "claude", + value=candidate.as_posix(), + ), + ), + ) + with pytest.raises(ValueError, match=r"bounded root|traversal sequence"): + LifecycleStateSnapshot.capture( + workspace, + external_roots=(LifecycleStateRoot("claude-user", "claude", "user", root),), + ) + assert secret.read_bytes() == b"outside-user-owned\n" + + def test_bounded_root_identity_and_path_collisions_are_rejected(tmp_path: Path) -> None: workspace = tmp_path / "workspace" workspace.mkdir() diff --git a/tests/integration/test_ownership_invariant_lifecycle.py b/tests/integration/test_ownership_invariant_lifecycle.py index 47ecca3dfb..2a91a542aa 100644 --- a/tests/integration/test_ownership_invariant_lifecycle.py +++ b/tests/integration/test_ownership_invariant_lifecycle.py @@ -15,12 +15,15 @@ from apm_cli.integration.targets import KNOWN_TARGETS, PrimitiveMapping, TargetProfile from apm_cli.utils.yaml_io import dump_yaml from tests.integration.test_required_lifecycle_state_machine import ( + _EXTERNAL_USER_ROOT_ENV, _OWNER, _assert_same_state, _audit, _hook, _instruction, _new_scenario, + _publish, + _publish_revision, _result_evidence, _run_success, _skill, @@ -804,3 +807,92 @@ def test_apm_install_dry_run_preserves_complete_unowned_state( assert (scenario.isolated.config_root / "apm_modules").exists() is False _assert_same_state(before, after) assert_snapshot_set_unchanged(before_artifacts, after_artifacts) + + +def test_global_update_preserves_owned_external_skill_targets( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """User-scope update must reuse the user lockfile for external skill ownership.""" + scenario = _new_scenario(tmp_path / "global-update-external-skills", apm_binary_path) + published = _publish(scenario, "global-audit-kit", skill="global-audit") + commit_a = _publish_revision(scenario, published, "a") + dependency = {**published.dependency, "ref": "main"} + consumer = LocalPackage( + "global-update-consumer", + scenario.isolated.work_root, + scenario.isolated.config_root / "apm.yml", + ) + external_roots = { + target: scenario.isolated.root / f"{target}-home" for target in ("claude", "hermes") + } + for root in external_roots.values(): + root.mkdir(parents=True) + + environment = dict(published.environment) + for target, root in external_roots.items(): + environment[_EXTERNAL_USER_ROOT_ENV[target]] = str(root) + + dump_yaml( + { + "name": consumer.name, + "version": "0.1.0", + "dependencies": {"apm": [dependency]}, + "targets": ["claude", "hermes"], + }, + consumer.manifest_path, + ) + + install_args = ("install", "--global", "--no-policy", "--parallel-downloads", "0") + _run_success( + scenario, + consumer, + install_args, + environment=environment, + scenario_id="global-update-external-install", + ) + + commit_b = _publish_revision(scenario, published, "b") + update = _run_success( + scenario, + consumer, + ("update", "--global", "--yes", "--parallel-downloads", "0", "--verbose"), + environment=environment, + scenario_id="global-update-external-update", + ) + + expected_skill = (_skill("global-audit") + "\nrevision-b\n").encode() + claude_skill = external_roots["claude"] / "skills" / "global-audit" / "SKILL.md" + hermes_skill = external_roots["hermes"] / "skills" / "global-audit" / "SKILL.md" + assert claude_skill.read_bytes() == expected_skill + assert hermes_skill.read_bytes() == expected_skill + assert "file skipped (local files exist)" not in (update.stdout + update.stderr) + canonical_cache = scenario.isolated.config_root / "apm_modules" / _OWNER / "global-audit-kit" + alias_cache = scenario.isolated.config_root / "apm_modules" / "global-audit-kit" + assert (canonical_cache / "skills" / "global-audit" / "SKILL.md").read_bytes() == ( + expected_skill + ) + assert not alias_cache.exists() + + lock = LockFile.read(scenario.isolated.config_root / "apm.lock.yaml") + assert lock is not None + dependency_lock = lock.get_package_dependencies()[0] + assert dependency_lock.resolved_commit == commit_b.sha + deployed = set(dependency_lock.deployed_files) + assert str(claude_skill) in deployed + assert str(hermes_skill) in deployed + assert str(claude_skill.parent) in deployed + assert str(hermes_skill.parent) in deployed + assert str(external_roots["claude"] / "rules" / "revision.md") in deployed + + _run_success( + scenario, + consumer, + ("compile", "--global"), + environment=environment, + scenario_id="global-update-external-compile", + ) + compiled_claude = (external_roots["claude"] / "CLAUDE.md").read_text(encoding="utf-8") + assert "revision-b" in compiled_claude + assert "revision-a" not in compiled_claude + assert commit_a.sha != commit_b.sha diff --git a/tests/integration/test_ref_freshness_lifecycle.py b/tests/integration/test_ref_freshness_lifecycle.py index 1013bf33c3..9fc9c5bc61 100644 --- a/tests/integration/test_ref_freshness_lifecycle.py +++ b/tests/integration/test_ref_freshness_lifecycle.py @@ -75,7 +75,9 @@ def _skill_document(marker: str) -> str: ) -def _new_scenario(root: Path, apm_binary_path: Path) -> _Scenario: +def _new_scenario( + root: Path, apm_binary_path: Path, *, reference: str | None = "main" +) -> _Scenario: isolated = IsolatedApmEnvironment.create(root, base_env=dict(os.environ)) environment = isolated.subprocess_env(overrides={"APM_TIERED_RESOLVER": "1"}) packages = LocalPackageFactory(isolated.package_root) @@ -100,7 +102,7 @@ def _new_scenario(root: Path, apm_binary_path: Path) -> _Scenario: dependencies=( { "git": remote_url, - "ref": "main", + **({"ref": reference} if reference is not None else {}), }, ), targets=("copilot",), @@ -195,6 +197,26 @@ def _combined_output(result: CommandResult) -> str: return result.stdout + result.stderr +def test_frozen_default_ref_rehydrates_cold_cache_without_ref_drift( + tmp_path: Path, apm_binary_path: Path +) -> None: + """An omitted ref replays its locked SHA even after upstream advances.""" + scenario = _new_scenario(tmp_path / "frozen-default-ref", apm_binary_path, reference=None) + _run(scenario, _INSTALL_ARGS, scenario_id="default-ref-install-a") + installed = _capture(scenario) + assert _locked_commit(scenario) == scenario.commit_a.sha + _advance(scenario, "commit-b") + _run(scenario, ("deps", "clean", "--yes"), scenario_id="default-ref-remove-modules") + _run(scenario, ("cache", "clean", "--yes"), scenario_id="default-ref-clear-cache") + assert not (scenario.consumer.root / "apm_modules").exists() + replay = _run(scenario, (*_INSTALL_ARGS, "--frozen"), scenario_id="default-ref-frozen-a") + assert _module_skill_bytes(scenario) == _skill_document("commit-a").encode() + assert _locked_commit(scenario) == scenario.commit_a.sha + _assert_same_state(installed, _capture(scenario)) + assert "sha_passthrough=1" in _combined_output(replay) + _run(scenario, _AUDIT_ARGS, scenario_id="default-ref-audit-a") + + def test_installed_cli_current_state_commands_bypass_stale_bare_cache( tmp_path: Path, apm_binary_path: Path, diff --git a/tests/integration/test_required_lifecycle_state_machine.py b/tests/integration/test_required_lifecycle_state_machine.py index d4c35f4e0e..a36f3af958 100644 --- a/tests/integration/test_required_lifecycle_state_machine.py +++ b/tests/integration/test_required_lifecycle_state_machine.py @@ -6,7 +6,7 @@ import os import shutil import zipfile -from collections.abc import Mapping +from collections.abc import Callable, Mapping from dataclasses import dataclass from pathlib import Path, PurePosixPath @@ -22,6 +22,7 @@ ArtifactSnapshotSet, assert_only_snapshot_paths_changed, assert_snapshot_changes_within, + assert_snapshot_set_unchanged, assert_unchanged, ) from tests.utils.isolated_apm_environment import IsolatedApmEnvironment @@ -113,6 +114,24 @@ def _skill(name: str) -> str: ) +def _revision_instruction(revision: str) -> str: + return f"---\ndescription: Global lifecycle revision\n---\n# revision-{revision}\n" + + +def _publish_revision(scenario: _Scenario, source: _PublishedPackage, revision: str) -> GitCommit: + """Publish in the Git worktree, not the original package authoring tree.""" + for skill_path in (source.repository.worktree / "skills").glob("*/SKILL.md"): + skill_path.write_text( + _skill(skill_path.parent.name) + f"\nrevision-{revision}\n", encoding="ascii" + ) + instruction_path = ( + source.repository.worktree / ".apm" / "instructions" / "revision.instructions.md" + ) + instruction_path.parent.mkdir(parents=True, exist_ok=True) + instruction_path.write_text(_revision_instruction(revision), encoding="ascii") + return scenario.repositories.commit(source.repository, message=f"publish revision {revision}") + + def _instruction(name: str) -> str: return ( "---\n" @@ -1304,7 +1323,6 @@ def test_required_legacy_content_hash_upgrade_preserves_skills_and_converges( "legacy-hash-kit", skill="legacy-hash", ) - package = source.package consumer = scenario.consumers.create( "legacy-hash-consumer", dependencies=(source.dependency,), @@ -1339,7 +1357,10 @@ def test_required_legacy_content_hash_upgrade_preserves_skills_and_converges( prior_deployed_files = list(locked_dependency["deployed_files"]) prior_deployed_hashes = dict(locked_dependency["deployed_file_hashes"]) - cached_package = consumer.root / "apm_modules" / package.name + _, installed_dependency = _single_locked_dependency(consumer.root) + cached_package = installed_dependency.to_dependency_ref().get_install_path( + consumer.root / "apm_modules" + ) receipt = cached_package / ".apm" / ".plugin-skill-sources.json" assert receipt.is_file() receipt.unlink() @@ -1432,7 +1453,10 @@ def test_required_parallel_fresh_fetch_bypasses_legacy_cache_upgrade( scenario_id="parallel-fresh-plugin-establish", ) - cached_package = consumer.root / "apm_modules" / source.package.name + _, installed_dependency = _single_locked_dependency(consumer.root) + cached_package = installed_dependency.to_dependency_ref().get_install_path( + consumer.root / "apm_modules" + ) lock_path = consumer.root / "apm.lock.yaml" lock_document = load_yaml(lock_path) locked_dependency = lock_document["dependencies"][0] @@ -1489,7 +1513,7 @@ def test_required_invalid_receiptless_legacy_cache_fails_with_recovery( apm_binary_path: Path, invalid_cache: str, ) -> None: - """Reject invalid 0.28 cache state without mutating cache or deployments.""" + """Reject invalid 0.28 cache state, preserve deployments, then recover.""" scenario = _new_scenario( tmp_path / f"invalid-legacy-cache-{invalid_cache}", apm_binary_path, @@ -1512,7 +1536,10 @@ def test_required_invalid_receiptless_legacy_cache_fails_with_recovery( scenario_id="invalid-legacy-plugin-establish", ) - cached_package = consumer.root / "apm_modules" / source.package.name + _, installed_dependency = _single_locked_dependency(consumer.root) + cached_package = installed_dependency.to_dependency_ref().get_install_path( + consumer.root / "apm_modules" + ) receipt = cached_package / ".apm" / ".plugin-skill-sources.json" assert receipt.is_file() receipt.unlink() @@ -1574,6 +1601,7 @@ def test_required_invalid_receiptless_legacy_cache_fails_with_recovery( dump_yaml(lock_document, lock_path) before_state = LifecycleStateSnapshot.capture(consumer.root, targets=("claude", "codex")) + before_artifacts = ArtifactSnapshotSet.capture({"project": consumer.root}) before_cache = ArtifactSnapshot.capture(cached_package) before_external = ArtifactSnapshot.capture(external_root) if external_root is not None else None package_link_target = ( @@ -1589,29 +1617,68 @@ def test_required_invalid_receiptless_legacy_cache_fails_with_recovery( output = " ".join((result.stdout + result.stderr).split()) assert result.returncode != 0, _result_evidence(result) - assert source.package.name in output - assert str(cached_package) in "".join(output.split()) - assert "apm deps clean --yes" in output - if invalid_cache == "plugin-path": - assert "is invalid" in output - elif invalid_cache == "missing-hash": - assert "no content hash" in output - elif invalid_cache == "missing-apm-yml": - assert "required apm.yml is missing" in output - elif invalid_cache == "missing-apm-dir": - assert "required .apm directory is missing" in output + compact_output = "".join(output.split()) + assert source.package.name in compact_output, _result_evidence(result) + rejected_redownload = invalid_cache in {"missing-apm-yml", "missing-apm-dir"} + if rejected_redownload: + assert "Content hash mismatch" in output, _result_evidence(result) + assert "apm install --update" in output + assert not cached_package.exists() + elif invalid_cache == "package-root-symlink": + assert "Cannot verify containment" in output, _result_evidence(result) + assert str(cached_package) in compact_output else: - assert "cache metadata contains a symlink" in output + assert str(cached_package) in compact_output + assert "apm deps clean --yes" in output + if invalid_cache == "plugin-path": + assert "is invalid" in output + elif invalid_cache == "missing-hash": + assert "no content hash" in output + else: + assert "cache metadata contains a symlink" in output if package_link_target is not None: assert cached_package.is_symlink() assert cached_package.readlink() == package_link_target - assert_unchanged(before_cache, ArtifactSnapshot.capture(cached_package)) + if not rejected_redownload: + assert_unchanged(before_cache, ArtifactSnapshot.capture(cached_package)) + assert_snapshot_changes_within( + before_artifacts, + ArtifactSnapshotSet.capture({"project": consumer.root}), + exact_paths={}, + tree_prefixes={"project": {cached_package.relative_to(consumer.root).as_posix()}}, + ) if before_external is not None and external_root is not None: assert_unchanged(before_external, ArtifactSnapshot.capture(external_root)) after_state = LifecycleStateSnapshot.capture(consumer.root, targets=("claude", "codex")) _assert_same_state(before_state, after_state) + # Follow targeted-cache-removal guidance without following a bad root link. + if package_link_target is not None: + cached_package.unlink() + elif cached_package.exists(): + shutil.rmtree(cached_package) + _run_success( + scenario, + consumer, + (*_INSTALL_ARGS, "--update"), + environment=source.environment, + scenario_id=f"invalid-legacy-plugin-{invalid_cache}-recovery", + ) + assert receipt.is_file() + assert (consumer.root / ".claude/skills/legacy-skill/SKILL.md").read_text( + encoding="ascii" + ) == _skill("legacy-skill") + if before_external is not None and external_root is not None: + assert_unchanged(before_external, ArtifactSnapshot.capture(external_root)) + _, recovered_audit = _audit( + scenario, + consumer, + environment=source.environment, + scenario_id=f"invalid-legacy-plugin-{invalid_cache}-recovered-audit", + ) + assert recovered_audit["passed"] is True + def test_required_dependency_prune_then_uninstall_cascades_owned_state( tmp_path: Path, @@ -1974,12 +2041,151 @@ def test_required_global_lock_ignores_inactive_experimental_resolver( assert not (mount / "Documents" / "Cowork" / "skills" / "inactive-resolver").exists() +def _exercise_global_revision_commands( + scenario: _Scenario, + source: _PublishedPackage, + commit_a: GitCommit, + run: Callable[..., CommandResult], + capture: Callable[[], LifecycleStateSnapshot], + assert_revision: Callable[[GitCommit, str], None], + artifact_roots: Mapping[str, Path], + compiled_path: Path, + install_args: tuple[str, ...], + manifest_path: Path, +) -> GitCommit: + """Advance one installed workspace, retaining byte and ownership oracles.""" + installed = capture() + assert_revision(commit_a, "a") + before = ArtifactSnapshotSet.capture(artifact_roots) + run(install_args, "global-reinstall-a") + _assert_same_state(installed, capture()) + assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) + for args in ( + ("deps", "list", "--global"), + ("deps", "tree", "--global"), + ("deps", "why", source.package.name, "--global", "--json"), + ("view", source.package.name, "--global"), + ("info", source.package.name, "--global"), + ): + result = run(args, f"global-reader-{'-'.join(args[:2])}") + assert source.package.name in result.stdout, _result_evidence(result) + _assert_same_state(installed, capture()) + assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) + + for args, expected_text in ( + (("deps", "info", source.package.name), source.package.name), + (("cache", "info"), "Git repositories"), + (("cache", "prune", "--days", "30"), "Pruned 0 SHA group(s)"), + ): + result = run( + args, + f"global-compatible-{'-'.join(args[:2])}", + command_cwd=manifest_path.parent, + ) + assert expected_text in result.stdout, _result_evidence(result) + _assert_same_state(installed, capture()) + assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) + # targets observes project markers, not the user-scope deployment inventory. + targets = run(("targets", "--json"), "global-compatible-targets") + target_rows = json.loads(targets.stdout) + assert target_rows and all(row["status"] == "inactive" for row in target_rows) + # find is a project-relative lookup; legacy absolute global records are not supported. + found = run( + ("find", str(compiled_path.parent / "skills" / "global-audit" / "SKILL.md")), + "global-find-absolute-refusal", + command_cwd=manifest_path.parent, + expected_returncode=1, + ) + assert "is not tracked by any installed package" in " ".join(found.stdout.split()), ( + _result_evidence(found) + ) + _assert_same_state(installed, capture()) + assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) + + run(("compile", "--global"), "global-compile-a") + assert "# revision-a" in compiled_path.read_text(encoding="ascii") + compiled_a = compiled_path.read_bytes() + before_lock = capture() + run(("lock", "--global", "--no-policy", "--parallel-downloads", "0"), "global-lock-a") + assert capture().deployment_records == before_lock.deployment_records + assert capture().files == before_lock.files + assert_revision(commit_a, "a") + locked_a = capture() + export_a = run(("lock", "export", "--global"), "global-lock-export-a") + assert json.loads(export_a.stdout)["bomFormat"] == "CycloneDX" + assert commit_a.sha in export_a.stdout + _assert_same_state(locked_a, capture()) + + installed_a = capture() + commit_b = _publish_revision(scenario, source, "b") + outdated = run( + ("outdated", "--global", "--parallel-checks", "0", "--verbose"), "global-outdated-b" + ) + assert commit_b.sha[:7] in outdated.stdout, _result_evidence(outdated) + _assert_same_state(installed_a, capture()) + assert_revision(commit_a, "a") + before = ArtifactSnapshotSet.capture(artifact_roots) + run( + ("update", "--global", "--dry-run", "--parallel-downloads", "0"), + "global-update-preview-b", + ) + _assert_same_state(installed_a, capture()) + assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) + run((*install_args, "--frozen"), "global-frozen-replay-a") + _assert_same_state(installed_a, capture()) + run(("update", "--global", "--yes", "--parallel-downloads", "0"), "global-update-b") + assert_revision(commit_b, "b") + assert capture().deployment_records != installed_a.deployment_records + run(("compile", "--global"), "global-compile-b") + assert "# revision-b" in compiled_path.read_text(encoding="ascii") + assert "# revision-a" not in compiled_path.read_text(encoding="ascii") + assert compiled_path.read_bytes() != compiled_a + installed_b = capture() + export_b = run(("lock", "export", "--global", "--format", "spdx"), "global-lock-export-b") + assert json.loads(export_b.stdout)["spdxVersion"].startswith("SPDX-") + assert commit_b.sha in export_b.stdout + _assert_same_state(installed_b, capture()) + for args, scenario_id in ( + (("compile", "--global", "--dry-run"), "global-compile-preview-b"), + (("compile", "--global"), "global-compile-noop-b"), + ((*install_args, "--dry-run"), "global-install-preview-b"), + ((*install_args, "--update"), "global-install-update-noop-b"), + (("deps", "update", "--global", "--parallel-downloads", "0"), "global-deps-update-noop-b"), + ( + ("lock", "--global", "--no-policy", "--parallel-downloads", "0", "--update"), + "global-lock-update-b", + ), + ): + run(args, scenario_id) + _assert_same_state(installed_b, capture()) + run((*install_args, "--frozen"), "global-frozen-replay-b") + _assert_same_state(installed_b, capture()) + + manifest_bytes = manifest_path.read_bytes() + manifest = load_yaml(manifest_path) + manifest["dependencies"]["apm"][0]["ref"] = commit_a.sha + dump_yaml(manifest, manifest_path) + stale = capture() + run((*install_args, "--frozen"), "global-frozen-refusal", expected_returncode=1) + _assert_same_state(stale, capture()) + manifest_path.write_bytes(manifest_bytes) + before = ArtifactSnapshotSet.capture(artifact_roots) + run(install_args, "global-reinstall-b") + _assert_same_state(installed_b, capture()) + assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) + return commit_b + + +@pytest.mark.parametrize("aliased_home", [False, True]) def test_required_global_audit_rule_matrix_for_external_roots( + aliased_home: bool, tmp_path: Path, apm_binary_path: Path, ) -> None: scenario = _new_scenario(tmp_path / "global-audit-matrix", apm_binary_path) source = _publish(scenario, "global-audit-kit", skill="global-audit") + commit_a = _publish_revision(scenario, source, "a") + dependency = {**source.dependency, "ref": "main"} cwd = scenario.isolated.work_root targets = ("claude", "hermes") external_roots = {target: scenario.isolated.root / f"{target}-home" for target in targets} @@ -1995,46 +2201,106 @@ def test_required_global_audit_rule_matrix_for_external_roots( sentinel_path.write_bytes(f"{target}-owned-by-user\n".encode("ascii")) environment = dict(source.environment) + if aliased_home: + alias = scenario.isolated.root / "home-alias" + try: + alias.symlink_to(scenario.isolated.home, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("directory symlinks unavailable") + environment["HOME"] = str(alias) + environment["USERPROFILE"] = str(alias) + environment["APM_HOME"] = str(alias / ".apm") for target, root in external_roots.items(): environment[_EXTERNAL_USER_ROOT_ENV[target]] = str(root) skill_paths = { target: PurePosixPath(_skill_deploy_path(target, "global-audit")) for target in targets } - snapshot_paths = {target: (sentinel_paths[target], skill_paths[target]) for target in targets} + snapshot_paths = { + target: (sentinel_paths[target], skill_paths[target], PurePosixPath("CLAUDE.md")) + for target in targets + } capture_roots = _external_root_specs(external_roots, config_paths=snapshot_paths) + physical_apm_home = scenario.isolated.config_root + artifact_roots = { + "home": scenario.isolated.home, + "caller": cwd, + **external_roots, + } + home_sentinel = scenario.isolated.home / "user-owned.txt" + home_sentinel.write_bytes(b"home-user-owned\n") + caller_sentinel = cwd / "user-owned.txt" + caller_sentinel.write_bytes(b"caller-user-owned\n") + + def capture() -> LifecycleStateSnapshot: + return LifecycleStateSnapshot.capture(physical_apm_home, external_roots=capture_roots) + + def run( + args: tuple[str, ...], + scenario_id: str, + *, + expected_returncode: int = 0, + command_cwd: Path | None = None, + ) -> CommandResult: + before = ArtifactSnapshotSet.capture(artifact_roots) + result = scenario.runner.run( + args, scenario_id=scenario_id, cwd=command_cwd or cwd, env=environment + ) + assert result.returncode == expected_returncode, _result_evidence(result) + assert_snapshot_changes_within( + before, + ArtifactSnapshotSet.capture(artifact_roots), + exact_paths={ + "home": {".local", ".local/state", ".local/state/gh", ".local/state/gh/device-id"}, + **{ + target: { + "skills", + "rules", + "rules/revision.md", + "CLAUDE.md", + "AGENTS.md", + ".apm", + } + for target in targets + }, + }, + tree_prefixes={ + "home": {".apm"}, + **{target: {"skills/global-audit", ".apm"} for target in targets}, + }, + ) + assert home_sentinel.read_bytes() == b"home-user-owned\n" + assert caller_sentinel.read_bytes() == b"caller-user-owned\n" + for target in targets: + assert (external_roots[target] / sentinel_paths[target]).read_bytes() == ( + f"{target}-owned-by-user\n".encode("ascii") + ) + if aliased_home: + assert alias.is_symlink() + assert alias.readlink() == scenario.isolated.home + return result + scenario.isolated.config_root.mkdir(parents=True, exist_ok=True) dump_yaml( { "name": "global-audit-consumer", "version": "0.1.0", - "dependencies": {"apm": [source.dependency]}, + "dependencies": {"apm": [dependency]}, "targets": list(targets), }, scenario.isolated.config_root / "apm.yml", ) - install = scenario.runner.run( - ( - "install", - "--global", - "--no-policy", - "--parallel-downloads", - "0", - ), - scenario_id="global-audit-install", - cwd=cwd, - env=environment, - ) - assert install.returncode == 0, _result_evidence(install) - installed = LifecycleStateSnapshot.capture(cwd, external_roots=capture_roots) + install_args = ("install", "--global", "--no-policy", "--parallel-downloads", "0") + run(install_args, "global-audit-install") + installed = capture() installed_home = LifecycleStateSnapshot.capture( cwd, external_roots=(_apm_home_root(scenario),), ) assert installed_home.file("apm.lock.yaml", root_id="apm-home").kind == "file" - source_skill_bytes = _skill("global-audit").encode() + source_skill_bytes = (_skill("global-audit") + "\nrevision-a\n").encode() for target in targets: assert ( installed.file(skill_paths[target].as_posix(), root_id=f"{target}-home").content @@ -2044,6 +2310,42 @@ def test_required_global_audit_rule_matrix_for_external_roots( sentinel_paths[target].as_posix(), root_id=f"{target}-home" ).content == f"{target}-owned-by-user\n".encode("ascii") + lock_path = physical_apm_home / "apm.lock.yaml" + + def assert_revision(commit: GitCommit, revision: str) -> None: + lock = LockFile.read(lock_path) + assert lock is not None + dependencies = lock.get_package_dependencies() + assert len(dependencies) == 1 + assert dependencies[0].resolved_commit == commit.sha + assert dependencies[0].content_hash + modules = physical_apm_home / "apm_modules" + assert ( + modules / _OWNER / source.package.name / "skills" / "global-audit" / "SKILL.md" + ).is_file() + assert not (modules / source.package.name).exists() + for target in targets: + assert (external_roots[target] / skill_paths[target]).read_bytes() == ( + _skill("global-audit") + f"\nrevision-{revision}\n" + ).encode() + assert capture().deployment_records + + manifest_path = physical_apm_home / "apm.yml" + + commit_b = _exercise_global_revision_commands( + scenario, + source, + commit_a, + run, + capture, + assert_revision, + artifact_roots, + external_roots["claude"] / "CLAUDE.md", + install_args, + manifest_path, + ) + installed = capture() + def audit_row( scenario_id: str, *, @@ -2052,6 +2354,8 @@ def audit_row( ) -> dict[str, object]: if expected_returncode is None: expected_returncode = 1 if failed else 0 + before = ArtifactSnapshotSet.capture(artifact_roots) + state_before = capture() _, payload = _audit_at( scenario, scenario.isolated.config_root, @@ -2060,6 +2364,8 @@ def audit_row( scenario_id=scenario_id, ) _assert_global_audit_rules(payload, failed=failed) + _assert_same_state(state_before, capture()) + assert_snapshot_set_unchanged(before, ArtifactSnapshotSet.capture(artifact_roots)) return payload def assert_clean(scenario_id: str) -> dict[str, object]: @@ -2071,7 +2377,7 @@ def assert_clean(scenario_id: str) -> dict[str, object]: clean_audit = assert_clean("global-audit-clean") assert _check(clean_audit, "deployed-files-present")["passed"] is True assert _check(clean_audit, "content-integrity")["passed"] is True - post_clean_audit = LifecycleStateSnapshot.capture(cwd, external_roots=capture_roots) + post_clean_audit = capture() _assert_same_state(installed, post_clean_audit) claude_skill = external_roots["claude"] / skill_paths["claude"] @@ -2200,13 +2506,23 @@ def assert_clean(scenario_id: str) -> dict[str, object]: hermes_skill.write_bytes(hermes_skill_bytes) assert_clean("global-audit-after-combo-restore") - uninstall = scenario.runner.run( + before_clean = capture() + run(("cache", "clean", "--yes"), "global-cache-clean") + _assert_same_state(before_clean, capture()) + run(("deps", "clean", "--dry-run"), "global-deps-clean-preview", command_cwd=physical_apm_home) + _assert_same_state(before_clean, capture()) + run(("deps", "clean", "--yes"), "global-deps-clean", command_cwd=physical_apm_home) + assert not modules_dir.exists() + assert capture().deployment_records == before_clean.deployment_records + audit_row("global-audit-after-deps-clean", failed={"config-consistency", "drift"}) + run(install_args, "global-rehydrate-after-deps-clean") + assert_revision(commit_b, "b") + assert_clean("global-audit-after-rehydrate") + + run( ("uninstall", "--global", source.remote_url), - scenario_id="global-audit-uninstall", - cwd=cwd, - env=environment, + "global-audit-uninstall", ) - assert uninstall.returncode == 0, _result_evidence(uninstall) removed = LifecycleStateSnapshot.capture(cwd, external_roots=capture_roots) for target in targets: assert ( @@ -2229,6 +2545,23 @@ def assert_clean(scenario_id: str) -> dict[str, object]: ) assert final_audit["passed"] is True assert final_audit["summary"]["failed"] == 0 + manifest = load_yaml(manifest_path) + manifest["dependencies"]["apm"] = [dependency] + dump_yaml(manifest, manifest_path) + run(install_args, "global-reinstall-after-removal") + assert_revision(commit_b, "b") + assert_clean("global-audit-reinstalled") + run(("uninstall", "--global", source.remote_url), "global-uninstall-closure") + _, closure = _audit_at( + scenario, + physical_apm_home, + environment=environment, + scenario_id="global-audit-closure", + ) + assert closure["passed"] is True + assert not lock_path.exists() + for target in targets: + assert not (external_roots[target] / skill_paths[target]).exists() def test_required_failed_lock_write_bounds_partial_state_and_recovers( diff --git a/tests/spec_conformance/test_lockfile_reqs.py b/tests/spec_conformance/test_lockfile_reqs.py index b1d7b08273..390e984bba 100644 --- a/tests/spec_conformance/test_lockfile_reqs.py +++ b/tests/spec_conformance/test_lockfile_reqs.py @@ -87,6 +87,32 @@ def test_lockfile_v1_remains_parseable_under_v2_reader(): validate_against("lockfile-v0.1.schema.json", load_yaml_fixture(*V1)) +@pytest.mark.req("req-lk-003") +def test_frozen_manifest_pin_requires_the_exact_locked_commit(): + from apm_cli.deps.lockfile import LockedDependency, LockFile + from apm_cli.install.plan import lockfile_satisfies_manifest + from apm_cli.models.dependency import DependencyReference + + pin = "abcdef0123456789" * 2 + "abcdef01" + declared = DependencyReference.parse(f"fixture/frozen-pin#{pin}") + entry = LockedDependency( + repo_url=declared.repo_url, resolved_ref=pin, resolved_commit=pin.upper() + ) + lock = LockFile(dependencies={declared.get_unique_key(): entry}) + before = lock.to_yaml() + assert lockfile_satisfies_manifest(lock, [declared]) == (True, []) + assert lock.to_yaml() == before + + entry.resolved_commit = "0123456789abcdef" * 2 + "01234567" + mismatched = lock.to_yaml() + satisfied, reasons = lockfile_satisfies_manifest(lock, [declared]) + assert satisfied is False + assert len(reasons) == 1 + assert "manifest commit" in reasons[0] + assert "lockfile resolved_commit" in reasons[0] + assert lock.to_yaml() == mismatched + + @pytest.mark.req("req-lk-005") def test_lockfile_dependency_carries_resolved_field(): schema = load_schema("lockfile-v0.1.schema.json") diff --git a/tests/unit/deps/test_tiered_ref_resolver.py b/tests/unit/deps/test_tiered_ref_resolver.py index cc86ce9b30..af8b387604 100644 --- a/tests/unit/deps/test_tiered_ref_resolver.py +++ b/tests/unit/deps/test_tiered_ref_resolver.py @@ -18,6 +18,7 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "..", "src")) from apm_cli.cache.url_normalize import cache_shard_key +from apm_cli.deps.lockfile import LockFile from apm_cli.deps.tiered_ref_resolver import ( L0PerRunCache, L1CommitsAPI, @@ -29,6 +30,7 @@ _repository_cache_identity, build_tiered_ref_resolver, is_tiered_resolver_enabled, + ref_freshness_policy_for_install, ) from apm_cli.models.dependency.reference import DependencyReference from apm_cli.models.dependency.types import GitReferenceType, ResolvedReference @@ -68,6 +70,27 @@ def test_freshness_policy_maps_install_intent_once(update_refs, refresh, expecte assert policy.allows_bare_cache is (expected is RefFreshnessPolicy.REPRODUCIBLE) +@pytest.mark.parametrize("has_lock", [False, True]) +@pytest.mark.parametrize("update_refs", [False, True]) +def test_install_freshness_requires_remote_without_replayable_lock( + has_lock: bool, update_refs: bool +) -> None: + context = types.SimpleNamespace( + ref_freshness_policy=None, + update_refs=update_refs, + refresh=False, + existing_lockfile=LockFile() if has_lock else None, + ) + policy = ref_freshness_policy_for_install(context) + assert policy is ( + RefFreshnessPolicy.REPRODUCIBLE + if has_lock and not update_refs + else RefFreshnessPolicy.CURRENT_REMOTE + ) + context.ref_freshness_policy = RefFreshnessPolicy.CURRENT_REMOTE + assert ref_freshness_policy_for_install(context) is RefFreshnessPolicy.CURRENT_REMOTE + + @pytest.mark.parametrize( "value,expected", [ diff --git a/tests/unit/install/test_frozen.py b/tests/unit/install/test_frozen.py index 259f1dae21..29593d1daa 100644 --- a/tests/unit/install/test_frozen.py +++ b/tests/unit/install/test_frozen.py @@ -72,7 +72,7 @@ def test_succeeds_when_lockfile_has_all_manifest_deps(self, tmp_path: Path): ), ], ) - dep = DependencyReference(repo_url="https://github.com/o/r") + dep = DependencyReference(repo_url="https://github.com/o/r", reference="main") req = _make_request(project_dir=tmp_path, manifest_deps=[dep]) InstallService.enforce_frozen(req) @@ -97,7 +97,7 @@ def test_orphan_lockfile_entries_dont_fail(self, tmp_path: Path): ), ], ) - dep = DependencyReference(repo_url="https://github.com/o/r") + dep = DependencyReference(repo_url="https://github.com/o/r", reference="main") req = _make_request(project_dir=tmp_path, manifest_deps=[dep]) InstallService.enforce_frozen(req) @@ -118,7 +118,7 @@ def test_mixed_project_rejects_stale_locked_mcp_state(self, tmp_path: Path): lock.mcp_servers = ["stale-mcp"] lock.mcp_configs = {"stale-mcp": {"name": "stale-mcp"}} lock.save(tmp_path / "apm.lock.yaml") - dep = DependencyReference(repo_url="https://github.com/o/r") + dep = DependencyReference(repo_url="https://github.com/o/r", reference="main") req = _make_request(project_dir=tmp_path, manifest_deps=[dep]) req.apm_package.get_all_mcp_dependencies.return_value = [] current = CurrentMcpConfigView( @@ -217,7 +217,7 @@ def test_cold_cache_git_apm_package_dep_passes(self, tmp_path: Path) -> None: lock.mcp_configs = {"root-mcp": {"name": "root-mcp"}} lock.save(tmp_path / "apm.lock.yaml") - manifest_dep = DependencyReference(repo_url="owner/some-pkg") + manifest_dep = DependencyReference(repo_url="owner/some-pkg", reference="v1.0.0") req = _make_request(project_dir=tmp_path, manifest_deps=[manifest_dep]) # Root manifest declares the MCP server directly. from apm_cli.models.dependency.mcp import MCPDependency @@ -246,7 +246,7 @@ def test_cold_cache_git_apm_package_with_own_mcp_passes(self, tmp_path: Path) -> lock.mcp_config_provenance = {"pkg-mcp": dep_name} lock.save(tmp_path / "apm.lock.yaml") - manifest_dep = DependencyReference(repo_url="owner/some-pkg") + manifest_dep = DependencyReference(repo_url="owner/some-pkg", reference="v1.0.0") req = _make_request(project_dir=tmp_path, manifest_deps=[manifest_dep]) req.apm_package.get_all_mcp_dependencies.return_value = [] @@ -275,7 +275,7 @@ def test_cold_cache_does_not_mask_real_mcp_drift_from_installed_dep( lock.mcp_config_provenance = {} lock.save(tmp_path / "apm.lock.yaml") - manifest_dep = DependencyReference(repo_url="owner/some-pkg") + manifest_dep = DependencyReference(repo_url="owner/some-pkg", reference="v1.0.0") req = _make_request(project_dir=tmp_path, manifest_deps=[manifest_dep]) req.apm_package.get_all_mcp_dependencies.return_value = [] current = CurrentMcpConfigView( @@ -324,7 +324,7 @@ def test_co_owned_server_not_exempted_by_cold_cache(self, tmp_path: Path) -> Non } lock.save(tmp_path / "apm.lock.yaml") - manifest_dep = DependencyReference(repo_url="owner/some-pkg") + manifest_dep = DependencyReference(repo_url="owner/some-pkg", reference="v1.0.0") req = _make_request(project_dir=tmp_path, manifest_deps=[manifest_dep]) req.apm_package.get_all_mcp_dependencies.return_value = [] current = CurrentMcpConfigView( @@ -374,7 +374,7 @@ def verbose_detail(self, message: str) -> None: lock.mcp_config_provenance = {"pkg-mcp": dep_name} lock.save(tmp_path / "apm.lock.yaml") - manifest_dep = DependencyReference(repo_url="owner/some-pkg") + manifest_dep = DependencyReference(repo_url="owner/some-pkg", reference="v1.0.0") recording_logger = _RecordingInstallLogger() pkg = MagicMock() pkg.package_path = tmp_path / "apm.yml" diff --git a/tests/unit/install/test_plan.py b/tests/unit/install/test_plan.py index f4c1c024bb..53fec03ca2 100644 --- a/tests/unit/install/test_plan.py +++ b/tests/unit/install/test_plan.py @@ -9,6 +9,8 @@ from __future__ import annotations +import pytest + from apm_cli.deps.lockfile import LockedDependency, LockFile from apm_cli.install.plan import ( PlanEntry, @@ -684,7 +686,7 @@ def test_satisfied_when_private_git_dep_lock_key_is_host_qualified(self): depth=1, ) ) - manifest = [DependencyReference.parse("git@git.example.com:org/private-skills.git")] + manifest = [DependencyReference.parse("git@git.example.com:org/private-skills.git#main")] ok, reasons = lockfile_satisfies_manifest(lock, manifest) @@ -720,7 +722,7 @@ def test_satisfied_when_github_git_dep_uses_default_host_key(self): depth=1, ) ) - manifest = [DependencyReference.parse("git@github.com:org/public-skills.git")] + manifest = [DependencyReference.parse("git@github.com:org/public-skills.git#main")] ok, reasons = lockfile_satisfies_manifest(lock, manifest) @@ -737,6 +739,35 @@ def test_unsatisfied_when_manifest_dep_missing_from_lock(self): assert len(reasons) == 1 assert "missing" in reasons[0] + @pytest.mark.parametrize( + ("manifest_ref", "locked_ref", "commit", "satisfied"), + [ + (None, None, "a" * 40, True), + ("main", "main", "a" * 40, True), + ("release", "main", "a" * 40, False), + (None, "main", "a" * 40, False), + ("main", None, "a" * 40, False), + ("a" * 40, "a" * 40, "a" * 40, True), + ("A" * 40, "A" * 40, "a" * 40, True), + ("a" * 40, "a" * 40, "b" * 40, False), + ], + ) + def test_frozen_compares_declared_identity_without_resolving_upstream( + self, manifest_ref: str | None, locked_ref: str | None, commit: str, satisfied: bool + ) -> None: + lock = _new_lockfile() + lock.add_dependency( + LockedDependency( + repo_url="owner/package", resolved_ref=locked_ref, resolved_commit=commit + ) + ) + manifest = [DependencyReference(repo_url="owner/package", reference=manifest_ref)] + before = lock.to_yaml() + ok, reasons = lockfile_satisfies_manifest(lock, manifest) + assert ok is satisfied + assert bool(reasons) is not satisfied + assert lock.to_yaml() == before + def test_local_deps_skipped(self): """Local file deps have no remote ref, so they're skipped.""" lock = _new_lockfile() diff --git a/tests/unit/install/test_security_scan_scope.py b/tests/unit/install/test_security_scan_scope.py index f89c874665..fee6c61d0d 100644 --- a/tests/unit/install/test_security_scan_scope.py +++ b/tests/unit/install/test_security_scan_scope.py @@ -7,6 +7,7 @@ import pytest +from apm_cli.deps.plugin_parser import normalize_plugin_directory from apm_cli.install.deployable_source_plan import DeployableSourcePlan from apm_cli.install.helpers.security_scan import _pre_deploy_security_scan from apm_cli.integration.agent_integrator import AgentIntegrator @@ -14,7 +15,7 @@ from apm_cli.integration.hook_integrator import HookIntegrator from apm_cli.integration.instruction_integrator import InstructionIntegrator from apm_cli.integration.prompt_integrator import PromptIntegrator -from apm_cli.integration.skill_integrator import copy_skill_to_target +from apm_cli.integration.skill_integrator import SkillIntegrator, copy_skill_to_target from apm_cli.integration.targets import KNOWN_TARGETS from apm_cli.models.apm_package import APMPackage, PackageInfo, PackageType from apm_cli.security.gate import SecurityGate @@ -205,6 +206,81 @@ def test_source_only_canvas_content_is_not_authorized_for_scan(tmp_path: Path) - ) == ["source-only"] +@pytest.mark.parametrize( + ("layout", "package_type"), + [ + (".", PackageType.CLAUDE_SKILL), + ("skills/demo", PackageType.SKILL_BUNDLE), + (".apm/skills/demo", PackageType.APM_PACKAGE), + ("skills/demo", PackageType.MARKETPLACE_PLUGIN), + ], +) +def test_package_skill_root_alias_preserves_authorization( + tmp_path: Path, layout: str, package_type: PackageType +) -> None: + """Root aliases deploy real files without authorizing descendant links.""" + source = tmp_path / "demo" + skill = source / layout + skill.mkdir(parents=True) + alias = tmp_path / "source-alias" + try: + alias.symlink_to(source, target_is_directory=True) + except (OSError, NotImplementedError): + pytest.skip("directory symlinks unavailable") + if package_type is PackageType.MARKETPLACE_PLUGIN: + manifest = source / ".claude-plugin" / "plugin.json" + manifest.parent.mkdir() + manifest.write_text('{"name":"demo","skills":["./skills/demo"]}\n', encoding="utf-8") + (skill / "SKILL.md").write_text("# Demo\n", encoding="utf-8") + references = skill / "references" + references.mkdir() + (references / "safe.md").write_text("safe resource\n", encoding="utf-8") + if package_type is PackageType.MARKETPLACE_PLUGIN: + normalize_plugin_directory(source, plugin_json_path=manifest) + outside = tmp_path / "outside" + outside.mkdir() + secret = outside / "secret.md" + secret.write_text("outside resource\n", encoding="utf-8") + (references / "escape.md").symlink_to(secret) + (references / "internal-link.md").symlink_to(references / "safe.md") + (skill / "linked-directory").symlink_to(outside, target_is_directory=True) + project = tmp_path / "project" + project.mkdir() + package_info = PackageInfo( + package=APMPackage(name="demo", version="1.0.0"), + install_path=alias, + package_type=package_type, + ) + original_metadata = vars(package_info).copy() + targets = [KNOWN_TARGETS["codex"]] + plan = DeployableSourcePlan.create( + package_info, + targets, + skill_subset=None, + hooks_approved=False, + canvas_approved=False, + skip_bin=True, + ) + + result = SkillIntegrator().integrate_package_skill( + package_info, + project, + targets=targets, + source_plan=plan, + skip_bin=True, + ) + + assert vars(package_info) == original_metadata + assert len(result.target_paths) == 1 + deployed = result.target_paths[0] + assert (deployed / "SKILL.md").read_text(encoding="utf-8") == "# Demo\n" + assert (deployed / "references" / "safe.md").read_text(encoding="utf-8") == "safe resource\n" + assert not (deployed / "references" / "escape.md").exists() + assert not (deployed / "references" / "internal-link.md").exists() + assert not (deployed / "linked-directory").exists() + assert not plan.includes((Path(layout) / "references" / "escape.md").as_posix()) + + @pytest.mark.windows_compat def test_direct_skill_copy_normalizes_equivalent_source_alias( tmp_path: Path, diff --git a/tests/utils/lifecycle_state.py b/tests/utils/lifecycle_state.py index 93135afa13..d313cb0449 100644 --- a/tests/utils/lifecycle_state.py +++ b/tests/utils/lifecycle_state.py @@ -161,10 +161,16 @@ def capture( roles_by_path: dict[tuple[str, str], set[LifecycleFileRole]] = {} for record in records: if record.locator.kind is LocatorKind.PROJECT_RELATIVE: + if Path(record.locator.value).is_absolute(): + root_id, relative_path = _bounded_legacy_absolute_path( + record, root, normalized_roots + ) + else: + root_id, relative_path = _WORKSPACE_ROOT_ID, record.locator.value _add_role( roles_by_path, - _WORKSPACE_ROOT_ID, - record.locator.value, + root_id, + relative_path, "deployment", ) elif record.locator.kind is LocatorKind.TARGET_RELATIVE: @@ -306,6 +312,31 @@ def file( ) +def _bounded_legacy_absolute_path( + record: DeploymentRecord, + workspace_root: Path, + external_roots: tuple[LifecycleStateRoot, ...], +) -> tuple[str, str]: + """Observe legacy global locators without resolving descendant links.""" + candidate = Path(record.locator.value) + roots = [ + (_WORKSPACE_ROOT_ID, workspace_root), + *( + (external.root_id, external.path) + for external in external_roots + if external.target == record.locator.target + ), + ] + for root_id, root_path in roots: + if candidate.is_relative_to(root_path): + relative = candidate.relative_to(root_path).as_posix() + _validate_relative_state_path(relative) + return root_id, relative + raise ValueError( + f"Lifecycle snapshot has no bounded root for absolute deployment {record.locator.value!r}" + ) + + def _add_role( roles_by_path: dict[tuple[str, str], set[LifecycleFileRole]], root_id: str,