fix(install): fail loudly when a package deploys to no target (closes #2796)

[Daniel Meppiel (danielmeppiel)]

Context: Agent Plugins v1.0.0 packages can only register natively with the copilot target today. When every selected target was non-copilot, apm install --skill ... --target codex skipped the package, deployed nothing, printed No changes, and exited 0.

WHY: A successful no-op hid an unsatisfied install request. The install outcome now fails only when target exclusion leaves the run with zero deployed packages, while mixed installs that deploy at least one package still exit 0.

BREAKING CHANGE: apm install now exits non-zero when Agent Plugin target exclusion means no package was deployed. Migration: install a direct skill subpath such as apm install kunchenguid/lavish-axi/skills/lavish#main --target codex, or select --target copilot for native Agent Plugin registration.

Scenario Evidence:

Scenario	Principle	Test path	Run evidence
Agent Plugin selected only for non-Copilot targets fails loudly, preserves the requested ref in the skill-subpath hint, and commits no durable state.	DevX; multi-harness support	tests/integration/test_required_lifecycle_state_machine.py::test_required_agent_plugin_target_exclusion_noop_fails_without_mutating_state	APM_E2E_TESTS=1 uv run --extra dev pytest tests/integration/test_required_lifecycle_state_machine.py::test_required_agent_plugin_target_exclusion_noop_fails_without_mutating_state -q -> 1 passed in 2.50s
Mixed installs stay successful when a target-excluded Agent Plugin deploys nothing but another package deploys.	DevX; multi-harness support	tests/unit/install/test_agent_plugin_deployment_boundary.py::test_target_exclusion_does_not_hard_fail_when_the_rest_of_the_batch_installs	uv run --extra dev pytest tests/unit/install/test_agent_plugin_deployment_boundary.py::test_target_exclusion_hint_names_remote_skill_subpath_form tests/unit/install/test_agent_plugin_deployment_boundary.py::test_target_exclusion_fails_when_no_package_is_deployed tests/unit/install/test_agent_plugin_deployment_boundary.py::test_target_exclusion_does_not_hard_fail_when_the_rest_of_the_batch_installs -q -> 10 passed in 2.44s

How to test:

• uv run --extra dev pytest tests/unit/install/test_agent_plugin_deployment_boundary.py::test_target_exclusion_hint_names_remote_skill_subpath_form tests/unit/install/test_agent_plugin_deployment_boundary.py::test_target_exclusion_fails_when_no_package_is_deployed tests/unit/install/test_agent_plugin_deployment_boundary.py::test_target_exclusion_does_not_hard_fail_when_the_rest_of_the_batch_installs -q
• uv run --extra dev pytest tests/unit/copilot_plugins/test_install_lifecycle.py -q
• APM_E2E_TESTS=1 uv run --extra dev pytest tests/integration/test_required_lifecycle_state_machine.py::test_required_agent_plugin_target_exclusion_noop_fails_without_mutating_state -q
• uv run --extra dev ruff check src/ tests/
• uv run --extra dev ruff format --check src/ tests/

Architecture: owner-extension. The deterministic owner-touch gate reports src/apm_cli/install/outcome.py under the canonical decision Install success / failure outcome; this PR extends that existing owner with the target-excluded no-deploy failure case while keeping per-package target exclusion detection in the Agent Plugin deployment boundary and diagnostics collector.

apm-spec-waiver: APM CLI-specific Agent Plugin target-exclusion UX; OpenAPM manifest and registry semantics are unchanged.

closes #2796

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes correctly implement the specified failure condition with targeted diagnostics and comprehensive unit coverage for the new behavior (pure no-op, mixed batch, and dry-run).

Review tier: Lite
Findings: None

What changed in this PR

This PR updates the apm install outcome logic so that an Agent Plugins v1.0.0 package that is excluded by target selection (i.e., no selected target can receive it) is no longer treated as a successful no-op. Instead, the install fails (non-zero exit) only when that exclusion results in zero packages being deployed, matching the intent described in issue #2796.

Changes:

• Add a dedicated diagnostic subtype/count for “Agent Plugin target excluded” warnings and plumb it into install outcome classification.
• Update the install outcome owner to fail when installed_count == 0 and at least one Agent Plugin was target-excluded.
• Expand/adjust unit tests to cover: pure-no-op failure, mixed-batch non-fatal behavior, dry-run preview remaining non-fatal, and the actionable “install via /skills/” hint.

File	Description
tests/​unit/​install/​test_agent_plugin_deployment_boundary.py	Updates and adds regression tests for the new “fail loudly on pure no-op” behavior, including hint wording and dry-run parity.
src/​apm_cli/​utils/​diagnostics.py	Extends DiagnosticCollector.warn() with a severity subtype and adds a first-class Agent Plugin target-exclusion warning + counter.
src/​apm_cli/​install/​template.py	Builds an actionable skip message (including /skills/<skill> workaround) and records target exclusion using the new diagnostic subtype.
src/​apm_cli/​install/​outcome.py	Adds agent_plugin_target_excluded_count() helper and fails the install when exclusion caused a zero-deploy run.
CHANGELOG.md	Documents the breaking change and migration guidance under [Unreleased].

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: ship_now

APM #2806 makes total Agent Plugin target exclusion fail honestly, preserves successful mixed installs, and supplies inventory-grounded, ref-preserving direct-skill recovery.

cc Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

Reviewed HEAD 9fc5127 against origin/main f8df1b7. All eight active reviewers report no findings; auth remains inactive after complete fallback scope inspection. The canonical install-outcome owner handles zero-deployment target exclusion without introducing a competing decision path. The reservation is preserved: successful mixed batches remain successful, direct-skill recovery stays actionable, and non-dry-run total target exclusion must not report success.

All four previous follow-ups are closed in source. The mixed lifecycle scenario now requires ordinary SKILL.md files and compares deployed bytes with fixture bytes. Recovery matches requested skill names or directories against canonical inventory, uses directory_name, and explicitly identifies an available alternative when necessary. shlex.quote protects the complete dependency path and retained ref, with POSIX-shell labeling. Documentation qualifies the failure as non-dry-run and links the canonical dry-run exception. The breaking exit-code change and migration are documented, supporting P4 and P6 without implying additional native target support.

Evidence remains bounded: the parent reports 64 targeted passes in 12.99s at the identical precommit tree, with exact-HEAD execution still parent-owned. Recorded mutation failures are intentional detections with guards restored, not current-code failures. req-tg-011 now binds to the renamed test; this is a source repair, not a conformance pass or waiver, and historical PR-body waiver text is not current evidence. Neither reviewers nor this synthesis executed validation. Fresh functional, conformance, lint and CI evidence remains parent-owned; current CI green is not established.

Aligned with: Pragmatic as npm: P4/P6: truthful install outcomes, useful mixed-batch success, and explicit preview limits preserve predictable package-manager behavior. Multi-harness / multi-host: P2/P3/P5: inventory-grounded direct-skill recovery retains the selected compatible target and requested ref without forcing native Copilot registration. Secure by default: Complete-argument POSIX quoting protects copied recovery commands from shell reinterpretation of dependency paths and refs.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	0	Prior architecture finding resolved. Canonical inventory drives recovery; the outcome owner preserves mixed success and rejects zero-deployment exclusion.
CLI Logging Expert	0	0	0	No logging concerns: total exclusion reports failure; mixed success and inventory-grounded, ref-preserving recovery remain intact.
DevX UX Expert	0	0	0	Prior recovery finding resolved: inventory-backed hints preserve refs; total exclusion fails, mixed success remains, and dry-run behavior is documented.
Supply Chain Security	0	0	0	Complete-argument quoting resolves prior feedback; total exclusion fails before cleanup, while mixed success and scoped rollback remain intact.
OSS Growth Hacker	0	0	0	Recovery is inventory-grounded and copyable; documentation explains failed total exclusions, successful mixed installs, and dry-run limits. No growth concerns remain.
Doc Writer	0	0	0	Prior documentation follow-up resolved: non-dry-run failure, successful mixed batches, direct-skill recovery, and preview limits are documented.
Test Coverage	0	0	0	Previous artifact gap is resolved; no-op, mixed-success, inventory and quoting regressions have targeted assertions. Exact-head execution remains parent-owned.
Performance Expert	0	0	0	No scoped performance regression found: inventory lookup uses set membership, and exclusion counting remains linear at outcome boundaries.

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

Architecture

classDiagram
    direction LR
    class DependencySource {
      <<Strategy>>
      +acquire() Materialization
    }
    class LocalDependencySource
    class CachedDependencySource
    class FreshDependencySource
    DependencySource <|-- LocalDependencySource
    DependencySource <|-- CachedDependencySource
    DependencySource <|-- FreshDependencySource
    class Materialization {
      <<Dataclass>>
      +package_info
      +deltas
    }
    class InstallContext {
      +installed_count
      +skill_subset
      +diagnostics
    }
    class InstallTemplate {
      <<Module>>
      +run_integration_template()
      +_record_agent_plugin_target_skip()
      +_agent_plugin_skill_name_for_hint()
    }
    note for InstallTemplate "Template Method: shared post-acquisition integration in install/template.py"
    class AgentPlugin {
      <<ImmutableIR>>
      +components
    }
    class AgentPluginComponents {
      +skills
    }
    class AgentPluginSkill {
      +directory_name
      +name
    }
    class DiagnosticCollector {
      +agent_plugin_target_excluded()
      +agent_plugin_target_excluded_count
      +render_summary()
    }
    class Diagnostic {
      <<Dataclass>>
      +category
      +severity
      +message
    }
    note for DiagnosticCollector "Collect-then-render: typed exclusion fact, aggregate classification, then presentation"
    class InstallOutcome {
      <<Module>>
      +result_from_install_context()
      +finalize_install_result()
      +apply_install_command_outcome()
    }
    class InstallResult {
      <<Dataclass>>
      +installed_count
      +disposition
      +exit_code
    }
    class InstallTransaction {
      +complete()
      +commit()
      +rollback()
    }
    DependencySource --> InstallContext
    DependencySource ..> Materialization : returns
    Materialization ..> AgentPlugin : package_info.package.agent_plugin
    AgentPlugin *-- AgentPluginComponents
    AgentPluginComponents *-- AgentPluginSkill
    InstallTemplate ..> Materialization
    InstallTemplate ..> AgentPluginSkill : reads canonical inventory
    InstallTemplate ..> DiagnosticCollector : records exclusion
    InstallContext --> DiagnosticCollector
    DiagnosticCollector *-- Diagnostic
    InstallOutcome ..> InstallContext
    InstallOutcome ..> DiagnosticCollector : reads facts
    InstallOutcome ..> InstallResult : owns classification
    InstallTransaction ..> InstallResult : consumes disposition
    class InstallTemplate:::touched
    class InstallOutcome:::touched
    class DiagnosticCollector:::touched
    class Diagnostic:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["apm install: install()<br/>src/apm_cli/commands/install.py"] --> B["run_install_pipeline()<br/>src/apm_cli/install/pipeline.py"]
    B --> D{"dry-run?"}
    D -->|yes| P["[I/O] preflight_agent_plugin_dry_run()<br/>install/template.py<br/>Target exclusion is swallowed; structural errors still propagate"]
    P --> PV["DRY_RUN preview: exit 0 for target exclusion<br/>No recovery diagnostic or certified deployment"]
    D -->|no| AC["[NET] [FS] download.run() and source.acquire()<br/>install/phases/download.py; install/sources.py"]
    AC --> PF["preflight_agent_plugin_materializations()<br/>install/template.py<br/>Target exclusion does not abort the batch"]
    PF --> T["run_integration_template() -> _integrate_materialization()<br/>install/template.py: each dependency"]
    T --> E{"enforce_agent_plugin_deployment_boundary()<br/>agent_plugins/errors.py"}
    E -->|target excluded| S["_record_agent_plugin_target_skip()<br/>installed delta = 0; deployed_files = []"]
    S --> H["_agent_plugin_skill_name_for_hint()<br/>install/template.py<br/>Read AgentPlugin.components.skills"]
    H --> HS{"inventoried skill available?"}
    HS -->|yes| HC["_agent_plugin_target_skip_message()<br/>Match requested name or directory; otherwise available alternative<br/>Quote complete skill path plus retained ref"]
    HS -->|no| HF["_agent_plugin_target_skip_message()<br/>Copilot or target-compatible-package fallback"]
    HC --> W["DiagnosticCollector.agent_plugin_target_excluded()<br/>utils/diagnostics.py"]
    HF --> W
    E -->|structural error| ER["_record_agent_plugin_boundary_failure()<br/>Record error; installed delta = 0"]
    E -->|admitted or ordinary| I["[FS] integrate_package_primitives()<br/>install/services.py<br/>Record deployment deltas"]
    W --> AG["integrate.run()<br/>install/phases/integrate.py<br/>Aggregate installed_count across batch"]
    ER --> AG
    I --> AG
    AG --> O["result_from_install_context() -> finalize_install_result()<br/>install/outcome.py"]
    O --> Q{"Existing failure condition OR<br/>installed_count == 0 and exclusion_count > 0?"}
    Q -->|yes| F["FAILED: return before cleanup/finalize"]
    Q -->|no| C["[FS] [LOCK] Remaining pipeline phases<br/>install/phases/finalize.py: finalize.run()"]
    F --> RB["[FS] [LOCK] InstallTransaction.complete()<br/>install/transaction.py: rollback"]
    C --> CM["[FS] [LOCK] InstallTransaction.complete()<br/>Commit successful result"]
    RB --> R["[I/O] render_post_install_summary()<br/>install/summary.py<br/>Consume canonical disposition"]
    CM --> R
    R --> X{"apply_install_command_outcome()<br/>install/outcome.py"}
    X -->|FAILED| X1["exit 1; success summary disallowed"]
    X -->|SUCCESS or PARTIAL_SUCCESS| X0["exit 0; successful mixed batch preserved"]

Loading

Recommendation

Recommend shipping the reviewed code with no remaining code follow-ups. This advisory stance is not policy approval or proof of current CI success. The parent should establish exact-head validation evidence for the maintainer's shipping decision; unexpected failures warrant reassessment, not speculative findings now.

---

Full per-persona findings

Python Architect

No findings.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security

No findings.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

Complete exact-head diff fallback check confirms src/apm_cli/install/outcome.py, src/apm_cli/install/template.py, src/apm_cli/utils/diagnostics.py and accompanying tests/docs change outcomes and recovery hints, not credentials, host classification or authentication semantics.

Doc Writer

No findings.

Test Coverage

No findings.

Performance Expert

No findings.

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

[Daniel Meppiel (danielmeppiel)]

Shepherd completion: code complete, required review outstanding

Head 9fc512786be6e0ca6fb079eb13964339af7a8f22 incorporates current main
f8df1b751efc30b32dc01b125616b51f777b4c81. The final advisory panel recommends
ship_now, with no remaining code follow-ups. All current CI checks pass.
GitHub reports MERGEABLE / BLOCKED because required human review remains.
Sergio Sisternes (@sergio-sisternes-epam): please review the latest push as the nonauthor CODEOWNER.
No merge or merge-queue entry was performed.

Reservations carried from strategic alignment

• Preserve successful mixed batches and actionable direct-skill recovery:
  mixed installs remain successful, and recovery uses an inventoried skill
  directory, retained ref and quoted POSIX-shell argument.
• No-target must not report success: a real target-excluded zero-deployment
  install fails through the canonical outcome owner and rolls back state.

Folded in this run

• Repaired the renamed req-tg-011 conformance binding without a waiver in
  2ef629e7a.
• Folded all four panel follow-ups in 9fc512786: mixed-install deployed-byte
  assertions, inventory-grounded hints, argument quoting, and the documented
  dry-run exception.
• Removed released changelog entries resurrected during main incorporation.
  Existing contributor commits are preserved; no fix(alias): reject path-traversing dependency aliases — fixes #2900 #2901 alias APIs changed.

Copilot signals reviewed

Two inline-comment reads returned no findings. The existing Copilot review is
COMMENTED, not a substitute for the required human approval.

Regression and lifecycle evidence

• Removing the zero-deployment outcome predicate reproduced exit 0 and
  No changes in the real candidate CLI; the regression and repaired
  conformance binding failed. Guard restored.
• Independent inventory mutation: 4 failed / 2 passed. Independent quoting
  mutation: all 3 cases failed. Guards restored.
• Exact-head local source-CLI lifecycle: 1 passed in 3.84s, explicitly
  pinned to this worktree's .venv/bin/apm, with imported source provenance
  checked before execution, fresh HOME/APM_HOME and an unchanged 60s command
  budget. It proves failed-no-op snapshot preservation and successful mixed
  deployment with skill bytes equal to source.
• An earlier local batch recorded 123 passes, one existing unrelated
  req-mf-016 skip, and a lifecycle timeout in 557.76s. The unchanged pinned
  quiet replay passed; resource contention is not asserted as the proven sole
  cause. A separate direct-Python invocation selected installed Homebrew
  0.30.0 and is explicitly excluded from candidate evidence.

Canonical owner and lint

Deterministic detection finds one touched owner: Install success / failure
outcome, src/apm_cli/install/outcome.py. This extends its existing predicate;
there is no new owner or routing centralization. The registered
install-deployment-outcome boundary remains intact. Completion schema and
strict exact-head owner/functional-evidence semantics passed; the persisted
shipping status remains blocked for human approval.

The complete current CI lint contract passed locally before push: ruff pair,
YAML safety, 2100-line limit, portable relative paths, pylint R0801,
auth signals and architecture boundaries.

CI and mergeability

Current CI includes
green Linux shards, lifecycle smoke (184 passed / 1 skipped), Windows
compatibility and binary smoke. Spec conformance
and CodeQL pass.
The required gate
passes. Docs deployment is intentionally skipped on the PR; no applicable
runtime proof was replaced by that skip.

PR	Head	Panel stance	Iterations	Folds	Deferred	Copilot rounds	CI	Mergeable	Merge state
#2806	9fc512786	ship_now	2	4 panel follow-ups	0	2	green	MERGEABLE	BLOCKED: required human review

Two outer iterations; zero CI-recovery iterations. Source, schemas, ownership,
mutation and lifecycle evidence are retained for the coordinating VP.