fix(install): cover skill bundles with per-file content-integrity + target-scoped union manifest (closes #1716)#1718
Conversation
…arget-scoped union manifest (closes #1716) Committed deployed content under `.agents/skills/**` could silently drift from its `packages/**` / `.apm/**` source without any CI gate catching it. Root cause was two independent install-manifest defects: 1. Manifest-clobber asymmetry. `apm install` REPLACED the lockfile deployment manifest with only the current target's paths, while on-disk stale cleanup correctly PRESERVES other targets' files. A `copilot-app` (user-machine DB) install therefore wiped all per-file content-integrity coverage of the committed `.github/` + `.agents/` tree. Fixed by making manifest reconciliation target-scoped UNION (symmetric with cleanup) in both the per-dependency block (phases/lockfile.py) and the project-root local block (phases/post_deps_local.py), via a shared helper `manifest_reconcile.union_preserving`. 2. Skills recorded as directory entries, never per-file hashed. Skill bundles were recorded as a single directory entry (e.g. `.agents/skills/auth`); `compute_deployed_hashes` skips directories, so skills had zero per-file hash coverage and SKILL.md drift escaped the documented `apm audit --ci --no-drift` content-integrity gate. Fixed by expanding each deployed skill directory into the directory entry PLUS its contained files at the single shared integration chokepoint (services._skill_bundle_file_entries). The directory entry is retained (cleanup's directory-rejection gate and the manifest dir-exclusion contract depend on it); the file entries get hashed. Net: the existing `--no-drift` content-integrity gate now protects `.agents/skills/...SKILL.md` with zero CI changes. Also regenerates the committed `apm.lock.yaml` (now per-file hashes for `.github/agents`, `.github/instructions`, and every `.agents/skills/<s>` file), re-syncs the drifted `apm-review-panel/SKILL.md` (415 -> 446 lines), and deploys the previously-missing `cut-release` skill. A third, deeper defect (the drift replay does not reproduce `copy_skill_to_target`'s cross-primitive link rewrite, so walking `.agents/` in the drift differ surfaces false positives) is documented in drift.py and deferred to a follow-up; Fix 2 already closes #1716 via content-integrity, so the drift differ need not walk `.agents/`. Tests: test_lockfile_union.py (union semantics, both manifest blocks); test_install_services_orchestration.py (skills recorded per-file). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens APM’s install/audit integrity model for multi-target deployments by ensuring the lockfile manifest is target-unioned (not clobbered by the last target installed) and by expanding skill bundle directory deployments into per-file entries so content-integrity can hash and detect drift under apm audit --ci --no-drift.
Changes:
- Add target-scoped manifest reconciliation (
union_preserving) and apply it to both per-dependency deployed manifests and the project-local deployed manifest. - Expand deployed skill bundle directories into per-file lockfile entries so skill contents (e.g.,
SKILL.md,assets/*) are covered by per-file hashing. - Regenerate
apm.lock.yamlto include per-file deployed hashes for committed skill bundles and related governed content; add regression tests.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/install/manifest_reconcile.py |
New shared helper to union current-target manifest entries with preserved other-target entries. |
src/apm_cli/install/phases/lockfile.py |
Use target-scoped union reconciliation when attaching per-dependency deployed files + hashes. |
src/apm_cli/install/phases/post_deps_local.py |
Apply the same union reconciliation to local_deployed_files / local_deployed_file_hashes. |
src/apm_cli/install/services.py |
Expand skill bundle directory deployments into per-file deployed-file entries for hashing coverage. |
tests/unit/install/phases/test_lockfile_union.py |
Unit regression coverage for union-preserving semantics and governance computation. |
tests/integration/test_install_services_orchestration.py |
Integration regression coverage for skill-dir expansion into per-file deployed entries. |
src/apm_cli/install/drift.py |
Document intentional exclusion of .agents/ from drift replay walking due to known replay/link-rewrite inconsistency. |
apm.lock.yaml |
Regenerated lockfile with per-file deployed hashes covering committed deployed content (notably .agents/skills/**). |
.agents/skills/apm-review-panel/SKILL.md |
Deployed skill content updated to include the performance expert persona and related guidance. |
.agents/skills/apm-review-panel/assets/panelist-return-schema.json |
Schema enum extended to include performance-expert. |
.agents/skills/cut-release/SKILL.md |
New deployed cut-release skill bundle defining a release-cut workflow. |
.agents/skills/cut-release/assets/entry-sanitizer.md |
Changelog entry sanitization rubric for release PR preparation. |
.agents/skills/cut-release/assets/pr-body-template.md |
Template for release PR body composition. |
.agents/skills/cut-release/assets/semver-rubric.md |
Semver bump decision rubric used by the release-cut workflow. |
.agents/skills/cut-release/evals/evals.json |
Trigger-eval configuration for the cut-release skill. |
.agents/skills/cut-release/scripts/bump-version.sh |
Script to bump pyproject.toml version and refresh uv.lock. |
.agents/skills/cut-release/scripts/list-changes-since-tag.sh |
Script to enumerate merged PRs since last tag and emit structured JSON. |
.agents/skills/cut-release/scripts/verify-lint-mirror.sh |
Script to run the repo’s CI-mirror lint chain locally. |
Copilot's findings
- Files reviewed: 18/18 changed files
- Comments generated: 4
| def install_governance(targets) -> tuple[set[str], set[str]]: | ||
| """Return ``(file_roots, uri_schemes)`` governed by *targets*. |
| def union_preserving( | ||
| current_files, | ||
| current_hashes, | ||
| prior_files, | ||
| prior_hashes, |
| for tp in skill_result.target_paths: | ||
| deployed.append(_deployed_path_entry(tp, project_root, targets)) | ||
| # #1716: also record the bundle's contained files so per-file | ||
| # content hashes cover SKILL.md / assets / scripts. The directory | ||
| # entry above is retained (cleanup's directory-rejection gate and | ||
| # the manifest dir-exclusion contract depend on it); the file |
| # Extract PR numbers from commit subjects (squash-merge convention: "(#NNNN)"). | ||
| # Fallback: use `gh pr list --search` keyed by sha if no number is in the subject. | ||
| PR_NUMS=() |
… traps (#1716) Phase 2 of the #1716 fix, extending the Phase 1 target-complete integrity manifest. Makes the install-replay drift differ walk per-primitive deploy roots so committed skill bundles under .agents/skills/** are compared against their source, and runs that gate live in CI. Changes: - drift.py: _governed_root_dirs() now includes each target primitive's deploy_root (first path segment), not just the target root_dir, so the differ walks .agents/ for the copilot skills target. Previously it only collected .apm + .github, leaving .agents/skills/** invisible to drift. - ci.yml (apm-self-check): drop --no-drift from the audit gate so drift runs target-aware; delete the dead Gate B git-status step (a guaranteed no-op -- no apm install runs in the job and its watch list omitted .agents/ anyway). - apm.lock.yaml + .agents/skills/<10>/SKILL.md: regenerate the committed deployed tree to the deterministic `apm install` output form. The committed copies carried stale cross-primitive agent links (../../agents/... pointing at the non-existent .agents/agents/) that install never self-heals because it skips skill dirs already byte-identical to source. Fresh install emits ../../../.apm/agents/... (resolves to the real source agent); regenerating makes committed == install == replay so the new drift gate is clean. - Acceptance traps as regression tests (red->green across the fix): - Trap A (content-integrity): tamper a deployed .agents/skills/<s>/SKILL.md -> _check_content_integrity fails. tests/unit/policy/test_ci_checks.py. - Trap B (drift): a skill deploy_root walked by the differ surfaces drift. tests/unit/install/test_drift.py (deploy_root-walk tests fail on the pre-fix _governed_root_dirs). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ift helpers Fold Copilot review: annotate the new manifest_reconcile helpers (install_governance, union_preserving) and the drift _governed_root_dirs helper with explicit parameter types, satisfying the repo typing guideline (.github/instructions/python.instructions.md). TargetProfile is imported under TYPE_CHECKING (no runtime cost, no import cycle). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Shepherd-driver advisory (terminal pass)Drove this PR's #1716 fix to a landing-ready state: classified the Copilot inline review, folded in-scope items, ran a focused defect review of the full diff, and confirmed CI green. Folded in this run
Copilot signals reviewed
Deferred (out-of-scope follow-ups)
Regression-trap evidence (mutation-break gate)
Lint contract
CIAll required checks green on head Focused defect reviewRan a high-signal review over the full Mergeability status
Convergence1 outer iteration; 1 Copilot round. Focused review verdict: clean / ship. Ready for maintainer review. |
The #1716 fix is user-observable (apm audit now catches drift/tampering in committed .agents/skills bundles), so it needs a changelog entry per .github/instructions/changelog.instructions.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge origin/main (PRs #1714, #1717, #1718, #1719) into the release branch. Only #1718 is user-facing (src/apm_cli/install drift + target-complete lockfile manifest); its entry was authored on main and now sits in the [0.19.0] Fixed block with the PR number appended. #1714, #1717, #1719 are maintainer-toolkit / test-only and dropped per the entry-sanitizer DROP list. Lint mirror green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ckfile The APM Self-Check (apm audit --ci) content-integrity gate failed: apm.lock.yaml recorded a stale deployed_file_hashes entry (fe6a73...) for .agents/skills/apm-review-panel/SKILL.md while the deployed file and its packages/** source are both fc63e25... #1714 updated the skill content but deliberately left apm.lock.yaml untouched; #1718 then turned the drift gate live (dropped --no-drift), exposing the mismatch. apm install reconciles the recorded hash (and refreshes apm_version to 0.19.0). All 9 audit checks now pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* chore: release v0.19.0 Bump pyproject.toml + uv.lock to 0.19.0 and move the [Unreleased] CHANGELOG block to [0.19.0] - 2026-06-09 with one 'so what' entry per merged PR. Lint mirror green locally (ruff check + format, pylint R0801, auth-signals). Post-merge: tag v0.19.0 to trigger the release workflow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: fold #1718 into v0.19.0 changelog after main merge Merge origin/main (PRs #1714, #1717, #1718, #1719) into the release branch. Only #1718 is user-facing (src/apm_cli/install drift + target-complete lockfile manifest); its entry was authored on main and now sits in the [0.19.0] Fixed block with the PR number appended. #1714, #1717, #1719 are maintainer-toolkit / test-only and dropped per the entry-sanitizer DROP list. Lint mirror green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(self-check): reconcile stale apm-review-panel SKILL.md hash in lockfile The APM Self-Check (apm audit --ci) content-integrity gate failed: apm.lock.yaml recorded a stale deployed_file_hashes entry (fe6a73...) for .agents/skills/apm-review-panel/SKILL.md while the deployed file and its packages/** source are both fc63e25... #1714 updated the skill content but deliberately left apm.lock.yaml untouched; #1718 then turned the drift gate live (dropped --no-drift), exposing the mismatch. apm install reconciles the recorded hash (and refreshes apm_version to 0.19.0). All 9 audit checks now pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e integrity) Sync with main through v0.19.0 (#1715, #1714, #1718, #1719, #1717). Conflicts resolved: - CHANGELOG.md: main cut the v0.19.0 release, moving prior Unreleased entries into a dated section. Took main's restructure; relocated the #1681 "Tightened Stage 2 code-complexity thresholds" entry to the new ## [Unreleased] section (released sections stay immutable per Keep a Changelog). Took main's #1702/#1710 close-ref for the lockfile entry. - src/apm_cli/install/services.py: #1718 added per-file skill-bundle integrity recording inline at a site my refactor had extracted into _log_skill_result. Kept the new _skill_bundle_file_entries helper in services.py (sibling of _deployed_path_entry); dropped main's duplicate inline _log_hook_display_payloads (already relocated to services_integrate.py and re-exported in my #1700 port); accepted the _log_skill_result delegator at the call site. - src/apm_cli/install/services_integrate.py: ported #1718's deployed.extend(_skill_bundle_file_entries(tp, ...)) into _log_skill_result's deployed-path loop (deployed aliases result["deployed_files"]), with a lazy import of the helper. Shadow gate: no markers, all src <=800 lines, ruff check + format clean, pylint R0801 EXIT=0, auth-signals clean; 11144 tests pass across the install/integration suites plus the #1718, #1700, and #1709 targeted sets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fix(install): catch silent drift in committed
.agents/skillsbundlesTL;DR
Committed deployed content under
.agents/skills/**could silently drift from itspackages/**/.apm/**source while every CI gate stayed green (#1716). The fix makes the lockfile integrity manifest target-complete (per-file hashes for every committed deploy target), makes the drift differ target-aware (walks each primitive'sdeploy_root), and runs that gate live in CI by dropping--no-drift. Two regression traps lock the behavior in.Note
Closes #1716. Discovered while shipping #1714 (fixes #1712), where
apm.lock.yamlwas deliberately left untouched to preserve evidence for this investigation.Problem (WHY)
.agents/skills/apm-review-panel/SKILL.mdshipped drifted (415 deployed lines vs 446 in source — an entire persona missing) yetapm audit --ci --no-driftstayed green.apm.lock.yamlcarried zerodeployed_file_hashes: a multi-target deploy then acopilot-appinstall rewrote the manifest to a single target, orphaning the committed.github/+.agents/files from every manifest-driven gate..agents/skills/auth), andcompute_deployed_hashesskips directories (phases/lockfile.pyline 42), soSKILL.mdhad no per-file hash coverage..agents/at all —_governed_root_dirsreturned only.apm+.github, so even with drift enabled the skill tree was invisible.Why these matter:
apm auditis APM's governance gate, and a gate that cannot observe its own deployed artifacts is not a gate. Grounding the audit in a deterministic install-replay is what makes drift verifiable rather than assumed — per PROSE, "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action."Approach (WHAT)
manifest_reconcile.union_preservingreconciles the lockfile symmetrically with on-disk cleanup: the current install's entries are authoritative, other targets' entries are preserved (not clobbered).services._skill_bundle_file_entriesexpands each deployed skill dir into the dir entry plus its files, socontent-integrityhashesSKILL.md/ assets / scripts.drift._governed_root_dirsnow includes every primitive'sdeploy_rootfirst segment, so the replay differ walks.agents/skills/**.ci.ymlapm-self-checkdrops--no-driftand deletes a dead Gate-B git-status step (a guaranteed no-op).apm installoutput (stale broken../../agents/links →../../../.apm/agents/) and refreshedapm.lock.yamlhashes.Implementation (HOW)
src/apm_cli/install/manifest_reconcile.py(new) —union_preserving+install_governance; a small shared helper imported by both manifest build sites so the reconciliation logic is identical.src/apm_cli/install/services.py—_skill_bundle_file_entriesat the single skill-integration chokepoint; the directory entry is retained (cleanup's directory-rejection contract depends on it).src/apm_cli/install/phases/lockfile.py&post_deps_local.py— callunion_preservingfor the per-dependency and project-root manifests.src/apm_cli/install/drift.py—_governed_root_dirswalks eachdeploy_root; the replay reproduces the deploy-time link rewrite, so byte-identical skills do not surface as false drift..github/workflows/ci.yml— Gate A is nowapm audit --ci; dead Gate B removed.apm.lock.yaml+.agents/skills/<10>/SKILL.md— regenerated tool output (data, not logic).Diagrams
Legend: the manifest path — how a
copilot-appinstall used to clobber skill coverage, and howunion_preserving(dashed) plus per-file skill entries (dashed) restore per-file hashing undercontent-integrity.flowchart LR subgraph Deploy["Deploy"] D1["copilot target writes .agents/skills + .github"] D2["copilot-app install writes DB-URI rows"] end subgraph Manifest["Lockfile manifest"] M3["union_preserving reconcile"] M1["deployed_files"] M4["per-file skill entries SKILL.md + assets"] M2["deployed_file_hashes"] end subgraph Audit["apm audit"] A1["content-integrity hashes each file"] end D1 --> M1 D2 --> M3 M3 --> M1 M1 --> M4 M4 --> M2 M2 --> A1 classDef new stroke-dasharray: 5 5; class M3,M4 new;Legend: the drift path —
apm audit --cireplays the install into a scratch tree, and the differ now walks the.agentsdeploy root (dashed) so committed skill bundles are compared against the replay.flowchart LR subgraph Source["Source"] S1[".apm/skills + packages"] end subgraph Replay["apm audit --ci replay"] R1["install replay scratch tree"] R2["_governed_root_dirs adds .agents deploy_root"] end subgraph Compare["Differ"] C1["walk .agents/skills"] C2["drift found, exit 1"] end S1 --> R1 R1 --> R2 R2 --> C1 C1 --> C2 classDef new stroke-dasharray: 5 5; class R2,C1 new;Trade-offs
manifest_reconcilerather than duplicating the union logic in both build sites — one source of truth, in the spirit of PROSE's "Favor small, chainable primitives over monolithic frameworks."../../agents/resolves to a non-existent.agents/agents/); a freshapm installdeterministically emits../../../.apm/agents/. Chose to make committed == install output so the differ is clean in steady state.apm installwon't self-heal an already-deployed skill dir whose bytes match source; the new drift gate now catches this, but a self-healing install is a separate PR.post_deps_localunion over overwrite. Could in principle preserve a stale entry for an ungoverned top-level file; verified unreachable today (local_deployed_filesholds only.github/**+.agents/**, both governed by the copilot target).Benefits
.agents/skills/<s>/SKILL.mdnow failsapm audit --ci --no-drift(exit 1) instead of passing silently.apm audit --ciwith the exact deployed path that drifted.apm.lock.yamlcovers every committed file-based deploy target (42 per-file skill entries), not one.apm-self-checkjob runs the drift gate on every PR — no--no-driftescape hatch.Validation
apm audit --ci(drift gate now live,--no-driftdropped):Live trap reproduction (both gates fail on drift)
CI on head
420ab863: all required checks green, including APM Self-Check running the new target-aware drift gate. Local unit gate:pytest tests/unit tests/test_console.py -n 2 --dist worksteal→ 16819 passed.Scenario Evidence
.agents/skills/<s>/SKILL.mdis caught byapm audit --ci --no-drifttests/unit/policy/test_ci_checks.py::test_agents_skill_tamper_fails_content_integrity(regression-trap for #1716)apm audit --citests/unit/install/test_drift.py::test_diff_engine_walks_skill_deploy_root_detects_drift(regression-trap for #1716)tests/unit/install/test_drift.py::test_diff_engine_skill_deploy_root_clean_when_identicaldeploy_root(.agents)tests/unit/install/test_drift.py::test_governed_root_dirs_includes_primitive_deploy_rootHow to test
uv run apm audit --cion a clean checkout → all 9 checks pass.printf '\nX\n' >> .agents/skills/auth/SKILL.mdthenuv run apm audit --ci --no-drift→ exit 1 (content-integrity); restore withgit checkout.printf '\nX\n' >> .apm/skills/auth/SKILL.mdthenuv run apm audit --ci→ exit 1 (drift: .agents/skills/auth/SKILL.md); restore.uv run pytest tests/unit/install/test_drift.py tests/unit/policy/test_ci_checks.py -q→ green.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com