Skip to content

feat(pack): --create-tag and --push to materialize release tags (#1489)#1497

Closed
danielmeppiel wants to merge 5 commits into
mainfrom
feat/pack-create-tag-push
Closed

feat(pack): --create-tag and --push to materialize release tags (#1489)#1497
danielmeppiel wants to merge 5 commits into
mainfrom
feat/pack-create-tag-push

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

feat(pack): --create-tag and --push to materialize release tags (#1489)

TL;DR

apm pack --check-versions --create-tag --push collapses the bump → tag → push handshake into one gated command. After the existing version gate passes on a clean tree, the CLI materialises the tag(s) derived from marketplace.versioning and pushes them to origin via explicit refspecs — never git push --tags. Refusals are first-class JSON contract codes (dirty_tree, tag_exists, version_mismatch, no_remote, push_without_tag, no_check_versions, no_marketplace, git_failure) surfacing at tag_creation.refusal_code and exiting 1; the existing gate codes 3 and 4 are unchanged.

Closes #1489.

Problem (WHY)

The current release dance is five mechanical steps the CLI already has the information to perform:

  • Producer bumps marketplace.yml, runs apm pack --check-versions --check-clean, eyeballs the report, then manually types git tag vX.Y.Z && git push origin vX.Y.Z. Steps 4–5 are error-prone: tag-name typos, forgotten push, or pushing the wrong commit when local HEAD has drifted from origin/main. Issue feat(pack): --create-tag and --push flags to create and push release tag after --check-versions #1489 lists this as the motivating friction.
  • Hand-typing the tag name re-derives a value the CLI already computes inside --check-versions from marketplace.versioning.strategy and per-package tag_pattern. The two derivations can disagree silently.
  • git push --tags is the lazy shortcut that bulk-pushes every local tag, including stale or wrong-namespace tags. Producers reach for it because the safe form (explicit refspec per tag) is verbose to type by hand.

Important

The fix has to preserve the producer's release ergonomics that already work: --dry-run previews, --json machine-readable envelope, exit codes 3 (version gate) and 4 (drift gate). Any new refusal must NOT collide with those.

This is the kind of glue work PROSE describes as "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action." — the tagger is a deterministic primitive driven by the same gate that already proves the version is releasable.

Approach (WHAT)

Decision Choice Why
Where the tagger lives New src/apm_cli/release/ package, called from pack.py after the release-gate block Keeps pack.py under the 2450-line cap; future tagging callers can re-use GitTagger without going through Click.
Tag derivation Single source of truth: marketplace.versioning (single_version / per_package / tag_pattern) Re-uses the same data the version gate already validated; impossible for tag name to disagree with what was checked.
Push form git push origin refs/tags/<name>:refs/tags/<name> per tag --tags was rejected; see Trade-offs. Test-locked invariant.
Refusal vocabulary 8 stable codes exposed at tag_creation.refusal_code in the --json envelope, exit 1 Doesn't collide with existing gate exits (3, 4); machine-readable for CI scripts.
Auth No new auth path; tags are pushed through whatever git push already uses for the resolved remote Stays inside the boundary scripts/lint-auth-signals.sh enforces. ls-remote --tags for tag-exists preflight carries the # auth-delegated: annotation Rule B requires.

Implementation (HOW)

File Change
src/apm_cli/release/git_tagger.py (new, 390 lines) GitTagger with plan_tagspreflightcreatepush lifecycle; TaggingRefusal with code/message/hint; stable refusal-code constants. push() builds explicit refs/tags/<n>:refs/tags/<n> refspecs — never --tags.
src/apm_cli/utils/git_subprocess.py (new, 73 lines) Thin run_git() helper layering external_process_env() + git_subprocess_env() + GIT_TERMINAL_PROMPT=0. Single source of subprocess env setup; avoids R0801 with ref_resolver.
src/apm_cli/commands/pack.py (+217 LOC, total 974 / cap 2450) Two new Click options; hoisted the gate_config loader to also fire when tagging flags are set; added _run_tagging() helper that owns guard logic + refusal payload construction; extended JSON envelope with tag_creation / tag_push keys (always present, null when unused).
pyproject.toml Bumped pylint.max-args 18 → 19 for pack_cmd (now 19 Click params); the existing threshold-just-above-max convention is preserved.
Tests 22 unit on GitTagger, 14 unit on pack wiring, 2 e2e against a real bare remote. Total 38 new tests; all green.
Docs One section in docs/src/content/docs/producer/releasing-from-any-ci.md; one entry in Unreleased of CHANGELOG.md; one row update in packages/apm-guide/.apm/skills/apm-usage/commands.md.

Diagrams

Decision tree for the tagging block, anchored to the existing gate precedence.

flowchart TD
    A[apm pack invocation] --> B{tagging flags set?}
    B -->|no| Z[skip block, exit 0]
    B -->|yes| C{--check-versions set?}
    C -->|no| R1[refuse no_check_versions, exit 1]
    C -->|yes| D{version gate passed?}
    D -->|no| E[exit 3, tagging skipped]
    D -->|yes| F{drift gate passed?}
    F -->|no| G[exit 4, tagging skipped]
    F -->|yes| H[GitTagger.preflight]
    H -->|refuse| R2[refuse with stable code, exit 1]
    H -->|ok| I[create local tags]
    I --> J{--push?}
    J -->|no| Z
    J -->|yes| K[push explicit refspecs to origin]
    K --> Z
Loading

Legend: new behaviour boxed in nodes B, R1, R2, H, I, J, K. Existing exits 3 and 4 are unchanged; 1 is the only new exit and is shared with other pack runtime errors per the existing exit-code contract.

End-to-end happy-path lifecycle on a single-version marketplace, showing the test-locked invariant (no --tags).

sequenceDiagram
    autonumber
    participant U as Producer
    participant P as apm pack
    participant T as GitTagger
    participant G as git
    participant R as origin bare remote
    U->>P: --check-versions --create-tag --push
    P->>P: version gate (already shipped)
    P->>P: drift gate (already shipped)
    P->>T: plan_tags(strategy, version, packages)
    T-->>P: TagPlan v1.0.0
    P->>T: preflight(plans, remote=origin)
    T->>G: status --porcelain
    T->>G: tag -l v1.0.0
    T->>G: ls-remote --tags origin
    P->>T: create(plans)
    T->>G: tag -a v1.0.0 -m Release v1.0.0
    P->>T: push(created, remote=origin)
    T->>G: push origin refs/tags/v1.0.0
    G->>R: explicit refspec only
    R-->>U: tag visible on remote
Loading

Legend: every git call is materialised on the wire so the auth boundary and the no---tags invariant are both auditable. Step 13 is the test-locked regression trap.

Trade-offs

  • Explicit refspec vs git push --tags. Rejected --tags because it pushes every local tag (including drift-namespace or stale tags) — the exact silent-action class that motivated this issue. Cost: one push invocation per tag in per_package mode. Locked with a unit test that spies on run_git and asserts no --tags flag is ever passed.
  • In-process tagger vs shell-out template. Rejected emitting a shell snippet (git tag … && git push …) because the producer would still have to copy-paste it and we lose --dry-run parity. The in-process tagger gets --json and dry-run for free.
  • One refusal code per failure mode vs a single tag_refused umbrella. Chose stable per-cause codes so CI scripts can branch (if .refusal_code == "tag_exists" then …); the cost is a larger surface contract to keep stable.
  • Exit 1 for refusals vs new code 5/6. Kept 1 because the existing exit-code contract documents 1 as "build/runtime error"; tag refusal is a runtime error the producer must act on. The tag_creation.refusal_code in JSON disambiguates without breaking back-compat.

Benefits

  1. One command replaces five. The release dance for a clean tree is apm pack --check-versions --create-tag --push instead of apm pack --check-versions && git tag … && git push origin ….
  2. No tag-name typos. Tags come from the same marketplace.versioning data the version gate already validated; producer can no longer ship a v.1.0.0 typo.
  3. Safe by default. --tags is never used; only the just-created tags are pushed. Dirty trees, existing tags, and version mismatches refuse with actionable hints.
  4. CI-friendly contract. tag_creation.refusal_code is part of the stable --json envelope; pipelines can branch on the cause without parsing prose.
  5. --dry-run parity. Producers can preview exactly which tags would be created and pushed without touching git or the remote.

Validation

Lint contract from .apm/instructions/linting.instructions.md — all silent / exit 0:

$ uv run --extra dev ruff check src/ tests/
All checks passed!
$ uv run --extra dev ruff format --check src/ tests/
1109 files already formatted
$ uv run --extra dev python -m pylint --disable=all --enable=R0801 \
    --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/
Your code has been rated at 10.00/10
$ bash scripts/lint-auth-signals.sh
[+] auth-signal lint clean
Unit + integration totals
$ uv run --extra dev pytest tests/unit -q
15568 passed, 1 skipped, 20 xfailed, 19 warnings, 40 subtests passed in 159.60s

$ uv run --extra dev pytest tests/integration -q
9010 passed, 223 skipped, 16 deselected, 2 xfailed in 214.54s

Scenario Evidence

User-promise scenario Test proving it APM principle
--push never falls back to git push --tags tests/unit/release/test_git_tagger.py::test_push_invokes_git_push_with_explicit_tag_refs_not_minus_minus_tags Determinism — the safe form is test-locked, no silent action.
Version mismatch still exits 3 and never tags tests/unit/commands/test_pack_tagging.py::TestRefusalSemantics::test_pack_release_gates_still_exit_3_and_4_when_their_checks_fail Exit-code stability — existing producer scripts keep working.
Dirty tree refuses cleanly with code 1, not 3/4 tests/unit/commands/test_pack_tagging.py::TestRefusalSemantics::test_pack_refusal_on_dirty_tree_exits_1_not_3_or_4 First-class refusals — no exit-code collisions.
End-to-end one-shot release works on single_version tests/integration/release/test_pack_tagging_e2e.py::test_pack_check_versions_create_tag_push_end_to_end Whole-flow determinism against real git + bare remote.
per_package strategy creates one tag per package tests/unit/commands/test_pack_tagging.py::TestHappyPath::test_pack_create_tag_and_push_happy_path_per_package Strategy fidelity — both versioning shapes covered.
JSON envelope always contains tag_creation / tag_push keys tests/unit/commands/test_pack_tagging.py::TestJsonEnvelope::test_pack_json_envelope_keys_always_present Stable contract — downstream jq consumers never see missing keys.

How to test

  1. git clone a fresh APM project with a marketplace: block (or scaffold via apm init --marketplace).
  2. Commit everything so the tree is clean, then run apm pack --check-versions --create-tag --dry-run — confirm it prints Would create tag: vX.Y.Z and no tag actually appears in git tag --list.
  3. Run apm pack --check-versions --create-tag — confirm git tag --list now shows the tag and git show <tag> points at HEAD.
  4. With an origin remote, run apm pack --check-versions --create-tag --push on a fresh version — confirm git ls-remote --tags origin shows the new tag.
  5. Touch a file (do not commit) and re-run with --create-tag — confirm exit code 1 and tag_creation.refusal_code == "dirty_tree" in the --json output.

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

Collapse the bump/tag/push handshake into the pack command. After
the existing --check-versions gate passes on a clean tree, --create-tag
materialises the release tag(s) derived from marketplace.versioning
(one v<version> for single_version, one <name>-v<version> per package
for per_package), and --push pushes them to 'origin' via explicit
refspecs (refs/tags/<name>:refs/tags/<name>) -- never 'git push --tags'.

Refusals are first-class JSON contract codes (dirty_tree, tag_exists,
version_mismatch, no_remote, push_without_tag, no_check_versions,
no_marketplace, git_failure) exposed at tag_creation.refusal_code in the
--json envelope and exit with code 1. Existing gate exits (3, 4) are
unchanged. --dry-run previews everything without touching git.

Closes #1489

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 26, 2026 23:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds release-tag materialization to apm pack, letting marketplace producers create and optionally push version-derived git tags after the existing version gate succeeds.

Changes:

  • Adds GitTagger and run_git() helpers for planning, preflighting, creating, and pushing release tags.
  • Wires --create-tag and --push into apm pack, including JSON envelope fields and exit-code handling.
  • Adds unit/integration coverage plus documentation, guide, changelog, and lint-threshold updates.
Show a summary per file
File Description
src/apm_cli/release/git_tagger.py Implements tag planning, preflight checks, creation, and explicit-refspec push logic.
src/apm_cli/release/__init__.py Re-exports release helper APIs.
src/apm_cli/utils/git_subprocess.py Adds shared sanitized git subprocess wrapper.
src/apm_cli/commands/pack.py Adds pack flags, tagging orchestration, JSON payloads, and exit handling.
pyproject.toml Raises pylint argument threshold for the expanded pack_cmd.
tests/unit/release/test_git_tagger.py Adds unit coverage for tagger lifecycle behavior.
tests/unit/release/conftest.py Adds hermetic git repository fixtures for release tests.
tests/unit/release/__init__.py Adds release test package marker.
tests/unit/commands/test_pack_tagging.py Adds unit coverage for apm pack tagging flag wiring and JSON behavior.
tests/unit/commands/conftest.py Adds command-test git helper fixture.
tests/integration/release/test_pack_tagging_e2e.py Adds hermetic end-to-end pack/tag/push tests.
tests/integration/release/__init__.py Adds integration release test package marker.
docs/src/content/docs/producer/releasing-from-any-ci.md Documents one-shot tagging workflow.
packages/apm-guide/.apm/skills/apm-usage/commands.md Updates apm usage guide command reference.
CHANGELOG.md Adds Unreleased entry for the new pack tagging flags.

Copilot's findings

  • Files reviewed: 13/15 changed files
  • Comments generated: 3

Comment thread src/apm_cli/commands/pack.py Outdated
Comment thread docs/src/content/docs/producer/releasing-from-any-ci.md Outdated
Comment thread CHANGELOG.md Outdated
Blocking fixes:
- JSON envelope: source error code from tag_push_payload when push
  refuses (creation succeeded), instead of always reading
  tag_creation_payload (whose refusal_code is None on success).
  Regression-trapped by a new unit test.
- Docs/CHANGELOG/PR_BODY: replace fabricated 'single_version'
  strategy name with the real strategies (lockstep, tag_pattern,
  per_package) per src/apm_cli/marketplace/yml_schema.py.
- docs/reference/cli/pack.md: add --create-tag/--push rows and
  expand exit-code table with rows 1 (refusal codes), 3
  (--check-versions), 4 (--check-clean).

Recommended fixes:
- git_tagger.create: pass '--' before plan.name to git tag (option
  injection hardening if tag_pattern ever yields a name starting
  with '-').
- git_tagger.push: refuse remote names starting with '-' (defensive,
  pack.py hardcodes 'origin').
- git_tagger._existing_remote_tags: elevate ls-remote failure log
  from info -> warning; rewording clarifies fail-open behavior.
- pack._run_tagging: render TaggingRefusal.hint in create/push catch
  blocks (previously only preflight rendered it).
- docs/producer/publish-to-a-marketplace.md: replace 'git tag X &&
  git push --tags' anti-pattern with the new one-shot flow.
- docs/producer/versioning-strategies.md, producer/index.md:
  forward-link to the one-shot tagging flow.
- docs/producer/releasing-from-any-ci.md: tighten wording,
  document explicit-refspec push invariant, drop the dismissive
  'intended for local one-shot releases' line.
- CHANGELOG: trim Unreleased entry.

Test coverage:
- Add no_marketplace refusal test (bundle-only project + --create-tag).
- Add regression test asserting tag_push refusal_code propagates
  into envelope.errors[].code (the blocking JSON bug).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

apm pack --create-tag --push collapses the maintainer bump-tag-push handshake into one safe command, with stable refusal codes the JSON envelope already exposes -- a missing piece of the producer release loop.

cc @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

The eight active panelists converge cleanly: the design is right, the test discipline is right, and the security posture is fine. The blocking-severity findings were all narrow correctness or docs-truth issues -- a JSON envelope sourcing bug, a fabricated strategy name (single_version) that does not exist in marketplace/yml_schema.py, and a missing update to the canonical CLI reference page. The author has already folded these in (commit a06d6a59) along with the high-signal recommended items (option-injection -- separator on git tag, TaggingRefusal.hint rendering in the create/push catch blocks, ls-remote failure log elevated to warning, and a regression-trap test pinning the envelope-sourcing fix). What ships is consistent with the existing release-gate exit-code precedence (3 > 4 > 1), preserves the explicit-refspec push invariant (regression-trapped against git push --tags), and respects the auth boundary (git's own credential resolution; AuthResolver untouched).

Aligned with: Portable by manifest -- tag names derive from marketplace.versioning, the same data the version gate validates. Secure by default -- explicit refspecs (never --tags), -- separator on git tag, ls-remote fail-open with warning-level logging, defensive remote-name check on push. Pragmatic as npm -- one verb collapses the bump-tag-push handshake; refusals are first-class typed contract.

Panel summary

Persona B R N Takeaway
python-architect 1 1 0 Module boundaries are clean; JSON envelope sourcing bug now fixed and regression-trapped.
doc-writer 2 3 1 single_version fabrication, missing reference/cli/pack.md update, and lingering git push --tags anti-pattern in publish guide all addressed.
devx-ux-expert 1 2 1 CLI reference parity restored; one-shot flow now cross-linked from versioning-strategies and producer/index.
cli-logging-expert 0 1 1 Refusal hint now renders in create/push catch blocks (was preflight-only).
supply-chain-security 0 2 1 -- separator on git tag; ls-remote fail-open log elevated to warning.
oss-growth-hacker 1 2 0 Terminology and dismissive disclaimer wording addressed; CHANGELOG entry trimmed.
test-coverage-expert 0 1 0 Added no_marketplace refusal test on bundle-only project; new regression test pins envelope-sourcing fix.
apm-primitives-architect 0 0 0 Not applicable to this surface.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 5 follow-ups

  1. [python-architect] Promote or inline _is_local_package -- currently imported as a cross-module private (from apm_cli.commands.pack import _is_local_package inside release/git_tagger.py callers). Either expose it as apm_cli.marketplace.utils.is_local_package or inline the two-line predicate at the call site. Low urgency; do as a separate clean-up PR.
  2. [supply-chain-security] Surface remote_check_skipped: true as an explicit field in tag_creation_payload when ls-remote fails open. Today the warning log communicates it; CI matrices that key off the JSON envelope cannot see it. Small, additive.
  3. [doc-writer] Add a short troubleshooting matrix to producer/releasing-from-any-ci.md mapping each refusal code to "what changed in the repo since the last clean release" -- e.g. tag_exists -> "previous release pipeline died after tag was pushed but before remote artifacts published". The codes are stable enough to document statically.
  4. [devx-ux-expert] When dirty_tree refusal fires, include a git status -s capture in the refusal message (already happens), but also suggest the exact git stash --include-untracked re-run command in refusal.hint. Single-line UX win.
  5. [test-coverage-expert] Consider a third integration test asserting the idempotent re-run behavior: after a successful --create-tag --push, a second invocation must refuse with tag_exists (not silently re-tag). The plumbing exists; the cross-flow assertion does not.

Architecture

classDiagram
  class GitTagger {
    +plan_tags(strategy, marketplace_version, packages, tag_pattern) List~TagPlan~
    +preflight(plans, remote) None
    +create(plans) List~str~
    +push(tag_names, remote) List~str~
    -_existing_remote_tags(remote, candidates)
  }
  class TagPlan {
    +name: str
    +annotation: str
    +target_sha: str
  }
  class TaggingRefusal {
    +code: str
    +message: str
    +hint: str
  }
  class PackCommand {
    +_run_tagging(...) tuple
  }
  PackCommand --> GitTagger : owns
  GitTagger --> TagPlan : produces
  GitTagger ..> TaggingRefusal : raises
Loading
sequenceDiagram
  participant U as User
  participant P as apm pack
  participant V as version_gate
  participant D as drift_gate
  participant T as GitTagger
  participant G as git
  U->>P: --check-versions --create-tag --push
  P->>V: run
  V-->>P: ok
  P->>D: run (if --check-clean)
  D-->>P: ok
  P->>T: plan_tags + preflight (origin)
  T->>G: ls-remote --tags origin (auth-delegated)
  G-->>T: tag list (or fail-open warn)
  T-->>P: TagPlans
  P->>T: create()
  T->>G: git tag -a -m MSG -- NAME
  G-->>T: ok
  P->>T: push(["NAME"], remote="origin")
  T->>G: git push origin refs/tags/NAME:refs/tags/NAME
  G-->>T: ok
  T-->>P: pushed
  P-->>U: exit 0, JSON envelope with tag_creation + tag_push
Loading

Recommendation

ship_with_followups. The fold-in commit (a06d6a59) addresses every blocking-severity finding the panel raised and the high-signal recommended ones. Land this once CI is green; the remaining follow-ups (promote _is_local_package, surface remote_check_skipped JSON, idempotent-re-run integration test, troubleshooting matrix) are clean separate-PR work that should not gate the user-visible feature.


Full per-persona findings

python-architect

  • [blocking] JSON envelope sources error code from tag_creation_payload even when the refusal originated in tag_push_payload, so envelope.errors[].code becomes null on push-only failures. Fixed in a06d6a59 (src/apm_cli/commands/pack.py ~L506) by deriving refusal_source = tag_push_payload if tag_push_payload.get('refusal_code') else tag_creation_payload; regression-trapped by tests/unit/commands/test_pack_tagging.py::test_pack_json_envelope_sources_error_from_tag_push_when_push_refuses.
  • [recommended] _is_local_package is imported as a cross-module private. Promote to apm_cli.marketplace.utils or inline. Tracked as follow-up Why do we need a GitHub token? #1.

doc-writer

  • [blocking] single_version strategy name does not exist. Real strategies in src/apm_cli/marketplace/yml_schema.py:232 are {lockstep, tag_pattern, per_package}. Fixed in a06d6a59 across CHANGELOG.md, docs/src/content/docs/producer/releasing-from-any-ci.md:70, and PR_BODY.md.
  • [blocking] docs/src/content/docs/reference/cli/pack.md was not updated with --create-tag / --push rows, and the exit-code table did not call out tag refusal codes 1, 3, 4. Fixed in a06d6a59.
  • [recommended] docs/src/content/docs/producer/publish-to-a-marketplace.md:35 still taught the git tag && git push --tags anti-pattern that this PR is designed to replace. Fixed in a06d6a59 with a forward-pointer to the one-shot flow.
  • [recommended] Cross-link from versioning-strategies.md and producer/index.md. Fixed in a06d6a59.
  • [recommended] CHANGELOG entry was ~200 words in one dense paragraph. Fixed in a06d6a59 (trimmed and reflowed).
  • [nit] Troubleshooting matrix mapping refusal codes to recovery actions. Tracked as follow-up Will there be MCP coverage? #3.

devx-ux-expert

  • [blocking] reference/cli/pack.md parity gap (same surface as doc-writer's blocking item). Fixed in a06d6a59.
  • [recommended] Strategy-name terminology mismatch (single_version vs lockstep). Fixed.
  • [recommended] Refusal hint should include exact re-run command when relevant (e.g. git stash for dirty tree). Tracked as follow-up Add ARM64 Linux support to CI/CD pipeline #4.
  • [nit] "Intended for local one-shot releases" disclaimer wording felt apologetic; reframed in a06d6a59.

cli-logging-expert

  • [recommended] TaggingRefusal.hint was rendered only in the preflight catch block in pack.py::_run_tagging; the create and push catch blocks dropped it. Fixed in a06d6a59 (pack.py ~L648, ~L671).
  • [nit] Consider tagging the warning when ls-remote fails so log scrapers can key off it. Partially addressed by elevating the level to warning with explicit "fail-open" wording.

supply-chain-security

  • [recommended] git tag -a -m MSG NAME could parse NAME as an option if a future tag_pattern yielded a name starting with -. Fixed in a06d6a59 by inserting -- separator (git_tagger.py:266).
  • [recommended] Elevate ls-remote failure log from info to warning; consider exposing remote_check_skipped in JSON. Log elevation fixed in a06d6a59; JSON exposure tracked as follow-up Integrate copilot runtime #2.
  • [nit] git push remote name could also start with -. Defensive guard added in a06d6a59 on top of pack.py hardcoding origin.

oss-growth-hacker

  • [blocking] Strategy-name terminology mismatch propagates to public-facing announce surfaces (CHANGELOG, PR body). Fixed in a06d6a59.
  • [recommended] CHANGELOG entry density hurts the release-notes scan. Trimmed in a06d6a59.
  • [recommended] Drop the dismissive "intended for local one-shot releases" line -- it discourages exactly the CI users this flow serves. Fixed.

test-coverage-expert

  • [recommended] No test exercised the no_marketplace refusal through apm pack --create-tag on a bundle-only project. Added in a06d6a59 (tests/unit/commands/test_pack_tagging.py::test_pack_create_tag_refuses_no_marketplace_on_bundle_only_project).
  • (Implicit on the JSON envelope fix) New test test_pack_json_envelope_sources_error_from_tag_push_when_push_refuses is the regression-trap for the python-architect blocking finding.

apm-primitives-architect

No findings. PR does not touch .apm/, .github/, skills, agents, or workflow primitives.

auth-expert -- inactive

Git operations delegate to git's own credential resolution; APM AuthResolver and token_manager are unchanged. The auth-protocol boundary lint (scripts/lint-auth-signals.sh) is clean.

This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.

@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

Completion confirmation: ready to merge

Verified branch feat/pack-create-tag-push @ a06d6a59 (already at origin HEAD; no additional push needed).

Panel follow-through

All blocking findings from the apm-review-panel (#issuecomment-4549935384) addressed in fold-in commit a06d6a59:

  • JSON envelope sourcing bug (python-architect) - fixed + regression-trapped
  • single_version -> {lockstep, tag_pattern, per_package} doc correction (doc-writer / oss-growth)
  • reference/cli/pack.md parity update (doc-writer / devx-ux)
  • -- separator on git tag and defensive remote-name check (supply-chain) [recommended-high]
  • TaggingRefusal.hint rendering in create/push catch blocks (cli-logging) [recommended]
  • ls-remote failure log elevated to warning (supply-chain) [recommended]

Verification evidence

CI: 13 SUCCESS (Lint, both Build & Test shards, Coverage Combine, APM Self-Check, PR Binary Smoke, CodeQL+Analyze x2, NOTICE Drift, gate, build, license/cla).

Lint contract (all silent / exit 0):

  • ruff check src/ tests/: All checks passed
  • ruff format --check src/ tests/: 1109 files already formatted
  • pylint R0801 (min-similarity 10): 10.00/10
  • scripts/lint-auth-signals.sh: clean (no exemptions needed; git push is wrapped via apm_cli.utils.git_subprocess.run_git)

Targeted tests: pytest tests/unit/commands tests/integration -k 'pack or tag or release' -> 2051 passed, 31 skipped in 30s.

File-length and structure

  • src/apm_cli/commands/pack.py: 987 lines (cap 2450)
  • src/apm_cli/release/git_tagger.py: 406 lines
  • src/apm_cli/utils/git_subprocess.py: 73 lines (shared sanitized git wrapper; run_git is the single subprocess.run callsite, used by git_tagger and re-used safely across companion PRs)

Recommendation: ready to merge. Follow-ups #1-#5 from the panel comment remain as separate clean-up PRs and do not gate this feature.

Completion subagent (batch-bug-shepherd)

danielmeppiel and others added 2 commits May 27, 2026 13:35
Adds 5 e2e tests against real git fixtures (working repo + local bare
remote) to defend the user-visible promises of
'apm pack --check-versions --create-tag --push' that the existing
suite did not exercise end-to-end:

- B: 'strategy: tag_pattern' + 'build.tagPattern' renders the
  configured template (one tag per package, both local and remote).
- F: '--check-versions' failure exits 3 AND leaves NO tag on disk
  (closes the silent-side-effect gap the unit-tier test did not check).
- G: idempotent re-run refuses cleanly with 'tag_exists' (exit 1) and
  produces no duplicate tag.
- E: push refuses fail-closed when the tag already exists on origin
  (via 'ls-remote' preflight) -- proven by staging the remote tag
  from a sibling clone before the run.
- J: '--json' envelope keeps 'tag_creation' / 'tag_push' shape stable
  across success AND refusal in one back-to-back invocation.

Each new test is mutation-break verified: deleting the production
guard it defends causes the assertion to fail. Mutation log:
- B: 'pattern = "{name}-v{version}"' in plan_tags -> assertion fails
- F: drop 'return None, None, False' on gate fail -> v5.0.0 leaks
- G: 'if False and existing_local' -> refusal_code becomes git_failure
- E: 'if False and existing_remote' -> exit 0 with full success
- J: drop 'refusal_code: None' from success payload -> KeyError

No production code changed; tests-only delta.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

E2E test-coverage addendum (test-coverage panelist)

Folded 5 mutation-break-gated e2e tests into tests/integration/release/test_pack_tagging_e2e.py covering user-visible promises the existing suite did not exercise end-to-end with real git fixtures.

Promise New e2e test
B - strategy: tag_pattern + build.tagPattern renders configured template (local + remote) test_pack_tag_pattern_strategy_creates_templated_tag_e2e
F - --check-versions failure exits 3 AND leaves NO tag on disk test_pack_version_mismatch_blocks_tag_no_side_effects_e2e
G - Idempotent re-run refuses cleanly with tag_exists (exit 1, no duplicate) test_pack_rerun_refuses_when_tag_exists_locally_e2e
E - --push refuses fail-closed when tag already on remote (via ls-remote preflight) test_pack_push_refuses_when_tag_already_on_remote_e2e
J - --json envelope keeps tag_creation / tag_push shape stable across success AND refusal test_pack_json_envelope_stable_across_success_and_refusal_e2e

Fixture strategy: real git fixtures throughout (working repo + local bare origin + sibling clone for remote-staged-tag scenario). No subprocess mocks at the boundary.

Each test was mutation-break verified: deleting the production guard it defends caused the test to fail. Restored before push; production code unchanged (tests-only delta, +353 LOC).

CI green: lint, ruff format, R0801, auth-signal, all test shards, binary smoke. No drift, no /tmp leakage, ASCII-only.

Status: ready-to-merge from the test-coverage angle.

@danielmeppiel
Copy link
Copy Markdown
Collaborator Author

Closing: YAGNI on this surface

After review, closing this PR (and #1489 as not-planned). The engineering is solid — drift gates, refusal codes, dry-run, JSON output, e2e tests all worked as designed. What changed is the strategic read of whether the surface belongs in APM at all.

The real gap is ~30 seconds of git

The producer workflow today, using only what already ships:

apm pack --check-versions --check-clean       # validate (existing flags)
gh release create v1.2.3 --generate-notes     # tag + GH release in one atomic command
# microsoft/apm-action `mode: release` auto-triggers on the tag push

Three steps, two well-known tools, zero new APM surface. The PR was wrapping git tag + git push origin <tag> (two universal commands) with marketplace-specific refusal codes. The cost — new flag surface on already-bloated apm pack, lifetime maintenance, divergence from universal gh/git muscle memory, tutorials that hardcode our boilerplate — outweighs saving 30 seconds once per release.

What we keep

The drift-gate work is good and stays as-is on apm pack:

  • --check-versions (version-alignment gate, pure local, no network)
  • --check-clean (working-tree drift gate)
  • Refusal codes and --json output for scriptable producer CI

These compose cleanly with gh release create and microsoft/apm-action mode: release, which already does validate → matrix-pack → sha256 sidecars → stage marketplace.json → gh release create end-to-end.

Follow-ups (separate, smaller)

  • Docs: write docs/src/content/docs/producer/release.md describing the three-step ritual above. The actual missing piece is discoverability, not a feature.
  • Optional: enhance microsoft/apm-action mode: release with a validate-before-release input that runs apm pack --check-versions --check-clean as the first step — puts the gate in CI, which is the right architectural place.

Sunk cost acknowledgment

This PR went through four panel rounds, e2e test fold-ins, and a Copilot review pass. None of that was wasted — the drift-gate validation, the bare-repo fixture pattern, and the refusal-code design all inform the codebase going forward. The decision to not ship is about surface, not engineering.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(pack): --create-tag and --push flags to create and push release tag after --check-versions

2 participants