Skip to content

fix: keep plugin hooks live during reinstall (closes #2717) - #2723

Merged
Daniel Meppiel (danielmeppiel) merged 6 commits into
mainfrom
fix/2717-atomic-plugin-reinstall
Aug 30, 2026
Merged

fix: keep plugin hooks live during reinstall (closes #2717)#2723
Daniel Meppiel (danielmeppiel) merged 6 commits into
mainfrom
fix/2717-atomic-plugin-reinstall

Conversation

@danielmeppiel

@danielmeppiel Daniel Meppiel (danielmeppiel) commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Stages replacement package trees before publishing them so a refresh or reinstall no longer unlinks the currently registered hook while network and package validation work is in flight. Failed replacement callbacks now fail closed without reusing stale content, and ResolutionStagingSession remains the canonical activation owner with behavioral and static mutation guards. Closes #2717.

The follow-up pass also preserves direct git-semver refreshes, validates candidates before activation, reserves each physical destination against concurrent writers, restores the prior package across activation errors and KeyboardInterrupt, and reports that the existing installation remains active after a failed refresh.

Scenario Evidence

User-visible promise Principle Exact test Reviewed head Evidence
The current hook stays live while replacement content downloads DevX tests/unit/install/test_resolve_resolution_paths.py::test_refresh_keeps_registered_hook_live_while_replacement_downloads a5ef135efd837027eb0caece262c7ca6ccb8b977 1 passed in 0.44s
Direct git-semver updates still re-resolve and publish the new tag DevX tests/integration/test_git_semver_install_e2e.py::TestUpdateReResolvesGitSemver::test_update_flag_re_resolves_when_install_path_exists_and_new_tag_published a5ef135efd837027eb0caece262c7ca6ccb8b977 1 passed in 7.90s
Activation failure or interruption restores the prior hook Secure by default tests/unit/install/test_resolution_staging_relocate.py::test_publish_replacement_restores_old_package_when_activation_fails a5ef135efd837027eb0caece262c7ca6ccb8b977 2 passed in 0.27s
Malformed update candidates fail with the prior hook and lockfile unchanged Secure by default tests/integration/test_git_semver_install_e2e.py::TestUpdateReResolvesGitSemver::test_invalid_update_preserves_live_hook_and_lockfile a5ef135efd837027eb0caece262c7ca6ccb8b977 1 passed in 4.20s
Existing local package cache paths are not activated as staged candidates DevX tests/unit/deps/test_apm_resolver_edge_cases.py::TestTryLoadDependencyPackageForceRecheck::test_cached_local_package_is_not_activated_as_candidate a5ef135efd837027eb0caece262c7ca6ccb8b977 1 passed in 0.89s
Nested package destinations use isolated journals and restore exact prior bytes Secure by default tests/unit/install/test_resolution_staging_relocate.py::test_nested_replacements_rollback_without_overlapping_staging_paths a5ef135efd837027eb0caece262c7ca6ccb8b977 2 passed
Activation OSError and KeyboardInterrupt preserve hook and lockfile state Secure by default tests/integration/test_git_semver_install_e2e.py::TestUpdateReResolvesGitSemver::test_activation_error_preserves_live_hook_and_lockfile a5ef135efd837027eb0caece262c7ca6ccb8b977 2 passed in 8.00s
Replacement activation has one canonical owner Governed by policy tests/integration/test_architecture_authorities.py::test_resolution_replacement_activation_has_one_owner a5ef135efd837027eb0caece262c7ca6ccb8b977 1 passed in 0.23s

Mutation checks removed the fail-closed callback return, validated publish call, activation rollback, destination reservation, and static owner selector in turn; each corresponding regression test failed before the guard was restored.

Validation: 135 targeted tests passed, apm audit --ci passed all 10 checks, and the full canonical lint mirror passed at the reviewed head.

apm-spec-waiver: Internal transaction repair; no OpenAPM manifest or registry contract changes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

Review tier: Lite
Findings: None

What changed in this PR

This PR fixes a reinstall/refresh failure mode where plugin hook scripts could be unlinked while their host settings entries remain registered, by staging replacement package trees and only publishing them after the replacement is fully downloaded and validated. It also codifies “resolution replacement activation” as a single-owner architecture boundary (ResolutionStagingSession) with both behavioral regression coverage and a static lint guard.

Changes:

  • Add ResolutionStagingSession.prepare_replacement() / publish_replacement() to stage replacement downloads without disturbing the live installed tree, and journal prior contents for rollback.
  • Update the resolve-phase download callback to download into a staged replacement path and publish atomically only after the download completes.
  • Tighten resolver behavior to fail closed on failed refresh/replacement attempts (do not silently reuse existing materializations), with new unit + integration architecture guard tests and a doc update.
File Description
src/​apm_cli/​install/​resolution_staging.py Adds explicit staging/publish APIs for replacement materializations while preserving rollback semantics.
src/​apm_cli/​install/​phases/​resolve.py Routes dependency downloads through the staging replacement flow and publishes only after successful materialization.
src/​apm_cli/​deps/​apm_resolver.py Ensures failed refresh/replacement callbacks don’t fall back to stale on-disk content (fail closed).
scripts/​lint-architecture-boundaries.sh Adds an architecture boundary lint rule enforcing a single “resolution replacement activation” owner and expected call-site shapes.
tests/​unit/​install/​test_resolution_staging_relocate.py Adds a unit regression covering “hook stays live until publish” semantics via staging session publish/rollback.
tests/​unit/​install/​test_resolve_resolution_paths.py Adds a resolve-phase regression ensuring refresh downloads stage replacements without unlinking the live hook during download.
tests/​unit/​deps/​test_apm_resolver_edge_cases.py Adds a unit regression ensuring failed refresh does not silently validate/reuse existing materialization.
tests/​integration/​test_architecture_authorities.py Adds an integration “mutation trap” asserting the new architecture boundary is enforced by the lint script.
docs/​src/​content/​docs/​enterprise/​security.md Documents the replacement staging behavior as part of integrity/installation hardening.
.github/​instructions/​architecture.instructions.md Registers “Resolution replacement activation” in the canonical owner table.
.apm/​instructions/​architecture.instructions.md Mirrors the canonical owner table update for agent instructions.

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

@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: needs_rework

PR #2723 targets uninterrupted plugin reinstalls, but transaction ordering still breaks semver refresh and can activate or reuse unvalidated content.

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

The panel supports making ResolutionStagingSession the canonical activation owner, but ownership must cover the complete prepare, validate, publish, callback, and rollback sequence. The direct-semver path prepares the live destination before replacement staging, causing prepare_replacement to reject it. Independently, publication and hook activation can precede package validation and integrity gates.

Callback exceptions must fail closed without reusing stale installed content, and the rename journal must survive interruption without losing the previous package. Existing tests do not exercise the reported semver conflict, interrupted refresh, or activation-failure boundaries.

Dissent. General cross-process collision hardening and architecture-test runtime are separate follow-ups unless reproduced here; semver refresh, gate ordering, callback containment, and recovery of this transaction's interruption window belong in scope.

Aligned with: Secure by default: integrity and audit gates must complete before replacement activation. Governed by policy: policy cannot govern execution if hooks activate first. Pragmatic as npm: semver reinstall must preserve the working installation and recover predictably.

Growth signal. Once corrected, uninterrupted fail-safe plugin refresh is a strong package-manager reliability story and warrants an Unreleased note.

Panel summary

Persona B R N Takeaway
Python Architect 3 1 0 Semver refresh, pre-validation activation, and interrupt windows violate the live-hook and rollback contracts.
CLI Logging Expert 0 1 0 Report when a failed reinstall preserves the active package and how to retry.
DevX UX Expert 2 1 0 Staging is directionally sound, but semver refresh and callback exceptions still violate fail-closed behavior.
Supply Chain Security Expert 4 1 0 Activation can expose unapproved content and stale reuse before validation.
OSS Growth Hacker 0 1 0 Expose the fail-closed upgrade value to plugin users.
Doc Writer 1 1 0 Correct the security overclaim and document hook continuity on the lifecycle page.
Test Coverage Expert 0 3 0 Add in-flight interruption and publish-failure regression traps.
Performance Expert 1 1 0 Semver purge remains destructive; the architecture guard is expensive.

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) Remove the prepare_path / prepare_replacement conflict in direct semver refresh and add a regression test.
  2. [Supply Chain Security Expert] (blocking-severity) Keep replacement content and hooks inactive until resolver validation, integrity checks, and audit gates succeed.
  3. [DevX UX Expert] (blocking-severity) Make callback exceptions fail closed without stale fallback and cover activation failure injection.
  4. [Python Architect] (blocking-severity) Durably record rename transitions and recover interruption while preserving the prior package.
  5. [Doc Writer] (blocking-severity) Correct the security validation-order claim, document lifecycle continuity, and add an Unreleased entry.

Architecture

classDiagram
    class InstallTransaction {
      +resolution ResolutionStagingSession
      +commit(result)
      +rollback()
    }
    class ResolutionStagingSession {
      +prepare_replacement(path)
      +publish_replacement(path, replacement)
      +commit()
      +rollback()
    }
    class ResolvePhase {
      +run(ctx)
      +download_callback(...)
    }
    class APMDependencyResolver {
      +resolve_dependencies(anchor)
      +validate package
    }
    InstallTransaction *-- ResolutionStagingSession : owns
    ResolvePhase ..> ResolutionStagingSession : stages and publishes
    ResolvePhase ..> APMDependencyResolver : injects callback
    APMDependencyResolver ..> ResolvePhase : receives live path too early
Loading
flowchart TD
    A["Install refresh"] --> B["Keep current package live"]
    B --> C["Download replacement to staging"]
    C --> D["Validate package, integrity, and policy"]
    D -->|success| E["Publish through ResolutionStagingSession"]
    D -->|failure| F["Discard replacement; current package stays live"]
    E --> G["Commit transaction"]
Loading

Recommendation

Revise the transaction before shipping: restore direct-semver refresh, enforce validate-before-activate ordering, close callback failure semantics, and make interruption recovery safe. Update documentation after those guarantees have focused tests.


Full per-persona findings

Python Architect

  • [blocking] Direct semver refresh removes the live package and then rejects replacement staging at src/apm_cli/install/phases/resolve.py:495. Remove the eager semver purge and route re-resolution through the replacement owner.
  • [blocking] Replacement content becomes live before package validation at src/apm_cli/install/phases/resolve.py:696. Validate the staged candidate before publication.
  • [blocking] Interruption during publication can discard the previous package at src/apm_cli/install/resolution_staging.py:71. Journal intent before mutation and handle each intermediate state.
  • [recommended] Callback exceptions can fall through to stale installed content at src/apm_cli/deps/apm_resolver.py:1106. Return failure after logging and add a regression trap.

CLI Logging Expert

  • [recommended] Explain in install diagnostics that the existing installation remains active after replacement failure and how to retry.

DevX UX Expert

  • [blocking] Direct semver updates fail before downloading replacement content because eager backup conflicts with replacement staging.
  • [blocking] Preparation exceptions can silently reuse stale installed content.
  • [recommended] Reconcile abandoned replacement journals on the next install.

Supply Chain Security Expert

  • [blocking] Replacement hooks activate before package and security gates complete.
  • [blocking] Git-semver updates fail against existing installs; the existing E2E semver update test failed on this head.
  • [blocking] Callback exceptions can bypass containment and reuse stale content.
  • [blocking] Distinct dependency identities can collide on one physical destination during parallel resolution; reject collisions or reserve destinations.
  • [recommended] Make publication recoverable after process interruption.

OSS Growth Hacker

  • [recommended] Add a concise Unreleased entry after the behavior is corrected.

Auth Expert -- inactive

The changed files do not alter authentication, token, credential, or host-resolution behavior.

Doc Writer

  • [blocking] docs/src/content/docs/enterprise/security.md:259 overstates validation-before-publication. Describe only guarantees actually enforced.
  • [recommended] Document refresh continuity in docs/src/content/docs/consumer/copilot-agent-plugins.md.

Test Coverage Expert

  • [recommended] Add fixture-backed coverage that invokes the old hook while refresh is paused, interrupts, retries, and then observes the new hook.
  • [recommended] Inject failure in staged-to-live activation and assert restoration plus cleanup.
  • [recommended] Add Scenario Evidence rows to the PR description.

Performance Expert

  • [blocking] Direct semver purge still unlinks live hooks before staged replacement.
  • [recommended] Narrow the architecture mutation guard to avoid another full repository copy and monolithic lint run.

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

Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Aug 30, 2026
Keep semver refreshes staged through package validation, fail closed on callback errors, restore interrupted publication, and tighten the canonical activation guard. Addresses the review-panel follow-ups for PR #2723.

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

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: needs_rework

PR #2723 needs a final fail-closed correction before its transactional install guarantees match APM's public security promise.

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

The panel converges on two necessary correctness fixes, not a broad resolver rewrite. Malformed candidates must terminate resolution before publication, metadata recording, or lockfile commit, preserving the prior installation and hook. Existing local-package cache paths must remain distinguishable from staged candidates when transitive dependencies are present.

These are within scope because they protect this PR's transaction contract. Result-object typing and wider resolver redesign are separate follow-ups. Green CI is encouraging but does not yet exercise the reported live-hook integration boundary.

Aligned with: Portable by manifest: lock state changes only after selected bytes validate and activate. Secure by default: malformed candidates fail before agent-visible activation. Pragmatic as npm: failed updates return nonzero with the prior install unchanged.

Panel summary

Persona B R N Takeaway
Python Architect 1 0 0 Cached local dependencies are misclassified as staged candidates.
CLI Logging Expert 0 1 0 Validation and publication failures need the same retry guidance.
DevX UX Expert 1 0 0 Malformed refresh candidates can appear successful against stale bytes.
Supply Chain Security Expert 1 0 0 Candidate metadata can be recorded before validation and activation.
OSS Growth Hacker 0 0 0 Changelog and lifecycle docs communicate the reliability value.
Doc Writer 1 2 0 Runtime ordering and two docs details remain inconsistent.
Test Coverage Expert 0 1 0 Live-hook continuity needs fixture-backed coverage.
Performance Expert 0 0 0 No performance concern remains.

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

Top 5 follow-ups

  1. [Supply Chain Security Expert] (blocking-severity) Fail malformed-candidate resolution and defer publication and metadata recording until acceptance succeeds.
  2. [Python Architect] (blocking-severity) Keep cached local paths distinct from staged candidates and add a transitive-dependency regression.
  3. [Doc Writer] (blocking-severity) Preserve the README security promise by keeping replacement content unexposed until applicable gates finish.
  4. [Test Coverage Expert] Add fixture-backed integration coverage for live-hook rollback.
  5. [CLI Logging Expert] Show preserved-install retry guidance for validation and publication failures.

Architecture

flowchart TD
    A["Resolve dependency"] --> B{"Live cache or staged candidate?"}
    B -->|live cache| C["Validate and consume live package"]
    B -->|candidate| D["Validate staged package"]
    D -->|invalid| E["Fail resolution; preserve live package"]
    D -->|valid| F["Run remaining acceptance gates"]
    F -->|success| G["Publish through ResolutionStagingSession"]
Loading

Recommendation

Fold the fail-closed candidate and local-cache fixes with focused regressions before shipping. Defer result-object typing and broader resolver restructuring.


Full per-persona findings

Python Architect

  • [blocking] Existing local package paths returned by the callback are treated as staged candidates and rejected by activation. Distinguish cache hits from prepared replacements and cover a local package with transitives.

CLI Logging Expert

  • [recommended] Apply preserved-install retry guidance to candidate validation and publication failures.

DevX UX Expert

  • [blocking] Candidate validation errors can be swallowed while stale live bytes remain. Return nonzero and keep lock state unchanged.

Supply Chain Security Expert

  • [blocking] callback_downloaded metadata is recorded before candidate validation and activation. Record only after acceptance.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

No authentication, token, credential, or host-resolution surface is touched.

Doc Writer

  • [blocking] Agent-visible publication before later gates conflicts with the README security promise; preserve the promise in implementation.
  • [recommended] Split --force from apm update in the lifecycle table.
  • [recommended] Correct the security page's policy timing statement.

Test Coverage Expert

  • [recommended] Add fixture-backed live-hook continuity and failed-refresh rollback coverage.

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) added a commit that referenced this pull request Aug 30, 2026
Fail malformed refreshes before metadata commit, distinguish live local cache paths from staged candidates, and add integration evidence for hook continuity and rollback. Addresses the second review-panel pass on PR #2723.

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

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: needs_rework

