fix(update): retain revision pins without release tags

[Daniel Meppiel (danielmeppiel)]

fix(update): retain revision pins without release tags

Fixes #2511.

TL;DR

apm update now separates revision pins that can move from pins that must be
retained because upstream has no eligible annotated semver tag. Retained pins
produce an actionable warning while unrelated dependency updates continue.
Malformed tag output, invalid hashes, incomplete annotated-tag records, and
transport failures remain fatal before writes.

Important

A missing release tag is the only nonfatal revision-pin outcome; APM never
substitutes a branch or lightweight tag for the pinned SHA.

Problem (WHY)

• One dependency with no release tag previously aborted the whole update,
  including unrelated planned changes.
• Raw git ls-remote --tags output could be malformed yet collapse into
  the same empty-ref state as a repository with no tags.
• A skipped pin needs an explicit, stable outcome so the command does not
  infer safety from a generic resolver exception.

The change keeps the decision at one owner. As
PROSE says,
"Grounding outputs in deterministic tool execution transforms probabilistic
generation into verifiable action."

Approach (WHAT)

#	Fix
1	Return immutable RevisionPinResolutionResult(updates, skips) from the revision-pin owner.
2	Warn for each retained SHA and stage only resolver-provided updates for the shared update plan.
3	Validate tag-only ls-remote output before parsing so corrupted output remains fatal.
4	Add an architecture boundary guard and mutation tests that reject command-local tag decisions or discarded skips.

Implementation (HOW)

Area	Change
deps/revision_pins.py	Defines typed updates and skips; catches only the no-eligible-annotated-tag outcome.
commands/update.py	Emits an actionable warning, preserves skipped references, and applies only updates.
deps/git_remote_ops.py and deps/git_reference_resolver.py	Reject malformed tag-only output before it can look like an empty tag set.
Tests	Cover mixed dry-run and apply behavior, transport and malformed failures, parsed tag integrity, and static-owner mutations.
Architecture and docs	Record the owner, enforce it in lint, and document partial revision-pin behavior.

Diagrams

Legend: the dashed validation node is the new boundary that distinguishes a
valid empty tag set from malformed remote output.

flowchart LR
    L[git ls-remote --tags] --> V[validate tag output]
    V --> P[parse remote tags]
    P --> R[RevisionPinResolutionResult]
    R -->|eligible annotated tag| U[stage manifest update]
    R -->|valid no eligible tag| S[retain SHA and warn]
    V -->|malformed output| F[fail before writes]
    classDef new stroke-dasharray: 5 5;
    class V new;

Loading

Trade-offs

• Typed result over per-pin exceptions. Keeps no-tag handling explicit;
  all other resolver failures still propagate.
• Strict tag-only validation over tolerant parsing. A repository with no
  tags remains valid, but malformed transport data cannot silently become a
  skip.
• No branch or lightweight-tag fallback. Preserves the existing
  annotated-tag integrity fence rather than maximizing update availability.

Benefits

1. A project with one unreleased revision pin can still preview and apply
   unrelated dependency updates.
2. The retained pin remains byte-for-byte unchanged in dry-run and apply
   flows unless the resolver returns an update for it.
3. Three malformed remote-response classes are rejected before tag selection:
   invalid line shape, unsupported SHA width, and incomplete or malformed
   peeled-tag records.
4. The architecture lint rejects both skipping resolver-provided outcomes and
   restoring a command-local annotated-tag lookup.

Validation

ruff check src/ tests/ and ruff format --check src/ tests/:

All checks passed!
1639 files already formatted

PYTHONPATH=src python3 -m pytest -q tests/unit/deps/test_git_remote_ops.py tests/unit/test_list_remote_refs.py tests/unit/deps/test_revision_pin_resolver.py:

........................................................................ [ 84%]
.............                                                            [100%]
85 passed in 14.72s

Focused cross-module regression suite

441 passed in 353.74s (0:05:53)

Scenario Evidence

