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

@@ -1,54 +0,0 @@
{
"name": "holocron",
"description": "AI development skills for Claude Code and GitHub Copilot CLI — factory, design, implement, review, and cross-cutting workflows.",
"version": "0.4.6",
"owner": {
"name": "Defame1297",
"email": "defame1297@rkdr.net",
"url": "https://git.dev.rkdr.net/Defame1297/"
},
"plugins": [
{
"name": "kyberforge",
"description": "Skills and agents for creating, maintaining, and managing a Claude Code / Copilot CLI plugin marketplace.",
"version": "1.6.2",
"category": "Developer Tools",
"source": "./plugins/kyberforge"
},
{
"name": "bin",
"description": "Skills for everyday AI-assisted development work that is not tied to a single tool, forge or language, and has not yet been split into a focused plugin.",
"version": "1.1.7",
"category": "Utilities",
"source": "./plugins/bin"
},
{
"name": "git",
"description": "Skills and agents for working with a local Git clone over the git wire protocol, and for authoring and running the pre-commit hooks that guard it.",
"version": "1.3.7",
"category": "Version Control",
"source": "./plugins/git"
},
{
"name": "gitea",
"description": "Skills and agents for working with a Gitea forge through its HTTP API — the forge's own objects, as distinct from the local git clone.",
"version": "1.3.8",
"category": "Version Control",
"source": "./plugins/gitea"
},
{
"name": "core",
"description": "Skills for authoring and auditing a repo's AGENTS.md and the provider adapter files that defer to it.",
"version": "1.1.2",
"category": "Productivity",
"source": "./plugins/core"
},
{
"name": "lint",
"description": "Skills and agents for configuring and running linters.",
"version": "1.1.7",
"category": "Developer Tools",
"source": "./plugins/lint"
}
]
}

View File