PR #2723 improves replacement installs, but overlapping destinations can make rollback delete parent package files and must be fixed before shipping.

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

The supply-chain finding outweighs green CI and otherwise clean architecture, performance, documentation, and growth reviews. A manual reproducer shows that ancestor and descendant replacement destinations can overlap transaction trees, allowing rollback to delete valid parent-package files.

Fold the transaction fix and exact rollback tests into this PR. Then address the bounded CLI polish: canonical reservation keys, accurate first-install guidance, cache-hit TUI completion, and one authoritative diagnostic.

Aligned with: Secure by default: replacement rollback preserves every pre-install file. Pragmatic as npm: cache hits finish visibly and failures give accurate, non-duplicate guidance.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 1 No architecture change requested.
CLI Logging Expert 0 2 0 Remove duplicate errors and make guidance state-aware.
DevX UX Expert 0 2 0 Complete cache-hit progress and correct first-install guidance.
Supply Chain Security Expert 1 1 0 Overlapping destinations can corrupt rollback.
OSS Growth Hacker 0 0 0 Reliability story is clear.
Doc Writer 0 0 0 No PR-caused drift remains.
Test Coverage Expert 0 1 0 Add install-level activation failure coverage.
Performance Expert 0 0 0 No performance regression.

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

Top 5 follow-ups

  1. [Supply Chain Security Expert] (blocking-severity) Reject ancestor/descendant destination overlap and preserve parent-package files exactly on rollback.
  2. [Supply Chain Security Expert] Canonicalize replacement reservation keys.
  3. [Test Coverage Expert] Add install-level activation OSError and KeyboardInterrupt regressions.
  4. [DevX UX Expert] Complete cache-hit TUI tasks and make first-install guidance accurate.
  5. [CLI Logging Expert] Remove duplicate resolver warning emission.