#	Scenario (user promise)	Principle(s)	Test(s) proving it	Type
1	A revision-pinned package without a release tag keeps its SHA while other updates are still previewed.	Governed by policy, DevX	tests/unit/commands/test_update_command.py::TestUpdateDryRun::test_dry_run_warns_for_retained_pin_and_plans_unrelated_update (regression-trap for #2511)	unit
2	Accepting an update changes only revision pins backed by an eligible annotated tag.	Governed by policy, Secure by default	tests/unit/commands/test_update_command.py::TestUpdateAssumeYes::test_yes_updates_resolved_pin_and_retains_skipped_sha	unit
3	Corrupted tag responses never become a successful no-tag outcome.	Secure by default, Governed by policy	tests/unit/test_list_remote_refs.py::TestListRemoteRefsGitHub::test_malformed_tag_output_is_fatal tests/unit/deps/test_git_remote_ops.py::TestValidateLsRemoteTagOutput	unit
4	Resolver and command ownership cannot split silently.	Governed by policy	tests/integration/test_architecture_authorities.py::test_revision_pin_resolution_guard_rejects_command_skip_bypass tests/integration/test_architecture_authorities.py::test_revision_pin_resolution_guard_rejects_direct_tag_lookup_in_command	integration

How to test

• Create an apm.yml with one full-SHA pin whose remote has no eligible
  annotated semver tag and one pin with an eligible newer annotated tag.
• Run apm update --dry-run; expect a retained-pin warning and the
  unrelated update plan, with no manifest or lockfile write.
• Run apm update --yes; expect only the eligible pin to change and gain
  its tag comment.
• Return malformed git ls-remote --tags output; expect a nonzero update
  failure rather than a retained-pin warning.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

Updates apm update to retain SHA-pinned dependencies without eligible annotated tags while continuing unrelated updates.

Changes:

• Adds typed update/skip outcomes and actionable warnings.
• Strictly validates remote tag output.
• Updates command handling, tests, architecture guards, and documentation.

Show a summary per file

File	Summary / final comments
tests/unit/test_list_remote_refs.py	Tests malformed tag responses.
tests/unit/deps/test_revision_pin_resolver.py	Tests typed outcomes and fatal failures.
tests/unit/deps/test_git_remote_ops.py	Tests remote-output validation.
tests/unit/commands/test_update_command.py	Tests retained and updated pins.
tests/integration/test_wave2_commands_coverage.py	Updates command coverage fixtures.
tests/integration/test_deps_registry_coverage.py	Updates resolver coverage expectations.
tests/integration/test_commands_config_coverage.py	Updates command coverage fixtures.
tests/integration/test_architecture_authorities.py	Tests ownership guard enforcement.
src/apm_cli/deps/revision_pins.py	Implements typed revision-pin outcomes.
src/apm_cli/deps/git_remote_ops.py	moderate (3 votes): Reject extra tab-separated fields. critical (1 vote): Reject null object IDs. critical (1 vote): Reject duplicate or conflicting tag records.
src/apm_cli/deps/git_reference_resolver.py	Applies tag-output validation before parsing.
src/apm_cli/commands/update.py	Warns on retained pins and stages only updates.
scripts/lint-architecture-boundaries.sh	Enforces revision-pin ownership.
docs/src/content/docs/reference/cli/update.md	nit (4 votes): Synchronize the packaged apm-usage command reference.
.github/instructions/architecture.instructions.md	Records revision-pin outcome ownership.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (6)

scripts/lint-architecture-boundaries.sh:1615

• The new guard only verifies that the skip loop text is present; it never verifies that resolution.updates is consumed. A future change such as assigning revision_pin_updates = () would discard resolver-provided updates while this guard still passes, despite the guard and mutation test claiming to prevent outcome bypasses. Add an assertion for the updates assignment and a corresponding mutation case.

    || ! grep -q 'resolution = resolve_revision_pin_updates(' "$revision_pin_command" \
    || ! grep -q 'for skipped in resolution.skips:' "$revision_pin_command" \
    || grep -q 'find_latest_annotated_tag(' "$revision_pin_command"; then

src/apm_cli/commands/update.py:186

• This warning says that no annotated semver tag exists, but the resolver also skips prereleases and tags that do not match the dependency's configured pattern. In those cases annotated semver tags do exist upstream, so the message misdiagnoses why the SHA was retained; describe the missing eligible release tag instead and keep the remediation specific.

            f"Skipped revision pin for {skipped.display_name}: no annotated semver tag exists "
            "upstream. Keeping the current SHA; publish an annotated release tag to refresh it."

src/apm_cli/commands/update.py:168

• The new tag validator raises RemoteRefParseError, but this helper only handles RevisionPinResolutionError, GitCommandError, and OSError. Because this call runs before the later install try block, malformed tag output bypasses the revision-pin error path (including its logger formatting and verbose hint) and escapes the helper. Handle RemoteRefParseError here, or place it in the intended resolution-error hierarchy, and cover the command-level path.

        resolution = resolve_revision_pin_updates(

src/apm_cli/commands/update.py:187

• This emits a separate top-level warning for every skipped pin inside the loop. A project with many retained pins will therefore flood normal update output, and successful update paths do not call logger.render_summary() to provide the command's deferred diagnostic grouping. Record these through logger.diagnostics and render one grouped, actionable summary after planning instead.

    for skipped in resolution.skips:
        logger.warning(
            f"Skipped revision pin for {skipped.display_name}: no annotated semver tag exists "
            "upstream. Keeping the current SHA; publish an annotated release tag to refresh it."
        )

src/apm_cli/deps/git_remote_ops.py:33

• Blank lines are silently skipped, so a non-empty response containing only whitespace (for example, "\n") is accepted as a valid empty tag set. That converts malformed transport output into the nonfatal retained-pin path, contrary to the fail-closed contract. Treat a blank line as malformed here; only the truly empty response should represent a repository with no tags.

    for line in output.splitlines():
        line = line.strip()
        if not line:
            continue

tests/unit/deps/test_git_remote_ops.py:150

• The new strict validator tests only empty and rejected responses; there is no positive test for a valid non-empty tag-only response. A regression that rejects every valid 40-character refs/tags/... record would still pass and make all annotated SHA pins fail before resolution. Add a case with a valid base plus peeled tag record (and, ideally, a valid lightweight tag record) and assert that validation succeeds.

class TestValidateLsRemoteTagOutput:
    def test_empty_output_is_a_valid_no_tag_result(self) -> None:
        validate_ls_remote_tag_output("")

    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")

• Files reviewed: 15/15 changed files
• Comments generated: 4
• Review effort level: Lite