@@ -84,15 +84,6 @@ repos:
pass_filenames: false
always_run: true
- id: check-marketplace-mirror-sync
name: Check marketplace mirror sync
description: Verify .github/plugin/marketplace.json (Copilot CLI's legacy manifest path) is byte-identical to .claude-plugin/marketplace.json -- apm has no output profile for this path, so it must be kept in sync explicitly (see issue #90)
entry: bash scripts/sync-marketplace-mirror.sh --check
language: system
stages: [pre-push]
pass_filenames: false
always_run: true
- id: check-executables-allow-sync
name: Check executables allow key sync
description: Verify root apm.yml's executables.allow key names kyberforge's actual version -- apm matches that key by exact "<package>#<version>" lookup, so a version bump on one side alone silently stops deploying kyberforge's hooks/ and bin/ and lets the apm install go stale (see ADR-0019)
@@ -102,15 +93,6 @@ repos:
pass_filenames: false
always_run: true
- id: apm-marketplace-check
name: apm marketplace check
description: Validate every marketplace.packages[] entry resolves, including network reachability of remote refs -- the only hook that checks remote package references rather than local-source paths
entry: apm marketplace check
language: system
stages: [pre-push]
pass_filenames: false
always_run: true
- id: apm-audit-ci
name: apm audit --ci
description: Run apm's producer-side CI gate over the root manifest AND each of the six plugin packages. Verifies exactly two things per manifest -- apm.yml parses as a valid APM manifest (manifest-parse), and, if it declares dependencies, apm.lock.yaml exists and is consistent (lockfile-exists). It does NOT enforce an org policy and does NOT scan for hidden Unicode; see the comment below for why. Reference:plugins/kyberforge/.apm/skills/apm-workflow/references/audit.md
@@ -153,8 +135,8 @@ repos:
# six plugin packages is what makes either reachable for them at all --
# the root-only invocation audits the marketplace manifest and nothing
# else. Costs ~0.5s per package, needs no network (checked under
# `unshare -rn`), so this does NOT join apm-marketplace-check and
# apm-pack-check-clean on the offline SKIP= list.
# `unshare -rn`) -- consistent with every other pre-push hook: none of
# them need the network (see README.md's "Offline?" section).
- id: check-apm-agents-valid
name: Validate real APM agent files

View File

@@ -30,7 +30,7 @@ Install all of these before setting up. Each one is a hard dependency of a git h
| Tool | Why | Install |
| --- | --- | --- |
| `apm` CLI | Four pre-push hooks shell out to it (`apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`, and `check-plugin-content-sync` via `scripts/sync-plugin-content.sh`) | The `apm-install` skill, or `curl -sSL https://aka.ms/apm-unix \| sh`. Verify with `apm --version` |
| `apm` CLI | Three pre-push hooks shell out to it (`apm-audit-ci`, `apm-pack-check-clean`, and `check-plugin-content-sync` via `scripts/sync-plugin-content.sh`) | The `apm-install` skill, or `curl -sSL https://aka.ms/apm-unix \| sh`. Verify with `apm --version` |
| `jq` | Required by `scripts/sync-plugin-content.sh`, pre-push | Your package manager |
| `python3` + PyYAML | Required by `scripts/skill-size-check.sh` (the `skill-size-check` pre-commit hook), which reads folded YAML frontmatter | `python3` is usually present — pre-commit is itself a Python application. `pip install pyyaml` if the hook reports PyYAML missing |
| `vale` | Required by the `vale-audit-prefilter-skill` / `-agent` pre-commit hooks and the `check-vale-style-sync` pre-push hook | `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or https://vale.sh/docs/vale-cli/installation/ |
@@ -92,7 +92,7 @@ Every other pre-push hook does run.
See [`docs/spec/gates.md`](docs/spec/gates.md) for what each hook enforces and why.
**Offline?** No pre-push hook needs the network: root `apm.yml`'s marketplace has no remote package entries (the last one, `mattpocock-skills`, was removed), so `apm-marketplace-check` and `apm-pack-check-clean` resolve everything from local sources. All pre-push hooks pass offline.
**Offline?** No pre-push hook needs the network: root `apm.yml`'s marketplace has no remote package entries (the last one, `mattpocock-skills`, was removed), so `apm-pack-check-clean` resolves everything from local sources. All pre-push hooks pass offline.
## Editing plugin content

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.

View File

@@ -44,7 +44,7 @@ These are routing boundaries, not inventories — they answer "where does a new
Two compilers produce the plugin roots you see in the tree:
- **`apm pack` compiles the manifests** (ADR-0015). Per plugin: `.claude-plugin/plugin.json` and `.github/plugin/plugin.json`, both generated from `plugins/<name>/apm.yml`. Repo-wide, from the root `apm.yml`'s `marketplace:` block: `.claude-plugin/marketplace.json` (apm's `claude` output profile) — the only marketplace output this repo declares. A second root manifest, `.github/plugin/marketplace.json` (Copilot CLI's legacy path), is a byte-identical mirror of the Claude one maintained by `scripts/sync-marketplace-mirror.sh` and gated by the `check-marketplace-mirror-sync` pre-push hook.
- **`apm pack` compiles the manifests** (ADR-0015). Per plugin: `.claude-plugin/plugin.json` and `.github/plugin/plugin.json`, both generated from `plugins/<name>/apm.yml`. Repo-wide, from the root `apm.yml`'s `marketplace:` block: `.claude-plugin/marketplace.json` (apm's `claude` output profile) — the only marketplace manifest this repo generates or ships. Copilot CLI checks for a marketplace manifest at several conventional paths, falling back through `.github/plugin/marketplace.json` to `.claude-plugin/marketplace.json` — since this repo already generates the latter, no dedicated Copilot-path mirror is maintained.
- **`scripts/sync-plugin-content.sh` compiles the content mirror** (ADR-0017). It wraps `apm pack --format plugin` and copies the resulting bundle's flat `agents/`, `skills/`, `commands/`, `instructions/`, `extensions/`, and merged `hooks/hooks.json` back to the plugin root. Claude Code's installer convention-scans those flat paths and has no `.apm/` awareness whatsoever, so the mirror exists solely to satisfy the host's discovery contract.
`.apm/` is the sole hand-edited authoring source for plugin content. An edit made in the flat mirror is discarded by the next sync and is reported as drift by the `check-plugin-content-sync` pre-push hook. Hand-authored material that is not an `.apm/` primitive — `README.md`, `docs/`, `bin/`, `sources.md`, `.mcp.json`, and per-plugin extras such as `plugins/gitea/references/` and `plugins/bin/evals/` — lives at the plugin **root** and is untouched by either compiler.

View File

@@ -21,24 +21,24 @@ Install hooks via `pc-run`, wiring **all three stages**. This repo's `.pre-commi
`default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits)
and `pre-push` (everything below).
The pre-push command reports **15** hooks, not 13. The extra two are pre-commit's own `meta` hooks,
The pre-push command reports **13** hooks, not 11. The extra two are pre-commit's own `meta` hooks,
`check-hooks-apply` and `check-useless-excludes`: they declare no `stages:`, so they run at every
stage including this one. Both are declared in this repo's `.pre-commit-config.yaml` like everything
else — what separates them is `repo: meta` (pre-commit's own built-ins) from `repo: local`. Thirteen
else — what separates them is `repo: meta` (pre-commit's own built-ins) from `repo: local`. Eleven
is the count of hooks this repo authors itself.
**The caveat: one of those 13 is a silent no-op under that invocation.**
**The caveat: one of those 11 is a silent no-op under that invocation.**
`check-release-needed` exits 0 immediately unless `PRE_COMMIT_REMOTE_BRANCH` equals
`refs/heads/main`, and pre-commit exports that variable only from the real pre-push git hook during
an actual `git push`. Running the stage by hand — or from a CI runner — therefore reports it
`Passed` having checked nothing. That is by design for feature branches — pushing WIP must not be
blocked on cutting a premature tag — but it means `--hook-stage pre-push --all-files` is a full
rehearsal of 12 hooks and a skip of the thirteenth. The script's own header records the same gap for
rehearsal of 10 hooks and a skip of the eleventh. The script's own header records the same gap for
a PR merged through Gitea's merge button, where no local push happens at all.
## The pre-push gate
Thirteen hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in.
Eleven hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in.
**Core checks**
@@ -51,7 +51,6 @@ Thirteen hooks, grouped below by what they guard rather than by the order `.pre-
| Hook | Guards |
|---|---|
| `check-plugin-content-sync` | each plugin's flat `skills/agents/commands/hooks` mirror matches `.apm/` (issue #90) |
| `check-marketplace-mirror-sync` | `.github/plugin/marketplace.json` is byte-identical to `.claude-plugin/marketplace.json` — no apm output profile targets that path |
| `check-vale-style-sync` | skill-audit's Vale copy matches agent-audit's canonical copy, plus six glob-coverage probes (see [Vale](#vale)) |
| `check-scope-walkup-sync` | `validate.sh`, `validate-provenance.sh`, `new-agent.sh` and `new-skill.sh`'s four independent `$HOME`/`.git`/`apm.yml` walk-up ports still agree behaviorally |
| `check-executables-allow-sync` | root `apm.yml`'s `executables.allow` key names kyberforge's actual version (see [apm gates](#apm-gates)) |
@@ -69,7 +68,6 @@ drift in generated text.
| Hook | Guards |
|---|---|
| `apm-marketplace-check` | every `marketplace.packages[]` entry resolves, including network reachability of remote refs |
| `apm-audit-ci` | `apm audit --ci` once per manifest — root plus each of the six plugin packages |
| `apm-pack-check-clean` | `apm pack --check-versions --check-clean --dry-run` — the compiled marketplace still matches what `apm.yml` + `.apm/` would generate, and per-package versions agree with the `per_package` strategy |
@@ -86,10 +84,10 @@ drift in generated text.
|---|---|
| `check-release-needed` | on a real `git push` to `main` only — fails if files exposed via `.pre-commit-hooks.yaml` changed since the last tag. A no-op everywhere else, including under `pre-commit run --hook-stage pre-push` (see [the caveat above](#running-the-gates)) |
Four of these shell out to `apm`: `apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`,
and `check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wraps `apm pack`). The
first and third are bare `apm …` entries and the second is a `bash -c` loop calling `apm` once per
package, so without the CLI the push dies with an unhelpful "command not found". Install with
Three of these shell out to `apm`: `apm-audit-ci`, `apm-pack-check-clean`, and
`check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wraps `apm pack`). The
second is a bare `apm …` entry and the first is a `bash -c` loop calling `apm` once per package, so
without the CLI the push dies with an unhelpful "command not found". Install with
`apm-install`, or `curl -sSL https://aka.ms/apm-unix | sh`; verify with `apm --version`. `jq` is
needed by `scripts/sync-plugin-content.sh` — it at least fails loudly (`Error: jq is required but
not installed`).
@@ -889,8 +887,7 @@ fix.
## Pushing without a network
No pre-push hook needs the network. Every entry in root `apm.yml`'s `marketplace.packages[]`
resolves from a local `./plugins/<name>` path, so `apm-marketplace-check` and `apm-pack-check-clean`
never call `git ls-remote`.
resolves from a local `./plugins/<name>` path, so `apm-pack-check-clean` never calls `git ls-remote`.
`apm-audit-ci` calls `apm` too but was always local: its org-policy discovery resolves nothing on
this remote before any network call.

View File

@@ -1,81 +0,0 @@
#!/usr/bin/env bash
set -euo pipefail
# .claude-plugin/marketplace.json is apm's compiled Claude marketplace output (see
# apm.yml's marketplace.outputs.claude). GitHub Copilot CLI's manifest lookup accepts
# that same file at .claude-plugin/marketplace.json directly, but also has a legacy
# convention path at .github/plugin/marketplace.json (see
# plugins/kyberforge/docs/research/docs/github-copilot-plugins/marketplace.md) -- and
# that path is a mirror of the Claude output, not a separate apm output profile (this
# repo declares only the "claude" output). This script keeps that legacy mirror
# byte-identical to .claude-plugin/marketplace.json instead of letting it silently
# drift (see issue #90 comment thread).
# Hard error, not a `|| pwd` fallback. Every path this script touches hangs off
# REPO_ROOT, and both of its exits-0 paths are "the files agree" or "neither file
# exists" -- so a REPO_ROOT pointing somewhere that is not this repo reports "no
# drift" over a tree it never looked at. Run `--check` from an empty directory
# outside any worktree and the fallback made that the literal outcome: rev-parse
# failed, REPO_ROOT became $PWD, neither file was there, exit 0. Refusing to guess
# is the only answer that cannot be silently wrong; the `-f "$DST"` branch below
# covers a genuinely stale mirror, which is a different condition.
if ! REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" || [[ -z "$REPO_ROOT" ]]; then
echo "Error: not inside a git worktree -- cannot locate the repository root, and guessing \$PWD would let --check report \"no drift\" over a tree it never inspected. Run this from within the repository." >&2
exit 1
fi
SRC="$REPO_ROOT/.claude-plugin/marketplace.json"
DST="$REPO_ROOT/.github/plugin/marketplace.json"
usage() {
echo "Usage: $0 [--check]" >&2
exit 1
}
CHECK=0
if [[ "${1:-}" == "--check" ]]; then
CHECK=1
shift
fi
[[ $# -eq 0 ]] || usage
if [[ ! -f "$SRC" ]]; then
# A missing source with a surviving mirror is drift, not absence: the mirror
# can only be stale (nothing is left for it to be byte-identical to), which is
# precisely the silent divergence this script exists to prevent. Exiting 0
# here would report "no drift" over a mirror of a file that no longer exists.
# (An unresolvable REPO_ROOT is handled above and is a hard error; this branch
# is only about a source file that is genuinely gone from a real worktree.)
# scripts/sync-plugin-content.sh --check --all already errors on the same
# condition ("requires .../marketplace.json"); this matches it.
# Neither file present stays a genuine no-op: nothing to mirror, nothing stale.
if [[ "$CHECK" -eq 1 && -f "$DST" ]]; then
echo "DRIFT $DST: mirror exists but .claude-plugin/marketplace.json does not" >&2
echo "Fix: restore .claude-plugin/marketplace.json (apm's compiled Claude marketplace output), or delete $DST" >&2
exit 1
fi
exit 0
fi
if [[ "$CHECK" -eq 1 ]]; then
if [[ ! -f "$DST" ]] || ! diff -q "$SRC" "$DST" >/dev/null 2>&1; then
echo "DRIFT $DST: out of sync with .claude-plugin/marketplace.json" >&2
# The runnable command gets a line to ITSELF, and the rationale gets its own
# echo. It was one line -- `Fix: bash scripts/sync-marketplace-mirror.sh --
# apm ships no output profile...` -- which put the prose after `--`, the
# POSIX end-of-options marker, so copy-pasting the Fix line ran this script
# with ~24 stray argv entries: `${1:-}` was `--` (so CHECK stayed 0 and no
# shift happened), `[[ $# -eq 0 ]]` failed, and the tool meant to fix the
# drift answered with its own usage error and exit 1. The backticks around
# `apm pack` made it worse: the paste also command-substituted a real
# `apm pack` run before the script was even reached. Hence plain quotes
# below too. Keep the command alone on its line.
echo "Fix: run, from the repository root:" >&2
echo " bash scripts/sync-marketplace-mirror.sh" >&2
echo "Note: apm ships no output profile targeting this path, so 'apm pack' does not refresh it. Expecting it to is exactly the drift this script and its pre-push hook exist to prevent." >&2
exit 1
fi
exit 0
fi
mkdir -p "$(dirname "$DST")"
cp "$SRC" "$DST"

View File

@@ -196,9 +196,9 @@ if [[ "$ALL" -eq 1 ]]; then
# duplicate at every call site (see .pre-commit-config.yaml's
# check-plugin-content-sync).
#
# Hard error rather than a `|| pwd` fallback, on scripts/sync-marketplace-mirror.sh's
# reasoning: --all's entire work list hangs off REPO_ROOT, so a REPO_ROOT pointing at
# something that is not this repo checks a plugin set that is not this repo's. Run
# Hard error rather than a `|| pwd` fallback: --all's entire work list hangs off
# REPO_ROOT, so a REPO_ROOT pointing at something that is not this repo checks a
# plugin set that is not this repo's. Run
# from outside a worktree the fallback happens to hit the `--all requires ...` error
# below instead -- but only by accident, because $PWD had no marketplace.json in it;
# $PWD holding an unrelated one is the case that would silently "pass".

View File

@@ -1,304 +0,0 @@
#!/usr/bin/env bash
set -euo pipefail
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
SCRIPT="$REPO_ROOT/scripts/sync-marketplace-mirror.sh"
PASS=0
FAIL=0
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
# Fixture: a temp directory that is its own git worktree. Since
# scripts/sync-marketplace-mirror.sh resolves REPO_ROOT via
# `git rev-parse --show-toplevel 2>/dev/null || pwd`, isolation has to come
# from that call answering "the fixture" -- so the fixture owns a real .git.
#
# Relying instead on `git rev-parse` FAILING inside a bare `mktemp -d` (its
# previous form) is not isolation: two ordinary conditions make it succeed and
# resolve to the LIVE repo, at which point every test writes to this repo's own
# tracked .github/plugin/marketplace.json. Both are reproduced and fixed here:
# - TMPDIR pointing inside a git worktree, which puts the fixture in one.
# - An inherited GIT_DIR/GIT_WORK_TREE, which re-targets `git -C` and
# `git rev-parse` regardless of cwd. tests/run-tests.sh is itself a pre-push
# hook, and git hooks export exactly those variables -- the same leak
# tests/test-git-hooks-install.sh:6-10 already defends against.
# run_script() strips GIT_DIR/GIT_WORK_TREE for the second; `git init` here
# covers the first and makes the resolution positive rather than accidental.
# run-tests.sh:52-53 asserts no test writes back into the live repo tree, and
# that claim now carries the concurrent runner.
make_fixture() {
local dir
dir="$(mktemp -d)"
# Physical path: `git rev-parse --show-toplevel` reports the resolved path,
# and on macOS `mktemp -d` hands back one under the /tmp -> /private/tmp
# symlink. Without -P the script's REPO_ROOT and the assertions' $FIXTURE
# would name the same directory differently and every diff would compare
# against a path the script never wrote.
dir="$(cd "$dir" && pwd -P)"
# Checked, not best-effort: this init IS the isolation invariant. `echo "$dir"`
# is the last statement, so a silently-failed init would return a plain temp
# dir, `git rev-parse --show-toplevel` would walk up to whatever repo encloses
# it, and the suite would go back to writing into the live tree.
if ! env -u GIT_DIR -u GIT_WORK_TREE git -C "$dir" init -q >/dev/null 2>&1; then
echo "make_fixture: 'git init' failed in $dir — every test would then resolve REPO_ROOT to an enclosing repo and write outside the fixture" >&2
exit 1
fi
echo "$dir"
}
SRC_REL=".claude-plugin/marketplace.json"
DST_REL=".github/plugin/marketplace.json"
CONTENT_A='{"plugins":[{"name":"demo","version":"1.0.0"}]}'
CONTENT_B='{"plugins":[{"name":"demo","version":"2.0.0"}]}'
write_src() {
local dir="$1" content="$2"
mkdir -p "$dir/.claude-plugin"
printf '%s' "$content" > "$dir/$SRC_REL"
}
write_dst() {
local dir="$1" content="$2"
mkdir -p "$dir/.github/plugin"
printf '%s' "$content" > "$dir/$DST_REL"
}
# `env -u GIT_DIR -u GIT_WORK_TREE` mirrors tests/test-git-hooks-install.sh:10:
# under a git hook (run-tests.sh runs as pre-push) those are exported, and the
# script's `git rev-parse --show-toplevel` would then answer with the LIVE repo
# no matter which directory it was invoked from.
run_script() {
local dir="$1"
shift
(cd "$dir" && env -u GIT_DIR -u GIT_WORK_TREE bash "$SCRIPT" "$@")
}
CLEANUP_DIRS=()
trap 'rm -rf ${CLEANUP_DIRS[@]+"${CLEANUP_DIRS[@]}"}' EXIT
track() { CLEANUP_DIRS+=("$1"); }
# --- 1. No .claude-plugin/marketplace.json at all: real-sync mode is a no-op, exit 0 ---
echo ""
echo "--- missing source: real sync exits 0 and creates nothing ---"
FIXTURE1="$(make_fixture)"; track "$FIXTURE1"
if run_script "$FIXTURE1" > /dev/null 2>&1 && [[ ! -e "$FIXTURE1/$DST_REL" ]]; then
pass "missing source exits 0 in real-sync mode without creating a mirror"
else
fail "missing source should exit 0 and create nothing in real-sync mode"
fi
# --- 2. No source: --check mode is also a no-op, exit 0 (nothing to drift-check) ---
echo ""
echo "--- missing source: --check exits 0 (nothing to compare) ---"
FIXTURE2="$(make_fixture)"; track "$FIXTURE2"
if run_script "$FIXTURE2" --check > /dev/null 2>&1; then
pass "missing source exits 0 in --check mode"
else
fail "missing source should exit 0 in --check mode, not report drift"
fi
# --- 2b. Source missing but a mirror still present: --check must FAIL ---
# --check used to exit 0 on any missing source, so deleting
# .claude-plugin/marketplace.json left a stale .github/plugin/marketplace.json
# reported as "no drift" -- a mirror of a file that no longer exists. That is
# the silent divergence this script's header says it prevents ("keeps that
# legacy mirror byte-identical ... instead of letting it silently drift"), and
# scripts/sync-plugin-content.sh --check --all already errors on the same
# condition. Case 2 above still holds: neither file present stays a no-op.
echo ""
echo "--- missing source with a surviving mirror: --check reports drift ---"
FIXTURE2B="$(make_fixture)"; track "$FIXTURE2B"
write_dst "$FIXTURE2B" "$CONTENT_A"
if run_script "$FIXTURE2B" --check > /dev/null 2>&1; then
fail "exited 0 with a stale mirror and no source -- expected drift (exit 1)"
else
pass "a mirror with no source left to mirror is reported as drift"
fi
# Real-sync mode keeps its no-op: it has nothing to copy, and deleting a
# tracked file is not this script's call to make.
if run_script "$FIXTURE2B" > /dev/null 2>&1 && [[ -f "$FIXTURE2B/$DST_REL" ]]; then
pass "real-sync mode still no-ops on a missing source, leaving the mirror alone"
else
fail "real-sync mode should no-op on a missing source, not fail or delete the mirror"
fi
# --- 3. Source exists, mirror missing entirely: --check reports drift (exit 1) ---
echo ""
echo "--- --check reports drift when the mirror file does not exist yet ---"
FIXTURE3="$(make_fixture)"; track "$FIXTURE3"
write_src "$FIXTURE3" "$CONTENT_A"
if run_script "$FIXTURE3" --check > /dev/null 2>&1; then
fail "exited 0 with no mirror file present -- expected drift (exit 1)"
else
pass "missing mirror file is reported as drift"
fi
# --- 4. Real sync creates the mirror (and its parent dir) byte-identical to the source ---
echo ""
echo "--- real sync creates .github/plugin/marketplace.json byte-identical to source ---"
FIXTURE4="$(make_fixture)"; track "$FIXTURE4"
write_src "$FIXTURE4" "$CONTENT_A"
if run_script "$FIXTURE4" > /dev/null 2>&1 \
&& [[ -f "$FIXTURE4/$DST_REL" ]] \
&& diff -q "$FIXTURE4/$SRC_REL" "$FIXTURE4/$DST_REL" > /dev/null 2>&1; then
pass "real sync creates the mirror file identical to the source"
else
fail "real sync did not create a byte-identical mirror file"
fi
# --- 5. --check is clean immediately after a real sync ---
echo ""
echo "--- --check is clean right after syncing ---"
if run_script "$FIXTURE4" --check > /dev/null 2>&1; then
pass "no drift reported immediately after syncing"
else
fail "drift reported right after syncing -- sync and check disagree"
fi
# --- 6. Editing the source after a sync introduces drift; re-sync clears it ---
echo ""
echo "--- source edited after sync is detected as drift, cleared by re-sync ---"
write_src "$FIXTURE4" "$CONTENT_B"
if run_script "$FIXTURE4" --check > /dev/null 2>&1; then
fail "no drift reported after editing the source -- expected drift"
else
pass "editing the source after a sync is detected as drift"
run_script "$FIXTURE4" > /dev/null 2>&1
if run_script "$FIXTURE4" --check > /dev/null 2>&1; then
pass "re-sync clears the drift"
else
fail "re-sync did not clear the drift"
fi
fi
# --- 7. A mirror that was hand-seeded already identical to the source: --check is clean ---
echo ""
echo "--- --check is clean when the mirror already matches, without ever syncing ---"
FIXTURE7="$(make_fixture)"; track "$FIXTURE7"
write_src "$FIXTURE7" "$CONTENT_A"
write_dst "$FIXTURE7" "$CONTENT_A"
if run_script "$FIXTURE7" --check > /dev/null 2>&1; then
pass "an already-in-sync mirror reports no drift"
else
fail "an already-in-sync mirror should report no drift"
fi
# --- 8. A stale, pre-existing mirror that differs from source: --check flags it, sync fixes it ---
echo ""
echo "--- a stale pre-existing mirror is flagged by --check and overwritten by sync ---"
FIXTURE8="$(make_fixture)"; track "$FIXTURE8"
write_src "$FIXTURE8" "$CONTENT_A"
write_dst "$FIXTURE8" "$CONTENT_B"
if run_script "$FIXTURE8" --check > /dev/null 2>&1; then
fail "a stale mirror was not flagged as drift"
else
pass "a stale pre-existing mirror is flagged as drift"
fi
run_script "$FIXTURE8" > /dev/null 2>&1
if diff -q "$FIXTURE8/$SRC_REL" "$FIXTURE8/$DST_REL" > /dev/null 2>&1; then
pass "real sync overwrites the stale mirror to match the source"
else
fail "real sync did not overwrite the stale mirror"
fi
# --- 9. An unknown/extra positional argument is rejected with a usage error ---
echo ""
echo "--- an unrecognized argument is rejected (usage, exit 1) ---"
FIXTURE9="$(make_fixture)"; track "$FIXTURE9"
write_src "$FIXTURE9" "$CONTENT_A"
if run_script "$FIXTURE9" --bogus > /dev/null 2>&1; then
fail "exited 0 with an unrecognized argument -- expected a usage error"
else
pass "an unrecognized argument is rejected"
fi
# --- 10. --check followed by a stray extra argument is also rejected ---
echo ""
echo "--- --check plus a trailing extra argument is rejected (usage, exit 1) ---"
if run_script "$FIXTURE9" --check extra > /dev/null 2>&1; then
fail "exited 0 with --check plus a trailing argument -- expected a usage error"
else
pass "--check plus a trailing extra argument is rejected"
fi
# --- 11. Real sync is idempotent: running it twice in a row leaves the mirror unchanged ---
echo ""
echo "--- running real sync twice in a row is idempotent ---"
FIXTURE11="$(make_fixture)"; track "$FIXTURE11"
write_src "$FIXTURE11" "$CONTENT_A"
run_script "$FIXTURE11" > /dev/null 2>&1
run_script "$FIXTURE11" > /dev/null 2>&1
if run_script "$FIXTURE11" --check > /dev/null 2>&1; then
pass "running sync twice in a row is idempotent"
else
fail "a second sync run introduced unexpected drift"
fi
# --- 12. Fixture isolation survives an inherited GIT_DIR/GIT_WORK_TREE ---
# The whole suite's isolation is REPO_ROOT resolving to the fixture. With
# GIT_DIR/GIT_WORK_TREE exported -- which is every git-hook context, and
# run-tests.sh runs as pre-push -- `git rev-parse --show-toplevel` answers with
# THAT repo from any cwd, so the script wrote to the live tree and 5 of these
# cases failed. Point both variables at a decoy repo (never the live one, so
# this assertion cannot itself write where it must not) and assert the fixture
# still wins: the mirror lands in the fixture and the decoy stays untouched.
echo ""
echo "--- fixture isolation holds with GIT_DIR/GIT_WORK_TREE inherited from elsewhere ---"
FIXTURE12="$(make_fixture)"; track "$FIXTURE12"
DECOY="$(make_fixture)"; track "$DECOY"
write_src "$FIXTURE12" "$CONTENT_A"
if (export GIT_DIR="$DECOY/.git" GIT_WORK_TREE="$DECOY"; run_script "$FIXTURE12" > /dev/null 2>&1) \
&& [[ -f "$FIXTURE12/$DST_REL" ]] && [[ ! -e "$DECOY/$DST_REL" ]]; then
pass "an inherited GIT_DIR/GIT_WORK_TREE does not redirect writes out of the fixture"
else
fail "an inherited GIT_DIR/GIT_WORK_TREE redirected the sync outside the fixture"
fi
# --- 13. Outside any git worktree, REPO_ROOT cannot be guessed: hard error ---
# `git rev-parse --show-toplevel 2>/dev/null || pwd` used to fall back to $PWD.
# Both of this script's exit-0 paths are "the two files agree" or "neither file
# exists", so a REPO_ROOT that is not this repo reports "no drift" over a tree it
# never inspected — run --check from an empty non-worktree directory and that was
# the literal outcome. Case 2 above (a real worktree with no source file) still
# exits 0; the difference is whether the tree was identified at all.
#
# GIT_CEILING_DIRECTORIES rather than trusting `mktemp -d` to land outside a
# worktree: TMPDIR may itself sit inside one (the same hazard make_fixture's
# header documents), in which case rev-parse would succeed and this case would
# quietly test nothing. The ceiling stops git's upward walk at the fixture's
# parent, and the precondition below asserts it actually did.
echo ""
echo "--- outside a git worktree, --check errors instead of guessing \$PWD ---"
NOREPO_PARENT="$(mktemp -d)"; track "$NOREPO_PARENT"
NOREPO_PARENT="$(cd "$NOREPO_PARENT" && pwd -P)"
NOREPO="$NOREPO_PARENT/not-a-worktree"
mkdir -p "$NOREPO"
if (cd "$NOREPO" && env -u GIT_DIR -u GIT_WORK_TREE GIT_CEILING_DIRECTORIES="$NOREPO_PARENT" \
git rev-parse --show-toplevel > /dev/null 2>&1); then
fail "precondition: git rev-parse still resolves a worktree under the ceiling — this case would test nothing"
else
for MODE in "--check" ""; do
RC13=0
OUT13="$( (cd "$NOREPO" && env -u GIT_DIR -u GIT_WORK_TREE \
GIT_CEILING_DIRECTORIES="$NOREPO_PARENT" bash "$SCRIPT" ${MODE:+"$MODE"}) 2>&1 )" || RC13=$?
case "$RC13:$OUT13" in
0:*)
fail "'${MODE:-real sync}' exited 0 outside a git worktree — it reported on a tree it never identified" ;;
*"not inside a git worktree"*)
pass "'${MODE:-real sync}' errors with a not-a-worktree message instead of falling back to \$PWD" ;;
*)
fail "'${MODE:-real sync}' failed for an unexpected reason (rc=$RC13): $OUT13" ;;
esac
done
# And it must not have written anything into the directory it refused to trust.
if [[ ! -e "$NOREPO/.github" ]]; then
pass "nothing is written into the unidentified directory"
else
fail "the script created files under a directory it could not identify as the repo root"
fi
fi
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

View File

@@ -1012,9 +1012,8 @@ make_repo_fixture() {
printf '%s' "$marketplace_json" > "$dir/.claude-plugin/marketplace.json"
echo "$dir"
}
# `env -u GIT_DIR -u GIT_WORK_TREE` for the same reason
# tests/test-sync-marketplace-mirror.sh does it: run-tests.sh runs as a pre-push
# hook, and git hooks export both variables, which would re-target the script's
# `env -u GIT_DIR -u GIT_WORK_TREE`: run-tests.sh runs as a pre-push hook, and git
# hooks export both variables, which would re-target the script's
# `git rev-parse --show-toplevel` at the LIVE repo from any cwd.
run_all() {
local dir="$1"
@@ -1066,8 +1065,7 @@ check_all_fails_with "an entry whose source is an array" \
# --- 28b. --all outside a git worktree refuses instead of guessing $PWD ---
# --all's entire work list hangs off REPO_ROOT, so a `|| pwd` fallback lets it derive
# that list from a marketplace.json belonging to some other tree. Same reasoning
# scripts/sync-marketplace-mirror.sh dropped its own fallback on. Run from a directory
# that list from a marketplace.json belonging to some other tree. Run from a directory
# with no marketplace.json the old form happened to hit the "--all requires ..." error,
# but only by accident — the dangerous case is a $PWD that HAS one.
echo ""