chore: drop redundant apm-marketplace-check and Copilot marketplace mirror (findings 1, 2)

apm-marketplace-check checked network reachability of remote marketplace
refs, but finding 35 already removed the only remote package entry, so
every marketplace.packages[] source is local and the hook is pure
overlap with apm-pack-check-clean. apm-audit-ci was re-examined and kept
as-is -- its pre-commit-config.yaml comment already carries a dated,
verified justification the audit had missed.

check-marketplace-mirror-sync guarded .github/plugin/marketplace.json
against drift from .claude-plugin/marketplace.json. Verified against
current GitHub Copilot CLI docs: Copilot's marketplace discovery already
falls back through .github/plugin/marketplace.json to
.claude-plugin/marketplace.json, which this repo generates anyway -- the
dedicated mirror bought a discovery-order preference, not a capability.
Deleted the mirror file, its sync script, its test, and the hook.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
This commit is contained in:
2026-09-14 11:35:44 +00:00
parent afa7187d33
commit 0dffff3c21
10 changed files with 25 additions and 484 deletions

View File

@@ -61,11 +61,14 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
1. **Six hooks validate overlapping sets of the same manifests.** `check-manifests`, `validate-plugins`, `validate-marketplace`, `apm-pack-check-clean`, `apm-marketplace-check`, `apm-audit-ci`. Keep the two `claude plugin validate` hooks plus `apm-pack-check-clean`. ~~Delete `check-manifests` (282 lines + 771 test lines; its `lib/marketplace-plugins.sh` stays because `sync-plugin-content.sh` sources it).~~ `apm-audit-ci` spends 12 s confirming that manifests `apm pack` already parses do parse; drop or keep on that basis. Move the network-dependent `apm-marketplace-check` to a release checklist. Effort S.
> **Done (2026-09-12):** see commit `e647f14` on `docs/simplification-audit`. Deleted the `check-manifests` pre-commit hook entry, `scripts/check-manifests.sh` (282 lines), and `tests/test-check-manifests.sh` (771 lines); kept `scripts/lib/marketplace-plugins.sh`, still sourced by `sync-plugin-content.sh`. Updated the now-stale `check-manifests.sh` mentions in `README.md` and `docs/spec/gates.md` (hook table row and hook counts). The `apm-audit-ci` and `apm-marketplace-check` decisions in this finding remain open — out of scope for this change.
> **Grilled and closed (2026-09-14):** `apm-audit-ci` — already resolved before this audit was written: `.pre-commit-config.yaml`'s own comment block (added in commit `a155af6`, months before this audit) already rebuts the "overclaimed description" complaint and gives a dated, verified justification for what the hook still checks. Keep, no action. `apm-marketplace-check` — its stated purpose ("the only hook that checks remote package references rather than local-source paths") is void: finding 35 (commit `568ca74`) already removed the only remote package entry, so every `marketplace.packages[]` entry is now a local `./plugins/<name>` path and the hook is pure overlap with `apm-pack-check-clean`. Removed the hook entry, and corrected the now-stale "does NOT join apm-marketplace-check ... on the offline SKIP= list" comment on `apm-audit-ci` (there is no offline skip list any more — every pre-push hook already passes offline per `README.md`). Updated `README.md` (tool table, "Offline?" section) and `docs/spec/gates.md` (hook table, hook counts 13→11 self-authored / 15→13 total, the "Three of these shell out to apm" paragraph, and the "Pushing without a network" section) accordingly. Verified: `apm audit --ci` still passes per-plugin, and the pre-push hook count now matches `.pre-commit-config.yaml`.
2. **Four "keep two copies in sync" gates: 1,100 script lines + 1,600 test lines.** Each one is a symptom of duplication that could be removed instead of guarded:
- `check-vale-style-sync`: 413 lines + 798 test lines guarding a byte-identical 526-line `vale-wrap.sh` and style directory copied between skill-audit and agent-audit. About 350 of its lines run Vale glob probes against the hook file patterns. Disappears if the two audit skills merge (finding 14); the probes belong in `test-vale-wrap.sh`.
- `check-scope-walkup-sync`: 365 lines cross-checking four independent ports of the same package-root walk-up. Disappears if the ports share one script or the skills merge.
- `check-marketplace-mirror-sync`: guards `.github/plugin/marketplace.json`. The script header calls it Copilot's legacy convention path and says Copilot also accepts the Claude path; the vendored Copilot docs list it as primary. Verify against current Copilot CLI before deleting hook, script, test, and mirror file.
> **Grilled, held (2026-09-14):** both of the above are gated on findings 14/15 (merging skill-audit+agent-audit and skill-author+agent-author), deliberately held for a separate session rather than decided here. Correction for that session: the audit's §8 grouping is wrong — these merges don't need ADR-0012 revisited (that ADR governs the unrelated `core` plugin's three `agentsmd-*` skills). The actual constraint is ADR-0014 (no-cross-skill file sharing on plugin cache-install), and merging sidesteps it rather than requiring it be reversed. The open question for that session is a design one — a shared skill's `description` carrying both skill- and agent-audit trigger phrases — not an ADR supersession. ADR-0012 revisit is needed only for finding 24.
- [x] ~~`check-marketplace-mirror-sync`: guards `.github/plugin/marketplace.json`. The script header calls it Copilot's legacy convention path and says Copilot also accepts the Claude path; the vendored Copilot docs list it as primary. Verify against current Copilot CLI before deleting hook, script, test, and mirror file.~~
> **Grilled and done (2026-09-14):** verified against GitHub's current Copilot CLI plugin docs (not the vendored copy, which risked drift). Copilot CLI's marketplace discovery checks paths in order — `marketplace.json`, `.plugin/marketplace.json`, `.github/plugin/marketplace.json`, `.claude-plugin/marketplace.json` — falling through to whichever exists first. `.claude-plugin/marketplace.json` (apm's own `claude` output) already satisfies that chain's last step, so the dedicated `.github/plugin/marketplace.json` mirror bought Copilot users its *preferred* discovery path rather than a required one. Decided against reopening ADR-0018 (native install for both Claude Code and Copilot CLI stays supported) to justify this — the deletion holds either way, since Copilot's own fallback covers it. Deleted `.github/plugin/marketplace.json`, `scripts/sync-marketplace-mirror.sh` (81 lines), `tests/test-sync-marketplace-mirror.sh` (304 lines), and the `check-marketplace-mirror-sync` pre-push hook; removed the dangling references to the deleted script in `scripts/sync-plugin-content.sh` and `tests/test-sync-plugin-content.sh` (both had comments citing its reasoning by name), and updated `docs/spec/architecture.md`'s description of the marketplace-manifest compile step. `tests/test-sync-plugin-content.sh` (92 cases) still passes in full.
- [x] ~~`check-executables-allow-sync`: 474 lines to assert one string equals kyberforge's version. A six-line grep, or drop it (the failure mode is visible and recoverable).~~
> **Corrected then partially done (2026-09-13):** see commit `1b01e25` on `docs/simplification-audit`. Independent re-verification found "drop it" unsafe — ADR-0019's own Consequences section calls this failure mode *silent* and says a silent-staleness failure here is worse than the duplication the other gates catch, directly contradicting the finding's "visible and recoverable" claim. The hook stays. Shrunk `scripts/check-executables-allow-sync.sh` 231 → 222 lines by deduplicating two comment blocks that re-derived ADR-0019's own reasoning inline, replacing them with a pointer at the ADR. The dual-reader design (PyYAML plus a hand-rolled fallback, so a missing PyYAML can't silently skip the check) was found to be load-bearing, not redundant, and left intact; test file unchanged (behavior unaffected). All 23 test cases and the live pre-push hook run still pass.
Effort S each, M for the walk-up.