Recommendation

Fix destination overlap and prove exact rollback, then fold the bounded CLI follow-ups before the terminal panel pass.


Full per-persona findings

Python Architect

No architecture changes requested; staged validation narrows exposure and preserves the canonical transaction boundary.

CLI Logging Expert

  • [recommended] Render fatal downloaded-package errors only through the command logger.
  • [recommended] Do not claim an existing installation survives a fresh-install failure.

DevX UX Expert

  • [recommended] Complete TUI tasks when the callback returns a cache hit.
  • [recommended] Make first-install failure guidance accurate.

Supply Chain Security Expert

  • [blocking] Ancestor and descendant reservations overlap candidate and backup trees, so rollback can restore only a fragment of the parent package.
  • [recommended] Use canonical replacement paths as reservation keys.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

No authentication-relevant surface changed.

Doc Writer

No findings.

Test Coverage Expert

  • [recommended] Add install-level activation failure coverage for exact hook and lock restoration.

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) added a commit that referenced this pull request Aug 30, 2026
Use opaque transaction slots for overlapping package paths, keep failure guidance state-aware, and prove activation rollback through the install pipeline. Addresses the third review-panel pass on PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Aug 30, 2026
Exercise the registered PreToolUse command during failed replacement activation and document the transactional refresh contract on every canonical CLI surface. Addresses the final review-panel follow-ups for PR #2723.

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

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

