feat(pack): switch --archive to .zip and add --archive-format zip|tar.gz

[nadav-y]

Summary

• Switch default archive format from .tar.gz to .zip for apm pack --archive. ZIP is the format Claude Code and other plugin hosts expect, gives better Windows compatibility, and aligns with apm publish's auto-pack format.
• Add --archive-format [zip|tar.gz] so callers who need .tar.gz (existing CI pipelines, tooling that expects gzip) can opt in explicitly without a breaking change.
• Add backward-compat .tar.gz extraction to apm unpack / apm install so existing .tar.gz bundles continue to work after producers migrate to .zip.

What changed

File	Change
bundle/packer.py	import tarfile; archive block branches on archive_format; passes format to export_plugin_bundle
bundle/plugin_exporter.py	Same archive_format param + branch in section 15
bundle/unpacker.py	New primary .zip extraction path; legacy .tar.gz path kept for backward compat; symlink detection via external_attr for zip entries
commands/pack.py	--archive help updated; new --archive-format [zip|tar.gz] option (default zip)
core/build_orchestrator.py	bundle_archive_format: str = "zip" field in BuildOptions; threaded to pack_bundle
Tests	Updated to zip glob / zip creation / zip verification; backward-compat tar.gz path covered

Usage

# Default (zip):
apm pack --archive

# Explicit zip (same result):
apm pack --archive --archive-format zip

# Legacy tar.gz for pipelines that need it:
apm pack --archive --archive-format tar.gz

Test plan

• pytest tests/unit/test_packer.py tests/unit/test_unpacker.py tests/integration/test_pack_unpack_e2e.py — all pass
• pytest tests/unit/commands/test_pack_cli_surface.py tests/unit/commands/test_pack_phase3.py — all pass
• apm pack --help shows --archive-format [zip|tar.gz] [default: zip]
• apm pack --archive produces .zip
• apm pack --archive --archive-format tar.gz produces .tar.gz
• apm install <bundle>.tar.gz still works (backward compat)

[Copilot]

Pull request overview

This PR switches apm pack --archive to produce .zip bundles by default, adds --archive-format zip|tar.gz for explicit selection, and extends bundle extraction to support .zip while keeping .tar.gz as a legacy-compatible input.

Changes:

