Use forked apm to stop settings.json hook duplication#508
Merged
Conversation
srid/agency delegates the test and CI steps to project-local skills, but kolu's workflow instructions only described `just check` and `just fmt` — so `/do` had no documented way to invoke `/test` or `/ci`. Point the test and CI steps at the colocated skills so the pipeline knows what to run.
Bare `apm install` duplicates a package's hook entries in .claude/settings.json on every re-run because the merge integrator extends the per-event list unconditionally (microsoft/apm#708). The kolu workaround — wiping .claude/ inside the `apm` recipe and `rm -f .claude/settings.json` inside `apm-update` — works but masks the upstream bug and leaks destructive behaviour into every install. Switch `apm_cmd` to juspay/apm#fix/hook-idempotent-install, which upserts by `_apm_source` before appending, and drop both workarounds. Revert to `microsoft/apm` once the fix lands upstream.
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.
Bare
apm installduplicates hook entries in.claude/settings.jsonon every re-run, so kolu'sapmrecipe papers over it by wiping.claude/before each install andapm-updatedoesrm -f settings.jsonfor the same reason. The real bug is microsoft/apm#708 — the merge integrator.extend()s the per-event hook list unconditionally instead of upserting by_apm_source.Pin
apm_cmdto microsoft/apm#709, which filters existing entries owned by the package before appending fresh ones. Fix is eight lines inhook_integrator.pyplus two regression tests covering idempotent re-integration and healing of pre-existing duplicates. Both workarounds inai.just— the wholesale.claude/wipe and the settings.jsonrm -f— are dropped;apm installis now safe to run over a live tree.The workflow instructions also pick up a small fix:
/donow knows to invoke the/testand/ciskills for the test and CI pipeline steps (previously onlyjust check/just fmtwere documented, leaving those steps without a verified execution path).