ci(plugin): gate plugin/** — the path that shipped two live defects
CI & Build / Plugin hooks (push) Failing after 2s
CI & Build / Python lint (push) Successful in 8s
CI & Build / integration (push) Successful in 31s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 55s
CI & Build / Build & push image (push) Successful in 20s
CI & Build / Plugin hooks (push) Failing after 2s
CI & Build / Python lint (push) Successful in 8s
CI & Build / integration (push) Successful in 31s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 55s
CI & Build / Build & push image (push) Successful in 20s
`plugin/` is not built into the image; installs fetch it from this repo via .claude-plugin/marketplace.json, so a push IS the release. It was absent from the workflow's `paths:` filter entirely, meaning plugin changes ran no CI at all. Two separate defects reached a live install through that gap: #2198 — all four hook scripts inert (lowercase userConfig env vars, line-oriented `jq -rR`, line-oriented `cut -c`) #2209 — the fix for #2198 couldn't reach an install because the manifest version wasn't bumped, so the installer never refreshed its cache Adds `plugin/**` + `.claude-plugin/**` to `paths:` and a `plugin` job running scripts/check_plugin.py: 1. `bash -n` on every hook. 2. The three known-bad patterns from #2198. Verified by replay against c569cdd^ — all three are caught. Narrow by design; see below. 3. Shipped plugin content differs from origin/main => the manifest version must differ too. Stated against the base branch, not per-commit, so a batch needs one bump rather than one per commit. Replayed againstc569cdd: correctly fails. The checker found a real outstanding bug on its first run: the `cut -c1-2000` prompt cap in scribe_autoinject.sh was still line-oriented. Only the prior-art hook's copy got fixed inc569cdd. Now `head -c`. It then failed on this very commit for a missing version bump, which is the third time that rule has mattered and the first time something other than memory enforced it. Manifest bumped to 0.1.20. WHAT THIS DOESN'T COVER, and why. shellcheck is the right tool for check 2 and is NOT in ci-python; nor is jq, which every hook requires and silently bails without — so a "runs and stays silent" smoke test would pass vacuously today and prove nothing. Both need those two packages added to the CI image in the CI-runner repo, which is a separate change to a separate repo (rule #5: the toolchain comes from the image, not from apt-get at job start). Verified against CI-runner's Dockerfile and scripts/install-common.sh rather than assumed (rule #37). `plugin` is not in the build job's `needs`: the plugin doesn't ship in the image, and blocking the build wouldn't un-publish a bad hook — the push already did. A failed job still reddens the run. Closes #2204 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
This commit is contained in:
@@ -48,6 +48,15 @@ on:
|
||||
- "Dockerfile"
|
||||
- "assets/**"
|
||||
- "fable-mcp/**"
|
||||
# The plugin ships straight from this repo — installs fetch it via
|
||||
# .claude-plugin/marketplace.json, NOT from the image. So a push here is
|
||||
# the release, with no build step in between. Omitting these paths meant
|
||||
# plugin changes triggered no workflow at all, which is how #2198's three
|
||||
# broken hooks and then #2209's missing version bump both reached a live
|
||||
# install. See the `plugin` job below.
|
||||
- "plugin/**"
|
||||
- ".claude-plugin/**"
|
||||
- "scripts/check_plugin.py"
|
||||
- ".forgejo/workflows/ci.yml"
|
||||
# Manual trigger from the Forgejo Actions UI. Useful when an image has
|
||||
# been built but the deployment didn't pick it up, or when re-running
|
||||
@@ -102,6 +111,34 @@ jobs:
|
||||
run: npx vue-tsc --noEmit
|
||||
working-directory: frontend
|
||||
|
||||
# Guards the one part of this repo that ships to users without a build step.
|
||||
# See scripts/check_plugin.py for what it checks and, as importantly, what it
|
||||
# can't check yet (shellcheck and jq are absent from ci-python).
|
||||
plugin:
|
||||
name: Plugin hooks
|
||||
if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v')
|
||||
runs-on: python-ci
|
||||
container:
|
||||
image: git.fabledsword.com/bvandeusen/ci-python:3.14
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
with:
|
||||
# The version-bump check diffs shipped plugin content against
|
||||
# origin/main, so it needs history — a shallow clone can't resolve it
|
||||
# and the check would fail loudly rather than pass blind.
|
||||
fetch-depth: 0
|
||||
|
||||
# On main the comparison is against itself, so only the syntax and
|
||||
# pattern checks are meaningful there.
|
||||
- name: Check plugin hooks and manifest
|
||||
run: |
|
||||
if [ "${{ github.ref }}" = "refs/heads/main" ]; then
|
||||
python3 scripts/check_plugin.py --no-version
|
||||
else
|
||||
git fetch --no-tags origin main:refs/remotes/origin/main
|
||||
python3 scripts/check_plugin.py
|
||||
fi
|
||||
|
||||
lint:
|
||||
name: Python lint
|
||||
if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v')
|
||||
@@ -228,6 +265,12 @@ jobs:
|
||||
|
||||
build:
|
||||
name: Build & push image
|
||||
# `plugin` is deliberately NOT in needs. The plugin isn't in the image —
|
||||
# installs fetch it from git — so gating the server image on a hook lint
|
||||
# would couple two things that don't ship together, and blocking the build
|
||||
# wouldn't un-publish a bad hook anyway: the push already did that. A failed
|
||||
# `plugin` job still turns the whole run red, which is the signal that
|
||||
# matters.
|
||||
needs: [typecheck, lint, test]
|
||||
# Build on dev, main, and v* tag pushes. dev → :dev, main → :latest,
|
||||
# tag → :latest + :<version>; every build also gets an immutable :<sha>.
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "scribe",
|
||||
"description": "Scribe system-of-record for Claude Code: MCP tools over your notes/tasks/projects/rules, a session-start push channel that surfaces your always-on rules + active-project context, process-skills (writing-plans, systematic-debugging, verification, brainstorming, reusing-code), and your saved Scribe Processes auto-surfaced as skills (/scribe:sync). Replaces superpowers + file-memory with one app-backed plugin.",
|
||||
"version": "0.1.19",
|
||||
"version": "0.1.20",
|
||||
"author": { "name": "Bryan Van Deusen" },
|
||||
"mcpServers": {
|
||||
"scribe": {
|
||||
|
||||
@@ -44,7 +44,11 @@ case "$token" in *'${'*) token="" ;; esac
|
||||
[ -n "$url" ] && [ -n "$token" ] || exit 0
|
||||
|
||||
# Cap the query length — a giant prompt makes a giant URL for no extra signal.
|
||||
q=$(printf '%s' "$prompt" | cut -c1-2000)
|
||||
# `head -c`, not `cut -c1-2000`: cut is line-oriented and caps EACH LINE, so a
|
||||
# long multi-line prompt sailed past the budget entirely. Same defect as the
|
||||
# prior-art hook's code cap; this copy was missed when that one was fixed, and
|
||||
# scripts/check_plugin.py caught it.
|
||||
q=$(printf '%s' "$prompt" | head -c 2000)
|
||||
# `-sRr`, not `-rR`: jq -R reads LINE BY LINE, so a multi-line prompt encoded as
|
||||
# several lines joined by raw newlines and the request died. Single-line prompts
|
||||
# worked, which is why this looked healthy — the long, substantial prompts most
|
||||
|
||||
Executable
+235
@@ -0,0 +1,235 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Guards for `plugin/` — the one part of this repo that ships straight to users.
|
||||
|
||||
WHY THIS EXISTS. `plugin/` is not built into the Docker image. Installs fetch it
|
||||
from this git repo via `.claude-plugin/marketplace.json`, so a push IS the
|
||||
release for plugin content: no build, no gate, immediately fetchable. Two
|
||||
separate defects have reached a live install through that path:
|
||||
|
||||
- #2198 — all four hook scripts were inert (wrong env-var case, line-oriented
|
||||
`jq -rR`, line-oriented `cut -c`). No CI ran, because `plugin/**` wasn't in
|
||||
the workflow's `paths:` filter at all.
|
||||
- #2209 — the fix for #2198 shipped to `main` and still couldn't reach an
|
||||
install, because `plugin.json`'s version wasn't bumped and the installer
|
||||
compares versions to decide whether to refresh its cache.
|
||||
|
||||
The rule for the second one was already written down and was still missed. A
|
||||
written rule that depends on being remembered during a long session is not a
|
||||
control; this is.
|
||||
|
||||
WHAT IT DOESN'T COVER. shellcheck would be the right tool for the pattern
|
||||
checks below and is NOT in `ci-python` (verified against CI-runner's Dockerfile
|
||||
+ scripts/install-common.sh, not from memory — rule #37). Neither is `jq`, which
|
||||
every hook requires and bails without, so a "runs and stays silent" smoke test
|
||||
would pass vacuously today and prove nothing. Both need the image to grow those
|
||||
two packages; until then this checks the specific bug classes that actually bit,
|
||||
which is narrower than shellcheck but not nothing.
|
||||
|
||||
Usage:
|
||||
python3 scripts/check_plugin.py # all checks
|
||||
python3 scripts/check_plugin.py --no-version # skip the bump check
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
PLUGIN_DIR = ROOT / "plugin"
|
||||
HOOKS_DIR = PLUGIN_DIR / "hooks"
|
||||
MANIFEST = PLUGIN_DIR / ".claude-plugin" / "plugin.json"
|
||||
|
||||
# Paths whose contents reach an install. Keep in step with the workflow's
|
||||
# `paths:` filter — a path that ships but isn't checked here is the gap again.
|
||||
SHIPPED = ("plugin", ".claude-plugin")
|
||||
|
||||
failures: list[str] = []
|
||||
|
||||
|
||||
def fail(msg: str) -> None:
|
||||
failures.append(msg)
|
||||
print(f"FAIL {msg}")
|
||||
|
||||
|
||||
def ok(msg: str) -> None:
|
||||
print(f"ok {msg}")
|
||||
|
||||
|
||||
def hook_scripts() -> list[Path]:
|
||||
return sorted(HOOKS_DIR.glob("*.sh"))
|
||||
|
||||
|
||||
# --- syntax ----------------------------------------------------------------
|
||||
|
||||
def check_syntax() -> None:
|
||||
"""`bash -n` every hook. Catches nothing subtle, costs nothing, and a syntax
|
||||
error here means a hook that silently never runs."""
|
||||
for script in hook_scripts():
|
||||
proc = subprocess.run(
|
||||
["bash", "-n", str(script)], capture_output=True, text=True
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
fail(f"{script.relative_to(ROOT)}: bash -n — {proc.stderr.strip()}")
|
||||
else:
|
||||
ok(f"{script.relative_to(ROOT)}: syntax")
|
||||
|
||||
|
||||
# --- known-bad patterns ----------------------------------------------------
|
||||
|
||||
# Each entry: (compiled pattern, short label, why it's wrong).
|
||||
# These are the exact classes from #2198. They are deliberately specific — a
|
||||
# broad shell linter belongs in the image, not hand-rolled here.
|
||||
PATTERNS: list[tuple[re.Pattern, str, str]] = [
|
||||
(
|
||||
re.compile(r"CLAUDE_PLUGIN_OPTION_[a-z]"),
|
||||
"lowercase userConfig env var",
|
||||
"Claude Code exports userConfig to hooks as CLAUDE_PLUGIN_OPTION_<KEY> "
|
||||
"with the key UPPERCASED. The lowercase spelling reads as empty and the "
|
||||
"hook then does nothing, silently.",
|
||||
),
|
||||
(
|
||||
# -R without -s: reads input line by line, so a multi-line payload is
|
||||
# encoded per line and joined with raw newlines. The class is a-r + t-z
|
||||
# (i.e. every letter EXCEPT `s`) so `-rR` is caught and `-sRr` is not —
|
||||
# an earlier a-q spelling silently excluded `r` and missed the real
|
||||
# defect, which is exactly the flag combination that shipped.
|
||||
re.compile(r"jq\s+-(?:[a-rt-zA-Z]*R[a-rt-zA-Z]*)\s"),
|
||||
"line-oriented jq -R",
|
||||
"jq -R reads input LINE BY LINE. Encoding a multi-line payload that way "
|
||||
"produces separate encoded lines joined by raw newlines — an invalid "
|
||||
"URL. Use -s (slurp) as well, e.g. `jq -sRr '@uri'`.",
|
||||
),
|
||||
(
|
||||
re.compile(r"\|\s*cut\s+-c"),
|
||||
"line-oriented cut for a payload cap",
|
||||
"cut -c truncates EACH LINE, so it does not cap total size. Use "
|
||||
"`head -c N` to bound a payload.",
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
def check_patterns() -> None:
|
||||
for script in hook_scripts():
|
||||
text = script.read_text(encoding="utf-8", errors="replace")
|
||||
rel = script.relative_to(ROOT)
|
||||
hits = 0
|
||||
for line_no, line in enumerate(text.splitlines(), 1):
|
||||
# A line that only *documents* the trap is fine — several hooks now
|
||||
# carry a comment naming the wrong form so the next reader knows.
|
||||
if line.lstrip().startswith("#"):
|
||||
continue
|
||||
for pattern, label, why in PATTERNS:
|
||||
if pattern.search(line):
|
||||
hits += 1
|
||||
fail(f"{rel}:{line_no}: {label}\n {line.strip()}\n {why}")
|
||||
if not hits:
|
||||
ok(f"{rel}: no known-bad patterns")
|
||||
|
||||
|
||||
# --- the version bump ------------------------------------------------------
|
||||
|
||||
def _git(*args: str) -> tuple[int, str]:
|
||||
proc = subprocess.run(
|
||||
["git", *args], capture_output=True, text=True, cwd=ROOT
|
||||
)
|
||||
return proc.returncode, (proc.stdout or proc.stderr).strip()
|
||||
|
||||
|
||||
def manifest_version(ref: str | None = None) -> str | None:
|
||||
"""The manifest version at `ref`, or in the working tree when ref is None."""
|
||||
if ref is None:
|
||||
try:
|
||||
return json.loads(MANIFEST.read_text()).get("version")
|
||||
except Exception:
|
||||
return None
|
||||
rel = MANIFEST.relative_to(ROOT).as_posix()
|
||||
code, out = _git("show", f"{ref}:{rel}")
|
||||
if code != 0:
|
||||
return None
|
||||
try:
|
||||
return json.loads(out).get("version")
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
|
||||
def check_version_bump(base: str = "origin/main") -> None:
|
||||
"""If shipped plugin content differs from `base`, the version must too.
|
||||
|
||||
Stated against the BASE BRANCH rather than the last commit on purpose. A
|
||||
per-commit rule would demand a bump from every commit in a batch; what
|
||||
actually matters is that whatever reaches an install carries a version the
|
||||
installer can tell apart from the one already cached. One bump per batch,
|
||||
which is also how a human would do it.
|
||||
"""
|
||||
code, _ = _git("rev-parse", "--verify", base)
|
||||
if code != 0:
|
||||
# Do NOT pass silently — a check that quietly no-ops is how this class
|
||||
# of bug survives in the first place.
|
||||
fail(
|
||||
f"cannot resolve {base}, so the version-bump check could not run. "
|
||||
f"Fetch it (checkout needs fetch-depth: 0) or pass --no-version "
|
||||
f"deliberately."
|
||||
)
|
||||
return
|
||||
|
||||
code, changed = _git("diff", "--name-only", base, "--", *SHIPPED)
|
||||
if code != 0:
|
||||
fail(f"git diff against {base} failed: {changed}")
|
||||
return
|
||||
if not changed.strip():
|
||||
ok(f"no shipped plugin changes against {base} — version bump not required")
|
||||
return
|
||||
|
||||
here, there = manifest_version(), manifest_version(base)
|
||||
if here is None:
|
||||
fail(f"could not read a version from {MANIFEST.relative_to(ROOT)}")
|
||||
return
|
||||
if there is None:
|
||||
ok(f"no manifest on {base} — treating as a new plugin (version {here})")
|
||||
return
|
||||
if here == there:
|
||||
files = "\n ".join(changed.splitlines())
|
||||
fail(
|
||||
f"plugin content changed but the manifest version is still {here}.\n"
|
||||
f" The installer compares versions to decide whether to refresh "
|
||||
f"its cache, so an unchanged version means these edits reach the repo "
|
||||
f"and stop there — the marketplace clone updates, the cache that "
|
||||
f"actually executes does not (issue #2209).\n"
|
||||
f" Bump `version` in {MANIFEST.relative_to(ROOT)}.\n"
|
||||
f" Changed:\n {files}"
|
||||
)
|
||||
else:
|
||||
ok(f"plugin content changed and version moved {there} -> {here}")
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument("--no-version", action="store_true",
|
||||
help="skip the manifest version-bump check")
|
||||
parser.add_argument("--base", default="origin/main",
|
||||
help="branch the version bump is measured against")
|
||||
args = parser.parse_args()
|
||||
|
||||
if not HOOKS_DIR.is_dir():
|
||||
print(f"FAIL no hooks directory at {HOOKS_DIR}")
|
||||
return 1
|
||||
|
||||
check_syntax()
|
||||
check_patterns()
|
||||
if not args.no_version:
|
||||
check_version_bump(args.base)
|
||||
|
||||
print()
|
||||
if failures:
|
||||
print(f"{len(failures)} problem(s) found.")
|
||||
return 1
|
||||
print("All plugin checks passed.")
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in New Issue
Block a user