• Default apm pack --archive output changes from .tar.gz to .zip, with a new --archive-format option to opt into .tar.gz.
• Bundle packing/exporting now branches on archive_format to write either ZIP or tar.gz archives.
• Bundle unpacking adds a primary ZIP extraction path with traversal + symlink-entry defenses, while retaining the existing tar.gz path.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/apm_cli/commands/pack.py	Updates CLI help text for archive output; adds `--archive-format zip
src/apm_cli/core/build_orchestrator.py	Threads new bundle_archive_format option from CLI into bundle production.
src/apm_cli/bundle/packer.py	Implements archive-format branching for APM-format bundle archives.
src/apm_cli/bundle/plugin_exporter.py	Implements archive-format branching for plugin-format bundle archives.
src/apm_cli/bundle/unpacker.py	Adds ZIP extraction + security validation; preserves legacy tar.gz extraction.
tests/unit/test_packer.py	Updates unit expectations/validation from tar.gz to zip for pack_bundle(..., archive=True).
tests/unit/test_unpacker.py	Updates archive helper to generate ZIP archives for unpack unit tests.
tests/unit/commands/test_pack_phase3.py	Updates hardcoded bundle file names in CLI-related tests from .tar.gz to .zip.
tests/unit/commands/test_pack_cli_surface.py	Same .tar.gz -> .zip updates for CLI surface tests.
tests/integration/test_pack_unpack_e2e.py	Updates end-to-end pack/unpack flow to look for *.zip artifacts.
tests/integration/test_wave6_init_pack_coverage.py	Updates --archive docstring expectation to .zip.
tests/integration/test_wave3_marketplace_coverage.py	Updates --archive docstring expectation to .zip.

[github-actions]

APM Review Panel: needs_rework

ZIP-by-default is the right call, but two correctness gaps -- broken apm install success tip and zero ZIP security tests on the new default extraction path -- must be fixed before merge.

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

All seven active panelists endorse the strategic direction without reservation: ZIP-by-default aligns APM with Claude Code expectations, removes the WSL/tar dependency for Windows CI pipelines, and completes the publish arc. There is no panel disagreement on direction. Every finding below is about execution gaps, not about whether this change belongs in APM.

Two findings carry test evidence and must be closed in this PR before merge. First, cli-logging-expert demonstrated via source inspection that detect_local_bundle() in local_bundle.py does not recognize .zip -- only .tar.gz and .tgz. The post-pack success message tells users to run apm install ./bundle.zip, but that command silently falls through to registry-install with a confusing error because the local-bundle fast path never fires. This PR changed the default output format without updating the recognizer that gates local install. The fix is 2-3 lines in local_bundle.py. Shipping a success tip that instructs users to run a broken command is a hard correctness regression in the default path; it cannot be deferred. Second, test-coverage-expert confirmed via exhaustive grep (zero matches for external_attr, unpack_bundle.*zip, and zip.*traversal in all test files) that the new ZIP extraction security gates -- path traversal rejection, absolute path rejection, external_attr symlink detection -- have zero test coverage. ZIP is now the DEFAULT extraction path. The tar.gz equivalents have explicit tests (test_unpack_with_path_traversal_tarball, test_unpack_with_symlink_tarball). Shipping new security-critical code on the default path with no tests is not acceptable regardless of whether the production implementation is correct. Both of these items are fast to fix and non-negotiable.

Three additional items are strong candidates for a companion PR that the maintainer must commit to opening before or at merge. (1) The ensure_path_within gap in packer.py was independently confirmed by python-architect and supply-chain-security-expert: a malicious apm.yml pkg_name such as ../evil can escape output_dir during pack. Two independent panelists reaching the same safety conclusion elevates this above its per-panelist recommended classification. (2) The ZIP bomb gap -- no uncompressed-size or entry-count guard before zf.extractall -- is a real denial-of-service surface that pairs naturally with the path-traversal fix. (3) ci-cd.md and gh-aw.md contain live tar xzf build/*.tar.gz CI commands that break silently for upgrading users, and no CHANGELOG entry exists for a default-changing behavior. The missing CHANGELOG entry is a governance gap, not a docs nit -- APM policy requires a migration line for every breaking change. These three items can land as a companion PR, but the commitment to that PR must be explicit before this branch merges.

Dissent. No panelist disagreed on direction or top findings. Two elevation notes: Both python-architect and supply-chain-security-expert independently classified ensure_path_within as recommended, but independent corroboration of a write-path-traversal gap from a security panelist and an architecture panelist warrants weighting it at near-blocking; treated as highest-priority companion PR item. The performance-expert's size-regression finding (ZIP 30-127% larger for typical bundle content) is measured, not estimated; weighted as a required CHANGELOG disclosure rather than a nit, even though the finding was classified recommended.

Aligned with: portable-by-manifest (improved -- ZIP removes WSL/tar dependency on Windows CI), secure-by-default (two gaps pending: ensure_path_within in packer.py and ZIP security tests in unpacker.py), governed-by-policy (CHANGELOG entry missing for a default-changing behavior), multi-harness-multi-host (improved -- ZIP aligns with Claude Code and plugin host expectations), oss-community-driven (neutral pending ci-cd.md and gh-aw.md docs fix), pragmatic-as-npm (format choice sound; rollout needs runtime migration hint and CHANGELOG).

Growth signal. The oss-growth-hacker correctly identifies that the Windows angle is the strongest unconveyed adoption story: APM pack now produces a ZIP that Windows CI can consume without WSL or a tar binary, making APM viable for Windows-first AI plugin pipelines with no friction. The combined narrative -- 'APM speaks ZIP end-to-end, the format Claude Code expects' -- is a concrete differentiator that should lead the CHANGELOG entry and the PR description. The biggest adoption risk is ci-cd.md's live tar xzf command, which will silently break upgrading users' CI pipelines and generate support noise that directly undercuts the narrative. Fix the docs before or at merge; lead the release note with the Windows and Claude Code angle, and disclose the size tradeoff (30-127% larger) as a one-liner so upgraders are not surprised.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	2	1	Well-motivated switch; packer.py skips ensure_path_within that plugin_exporter.py applies; extract shared _write_archive helper.
CLI Logging Expert	1	2	2	Post-pack success tip instructs 'apm install ./bundle.zip' but detect_local_bundle() in local_bundle.py does not recognize .zip -- command silently fails.
DevX UX Expert	0	3	2	ZIP-by-default is sound; no runtime hint for breaking default change; --archive-format silently no-ops without --archive; three doc surfaces stale.
Supply Chain Security Expert	0	3	1	Pre-validation chain is sound; packer.py omits ensure_path_within for archive_path; no ZIP bomb guard before extractall; Python 3.12 rglob symlink gap.
OSS Growth Hacker	0	2	1	Strong strategic alignment; missing CHANGELOG; ci-cd.md 'tar xzf' command breaks silently for upgrading users.
Doc Writer	0	4	1	Five documentation surfaces now incorrect: CHANGELOG, pack.md, ci-cd.md/gh-aw.md (live CI commands break), commands.md, unpack.md/install.md.
Test Coverage Expert	1	1	0	ZIP extraction security checks (path traversal, symlink, absolute path) have zero test coverage despite being the new default extraction path.
Performance Expert	0	3	3	ZIP archives are 30-127% larger for typical APM bundles (measured); size regression is real and undocumented in CHANGELOG.

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

Top 5 follow-ups

1. [CLI Logging Expert] (blocking-severity) Update local_bundle.py detect_local_bundle() and _looks_like_archive() to recognize .zip so the post-pack success message gives users a command that actually works. -- This is a correctness regression in the default path: the success tip instructs users to run a broken command. Fix is 2-3 lines. Must land in this PR before merge.

2. [Test Coverage Expert] (blocking-severity) Add ZIP extraction security tests to test_unpacker.py covering path traversal, absolute path rejection, and external_attr symlink detection -- mirroring the existing test_unpack_with_path_traversal_tarball and test_unpack_with_symlink_tarball tests for the tar.gz path. Also add a test for --archive-format tar.gz round-trip. -- ZIP is now the DEFAULT extraction path. Exhaustive grep confirms zero test coverage for these cases. New security-critical code on the default path with no automated guardrail is a regression trap. Must land in this PR before merge.

3. [Python Architect + Supply Chain Security Expert] Add ensure_path_within(archive_path, output_dir) in packer.py (both archive branches) and an uncompressed-size/entry-count guard before zf.extractall in unpacker.py. -- Two independent panelists confirmed the write-path-traversal gap: a pkg_name like ../evil in apm.yml escapes output_dir at pack time. The ZIP bomb guard prevents denial-of-service. Should land in this PR or a companion PR opened before merge.

4. [Doc Writer + OSS Growth Hacker + DevX UX Expert] Replace tar xzf build/*.tar.gz with the .zip equivalent in ci-cd.md and gh-aw.md. Add CHANGELOG [Unreleased] entry for the default format change (size tradeoff disclosure + migration note). Add --archive-format to pack.md options table. Emit a runtime warning when --archive-format is passed without --archive. -- Three panelists flagged these surfaces independently. The CHANGELOG gap violates APM governance policy; the ci-cd.md CI command breaks silently for upgrading users.

5. [Python Architect] Extract a shared _write_archive() helper from the duplicate archive-writing blocks in packer.py and plugin_exporter.py. Type-narrow archive_format to Literal['zip', 'tar.gz'] in all function signatures and BuildOptions. -- The two blocks have already drifted (packer.py missing ensure_path_within); the shared helper prevents future divergence and eliminates the security gap by construction.

Architecture

classDiagram
    direction LR
    class BuildOrchestrator {
        <<Coordinator>>
        -_producers list
        +run(options, logger) BuildResult
    }
    class ArtifactProducer {
        <<Protocol>>
        +kind OutputKind
        +produce(options, logger) ProducerResult
    }
    class BundleProducer {
        <<ConcreteStrategy>>
        +kind OutputKind
        +produce(options, logger) ProducerResult
    }
    class BuildOptions {
        <<ValueObject>>
        +bundle_archive bool
        +bundle_archive_format str
        +bundle_format str
        +bundle_output Path
        +dry_run bool
    }
    class PackResult {
        <<ValueObject>>
        +bundle_path Path
        +files list
    }
    class pack_bundle {
        <<Function>>
        +archive bool
        +archive_format str
        +fmt str
    }
    class export_plugin_bundle {
        <<Function>>
        +archive bool
        +archive_format str
    }
    class unpack_bundle {
        <<Function>>
        +bundle_path Path
    }
    note for pack_bundle "packer.py apm fmt: NO sanitize NO ensure_path_within"
    note for export_plugin_bundle "plugin_exporter.py: sanitize then ensure_path_within both branches"
    BuildOrchestrator o-- ArtifactProducer : manages
    BundleProducer ..|> ArtifactProducer : implements
    BundleProducer ..> BuildOptions : reads
    BundleProducer ..> pack_bundle : calls
    pack_bundle ..> export_plugin_bundle : delegates plugin fmt
    pack_bundle ..> PackResult : returns
    export_plugin_bundle ..> PackResult : returns
    class BuildOptions:::touched
    class pack_bundle:::touched
    class export_plugin_bundle:::touched
    class unpack_bundle:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    CLI["apm pack --archive --archive-format zip|tar.gz"]
    BO["BuildOptions.bundle_archive_format - build_orchestrator.py:44"]
    ORCH["BuildOrchestrator.run - build_orchestrator.py:414"]
    BP["BundleProducer.produce - build_orchestrator.py:98"]
    PB["pack_bundle - packer.py:26"]
    EPB["export_plugin_bundle - plugin_exporter.py:406"]
    ARCH1{"archive_format? plugin fmt"}
    ARCH2{"archive_format? apm legacy fmt"}
    ZIP1["ZipFile.write - plugin_exporter.py:662"]
    TAR1["tarfile.open - plugin_exporter.py:654"]
    EPW1["ensure_path_within archive_path - plugin_exporter.py:661"]
    EPW2["ensure_path_within archive_path - plugin_exporter.py:653"]
    RMTREE1["shutil.rmtree bundle_dir - plugin_exporter.py:669"]
    ZIP2["ZipFile.write - packer.py:286"]
    TAR2["tarfile.open - packer.py:279"]
    GAP["GAP: no sanitize, no ensure_path_within - packer.py:277-294"]
    RMTREE2["shutil.rmtree bundle_dir - packer.py:293"]
    UNPACK["apm unpack .zip - unpacker.py:29"]
    SCAN["pre-validate zip members: path-traversal + symlink - unpacker.py:66-83"]
    EXTALL["zf.extractall temp_dir - unpacker.py:84"]
    FINAL["shutil.rmtree temp_dir finally - unpacker.py:282"]
    CLI --> BO --> ORCH --> BP --> PB
    PB -->|"fmt=plugin (default)"| EPB
    PB -->|"fmt=apm legacy"| ARCH2
    EPB --> ARCH1
    ARCH1 -->|"zip (new default)"| ZIP1
    ARCH1 -->|"tar.gz (opt-in)"| TAR1
    ZIP1 --> EPW1 --> RMTREE1
    TAR1 --> EPW2 --> RMTREE1
    ARCH2 -->|"zip"| ZIP2
    ARCH2 -->|"tar.gz"| TAR2
    ZIP2 --> GAP
    TAR2 --> GAP
    GAP --> RMTREE2
    UNPACK --> SCAN --> EXTALL --> FINAL

Loading

Recommendation

The direction is correct and must ship. ZIP-by-default is the right call for Claude Code compatibility and Windows CI adoption, and the panel is unanimous on this. The rework ask is narrow and fast.

Two items must be fixed in this PR before merge. First, update local_bundle.py to recognize .zip so the post-pack success message gives users a command that actually works. This is 2-3 lines; there is no justification for shipping a success tip that instructs users to run a broken command. Second, add ZIP extraction security tests to test_unpacker.py covering path traversal, absolute path rejection, and symlink detection via external_attr. ZIP is now the default extraction path, the security gates are new code, and the tar.gz equivalents have explicit tests. Both items are fast to address in the existing PR branch.

Before merging, the maintainer should also open a companion PR covering: ensure_path_within in packer.py (confirmed write-path-traversal gap), the ci-cd.md and CHANGELOG updates (the live tar xzf CI command breaks upgrading users silently), and the ZIP bomb guard in unpacker.py. The companion PR does not need to merge first, but the commitment to it must be explicit -- a tracking issue at minimum -- before this branch lands. Once the two items are fixed in the PR and the companion PR is opened, this change ships cleanly and the Windows plus Claude Code narrative is ready to amplify.

---

Full per-persona findings

Python Architect

• [recommended] packer.py archive block skips _sanitize_bundle_name and ensure_path_within that plugin_exporter.py applies for both formats (src/apm_cli/bundle/packer.py:276)
  plugin_exporter.py applies three layered guards before writing an archive: (1) _sanitize_bundle_name(pkg_name/pkg_version), (2) ensure_path_within(bundle_dir, output_dir), (3) ensure_path_within(archive_path, output_dir) for each branch. packer.py constructs archive_path directly from raw pkg_name and pkg_version with none of these guards. A pkg_name value of ../evil would produce an archive_path that escapes output_dir silently.
  Suggested: Add ensure_path_within import to packer.py. Apply _sanitize_bundle_name to pkg_name/pkg_version, then call ensure_path_within(archive_path, output_dir) in both branches, mirroring plugin_exporter.py:646-649 and plugin_exporter.py:653/661.

• [recommended] Identical archive-writing blocks in packer.py and plugin_exporter.py have already drifted; extract a shared _write_archive helper (src/apm_cli/bundle/packer.py:276)
  packer.py:276-294 and plugin_exporter.py:650-670 contain functionally identical zip/tar.gz branch code. The only diff is that plugin_exporter.py calls ensure_path_within and packer.py does not -- exactly the divergence that duplicated blocks produce over time.
  Suggested: Extract to _write_archive(bundle_dir, archive_path, archive_format, output_dir) that calls ensure_path_within internally, eliminating the security gap by construction.

• [nit] archive_format: str should be Literal['zip', 'tar.gz'] in function signatures and BuildOptions (src/apm_cli/bundle/packer.py:32)
  click.Choice validates at the CLI boundary, but internal callers pass a raw string with no type-checker enforcement. Literal makes the two-value contract explicit to mypy/pyright.
  Suggested: from typing import Literal then Literal['zip', 'tar.gz'] in pack_bundle, export_plugin_bundle, and BuildOptions.bundle_archive_format.

CLI Logging Expert

• [blocking] 'Share with: apm install (bundle).zip' emits a command that does not work (src/apm_cli/bundle/local_bundle.py:134)
  Before this PR, apm pack --archive produced .tar.gz; detect_local_bundle() matches .tar.gz via _looks_like_archive(). After this PR, the default output is .zip, but _looks_like_archive() only returns True for .tar.gz/.tgz -- not .zip. detect_local_bundle() returns None for .zip paths; the install command falls through to registry-install with a confusing unrelated error. local_bundle.py was NOT modified by this PR.
  Suggested: Extend _looks_like_archive() to also match .zip and add the corresponding zip extraction branch to detect_local_bundle() (mirroring the .zip extraction logic already in unpacker.py). Also extend the IM7 guard in install.py to check '.zip'.

• [recommended] --archive-format help text gives no basis for choosing between zip and tar.gz (src/apm_cli/commands/pack.py:179)
  Current: 'Archive format when --archive is set.' The PR body has the right framing (Claude Code compatibility vs legacy CI) but it never reaches the terminal help text.
  Suggested: "Archive format: 'zip' (default, Claude Code and plugin-host compatible; matches apm publish output) or 'tar.gz' (legacy CI pipelines that depended on the previous default)."

• [recommended] Three stale .tar.gz-only references in install.py user-facing strings not updated (src/apm_cli/commands/install.py:1100)
  (1) --as option help at line 1100, (2) example usage at line 1176, (3) --as error message at line 1326 all name only .tar.gz as the accepted archive format.
  Suggested: Change '(directory or .tar.gz produced by apm pack)' to '(directory, .zip, or .tar.gz archive produced by apm pack)'.

• [nit] --archive help cross-reference does not surface the default format (src/apm_cli/commands/pack.py:171)
  Suggested: 'Produce an archive instead of a directory. Default format is .zip (see --archive-format to change).'

• [nit] local_bundle.py module docstring and is_archive docstring still describe .tar.gz as the only archive format (src/apm_cli/bundle/local_bundle.py:4)
  Suggested: Update both to include '.zip or legacy .tar.gz'.

DevX UX Expert

• [recommended] No runtime hint on the silent default-format change: existing scripts will break downstream (src/apm_cli/commands/pack.py)
  apm pack --archive previously produced .tar.gz; it now produces .zip with zero terminal output indicating the change. CI pipelines that run tar xzf *.tar.gz or ls *.tar.gz after pack will fail silently -- APM exits 0, the downstream shell command fails.
  Suggested: In _render_bundle_result or in pack_cmd after a successful archive, emit logger.info or logger.warning when archive=True and archive_format='zip', pointing to --archive-format tar.gz for pipelines that relied on the old default.

• [recommended] --archive-format silently no-ops without --archive; should warn (src/apm_cli/commands/pack.py)
  archive_format is accepted by the CLI and forwarded to pack_bundle() but is only evaluated inside the if archive: guard. Running apm pack --archive-format tar.gz without --archive produces a directory, discards the flag, and exits 0 with no warning.
  Suggested: After effective_target assignment, add: if archive_format != 'zip' and not archive: logger.warning('--archive-format has no effect without --archive').

• [recommended] Three doc surfaces still describe --archive as producing .tar.gz; --archive-format absent from options table (docs/src/content/docs/reference/cli/pack.md)
  (a) pack.md line 32 says --archive produces .tar.gz; --archive-format missing; (b) pack-a-bundle.md line 45 says '.tar.gz instead of a directory'; (c) commands.md line 83 lists --archive but not --archive-format.
  Suggested: Update pack.md --archive row and add --archive-format row. Update pack-a-bundle.md .tar.gz examples to .zip. Add --archive-format to commands.md.

• [nit] --archive help text defers to cross-reference instead of naming the default format (src/apm_cli/commands/pack.py)
  Suggested: help='Produce a .zip archive instead of a directory (use --archive-format tar.gz for legacy pipelines).'

• [nit] --archive-format help text does not explain when to choose each value (src/apm_cli/commands/pack.py)
  Suggested: help='Archive format when --archive is set. Use tar.gz for CI pipelines that relied on the previous default.'

Supply Chain Security Expert

• [recommended] packer.py omits ensure_path_within for archive_path; pkg_name from apm.yml can escape output_dir (src/apm_cli/bundle/packer.py:278)
  plugin_exporter.py calls ensure_path_within(archive_path, output_dir) for both archive format branches. packer.py constructs archive_path directly from output_dir / f'{pkg_name}-{pkg_version}.{ext}' where pkg_name/pkg_version come from apm.yml (user-controlled). A name like ../../cron.d/evil causes archive_path.resolve() to escape output_dir.
  Suggested: After each archive_path assignment (lines 278 and 285), add ensure_path_within(archive_path, output_dir), mirroring plugin_exporter.py:653 and :661.

• [recommended] No uncompressed-size or entry-count guard before zf.extractall; ZIP bomb denial-of-service possible (src/apm_cli/bundle/unpacker.py:67)
  The pre-validation loop checks path safety but never reads member.file_size. A crafted ZIP with high compression ratio can decompress to gigabytes. Python's zipfile imposes no decompression limit.
  Suggested: Accumulate total = sum(m.file_size for m in zf.infolist()) and check entry count. Raise ValueError before extractall if total exceeds a size cap (e.g., 512 MB) or entry count exceeds a count cap (e.g., 10000).

• [recommended] Python 3.12 rglob follows symlinked directories by default; is_symlink() guard misses transitive files on that version (src/apm_cli/bundle/plugin_exporter.py:655)
  APM declares requires-python >= 3.10. In Python 3.12, Path.rglob() changed its default to follow symbolic links to subdirectories. If bundle_dir contains a symlink to an external directory, rglob yields every file inside that external directory; those files have is_symlink() == False and is_file() == True and pass the guard. The old tarfile.add with _tar_filter never recursed into symlinked directories.
  Suggested: After the is_symlink() check, add ensure_path_within(fp, bundle_dir) (catching PathTraversalError and skipping) to reject files reachable only through symlinked directories regardless of Python version.

• [nit] Windows-format ZIP symlink detection via external_attr is a known limitation aligned with Python stdlib (src/apm_cli/bundle/unpacker.py:82)
  The check reads Unix file-type bits; ZIPs created by Windows tools leave the upper 16 bits zero. However, Python's zipfile.extractall() uses the same external_attr mechanism -- the check and the extractor are in sync. No exploitable gap; worth documenting as a known limitation in a code comment.

OSS Growth Hacker

• [recommended] Missing CHANGELOG entry for a default-changing PR -- trust risk for upgrading users
  The [Unreleased] section records the apm publish zip switch but is entirely silent on apm pack --archive now emitting .zip. An operator running apm upgrade has no signal that their artifact filename changed. The escape hatch only helps users who know they need it.

• [recommended] Three docs pages become stale copy-paste traps on merge -- including a live CI command that silently fails
  Confirmed stale: (1) pack.md options table still says .tar.gz; (2) pack-a-bundle.md shows .tar.gz output examples; (3) ci-cd.md contains tar xzf build/*.tar.gz -C ./ which silently fails for upgrading users copying the snippet.

• [nit] The Windows story is the strongest adoption angle and it is unplayed in the PR body
  ZIP is natively extractable on Windows without WSL, GNU tar, or third-party tools. Worth one sentence in the CHANGELOG entry and a phrase in the release note.

Auth Expert -- inactive

No auth files touched; PR changes archive format (zip vs tar.gz) in bundle/packer.py, bundle/plugin_exporter.py, bundle/unpacker.py, commands/pack.py, and core/build_orchestrator.py -- no authentication, token management, credential resolution, or AuthResolver surface is affected.

Doc Writer

• [recommended] CHANGELOG.md has no [Unreleased] entry for the apm pack --archive format change or the new --archive-format flag (CHANGELOG.md)
  Suggested: Add two bullets under ### Added in [Unreleased]: (1) apm pack --archive now produces .zip by default instead of .tar.gz; (2) apm pack --archive-format [zip|tar.gz] escape hatch for legacy CI.

• [recommended] reference/cli/pack.md option table describes --archive as producing .tar.gz and is missing --archive-format (docs/src/content/docs/reference/cli/pack.md)
  Line 32: '--archive | off | Produce a .tar.gz archive instead of a directory.' -- now wrong. --archive-format has no row in the options table.
  Suggested: Update line 32 and add --archive-format [zip|tar.gz] row immediately after. Fix line 53 comment and line 206 reference.

• [recommended] integrations/ci-cd.md and integrations/gh-aw.md show tar xzf restore commands that break against the new .zip default (docs/src/content/docs/integrations/ci-cd.md)
  ci-cd.md line 211: tar xzf build/*.tar.gz -C ./. gh-aw.md line 126: similar claim. These will fail at runtime for upgrading users.
  Suggested: Update restore guidance to unzip -d ./ build/*.zip, or show --archive-format tar.gz explicitly for pipelines requiring tar.gz.

• [recommended] packages/apm-guide/.apm/skills/apm-usage/commands.md -- apm pack options column missing --archive-format (packages/apm-guide/.apm/skills/apm-usage/commands.md)
  Line 83 lists --archive but not --archive-format. This is the authoritative machine-readable CLI summary consumed by agent skills.
  Suggested: Append --archive-format [zip|tar.gz] (default zip) to the apm pack options cell.

• [nit] reference/cli/unpack.md and reference/cli/install.md still describe only .tar.gz as a valid bundle archive format (docs/src/content/docs/reference/cli/unpack.md)
  Suggested: Change 'a .tar.gz archive' to 'a .zip or .tar.gz archive' in unpack.md. Add .zip to packed bundles description in install.md.

Test Coverage Expert

• [blocking] New ZIP-extraction security checks in unpacker.py -- path traversal, absolute path, external_attr symlink -- have no test; tar.gz equivalents are well covered (src/apm_cli/bundle/unpacker.py)
  Probed: (a) grep'd tests/ for 'external_attr' combined with 'unpack_bundle' -- zero matches; (b) read tests/unit/test_unpacker.py in full -- security tests at lines 191 and 205 exercise the directory bundle path, NOT the ZIP extraction path; (c) grep'd for 'zip.*traversal' and 'unpack_bundle.*zip' -- zero hits. By contrast, tar.gz equivalents DO have tests: test_unpack_with_path_traversal_tarball (test_wave4_pure_logic_coverage.py:945), test_unpack_with_symlink_tarball (test_wave4_pure_logic_coverage.py:962).
  Proof (missing): tests/unit/test_unpacker.py::test_unpack_zip_rejects_path_traversal_member -- proves: apm unpack rejects ZIP archives whose member names contain .. or absolute paths, and rejects Unix symlinks stored via external_attr [secure-by-default]

• [recommended] --archive-format tar.gz CLI flag and packer.py tar.gz branch are untested at every tier (src/apm_cli/bundle/packer.py)
  grep'd tests/ for 'archive_format', 'archive-format', 'archive_format.*tar' -- zero matches in executable test code. The unit test test_pack_archive calls pack_bundle(project, out, archive=True) without archive_format, exercising only the default zip branch. packer.py lines 277-283 (the tar.gz branch) has zero test coverage.
  Proof (missing): tests/integration/test_wave6_init_pack_coverage.py::test_pack_archive_format_tar_gz -- proves: apm pack --archive --archive-format tar.gz produces exactly one .tar.gz archive in the build directory, not a .zip [devx, portability]

Performance Expert

• [recommended] ZIP archives are 30-127% larger than tar.gz for typical APM bundle content; the size regression is real and undocumented (src/apm_cli/bundle/packer.py)
  Measured on representative APM bundles: 50 varied files produce ZIP=11.7 KB vs tar.gz=5.1 KB (+127%). Root cause: ZIP DEFLATE resets the compression dictionary at every entry boundary; tar.gz compresses as one stream, benefiting from cross-file pattern matching across YAML/Markdown skill files.
  Suggested: Add a one-line note to the CHANGELOG: 'ZIP archives are typically 30-130% larger than tar.gz for text-heavy skill bundles due to per-file compression; use --archive-format tar.gz when archive size is a priority.'

• [recommended] compresslevel is unset in both ZipFile constructors; default (zlib level 6) is correct but not observable (src/apm_cli/bundle/packer.py:286)
  Both packer.py:286 and plugin_exporter.py:662 call ZipFile without compresslevel. Consider exposing as an internal constant _ZIP_COMPRESS_LEVEL = 6 for observability.

• [recommended] No perf benchmarks accompany the default archive format switch; size and speed trade-offs are unquantified in the PR
  The PR changes the default archive format for all users with no test measuring archive size, pack wall-time, or unpack wall-time. A benchmark comment in the PR body establishes a regression baseline.

• [nit] sorted(bundle_dir.rglob(*)) overhead is negligible -- no change warranted (src/apm_cli/bundle/packer.py)
  Measured at 200 entries: sort overhead = 0.67 ms vs compression I/O phase of 60-321 ms. The sort is valuable for deterministic archive member ordering. No change warranted.

• [nit] Two-pass extraction in unpacker.py is not actually two disk passes (src/apm_cli/bundle/unpacker.py)
  ZipFile.infolist() returns a list populated during init; the central directory is read from disk exactly once. The validation loop iterates an in-memory list. Overhead measured: 0.7 ms for 200 files vs 26.3 ms extraction. Negligible.

• [nit] cleanup_temp handling is correct; no temp dir leak identified (src/apm_cli/bundle/unpacker.py)
  cleanup_temp is set to True and temp_dir is assigned before the try block. Exception handling and the outer finally block both handle cleanup correctly across all branches.

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

Generated by PR Review Panel for issue #1720 · sonnet46 22.4M · ◷

[nadav-y]

Addressed the full panel review comments:

detect_local_bundle gap: detect_local_bundle() now handles .zip via a new _extract_zip_bundle() helper that runs the full security chain — path traversal rejection, absolute path rejection, and Unix symlink detection via external_attr — before extractall. _looks_like_archive() is intentionally not changed: adding .zip there would silently route zip files into the tar branch and fail with TarError. The .zip branch is an explicit check that runs before the tar branch. The IM7 guard in install.py is extended to cover .zip as well.

ZIP security tests: TestUnpackZipSecurity in test_unpacker.py now covers path traversal, absolute path, and external_attr-based symlink detection, mirroring the existing tar.gz equivalents. The --archive-format tar.gz round-trip and invalid-format rejection are also covered in test_packer.py and test_pack_cli_surface.py.

Also addressed in this branch rather than deferred: ensure_path_within added to both archive branches in packer.py (mirrors what plugin_exporter.py already did); ZIP bomb guard before extractall in unpacker.py — rejects archives over 512 MB uncompressed or 10,000 entries (the check reads file_size, the declared uncompressed size, so it catches high-compression-ratio attacks before extraction starts); CHANGELOG [Unreleased] entry with the size tradeoff disclosure; ci-cd.md and gh-aw.md restore commands updated from tar xzf to unzip; pack.md options table updated with --archive-format.

Also picked up from the advisory findings: --archive-format warns when passed without --archive; install.py user-facing strings updated to include .zip; help text rewritten with the Claude Code / legacy CI framing.

Summary for the review: the panel reviewed new code, found real issues, but most of those issues were pre-existing on the tar.gz path too. But as those are security issues I think we must patch them at once.

Also, fixed a small issue whereby publish_version in the registry client had Content-Type: application/gzip hard-coded from when publish used to produce tarballs. The archive bytes being uploaded were already a .zip, but the server was being told it was gzip. Fixed to application/zip, parameter renamed from tarball_bytes to archive_bytes

[github-actions]

APM Review Panel: needs_rework

needs_rework: ZIP bomb guard absent from apm install path; releasing-from-any-ci.md CI globs will silently break; 5 doc surfaces contradict the new .zip default

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

Three panelists converge on a confirmed security gap: local_bundle._extract_zip_bundle() accepts user-supplied .zip files, runs path-traversal checks, then calls zf.extractall() with no entry-count or size limit. unpacker.py -- the apm unpack path -- is protected by _MAX_ENTRIES=10,000 and _MAX_UNCOMPRESSED=512 MB at lines 85-96. The asymmetry is real and verified: grep for MAX_ENTRIES/MAX_UNCOMPRESSED returns zero hits in local_bundle.py. The gap is not pre-existing in the relevant sense: ZIP was previously not the default distribution format. This PR promotes apm install <bundle.zip> to the primary user path, making the unguarded extractall() the first thing a new user exercises. test-coverage-expert confirms outcome: missing for TestUnpackZipSecurity::test_zip_bomb_entry_count_rejected against a secure-by-default surface -- under the evidence-weighting rules, a missing test on a secure-by-default promise inherits blocking weight. supply-chain-security additionally flags that the existing guard in unpacker.py sums attacker-controlled ZipInfo.file_size rather than measured output bytes; a crafted ZIP declaring file_size=0 could defeat the 512 MB threshold entirely while extractall() writes unbounded content. This second gap is pre-existing and should be filed as a separate hardening issue, but the maintainer should know it exists before ratifying the guard as the reference implementation to port.

The doc-writer surfaces a second must-fix that is operationally dangerous, not merely stale: releasing-from-any-ci.md contains CI pipeline examples that glob build/*.tar.gz across all four CI provider sections (confirmed at lines 24, 30-31, 99, 104, 123-129, 146-151, 174-180). Once --archive produces .zip by default, those globs silently match nothing, breaking release pipelines for every team that follows the docs. Four additional pages (pack-a-bundle.md, install.md, unpack.md, commands.md) also contradict the new default and will give new users a wrong mental model on day one, but they do not carry the silent-failure risk of the CI globs. All five doc surfaces should be addressed before the next release note.

The core design is sound: ZIP-as-default removes format friction for Windows users and Claude Code plugin authors, --archive-format tar.gz provides a clean escape hatch, and backward-compat extraction is preserved. Fix the bomb guard and the CI globs in this PR, queue the remaining doc sweep and the UsageError fix as immediate follow-ups, and the narrative earns its beat.

Dissent. Severity disagreement on the missing bomb guard: python-architect calls it blocking; supply-chain-security and performance-expert both call it recommended. I side with python-architect: the fix is bounded (port five lines, hoist two constants), the surface is secure-by-default, and the missing test evidence from test-coverage-expert upgrades the finding to blocking weight per evidence-weighting rules regardless of the opinion-only severity labels. devx-ux-expert argues --archive-format without --archive should raise click.UsageError; cli-logging-expert proposes a warning with an action hint. I side with devx-ux-expert: APM pattern is UsageError for inapplicable flags, and the current warning fires only for tar.gz (not explicit zip), producing internally inconsistent CLI behavior. The cli-logging-expert action-hint improvement should be folded into the same UsageError message. This is a followup, not a blocker.

Aligned with: Portable by Manifest, Secure by Default, Multi-Harness / Multi-Host, OSS Community-Driven, Pragmatic as npm

Growth signal. oss-growth-hacker (2026-06-10): this PR completes format alignment -- apm publish (#1695) + apm pack --archive (#1720) now both produce .zip, matching Claude Code and all major plugin hosts. Story angle: "APM now speaks the same format as every plugin host -- no conversion, no tar binary, no WSL." Windows-first launch beat is viable; social post targeting the Claude plugin author community (Anthropic Discord) is recommended. Hold the announcement until the bomb guard and CI doc fixes land -- shipping a known resource-exhaustion gap alongside a launch narrative would undercut the production-ready positioning.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	1	2	2	Correctly switches ZIP to primary and adds --archive-format for backward-compat. ZIP bomb guard absent from local_bundle._extract_zip_bundle(); rglob archive block duplicated in packer.py and plugin_exporter.py; three divergent ZIP validation implementations.
CLI Logging Expert	0	2	3	CLI changes are well-scoped. No-op warning missing an action hint; ZIP-bomb ValueError reports raw byte integers instead of human-readable MB.
DevX UX Expert	0	2	2	Core ergonomics sound. Skill prose in commands.md still lists only .tar.gz for bundle install; --archive-format without --archive should be UsageError not a warning.
Supply Chain Security Expert	0	2	3	ZIP extraction in unpacker.py is solid. Bomb guard absent from _extract_zip_bundle(); guard sums attacker-controlled file_size field not measured output bytes.
OSS Growth Hacker	0	2	2	Strong net-positive for adoption. unpack.md and install.md still describe .tar.gz as the canonical format, creating a mismatch for new users.
Doc Writer	0	5	5	pack.md and ci-cd.md edits accurate. Five additional pages (pack-a-bundle.md, releasing-from-any-ci.md, install.md, unpack.md, commands.md) contradict the new .zip default -- releasing-from-any-ci.md CI globs will silently break.
Test Coverage Expert	0	3	1	Pack unit coverage solid. No integration test for apm install bundle.zip (new default format); no regression trap for the ZIP bomb guard in unpacker.py.
Performance Expert	0	1	3	Format-correctness PR with negligible performance impact. Missing bomb guard in _extract_zip_bundle() is the only substantive gap.

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

Top 5 follow-ups

1. [Python Architect] (blocking-severity) Port _MAX_ENTRIES and _MAX_UNCOMPRESSED bomb guard from unpacker.py lines 85-96 to local_bundle._extract_zip_bundle(); hoist both constants to module level for sharing -- apm install <crafted.zip> is unguarded while apm unpack <crafted.zip> is rejected cleanly; missing test on a secure-by-default surface inherits blocking weight; the fix is five lines.
2. [Doc Writer] Fix releasing-from-any-ci.md: replace all build/*.tar.gz globs (10+ instances across four CI provider sections) with build/*.zip; add a migration callout -- CI pipelines following the docs will silently produce zero release artifacts once --archive defaults to .zip; confirmed at lines 24, 30-31, 99, 104, 123-129, 146-151, 174-180.
3. [Doc Writer] Update pack-a-bundle.md, install.md, unpack.md, and commands.md to show .zip as primary and .tar.gz as legacy escape hatch -- five doc surfaces still describe .tar.gz as canonical, giving new users a wrong mental model on day one; converges findings from doc-writer, oss-growth-hacker, and devx-ux-expert.
4. [Test Coverage Expert] Add hermetic integration test TestInstallLocalBundleE2E::test_install_local_bundle_from_zip in tests/integration/test_install_local_bundle_e2e.py covering detect -> _extract_zip_bundle -> install -> file-deployment -- outcome: missing on a portability-by-manifest surface; the primary distribution path in this PR has no integration-tier guardrail.
5. [DevX UX Expert] Replace the --archive-format-without---archive warning with click.UsageError using ctx.get_parameter_source('archive_format') != ParameterSource.DEFAULT; fold the action-hint into the error message; add a test in test_pack_cli_surface.py -- current warning fires only for tar.gz (not explicit zip), inconsistent with APM flag-misuse pattern.

Architecture

classDiagram
    direction TB

    class BuildOptions {
        <<ValueObject>>
        +bundle_archive bool
        +bundle_archive_format str
    }

    class PackResult {
        <<ValueObject>>
        +bundle_path Path
        +files list
    }

    class LocalBundleInfo {
        <<ValueObject>>
        +is_archive bool
        +temp_dir Path
    }

    class packer {
        <<Module>>
        +pack_bundle(archive_format) PackResult
    }

    class plugin_exporter {
        <<Module>>
        +export_plugin_bundle(archive_format) PackResult
    }

    class unpacker {
        <<Module>>
        +unpack_bundle(bundle_path) UnpackResult
    }

    class local_bundle {
        <<Module>>
        +detect_local_bundle(path) LocalBundleInfo
        -_extract_zip_bundle(path) LocalBundleInfo
    }

    class extractor {
        <<Module>>
        +extract_archive(data, digest, dest) str
        -_safe_extract_zip(zf, dest)
    }

    class BuildOrchestrator {
        <<Orchestrator>>
        +run(options) BuildResult
    }

    BuildOrchestrator ..> BuildOptions : reads
    BuildOrchestrator ..> packer : fmt=apm
    BuildOrchestrator ..> plugin_exporter : fmt=plugin
    packer ..> PackResult : produces
    plugin_exporter ..> PackResult : produces
    local_bundle ..> LocalBundleInfo : produces

    class packer:::touched
    class plugin_exporter:::touched
    class unpacker:::touched
    class local_bundle:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    P1["apm pack --format apm --archive"] --> W1["pack_bundle packer.py:277"]
    P2["apm pack --format plugin --archive"] --> W2["export_plugin_bundle plugin_exporter.py:650"]
    W1 --> WL["FS: identical rglob write-archive block\narchive_format==tar.gz: tarfile.open w:gz\nelse: ZipFile ZIP_DEFLATED + zf.write"]
    W2 --> WL

    I1["apm install local.zip"] --> R2["_extract_zip_bundle local_bundle.py:201"]
    I2["apm unpack bundle.zip"] --> R1["unpack_bundle unpacker.py:59"]
    I3["registry install path"] --> R3["_safe_extract_zip extractor.py:202"]

    R1 --> RV1["validate path+symlink\nbomb guard: _MAX_ENTRIES=10k _MAX_UNCOMPRESSED=512MB\nunpacker.py:85-97"]
    R2 --> RV2["validate path+symlink\nNO bomb guard\nlocal_bundle.py:212-230"]
    R3 --> RV3["stream per-member via zf.open\nno extractall call\nextractor.py:209-233"]

    RV1 --> EX1["FS: zf.extractall temp_dir"]
    RV2 --> EX2["FS: zf.extractall temp_dir"]

    style RV2 fill:#ffcccc,stroke:#cc0000
    style WL fill:#fff3cc,stroke:#cc8800

Loading

Recommendation

Fix two items before merge: (1) port the ZIP bomb guard (_MAX_ENTRIES, _MAX_UNCOMPRESSED) from unpacker.py to local_bundle._extract_zip_bundle() -- five lines, eliminating an unguarded resource-exhaustion path on the new default distribution format; (2) update releasing-from-any-ci.md to replace build/*.tar.gz globs with build/*.zip across all four CI provider sections, preventing silent pipeline breakage for any team following the docs. Both fixes are bounded, reviewable in the same PR, and do not touch the core design. All remaining findings -- stale docs in pack-a-bundle.md, install.md, unpack.md, commands.md; UsageError vs warning; integration test for apm install bundle.zip; the pre-existing file_size accounting weakness in the bomb guard -- are queued as immediate follow-ups to land before the next release note. The core design is correct and the growth signal is real; two trivial fixes are all that stand between this PR and a clean merge.

---

Full per-persona findings

Python Architect

• [blocking] ZIP bomb guard absent in local_bundle._extract_zip_bundle() while present in unpacker.unpack_bundle() at src/apm_cli/bundle/local_bundle.py:201
  Both unpack_bundle() and _extract_zip_bundle() accept user-supplied .zip files and call zf.extractall() after per-member validation. unpacker.py guards with _MAX_ENTRIES=10,000 and _MAX_UNCOMPRESSED=512 MB at lines 85-97. _extract_zip_bundle() applies identical path-traversal and symlink checks but has no bomb guard: apm install evil.zip with 10,001 entries or >512 MB uncompressed will exhaust file descriptors or RAM during extractall(), while apm unpack evil.zip is rejected cleanly.
  Suggested: Add the same guard immediately after zf.infolist() in _extract_zip_bundle(): if len(members) > _MAX_ZIP_ENTRIES or sum(m.file_size for m in members) > _MAX_ZIP_UNCOMPRESSED: shutil.rmtree(temp_dir, ignore_errors=True); return None. Hoist both constants to module level for sharing.

• [recommended] Archive-creation rglob block duplicated verbatim in packer.py section 10 and plugin_exporter.py section 15 at src/apm_cli/bundle/packer.py:277
  Both files contain byte-for-byte identical archive-creation logic: archive_format branch, ensure_path_within() guard, rglob('*') loop with symlink filter, and tarfile.add/zf.write with the same arcname pattern. A future third format requires editing both files; a symlink-filter regression in one will not propagate to the other.
  Suggested: Extract to write_bundle_archive(bundle_dir, output_dir, name, archive_format) -> Path helper in bundle/packer.py.

• [recommended] Three independent ZIP member-validation implementations with diverging safety properties at src/apm_cli/bundle/unpacker.py:65
extractor._safe_extract_zip() checks the full 16-bit Unix mode word; unpacker.py and local_bundle.py check only the upper 4-bit type field. Path-traversal predicates are copy-pasted across files.
  Suggested: Add validate_zip_member(member: zipfile.ZipInfo) -> None to utils/path_security.py and route all three paths through it.

• [nit] _MAX_UNCOMPRESSED and _MAX_ENTRIES defined as block-local variables inside unpacker.py if-branch at src/apm_cli/bundle/unpacker.py:85
  Constants invisible to module-level tooling and cannot be referenced by callers or shared with local_bundle.py.
  Suggested: Hoist to module level: _MAX_ZIP_UNCOMPRESSED = 512 * 1024 * 1024 and _MAX_ZIP_ENTRIES = 10_000.

• [nit] _looks_like_archive() only matches .tar.gz/.tgz -- misleading name at src/apm_cli/bundle/local_bundle.py:135
  A future contributor scanning for archive-type detection will miss the .zip branch dispatched separately.
  Suggested: Rename to _looks_like_tarball().

CLI Logging Expert

• [recommended] No-op warning missing an action hint at src/apm_cli/commands/pack.py
  APM output principle: every warning must include the fix. "--archive-format tar.gz has no effect without --archive" leaves the user to infer the remedy.
  Suggested: logger.warning(f"--archive-format {archive_format} has no effect without --archive -- add --archive to produce a .{archive_format} archive")

• [recommended] ZIP-bomb ValueError reports raw byte integers; human-readable units needed for CI operators at src/apm_cli/bundle/unpacker.py
  "629145600 bytes exceeds 536870912 bytes" requires mental arithmetic. CI operators need MB units to quickly assess severity.
  Suggested: raise ValueError(f"ZIP archive uncompressed size {total_size // (1024 * 1024)} MB exceeds limit of {_MAX_UNCOMPRESSED // (1024 * 1024)} MB")

• [nit] {archive_format!r} renders Python repr quotes in the warning string at src/apm_cli/commands/pack.py
  With archive_format = "tar.gz", !r produces 'tar.gz' with single quotes in user output. Change to bare {archive_format}.

• [nit] Code comment overstates the warning scope at src/apm_cli/commands/pack.py
  Comment: "Warn when --archive-format is set but --archive is not -- the flag is a no-op." But condition is archive_format != "zip", so explicit --archive-format zip without --archive is silently swallowed. Better: "Warn when a non-default --archive-format is given without --archive."

• [nit] ZIP error says "symlink" while tar.gz branch says "symlink/hardlink" -- minor inconsistency at src/apm_cli/bundle/unpacker.py
  Align phrasing across archive format branches. ZIP has no hardlink concept so "symlink" is correct; make error message style consistent with sibling message.

DevX UX Expert

• [recommended] Shipped skill prose for apm install <BUNDLE-PATH> still lists only .tar.gz/.tgz at packages/apm-guide/.apm/skills/apm-usage/commands.md
  Agents loading this skill file at runtime will tell users only .tar.gz archives are supported -- wrong guidance for the new .zip default on day one of merge.
  Suggested: Change "or to a .tar.gz/.tgz archive whose extracted root contains plugin.json" to "or to a .zip, .tar.gz, or .tgz archive whose extracted root contains plugin.json".

• [recommended] --archive-format without --archive should raise click.UsageError, not emit a warning at src/apm_cli/commands/pack.py
  Guard fires only for tar.gz (not explicit zip), producing inconsistent behavior. APM pattern is UsageError for inapplicable flags (--as without a bundle path raises UsageError). Package managers in the reference set (cargo, pip) error on inapplicable flags.
  Suggested: if not archive and ctx.get_parameter_source('archive_format') != click.core.ParameterSource.DEFAULT: raise click.UsageError('--archive-format has no effect without --archive; add --archive to produce an archive.')

• [nit] tar.gz as a period-containing click.Choice string may not complete cleanly in generated shell completions at src/apm_cli/commands/pack.py
  Validate apm pack --archive-format <TAB> completes correctly in bash, zsh, and fish before shipping. Consider aliasing to tgz with help clarifying it means .tar.gz output if completions break.

• [nit] --archive help text cross-references --archive-format, making a simple boolean flag paragraph busy at src/apm_cli/commands/pack.py
  Suggested: "Produce an archive instead of a directory. Default format: .zip. See --archive-format."

Supply Chain Security Expert

• [recommended] ZIP bomb guard absent from local_bundle._extract_zip_bundle() -- asymmetry with unpacker.py at src/apm_cli/bundle/local_bundle.py
apm install <user-supplied-zip> routes through _extract_zip_bundle(). No entry-count or size limit before zf.extractall(). A crafted ZIP with 50,000 entries can exhaust temp-dir disk space while leaving no actionable error message.
  Suggested: Port _MAX_ENTRIES and _MAX_UNCOMPRESSED guard from unpacker.py immediately before zf.extractall() in _extract_zip_bundle().
  Proof (missing at static): grep for bomb guard in local_bundle.py returns zero results; same check confirmed present in unpacker.py lines 85-97.

• [recommended] Bomb guard sums attacker-controlled ZipInfo.file_size, not measured output bytes at src/apm_cli/bundle/unpacker.py
  A crafted ZIP can declare file_size=0 for every entry while DEFLATE stream expands to gigabytes. sum(m.file_size) would be 0 -- passing the 512 MB guard -- while extractall() writes unbounded content. APM-produced ZIPs have accurate metadata, but apm unpack and apm install both accept arbitrary user-supplied archives.
  Suggested: Stream extraction counting bytes written, or enforce RLIMIT_FSIZE on the temp_dir before calling extractall().
  Proof (manual at static): Python docs: ZipInfo.file_size sourced from central directory record, not from decompression. Python's zipfile.ZipFile does not cross-check actual output size against this field.

• [nit] Entry-count and size bomb guard should precede member-iteration loop for early exit at src/apm_cli/bundle/unpacker.py
  A ZIP with 1,000,000 entries triggers 1,000,000 validate_path_segments() calls before the 10,000-entry limit fires. Hoist members = zf.infolist() and the bomb guard immediately inside the with zipfile.ZipFile(...) block before the for-member validation loop.

• [nit] No unit tests cover ZIP bomb guard thresholds at tests/unit/test_unpacker.py
TestUnpackZipSecurity tests path traversal and symlinks but not _MAX_ENTRIES/_MAX_UNCOMPRESSED. If either constant is removed, no test regresses.
  Proof (missing at unit): tests/unit/test_unpacker.py -- proves: bomb guard constants have regression coverage

• [nit] Windows reserved device names (COM1, NUL, CON) pass all path guards at src/apm_cli/bundle/unpacker.py
PureWindowsPath('COM1').drive == '' (falsy), so device names are not caught. Not a traversal bypass but a platform-specific DoS vector on Windows installations.
  Suggested: Add segment-level check: re.compile(r'^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])(\\..*)?$', re.IGNORECASE) applied to each path segment.

OSS Growth Hacker

• [recommended] apm unpack docs still document .tar.gz exclusively despite this PR adding .zip extraction support at docs/src/content/docs/reference/cli/unpack.md
  Description says "extracts a .tar.gz archive". The PR adds .zip support to unpack_bundle() but the docs don't reflect it. New producers who apm pack --archive and share a .zip will see docs that describe only .tar.gz.
  Suggested: Update Description and first example to show .zip as primary, .tar.gz as legacy.

• [recommended] install.md and commands.md still cite ./bundle.tar.gz as canonical packed-bundle example at docs/src/content/docs/reference/cli/install.md
install.md PACKAGE_REF says "packed bundles (./bundle.tar.gz)". commands.md local-bundle description still names only .tar.gz/.tgz. New users have the wrong extension for a bundle newly produced by apm pack --archive.
  Suggested: Update install.md to list .zip first; update commands.md paragraph.

• [nit] CHANGELOG Changed entry lacks a CI migration one-liner for pipeline maintainers at CHANGELOG.md
  CI pipelines that glob *.tar.gz artifacts will silently break. Append to the bullet: "CI migration: update artifact globs from build/*.tar.gz to build/*.zip, or add --archive-format tar.gz to your apm pack invocation to preserve the previous default."

• [nit] publish.md still documents .tar.gz as auto-pack format (pre-existing drift from feat(publish): switch publish command archive format from tar.gz to zip #1695) at docs/src/content/docs/reference/cli/publish.md
  CHANGELOG references format alignment with apm publish, but publish.md still describes .tar.gz output. Recommend a companion doc-fix PR before the next release note.

Auth Expert -- inactive

No auth-specific files changed. PR is a pure archive-format change with no impact on AuthResolver, token management, credential resolution, or remote-host auth semantics.

Doc Writer

• [recommended] producer/pack-a-bundle.md still describes .tar.gz as the archive output in three places at docs/src/content/docs/producer/pack-a-bundle.md
  Primary producer guide says "Add --archive to get a single .tar.gz", shows .tar.gz inline comment and a distribution example. New users following this guide will look for a file that no longer exists by default.

• [recommended] releasing-from-any-ci.md globs build/*.tar.gz across all four CI examples -- those globs match nothing once --archive produces .zip by default at docs/src/content/docs/producer/releasing-from-any-ci.md
  CI pipeline examples use build/*.tar.gz and build/*.tar.gz.sha256 for sha256 loop and release-create upload. Globs expand to nothing once the default is .zip. Release artifacts silently omitted.
  Suggested: Change all globs to build/*.zip; add --archive to the apm pack call; add a migration callout in the section header.

• [recommended] install.md description still references ./bundle.tar.gz as the packed bundle format at docs/src/content/docs/reference/cli/install.md
  PACKAGE_REF description says "packed bundles (./bundle.tar.gz)". Install examples show only .tar.gz for local bundle with --as.

• [recommended] unpack.md description and all examples reference only .tar.gz at docs/src/content/docs/reference/cli/unpack.md
  BUNDLE_PATH description says "accepts either a .tar.gz archive". All five code examples use bundle.tar.gz. The CHANGELOG states .zip is now supported but docs don't reflect it.

• [recommended] ci-cd.md updates file patterns without a migration callout for existing pipelines at docs/src/content/docs/integrations/ci-cd.md
  The "Pack & Distribute" section silently changes the glob without noting this is a change from previous behavior. Teams with existing pipelines hardcoding *.tar.gz patterns get no warning.
  Suggested: Add a :::note migration callout in the "Pack in CI" subsection.

• [nit] CHANGELOG places --archive-format under ### Changed instead of ### Added at CHANGELOG.md
  Keep a Changelog convention: new flags go in Added. The bullet even starts with "Add".

• [nit] CHANGELOG references apm publish format alignment but publish.md still documents .tar.gz at CHANGELOG.md
  Drop the apm publish cross-reference or open a companion task to update publish.md first.

• [nit] producer/index.md step 4 description still says .tar.gz at docs/src/content/docs/producer/index.md
  Suggested: Change to "apm pack produces a .zip bundle you can ship offline or to a marketplace".

• [nit] concepts/glossary.md bundle definition only mentions .tar.gz at docs/src/content/docs/concepts/glossary.md
  Suggested: "Either a directory or a .zip (default) or .tar.gz archive containing plugin.json at the root."

• [nit] enterprise/security.md local bundle trust model says .tar.gz only at docs/src/content/docs/enterprise/security.md
  Suggested: "apm install <bundle> accepts a directory, .zip, or .tar.gz produced by apm pack."

Test Coverage Expert

• [recommended] No hermetic integration test for apm install <bundle.zip> -- the new default archive format at tests/integration/test_install_local_bundle_e2e.py
test_install_local_bundle_e2e.py tests directory and tarball installs but has no zip test. The full path detect_local_bundle(.zip) -> _extract_zip_bundle() -> install_local_bundle() -> file deployment is untested at integration tier. A regression in the primary distribution path would be invisible until a user reports it.
  Proof (missing at integration-with-fixtures): tests/integration/test_install_local_bundle_e2e.py::TestInstallLocalBundleE2E::test_install_local_bundle_from_zip -- proves: apm install (bundle.zip) deploys plugin bundle files end-to-end [devx,portability-by-manifest]

• [recommended] ZIP bomb guard in unpacker.py has no regression trap at tests/unit/test_unpacker.py
_MAX_ENTRIES=10,000 and _MAX_UNCOMPRESSED=512 MB have no test. If either constant is changed or the guard removed, no test catches the regression.
  Proof (missing at unit): tests/unit/test_unpacker.py::TestUnpackZipSecurity::test_zip_bomb_entry_count_rejected -- proves: apm install (bloated.zip) is rejected before extraction begins [secure-by-default]

• [recommended] TestUnpackZipSecurity is unit tier; path-segment validation on a security surface warrants an integration-with-fixtures CLI-level test at tests/unit/test_unpacker.py
  Unit tests exercise unpack_bundle() directly with real zip files -- genuine protection. But no CLI-level test asserts apm install <traversal.zip> exits non-zero with a user-readable error message.
  Proof (passed at unit): tests/unit/test_unpacker.py::TestUnpackZipSecurity::test_zip_path_traversal_rejected -- proves: unpack_bundle() rejects ZIP path-traversal entries at function level [secure-by-default]

• [nit] --archive-format without --archive emits a warning but no test asserts the warning fires at tests/unit/commands/test_pack_cli_surface.py
  Proof (missing at unit): tests/unit/commands/test_pack_cli_surface.py::TestPackCmdFlags::test_archive_format_without_archive_emits_warning -- proves: --archive-format tar.gz without --archive shows the user a clear warning

Performance Expert

• [recommended] Missing ZIP bomb guard in _extract_zip_bundle() -- asymmetric with unpacker.py at src/apm_cli/bundle/local_bundle.py
_extract_zip_bundle() calls zf.extractall() with no entry-count or size limit. Disk-exhaustion from crafted ZIP via apm install ./crafted.zip on the new primary distribution path.
  Suggested: Port the five-line guard from unpacker.py:85-97 to _extract_zip_bundle().

• [nit] Two separate zf.infolist() calls in unpacker.py -- could be a single merged pass at src/apm_cli/bundle/unpacker.py
  CPython's infolist() returns self.filelist directly; two O(N) passes over the same list is sub-0.1ms for any realistic bundle. Aesthetic tightening only.

• [nit] sorted(bundle_dir.rglob('*')) eagerly materializes all paths before filtering at src/apm_cli/bundle/packer.py
  Negligible at bundle sizes (<200 files). Positive side-effect: deterministic archive entry order. No action required.

• [nit] No compresslevel on ZIP_DEFLATED -- default level 6 is appropriate but undocumented at src/apm_cli/bundle/packer.py
  Suggested: Add comment: # level 6 (zlib default) -- sufficient for agent bundle sizes < 2MB.

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

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• pypi.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "pypi.org"

See Network Configuration for more information.

Generated by PR Review Panel for issue #1720 · sonnet46 22.7M · ◷

[nadav-y]

All findings addressed in da6d293.

ZIP bomb guard scope (blocking)

The guard was present in unpacker.py (the apm unpack path) but missing entirely from local_bundle._extract_zip_bundle() (the apm install ./bundle.zip path). Fixed: the same guard now applies in both paths. I also hoisted _MAX_ZIP_ENTRIES and _MAX_ZIP_UNCOMPRESSED to module level in both files so the thresholds are visible and easy to adjust, moved the check before the per-member validation loop (previously a million-entry ZIP still triggered a million validate_path_segments calls before being rejected), and updated the error messages to show sizes in MB rather than raw bytes.

_looks_like_archive rename

Renamed to _looks_like_tarball -- the old name was accurate for the original code but became actively misleading once .zip became the primary format. All three call sites updated.

--archive-format without --archive

Changed from a logger.warning (which only fired when the value was non-default) to a click.UsageError that fires whenever the flag is explicitly set on the command line without --archive, using ctx.get_parameter_source() to distinguish an explicit value from the default. The error message includes an action hint.

releasing-from-any-ci.md CI globs

All build/.tar.gz and build/.tar.gz.sha256 patterns replaced with build/.zip / build/.zip.sha256 across the canonical sequence and all four CI provider sections (GitHub Actions, GitLab CI, Jenkins, Azure DevOps).

Remaining doc drift

Updated pack-a-bundle.md, install.md, unpack.md, producer/index.md, glossary.md, security.md, and the commands.md skill file. The apm unpack reference preserves the mention of legacy .tar.gz support since that command is specifically the deploy path for --format apm tarballs.

Tests

Added two bomb guard threshold tests to TestUnpackZipSecurity (using monkeypatch to lower the limits so the tests are fast and deterministic) and two --archive-format CLI surface tests to TestPackCmdFlags.