PR #2723 makes replacement installs transactional so registered hooks survive refresh failures.

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

The final panel found the transaction architecture sound at a5ef135efd837027eb0caece262c7ca6ccb8b977. Candidate downloads remain isolated through package validation, invalid candidates fail closed, nested destinations use non-overlapping journals, and activation errors restore the prior package. The final evidence, registered-hook E2E, and canonical documentation follow-ups were folded after arbitration. No transaction correctness item remains open.

Aligned with: Secure by default: failed activation restores the prior package and lockfile. Governed by policy: ResolutionStagingSession remains the single replacement owner. Pragmatic as npm: updates preserve a working install while replacement content downloads and validates.

Panel summary

Persona B R N Takeaway
Python Architect 0 1 1 Architecture is sound; the evidence SHA was corrected.
CLI Logging Expert 0 0 0 Progress and recovery guidance are state-aware.
DevX UX Expert 0 1 0 Canonical CLI and shipped usage docs were updated.
Supply Chain Security Expert 0 0 0 Validation, containment, activation, and rollback fail closed.
OSS Growth Hacker 0 0 0 The reliability value is clear in docs and changelog.
Doc Writer 0 0 0 Documentation matches the implemented boundary.
Test Coverage Expert 0 2 0 Registered-hook E2E and exact evidence SHAs were folded.
Performance Expert 0 0 0 Opaque slots and O(1) maps add no material regression.

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

