diff --git a/.apm/architecture/owners/transport-auth-platform.json b/.apm/architecture/owners/transport-auth-platform.json index b731c814e..119fc9b03 100644 --- a/.apm/architecture/owners/transport-auth-platform.json +++ b/.apm/architecture/owners/transport-auth-platform.json @@ -60,6 +60,13 @@ "selectors": ["src/apm_cli/deps/tiered_ref_resolver.py"], "guards": ["transport-platform-ref-freshness"] }, + { + "id": "revision-pin-update-outcome", + "decision": "Revision-pin update outcome (updates vs retained SHA pins)", + "owner": "deps/revision_pins.py (RevisionPinResolutionResult)", + "selectors": ["src/apm_cli/deps/revision_pins.py"], + "guards": ["transport-platform-revision-pin-outcome"] + }, { "id": "git-semver-preflight-resolution", "decision": "Git semver preflight eligibility and resolution", diff --git a/CHANGELOG.md b/CHANGELOG.md index f8db6e63d..f873311ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 must run `apm pack` separately; see [Releasing from any CI](docs/src/content/docs/producer/releasing-from-any-ci.md#the-canonical-sequence). (by @danielmeppiel, closes #2727, #2730) +- `apm update` now retains full-SHA pins without an eligible stable annotated + semver tag while continuing unrelated updates; malformed remote tag records + still fail before writes. The contract is recorded in `openapm-v0.1.md`. + (#2667) - Distributed `apm compile` now reconciles existing managed-section `AGENTS.md` files without overwriting hand-authored content, generates new placements safely, and never discovers, writes, or cleans content across diff --git a/CONFORMANCE.json b/CONFORMANCE.json index 2d3c9168a..7e07ac758 100644 --- a/CONFORMANCE.json +++ b/CONFORMANCE.json @@ -1045,6 +1045,31 @@ "tests/spec_conformance/test_resolution_reqs.py::test_resolver_cache_preserves_complete_repository_identity" ] }, + { + "conformance_class": "consumer", + "id": "req-rs-017", + "keyword": "MUST", + "section": "7.7", + "status": "active", + "test_count": 15, + "tests": [ + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_accepts_each_normative_tag_form[1.2.3]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_accepts_each_normative_tag_form[revision-pin--v1.2.3]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_accepts_each_normative_tag_form[revision-pin-v1.2.3]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_accepts_each_normative_tag_form[v1.2.3]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_derives_repository_and_virtual_names", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_projection_and_scope_are_normative", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_rejects_malformed_tag_records[0000000000000000000000000000000000000000\\trefs/tags/v2.0.0]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_rejects_malformed_tag_records[\\n]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_rejects_malformed_tag_records[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\trefs/tags/bad name]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_rejects_malformed_tag_records[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\trefs/tags/v2.0.0 ]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_rejects_malformed_tag_records[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\trefs/tags/v2.0.0\\naaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\trefs/tags/v2.0.0]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_rejects_malformed_tag_records[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\trefs/tags/v2.0.0^{}]", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_retains_missing_release", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_selects_highest_stable_tag_deterministically", + "tests/spec_conformance/test_resolution_reqs.py::test_revision_pin_update_wraps_transport_parse_failure" + ] + }, { "conformance_class": "consumer", "id": "req-sc-001", @@ -1368,7 +1393,7 @@ "spec_version": "v0.1.1", "summary_by_class": { "consumer": { - "active": 88, + "active": 89, "skipped": 1, "unbound": 0, "xfail": 0 @@ -1392,5 +1417,5 @@ "xfail": 0 } }, - "total_requirements": 119 + "total_requirements": 120 } diff --git a/CONFORMANCE.md b/CONFORMANCE.md index 0b201f575..4fca09248 100644 --- a/CONFORMANCE.md +++ b/CONFORMANCE.md @@ -19,7 +19,7 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac | Class | Active | Skipped | Xfail | Unbound | |-------|-------:|--------:|------:|--------:| | Producer | 12 | 0 | 0 | 0 | -| Consumer | 88 | 1 | 0 | 0 | +| Consumer | 89 | 1 | 0 | 0 | | Registry | 1 | 0 | 0 | 0 | | Governance | 17 | 0 | 0 | 0 | @@ -118,6 +118,7 @@ All four conformance classes (Producer, Consumer, Registry, Governance) carry ac | [req-rs-014](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-014) | MUST | 7.3.1 | consumer | active | 1 | - | | [req-rs-015](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-015) | MUST | 7.5 | consumer | active | 1 | - | | [req-rs-016](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-016) | MUST | 7.2 | consumer | active | 7 | - | +| [req-rs-017](docs/src/content/docs/specs/openapm-v0.1.md#req-rs-017) | MUST | 7.7 | consumer | active | 15 | - | | [req-sc-001](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-001) | MUST | 10.4 | consumer | active | 2 | - | | [req-sc-002](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-002) | MUST | 10.9 | consumer | active | 1 | - | | [req-sc-003](docs/src/content/docs/specs/openapm-v0.1.md#req-sc-003) | MUST | 10.3 | consumer | active | 1 | - | diff --git a/docs/public/specs/manifests/openapm-v0.1.requirements.yml b/docs/public/specs/manifests/openapm-v0.1.requirements.yml index b6057f63a..d4e0c3bd8 100644 --- a/docs/public/specs/manifests/openapm-v0.1.requirements.yml +++ b/docs/public/specs/manifests/openapm-v0.1.requirements.yml @@ -356,6 +356,11 @@ requirements: section: "7.2" conformance_class: consumer notes: "minimum repository identity binds every cache layer; distinct identities must not share source material" + - id: req-rs-017 + keyword: MUST + section: "7.7" + conformance_class: consumer + notes: "full-SHA update extensions deterministically select the highest eligible non-prerelease annotated tag, retain pins with no eligible tag, persist git-literal provenance, and stop before writes on malformed or failed remote resolution" - id: req-pr-001 keyword: MUST section: "8.2" diff --git a/docs/src/content/docs/reference/cli/update.md b/docs/src/content/docs/reference/cli/update.md index d6f6ac4d4..a1e2a72ef 100644 --- a/docs/src/content/docs/reference/cli/update.md +++ b/docs/src/content/docs/reference/cli/update.md @@ -110,6 +110,13 @@ apm update - **No-op and service-only repair.** An accepted update with no dependency ref changes still reconciles missing MCP/LSP config. A manifest with only MCP/LSP dependencies also uses `apm update` as a configuration repair pass; `--dry-run` previews this without writing. - **Empty caches are restored.** If the lockfile expects dependencies but `apm_modules/` has no materialized packages, an otherwise unchanged update restores the cache from the same refs and reports `Restored dependency cache without changing refs.` No confirmation is required because dependency refs do not move. +### Missing annotated revision-pin tags + +When a revision-pinned dependency has no eligible stable annotated semver tag, +APM emits one summary warning and retains its current SHA while continuing with +unrelated updates. Use `--verbose` to list each retained pin. Transport failures +and malformed or ambiguous tag records still fail the update before writes. + ## Back-compat: `apm update` used to be the self-updater In earlier releases, `apm update` self-updated the **APM CLI binary**. That behavior moved to [`apm self-update`](../self-update/) and `apm update` was repurposed as the dependency updater described above. diff --git a/docs/src/content/docs/specs/openapm-v0.1.md b/docs/src/content/docs/specs/openapm-v0.1.md index 957762632..9e3cdd3f4 100644 --- a/docs/src/content/docs/specs/openapm-v0.1.md +++ b/docs/src/content/docs/specs/openapm-v0.1.md @@ -136,7 +136,7 @@ between the companion corpus and the implementation. ### 1.3 Document conventions -- OpenAPM v0.1 carries **119 normative statements** indexed in +- OpenAPM v0.1 carries **120 normative statements** indexed in [Appendix C](#appendix-c-index-of-normative-statements). - All on-disk files defined by this specification are **YAML 1.2** parsed under the safe subset defined in @@ -866,8 +866,8 @@ unknown fields on round-trip. Field availability is **monotonic** in | `local_path` | Original path for local deps. | | `content_hash` | Hash envelope (`sha256:`) of a local package's source tree. | | `is_dev` | True when declared under `devDependencies`. | -| `constraint` | git-semver: the original semver range from the manifest (verbatim). | -| `resolved_tag` | git-semver: the literal tag the range resolved to. | +| `constraint` | git-semver discriminator: the original semver range from the manifest (verbatim). | +| `resolved_tag` | git-semver selected tag, or advisory tag provenance for a full-SHA git-literal update under [req-rs-017](#req-rs-017). | | `resolved_at` | git-semver: ISO 8601 UTC timestamp; advisory (see [Section 7.3](#73-git-semver-resolution)). | | `name` | Self-asserted display/inventory name; non-identity (see [req-lk-019](#req-lk-019)). | | `attestations` | Reserved for v0.2 (publisher provenance). | @@ -1206,6 +1206,10 @@ When the resolver picks a git tag from a semver range (see [Section 7.3](#73-git-semver-resolution)), it records three additional fields on the resolved entry. These fields are valid in both `lockfile_version: "1"` and `"2"` (see [req-lk-002](#req-lk-002)). +The presence of `constraint` identifies the git-semver shape. A +git-literal entry updated under [req-rs-017](#req-rs-017) has no +`constraint`; its optional `resolved_tag` is advisory provenance and +does not change replay or trust semantics. **[req-lk-008]** A conforming **consumer** implementation MUST @@ -1983,14 +1987,15 @@ re-resolution. **[req-rs-015]** A conforming **consumer** implementation performing a non-update install (that is, not an `apm update` and not an explicit `--refresh`/re-resolution invocation) MUST replay a lockfile entry -that records a `resolved_commit` without a corresponding `resolved_tag` -(that is, git-literal and untagged-branch entries per -[req-rs-003](#req-rs-003)) by reusing that recorded commit as the +that records a `resolved_commit` for a git-literal or untagged-branch +entry per [req-rs-003](#req-rs-003) by reusing that recorded commit as the resolution result WITHOUT issuing a network ref-resolution -- no commits-API query, no `git ls-remote`, and no clone for ref discovery (illustrative, not exhaustive) -- for that entry, provided drift detection against the manifest reference does not require -re-resolution. Object fetch to materialise content at the +re-resolution. An advisory `resolved_tag` recorded under +[req-rs-017](#req-rs-017) does not change this requirement's +applicability. Object fetch to materialise content at the already-resolved commit is not constrained by this requirement. When the manifest reference for the entry has changed so that the recorded pin no longer matches (drift -- defined for entries scoped by this @@ -2066,13 +2071,15 @@ delta from the same inputs. **[req-rs-011]** A conforming **consumer** implementation that exposes an `apm update` (or equivalent) command MUST, when invoked -without a package argument, re-resolve every direct dependency -against its **current** manifest constraint (holding the manifest -unchanged), MUST rewrite the lockfile pins to the new highest -matching version for each direct dep, MUST re-resolve all -transitive dependencies as a side-effect, and MUST honour the -active Governance policy's `require_pinned_constraint` rule -([req-pl-007](#req-pl-007)). +without a package argument, re-resolve every direct dependency. +Dependencies other than full-SHA git-literal entries MUST resolve +against their **current** manifest constraint while leaving that +constraint unchanged, and the consumer MUST rewrite their lockfile +pins to the new highest matching version. A full-SHA git-literal entry +follows [req-rs-017](#req-rs-017) when the consumer offers that update +extension. The consumer MUST re-resolve all transitive dependencies as +a side-effect and MUST honour the active Governance policy's +`require_pinned_constraint` rule ([req-pl-007](#req-pl-007)). **[req-rs-012]** A conforming **consumer** implementation that @@ -2080,7 +2087,64 @@ exposes `apm update ` MUST scope re-resolution to the named package and its subtree only, MUST hold every other resolved entry at its prior pin, and MUST refuse to operate on a frozen install (see [req-lk-006](#req-lk-006)) without an explicit override -flag. +flag. When the named package is a direct full-SHA git-literal entry +and the consumer offers the [req-rs-017](#req-rs-017) extension, that +entry follows req-rs-017; the consumer MUST NOT rewrite a transitive +package manifest. + + +**[req-rs-017]** A conforming **consumer** implementation that offers +an update extension for a git-literal dependency pinned to a full +hexadecimal commit ID MUST form one candidate set from annotated tags +whose peeled object has been verified as a commit and whose parsed +semantic version has an empty pre-release identifier under +[Section 7.3.1](#731-semver-dialect-normative). A `0.x.y` version with +an empty pre-release identifier remains eligible. The consumer MUST NOT +select a branch or lightweight tag. + +A candidate tag name MUST use one of `v{version}`, +`{name}--v{version}`, `{name}-v{version}`, or the bare `{version}`. +For a repository dependency, `name` is the final repository path +component after removing at most one trailing `.git`; for a selected +virtual subdirectory, it is the final non-empty virtual-path component. +The consumer MUST select the candidate with the highest version under +Section 7.3.1 precedence. When candidates have equal precedence, it +MUST apply the bytewise ASCII full-tag-string tie break in +[req-rs-014](#req-rs-014), independently of remote record order. A +winning candidate whose peeled commit equals the current pin is a +no-op. + +When the authoritative upstream contains no eligible tag, the consumer +MUST retain the current commit ID, emit a default-visible diagnostic, +and continue resolving other dependencies in the requested update +scope. A transport failure, an invalid or all-zero object ID, an invalid +tag refname, a duplicate tag ref record, or a peeled tag record without +its base record MUST fail the update before any manifest or lockfile +write. An otherwise eligible annotated tag that peels to a tree, blob, +or other non-commit object is also a fatal outcome, not an ignored +candidate. These failures MUST NOT be converted into the retained-pin +outcome. + +For a successful replacement, the consumer MUST rewrite only the +direct root-manifest entry's `ref` to the selected peeled commit. The +matching lockfile entry MUST record that same commit in `resolved_ref` +and `resolved_commit`, and MUST record the selected tag in +`resolved_tag` as advisory provenance. The tag does not change the +entry's git-literal reference kind and does not become a trust anchor. +A later non-update install MUST replay the full commit without remote +tag enumeration under [req-rs-015](#req-rs-015). A retained-pin outcome +MUST leave the direct manifest entry and its resolved lock values +unchanged. + +A no-argument update applies this rule to every direct full-SHA +git-literal entry. A package-scoped update applies it only to the named +direct entry; ordinary subtree resolution continues, but transitive +package manifests MUST NOT be rewritten. The consumer MUST finish +candidate validation for every scoped direct entry before writing +either the manifest or lockfile. A retained-pin outcome is entry-local +and does not stop unrelated scoped updates. Any fatal outcome named +above aborts the requested update scope and leaves both files +unchanged. Range-widening update modes (for example `apm update --aggressive`, which would mutate the manifest's range upper bounds) are @@ -2183,7 +2247,8 @@ This section's normative statements are: [req-rs-009](#req-rs-009), [req-rs-010](#req-rs-010), [req-rs-011](#req-rs-011), [req-rs-012](#req-rs-012), [req-rs-013](#req-rs-013), [req-rs-014](#req-rs-014), - [req-rs-015](#req-rs-015), [req-rs-016](#req-rs-016). + [req-rs-015](#req-rs-015), [req-rs-016](#req-rs-016), + [req-rs-017](#req-rs-017). - Producer: [req-pr-004](#req-pr-004). - Producer (SHOULD): [req-pr-005](#req-pr-005). @@ -3259,6 +3324,7 @@ conformance statement identifying: [req-rs-011](#req-rs-011), [req-rs-012](#req-rs-012), [req-rs-013](#req-rs-013), [req-rs-014](#req-rs-014), [req-rs-015](#req-rs-015), [req-rs-016](#req-rs-016), +[req-rs-017](#req-rs-017), [req-pr-001](#req-pr-001), [req-pr-002](#req-pr-002), [req-pr-003](#req-pr-003), [req-tg-001](#req-tg-001), [req-pr-006](#req-pr-006), [req-pr-007](#req-pr-007), @@ -3697,6 +3763,7 @@ renumbering of conformance classes. | [req-rs-014](#req-rs-014) | MUST | 7.3.1 | consumer | | [req-rs-015](#req-rs-015) | MUST | 7.5 | consumer | | [req-rs-016](#req-rs-016) | MUST | 7.2 | consumer | +| [req-rs-017](#req-rs-017) | MUST | 7.7 | consumer | | [req-pr-001](#req-pr-001) | MUST | 8.2 | consumer | | [req-pr-002](#req-pr-002) | MUST | 8.3 | consumer | | [req-pr-003](#req-pr-003) | MUST | 8.3 | consumer | @@ -3736,7 +3803,7 @@ renumbering of conformance classes. | [req-cf-001](#req-cf-001) | MUST | 12.5 | consumer | | [req-cf-002](#req-cf-002) | MUST | 12.3 | consumer | -**Total normative statements: 119** (114 MUST, 5 SHOULD). +**Total normative statements: 120** (115 MUST, 5 SHOULD). --- @@ -3782,6 +3849,7 @@ renumbering of conformance classes. | 0.1.34 | 2026-08-25 | Spec-citation fold for root-declared Plugin component staging containment (closes #2556). Added [req-pr-007] (Section 8.1, consumer MUST): a consumer canonicalizes the non-symlink component-source root and prunes the current operation's materialization subtree before traversal. Section 8.7, Section 11.3.2, Appendix C, and conformance coverage updated. Statement count: 117 -> 118 (113 MUST, 5 SHOULD). | | 0.1.35 | 2026-08-27 | Stale-spec (Mode C) amendment recording a machine-verifiable native Agent Plugins lifecycle. Added [req-tg-013] (Section 8.5.7, consumer MUST): schema, effective-target, integrity, security, and executable admission drives one aggregate direct-plus-transitive registration per scope without locating, invoking, or version-checking a host binary during lifecycle operations; packages remain materialized in place and opaque to legacy projection; direct dependencies win plugin-name collisions over transitive dependencies, same-precedence collisions fail, and recorded ownership does not silently repoint to a transitive claimant; a consumer-owned marketplace identifier and activation suffix are reserved only with the exact generated directory-marketplace entry; the ownership record is primary evidence, while missing-record recovery may re-adopt only that exact entry and reconcile the reserved namespace; foreign collisions and invalid JSON fail closed; unrelated JSON values are preserved semantically though stable serialization may reformat them; and catalog, ownership-record, and settings writes form one rollback unit. Revised [req-tg-011] to clarify that acquisition, materialization, and lock recording may precede target exclusion, which creates no target registration or primitive projection and does not block ordinary dependencies in the same batch. Compatibility is qualified at release or build time by the pinned real-host lifecycle suite; runtime availability is the operator's responsibility. Added the native plugin namespace and ownership-recovery threat to Section 10. Section 8.7, Section 11.3.2 Consumer enumeration, Appendix C, and conformance coverage updated. Statement count: 118 -> 119 (114 MUST, 5 SHOULD). | | 0.1.36 | 2026-08-29 | Editorial and defensive alignment for [req-tg-011] and [req-tg-013]. Named the [req-tg-008] result as the effective target intersection; scoped aggregate registration and plugin-name claimant selection to dependencies that passed admission; required target contraction to retire consumer-owned native registration; required advisory uninstall, prune, and restore reconciliation to omit ambiguous or changed-owner plugin entries without blocking cleanup; restored exact removal boundaries; defined directory-marketplace entries; and added reserved namespace disclosure to Section 11.2. Added conformance coverage for direct-owner promotion, advisory collision cleanup, and transitive owner-repoint refusal. Statement count remains 119 (114 MUST, 5 SHOULD). | +| 0.1.37 | 2026-09-01 | Spec-citation fold for safe full-SHA revision-pin updates (closes #2511 Mode-B silent-extension gate). Added [req-rs-017] (Section 7.7, consumer MUST): a consumer extension may replace a full commit pin only with the peeled commit of the highest eligible non-prerelease annotated tag, including 0.x; no eligible tag retains the current commit and allows unrelated updates to continue; malformed, ambiguous, or failed remote tag resolution stops before manifest or lockfile writes. Revised [req-rs-011], [req-rs-012], and [req-rs-015] for bounded manifest rewrite, scoped operation, advisory tag provenance, and network-free replay. Section 5.2, Section 5.6, Section 7.11, Section 11.3.2, Appendix C, and conformance coverage updated. Statement count: 119 -> 120 (115 MUST, 5 SHOULD). | Errata (none at publication). diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index d496dfe36..f4a81462e 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -340,7 +340,7 @@ Experimental flags MUST NOT gate security-critical behaviour (content scanning, | `apm config unset KEY` | Remove a stored config value (`target`, `self-update.channel`, `self-update.install-dir`, `temp-dir`, `allow-protocol-fallback`, `prefer-ssh`, `copilot-cowork-skills-dir`, `mcp-registry-url`) | -- | | `apm lock` | Resolve all dependencies in `apm.yml` and write `apm.lock.yaml` **without** deploying or deleting files in agent targets. Existing deployed-file provenance stays recorded while those bytes remain on disk; the next normal install performs any deferred prune. Mirrors `cargo generate-lockfile` / `pnpm lock`. | `--update` re-resolve to latest upstream SHAs without accepting stale local bare refs, `--verbose`, `-g/--global`, `--no-policy`, `--target` (comma-separated), `--parallel-downloads N` | | `apm lock export` | Export an SBOM/inventory from the **existing** `apm.lock.yaml` -- reads the lockfile only (no re-resolve, no re-hash, no network). Emits component identity (purl), recorded hashes, and the declared license. Output is deterministic (components sorted by purl, pinned timestamp) for byte-identical reproducibility. Diagnostics and startup notices use stderr so stdout stays machine-readable. This is an inventory export, not a security attestation. | `-f/--format [cyclonedx\|spdx]` (default `cyclonedx`), `-o/--output FILE` (default stdout), `-g/--global` read user-scope lockfile, `--timestamp ISO8601` pin the document timestamp (falls back to `SOURCE_DATE_EPOCH`, then the lockfile's `generated_at`) | -| `apm update [PKGS...]` | Refresh APM dependencies: resolves `apm.yml` against authenticated upstream refs, never stale bare-cache refs, prints a structured plan (added/updated/removed/unchanged), and prompts before changing refs (default `[y/N]`). Remote-resolution failure aborts without substituting cached ref state. Full-SHA pins are resolved against the latest annotated semver tag, rewritten to that tag's SHA, and annotated as `# ` in `apm.yml`. Pass `[PKGS...]` to refresh only those deps, or `-g` for user scope (`~/.apm/`). Successful no-op updates still reconcile deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries when the declared target set contracts. If the lock expects dependencies but `apm_modules/` is empty, an unchanged update restores the cache from the same refs without prompting or rewriting the manifest/lock; `--dry-run` remains read-only. `--force` changes collision/security handling, not freshness; use it only after independent verification. Strict superset of the deprecated `apm deps update`. Skips the ref-change prompt with `--yes`; previews with `--dry-run`. | `--yes`, `--dry-run`, `--verbose`, `-g/--global`, `--force`, `--parallel-downloads N`, `--target` (comma-separated) | +| `apm update [PKGS...]` | Refresh APM dependencies: resolves `apm.yml` against authenticated upstream refs, never stale bare-cache refs, prints a structured plan (added/updated/removed/unchanged), and prompts before changing refs (default `[y/N]`). Remote-resolution failure aborts without substituting cached ref state. Full-SHA pins move only to an eligible stable annotated semver tag. If none exists, APM keeps the current SHA, emits one summary warning, and continues unrelated updates; malformed or ambiguous tag records remain fatal before writes. Accepted pin updates are annotated as `# ` in `apm.yml`. Pass `[PKGS...]` to refresh only those deps, or `-g` for user scope (`~/.apm/`). Successful no-op updates still reconcile deployed artifacts, lockfile ownership, and merge-hook config/sidecar entries when the declared target set contracts. If the lock expects dependencies but `apm_modules/` is empty, an unchanged update restores the cache from the same refs without prompting or rewriting the manifest/lock; `--dry-run` remains read-only. `--force` changes collision/security handling, not freshness; use it only after independent verification. Strict superset of the deprecated `apm deps update`. Skips the ref-change prompt with `--yes`; previews with `--dry-run`. | `--yes`, `--dry-run`, `--verbose`, `-g/--global`, `--force`, `--parallel-downloads N`, `--target` (comma-separated) | | `apm self-update` | Update the APM CLI itself (or show distributor guidance when self-update is disabled at build time). | `--check` only check | `apm config set prefer-ssh true` and `apm config set allow-protocol-fallback true` persist transport preferences to `~/.apm/config.json` so SSH-only and corporate GHES users no longer need to re-pass `--ssh` / `--allow-protocol-fallback` on every `apm install`. Resolution order: CLI flag > `APM_GIT_PROTOCOL` / `APM_ALLOW_PROTOCOL_FALLBACK` env var > `apm config` value > built-in default (`false`). `apm config unset prefer-ssh` and `apm config unset allow-protocol-fallback` remove the persisted value. In `apm config` / `apm config list` / `apm config get` (no key), the two transport rows surface only when they have been enabled (the `false`-default rows are suppressed to keep the output noise-free); `apm config get ` always returns the effective value. Setting `allow-protocol-fallback=true` while `CI=1` emits a warning because the persisted value affects every subsequent `apm install` on a shared `$HOME`; prefer the env var in CI. diff --git a/scripts/architecture_linter/checks/transport_revision_pins.py b/scripts/architecture_linter/checks/transport_revision_pins.py new file mode 100644 index 000000000..95b4eedf7 --- /dev/null +++ b/scripts/architecture_linter/checks/transport_revision_pins.py @@ -0,0 +1,131 @@ +"""Revision-pin outcome ownership checks for the transport/platform group.""" + +from __future__ import annotations + +import re + +from scripts.architecture_linter.checks.transport_platform_shared import ( + GROUP, + _count_checks, + _forbid_scan, + _require_subs, + _src_python, +) +from scripts.architecture_linter.facts import FactsProvider +from scripts.architecture_linter.models import Rule, Violation + +_RULE_ID = "transport-platform-revision-pin-outcome" +_OWNER = "src/apm_cli/deps/revision_pins.py" +_DEPENDENCY_RESOLVER = "src/apm_cli/deps/apm_resolver.py" +_COMMAND = "src/apm_cli/commands/update.py" +_INSTALL_RESOLVER = "src/apm_cli/install/phases/resolve.py" +_OWNER_DEFINITIONS = re.compile( + r"^class (RevisionPinResolutionResult|RevisionPinSkip):" + r"|^def resolve_revision_pin_updates\(" +) +_COMMAND_TAG_LOOKUP = re.compile(r"find_latest_annotated_tag\(") + + +def _check_revision_pin_outcome(provider: FactsProvider) -> tuple[Violation, ...]: + """Require update commands to consume the revision-pin owner's full result.""" + inventory = frozenset(provider.inventory) + findings: list[Violation] = [] + findings.extend( + _count_checks( + provider, + inventory, + _RULE_ID, + _OWNER, + ( + ("re", _OWNER_DEFINITIONS.pattern, 3, "eq"), + ("sub", "return RevisionPinResolutionResult(", 2, "eq"), + ), + "Revision-pin outcomes must stay owned by RevisionPinResolutionResult", + ) + ) + findings.extend( + _forbid_scan( + provider, + inventory, + _RULE_ID, + _src_python(provider, exclude={_OWNER}), + _OWNER_DEFINITIONS, + "Revision-pin outcome definitions must stay in deps/revision_pins.py", + exempt=True, + ) + ) + findings.extend( + _require_subs( + provider, + inventory, + _RULE_ID, + _OWNER, + ( + '.removesuffix(".git")', + "max(candidates, key=lambda item: (item[0], item[1]))", + ), + "Revision-pin candidate naming and tie-breaking must stay deterministic", + ) + ) + findings.extend( + _require_subs( + provider, + inventory, + _RULE_ID, + _COMMAND, + ( + "resolution = resolve_revision_pin_updates(", + "logger.revision_pins_retained(resolution.skips)", + "logger.revision_pin_resolution_failed(e)", + "revision_pin_updates = revision_pin_resolution.updates", + ), + "The update command must consume both revision-pin outcome collections", + ) + ) + findings.extend( + _forbid_scan( + provider, + inventory, + _RULE_ID, + (_COMMAND,), + _COMMAND_TAG_LOOKUP, + "The update command must not independently resolve annotated tags", + exempt=True, + ) + ) + findings.extend( + _require_subs( + provider, + inventory, + _RULE_ID, + _INSTALL_RESOLVER, + ("root_package=ctx.apm_package",), + "Install resolution must consume the caller's staged root package", + ) + ) + findings.extend( + _require_subs( + provider, + inventory, + _RULE_ID, + _DEPENDENCY_RESOLVER, + ("root_package = replace(root_package, source_path=project_root.resolve())",), + "Staged root packages must retain a portable project source anchor", + ) + ) + return tuple(findings) + + +RULES: tuple[Rule, ...] = ( + Rule( + id=_RULE_ID, + group=GROUP, + guard_ids=(_RULE_ID,), + description="Revision-pin updates and retained SHAs share one typed outcome owner.", + check=_check_revision_pin_outcome, + ), +) + +COLLECTORS: tuple[object, ...] = () + +__all__ = ["COLLECTORS", "RULES"] diff --git a/scripts/architecture_linter/groups/transport_platform.py b/scripts/architecture_linter/groups/transport_platform.py index d386c4318..4056d311b 100644 --- a/scripts/architecture_linter/groups/transport_platform.py +++ b/scripts/architecture_linter/groups/transport_platform.py @@ -1,10 +1,4 @@ -"""Thin rule catalog for transport/platform architecture checks. - -Composed from four cohesive check-family modules -- auth/URL-path/Windows, -repository cache identity, sparse-checkout/self-update, and -network/ref-resolution/runtime-safety -- plus their shared helper module, so -no single check module outgrows the module size budget. -""" +"""Thin rule catalog for transport/platform architecture checks.""" from scripts.architecture_linter.checks.transport_auth_platform import ( COLLECTORS as _AUTH_COLLECTORS, @@ -20,6 +14,12 @@ from scripts.architecture_linter.checks.transport_network_and_runtime import ( RULES as _NETWORK_RULES, ) +from scripts.architecture_linter.checks.transport_revision_pins import ( + COLLECTORS as _REVISION_PIN_COLLECTORS, +) +from scripts.architecture_linter.checks.transport_revision_pins import ( + RULES as _REVISION_PIN_RULES, +) from scripts.architecture_linter.checks.transport_sparse_and_updates import ( COLLECTORS as _SPARSE_COLLECTORS, ) @@ -27,7 +27,13 @@ RULES as _SPARSE_RULES, ) -RULES = _AUTH_RULES + _CACHE_RULES + _SPARSE_RULES + _NETWORK_RULES -COLLECTORS = _AUTH_COLLECTORS + _CACHE_COLLECTORS + _SPARSE_COLLECTORS + _NETWORK_COLLECTORS +RULES = _AUTH_RULES + _CACHE_RULES + _SPARSE_RULES + _REVISION_PIN_RULES + _NETWORK_RULES +COLLECTORS = ( + _AUTH_COLLECTORS + + _CACHE_COLLECTORS + + _SPARSE_COLLECTORS + + _REVISION_PIN_COLLECTORS + + _NETWORK_COLLECTORS +) __all__ = ["COLLECTORS", "RULES"] diff --git a/src/apm_cli/commands/update.py b/src/apm_cli/commands/update.py index 2bafc2211..b411e4122 100644 --- a/src/apm_cli/commands/update.py +++ b/src/apm_cli/commands/update.py @@ -69,6 +69,7 @@ from ..deps.revision_pins import ( RemoteRefDownloader, RevisionPinResolutionError, + RevisionPinResolutionResult, RevisionPinUpdate, apply_revision_pin_updates, render_revision_pin_update_plan, @@ -146,7 +147,7 @@ def _resolve_and_stage_revision_pin_updates( logger: InstallLogger, downloader: RemoteRefDownloader | None = None, max_workers: int = 4, -) -> list[RevisionPinUpdate]: +) -> RevisionPinResolutionResult: """Resolve SHA pins and stage their in-memory references for the plan. The passed dependency references belong to a staged APMPackage copy, not to @@ -164,14 +165,14 @@ def _resolve_and_stage_revision_pin_updates( # independently re-resolves the freshly-written pin against upstream # before downloading. Threading the SHA resolved here into install # would collapse the authoritative-upstream fence. - updates = resolve_revision_pin_updates( + resolution = resolve_revision_pin_updates( all_declared_deps, downloader or _build_revision_pin_downloader(), only_packages=only_set, max_workers=max_workers, ) except RevisionPinResolutionError as e: - logger.error(str(e)) + logger.revision_pin_resolution_failed(e) sys.exit(1) except (GitCommandError, OSError) as e: logger.error(f"Failed to resolve revision pins: {e}") @@ -179,12 +180,14 @@ def _resolve_and_stage_revision_pin_updates( logger.info("Run with --verbose for detailed diagnostics.") sys.exit(1) - updates_by_key = {update.dep_key: update for update in updates} + logger.revision_pins_retained(resolution.skips) + + updates_by_key = {update.dep_key: update for update in resolution.updates} for dep_ref in all_declared_deps: update = updates_by_key.get(dep_ref.get_unique_key()) if update is not None: dep_ref.reference = update.new_sha - return updates + return resolution def _annotate_lockfile_revision_tags(project_root: Path, updates: list[RevisionPinUpdate]) -> None: @@ -629,12 +632,13 @@ def _run_dep_update( _rich_info(f"Available: {', '.join(e.available)}", symbol="info") sys.exit(1) - revision_pin_updates = _resolve_and_stage_revision_pin_updates( + revision_pin_resolution = _resolve_and_stage_revision_pin_updates( all_declared_deps=all_declared_deps, only_packages=only_packages, logger=logger, max_workers=parallel_downloads if parallel_downloads > 0 else 1, ) + revision_pin_updates = revision_pin_resolution.updates plan_state = _UpdateRunState() @@ -690,10 +694,18 @@ def _plan_callback(plan: UpdatePlan) -> bool: _rich_echo("") elif not revision_pin_updates: if not _cache_rehydration_required: - _rich_success( - "All dependencies already at their latest matching refs.", - symbol="check", - ) + retained_count = len(revision_pin_resolution.skips) + if retained_count: + noun = "pin" if retained_count == 1 else "pins" + logger.info( + f"No dependencies updated; retained {retained_count} revision " + f"{noun} at the current SHA." + ) + else: + _rich_success( + "All dependencies already at their latest matching refs.", + symbol="check", + ) return False plan_state.cache_rehydration_requested = True diff --git a/src/apm_cli/core/command_logger.py b/src/apm_cli/core/command_logger.py index ca1b584e8..9f5dcb08a 100644 --- a/src/apm_cli/core/command_logger.py +++ b/src/apm_cli/core/command_logger.py @@ -5,7 +5,11 @@ from apm_cli.utils.console — no new output primitives. """ +from __future__ import annotations + +from collections.abc import Sequence from dataclasses import dataclass +from typing import TYPE_CHECKING from apm_cli.models.results import InstallDisposition from apm_cli.utils.console import ( @@ -16,6 +20,9 @@ _rich_warning, ) +if TYPE_CHECKING: + from apm_cli.deps.revision_pins import RevisionPinSkip + def _strip_source_prefix(source: str) -> str: """Strip the ``org:`` / ``url:`` prefix from a policy source string.""" @@ -232,6 +239,35 @@ def __init__(self, verbose: bool = False, dry_run: bool = False, partial: bool = self.partial = partial # True when specific packages are passed to `apm install` self._stale_cleaned_total = 0 # Accumulated by stale_cleanup / orphan_cleanup + def revision_pins_retained(self, skips: Sequence[RevisionPinSkip]) -> None: + """Report retained revision pins once, with per-pin verbose detail.""" + count = len(skips) + if count == 0: + return + noun = "pin" if count == 1 else "pins" + self.warning( + f"Retained {count} revision {noun}: no eligible stable annotated semver tag was found." + ) + self.info( + "Keeping the current SHA. Publish an annotated stable tag matching a " + "supported pattern to refresh it." + ) + if not self.verbose: + self.info("Run with --verbose to see retained pins.") + for skipped in skips: + self.verbose_detail(f" {skipped.display_name}: {skipped.old_sha[:8]}") + + def revision_pin_resolution_failed(self, error: Exception) -> None: + """Render a safe-stop error with verbose parser context.""" + self.error(str(error)) + if error.__cause__ is not None: + self.verbose_detail(f" Parser cause: {error.__cause__}") + self.info( + "No files changed. Retry the update; report the upstream response if it persists." + ) + if not self.verbose: + self.info("Run with --verbose for parser context.") + # --- Validation phase --- def validation_start(self, count: int): diff --git a/src/apm_cli/deps/apm_resolver.py b/src/apm_cli/deps/apm_resolver.py index 29a2337d9..bb6ceab43 100644 --- a/src/apm_cli/deps/apm_resolver.py +++ b/src/apm_cli/deps/apm_resolver.py @@ -221,12 +221,19 @@ def _signature_accepts_parent_pkg(callback) -> bool: return True return False - def resolve_dependencies(self, project_root: Path) -> DependencyGraph: + def resolve_dependencies( + self, + project_root: Path, + *, + root_package: APMPackage | None = None, + ) -> DependencyGraph: """ Resolve all APM dependencies recursively. Args: - project_root: Path to the project root containing apm.yml + project_root: Path to the project root containing apm.yml. + root_package: Optional parsed root package. A caller can supply + staged dependency refs without writing the manifest first. Returns: DependencyGraph: Complete resolved dependency graph @@ -238,7 +245,7 @@ def resolve_dependencies(self, project_root: Path) -> DependencyGraph: # Load the root package apm_yml_path = project_root / "apm.yml" - if not apm_yml_path.exists(): + if not apm_yml_path.exists() and root_package is None: # Create empty dependency graph for projects without apm.yml empty_package = APMPackage(name="unknown", version="0.0.0", package_path=project_root) empty_tree = DependencyTree(root_package=empty_package) @@ -249,23 +256,32 @@ def resolve_dependencies(self, project_root: Path) -> DependencyGraph: flattened_dependencies=empty_flat, ) - try: - root_package = APMPackage.from_apm_yml(apm_yml_path, source_path=project_root.resolve()) - except (ValueError, FileNotFoundError) as e: - # Create error graph - empty_package = APMPackage(name="error", version="0.0.0", package_path=project_root) - empty_tree = DependencyTree(root_package=empty_package) - empty_flat = FlatDependencyMap() - graph = DependencyGraph( - root_package=empty_package, - dependency_tree=empty_tree, - flattened_dependencies=empty_flat, - ) - graph.add_error(f"Failed to load root apm.yml: {e}") - return graph + if root_package is None: + try: + root_package = APMPackage.from_apm_yml( + apm_yml_path, + source_path=project_root.resolve(), + ) + except (ValueError, FileNotFoundError) as e: + # Create error graph + empty_package = APMPackage(name="error", version="0.0.0", package_path=project_root) + empty_tree = DependencyTree(root_package=empty_package) + empty_flat = FlatDependencyMap() + graph = DependencyGraph( + root_package=empty_package, + dependency_tree=empty_tree, + flattened_dependencies=empty_flat, + ) + graph.add_error(f"Failed to load root apm.yml: {e}") + return graph + elif root_package.source_path is None: + root_package = replace(root_package, source_path=project_root.resolve()) # Build the complete dependency tree - dependency_tree = self.build_dependency_tree(apm_yml_path) + dependency_tree = self.build_dependency_tree( + apm_yml_path, + root_package=root_package, + ) # Detect circular dependencies circular_deps = self.detect_circular_dependencies(dependency_tree) @@ -589,7 +605,12 @@ def _resolve_marketplace_or_record_error( ) return None - def build_dependency_tree(self, root_apm_yml: Path) -> DependencyTree: + def build_dependency_tree( + self, + root_apm_yml: Path, + *, + root_package: APMPackage | None = None, + ) -> DependencyTree: """ Build complete tree of all dependencies and sub-dependencies. @@ -597,25 +618,28 @@ def build_dependency_tree(self, root_apm_yml: Path) -> DependencyTree: This allows for early conflict detection and clearer error reporting. Args: - root_apm_yml: Path to the root apm.yml file + root_apm_yml: Path to the root apm.yml file. + root_package: Optional parsed root package to use instead of + reading the file again. Returns: DependencyTree: Hierarchical dependency tree """ - # Load root package. Anchor source_path on the project root so direct - # dep relative paths resolve from there (#857). - try: - root_package = APMPackage.from_apm_yml( - root_apm_yml, - source_path=self._project_root.resolve() - if self._project_root is not None - else root_apm_yml.parent.resolve(), - ) - except (ValueError, FileNotFoundError) as e: - _logger.warning("Failed to parse root apm.yml: %s", e) - empty_package = APMPackage(name="error", version="0.0.0") - tree = DependencyTree(root_package=empty_package) - return tree + # Load the root unless the caller has staged in-memory refs that must + # remain unwritten until a consent gate. + if root_package is None: + try: + root_package = APMPackage.from_apm_yml( + root_apm_yml, + source_path=self._project_root.resolve() + if self._project_root is not None + else root_apm_yml.parent.resolve(), + ) + except (ValueError, FileNotFoundError) as e: + _logger.warning("Failed to parse root apm.yml: %s", e) + empty_package = APMPackage(name="error", version="0.0.0") + tree = DependencyTree(root_package=empty_package) + return tree # Initialize the tree tree = DependencyTree(root_package=root_package) diff --git a/src/apm_cli/deps/git_reference_resolver.py b/src/apm_cli/deps/git_reference_resolver.py index 0d3643868..18f780798 100644 --- a/src/apm_cli/deps/git_reference_resolver.py +++ b/src/apm_cli/deps/git_reference_resolver.py @@ -42,6 +42,7 @@ is_ado_auth_failure_signal, is_github_hostname, ) +from .git_remote_ops import validate_ls_remote_tag_output from .github_rate_limit import raise_for_github_throttle if TYPE_CHECKING: @@ -306,6 +307,8 @@ def _public_github_op( ado_bearer_also_failed = False if outcome[0] == "ok": + if not include_heads: + validate_ls_remote_tag_output(outcome[1]) refs = host._parse_ls_remote_output(outcome[1]) return host._sort_remote_refs(refs) diff --git a/src/apm_cli/deps/git_remote_ops.py b/src/apm_cli/deps/git_remote_ops.py index 75851d846..93bea9ada 100644 --- a/src/apm_cli/deps/git_remote_ops.py +++ b/src/apm_cli/deps/git_remote_ops.py @@ -10,6 +10,72 @@ from ..models.apm_package import GitReferenceType, RemoteRef +class RemoteRefParseError(RuntimeError): + """Raised when git ls-remote output cannot be safely interpreted.""" + + +_REMOTE_SHA_RE = re.compile(r"^[a-fA-F0-9]{40}$") +_INVALID_REF_CHARS = frozenset(" ~^:?*[\\") + + +def _is_valid_tag_refname(refname: str) -> bool: + """Return whether *refname* follows Git's tag refname restrictions.""" + if not refname.startswith("refs/tags/"): + return False + tag_name = refname.removeprefix("refs/tags/") + if not tag_name or tag_name == "@" or tag_name.endswith("."): + return False + if ".." in tag_name or "@{" in tag_name or "//" in tag_name: + return False + if any(ord(char) < 32 or ord(char) == 127 or char in _INVALID_REF_CHARS for char in tag_name): + return False + components = tag_name.split("/") + return all( + component and not component.startswith(".") and not component.casefold().endswith(".lock") + for component in components + ) + + +def validate_ls_remote_tag_output(output: str) -> None: + """Reject malformed output from ``git ls-remote --tags``. + + An empty response is valid for a repository without tags. Any nonempty + response must use the documented SHA-and-tag-ref wire format; otherwise a + revision-pin refresh must fail rather than treating transport corruption as + a missing release tag. + """ + if output == "": + return + + plain_tags: set[str] = set() + peeled_tags: set[str] = set() + records: dict[str, str] = {} + for line in output.splitlines(): + if not line.strip(): + raise RemoteRefParseError("Malformed git ls-remote tag output.") + parts = line.split("\t") + if len(parts) != 2: + raise RemoteRefParseError("Malformed git ls-remote tag output.") + raw_sha, raw_refname = parts + if raw_sha != raw_sha.strip() or raw_refname != raw_refname.strip(): + raise RemoteRefParseError("Malformed git ls-remote tag output.") + sha, refname = raw_sha, raw_refname + if not _REMOTE_SHA_RE.fullmatch(sha) or sha == "0" * 40 or refname in records: + raise RemoteRefParseError("Malformed git ls-remote tag output.") + records[refname] = sha + is_peeled = refname.endswith("^{}") + base_refname = refname[:-3] if is_peeled else refname + if not _is_valid_tag_refname(base_refname): + raise RemoteRefParseError("Malformed git ls-remote tag output.") + tag_name = base_refname.removeprefix("refs/tags/") + if is_peeled: + peeled_tags.add(tag_name) + else: + plain_tags.add(tag_name) + if peeled_tags - plain_tags: + raise RemoteRefParseError("Malformed git ls-remote tag output.") + + def parse_ls_remote_output(output: str) -> list[RemoteRef]: """Parse ``git ls-remote --tags --heads`` output into RemoteRef objects. @@ -56,10 +122,11 @@ def parse_ls_remote_output(output: str) -> list[RemoteRef]: # branch or lightweight tag named like a release can never # masquerade as a SHA-pin update target. A transport that # suppressed peeled refs would misclassify a genuine annotated - # tag as lightweight -- the resolver then raises rather than - # downgrading the pin, which is the safe direction. Any future - # edit here that marks a non-peeled ref as annotated would - # break this anti-spoofing fence. + # tag as lightweight. Revision-pin updates then retain the + # current SHA rather than selecting an unverified target, which + # is the safe direction. Any future edit here that marks a + # non-peeled ref as annotated would break this anti-spoofing + # fence. tag_name = tag_name[:-3] tags[tag_name] = sha annotated_tags.add(tag_name) diff --git a/src/apm_cli/deps/revision_pins.py b/src/apm_cli/deps/revision_pins.py index 047229126..8a24b02c0 100644 --- a/src/apm_cli/deps/revision_pins.py +++ b/src/apm_cli/deps/revision_pins.py @@ -13,6 +13,8 @@ from apm_cli.models.dependency.types import GitReferenceType, RemoteRef from apm_cli.utils.console import STATUS_SYMBOLS +from .git_remote_ops import RemoteRefParseError + if TYPE_CHECKING: from apm_cli.deps.lockfile import LockedDependency @@ -24,6 +26,10 @@ class RevisionPinResolutionError(RuntimeError): """Raised when a SHA pin cannot be safely mapped to an annotated tag.""" +class NoAnnotatedRevisionPinTagError(RevisionPinResolutionError): + """Raised when an upstream has no eligible annotated semver tag.""" + + class RemoteRefDownloader(Protocol): """Downloader surface needed for authoritative remote tag checks.""" @@ -51,6 +57,23 @@ class RevisionPinUpdate: display_name: str +@dataclass(frozen=True) +class RevisionPinSkip: + """A SHA pin retained because upstream has no eligible replacement tag.""" + + dep_key: str + old_sha: str + display_name: str + + +@dataclass(frozen=True) +class RevisionPinResolutionResult: + """Typed outcome of resolving every eligible revision pin.""" + + updates: tuple[RevisionPinUpdate, ...] = () + skips: tuple[RevisionPinSkip, ...] = () + + def is_full_revision_pin(ref: str | None) -> bool: """Return True when *ref* is a full 40-character commit SHA.""" return bool(ref and _SHA_RE.match(ref.strip())) @@ -65,7 +88,7 @@ def package_name(dep_ref: DependencyReference) -> str: """Return the tag-pattern package name for *dep_ref*.""" if dep_ref.is_virtual_subdirectory() and dep_ref.virtual_path: return dep_ref.virtual_path.rstrip("/").rsplit("/", 1)[-1] - return dep_ref.repo_url.rstrip("/").rsplit("/", 1)[-1] + return dep_ref.repo_url.rstrip("/").rsplit("/", 1)[-1].removesuffix(".git") def find_latest_annotated_tag( @@ -107,12 +130,12 @@ def find_latest_annotated_tag( break if not candidates: - raise RevisionPinResolutionError( + raise NoAnnotatedRevisionPinTagError( "No annotated tag found for revision-pinned dependency. " "APM will not replace a SHA pin with a branch or lightweight tag." ) - _, tag, sha = max(candidates, key=lambda item: item[0]) + _, tag, sha = max(candidates, key=lambda item: (item[0], item[1])) return AnnotatedTagCandidate(tag=tag, commit_sha=sha) @@ -122,8 +145,13 @@ def resolve_revision_pin_updates( *, only_packages: set[str] | None = None, max_workers: int = 4, -) -> list[RevisionPinUpdate]: - """Resolve direct SHA-pinned dependencies to latest annotated tag SHAs.""" +) -> RevisionPinResolutionResult: + """Resolve SHA pins, retaining pins that lack an eligible upstream tag. + + A missing annotated semver tag is a nonfatal outcome: the current SHA stays + intact while unrelated dependencies can still update. Transport, malformed + remote data, and integrity failures remain fatal and are propagated. + """ eligible: list[DependencyReference] = [] for dep_ref in dependencies: dep_key = dep_ref.get_unique_key() @@ -135,13 +163,27 @@ def resolve_revision_pin_updates( eligible.append(dep_ref) if not eligible: - return [] + return RevisionPinResolutionResult() - def _resolve_one(dep_ref: DependencyReference) -> RevisionPinUpdate | None: + def _resolve_one( + dep_ref: DependencyReference, + ) -> RevisionPinUpdate | RevisionPinSkip | None: dep_key = dep_ref.get_unique_key() old_sha = (dep_ref.reference or "").strip().lower() - remote_refs = downloader.list_remote_tag_refs(dep_ref) - latest = find_latest_annotated_tag(remote_refs, package_name=package_name(dep_ref)) + try: + remote_refs = downloader.list_remote_tag_refs(dep_ref) + except RemoteRefParseError as exc: + raise RevisionPinResolutionError( + f"Malformed remote tag data for {dep_key}; refusing to update or retain its SHA." + ) from exc + try: + latest = find_latest_annotated_tag(remote_refs, package_name=package_name(dep_ref)) + except NoAnnotatedRevisionPinTagError: + return RevisionPinSkip( + dep_key=dep_key, + old_sha=old_sha, + display_name=dep_key, + ) latest_sha = latest.commit_sha.strip().lower() if not is_full_revision_pin(latest_sha): raise RevisionPinResolutionError( @@ -163,7 +205,10 @@ def _resolve_one(dep_ref: DependencyReference) -> RevisionPinUpdate | None: else: with ThreadPoolExecutor(max_workers=worker_count) as executor: resolved = list(executor.map(_resolve_one, eligible)) - return [update for update in resolved if update is not None] + return RevisionPinResolutionResult( + updates=tuple(item for item in resolved if isinstance(item, RevisionPinUpdate)), + skips=tuple(item for item in resolved if isinstance(item, RevisionPinSkip)), + ) def render_revision_pin_update_plan(updates: Iterable[RevisionPinUpdate]) -> str: diff --git a/src/apm_cli/install/phases/resolve.py b/src/apm_cli/install/phases/resolve.py index 5e6086cb2..fdb0735d3 100644 --- a/src/apm_cli/install/phases/resolve.py +++ b/src/apm_cli/install/phases/resolve.py @@ -346,7 +346,10 @@ def _attach_resolver_marketplace_provenance( def _build_dependency_graph(ctx: InstallContext, resolver): """Resolve the manifest graph and retain its marketplace provenance.""" manifest_anchor = ctx.source_root if ctx.source_root != ctx.project_root else ctx.apm_dir - dependency_graph = resolver.resolve_dependencies(manifest_anchor) + dependency_graph = resolver.resolve_dependencies( + manifest_anchor, + root_package=ctx.apm_package, + ) ctx.dependency_graph = dependency_graph _attach_resolver_marketplace_provenance(ctx, resolver) return dependency_graph diff --git a/tests/integration/test_architecture_owner_rule_mutations.py b/tests/integration/test_architecture_owner_rule_mutations.py index bd648c936..9443228a2 100644 --- a/tests/integration/test_architecture_owner_rule_mutations.py +++ b/tests/integration/test_architecture_owner_rule_mutations.py @@ -539,6 +539,14 @@ class MutationCase: new="if ctx.update_refs or ctx.refresh:", intent="Ref seeding makes a parallel freshness decision outside RefFreshnessPolicy.", ), + MutationCase( + guard_id="transport-platform-revision-pin-outcome", + rule_id="transport-platform-revision-pin-outcome", + path="src/apm_cli/commands/update.py", + old="logger.revision_pins_retained(resolution.skips)", + new="logger.revision_pins_retained(())", + intent="Update discards resolver-provided retained revision pins.", + ), MutationCase( guard_id="transport-platform-self-update-resolution", rule_id="transport-platform-self-update-resolution", diff --git a/tests/integration/test_architecture_revision_pins.py b/tests/integration/test_architecture_revision_pins.py new file mode 100644 index 000000000..68a6d99dd --- /dev/null +++ b/tests/integration/test_architecture_revision_pins.py @@ -0,0 +1,146 @@ +"""Architecture guardrails for revision-pin resolution outcomes.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from scripts.architecture_linter.models import RunReport +from scripts.architecture_linter.runner import registered_rules, run_selected_rules + +pytestmark = pytest.mark.component + +ROOT = Path(__file__).resolve().parents[2] +RULE_ID = "transport-platform-revision-pin-outcome" + + +def _violated(report: RunReport) -> bool: + """Return whether the revision-pin owner rule reported a violation.""" + return any(item.rule_id == RULE_ID for item in report.violations) + + +def _command_source() -> str: + """Return the update command source used by mutation tests.""" + return (ROOT / "src/apm_cli/commands/update.py").read_text(encoding="utf-8") + + +def test_revision_pin_resolution_has_single_owner() -> None: + """The registered rule must defend the typed owner and both consumers.""" + owner = (ROOT / "src/apm_cli/deps/revision_pins.py").read_text(encoding="utf-8") + command = _command_source() + resolver = (ROOT / "src/apm_cli/install/phases/resolve.py").read_text(encoding="utf-8") + dependency_resolver = (ROOT / "src/apm_cli/deps/apm_resolver.py").read_text(encoding="utf-8") + rule = next(rule for rule in registered_rules() if rule.id == RULE_ID) + + assert owner.count("class RevisionPinResolutionResult:") == 1 + assert owner.count("class RevisionPinSkip:") == 1 + assert owner.count("def resolve_revision_pin_updates(") == 1 + assert '.removesuffix(".git")' in owner + assert "max(candidates, key=lambda item: (item[0], item[1]))" in owner + assert "logger.revision_pins_retained(resolution.skips)" in command + assert "logger.revision_pin_resolution_failed(e)" in command + assert "revision_pin_updates = revision_pin_resolution.updates" in command + assert "root_package=ctx.apm_package" in resolver + assert ( + "root_package = replace(root_package, source_path=project_root.resolve())" + in dependency_resolver + ) + assert "Revision-pin updates and retained SHAs share one typed outcome owner" in ( + rule.description + ) + + +@pytest.mark.parametrize( + ("old", "new"), + [ + ( + "logger.revision_pins_retained(resolution.skips)", + "logger.revision_pins_retained(())", + ), + ( + "revision_pin_updates = revision_pin_resolution.updates", + "revision_pin_updates = ()", + ), + ], +) +def test_revision_pin_guard_rejects_discarded_outcomes(old: str, new: str) -> None: + """The boundary rule must reject dropping either typed outcome collection.""" + command = _command_source() + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={"src/apm_cli/commands/update.py": command.replace(old, new, 1)}, + ) + + assert _violated(report) + + +def test_revision_pin_guard_rejects_command_local_tag_lookup() -> None: + """The command cannot restore an independent annotated-tag decision.""" + command = _command_source() + '\nfind_latest_annotated_tag("origin")\n' + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={"src/apm_cli/commands/update.py": command}, + ) + + assert _violated(report) + + +def test_revision_pin_guard_rejects_unstaged_root_resolution() -> None: + """The install resolver must not reload pre-consent refs from disk.""" + path = "src/apm_cli/install/phases/resolve.py" + source = (ROOT / path).read_text(encoding="utf-8") + mutated = source.replace( + "resolver.resolve_dependencies(\n manifest_anchor,\n" + " root_package=ctx.apm_package,\n )", + "resolver.resolve_dependencies(manifest_anchor)", + 1, + ) + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={path: mutated}, + ) + + assert _violated(report) + + +def test_revision_pin_guard_rejects_unanchored_staged_root() -> None: + """A staged root must preserve portable local-dependency anchoring.""" + path = "src/apm_cli/deps/apm_resolver.py" + source = (ROOT / path).read_text(encoding="utf-8") + mutated = source.replace( + "root_package = replace(root_package, source_path=project_root.resolve())", + "root_package = root_package", + 1, + ) + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={path: mutated}, + ) + + assert _violated(report) + + +def test_revision_pin_guard_rejects_nondeterministic_tag_tie() -> None: + """Equal-precedence tags must not depend on remote record order.""" + path = "src/apm_cli/deps/revision_pins.py" + source = (ROOT / path).read_text(encoding="utf-8") + mutated = source.replace( + "max(candidates, key=lambda item: (item[0], item[1]))", + "max(candidates, key=lambda item: item[0])", + 1, + ) + + report = run_selected_rules( + ROOT, + (RULE_ID,), + source_overrides={path: mutated}, + ) + + assert _violated(report) diff --git a/tests/integration/test_commands_config_coverage.py b/tests/integration/test_commands_config_coverage.py index 712153dd5..082a6aedd 100644 --- a/tests/integration/test_commands_config_coverage.py +++ b/tests/integration/test_commands_config_coverage.py @@ -22,6 +22,9 @@ from click.testing import CliRunner from apm_cli.cli import cli +from apm_cli.deps.revision_pins import RevisionPinResolutionResult + +_EMPTY_REVISION_PIN_RESOLUTION = RevisionPinResolutionResult() # --------------------------------------------------------------------------- # Shared fixtures @@ -577,7 +580,7 @@ def test_update_dry_run_no_deps( with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ), patch( "apm_cli.commands.install._install_apm_dependencies", @@ -631,7 +634,7 @@ def test_update_non_tty_without_yes_exits_error( with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ), patch("apm_cli.commands.install._install_apm_dependencies") as mock_install, ): @@ -1679,7 +1682,7 @@ def test_update_with_apm_yml_no_deps_exits_success( ) with patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ): result = runner.invoke(cli, ["update", "--yes"], catch_exceptions=False) assert result.exit_code in (0, 1) @@ -2141,7 +2144,7 @@ def test_update_global_with_apm_yml_no_deps( patch("apm_cli.core.scope.get_apm_dir", return_value=fake_apm_dir), patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ), ): result = runner.invoke(cli, ["update", "--global", "--yes"], catch_exceptions=False) @@ -2163,7 +2166,7 @@ def test_update_global_check_only_emits_warning( patch("apm_cli.core.scope.get_apm_dir", return_value=fake_apm_dir), patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ), ): result = runner.invoke( diff --git a/tests/integration/test_deps_registry_coverage.py b/tests/integration/test_deps_registry_coverage.py index 7c22426f0..388326eca 100644 --- a/tests/integration/test_deps_registry_coverage.py +++ b/tests/integration/test_deps_registry_coverage.py @@ -49,6 +49,7 @@ _split_owner_repo, ) from apm_cli.deps.revision_pins import ( + RevisionPinResolutionResult, RevisionPinUpdate, abbreviate_sha, apply_revision_pin_updates, @@ -1151,10 +1152,11 @@ def test_update_found(self) -> None: downloader = MagicMock() downloader.list_remote_tag_refs.return_value = refs - updates = resolve_revision_pin_updates([dep], downloader) - assert len(updates) == 1 - assert updates[0].new_sha == new_sha.lower() - assert updates[0].tag == "my-tool-v2.0.0" + result = resolve_revision_pin_updates([dep], downloader) + assert len(result.updates) == 1 + assert result.updates[0].new_sha == new_sha.lower() + assert result.updates[0].tag == "my-tool-v2.0.0" + assert result.skips == () def test_already_up_to_date_returns_empty(self) -> None: """No update when the remote SHA matches the pinned SHA.""" @@ -1171,8 +1173,7 @@ def test_already_up_to_date_returns_empty(self) -> None: downloader = MagicMock() downloader.list_remote_tag_refs.return_value = refs - updates = resolve_revision_pin_updates([dep], downloader) - assert updates == [] + assert resolve_revision_pin_updates([dep], downloader) == RevisionPinResolutionResult() def test_registry_dep_skipped(self) -> None: """Registry-sourced deps are not eligible for revision-pin updates.""" @@ -1183,8 +1184,7 @@ def test_registry_dep_skipped(self) -> None: reference=_FAKE_SHA, ) downloader = MagicMock() - updates = resolve_revision_pin_updates([dep], downloader) - assert updates == [] + assert resolve_revision_pin_updates([dep], downloader) == RevisionPinResolutionResult() downloader.list_remote_tag_refs.assert_not_called() def test_local_dep_skipped(self) -> None: @@ -1197,8 +1197,7 @@ def test_local_dep_skipped(self) -> None: local_path="./local-tool", ) downloader = MagicMock() - updates = resolve_revision_pin_updates([dep], downloader) - assert updates == [] + assert resolve_revision_pin_updates([dep], downloader) == RevisionPinResolutionResult() class TestApplyRevisionPinUpdates: diff --git a/tests/integration/test_deps_resolver_resolution.py b/tests/integration/test_deps_resolver_resolution.py index 062803091..b8b158c59 100644 --- a/tests/integration/test_deps_resolver_resolution.py +++ b/tests/integration/test_deps_resolver_resolution.py @@ -696,6 +696,23 @@ def test_error_graph_returned_for_invalid_apm_yml(self, tmp_path: Path) -> None: # Should return some graph without crashing assert graph is not None + def test_staged_root_package_gets_project_source_anchor(self, tmp_path: Path) -> None: + """An in-memory root retains portable local-dependency anchoring.""" + from apm_cli.deps.apm_resolver import APMDependencyResolver + from apm_cli.models.apm_package import APMPackage + + manifest = tmp_path / "apm.yml" + _write_apm_yml(manifest, {"name": "staged-root", "version": "1.0.0"}) + root_package = APMPackage.from_apm_yml(manifest) + assert root_package.source_path is None + + graph = APMDependencyResolver().resolve_dependencies( + tmp_path, + root_package=root_package, + ) + + assert graph.root_package.source_path == tmp_path.resolve() + class TestResolverBuildDependencyTree: """build_dependency_tree constructs the tree for simple packages.""" diff --git a/tests/integration/test_revision_pin_update_lifecycle.py b/tests/integration/test_revision_pin_update_lifecycle.py new file mode 100644 index 000000000..bad5d4b5d --- /dev/null +++ b/tests/integration/test_revision_pin_update_lifecycle.py @@ -0,0 +1,301 @@ +"""Installed-CLI lifecycle coverage for retained revision pins.""" + +from __future__ import annotations + +import os +import shutil +import stat +import subprocess +from dataclasses import dataclass +from pathlib import Path + +import pytest + +from apm_cli.utils.yaml_io import load_yaml +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment +from tests.utils.local_git_repository import ( + GitCommit, + LocalGitRepository, + LocalGitRepositoryFactory, +) +from tests.utils.local_package import LocalPackage, LocalPackageFactory + +pytestmark = [ + pytest.mark.integration, + pytest.mark.e2e, + pytest.mark.lifecycle_smoke, + pytest.mark.requires_apm_binary, + pytest.mark.requires_e2e_mode, +] + +_OWNER = "apm-fixture-org" +_INSTALL_ARGS = ( + "install", + "--target", + "claude", + "--parallel-downloads", + "0", + "--no-policy", +) +_UPDATE_ARGS = ( + "update", + "--target", + "claude", + "--parallel-downloads", + "0", +) + + +@dataclass(frozen=True) +class _Scenario: + """Owned repositories and consumer project for one revision-pin update.""" + + environment: dict[str, str] + consumer: LocalPackage + repositories: LocalGitRepositoryFactory + released_repository: LocalGitRepository + released_old: GitCommit + released_new: GitCommit + retained: GitCommit + + +def _instruction(marker: str) -> str: + """Return a minimal APM instruction document.""" + return f"---\napplyTo: '**'\n---\n# {marker}\n" + + +def _new_scenario(root: Path) -> _Scenario: + """Create two GitHub-shaped local repositories and one pinned consumer.""" + isolated = IsolatedApmEnvironment.create(root, base_env=dict(os.environ)) + environment = isolated.subprocess_env() + packages = LocalPackageFactory(isolated.package_root) + repositories = LocalGitRepositoryFactory(isolated.repository_root, env=environment) + + released_source = packages.create("released", targets=("claude",)) + released_instruction = packages.add_instruction( + released_source, + "released", + _instruction("old"), + ) + released_repo = repositories.create("released", source_tree=released_source.root) + released_old = repositories.commit(released_repo, message="publish released old") + released_repo_instruction = released_repo.worktree / released_instruction.relative_to( + released_source.root + ) + released_repo_instruction.write_text(_instruction("new"), encoding="utf-8") + released_new = repositories.commit(released_repo, message="publish released new") + repositories.tag(released_repo, "v2.0.0", released_new, annotated=True) + + retained_source = packages.create("retained", targets=("claude",)) + packages.add_instruction(retained_source, "retained", _instruction("unchanged")) + retained_repo = repositories.create("retained", source_tree=retained_source.root) + retained = repositories.commit(retained_repo, message="publish retained") + + released_url = f"https://github.com/{_OWNER}/released" + retained_url = f"https://github.com/{_OWNER}/retained" + environment = repositories.url_rewrite_subprocess_env_many( + ((released_repo, released_url), (retained_repo, retained_url)) + ) + consumer = LocalPackageFactory(isolated.work_root).create( + "revision-pin-consumer", + dependencies=( + {"git": released_url, "ref": released_old.sha, "alias": "released"}, + {"git": retained_url, "ref": retained.sha, "alias": "retained"}, + ), + targets=("claude",), + ) + return _Scenario( + environment=environment, + consumer=consumer, + repositories=repositories, + released_repository=released_repo, + released_old=released_old, + released_new=released_new, + retained=retained, + ) + + +def _run( + apm_binary_path: Path, + scenario: _Scenario, + *extra_args: str, +) -> subprocess.CompletedProcess[str]: + """Run the installed CLI for one scenario.""" + return subprocess.run( + (str(apm_binary_path), *_UPDATE_ARGS, *extra_args), + cwd=scenario.consumer.root, + env=scenario.environment, + capture_output=True, + text=True, + timeout=180, + check=False, + ) + + +def _install( + apm_binary_path: Path, + scenario: _Scenario, +) -> subprocess.CompletedProcess[str]: + """Install the pinned baseline before exercising update behavior.""" + return subprocess.run( + (str(apm_binary_path), *_INSTALL_ARGS), + cwd=scenario.consumer.root, + env=scenario.environment, + capture_output=True, + text=True, + timeout=180, + check=False, + ) + + +def test_update_retains_unreleased_pin_in_dry_run_and_apply( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """Dry-run and apply move only the pin backed by an eligible release tag.""" + scenario = _new_scenario(tmp_path / "lifecycle") + manifest = scenario.consumer.manifest_path + installed = _install(apm_binary_path, scenario) + assert installed.returncode == 0, installed.stdout + installed.stderr + manifest_before = manifest.read_bytes() + lock_path = scenario.consumer.root / "apm.lock.yaml" + lock_before = lock_path.read_bytes() + + dry_run = _run(apm_binary_path, scenario, "--dry-run") + + assert dry_run.returncode == 0, dry_run.stdout + dry_run.stderr + assert "Retained 1 revision pin" in dry_run.stdout + assert ( + f"{scenario.released_old.sha[:8]} -> {scenario.released_new.sha[:8]} (v2.0.0)" + in dry_run.stdout + ) + assert manifest.read_bytes() == manifest_before + assert lock_path.read_bytes() == lock_before + + applied = _run(apm_binary_path, scenario, "--yes") + + assert applied.returncode == 0, applied.stdout + applied.stderr + manifest_text = manifest.read_text(encoding="utf-8") + assert scenario.released_new.sha in manifest_text + assert scenario.released_old.sha not in manifest_text + assert scenario.retained.sha in manifest_text + lock = load_yaml(lock_path) + assert {entry["resolved_commit"] for entry in lock["dependencies"]} == { + scenario.released_new.sha, + scenario.retained.sha, + } + released_lock = next( + entry + for entry in lock["dependencies"] + if entry["resolved_commit"] == scenario.released_new.sha + ) + assert released_lock["resolved_ref"] == scenario.released_new.sha + assert released_lock["resolved_tag"] == "v2.0.0" + assert "constraint" not in released_lock + + +def _write_malformed_git_shim(shim_dir: Path, real_git: Path) -> None: + """Write a git shim whose tag-only listing returns a null object ID.""" + shim_dir.mkdir() + shim = shim_dir / "git" + shim.write_text( + """#!/usr/bin/env python3 +import os +import subprocess +import sys + +arguments = sys.argv[1:] +if "ls-remote" in arguments and "--tags" in arguments and "--heads" not in arguments: + sys.stdout.write(("0" * 40) + "\\trefs/tags/v2.0.0\\n") + raise SystemExit(0) +raise SystemExit( + subprocess.run([os.environ["APM_TEST_REAL_GIT"], *arguments], env=os.environ).returncode +) +""", + encoding="utf-8", + ) + shim.chmod(shim.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + assert real_git.is_file() + + +def _publish_tree_tag(scenario: _Scenario) -> None: + """Publish a higher annotated tag whose peeled object is a tree.""" + worktree = scenario.released_repository.worktree + tree = subprocess.run( + ("git", "rev-parse", "HEAD^{tree}"), + cwd=worktree, + env=scenario.environment, + capture_output=True, + text=True, + check=True, + ).stdout.strip() + subprocess.run( + ("git", "tag", "-a", "v3.0.0", tree, "-m", "Release v3.0.0"), + cwd=worktree, + env=scenario.environment, + capture_output=True, + text=True, + check=True, + ) + subprocess.run( + ("git", "push", "origin", "refs/tags/v3.0.0"), + cwd=worktree, + env=scenario.environment, + capture_output=True, + text=True, + check=True, + ) + + +def test_update_rejects_annotated_noncommit_before_writes( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """An annotated tag peeled to a tree cannot update project state.""" + scenario = _new_scenario(tmp_path / "noncommit") + installed = _install(apm_binary_path, scenario) + assert installed.returncode == 0, installed.stdout + installed.stderr + _publish_tree_tag(scenario) + manifest_before = scenario.consumer.manifest_path.read_bytes() + lock_path = scenario.consumer.root / "apm.lock.yaml" + lock_before = lock_path.read_bytes() + + result = _run(apm_binary_path, scenario, "--yes", "--verbose") + + assert result.returncode == 1, result.stdout + result.stderr + assert scenario.consumer.manifest_path.read_bytes() == manifest_before + assert lock_path.read_bytes() == lock_before + + +def test_update_malformed_tag_output_exits_before_writes( + tmp_path: Path, + apm_binary_path: Path, +) -> None: + """Malformed tag output must fail without changing project artifacts.""" + scenario = _new_scenario(tmp_path / "malformed") + manifest = scenario.consumer.manifest_path + installed = _install(apm_binary_path, scenario) + assert installed.returncode == 0, installed.stdout + installed.stderr + manifest_before = manifest.read_bytes() + lock_path = scenario.consumer.root / "apm.lock.yaml" + lock_before = lock_path.read_bytes() + resolved_git = shutil.which("git", path=scenario.environment["PATH"]) + assert resolved_git is not None + real_git = Path(resolved_git) + shim_dir = tmp_path / "malformed" / "bin" + _write_malformed_git_shim(shim_dir, real_git) + scenario.environment.update( + { + "PATH": f"{shim_dir}{os.pathsep}{scenario.environment['PATH']}", + "APM_TEST_REAL_GIT": str(real_git), + } + ) + + result = _run(apm_binary_path, scenario, "--yes", "--verbose") + + output = result.stdout + result.stderr + assert result.returncode == 1, output + assert "Malformed remote tag data for" in output + assert "No files changed" in output + assert manifest.read_bytes() == manifest_before + assert lock_path.read_bytes() == lock_before diff --git a/tests/integration/test_wave2_commands_coverage.py b/tests/integration/test_wave2_commands_coverage.py index eef4692dd..b66f09e25 100644 --- a/tests/integration/test_wave2_commands_coverage.py +++ b/tests/integration/test_wave2_commands_coverage.py @@ -29,6 +29,9 @@ from click.testing import CliRunner from apm_cli.cli import cli +from apm_cli.deps.revision_pins import RevisionPinResolutionResult + +_EMPTY_REVISION_PIN_RESOLUTION = RevisionPinResolutionResult() # --------------------------------------------------------------------------- # Shared fixtures @@ -643,7 +646,7 @@ def test_update_yes_no_deps_returns_early( ) with patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ): result = runner.invoke(cli, ["update", "--yes"], catch_exceptions=False) assert result.exit_code == 0 @@ -658,7 +661,7 @@ def test_update_dry_run_with_deps(self, runner: CliRunner, isolated_config: Path with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ), patch( "apm_cli.commands.install._install_apm_dependencies", @@ -693,7 +696,7 @@ def test_update_positional_unknown_package( ) with patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ): result = runner.invoke(cli, ["update", "org/unknown"], catch_exceptions=False) # Either exits non-zero (UnknownPackageError) or 0 -- no unhandled exception @@ -711,7 +714,7 @@ def test_update_ci_env_emits_info_banner( ) with patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[], + return_value=_EMPTY_REVISION_PIN_RESOLUTION, ): result = runner.invoke(cli, ["update", "--yes"], catch_exceptions=False) # Banner should mention apm update vs apm self-update distinction diff --git a/tests/spec_conformance/test_resolution_reqs.py b/tests/spec_conformance/test_resolution_reqs.py index 8e91e20e3..3a1f91126 100644 --- a/tests/spec_conformance/test_resolution_reqs.py +++ b/tests/spec_conformance/test_resolution_reqs.py @@ -20,7 +20,16 @@ import pytest from apm_cli.cache.url_normalize import normalize_repo_url +from apm_cli.deps.git_remote_ops import RemoteRefParseError, validate_ls_remote_tag_output from apm_cli.deps.plugin_parser import _map_plugin_artifacts +from apm_cli.deps.revision_pins import ( + RevisionPinResolutionError, + RevisionPinResolutionResult, + RevisionPinSkip, + find_latest_annotated_tag, + package_name, + resolve_revision_pin_updates, +) from apm_cli.deps.shared_clone_cache import SharedCloneCache from apm_cli.deps.tiered_ref_resolver import ( L0PerRunCache, @@ -29,6 +38,7 @@ _repository_cache_identity, ) from apm_cli.models.dependency.reference import DependencyReference +from apm_cli.models.dependency.types import GitReferenceType, RemoteRef from tests.integration.test_install_subdir_dedup_e2e import ( test_nested_gitlab_identity_survives_cache_lock_and_deployment as _run_nested_install_contract, ) @@ -343,6 +353,130 @@ def try_resolve(self, dep_ref: DependencyReference, _ref: str) -> str: assert cache.size() == 2 +@pytest.mark.req("req-rs-017") +@pytest.mark.parametrize( + "tag", + ["v1.2.3", "revision-pin--v1.2.3", "revision-pin-v1.2.3", "1.2.3"], +) +def test_revision_pin_update_accepts_each_normative_tag_form(tag: str) -> None: + """Every req-rs-017 tag form maps to the same semantic version.""" + candidate = find_latest_annotated_tag( + [RemoteRef(tag, GitReferenceType.TAG, "b" * 40, annotated=True)], + package_name="revision-pin", + ) + + assert candidate.tag == tag + + +@pytest.mark.req("req-rs-017") +def test_revision_pin_update_selects_highest_stable_tag_deterministically() -> None: + """Selection excludes prerelease/lightweight refs and breaks ties by name.""" + refs = [ + RemoteRef("v0.9.0", GitReferenceType.TAG, "b" * 40, annotated=True), + RemoteRef("v1.0.0+alpha", GitReferenceType.TAG, "c" * 40, annotated=True), + RemoteRef("v1.0.0+zeta", GitReferenceType.TAG, "d" * 40, annotated=True), + RemoteRef("v2.0.0-rc.1", GitReferenceType.TAG, "e" * 40, annotated=True), + RemoteRef("v3.0.0", GitReferenceType.TAG, "f" * 40, annotated=False), + RemoteRef("v4.0.0", GitReferenceType.BRANCH, "1" * 40), + ] + + forward = find_latest_annotated_tag(refs, package_name="revision-pin") + reverse = find_latest_annotated_tag(reversed(refs), package_name="revision-pin") + + assert forward == reverse + assert forward.tag == "v1.0.0+zeta" + assert forward.commit_sha == "d" * 40 + + +@pytest.mark.req("req-rs-017") +def test_revision_pin_update_derives_repository_and_virtual_names() -> None: + """Repository suffixes and virtual paths produce deterministic tag names.""" + repository = DependencyReference(repo_url="acme/revision-pin.git") + virtual = DependencyReference( + repo_url="acme/mono", + virtual_path="packages/revision-pin", + is_virtual=True, + ) + + assert package_name(repository) == "revision-pin" + assert package_name(virtual) == "revision-pin" + + +@pytest.mark.req("req-rs-017") +def test_revision_pin_update_retains_missing_release() -> None: + """No eligible tag is an entry-local retained-pin result.""" + + class TagDownloader: + def __init__(self, refs: list[RemoteRef]) -> None: + self.refs = refs + + def list_remote_tag_refs(self, _dep_ref: DependencyReference) -> list[RemoteRef]: + return self.refs + + old_sha = "a" * 40 + dependency = DependencyReference( + repo_url="acme/revision-pin", + host="github.com", + reference=old_sha, + ) + + retained = resolve_revision_pin_updates([dependency], TagDownloader([]), max_workers=1) + assert retained == RevisionPinResolutionResult( + skips=(RevisionPinSkip("acme/revision-pin", old_sha, "acme/revision-pin"),), + ) + + +@pytest.mark.req("req-rs-017") +@pytest.mark.parametrize( + "output", + [ + "\n", + f"{'0' * 40}\trefs/tags/v2.0.0", + f"{'a' * 40}\trefs/tags/bad name", + f"{'a' * 40}\trefs/tags/v2.0.0 ", + f"{'a' * 40}\trefs/tags/v2.0.0\n{'a' * 40}\trefs/tags/v2.0.0", + f"{'a' * 40}\trefs/tags/v2.0.0^{{}}", + ], +) +def test_revision_pin_update_rejects_malformed_tag_records(output: str) -> None: + """Every listed malformed-record class fails before candidate selection.""" + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output(output) + + +@pytest.mark.req("req-rs-017") +def test_revision_pin_update_wraps_transport_parse_failure() -> None: + """Malformed remote data cannot become a retained-pin outcome.""" + + class BrokenDownloader: + def list_remote_tag_refs(self, _dep_ref: DependencyReference) -> list[RemoteRef]: + raise RemoteRefParseError("Malformed git ls-remote tag output.") + + dependency = DependencyReference( + repo_url="acme/revision-pin", + host="github.com", + reference="a" * 40, + ) + + with pytest.raises(RevisionPinResolutionError, match="Malformed remote tag data"): + resolve_revision_pin_updates([dependency], BrokenDownloader(), max_workers=1) + + +@pytest.mark.req("req-rs-017") +def test_revision_pin_update_projection_and_scope_are_normative() -> None: + """The spec pins persistence, replay, scope, and atomic failure semantics.""" + assert_spec_contains( + "resolved_ref", + "resolved_commit", + "resolved_tag", + "does not become a trust anchor", + "MUST replay the full commit", + "package manifests MUST NOT be rewritten", + "aborts the requested update scope", + "non-commit object is also a fatal outcome", + ) + + # --- req-pr-001..005: primitives --------------------------------------- diff --git a/tests/unit/commands/test_update_command.py b/tests/unit/commands/test_update_command.py index 3cc90f6ba..9413acf58 100644 --- a/tests/unit/commands/test_update_command.py +++ b/tests/unit/commands/test_update_command.py @@ -26,6 +26,7 @@ from apm_cli.core.scope import InstallScope from apm_cli.core.target_detection import EffectiveTargetDecision from apm_cli.deps.lockfile import LockedDependency, LockFile +from apm_cli.deps.revision_pins import RevisionPinResolutionResult from apm_cli.install.errors import RequiredIntegrationError from apm_cli.install.plan import PlanEntry, UpdatePlan @@ -57,6 +58,11 @@ def _make_apm_yml(project_dir: Path) -> None: ) +def _revision_pin_result(*updates, skips=()) -> RevisionPinResolutionResult: + """Build the resolver's typed result for command-level tests.""" + return RevisionPinResolutionResult(updates=tuple(updates), skips=tuple(skips)) + + def test_service_only_required_failure_renders_before_exit(tmp_path: Path) -> None: """Service-only updates must not expose an unhandled integration error.""" package = MagicMock() @@ -149,9 +155,9 @@ def fake_install(_apm, **kwargs): with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[ + return_value=_revision_pin_result( RevisionPinUpdate("org/pkg", old_sha, new_sha, "v2.0.0", "org/pkg") - ], + ), ), patch( "apm_cli.commands.install._install_apm_dependencies", @@ -171,6 +177,108 @@ def fake_install(_apm, **kwargs): assert captured["proceeded"] is False assert manifest.read_text(encoding="utf-8") == original + def test_dry_run_warns_for_retained_pin_and_plans_unrelated_update( + self, runner, tmp_path + ) -> None: + """Skipped pins remain immutable while another dependency's plan is shown.""" + retained_sha = "a" * 40 + old_sha = "b" * 40 + new_sha = "c" * 40 + with runner.isolated_filesystem(temp_dir=tmp_path): + manifest = Path.cwd() / "apm.yml" + manifest.write_text( + "name: test\n" + "version: 1.0.0\n" + "dependencies:\n" + " apm:\n" + f" - org/no-release#{retained_sha}\n" + f" - org/released#{old_sha}\n", + encoding="utf-8", + ) + original = manifest.read_text(encoding="utf-8") + + from apm_cli.models.dependency.reference import DependencyReference + from apm_cli.models.dependency.types import GitReferenceType, RemoteRef + from apm_cli.models.results import InstallResult + + class MixedTagDownloader: + def list_remote_tag_refs(self, dep_ref: DependencyReference) -> list[RemoteRef]: + if dep_ref.repo_url == "org/no-release": + return [] + return [RemoteRef("v2.0.0", GitReferenceType.TAG, new_sha, annotated=True)] + + captured: dict[str, list[str] | bool] = {} + lock_path = Path.cwd() / "apm.lock.yaml" + LockFile().save(lock_path) + original_lock = lock_path.read_bytes() + + def fake_install(apm_package, **kwargs): + captured["references"] = [ + dep.reference for dep in apm_package.get_apm_dependencies() + ] + captured["proceeded"] = kwargs["plan_callback"](_stub_plan_with_changes()) + return InstallResult() + + with ( + patch( + "apm_cli.commands.update._build_revision_pin_downloader", + return_value=MixedTagDownloader(), + ), + patch( + "apm_cli.commands.install._install_apm_dependencies", + side_effect=fake_install, + ), + patch("apm_cli.commands.update._annotate_lockfile_revision_tags") as annotate, + ): + result = runner.invoke(cli, ["update", "--dry-run"]) + + assert result.exit_code == 0, result.output + assert "Retained 1 revision pin" in result.output + assert "Keeping the current SHA" in result.output + assert "Revision pin updates" in result.output + assert "Update plan" in result.output + assert captured["references"] == [retained_sha, new_sha] + assert captured["proceeded"] is False + assert manifest.read_text(encoding="utf-8") == original + assert lock_path.read_bytes() == original_lock + annotate.assert_not_called() + + def test_dry_run_retained_only_does_not_claim_latest_refs(self, runner, tmp_path) -> None: + retained_sha = "a" * 40 + with runner.isolated_filesystem(temp_dir=tmp_path): + Path("apm.yml").write_text( + "name: test\n" + "version: 1.0.0\n" + "dependencies:\n" + " apm:\n" + f" - org/no-release#{retained_sha}\n", + encoding="utf-8", + ) + from apm_cli.deps.revision_pins import RevisionPinSkip + from apm_cli.models.results import InstallResult + + def fake_install(_apm, **kwargs): + assert kwargs["plan_callback"](UpdatePlan(entries=())) is False + return InstallResult() + + with ( + patch( + "apm_cli.commands.update.resolve_revision_pin_updates", + return_value=_revision_pin_result( + skips=(RevisionPinSkip("org/no-release", retained_sha, "org/no-release"),) + ), + ), + patch( + "apm_cli.commands.install._install_apm_dependencies", + side_effect=fake_install, + ), + ): + result = runner.invoke(cli, ["update", "--dry-run"]) + + assert result.exit_code == 0, result.output + assert "No dependencies updated; retained 1 revision pin" in result.output + assert "already at their latest matching refs" not in result.output + class TestUpdateAssumeYes: def test_yes_skips_prompt_and_proceeds(self, runner, tmp_path): @@ -216,9 +324,9 @@ def fake_install(apm_package, **kwargs): with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[ + return_value=_revision_pin_result( RevisionPinUpdate("org/pkg", old_sha, new_sha, "v2.0.0", "org/pkg") - ], + ), ), patch( "apm_cli.commands.install._install_apm_dependencies", @@ -254,9 +362,9 @@ def fake_install(_apm, **kwargs): with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[ + return_value=_revision_pin_result( RevisionPinUpdate("org/pkg", old_sha, new_sha, "v2.0.0", "org/pkg") - ], + ), ), patch( "apm_cli.commands.install._install_apm_dependencies", @@ -269,6 +377,54 @@ def fake_install(_apm, **kwargs): assert result.exit_code == 0, result.output assert "Updated 1 revision pin in apm.yml." in result.output + def test_yes_updates_resolved_pin_and_retains_skipped_sha(self, runner, tmp_path) -> None: + """Only resolver-provided updates are written or passed to lock annotation.""" + retained_sha = "a" * 40 + old_sha = "b" * 40 + new_sha = "c" * 40 + with runner.isolated_filesystem(temp_dir=tmp_path): + manifest = Path.cwd() / "apm.yml" + manifest.write_text( + "name: test\n" + "version: 1.0.0\n" + "dependencies:\n" + " apm:\n" + f" - org/no-release#{retained_sha}\n" + f" - org/released#{old_sha}\n", + encoding="utf-8", + ) + + from apm_cli.deps.revision_pins import RevisionPinSkip, RevisionPinUpdate + from apm_cli.models.results import InstallResult + + update = RevisionPinUpdate("org/released", old_sha, new_sha, "v2.0.0", "org/released") + + def fake_install(_apm, **kwargs): + assert kwargs["plan_callback"](UpdatePlan(entries=())) is True + return InstallResult() + + with ( + patch( + "apm_cli.commands.update.resolve_revision_pin_updates", + return_value=_revision_pin_result( + update, + skips=(RevisionPinSkip("org/no-release", retained_sha, "org/no-release"),), + ), + ), + patch( + "apm_cli.commands.install._install_apm_dependencies", + side_effect=fake_install, + ), + patch("apm_cli.commands.update._annotate_lockfile_revision_tags") as annotate, + ): + result = runner.invoke(cli, ["update", "--yes"]) + + assert result.exit_code == 0, result.output + manifest_text = manifest.read_text(encoding="utf-8") + assert f"org/no-release#{retained_sha}" in manifest_text + assert f"org/released#{new_sha} # v2.0.0" in manifest_text + annotate.assert_called_once_with(Path.cwd(), (update,)) + def test_revision_pin_decline_keeps_manifest_unchanged(self, runner, tmp_path): old_sha = "a" * 40 new_sha = "b" * 40 @@ -293,9 +449,9 @@ def fake_install(apm_package, **kwargs): with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[ + return_value=_revision_pin_result( RevisionPinUpdate("org/pkg", old_sha, new_sha, "v2.0.0", "org/pkg") - ], + ), ), patch( "apm_cli.commands.install._install_apm_dependencies", @@ -388,9 +544,9 @@ def fake_install(_apm, **kwargs): with ( patch( "apm_cli.commands.update.resolve_revision_pin_updates", - return_value=[ + return_value=_revision_pin_result( RevisionPinUpdate("org/pkg", old_sha, new_sha, "v2.0.0", "org/pkg") - ], + ), ), patch( "apm_cli.commands.install._install_apm_dependencies", @@ -927,12 +1083,36 @@ def test_revision_pin_resolution_error_exits_1(self, runner, tmp_path) -> None: _make_apm_yml(Path.cwd()) with _patch( "apm_cli.commands.update.resolve_revision_pin_updates", - side_effect=RevisionPinResolutionError("No annotated tag found"), + side_effect=RevisionPinResolutionError("Remote returned an invalid SHA"), ): result = runner.invoke(cli, ["update"]) assert result.exit_code == 1 - assert "No annotated tag found" in result.output + assert "Remote returned an invalid SHA" in result.output + + def test_revision_pin_parse_error_has_verbose_context_without_stale_hint( + self, + runner, + tmp_path, + ) -> None: + from apm_cli.deps.git_remote_ops import RemoteRefParseError + from apm_cli.deps.revision_pins import RevisionPinResolutionError + + parse_error = RemoteRefParseError("Malformed git ls-remote tag output.") + resolution_error = RevisionPinResolutionError("Malformed remote tag data for org/pkg") + resolution_error.__cause__ = parse_error + with runner.isolated_filesystem(temp_dir=tmp_path): + _make_apm_yml(Path.cwd()) + with _patch( + "apm_cli.commands.update.resolve_revision_pin_updates", + side_effect=resolution_error, + ): + result = runner.invoke(cli, ["update", "--verbose"]) + + assert result.exit_code == 1 + assert "Parser cause: Malformed git ls-remote tag output." in result.output + assert "report the upstream response" in result.output + assert "Run with --verbose" not in result.output def test_revision_pin_git_error_exits_1_with_verbose_hint(self, runner, tmp_path) -> None: from git.exc import GitCommandError diff --git a/tests/unit/deps/test_git_remote_ops.py b/tests/unit/deps/test_git_remote_ops.py index b05fdd4c2..eb2baf1e0 100644 --- a/tests/unit/deps/test_git_remote_ops.py +++ b/tests/unit/deps/test_git_remote_ops.py @@ -10,10 +10,14 @@ from __future__ import annotations +import pytest + from apm_cli.deps.git_remote_ops import ( + RemoteRefParseError, parse_ls_remote_output, semver_sort_key, sort_remote_refs, + validate_ls_remote_tag_output, ) from apm_cli.models.apm_package import GitReferenceType, RemoteRef @@ -121,6 +125,63 @@ def test_whitespace_is_stripped_from_sha_and_refname(self) -> None: assert refs[0].name == "main" +class TestValidateLsRemoteTagOutput: + def test_empty_output_is_a_valid_no_tag_result(self) -> None: + validate_ls_remote_tag_output("") + + def test_valid_lightweight_and_annotated_tags_are_accepted(self) -> None: + validate_ls_remote_tag_output( + f"{'a' * 40}\trefs/tags/v1.0.0\n" + f"{'b' * 40}\trefs/tags/v2.0.0\n" + f"{'c' * 40}\trefs/tags/v2.0.0^{{}}\n" + ) + + def test_malformed_line_is_rejected(self) -> None: + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output("not a git ref") + + def test_branch_ref_is_rejected_for_tag_only_response(self) -> None: + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output(f"{'a' * 40}\trefs/heads/main") + + def test_peeled_tag_without_base_record_is_rejected(self) -> None: + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output(f"{'a' * 40}\trefs/tags/v1.0.0^{{}}") + + def test_malformed_peeled_tag_suffix_is_rejected(self) -> None: + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output(f"{'a' * 40}\trefs/tags/v1.0.0^{{}}garbage") + + def test_unsupported_sha_width_is_rejected(self) -> None: + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output(f"{'a' * 64}\trefs/tags/v1.0.0") + + @pytest.mark.parametrize( + "output", + [ + "\n", + f"{'a' * 40}\trefs/tags/v1.0.0\textra", + f"{'0' * 40}\trefs/tags/v1.0.0", + f"{'a' * 40}\trefs/tags/bad name", + f"{'a' * 40}\trefs/tags/bad\x00name", + f"{'a' * 40}\trefs/tags/.hidden", + f"{'a' * 40}\trefs/tags/release.lock", + f"{'a' * 40}\trefs/tags/v1..0", + f"{'a' * 40}\trefs/tags/v1.0.0 ", + f" {'a' * 40}\trefs/tags/v1.0.0", + ], + ) + def test_invalid_or_ambiguous_records_are_rejected(self, output: str) -> None: + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output(output) + + @pytest.mark.parametrize("conflicting_sha", ["a", "b"]) + def test_duplicate_records_are_rejected(self, conflicting_sha: str) -> None: + output = f"{'a' * 40}\trefs/tags/v1.0.0\n{conflicting_sha * 40}\trefs/tags/v1.0.0\n" + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + validate_ls_remote_tag_output(output) + + # --------------------------------------------------------------------------- # semver_sort_key # --------------------------------------------------------------------------- diff --git a/tests/unit/deps/test_revision_pin_resolver.py b/tests/unit/deps/test_revision_pin_resolver.py index 91c0f72f7..eb39c701b 100644 --- a/tests/unit/deps/test_revision_pin_resolver.py +++ b/tests/unit/deps/test_revision_pin_resolver.py @@ -8,13 +8,17 @@ from unittest.mock import patch import pytest +from git.exc import GitCommandError -from apm_cli.deps.git_remote_ops import parse_ls_remote_output +from apm_cli.deps.git_remote_ops import RemoteRefParseError, parse_ls_remote_output from apm_cli.deps.revision_pins import ( RevisionPinResolutionError, + RevisionPinResolutionResult, + RevisionPinSkip, RevisionPinUpdate, apply_revision_pin_updates, find_latest_annotated_tag, + package_name, resolve_revision_pin_updates, ) from apm_cli.models.dependency.reference import DependencyReference @@ -79,6 +83,24 @@ def test_latest_revision_pin_tag_ignores_prereleases_by_default() -> None: assert candidate.commit_sha == OLD_SHA +def test_latest_revision_pin_tag_breaks_equal_precedence_ties_by_tag_name() -> None: + refs = [ + RemoteRef("v2.0.0+alpha", GitReferenceType.TAG, "b" * 40, annotated=True), + RemoteRef("v2.0.0+zeta", GitReferenceType.TAG, "c" * 40, annotated=True), + ] + + latest = find_latest_annotated_tag(refs, package_name="pkg") + + assert latest.tag == "v2.0.0+zeta" + assert latest.commit_sha == "c" * 40 + + +def test_revision_pin_package_name_strips_git_suffix() -> None: + dependency = DependencyReference(repo_url="org/pkg.git") + + assert package_name(dependency) == "pkg" + + def test_apply_revision_pin_updates_annotates_manifest_atomically(tmp_path: Path) -> None: manifest = tmp_path / "apm.yml" manifest.write_text( @@ -196,12 +218,74 @@ def list_remote_refs(self, _dep_ref: DependencyReference) -> list[RemoteRef]: def list_remote_tag_refs(self, _dep_ref: DependencyReference) -> list[RemoteRef]: return [RemoteRef("v2.0.0", GitReferenceType.TAG, NEW_SHA, annotated=True)] - updates = resolve_revision_pin_updates( + result = resolve_revision_pin_updates( [DependencyReference(repo_url="org/pkg", reference=OLD_SHA)], TagsOnlyDownloader(), ) - assert updates == [RevisionPinUpdate("org/pkg", OLD_SHA, NEW_SHA, "v2.0.0", "org/pkg")] + assert result == RevisionPinResolutionResult( + updates=(RevisionPinUpdate("org/pkg", OLD_SHA, NEW_SHA, "v2.0.0", "org/pkg"),) + ) + + +def test_resolve_revision_pin_updates_skips_only_missing_annotated_tags() -> None: + """A missing tag leaves its SHA in place without blocking another pin.""" + + class MixedTagDownloader: + def list_remote_tag_refs(self, dep_ref: DependencyReference) -> list[RemoteRef]: + if dep_ref.repo_url == "org/no-release": + return [] + return [RemoteRef("v2.0.0", GitReferenceType.TAG, NEW_SHA, annotated=True)] + + retained = DependencyReference(repo_url="org/no-release", reference=OLD_SHA) + updated = DependencyReference(repo_url="org/released", reference=OLD_SHA) + + result = resolve_revision_pin_updates( + [retained, updated], + MixedTagDownloader(), + max_workers=1, + ) + + assert result.updates == ( + RevisionPinUpdate("org/released", OLD_SHA, NEW_SHA, "v2.0.0", "org/released"), + ) + assert result.skips == (RevisionPinSkip("org/no-release", OLD_SHA, "org/no-release"),) + + +@pytest.mark.parametrize("max_workers", [1, 4]) +def test_resolve_revision_pin_updates_propagates_transport_failures(max_workers: int) -> None: + """Transport failures remain fatal in both serial and concurrent resolution.""" + + class FailingDownloader: + def list_remote_tag_refs(self, _dep_ref: DependencyReference) -> list[RemoteRef]: + raise GitCommandError("ls-remote", 128, stderr="network down") + + with pytest.raises(GitCommandError, match="network down"): + resolve_revision_pin_updates( + [ + DependencyReference(repo_url="org/a", reference=OLD_SHA), + DependencyReference(repo_url="org/b", reference=OLD_SHA), + ], + FailingDownloader(), + max_workers=max_workers, + ) + + +def test_resolve_revision_pin_updates_propagates_malformed_remote_output() -> None: + """Malformed remote output cannot be converted into a retained-pin skip.""" + + class MalformedOutputDownloader: + def list_remote_tag_refs(self, _dep_ref: DependencyReference) -> list[RemoteRef]: + raise RemoteRefParseError("Malformed git ls-remote tag output.") + + with pytest.raises( + RevisionPinResolutionError, + match="Malformed remote tag data for org/pkg", + ): + resolve_revision_pin_updates( + [DependencyReference(repo_url="org/pkg", reference=OLD_SHA)], + MalformedOutputDownloader(), + ) def test_resolve_revision_pin_updates_rejects_invalid_remote_sha() -> None: diff --git a/tests/unit/install/test_resolve_resolution_paths.py b/tests/unit/install/test_resolve_resolution_paths.py index 6198c7223..57446c3d1 100644 --- a/tests/unit/install/test_resolve_resolution_paths.py +++ b/tests/unit/install/test_resolve_resolution_paths.py @@ -150,7 +150,8 @@ def __init__(self, *, download_callback, activation_callback, **_kwargs): self.marketplace_provenance = {} self._rejected_remote_local_keys = set() - def resolve_dependencies(self, _anchor): + def resolve_dependencies(self, _anchor, *, root_package=None): + assert root_package is ctx.apm_package downloaded = self._download_callback(dependency, modules) assert downloaded != package assert hook.read_text(encoding="ascii") == "old hook" diff --git a/tests/unit/scripts/test_architecture_runner.py b/tests/unit/scripts/test_architecture_runner.py index b65e080b6..b686a858f 100644 --- a/tests/unit/scripts/test_architecture_runner.py +++ b/tests/unit/scripts/test_architecture_runner.py @@ -694,6 +694,7 @@ def exiting_import( transport-platform-host-credential-resolution transport-platform-network-host-parsing transport-platform-ref-freshness +transport-platform-revision-pin-outcome transport-platform-runtime-deadline-safety transport-platform-self-update-resolution transport-platform-sparse-symlink-validation diff --git a/tests/unit/test_list_remote_refs.py b/tests/unit/test_list_remote_refs.py index 050b25970..7a9e0fa1f 100644 --- a/tests/unit/test_list_remote_refs.py +++ b/tests/unit/test_list_remote_refs.py @@ -6,6 +6,7 @@ from git.exc import GitCommandError from apm_cli.core.auth import AuthResolver +from apm_cli.deps.git_remote_ops import RemoteRefParseError from apm_cli.deps.github_downloader import GitHubPackageDownloader from apm_cli.models.dependency.reference import DependencyReference from apm_cli.models.dependency.types import GitReferenceType, RemoteRef @@ -396,6 +397,32 @@ def test_deref_tags_use_commit_sha(self, MockGitCmd): assert tag_map["v1.0.0"] == "com1111111111111111111111111111111111111" assert tag_map["v2.0.0"] == "com2222222222222222222222222222222222222" + @patch("apm_cli.deps.github_downloader.git.cmd.Git") + def test_malformed_tag_output_is_fatal(self, MockGitCmd): + """A tag-only response cannot downgrade malformed transport output to no tags.""" + dl = _build_downloader() + dep = _make_dep_ref(host="github.com") + dl._resolve_dep_token = MagicMock(return_value="tok") + dl._resolve_dep_auth_ctx = MagicMock(return_value=None) + dl._build_repo_url = MagicMock(return_value="https://github.com/owner/repo.git") + MockGitCmd.return_value.ls_remote.return_value = "not a git ref" + + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + dl.list_remote_tag_refs(dep) + + @patch("apm_cli.deps.github_downloader.git.cmd.Git") + def test_unsupported_tag_sha_width_is_fatal_before_tag_selection(self, MockGitCmd): + """Unsupported remote hashes cannot become a nonfatal no-tag outcome.""" + dl = _build_downloader() + dep = _make_dep_ref(host="github.com") + dl._resolve_dep_token = MagicMock(return_value="tok") + dl._resolve_dep_auth_ctx = MagicMock(return_value=None) + dl._build_repo_url = MagicMock(return_value="https://github.com/owner/repo.git") + MockGitCmd.return_value.ls_remote.return_value = f"{'a' * 64}\trefs/tags/not-a-release" + + with pytest.raises(RemoteRefParseError, match="Malformed git ls-remote tag output"): + dl.list_remote_tag_refs(dep) + # --------------------------------------------------------------------------- # list_remote_refs -- Azure DevOps (git ls-remote path) diff --git a/tests/utils/local_git_repository.py b/tests/utils/local_git_repository.py index 0d720d874..e747b214e 100644 --- a/tests/utils/local_git_repository.py +++ b/tests/utils/local_git_repository.py @@ -124,12 +124,19 @@ def tag( repository: LocalGitRepository, name: str, target: GitCommit, + *, + annotated: bool = False, ) -> None: """Create and publish a tag at the target commit.""" repository = self._owned_repository(repository) target = self._owned_commit(target) + command = ( + ("git", "tag", "-a", name, target.sha, "-m", f"Release {name}") + if annotated + else ("git", "tag", name, target.sha) + ) self._run( - ("git", "tag", name, target.sha), + command, cwd=repository.worktree, ) self._run(