From 02afa2d6fbe786bbb0217505d0224068d55da56d Mon Sep 17 00:00:00 2001 From: Ron Date: Wed, 3 Jun 2026 17:51:04 -0700 Subject: [PATCH 1/2] ci(plugins): add cross-size plugin safety workflow + clock-simple goldens - .github/workflows/test-plugins.yml: runs the LEDMatrix core safety harness against changed plugins on PR (installs their deps), validates manifests against schema/manifest_schema.json, and enforces the version bump. Test-only changes (plugins//test/**) don't trigger the gate. - clock-simple: test/harness.json + golden images (128x32, 128x64, 256x32) as the worked example for opt-in golden-image regression. - CLAUDE.md: document the safety harness workflow for contributors. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/test-plugins.yml | 132 ++++++++++++++++++ CLAUDE.md | 31 ++++ .../test/golden/128x32/clock-simple.png | Bin 0 -> 479 bytes .../test/golden/128x64/clock-simple.png | Bin 0 -> 512 bytes .../test/golden/256x32/clock-simple.png | Bin 0 -> 519 bytes plugins/clock-simple/test/harness.json | 10 ++ 6 files changed, 173 insertions(+) create mode 100644 .github/workflows/test-plugins.yml create mode 100644 plugins/clock-simple/test/golden/128x32/clock-simple.png create mode 100644 plugins/clock-simple/test/golden/128x64/clock-simple.png create mode 100644 plugins/clock-simple/test/golden/256x32/clock-simple.png create mode 100644 plugins/clock-simple/test/harness.json diff --git a/.github/workflows/test-plugins.yml b/.github/workflows/test-plugins.yml new file mode 100644 index 00000000..d4531a1a --- /dev/null +++ b/.github/workflows/test-plugins.yml @@ -0,0 +1,132 @@ +name: Plugin Safety + +# Renders every changed plugin across all supported matrix sizes and screens +# using the LEDMatrix core harness, and validates manifests. Gates PRs that +# touch plugin code so a change can't silently break a size or screen. + +on: + pull_request: + paths: + - 'plugins/**' + workflow_dispatch: + inputs: + all: + description: 'Check every plugin (not just changed ones)' + type: boolean + default: false + +env: + # The core repo provides the harness (scripts/check_plugin.py). Point these at + # the fork/branch where the harness lives until it is merged upstream. + CORE_REPO: ChuckBuilds/LEDMatrix + CORE_REF: main + +jobs: + safety: + runs-on: ubuntu-latest + steps: + - name: Checkout plugins + uses: actions/checkout@v4 + with: + fetch-depth: 0 + path: plugins-repo + + - name: Checkout core (harness) + uses: actions/checkout@v4 + with: + repository: ${{ env.CORE_REPO }} + ref: ${{ env.CORE_REF }} + path: core + + - uses: actions/setup-python@v5 + with: + python-version: '3.12' + cache: pip + + - name: Install core + harness deps + working-directory: core + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt -r requirements-test.txt + pip install RGBMatrixEmulator + + - name: Determine changed plugins (non-test code only) + id: changed + working-directory: plugins-repo + run: | + if [ "${{ github.event.inputs.all }}" = "true" ]; then + ids=$(ls plugins) + else + base="${{ github.event.pull_request.base.sha }}" + # Plugins with a changed file outside their test/ dir. + ids=$(git diff --name-only "$base"...HEAD -- 'plugins/*' \ + | grep -vE '^plugins/[^/]+/test/' \ + | sed -E 's#^plugins/([^/]+)/.*#\1#' | sort -u) + fi + echo "ids<> "$GITHUB_OUTPUT" + echo "$ids" >> "$GITHUB_OUTPUT" + echo "EOF" >> "$GITHUB_OUTPUT" + echo "Changed plugins:"; echo "$ids" + + - name: Enforce version bump on changed plugins + if: steps.changed.outputs.ids != '' && github.event.inputs.all != 'true' + working-directory: plugins-repo + run: | + base="${{ github.event.pull_request.base.sha }}" + fail=0 + for pid in ${{ steps.changed.outputs.ids }}; do + cur=$(python -c "import json;print(json.load(open('plugins/$pid/manifest.json'))['version'])") + old=$(git show "$base:plugins/$pid/manifest.json" 2>/dev/null \ + | python -c "import json,sys;print(json.load(sys.stdin)['version'])" 2>/dev/null || echo "") + if [ -n "$old" ] && [ "$cur" = "$old" ]; then + echo "::error::$pid code changed but version not bumped (still $cur). Users won't get the update." + fail=1 + else + echo "[ok] $pid version $old -> $cur" + fi + done + exit $fail + + - name: Validate manifests against schema + if: steps.changed.outputs.ids != '' + run: | + python - <<'PY' + import json, sys + from pathlib import Path + import jsonschema + schema = json.load(open("core/schema/manifest_schema.json")) + ids = """${{ steps.changed.outputs.ids }}""".split() + failed = False + for pid in ids: + mpath = Path("plugins-repo/plugins") / pid / "manifest.json" + if not mpath.exists(): + print(f"[skip] {pid}: no manifest (deleted?)"); continue + try: + jsonschema.validate(json.load(open(mpath)), schema) + print(f"[ok] {pid}: manifest valid") + except jsonschema.ValidationError as e: + print(f"[FAIL] {pid}: {e.message}"); failed = True + sys.exit(1 if failed else 0) + PY + + - name: Run safety harness on changed plugins + if: steps.changed.outputs.ids != '' + run: | + set -e + ids="${{ steps.changed.outputs.ids }}" + fail=0 + for pid in $ids; do + pdir="plugins-repo/plugins/$pid" + [ -d "$pdir" ] || { echo "::notice::$pid removed, skipping"; continue; } + echo "::group::$pid" + # Install the plugin's own runtime deps so it loads like a real install. + if [ -f "$pdir/requirements.txt" ]; then pip install -r "$pdir/requirements.txt" || true; fi + python core/scripts/check_plugin.py --plugin "$pid" \ + --plugin-dir "$PWD/plugins-repo/plugins" || fail=1 + echo "::endgroup::" + done + exit $fail + + - name: Nothing to check + if: steps.changed.outputs.ids == '' + run: echo "No plugin code changed (test-only or docs change) — safety harness skipped." diff --git a/CLAUDE.md b/CLAUDE.md index aa44ea26..8ca823d3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,3 +49,34 @@ Third-party plugins keep their own `repo` URL and empty `plugin_path`. ## Git Hooks - `scripts/pre-commit` — Auto-syncs `plugins.json` when manifest versions change - Install: `cp scripts/pre-commit .git/hooks/pre-commit` + +## Plugin Safety Harness (cross-size / cross-screen) + +Each plugin can expose multiple screens and must render on every supported matrix +size (64×32, 128×32, 128×64, 256×32). The harness lives in the **core** repo +(`LEDMatrix/scripts/check_plugin.py`) and renders every screen at every size, +failing on crashes, content drawn past the panel edge, or visual drift vs +committed golden images. + +**Before opening a PR that changes a plugin:** +```bash +# from a LEDMatrix (core) checkout, with the monorepo plugins on the path: +python scripts/check_plugin.py --plugin \ + --plugin-dir /path/to/ledmatrix-plugins/plugins --out-dir /tmp/preview +``` +Eyeball the PNGs in `/tmp/preview`, then fix any FAIL (overflow/crash) before pushing. + +**Golden images (optional, per plugin):** commit reference PNGs so visual drift is +caught automatically: +``` +plugins//test/harness.json # deterministic config / mock data / frozen time +plugins//test/golden//.png +``` +Regenerate with `check_plugin.py --update-golden` and review the diff. See +`clock-simple/test/` for a worked example and `LEDMatrix/docs/plugin-safety-harness.md` +for the full reference. + +**CI:** `.github/workflows/test-plugins.yml` runs the harness against every +*changed* plugin on each PR (installs that plugin's `requirements.txt` first), +validates its manifest against `schema/manifest_schema.json`, and enforces the +version bump. Test-only changes (`plugins//test/**`) don't trigger the gate. diff --git a/plugins/clock-simple/test/golden/128x32/clock-simple.png b/plugins/clock-simple/test/golden/128x32/clock-simple.png new file mode 100644 index 0000000000000000000000000000000000000000..ca9e396ca8e10b9e735967813fa24dea5852461e GIT binary patch literal 479 zcmV<50U-W~P)aGft@}&=>!w~Z`piEN=f*>#hl0Oi!ngM`udoupj z31Suoxy;kG94_aP`~eqbSZ?7D+?a;^5pGcCwGaYi`c)yJYZ;{FJd;=OQnqhag{SF6 zT8=GXyZTNuATi-&d1>m%GlLBHyRQ|c4wMu7Dv$RJfJ({d3zg37(O<0lA$APDR++m|rEa zv9!!^g~t-K1T8Zthy29mP60rsTG3Y%v?KJ${&9?N{5*oK%{ll|7H2^7n0N-9JW%eo zH~2@4FVVcV_VKg4Wf`C~IGx4^m7R0YzIpbqZjCQ^xqt7cpiTodTLGFa?kv0g9}2*} z)CO^^T&u-y|AzuF{zm_yA%e}H7EC@}Y%f8x4G}2z3kz|xq1#&Moi<_^4N;J+`2d)V VMR9vUNc#W)002ovPDHLkV1l8e*~I_= literal 0 HcmV?d00001 diff --git a/plugins/clock-simple/test/golden/128x64/clock-simple.png b/plugins/clock-simple/test/golden/128x64/clock-simple.png new file mode 100644 index 0000000000000000000000000000000000000000..b6e01a7cd7ecace1e81b11b56f689402b2e20a38 GIT binary patch literal 512 zcmeAS@N?(olHy`uVBq!ia0vp^4M6O`!2~2@x4h6`U|>A%>EaktG3V`^=)5BeJg$XT zThCtFwB_BIsa;JvajLp3?!|r2lm*xqzP|Cm!-oT?Zv!jmzksW2++B{nUHox<^9s4v zgA;!}F1|*3a~QRQhts%Nn-zFIMK<*D3Zd zop7lrhw1dyaw*1x9O28Ruj(^1TGUwkJ=$}nb5CdK%;VK8Csr1PTrFHvEI7)`06)P9%p|z zI4$-6I%l)_f|Fl+*?Qb@m%GIMOxG`Pl8b(I)WMK%$K5vx+>|q%ExW*Nu@9?l+)}-3 z+GQ=5oR>~s#hiCMpKDTm_th?*@CqNt;0#x{9!2|om6MpSteEyCm-CijRm$HTz2!DG z>$V=1yf@{fV8C6qd(Clo0@iK2|DoQotm}4hw!Ho2*x=rNvshv@4I%_w2t|uhEh6ae1rme7$MQ x7p!KTX!1r~?NOHFc9Vo<6ll-&+1>`_gkBR4(?>NlYR>_QT*9^O=SoL2@3$oqb4P}GCe1V%mwv zr@Ak9@8*$vKN#ROBcCGPaz%fhhUoD_0}S9y2%(ti<|1wgTF-qOc>n+a0001B1m;zP z#N}T3HJ6{x8?DIpRd9z@peY8Finvb&2Jk2K;;Bj`(d!ngnCN!7r6>_pj7c!S6uCx= zu01Yy-Pz}^$TMh%Xp>YE!Xy}AiiRTR#cEXsa0COK6ss~w4Z#~;V1VPuRh#r}3A!Ql zp7{fG!ZPUlQJowE0BnW|@IZB$@)))*Xzm^VA@Gl|xEd3vGHWh zTsz~dYyZWiO+C7A{ttmwpg|fNSUvGiw|FO?$36d5!2kyhE%dUcuNL>te^oHR1N}Yv zz=qV`uQu@N!~huJ30pU$ta!(Pcy&XKf&Lgwg2!P)3IJ|z%`YBMMR8- Date: Fri, 5 Jun 2026 17:52:28 -0700 Subject: [PATCH 2/2] ci(plugins): harden safety workflow per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit review on #144: - Pass changed-plugin ids to steps via env vars instead of interpolating ${{ }} directly into shell/python bodies, and validate each id against ^[a-z0-9][a-z0-9._-]*$ before use — plugin ids flow into `python -c` strings, so a crafted directory name on a PR branch could otherwise inject code (template-injection / code-injection). - Stop swallowing plugin dependency install failures (`|| true`); a plugin whose runtime deps can't install can't be loaded, so let the job fail. - CLAUDE.md: add a language tag to the harness/golden fenced block (MD040). - clock-simple/test/harness.json: add explicit empty `mock_data` object. Skipped CodeRabbit's "pin actions to commit SHAs" suggestion: this repo's existing update-registry.yml uses tag refs (actions/checkout@v4, setup-python@v4), so the workflow matches the repo's established convention. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/test-plugins.yml | 48 ++++++++++++++++++-------- CLAUDE.md | 2 +- plugins/clock-simple/test/harness.json | 1 + 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test-plugins.yml b/.github/workflows/test-plugins.yml index d4531a1a..75a8c4a4 100644 --- a/.github/workflows/test-plugins.yml +++ b/.github/workflows/test-plugins.yml @@ -1,8 +1,8 @@ name: Plugin Safety -# Renders every changed plugin across all supported matrix sizes and screens -# using the LEDMatrix core harness, and validates manifests. Gates PRs that -# touch plugin code so a change can't silently break a size or screen. +# Renders every changed plugin across a representative spread of matrix sizes and +# screens using the LEDMatrix core harness, and validates manifests. Gates PRs +# that touch plugin code so a change can't silently break a size or screen. on: pull_request: @@ -53,13 +53,15 @@ jobs: - name: Determine changed plugins (non-test code only) id: changed working-directory: plugins-repo + env: + ALL_INPUT: ${{ github.event.inputs.all }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | - if [ "${{ github.event.inputs.all }}" = "true" ]; then + if [ "$ALL_INPUT" = "true" ]; then ids=$(ls plugins) else - base="${{ github.event.pull_request.base.sha }}" # Plugins with a changed file outside their test/ dir. - ids=$(git diff --name-only "$base"...HEAD -- 'plugins/*' \ + ids=$(git diff --name-only "$BASE_SHA"...HEAD -- 'plugins/*' \ | grep -vE '^plugins/[^/]+/test/' \ | sed -E 's#^plugins/([^/]+)/.*#\1#' | sort -u) fi @@ -71,12 +73,20 @@ jobs: - name: Enforce version bump on changed plugins if: steps.changed.outputs.ids != '' && github.event.inputs.all != 'true' working-directory: plugins-repo + env: + # Read IDs from env (never interpolate ${{ }} into the script body) and + # validate each one — plugin ids flow into python -c strings below, so a + # crafted directory name on a PR branch could otherwise inject code. + IDS: ${{ steps.changed.outputs.ids }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | - base="${{ github.event.pull_request.base.sha }}" fail=0 - for pid in ${{ steps.changed.outputs.ids }}; do + for pid in $IDS; do + case "$pid" in + '' | *[!a-z0-9._-]*) echo "::error::invalid plugin id '$pid'"; fail=1; continue ;; + esac cur=$(python -c "import json;print(json.load(open('plugins/$pid/manifest.json'))['version'])") - old=$(git show "$base:plugins/$pid/manifest.json" 2>/dev/null \ + old=$(git show "$BASE_SHA:plugins/$pid/manifest.json" 2>/dev/null \ | python -c "import json,sys;print(json.load(sys.stdin)['version'])" 2>/dev/null || echo "") if [ -n "$old" ] && [ "$cur" = "$old" ]; then echo "::error::$pid code changed but version not bumped (still $cur). Users won't get the update." @@ -89,15 +99,20 @@ jobs: - name: Validate manifests against schema if: steps.changed.outputs.ids != '' + env: + IDS: ${{ steps.changed.outputs.ids }} run: | python - <<'PY' - import json, sys + import json, os, re, sys from pathlib import Path import jsonschema schema = json.load(open("core/schema/manifest_schema.json")) - ids = """${{ steps.changed.outputs.ids }}""".split() + valid = re.compile(r"^[a-z0-9][a-z0-9._-]*$") + ids = os.environ.get("IDS", "").split() failed = False for pid in ids: + if not valid.match(pid): + print(f"[FAIL] invalid plugin id: {pid!r}"); failed = True; continue mpath = Path("plugins-repo/plugins") / pid / "manifest.json" if not mpath.exists(): print(f"[skip] {pid}: no manifest (deleted?)"); continue @@ -111,16 +126,21 @@ jobs: - name: Run safety harness on changed plugins if: steps.changed.outputs.ids != '' + env: + IDS: ${{ steps.changed.outputs.ids }} run: | set -e - ids="${{ steps.changed.outputs.ids }}" fail=0 - for pid in $ids; do + for pid in $IDS; do + case "$pid" in + '' | *[!a-z0-9._-]*) echo "::error::invalid plugin id '$pid'"; fail=1; continue ;; + esac pdir="plugins-repo/plugins/$pid" [ -d "$pdir" ] || { echo "::notice::$pid removed, skipping"; continue; } echo "::group::$pid" # Install the plugin's own runtime deps so it loads like a real install. - if [ -f "$pdir/requirements.txt" ]; then pip install -r "$pdir/requirements.txt" || true; fi + # A failure here is a real problem (the plugin can't load), so let it fail. + if [ -f "$pdir/requirements.txt" ]; then pip install -r "$pdir/requirements.txt"; fi python core/scripts/check_plugin.py --plugin "$pid" \ --plugin-dir "$PWD/plugins-repo/plugins" || fail=1 echo "::endgroup::" diff --git a/CLAUDE.md b/CLAUDE.md index 8ca823d3..62e12856 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,7 +68,7 @@ Eyeball the PNGs in `/tmp/preview`, then fix any FAIL (overflow/crash) before pu **Golden images (optional, per plugin):** commit reference PNGs so visual drift is caught automatically: -``` +```text plugins//test/harness.json # deterministic config / mock data / frozen time plugins//test/golden//.png ``` diff --git a/plugins/clock-simple/test/harness.json b/plugins/clock-simple/test/harness.json index e4abc3c5..f3eea3b5 100644 --- a/plugins/clock-simple/test/harness.json +++ b/plugins/clock-simple/test/harness.json @@ -6,5 +6,6 @@ "show_seconds": false, "show_date": true }, + "mock_data": {}, "freeze_time": "2025-08-01 15:25:00" }