Architecture

flowchart TD
    A["apm update or install --refresh"] --> B["Download to opaque staging slot"]
    B --> C{"Package validation"}
    C -->|invalid| D["Fail; keep current package and lockfile"]
    C -->|valid| E["Publish through ResolutionStagingSession"]
    E --> F{"Later install result"}
    F -->|success| G["Commit and remove journal"]
    F -->|failure or interruption| H["Restore exact prior package"]
Loading

Recommendation

The final follow-ups were folded and CI is green. Ready for maintainer review.

Folded in this run

  • (panel) Added a registered PreToolUse hook invocation during replacement download and after activation interruption -- resolved in a5ef135efd.
  • (panel) Corrected every Scenario Evidence row to the actual reviewed head -- resolved in a5ef135efd.
  • (panel) Documented transactional refresh and retry behavior in apm install, apm update, and the shipped apm-usage command reference -- resolved in a5ef135efd.
  • (panel) Isolated nested parent/child replacement journals and covered both publication orders -- resolved in 12f02d6ee7.
  • (panel) Rejected malformed candidates, preserved lock state, and distinguished live local cache paths -- resolved in ed81bcb3a0.
  • (panel) Restored direct semver refresh, validate-before-activate, fail-closed callbacks, and interruption rollback -- resolved in d906bc7faa.

