chore: delete the check-manifests pre-commit hook
Six pre-push hooks were validating overlapping sets of the same manifests. check-manifests (marketplace.json/plugin.json path checks) is redundant with validate-plugins (claude plugin validate) and apm-pack-check-clean, which already cover the same ground. Deletes the check-manifests hook entry, scripts/check-manifests.sh (282 lines), and tests/test-check-manifests.sh (771 lines). scripts/lib/marketplace-plugins.sh is kept — it is still sourced by sync-plugin-content.sh. Updates the now-stale check-manifests.sh mentions and hook counts in README.md and docs/spec/gates.md. The apm-audit-ci and apm-marketplace-check hooks named in the same finding are left untouched — the audit flags them as needing a separate decision. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
This commit is contained in:
@@ -75,15 +75,6 @@ repos:
|
|||||||
pass_filenames: false
|
pass_filenames: false
|
||||||
always_run: true
|
always_run: true
|
||||||
|
|
||||||
- id: check-manifests
|
|
||||||
name: Check plugin manifests
|
|
||||||
description: Validate marketplace.json and plugin.json paths
|
|
||||||
entry: bash scripts/check-manifests.sh
|
|
||||||
language: system
|
|
||||||
stages: [pre-push]
|
|
||||||
pass_filenames: false
|
|
||||||
always_run: true
|
|
||||||
|
|
||||||
- id: check-plugin-content-sync
|
- id: check-plugin-content-sync
|
||||||
name: Check plugin content sync
|
name: Check plugin content sync
|
||||||
description: Verify each plugin's flat skills/agents/commands/hooks/hooks.json mirror is in sync with .apm/ -- Claude Code has no .apm/ awareness so this compiled mirror must stay current (see issue #90)
|
description: Verify each plugin's flat skills/agents/commands/hooks/hooks.json mirror is in sync with .apm/ -- Claude Code has no .apm/ awareness so this compiled mirror must stay current (see issue #90)
|
||||||
|
|||||||
@@ -31,7 +31,7 @@ Install all of these before setting up. Each one is a hard dependency of a git h
|
|||||||
| Tool | Why | Install |
|
| 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 | 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` |
|
||||||
| `jq` | Required by `scripts/check-manifests.sh` and `scripts/sync-plugin-content.sh`, both pre-push | Your package manager |
|
| `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 |
|
| `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/ |
|
| `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/ |
|
||||||
| `claude` CLI | Required by the `validate-plugins` and `validate-marketplace` pre-push hooks | Claude Code |
|
| `claude` CLI | Required by the `validate-plugins` and `validate-marketplace` pre-push hooks | Claude Code |
|
||||||
|
|||||||
@@ -59,7 +59,8 @@ Five suites account for 215 s of 276 s. Three of those five (sync-plugin-content
|
|||||||
|
|
||||||
This is the area you named as hardest to understand and slowest. Root cause: most pre-push hooks exist to keep two copies of something in sync, or to re-validate what another hook already validates.
|
This is the area you named as hardest to understand and slowest. Root cause: most pre-push hooks exist to keep two copies of something in sync, or to re-validate what another hook already validates.
|
||||||
|
|
||||||
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.
|
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 `9051d14` 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.
|
||||||
|
|
||||||
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:
|
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-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`.
|
||||||
|
|||||||
@@ -21,31 +21,30 @@ 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)
|
`default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits)
|
||||||
and `pre-push` (everything below).
|
and `pre-push` (everything below).
|
||||||
|
|
||||||
The pre-push command reports **16** hooks, not 14. The extra two are pre-commit's own `meta` hooks,
|
The pre-push command reports **15** hooks, not 13. 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
|
`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
|
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`. Fourteen
|
else — what separates them is `repo: meta` (pre-commit's own built-ins) from `repo: local`. Thirteen
|
||||||
is the count of hooks this repo authors itself.
|
is the count of hooks this repo authors itself.
|
||||||
|
|
||||||
**The caveat: one of those 14 is a silent no-op under that invocation.**
|
**The caveat: one of those 13 is a silent no-op under that invocation.**
|
||||||
`check-release-needed` exits 0 immediately unless `PRE_COMMIT_REMOTE_BRANCH` equals
|
`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
|
`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
|
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
|
`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
|
blocked on cutting a premature tag — but it means `--hook-stage pre-push --all-files` is a full
|
||||||
rehearsal of 13 hooks and a skip of the fourteenth. The script's own header records the same gap for
|
rehearsal of 12 hooks and a skip of the thirteenth. 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.
|
a PR merged through Gitea's merge button, where no local push happens at all.
|
||||||
|
|
||||||
## The pre-push gate
|
## The pre-push gate
|
||||||
|
|
||||||
Fourteen hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in.
|
Thirteen hooks, grouped below by what they guard rather than by the order `.pre-commit-config.yaml` declares them in.
|
||||||
|
|
||||||
**Core checks**
|
**Core checks**
|
||||||
|
|
||||||
| Hook | Guards |
|
| Hook | Guards |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `run-tests` | `bash tests/run-tests.sh --strict` — the whole suite, skips fatal (see [Tests](#tests)) |
|
| `run-tests` | `bash tests/run-tests.sh --strict` — the whole suite, skips fatal (see [Tests](#tests)) |
|
||||||
| `check-manifests` | `marketplace.json` and `plugin.json` paths resolve (needs `jq`) |
|
|
||||||
|
|
||||||
**Generated-content drift gates**
|
**Generated-content drift gates**
|
||||||
|
|
||||||
@@ -92,8 +91,8 @@ and `check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wra
|
|||||||
first and third are bare `apm …` entries and the second is a `bash -c` loop calling `apm` once per
|
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
|
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
|
`apm-install`, or `curl -sSL https://aka.ms/apm-unix | sh`; verify with `apm --version`. `jq` is
|
||||||
needed by `scripts/check-manifests.sh` and `scripts/sync-plugin-content.sh` — those at least fail
|
needed by `scripts/sync-plugin-content.sh` — it at least fails loudly (`Error: jq is required but
|
||||||
loudly (`Error: jq is required but not installed`).
|
not installed`).
|
||||||
|
|
||||||
## Skill and agent context gates (ADR-0020)
|
## Skill and agent context gates (ADR-0020)
|
||||||
|
|
||||||
|
|||||||
@@ -1,282 +0,0 @@
|
|||||||
#!/usr/bin/env bash
|
|
||||||
set -euo pipefail
|
|
||||||
|
|
||||||
# Validates that marketplace.json's local plugin entries resolve to a real directory
|
|
||||||
# containing a .claude-plugin/plugin.json. Run from repo root or pass REPO_ROOT as arg.
|
|
||||||
#
|
|
||||||
# Per ADR-0015, apm.yml is the authoring source and .claude-plugin/plugin.json is
|
|
||||||
# compiled output with no skills/hooks/mcpServers/agents pointer fields (apm's plugin.json
|
|
||||||
# builder deliberately omits them -- Claude Code auto-discovers those convention
|
|
||||||
# directories, so listing them would be redundant/invalid). For a plugin with an .apm/
|
|
||||||
# directory, this script no longer checks those pointer fields itself; that's
|
|
||||||
# scripts/sync-plugin-content.sh --check's job (drift between .apm/ and the flat
|
|
||||||
# plugin-root mirror), wired as its own pre-push hook.
|
|
||||||
#
|
|
||||||
# sync-plugin-content.sh --check explicitly skips any plugin directory lacking .apm/
|
|
||||||
# (an apm-native package it has nothing to compile), so that delegation leaves a real
|
|
||||||
# gap for a non-apm plugin whose hand-authored plugin.json still uses the old
|
|
||||||
# skills/hooks/mcpServers/agents pointer-field convention: nothing would check whether
|
|
||||||
# those paths resolve. The fallback block below restores that check, but only for
|
|
||||||
# plugins without .apm/ -- apm-native plugins keep relying on the delegation above so
|
|
||||||
# the two checks don't duplicate (and disagree) on the same manifest.
|
|
||||||
#
|
|
||||||
# Both of the above walk marketplace.json -> disk. Nothing walked disk -> marketplace,
|
|
||||||
# so a plugins/<name>/ directory that never made it into marketplace.json was invisible
|
|
||||||
# to every marketplace-derived gate at once (this script and sync-plugin-content.sh
|
|
||||||
# --all both derive their plugin set from marketplace.json). The final block below
|
|
||||||
# closes that direction: per ADR-0015 marketplace.json is compiled output of root
|
|
||||||
# apm.yml's marketplace.packages[], so an on-disk apm package with no entry is
|
|
||||||
# compiled-output drift of exactly the kind ADR-0017 wires pre-push gates for -- and it
|
|
||||||
# is the same plugin set the validate-plugins pre-commit hook already globs as
|
|
||||||
# plugins/*/.
|
|
||||||
#
|
|
||||||
# Every pass above reads its plugin set out of marketplace.json, so anything that makes
|
|
||||||
# that file yield nothing -- absent, unparseable, a non-object root, or an entry whose
|
|
||||||
# `source` is neither a path string nor a remote object -- used to read as "clean"
|
|
||||||
# rather than "unchecked". The same is true one level down, of a per-plugin
|
|
||||||
# .claude-plugin/plugin.json that does not parse: it aborted the walk mid-loop and left
|
|
||||||
# every later plugin silently unchecked. The guards below turn each of those into an
|
|
||||||
# explicit, attributable failure instead, because a vacuous pass is the one result a gate
|
|
||||||
# must never produce.
|
|
||||||
|
|
||||||
# Hard error, not a `|| pwd` fallback, for the reason spelled out in
|
|
||||||
# scripts/sync-marketplace-mirror.sh: every path below hangs off REPO_ROOT, and the
|
|
||||||
# exit-0 path is "nothing on disk and no manifest", so a REPO_ROOT pointing somewhere
|
|
||||||
# that is not this repo reports "clean" over a tree it never looked at. Run this from
|
|
||||||
# an empty directory outside any worktree and the fallback made that the literal
|
|
||||||
# outcome -- rev-parse failed, REPO_ROOT became $PWD, no plugins/ and no
|
|
||||||
# marketplace.json were found, exit 0, silent.
|
|
||||||
if [[ -n "${1:-}" ]]; then
|
|
||||||
REPO_ROOT="$1"
|
|
||||||
elif ! 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 this check report \"clean\" over a tree it never inspected. Run it from within the repository, or pass the repo root as an argument." >&2
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
FAIL=0
|
|
||||||
|
|
||||||
err() { echo " FAIL: $1" >&2; FAIL=$((FAIL + 1)); }
|
|
||||||
|
|
||||||
if ! command -v jq &>/dev/null; then
|
|
||||||
echo "Error: jq is required but not installed" >&2
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
MARKETPLACE="$REPO_ROOT/.claude-plugin/marketplace.json"
|
|
||||||
|
|
||||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
||||||
# Repo-root-relative, not script-dir-relative -- see tests/run-tests.sh for why.
|
|
||||||
# shellcheck source=scripts/lib/marketplace-plugins.sh
|
|
||||||
source "$SCRIPT_DIR/lib/marketplace-plugins.sh"
|
|
||||||
|
|
||||||
# Candidate plugin directories on disk. The trigger is any of the three markers that
|
|
||||||
# make a directory a plugin rather than scratch -- apm.yml (the ADR-0015 authoring
|
|
||||||
# source), .apm/ (its content tree), or a compiled .claude-plugin/plugin.json. Matching
|
|
||||||
# all three keeps this set aligned with the plugins/*/ glob the validate-plugins
|
|
||||||
# pre-commit hook uses, which is the disagreement the disk -> marketplace pass below
|
|
||||||
# exists to close; a directory with none of them is scratch and stays out of scope.
|
|
||||||
#
|
|
||||||
# It is collected before the marketplace is read because a missing marketplace.json is
|
|
||||||
# only "nothing to check" when there is also nothing on disk to check against it.
|
|
||||||
PLUGIN_DIRS=()
|
|
||||||
for candidate in "$REPO_ROOT"/plugins/*/; do
|
|
||||||
candidate="${candidate%/}"
|
|
||||||
[[ -d "$candidate" ]] || continue
|
|
||||||
if [[ ! -f "$candidate/apm.yml" && ! -d "$candidate/.apm" && ! -f "$candidate/.claude-plugin/plugin.json" ]]; then
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
PLUGIN_DIRS+=("$candidate")
|
|
||||||
done
|
|
||||||
|
|
||||||
# An absent marketplace.json used to exit 0 unconditionally -- the same empty-set-reads-
|
|
||||||
# as-pass shape this script's other passes were fixed for. Per ADR-0015 the manifest is
|
|
||||||
# compiled output of root apm.yml's marketplace.packages[], so its absence alongside
|
|
||||||
# on-disk packages is drift, not an opt-out: it leaves every marketplace-derived gate
|
|
||||||
# (this one and sync-plugin-content.sh --all) walking an empty plugin set in silence.
|
|
||||||
if [[ ! -f "$MARKETPLACE" ]]; then
|
|
||||||
if [[ ${#PLUGIN_DIRS[@]} -eq 0 ]]; then
|
|
||||||
exit 0
|
|
||||||
fi
|
|
||||||
listing=""
|
|
||||||
# Guarded expansion even though the check above makes an empty array unreachable
|
|
||||||
# here: bash 3.2 under `set -u` aborts on a bare expansion of an empty array, and
|
|
||||||
# tests/test-vale-wrap.sh's bash32_glob scan is line-based, so a guard two lines up
|
|
||||||
# cannot clear it. Same form as the disk -> marketplace loop below.
|
|
||||||
for candidate in ${PLUGIN_DIRS[@]+"${PLUGIN_DIRS[@]}"}; do
|
|
||||||
listing+="${listing:+, }${candidate#"$REPO_ROOT"/}"
|
|
||||||
done
|
|
||||||
err ".claude-plugin/marketplace.json does not exist, but plugins/ holds ${#PLUGIN_DIRS[@]} plugin directory/ies ($listing) — every marketplace-derived check (this one, and sync-plugin-content.sh --all) silently walks an empty plugin set without it. Recompile the manifests from root apm.yml with \`apm pack\`."
|
|
||||||
echo "Manifest check failed: $FAIL error(s)" >&2
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
|
|
||||||
# Preconditions the marketplace walk below cannot report on itself: it runs inside a
|
|
||||||
# process substitution, so an abort in there is swallowed (see the helper's comment).
|
|
||||||
assert_marketplace_manifest_usable "$MARKETPLACE"
|
|
||||||
|
|
||||||
# Validates one plugin.json pointer field against disk, for the non-apm fallback below.
|
|
||||||
#
|
|
||||||
# check_pointer_field <plugin_name> <plugin_dir> <field> <test_flag>
|
|
||||||
#
|
|
||||||
# test_flag is `test`'s: -d where only a directory is meaningful, -e otherwise.
|
|
||||||
#
|
|
||||||
# Per the vendored host docs (plugins/kyberforge/docs/research/docs/
|
|
||||||
# claude-code-plugins/configuration.md and .../github-copilot-plugins/configuration.md)
|
|
||||||
# these fields are legally `string | string[] | object`. Reading them with
|
|
||||||
# `jq -r ".$field // empty"` collapsed the array and object shapes to their
|
|
||||||
# pretty-printed JSON text, which then matched no path on disk -- a manifest that
|
|
||||||
# resolves fine reported as broken. Reading `.skills | length` was worse than wrong: on
|
|
||||||
# a (legal) string value it returned the character count, and the `.skills[$i]` that
|
|
||||||
# followed aborted the whole script mid-loop under `set -e` with no summary line, so
|
|
||||||
# every plugin later in the marketplace went unchecked.
|
|
||||||
#
|
|
||||||
# The bare `$(jq ...)` assignments below are safe only because the caller has already
|
|
||||||
# established that $manifest parses AND that its root is an object (see the
|
|
||||||
# precondition in the marketplace walk). Do not call this without that check: `set -e`
|
|
||||||
# turns any jq failure in here into the same silent mid-loop abort described above.
|
|
||||||
check_pointer_field() {
|
|
||||||
local name="$1" plugin_dir="$2" field="$3" test_flag="$4"
|
|
||||||
local manifest="$plugin_dir/.claude-plugin/plugin.json"
|
|
||||||
local field_type count i elem_type
|
|
||||||
|
|
||||||
field_type="$(jq -r ".${field} | type" "$manifest")"
|
|
||||||
case "$field_type" in
|
|
||||||
null) ;;
|
|
||||||
# An inline definition (a hooks or mcpServers object written straight into the
|
|
||||||
# manifest) declares no path, so there is nothing on disk to resolve.
|
|
||||||
object) ;;
|
|
||||||
string)
|
|
||||||
check_pointer_path "$name" "$plugin_dir" "$field" "$(jq -r ".${field}" "$manifest")" "$test_flag"
|
|
||||||
;;
|
|
||||||
array)
|
|
||||||
count="$(jq ".${field} | length" "$manifest")"
|
|
||||||
for ((i = 0; i < count; i++)); do
|
|
||||||
elem_type="$(jq -r ".${field}[$i] | type" "$manifest")"
|
|
||||||
if [[ "$elem_type" != "string" ]]; then
|
|
||||||
err "plugin '$name': ${field}[$i] must be a path string, got $elem_type"
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
check_pointer_path "$name" "$plugin_dir" "$field" "$(jq -r ".${field}[$i]" "$manifest")" "$test_flag"
|
|
||||||
done
|
|
||||||
;;
|
|
||||||
*)
|
|
||||||
err "plugin '$name': $field must be a path string, an array of path strings, or an inline object, got $field_type"
|
|
||||||
;;
|
|
||||||
esac
|
|
||||||
}
|
|
||||||
|
|
||||||
check_pointer_path() {
|
|
||||||
local name="$1" plugin_dir="$2" field="$3" ref="$4" test_flag="$5"
|
|
||||||
local full_path="$plugin_dir/$ref"
|
|
||||||
full_path="${full_path%/}"
|
|
||||||
if ! test "$test_flag" "$full_path"; then
|
|
||||||
err "plugin '$name': $field path not found: $ref"
|
|
||||||
fi
|
|
||||||
}
|
|
||||||
|
|
||||||
# Every local plugin directory marketplace.json claimed, canonicalized, so the
|
|
||||||
# disk -> marketplace pass below can tell "listed" from "unlisted" regardless of how
|
|
||||||
# the `source:` string was spelled (./plugins/x, plugins/x, plugins/x/).
|
|
||||||
SEEN_PLUGIN_DIRS=()
|
|
||||||
|
|
||||||
while IFS=$'\t' read -r name plugin_dir; do
|
|
||||||
source_rel="${plugin_dir#"$REPO_ROOT"/}"
|
|
||||||
|
|
||||||
if [[ ! -d "$plugin_dir" ]]; then
|
|
||||||
err "plugin '$name': source directory not found: $source_rel"
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
# -P so a plugin directory reached through a symlink compares equal to the same
|
|
||||||
# directory reached directly; the disk-side walk below resolves the same way.
|
|
||||||
SEEN_PLUGIN_DIRS+=("$(cd "$plugin_dir" && pwd -P)")
|
|
||||||
|
|
||||||
manifest="$plugin_dir/.claude-plugin/plugin.json"
|
|
||||||
if [[ ! -f "$manifest" ]]; then
|
|
||||||
err "plugin '$name': .claude-plugin/plugin.json not found in $source_rel"
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
|
|
||||||
# apm-native plugin: pointer-field validation is sync-plugin-content.sh --check's
|
|
||||||
# job (see header comment above).
|
|
||||||
[[ -d "$plugin_dir/.apm" ]] && continue
|
|
||||||
|
|
||||||
# Precondition for check_pointer_field, which reads the manifest with bare
|
|
||||||
# `field_type="$(jq ... )"` assignments. Under `set -e` a jq failure in one of
|
|
||||||
# those aborts the whole script mid-loop: rc=5, a raw `jq: parse error` and no
|
|
||||||
# `Manifest check failed:` summary, with every later plugin left unchecked --
|
|
||||||
# the same failure class the marketplace's own `jq empty` precondition closes,
|
|
||||||
# for a file that is equally generated output. Both shapes have to be caught
|
|
||||||
# here: `jq empty` passes on a valid non-object document like `[]` or `123`, and
|
|
||||||
# it is the `.skills` lookup on such a root ("Cannot index array with string")
|
|
||||||
# that aborts, not the parse.
|
|
||||||
if ! jq empty "$manifest" >/dev/null 2>&1; then
|
|
||||||
err "plugin '$name': .claude-plugin/plugin.json is not valid JSON — it is compiled output, so recompile it with \`apm pack\`."
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
manifest_type="$(jq -r 'type' "$manifest")"
|
|
||||||
if [[ "$manifest_type" != "object" ]]; then
|
|
||||||
err "plugin '$name': .claude-plugin/plugin.json is a JSON $manifest_type at its top level; expected an object."
|
|
||||||
continue
|
|
||||||
fi
|
|
||||||
|
|
||||||
# Fallback for a non-apm plugin: validate that any skills/hooks/mcpServers/agents
|
|
||||||
# pointer fields in its hand-authored plugin.json still resolve to real paths.
|
|
||||||
# skills/agents point at directories; hooks/mcpServers may point at a file.
|
|
||||||
check_pointer_field "$name" "$plugin_dir" skills -d
|
|
||||||
check_pointer_field "$name" "$plugin_dir" agents -e
|
|
||||||
check_pointer_field "$name" "$plugin_dir" hooks -e
|
|
||||||
check_pointer_field "$name" "$plugin_dir" mcpServers -e
|
|
||||||
done < <(list_marketplace_local_plugins "$REPO_ROOT" "$MARKETPLACE")
|
|
||||||
|
|
||||||
# Disk -> marketplace, over the PLUGIN_DIRS candidate set collected above.
|
|
||||||
#
|
|
||||||
# A candidate counts as listed if it is either a directory some local entry pointed at
|
|
||||||
# (path match, canonicalized above) or a directory whose name matches a REMOTE entry's
|
|
||||||
# name. The name axis exists only for a plugin vendored on disk but declared with the
|
|
||||||
# remote-object `source:` shape: list_marketplace_local_plugins deliberately skips those,
|
|
||||||
# so a path-only match would report a missing entry that is in fact already there.
|
|
||||||
#
|
|
||||||
# It is restricted to non-string sources on purpose. Applied to local entries too, the
|
|
||||||
# name axis silently rescues genuine orphans, because a local entry's name need not equal
|
|
||||||
# the basename of the directory it points at: an entry named "beta" pointing at
|
|
||||||
# ./plugins/alpha would mark an unrelated, entirely unlisted plugins/beta/ as listed.
|
|
||||||
# Local entries already have an exact path to match on, so they need no name fallback.
|
|
||||||
#
|
|
||||||
# The select is an allowlist of the object shape, not a denylist of the string one --
|
|
||||||
# see list_marketplace_remote_plugin_names in scripts/lib/marketplace-plugins.sh, which
|
|
||||||
# owns it, and tests/test-check-manifests.sh, which exercises it directly against
|
|
||||||
# malformed entries rather than through this caller (where
|
|
||||||
# assert_marketplace_manifest_usable rejects them first, and so would mask a regression
|
|
||||||
# in the select itself).
|
|
||||||
MARKETPLACE_NAMES=()
|
|
||||||
while IFS= read -r entry_name; do
|
|
||||||
[[ -n "$entry_name" ]] && MARKETPLACE_NAMES+=("$entry_name")
|
|
||||||
done < <(list_marketplace_remote_plugin_names "$MARKETPLACE")
|
|
||||||
|
|
||||||
for candidate in ${PLUGIN_DIRS[@]+"${PLUGIN_DIRS[@]}"}; do
|
|
||||||
candidate_abs="$(cd "$candidate" && pwd -P)"
|
|
||||||
candidate_name="$(basename "$candidate")"
|
|
||||||
listed=0
|
|
||||||
for seen in ${SEEN_PLUGIN_DIRS[@]+"${SEEN_PLUGIN_DIRS[@]}"}; do
|
|
||||||
if [[ "$seen" == "$candidate_abs" ]]; then
|
|
||||||
listed=1
|
|
||||||
break
|
|
||||||
fi
|
|
||||||
done
|
|
||||||
if [[ $listed -eq 0 ]]; then
|
|
||||||
for entry_name in ${MARKETPLACE_NAMES[@]+"${MARKETPLACE_NAMES[@]}"}; do
|
|
||||||
if [[ "$entry_name" == "$candidate_name" ]]; then
|
|
||||||
listed=1
|
|
||||||
break
|
|
||||||
fi
|
|
||||||
done
|
|
||||||
fi
|
|
||||||
|
|
||||||
if [[ $listed -eq 0 ]]; then
|
|
||||||
err "plugin directory '${candidate#"$REPO_ROOT"/}' has no entry in .claude-plugin/marketplace.json — it is skipped by every marketplace-derived check (this one, and sync-plugin-content.sh --all) while still being globbed by the validate-plugins hook. Add it to root apm.yml's marketplace.packages[] and recompile the manifests."
|
|
||||||
fi
|
|
||||||
done
|
|
||||||
|
|
||||||
if [[ $FAIL -gt 0 ]]; then
|
|
||||||
echo "Manifest check failed: $FAIL error(s)" >&2
|
|
||||||
exit 1
|
|
||||||
fi
|
|
||||||
@@ -1,771 +0,0 @@
|
|||||||
#!/usr/bin/env bash
|
|
||||||
set -euo pipefail
|
|
||||||
|
|
||||||
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
|
||||||
SCRIPT="$REPO_ROOT/scripts/check-manifests.sh"
|
|
||||||
PASS=0
|
|
||||||
FAIL=0
|
|
||||||
|
|
||||||
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
|
|
||||||
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
|
|
||||||
|
|
||||||
# Several distinct faults all end in exit 1, and the bugs fixed below were precisely
|
|
||||||
# about the WRONG one being reported (a corrupt manifest blamed on six unlisted plugin
|
|
||||||
# directories, a legal manifest blamed for unresolvable paths). Exit-code-only
|
|
||||||
# assertions cannot see that, so these cases assert on the message text.
|
|
||||||
RUN_OUT=""
|
|
||||||
RUN_RC=0
|
|
||||||
run_script() { RUN_OUT="$(bash "$SCRIPT" "$1" 2>&1)" && RUN_RC=0 || RUN_RC=$?; }
|
|
||||||
|
|
||||||
# assert_fails_with <fixture> <label> <expected substring>...
|
|
||||||
assert_fails_with() {
|
|
||||||
local fixture="$1" label="$2"
|
|
||||||
shift 2
|
|
||||||
run_script "$fixture"
|
|
||||||
if [[ $RUN_RC -eq 0 ]]; then
|
|
||||||
fail "$label -- expected exit 1, got 0. Output: $RUN_OUT"
|
|
||||||
return
|
|
||||||
fi
|
|
||||||
local needle
|
|
||||||
for needle in "$@"; do
|
|
||||||
if [[ "$RUN_OUT" != *"$needle"* ]]; then
|
|
||||||
fail "$label -- exited $RUN_RC but message lacked '$needle'. Output: $RUN_OUT"
|
|
||||||
return
|
|
||||||
fi
|
|
||||||
done
|
|
||||||
pass "$label"
|
|
||||||
}
|
|
||||||
|
|
||||||
# assert_passes <fixture> <label>
|
|
||||||
assert_passes() {
|
|
||||||
run_script "$1"
|
|
||||||
if [[ $RUN_RC -eq 0 ]]; then
|
|
||||||
pass "$2"
|
|
||||||
else
|
|
||||||
fail "$2 -- expected exit 0, got $RUN_RC. Output: $RUN_OUT"
|
|
||||||
fi
|
|
||||||
}
|
|
||||||
|
|
||||||
# Writes a marketplace.json listing every "<name>=<source>" pair given.
|
|
||||||
write_marketplace() {
|
|
||||||
local dir="$1" entries="" pair name src
|
|
||||||
shift
|
|
||||||
for pair in "$@"; do
|
|
||||||
name="${pair%%=*}"
|
|
||||||
src="${pair#*=}"
|
|
||||||
entries+="${entries:+,}"$'\n'" { \"name\": \"$name\", \"source\": \"$src\" }"
|
|
||||||
done
|
|
||||||
mkdir -p "$dir/.claude-plugin"
|
|
||||||
printf '{\n "name": "test-marketplace",\n "plugins": [%s\n ]\n}\n' "$entries" > "$dir/.claude-plugin/marketplace.json"
|
|
||||||
}
|
|
||||||
|
|
||||||
# One trap over a registry rather than a fresh `trap 'rm -rf "$FIXTUREn"' EXIT`
|
|
||||||
# per fixture: each such trap REPLACES the previous one, so only the last
|
|
||||||
# fixture was ever cleaned and the rest leaked into TMPDIR every run. Same
|
|
||||||
# pattern as tests/test-check-vale-style-sync.sh and
|
|
||||||
# tests/test-check-scope-walkup-sync.sh; the emptiness guard is there because
|
|
||||||
# bash 3.2 treats "${arr[@]}" on an empty array as unbound under `set -u`.
|
|
||||||
FIXTURES=()
|
|
||||||
cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; }
|
|
||||||
trap cleanup EXIT
|
|
||||||
|
|
||||||
# Helper: make a minimal valid repo fixture with marketplace + plugin structure.
|
|
||||||
# Per ADR-0015/ADR-0017, the manifest check-manifests.sh validates is
|
|
||||||
# .claude-plugin/plugin.json (compiled output) -- not the root-level plugin.json,
|
|
||||||
# which was deleted repo-wide, and not the skills/hooks/mcpServers/agents pointer
|
|
||||||
# fields apm's compiler deliberately never populates (see scripts/check-manifests.sh's
|
|
||||||
# own header comment). Content-presence drift is scripts/sync-plugin-content.sh's job.
|
|
||||||
make_valid_fixture() {
|
|
||||||
local dir
|
|
||||||
dir="$(mktemp -d)"
|
|
||||||
mkdir -p "$dir/.claude-plugin"
|
|
||||||
mkdir -p "$dir/plugins/myplugin/.claude-plugin"
|
|
||||||
cat > "$dir/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "myplugin", "source": "./plugins/myplugin" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
cat > "$dir/plugins/myplugin/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "myplugin"
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
echo "$dir"
|
|
||||||
}
|
|
||||||
|
|
||||||
# --- 1. Exits 0 against valid repo structure ---
|
|
||||||
echo ""
|
|
||||||
echo "--- exits 0 when all references are valid ---"
|
|
||||||
FIXTURE="$(make_valid_fixture)"
|
|
||||||
FIXTURES+=("$FIXTURE")
|
|
||||||
if bash "$SCRIPT" "$FIXTURE" > /dev/null 2>&1; then
|
|
||||||
pass "exits 0 when all manifest references resolve"
|
|
||||||
else
|
|
||||||
fail "exited non-zero against a valid fixture"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 2. Exits 1 when plugin source dir is missing ---
|
|
||||||
echo ""
|
|
||||||
echo "--- exits 1 when plugin source directory missing ---"
|
|
||||||
FIXTURE2="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE2")
|
|
||||||
mkdir -p "$FIXTURE2/.claude-plugin"
|
|
||||||
cat > "$FIXTURE2/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "ghost", "source": "./plugins/ghost" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
if bash "$SCRIPT" "$FIXTURE2" > /dev/null 2>&1; then
|
|
||||||
fail "exited 0 when plugin source dir is missing — expected exit 1"
|
|
||||||
else
|
|
||||||
pass "exits non-zero when plugin source directory does not exist"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 3. Exits 1 when .claude-plugin/plugin.json is missing from plugin dir ---
|
|
||||||
echo ""
|
|
||||||
echo "--- exits 1 when .claude-plugin/plugin.json missing from plugin directory ---"
|
|
||||||
FIXTURE3="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE3")
|
|
||||||
mkdir -p "$FIXTURE3/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE3/plugins/nomanifest"
|
|
||||||
cat > "$FIXTURE3/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "nomanifest", "source": "./plugins/nomanifest" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
if bash "$SCRIPT" "$FIXTURE3" > /dev/null 2>&1; then
|
|
||||||
fail "exited 0 when .claude-plugin/plugin.json is missing — expected exit 1"
|
|
||||||
else
|
|
||||||
pass "exits non-zero when .claude-plugin/plugin.json is missing from plugin directory"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 4. Exits 1 when a remote-source plugin entry's local plugin still lacks a manifest ---
|
|
||||||
# Remote sources (object-typed `source:`) are skipped entirely; only string (local path)
|
|
||||||
# sources are checked. This guards that a mixed marketplace.json still catches a broken
|
|
||||||
# local entry alongside a legitimately-skipped remote one.
|
|
||||||
echo ""
|
|
||||||
echo "--- exits 1 for a broken local entry even when a remote entry is present ---"
|
|
||||||
FIXTURE4="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE4")
|
|
||||||
mkdir -p "$FIXTURE4/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE4/plugins/broken"
|
|
||||||
cat > "$FIXTURE4/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "remote-thing", "source": { "repo": "someorg/somerepo", "source": "github" } },
|
|
||||||
{ "name": "broken", "source": "./plugins/broken" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
if bash "$SCRIPT" "$FIXTURE4" > /dev/null 2>&1; then
|
|
||||||
fail "exited 0 with a broken local entry present — expected exit 1"
|
|
||||||
else
|
|
||||||
pass "exits non-zero for a broken local entry even alongside a skipped remote entry"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 5. Non-.apm/ plugin with a broken pointer field is caught by the fallback path ---
|
|
||||||
# apm-native plugins (.apm/ present) get their skills/hooks/mcpServers/agents
|
|
||||||
# pointer-field validation from sync-plugin-content.sh --check instead (see this
|
|
||||||
# script's header comment) -- but that script skips any plugin dir lacking .apm/
|
|
||||||
# outright, so a non-apm plugin's hand-authored plugin.json needs this script's own
|
|
||||||
# fallback validation to catch a broken pointer field.
|
|
||||||
echo ""
|
|
||||||
echo "--- catches a broken pointer field in a non-apm plugin's plugin.json ---"
|
|
||||||
FIXTURE5="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE5")
|
|
||||||
mkdir -p "$FIXTURE5/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE5/plugins/legacy/.claude-plugin"
|
|
||||||
cat > "$FIXTURE5/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "legacy", "source": "./plugins/legacy" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
cat > "$FIXTURE5/plugins/legacy/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "legacy",
|
|
||||||
"skills": ["./skills/does-not-exist"]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
if bash "$SCRIPT" "$FIXTURE5" > /dev/null 2>&1; then
|
|
||||||
fail "exited 0 for a non-apm plugin with a broken skills pointer -- expected exit 1"
|
|
||||||
else
|
|
||||||
pass "catches a broken skills pointer field in a non-apm (no .apm/) plugin.json"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 6. Non-.apm/ plugin with valid pointer fields still passes (no false positive) ---
|
|
||||||
echo ""
|
|
||||||
echo "--- a non-apm plugin with valid pointer fields still passes ---"
|
|
||||||
FIXTURE6="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE6")
|
|
||||||
mkdir -p "$FIXTURE6/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE6/plugins/legacy-ok/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE6/plugins/legacy-ok/skills/real-skill"
|
|
||||||
cat > "$FIXTURE6/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "legacy-ok", "source": "./plugins/legacy-ok" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
cat > "$FIXTURE6/plugins/legacy-ok/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "legacy-ok",
|
|
||||||
"skills": ["./skills/real-skill"]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
if bash "$SCRIPT" "$FIXTURE6" > /dev/null 2>&1; then
|
|
||||||
pass "a non-apm plugin with a resolving skills pointer passes"
|
|
||||||
else
|
|
||||||
fail "exited non-zero for a non-apm plugin whose pointer fields all resolve"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 7. An .apm/ plugin with a broken pointer field is NOT caught here (delegated) ---
|
|
||||||
# Guards against the fallback path in finding #6 accidentally widening to also
|
|
||||||
# validate apm-native plugins, which would duplicate (and could disagree with)
|
|
||||||
# sync-plugin-content.sh --check's own drift detection.
|
|
||||||
echo ""
|
|
||||||
echo "--- an apm-native plugin's pointer fields are left to sync-plugin-content.sh --check ---"
|
|
||||||
FIXTURE7="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE7")
|
|
||||||
mkdir -p "$FIXTURE7/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE7/plugins/apm-plugin/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE7/plugins/apm-plugin/.apm"
|
|
||||||
cat > "$FIXTURE7/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "apm-plugin", "source": "./plugins/apm-plugin" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
cat > "$FIXTURE7/plugins/apm-plugin/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "apm-plugin",
|
|
||||||
"skills": ["./skills/does-not-exist"]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
if bash "$SCRIPT" "$FIXTURE7" > /dev/null 2>&1; then
|
|
||||||
pass "an apm-native plugin (has .apm/) is not checked here, even with a broken pointer field"
|
|
||||||
else
|
|
||||||
fail "check-manifests.sh failed on an apm-native plugin -- pointer-field validation should be delegated, not duplicated"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 8. Disk -> marketplace: an apm package dir with no marketplace entry is caught ---
|
|
||||||
# Both this script and sync-plugin-content.sh --all derive their plugin set from
|
|
||||||
# marketplace.json, so before this check an unlisted plugins/<name>/ was skipped by
|
|
||||||
# every marketplace-derived gate at once while still being globbed by the
|
|
||||||
# validate-plugins pre-commit hook -- two different notions of "the plugin set".
|
|
||||||
# Per ADR-0015 marketplace.json is compiled from root apm.yml's marketplace.packages[],
|
|
||||||
# so an on-disk apm package missing from it is compiled-output drift.
|
|
||||||
echo ""
|
|
||||||
echo "--- exits 1 for a plugins/<name>/ apm package with no marketplace entry ---"
|
|
||||||
FIXTURE8="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE8")
|
|
||||||
mkdir -p "$FIXTURE8/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE8/plugins/listed/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE8/plugins/orphan/.claude-plugin" "$FIXTURE8/plugins/orphan/.apm/skills"
|
|
||||||
cat > "$FIXTURE8/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "listed", "source": "./plugins/listed" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
echo '{ "name": "listed" }' > "$FIXTURE8/plugins/listed/.claude-plugin/plugin.json"
|
|
||||||
echo '{ "name": "orphan" }' > "$FIXTURE8/plugins/orphan/.claude-plugin/plugin.json"
|
|
||||||
printf 'name: orphan\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE8/plugins/orphan/apm.yml"
|
|
||||||
if bash "$SCRIPT" "$FIXTURE8" > /dev/null 2>&1; then
|
|
||||||
fail "exited 0 for an on-disk apm package absent from marketplace.json -- expected exit 1"
|
|
||||||
else
|
|
||||||
pass "catches a plugins/<name>/ apm package that produced no marketplace entry"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 9. A plugins/<name>/ dir with none of the three plugin markers is not flagged ---
|
|
||||||
# The trigger is apm.yml || .apm/ || .claude-plugin/plugin.json -- broad enough to match
|
|
||||||
# the plugins/*/ set the validate-plugins hook globs, which is the disagreement this check
|
|
||||||
# closes. A directory carrying none of the three is scratch and stays out of scope.
|
|
||||||
echo ""
|
|
||||||
echo "--- a plugins/<name>/ directory with none of the three plugin markers is not flagged ---"
|
|
||||||
FIXTURE9="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE9")
|
|
||||||
mkdir -p "$FIXTURE9/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE9/plugins/listed/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE9/plugins/scratch/notes"
|
|
||||||
cat > "$FIXTURE9/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "listed", "source": "./plugins/listed" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
echo '{ "name": "listed" }' > "$FIXTURE9/plugins/listed/.claude-plugin/plugin.json"
|
|
||||||
if bash "$SCRIPT" "$FIXTURE9" > /dev/null 2>&1; then
|
|
||||||
pass "a plugins/<name>/ directory with no plugin markers is left alone"
|
|
||||||
else
|
|
||||||
fail "flagged a non-package directory under plugins/ -- expected exit 0"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 9b. Each of the three markers on its own is enough to trigger the check ---
|
|
||||||
# Keying only off apm.yml would leave a plugin dir carrying just .apm/ or just a
|
|
||||||
# compiled .claude-plugin/plugin.json invisible -- exactly the class of gap this
|
|
||||||
# check exists to close, since validate-plugins would still glob it.
|
|
||||||
marker_case() {
|
|
||||||
local label="$1" marker_setup="$2" dir
|
|
||||||
dir="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$dir")
|
|
||||||
mkdir -p "$dir/.claude-plugin" "$dir/plugins/listed/.claude-plugin" "$dir/plugins/orphan"
|
|
||||||
cat > "$dir/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "listed", "source": "./plugins/listed" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
echo '{ "name": "listed" }' > "$dir/plugins/listed/.claude-plugin/plugin.json"
|
|
||||||
case "$marker_setup" in
|
|
||||||
apm-dir) mkdir -p "$dir/plugins/orphan/.apm/skills" ;;
|
|
||||||
plugin-json)
|
|
||||||
mkdir -p "$dir/plugins/orphan/.claude-plugin"
|
|
||||||
echo '{ "name": "orphan" }' > "$dir/plugins/orphan/.claude-plugin/plugin.json"
|
|
||||||
;;
|
|
||||||
esac
|
|
||||||
if bash "$SCRIPT" "$dir" > /dev/null 2>&1; then
|
|
||||||
fail "an unlisted plugin dir carrying only $label was not flagged"
|
|
||||||
else
|
|
||||||
pass "an unlisted plugin dir carrying only $label is flagged"
|
|
||||||
fi
|
|
||||||
}
|
|
||||||
echo ""
|
|
||||||
echo "--- .apm/ alone and .claude-plugin/plugin.json alone each trigger the check ---"
|
|
||||||
marker_case ".apm/" apm-dir
|
|
||||||
marker_case ".claude-plugin/plugin.json" plugin-json
|
|
||||||
|
|
||||||
# --- 9c. A vendored plugin declared with a remote-object source: is already listed ---
|
|
||||||
# list_marketplace_local_plugins deliberately skips remote-object entries, so a
|
|
||||||
# path-only listed/unlisted match reported a missing entry for a directory whose
|
|
||||||
# entry is in fact right there -- telling the author to add what already exists.
|
|
||||||
# The name axis of the match closes that.
|
|
||||||
echo ""
|
|
||||||
echo "--- a vendored plugin whose marketplace entry uses a remote source: is not flagged ---"
|
|
||||||
FIXTURE9C="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE9C")
|
|
||||||
mkdir -p "$FIXTURE9C/.claude-plugin" "$FIXTURE9C/plugins/vendored/.claude-plugin"
|
|
||||||
cat > "$FIXTURE9C/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "vendored", "source": { "repo": "someorg/somerepo", "source": "github" } }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
echo '{ "name": "vendored" }' > "$FIXTURE9C/plugins/vendored/.claude-plugin/plugin.json"
|
|
||||||
printf 'name: vendored\nversion: 1.2.3\ntype: skill\n' > "$FIXTURE9C/plugins/vendored/apm.yml"
|
|
||||||
if bash "$SCRIPT" "$FIXTURE9C" > /dev/null 2>&1; then
|
|
||||||
pass "a vendored dir matching a remote-source entry's name counts as listed"
|
|
||||||
else
|
|
||||||
fail "flagged a vendored plugin that already has a remote-source marketplace entry"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 9d. The name axis must NOT rescue an orphan via a LOCAL entry's name ---
|
|
||||||
# A local entry's name need not equal the basename of the directory it points at. An
|
|
||||||
# entry named "beta" pointing at ./plugins/alpha must not mark an unrelated, entirely
|
|
||||||
# unlisted plugins/beta/ as listed -- local entries match on their exact path, so
|
|
||||||
# extending the name fallback to them just reopens the gap this check exists to close.
|
|
||||||
echo ""
|
|
||||||
echo "--- a local entry's name does not rescue a same-named but unlisted directory ---"
|
|
||||||
FIXTURE9D="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE9D")
|
|
||||||
mkdir -p "$FIXTURE9D/.claude-plugin" "$FIXTURE9D/plugins/alpha/.claude-plugin" "$FIXTURE9D/plugins/beta"
|
|
||||||
cat > "$FIXTURE9D/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "beta", "source": "./plugins/alpha" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
echo '{ "name": "alpha" }' > "$FIXTURE9D/plugins/alpha/.claude-plugin/plugin.json"
|
|
||||||
printf 'name: beta\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE9D/plugins/beta/apm.yml"
|
|
||||||
if bash "$SCRIPT" "$FIXTURE9D" > /dev/null 2>&1; then
|
|
||||||
fail "an unlisted plugins/beta/ was rescued by an unrelated local entry named beta -- expected exit 1"
|
|
||||||
else
|
|
||||||
pass "an unlisted directory is not rescued by a local entry that merely shares its name"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 10. Marketplace `source:` spelling variants still count as "listed" ---
|
|
||||||
# The disk -> marketplace comparison canonicalizes both sides, so `plugins/x` and
|
|
||||||
# `./plugins/x/` must resolve to the same directory as the glob's `plugins/x/`.
|
|
||||||
#
|
|
||||||
# The entry names deliberately DIFFER from the directory basenames. With names equal to
|
|
||||||
# basenames this fixture proved nothing whenever the name axis was permissive: deleting
|
|
||||||
# the canonicalization entirely still left it passing, because the name match rescued it.
|
|
||||||
# Restricting the name axis to remote entries fixed that, but making the names differ is
|
|
||||||
# what keeps this assertion honest independently of that restriction.
|
|
||||||
echo ""
|
|
||||||
echo "--- a marketplace source without ./ or with a trailing slash still counts as listed ---"
|
|
||||||
FIXTURE10="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE10")
|
|
||||||
mkdir -p "$FIXTURE10/.claude-plugin"
|
|
||||||
mkdir -p "$FIXTURE10/plugins/bare/.claude-plugin" "$FIXTURE10/plugins/trailing/.claude-plugin"
|
|
||||||
cat > "$FIXTURE10/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "bare-entry", "source": "plugins/bare" },
|
|
||||||
{ "name": "trailing-entry", "source": "./plugins/trailing/" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
echo '{ "name": "bare" }' > "$FIXTURE10/plugins/bare/.claude-plugin/plugin.json"
|
|
||||||
echo '{ "name": "trailing" }' > "$FIXTURE10/plugins/trailing/.claude-plugin/plugin.json"
|
|
||||||
printf 'name: bare\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE10/plugins/bare/apm.yml"
|
|
||||||
printf 'name: trailing\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE10/plugins/trailing/apm.yml"
|
|
||||||
if bash "$SCRIPT" "$FIXTURE10" > /dev/null 2>&1; then
|
|
||||||
pass "source: spelling variants are canonicalized before the listed/unlisted comparison"
|
|
||||||
else
|
|
||||||
fail "flagged a listed plugin because its source: string was spelled differently"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 11. A marketplace entry with no `source` at all is rejected outright ---
|
|
||||||
# It used to disable BOTH directions of the check for that plugin at once:
|
|
||||||
# list_marketplace_local_plugins requires a string `source`, so the entry was skipped and
|
|
||||||
# its .claude-plugin/plugin.json never checked; and the disk -> marketplace name axis
|
|
||||||
# selected on `(.source | type) != "string"`, which is TRUE for null, so the same entry
|
|
||||||
# also marked its on-disk directory "listed". Net effect: a plugin with a broken manifest
|
|
||||||
# and a malformed entry passed clean, and silently dropped out of
|
|
||||||
# sync-plugin-content.sh --all's work list too, since that derives from the same helper.
|
|
||||||
echo ""
|
|
||||||
echo "--- a marketplace entry with no source: field is a hard error ---"
|
|
||||||
FIXTURE11="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE11")
|
|
||||||
mkdir -p "$FIXTURE11/.claude-plugin" "$FIXTURE11/plugins/lint"
|
|
||||||
cat > "$FIXTURE11/.claude-plugin/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "lint" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
printf 'name: lint\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE11/plugins/lint/apm.yml"
|
|
||||||
assert_fails_with "$FIXTURE11" \
|
|
||||||
"an entry with no source: is reported by name instead of silently disabling both checks" \
|
|
||||||
'`source` is neither a local path string nor a remote source object' 'lint (source: null)'
|
|
||||||
|
|
||||||
# --- 11b. Any other unclassifiable `source` is rejected the same way ---
|
|
||||||
# The guard used to test `.source == null` specifically, so every OTHER malformed value
|
|
||||||
# reached exactly the state the null case was fixed for: `"source": 42` passed the
|
|
||||||
# assert, was skipped by list_marketplace_local_plugins for not being a string, AND was
|
|
||||||
# rescued by the disk -> marketplace name axis (whose select was the denylist
|
|
||||||
# `(.source|type) != "string"`, true for a number). Verbatim the same defect, one value
|
|
||||||
# over. Only two shapes are classifiable -- a local path string and a remote source
|
|
||||||
# object -- so the guard is typed as "neither of those", not as a list of known-bad
|
|
||||||
# values.
|
|
||||||
echo ""
|
|
||||||
echo "--- a non-string, non-object source: is rejected by type, not by enumerating null ---"
|
|
||||||
for BAD_SOURCE in '42' '[]' 'true'; do
|
|
||||||
FIXTURE11B="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE11B")
|
|
||||||
mkdir -p "$FIXTURE11B/.claude-plugin" "$FIXTURE11B/plugins/lint"
|
|
||||||
printf '{ "name": "test-marketplace", "plugins": [ { "name": "lint", "source": %s } ] }\n' \
|
|
||||||
"$BAD_SOURCE" > "$FIXTURE11B/.claude-plugin/marketplace.json"
|
|
||||||
printf 'name: lint\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE11B/plugins/lint/apm.yml"
|
|
||||||
assert_fails_with "$FIXTURE11B" \
|
|
||||||
"a source: of $BAD_SOURCE is rejected instead of silently disabling both checks" \
|
|
||||||
'`source` is neither a local path string nor a remote source object' 'lint (source:'
|
|
||||||
done
|
|
||||||
|
|
||||||
# --- 11c. The disk -> marketplace name axis, exercised WITHOUT the precondition ---
|
|
||||||
# This is the one assertion that cannot go through bash "$SCRIPT": every malformed entry
|
|
||||||
# the select must reject is rejected first by assert_marketplace_manifest_usable, which
|
|
||||||
# exits before the select ever runs. So reverting the select alone left the whole suite
|
|
||||||
# green -- the code carried a comment claiming it "must not depend on that check running
|
|
||||||
# first", and nothing tested that independence. Call the function directly instead.
|
|
||||||
#
|
|
||||||
# The invariant: the name axis exists solely for a plugin vendored on disk under a
|
|
||||||
# REMOTE (object) `source:`, which has no local path to match on. Every other shape --
|
|
||||||
# a local string (which matches by path and needs no name fallback, see case 9d) and
|
|
||||||
# every unclassifiable value -- must produce no name at all.
|
|
||||||
echo ""
|
|
||||||
echo "--- list_marketplace_remote_plugin_names emits object-source names only ---"
|
|
||||||
# shellcheck source=scripts/lib/marketplace-plugins.sh
|
|
||||||
source "$REPO_ROOT/scripts/lib/marketplace-plugins.sh"
|
|
||||||
FIXTURE11C="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE11C")
|
|
||||||
cat > "$FIXTURE11C/marketplace.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "test-marketplace",
|
|
||||||
"plugins": [
|
|
||||||
{ "name": "remote-obj", "source": { "repo": "someorg/somerepo", "source": "github" } },
|
|
||||||
{ "name": "local-str", "source": "./plugins/local-str" },
|
|
||||||
{ "name": "null-src" },
|
|
||||||
{ "name": "explicit-null", "source": null },
|
|
||||||
{ "name": "number-src", "source": 42 },
|
|
||||||
{ "name": "array-src", "source": [] },
|
|
||||||
{ "name": "bool-src", "source": true }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
NAMES11C="$(list_marketplace_remote_plugin_names "$FIXTURE11C/marketplace.json")"
|
|
||||||
if [[ "$NAMES11C" == "remote-obj" ]]; then
|
|
||||||
pass "only the remote object-source entry yields a name for the disk -> marketplace name axis"
|
|
||||||
else
|
|
||||||
fail "the name axis emitted $(printf '%s' "$NAMES11C" | tr '\n' ' ')— expected exactly 'remote-obj'; every other shape would rescue a same-named orphan directory"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 11d. A valid-JSON, non-object marketplace root is named, not left to crash jq ---
|
|
||||||
# `jq empty` passes on `[]`, `"x"` and `123`; the `.plugins` lookup on the next line then
|
|
||||||
# died with a raw `jq: error: Cannot index array with string "plugins"` and rc=5,
|
|
||||||
# attributed to nothing at all.
|
|
||||||
echo ""
|
|
||||||
echo "--- a valid-JSON non-object marketplace root is reported as such ---"
|
|
||||||
FIXTURE11D="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE11D")
|
|
||||||
mkdir -p "$FIXTURE11D/.claude-plugin" "$FIXTURE11D/plugins/one"
|
|
||||||
printf '[]\n' > "$FIXTURE11D/.claude-plugin/marketplace.json"
|
|
||||||
printf 'name: one\nversion: 0.1.0\ntype: skill\n' > "$FIXTURE11D/plugins/one/apm.yml"
|
|
||||||
assert_fails_with "$FIXTURE11D" \
|
|
||||||
"a JSON array at the marketplace root is named as a root-shape error" \
|
|
||||||
'is a JSON array at its top level'
|
|
||||||
|
|
||||||
# --- 12. A `skills` string (a legal shape per the host docs) is resolved, not counted ---
|
|
||||||
# `jq '.skills | if . then length else 0 end'` is null-safe but not type-safe: on the
|
|
||||||
# string "./skills/x" it returned the CHARACTER count, and the `.skills[0]` that followed
|
|
||||||
# errored ("Cannot index string with number"), killing the whole script under `set -e`
|
|
||||||
# with no "Manifest check failed:" line -- and every plugin later in the marketplace
|
|
||||||
# unchecked. Both configuration.md references document `skills` as string | string[].
|
|
||||||
echo ""
|
|
||||||
echo "--- a string-valued skills field resolves instead of crashing the script ---"
|
|
||||||
FIXTURE12="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE12")
|
|
||||||
mkdir -p "$FIXTURE12/plugins/strskills/.claude-plugin" "$FIXTURE12/plugins/strskills/custom/skills"
|
|
||||||
write_marketplace "$FIXTURE12" "strskills=./plugins/strskills"
|
|
||||||
cat > "$FIXTURE12/plugins/strskills/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "strskills",
|
|
||||||
"skills": "./custom/skills/"
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
assert_passes "$FIXTURE12" "a resolving string-valued skills field passes"
|
|
||||||
|
|
||||||
# --- 13. A broken string `skills` is reported, and later plugins are still checked ---
|
|
||||||
# The mid-loop `set -e` abort meant a fault in the FIRST plugin hid every fault after it.
|
|
||||||
# The second entry here is broken in an unrelated way; both messages must appear.
|
|
||||||
echo ""
|
|
||||||
echo "--- a broken string skills field is reported without aborting the marketplace walk ---"
|
|
||||||
FIXTURE13="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE13")
|
|
||||||
mkdir -p "$FIXTURE13/plugins/first/.claude-plugin" "$FIXTURE13/plugins/second"
|
|
||||||
write_marketplace "$FIXTURE13" "first=./plugins/first" "second=./plugins/second"
|
|
||||||
cat > "$FIXTURE13/plugins/first/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "first",
|
|
||||||
"skills": "./skills/does-not-exist"
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
assert_fails_with "$FIXTURE13" \
|
|
||||||
"a broken string skills field is reported and the walk continues to later plugins" \
|
|
||||||
'skills path not found: ./skills/does-not-exist' \
|
|
||||||
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
||||||
'Manifest check failed: 2 error(s)'
|
|
||||||
|
|
||||||
# --- 14. A genuinely wrong-typed `skills` is named as such, walk still continues ---
|
|
||||||
echo ""
|
|
||||||
echo "--- a wrong-typed skills field is reported as a type error, not a missing path ---"
|
|
||||||
FIXTURE14="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE14")
|
|
||||||
mkdir -p "$FIXTURE14/plugins/first/.claude-plugin" "$FIXTURE14/plugins/second"
|
|
||||||
write_marketplace "$FIXTURE14" "first=./plugins/first" "second=./plugins/second"
|
|
||||||
cat > "$FIXTURE14/plugins/first/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "first",
|
|
||||||
"skills": 42
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
assert_fails_with "$FIXTURE14" \
|
|
||||||
"a wrong-typed skills field names the type and does not abort the walk" \
|
|
||||||
'skills must be a path string, an array of path strings, or an inline object, got number' \
|
|
||||||
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
||||||
'Manifest check failed: 2 error(s)'
|
|
||||||
|
|
||||||
# --- 15. Array- and object-valued pointer fields that resolve are not reported missing ---
|
|
||||||
# `ref=$(jq -r ".$field // empty")` returned the PRETTY-PRINTED JSON for an array or an
|
|
||||||
# object, which `[[ ! -e ]]` then rejected: a manifest whose paths all resolve was
|
|
||||||
# reported broken. Both host docs give `agents` as string | string[] and `hooks` /
|
|
||||||
# `mcpServers` as string | object (an inline definition, with no path to resolve).
|
|
||||||
echo ""
|
|
||||||
echo "--- array- and inline-object pointer fields that resolve are accepted ---"
|
|
||||||
FIXTURE15="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE15")
|
|
||||||
mkdir -p "$FIXTURE15/plugins/shapes/.claude-plugin" "$FIXTURE15/plugins/shapes/agents" "$FIXTURE15/plugins/shapes/skills/one"
|
|
||||||
touch "$FIXTURE15/plugins/shapes/agents/real.md"
|
|
||||||
write_marketplace "$FIXTURE15" "shapes=./plugins/shapes"
|
|
||||||
cat > "$FIXTURE15/plugins/shapes/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "shapes",
|
|
||||||
"skills": ["./skills/one"],
|
|
||||||
"agents": ["./agents/real.md"],
|
|
||||||
"hooks": { "PreToolUse": [{ "hooks": [{ "type": "command", "command": "true" }] }] },
|
|
||||||
"mcpServers": { "demo": { "command": "true" } }
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
assert_passes "$FIXTURE15" \
|
|
||||||
"an array-valued agents and an inline-object hooks/mcpServers are not reported missing"
|
|
||||||
|
|
||||||
# --- 16. A broken element inside an array-valued pointer field is still caught ---
|
|
||||||
# Guards the fix in #15 against over-correcting into "arrays are always fine".
|
|
||||||
echo ""
|
|
||||||
echo "--- a broken path inside an array-valued pointer field is still caught ---"
|
|
||||||
FIXTURE16="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE16")
|
|
||||||
mkdir -p "$FIXTURE16/plugins/shapes/.claude-plugin" "$FIXTURE16/plugins/shapes/agents"
|
|
||||||
touch "$FIXTURE16/plugins/shapes/agents/real.md"
|
|
||||||
write_marketplace "$FIXTURE16" "shapes=./plugins/shapes"
|
|
||||||
cat > "$FIXTURE16/plugins/shapes/.claude-plugin/plugin.json" <<'JSON'
|
|
||||||
{
|
|
||||||
"name": "shapes",
|
|
||||||
"agents": ["./agents/real.md", "./agents/ghost.md"]
|
|
||||||
}
|
|
||||||
JSON
|
|
||||||
assert_fails_with "$FIXTURE16" \
|
|
||||||
"a missing path in an array-valued agents field is reported with its own path" \
|
|
||||||
'agents path not found: ./agents/ghost.md'
|
|
||||||
|
|
||||||
# --- 17. An unparseable marketplace.json is reported as such, not as unlisted plugins ---
|
|
||||||
# The walk runs in a process substitution, so the helper's `set -e` abort on invalid JSON
|
|
||||||
# never reached the caller. The run still exited 1 -- backstopped by the disk -> marketplace
|
|
||||||
# pass -- but printed one "has no entry in .claude-plugin/marketplace.json ... add it to
|
|
||||||
# root apm.yml" per plugin directory, sending the reader to edit apm.yml when the actual
|
|
||||||
# fault was a corrupt manifest.
|
|
||||||
echo ""
|
|
||||||
echo "--- an unparseable marketplace.json is attributed to the manifest, not to the plugins ---"
|
|
||||||
FIXTURE17="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE17")
|
|
||||||
mkdir -p "$FIXTURE17/.claude-plugin" "$FIXTURE17/plugins/one/.claude-plugin" "$FIXTURE17/plugins/two/.claude-plugin"
|
|
||||||
printf '{ "name": "test-marketplace", "plugins": [ { "name": "one",\n' > "$FIXTURE17/.claude-plugin/marketplace.json"
|
|
||||||
echo '{ "name": "one" }' > "$FIXTURE17/plugins/one/.claude-plugin/plugin.json"
|
|
||||||
echo '{ "name": "two" }' > "$FIXTURE17/plugins/two/.claude-plugin/plugin.json"
|
|
||||||
run_script "$FIXTURE17"
|
|
||||||
if [[ $RUN_RC -eq 0 ]]; then
|
|
||||||
fail "exited 0 on an unparseable marketplace.json -- expected exit 1"
|
|
||||||
elif [[ "$RUN_OUT" != *"is not valid JSON"* ]]; then
|
|
||||||
fail "an unparseable marketplace.json was not named as such. Output: $RUN_OUT"
|
|
||||||
elif [[ "$RUN_OUT" == *"has no entry in .claude-plugin/marketplace.json"* ]]; then
|
|
||||||
fail "an unparseable marketplace.json was misreported as unlisted plugin directories. Output: $RUN_OUT"
|
|
||||||
else
|
|
||||||
pass "an unparseable marketplace.json is reported as invalid JSON, not as unlisted plugin directories"
|
|
||||||
fi
|
|
||||||
|
|
||||||
# --- 18. A missing marketplace.json with plugins on disk is drift, not an opt-out ---
|
|
||||||
# `[[ ! -f "$MARKETPLACE" ]] && exit 0` was the same empty-set-reads-as-pass shape as the
|
|
||||||
# rest: per ADR-0015 the manifest is compiled from root apm.yml, so its absence next to
|
|
||||||
# on-disk packages means the compiled output is missing, and every marketplace-derived
|
|
||||||
# gate walks an empty plugin set in silence.
|
|
||||||
echo ""
|
|
||||||
echo "--- a missing marketplace.json alongside on-disk plugin directories fails ---"
|
|
||||||
FIXTURE18="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE18")
|
|
||||||
mkdir -p "$FIXTURE18/plugins/orphan/.apm/skills"
|
|
||||||
assert_fails_with "$FIXTURE18" \
|
|
||||||
"a missing marketplace.json with plugin directories present is reported as drift" \
|
|
||||||
'.claude-plugin/marketplace.json does not exist' \
|
|
||||||
'plugins/orphan'
|
|
||||||
|
|
||||||
# --- 18b. A missing marketplace.json with nothing to check still exits 0 ---
|
|
||||||
# Guards the fix above against over-correcting into "always fail without a manifest":
|
|
||||||
# a repo with no plugin directories genuinely has nothing for this gate to check.
|
|
||||||
echo ""
|
|
||||||
echo "--- a missing marketplace.json with no plugin directories still exits 0 ---"
|
|
||||||
FIXTURE18B="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE18B")
|
|
||||||
mkdir -p "$FIXTURE18B/plugins/scratch/notes" "$FIXTURE18B/docs"
|
|
||||||
assert_passes "$FIXTURE18B" \
|
|
||||||
"no marketplace.json and no plugin-marked directories is a genuine no-op, not a failure"
|
|
||||||
|
|
||||||
# --- 19. An unparseable per-plugin plugin.json is attributed, and the walk continues ---
|
|
||||||
# check_pointer_field reads the manifest with bare `$(jq ...)` assignments, so under
|
|
||||||
# `set -e` a parse failure aborted the whole script mid-loop: rc=5, a raw
|
|
||||||
# `jq: parse error` on stderr, no `Manifest check failed:` summary, and every plugin
|
|
||||||
# later in the marketplace silently unchecked. That is the same failure class the
|
|
||||||
# marketplace's own `jq empty` precondition closes -- and .claude-plugin/plugin.json is
|
|
||||||
# equally generated output, so it is equally capable of being corrupt.
|
|
||||||
#
|
|
||||||
# The second entry is broken in an unrelated way; both messages plus the summary must
|
|
||||||
# appear, which is what proves the walk survived the first fault.
|
|
||||||
echo ""
|
|
||||||
echo "--- an unparseable plugin.json is reported and does not abort the marketplace walk ---"
|
|
||||||
FIXTURE19="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE19")
|
|
||||||
mkdir -p "$FIXTURE19/plugins/corrupt/.claude-plugin" "$FIXTURE19/plugins/second"
|
|
||||||
write_marketplace "$FIXTURE19" "corrupt=./plugins/corrupt" "second=./plugins/second"
|
|
||||||
printf '{ "name": "corrupt",\n' > "$FIXTURE19/plugins/corrupt/.claude-plugin/plugin.json"
|
|
||||||
assert_fails_with "$FIXTURE19" \
|
|
||||||
"an unparseable plugin.json is named and later plugins are still checked" \
|
|
||||||
"plugin 'corrupt': .claude-plugin/plugin.json is not valid JSON" \
|
|
||||||
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
||||||
'Manifest check failed: 2 error(s)'
|
|
||||||
|
|
||||||
# --- 19b. A valid-JSON but non-object plugin.json is caught too ---
|
|
||||||
# `jq empty` passes on `[]`; it is the `.skills` lookup on such a root that aborts
|
|
||||||
# ("Cannot index array with string"), not the parse -- so the parse check alone would
|
|
||||||
# leave this exact crash reachable.
|
|
||||||
echo ""
|
|
||||||
echo "--- a valid-JSON non-object plugin.json is reported, not left to crash jq ---"
|
|
||||||
FIXTURE19B="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE19B")
|
|
||||||
mkdir -p "$FIXTURE19B/plugins/arrayjson/.claude-plugin" "$FIXTURE19B/plugins/second"
|
|
||||||
write_marketplace "$FIXTURE19B" "arrayjson=./plugins/arrayjson" "second=./plugins/second"
|
|
||||||
printf '[]\n' > "$FIXTURE19B/plugins/arrayjson/.claude-plugin/plugin.json"
|
|
||||||
assert_fails_with "$FIXTURE19B" \
|
|
||||||
"a non-object plugin.json is named by type and later plugins are still checked" \
|
|
||||||
"plugin 'arrayjson': .claude-plugin/plugin.json is a JSON array at its top level" \
|
|
||||||
"plugin 'second': .claude-plugin/plugin.json not found" \
|
|
||||||
'Manifest check failed: 2 error(s)'
|
|
||||||
|
|
||||||
# --- 20. Run with no argument outside a worktree: refuse, do not guess $PWD ---
|
|
||||||
# Every path this script touches hangs off REPO_ROOT, and its exit-0 path is "no
|
|
||||||
# manifest and nothing on disk" -- so `|| pwd` made a run from an empty directory
|
|
||||||
# outside any worktree exit 0, silently, having inspected no repository at all. Same
|
|
||||||
# reasoning as scripts/sync-marketplace-mirror.sh, which dropped its fallback first.
|
|
||||||
#
|
|
||||||
# `env -u GIT_DIR -u GIT_WORK_TREE` because run-tests.sh runs as a pre-push hook and git
|
|
||||||
# hooks export both, which would re-target `git rev-parse --show-toplevel` at the LIVE
|
|
||||||
# repo from any cwd -- making this case pass for the wrong reason.
|
|
||||||
echo ""
|
|
||||||
echo "--- with no argument outside a git worktree, it refuses instead of guessing \$PWD ---"
|
|
||||||
FIXTURE20="$(mktemp -d)"
|
|
||||||
FIXTURES+=("$FIXTURE20")
|
|
||||||
if (cd "$FIXTURE20" && env -u GIT_DIR -u GIT_WORK_TREE git rev-parse --show-toplevel) >/dev/null 2>&1; then
|
|
||||||
fail "fixture precondition: $FIXTURE20 is inside a git worktree, so this case cannot test the no-worktree path"
|
|
||||||
else
|
|
||||||
RC20=0
|
|
||||||
OUT20="$(cd "$FIXTURE20" && env -u GIT_DIR -u GIT_WORK_TREE bash "$SCRIPT" 2>&1)" || RC20=$?
|
|
||||||
if [[ $RC20 -eq 0 ]]; then
|
|
||||||
fail "exited 0 from outside a worktree with no argument -- it checked nothing and said so to no one"
|
|
||||||
elif [[ "$OUT20" != *"not inside a git worktree"* ]]; then
|
|
||||||
fail "exited $RC20 outside a worktree but not for the stated reason. Output: $OUT20"
|
|
||||||
else
|
|
||||||
pass "refuses to guess \$PWD when it cannot locate the repository root"
|
|
||||||
fi
|
|
||||||
fi
|
|
||||||
|
|
||||||
echo ""
|
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
|
||||||
[[ $FAIL -eq 0 ]]
|
|
||||||
Reference in New Issue
Block a user