fix: honor per-dep targets and plural targets: in audit drift replay#1930
Merged
Conversation
Fixes two false-positive 'unintegrated' drift bugs in apm audit --ci: #1923 -- dep_target_subset not threaded into replay run_replay called integrate_package_primitives without dep_target_subset, so a dependency narrowed to targets: [claude] via per-dependency 'targets:' in apm.yml would still have copilot replay skills into .agents/skills/ in the scratch tree, causing false 'unintegrated' findings for paths the install never wrote. Fix: pass dep_target_subset=lock_dep.target_subset or None to integrate_package_primitives from the loop in run_replay. #1924 -- _read_apm_yml_target read 'target:' but not 'targets:' _read_apm_yml_target used data.get('target') directly and parse_target_field, missing the plural 'targets:' list form. A project with 'targets: [claude, codex]' (no copilot) that also has .github/ for unrelated CI workflows had copilot auto-detected during replay, writing .github/instructions/ into scratch and reporting false 'unintegrated' drift. Fix: replace the hand-rolled single-key lookup with parse_targets_field from core/apm_yml.py, which handles both 'target:' (singular) and 'targets:' (plural list) forms and validates tokens against the canonical target set. Tests: add test_run_replay_threads_dep_target_subset; add test_apm_yml_with_targets_list_returns_list; update stale test_apm_yml_with_target_returns_value / test_parse_target_field_* tests in test_drift_detection.py and test_drift_phase3.py to reflect that _read_apm_yml_target now returns list[str] via parse_targets_field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes two false-positive unintegrated drift reports in apm audit --ci by ensuring drift replay uses the same target resolution and per-dependency target narrowing as apm install.
Changes:
- Thread
lock_dep.target_subsetintointegrate_package_primitives(..., dep_target_subset=...)during drift replay to honor per-dependencytargets:narrowing (#1923). - Update
_read_apm_yml_target()to useparse_targets_field()so bothtarget:andtargets:forms are honored during replay (#1924). - Adjust unit tests to reflect the new
_read_apm_yml_target()return shape (list[str] | None) and add coverage for per-dep target subset forwarding.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/apm_cli/install/drift.py | Aligns drift replay with install-time target parsing (targets:) and per-dependency target filtering (dep_target_subset). |
| tests/unit/install/test_drift.py | Adds a unit test asserting dep_target_subset is forwarded into integration during replay. |
| tests/unit/install/test_drift_phase3.py | Updates _read_apm_yml_target expectations to the new list-return behavior and patches parse_targets_field. |
| tests/unit/install/test_drift_detection.py | Adds coverage for plural targets: parsing and updates singular/exception tests for the list-return behavior. |
…et aware Address Copilot code-review suggestions on test_run_replay_threads_dep_target_subset: - Use an explicit apm.yml with 'targets: [claude, copilot]' instead of relying on directory-based auto-detection via .github/ presence. This makes target resolution deterministic and matches the real #1923 scenario where a copilot target is active but the dep is scoped to claude only. - Expand the spy to capture both the resolved 'targets' list (target names) and 'dep_target_subset'. The test now also asserts that 'copilot' is in the resolved target set, confirming the subset actually narrows integration (not just a no-op subset on a single-target replay). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
approved these changes
Jun 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Two false-positive
unintegrateddrift bugs inapm audit --cifixedin a single, minimal change to
src/apm_cli/install/drift.py.Problem (WHY)
#1923 -- per-dependency
targets:not honored in replayapm installcorrectly narrows deployment for a dependency declared withtargets: [claude]inapm.yml-- only.claude/skills/is written.The lockfile stores
target_subset: [claude]on the locked entry.However,
run_replaycalledintegrate_package_primitiveswithoutdep_target_subset, so the replay ran copilot primitives into.agents/skills/in scratch even though the dep was scoped to claude.diff_scratch_against_projectthen found.agents/skills/<dep>/SKILL.mdin scratch but not in the project and reported false
unintegrateddrift.#1924 -- plural
targets:field ignored during replay_read_apm_yml_targetreaddata.get("target")(singular) and passedit to
parse_target_field. A project withtargets: [claude, codex](plural list form, no copilot) returns
Nonefrom the old lookup, soresolve_targetsfell through to directory-based auto-detection. With.github/present for unrelated CI workflows, copilot was auto-detected,and the replay wrote
.github/instructions/local.instructions.mdintoscratch. The project tree had no such file, yielding false
unintegrateddrift.Approach (WHAT)
Two surgical fixes, both in
src/apm_cli/install/drift.py:Pass
dep_target_subsetinrun_replay-- threadlock_dep.target_subset or Noneintointegrate_package_primitivesso the per-dependency target filter that already works at install time
is applied identically during replay.
Use
parse_targets_fieldin_read_apm_yml_target-- replace thehand-rolled
data.get("target")+parse_target_fieldpath withparse_targets_fieldfromcore/apm_yml.py, which already handlesboth
target:(singular) andtargets:(plural list) forms andvalidates tokens against the canonical set.
Implementation (HOW)
No CLI surface change. No new dependencies. No docs change required
(the
targets:per-dependency semantics are already documented andthe fix restores intended behavior).
Architecture
flowchart TD A[apm audit --ci] --> B[_check_drift] B --> C[run_replay] C --> D[_read_apm_yml_target] D --> |"parse_targets_field (both forms)"| E[resolve_targets with explicit_target] C --> F[integrate_package_primitives] F --> |"dep_target_subset=lock_dep.target_subset"| G[filter_targets_for_dependency] G --> H[Write to scratch only for allowed targets] B --> I[diff_scratch_against_project] I --> J[No false unintegrated findings]sequenceDiagram participant Audit as apm audit --ci participant Replay as run_replay participant Parse as _read_apm_yml_target participant Integrate as integrate_package_primitives participant Filter as filter_targets_for_dependency Audit->>Replay: run_replay(config) Replay->>Parse: _read_apm_yml_target(project_root) Note over Parse: parse_targets_field handles<br/>target: and targets: (#1924) Parse-->>Replay: ["claude", "codex"] Replay->>Integrate: ...dep_target_subset=["claude"] Note over Integrate: filter_targets_for_dependency<br/>narrows to claude only (#1923) Integrate-->>Replay: scratch written to .claude/ only Replay-->>Audit: scratch_root Note over Audit: diff finds no phantom .agents/ entriesTrade-offs
_read_apm_yml_targetnow returnslist[str]instead ofstr | None.resolve_targets(explicit_target=...)already accepts a list, so nodownstream change is needed. Updated tests reflect the new return type.
parse_targets_fieldmeans an invalid token intargets:nowraises inside
_read_apm_yml_target'sexcept Exceptionguard andfalls back to auto-detect (same safe fallback as before).
Validation evidence
How to test
targets: [claude, codex]and a.github/directory (no copilot target).
apm audit --ci-- should report no drift (was falseunintegratedfor.github/instructions/before this fix).For #1923:
targets: [claude]inapm.yml.apm install) -- only.claude/skills/is written.apm audit --ci-- should report no drift (was falseunintegratedfor.agents/skills/<dep>/SKILL.mdbefore this fix).Closes #1923
Closes #1924