Deferred (out-of-scope follow-ups)

  • (panel) Move every install integration output behind integrity and audit acceptance -- scope boundary: this PR covers package replacement hook liveness; a pipeline-wide transactional deployment redesign spans every target integrator and is a separate architecture theme.

Regression-trap evidence (mutation-break gate)

  • Registered-hook rollback E2E failed when immediate and transaction restoration were removed; both guards were restored.
  • Nested destination rollback tests failed when opaque staging slots were replaced with mirrored paths; the guard was restored.
  • Malformed candidate, local-cache distinction, activation ordering, callback failure, destination reservation, and AC36 owner tests each failed when their production/static guard was removed; all guards were restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ passed with no diagnostics. Pylint R0801, auth-signals, and architecture-boundary checks also passed.

CI

All checks passed on https://github.com/microsoft/apm/actions/runs/33314259137 after 0 CI fix iterations.

Mergeability status

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#2723 a5ef135 ship_with_followups 4 10 1 2 green MERGEABLE BLOCKED pending required review

Convergence

4 outer iterations; 2 Copilot rounds with no inline findings. Final panel stance: ship_with_followups; all in-scope follow-ups were folded.


Full per-persona findings
  • Python Architect: architecture sound; requested SHA discrepancy was clerical and is corrected.
  • CLI Logging Expert: no findings.
  • DevX UX Expert: canonical update/install and shipped usage docs were requested and folded.
  • Supply Chain Security Expert: no findings.
  • OSS Growth Hacker: no findings.
  • Auth Expert: inactive; no auth surface changed.
  • Doc Writer: no findings.
  • Test Coverage Expert: registered-hook interruption E2E and evidence SHA corrections were requested and folded.
  • 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) added a commit that referenced this pull request Aug 30, 2026
Keep semver refreshes staged through package validation, fail closed on callback errors, restore interrupted publication, and tighten the canonical activation guard. Addresses the review-panel follow-ups for PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Aug 30, 2026
Fail malformed refreshes before metadata commit, distinguish live local cache paths from staged candidates, and add integration evidence for hook continuity and rollback. Addresses the second review-panel pass on PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Aug 30, 2026
Use opaque transaction slots for overlapping package paths, keep failure guidance state-aware, and prove activation rollback through the install pipeline. Addresses the third review-panel pass on PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Daniel Meppiel (danielmeppiel) added a commit that referenced this pull request Aug 30, 2026
Exercise the registered PreToolUse command during failed replacement activation and document the transactional refresh contract on every canonical CLI surface. Addresses the final review-panel follow-ups for PR #2723.

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

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main at b68fcad -> fd3d7fb.

Conflicting paths resolved (faithful merge of both intents):

  • apm.lock.yaml

Regression-trap tests re-verified post-rebase (mutation-break gate):

  • BOM frontmatter unit, compile, and owner tests -- deleted the canonical utf-8-sig and removeprefix guards; tests FAILED as expected; guards restored.
  • tests/integration/test_architecture_frontmatter_bom.py::test_frontmatter_bom_guard_rejects_caller_owned_encoding -- deleted the AC36 boundary guard; test FAILED as expected; guard restored.

Lint contract: uv run --extra dev ruff check src/ tests/ and
uv run --extra dev ruff format --check src/ tests/ both silent
post-rebase.

Post-push mergeability: gh pr view --json mergeStateStatus,mergeable
reports BLOCKED / MERGEABLE (BLOCKED is the pending policy/review gate, not a conflict). Push used
git push --force-with-lease=fix/2717-atomic-plugin-reinstall:a5ef135efd837027eb0caece262c7ca6ccb8b977 origin HEAD:fix/2717-atomic-plugin-reinstall (--force-with-lease, never bare --force).

Ready for maintainer review.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep semver refreshes staged through package validation, fail closed on callback errors, restore interrupted publication, and tighten the canonical activation guard. Addresses the review-panel follow-ups for PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fail malformed refreshes before metadata commit, distinguish live local cache paths from staged candidates, and add integration evidence for hook continuity and rollback. Addresses the second review-panel pass on PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use opaque transaction slots for overlapping package paths, keep failure guidance state-aware, and prove activation rollback through the install pipeline. Addresses the third review-panel pass on PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Exercise the registered PreToolUse command during failed replacement activation and document the transactional refresh contract on every canonical CLI surface. Addresses the final review-panel follow-ups for PR #2723.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel
Daniel Meppiel (danielmeppiel) merged commit db031e5 into main Aug 30, 2026
18 checks passed
@danielmeppiel
Daniel Meppiel (danielmeppiel) deleted the fix/2717-atomic-plugin-reinstall branch August 30, 2026 15:46
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.

[BUG] apm install unlinks plugin hook scripts while their settings.json entries stay registered, wedging the agent

2 participants