ci(plugin): gate the path that ships straight to users, and fix one more line-oriented cap (#84)

.forgejo/workflows/ci.yml

@@ -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,48 @@ 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:
+      # Bare `uses:`, no `with:` block. Adding one made this action fail to
+      # extract on the act_runner ("Cannot find module .../dist/index.js") while
+      # every bare checkout in the same run succeeded — see run 3027. Nothing
+      # here needs `fetch-depth: 0` anyway: the version check diffs two trees,
+      # and a tree diff needs both trees, not a common ancestor. A depth-1 fetch
+      # of main's tip is enough, and cheaper.
+      - uses: actions/checkout@v6
+
+      # Per-job, not in the image, per CI-runner's docs/process.md: "If only one
+      # project needs the dep, prefer that project installing it per-job in
+      # their workflow — at least until a second consumer arrives." Scribe is
+      # the only consumer today. Promotion into ci-python is filed as an issue
+      # on CI-runner rather than assumed here.
+      #
+      # jq is not optional for the smoke test: every hook exits at line 1
+      # without it, so the check would pass while exercising nothing.
+      - name: Install shell tooling
+        run: |
+          apt-get update -qq
+          apt-get install -y -qq --no-install-recommends jq shellcheck
+
+      # On main the comparison would be against itself, so only the syntax and
+      # pattern checks mean anything 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 --depth=1 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 +279,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>.

ci-requirements.md

@@ -9,8 +9,9 @@
 git.fabledsword.com/bvandeusen/ci-python:3.14
 ```
 
-Used by all four jobs in `.forgejo/workflows/ci.yml`: typecheck (Vue/TS),
-lint (ruff), test (pytest), build (docker buildx).
+Used by all six jobs in `.forgejo/workflows/ci.yml`: typecheck (Vue/TS),
+plugin (hook checks), lint (ruff), test (pytest), integration (pytest +
+real Postgres), build (docker buildx).
 
 ## Image deps used
 
@@ -18,6 +19,8 @@ lint (ruff), test (pytest), build (docker buildx).
 - node 24 (used for `npm ci` + `vue-tsc` in the typecheck job, and as the
   frontend builder stage inside the production `Dockerfile`)
 - ruff (lint job runs `ruff check src/` with zero install overhead)
+- uv (test + integration jobs run `uv sync --locked`; installed in the
+  image since the ci-python Dockerfile started pip-installing it)
 - docker CLI + buildx (build job pushes the production image to the
   Forgejo registry)
 
@@ -26,14 +29,15 @@ lint (ruff), test (pytest), build (docker buildx).
 Anything CI installs at job time that isn't in the image. Promotion
 candidates if more than one project needs them.
 
-- `uv` — installed inline in the test job (`curl -LsSf
-  https://astral.sh/uv/install.sh | sh`). **Temporary**: belongs in the
-  ci-python image so every consumer doesn't re-install on cold start.
-  Tracked at [CI-Runner](https://git.fabledsword.com/bvandeusen/CI-runner).
-- `http-ece` is `--no-build-isolation`-installed before the editable
-  package install because http-ece doesn't declare `setuptools` as a
-  build dep and uv creates bare venvs without it. Not promotion-worthy
-  (one project, one wheel).
+- `jq` + `shellcheck` — apt-installed in the **plugin** job, which lints
+  the four Claude Code hook scripts and runs their fail-open smoke test.
+  Per `docs/process.md`'s decision checkpoint, single-consumer deps stay
+  per-job until a second consumer wants them; Scribe is the only one so
+  far. Both are small (jq ~1 MB, shellcheck ~20 MB) and would be
+  promotion candidates the moment another project lints shell.
+  **jq is load-bearing for the smoke test specifically**: every hook
+  starts with `command -v jq || exit 0`, so without it the test passes
+  while exercising nothing.
 
 ## Notes
 

plugin/.claude-plugin/plugin.json

@@ -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": {

plugin/hooks/scribe_autoinject.sh

@@ -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

plugin/hooks/scribe_session_context.sh

@@ -41,6 +41,11 @@ set -uo pipefail
 
 command -v jq >/dev/null 2>&1 || exit 0   # needed to emit the JSON envelope safely
 
+# `CDPATH= cd` is deliberate, not a typo'd assignment: it runs this one `cd`
+# with CDPATH empty, so an operator whose CDPATH happens to contain a matching
+# directory name can't send us somewhere else — and `cd` won't echo the resolved
+# path into our output. shellcheck can't tell that idiom from `CDPATH=cd`.
+# shellcheck disable=SC1007
 here=$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) || exit 0
 
 # SessionStart delivers a JSON event on stdin; `source` is startup|resume|compact|clear.

scripts/check_plugin.py

@@ -0,0 +1,359 @@
+#!/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.
+
+shellcheck and jq are NOT in `ci-python` (verified against CI-runner's Dockerfile
+and scripts/install-common.sh, not from memory — rule #37). CI installs both
+per-job, which is what CI-runner's own docs/process.md prescribes for a dep with
+a single consumer: "If only one project needs the dep, prefer that project
+installing it per-job in their workflow — at least until a second consumer
+arrives." Promotion into the image is filed as an issue there rather than
+assumed here.
+
+Both are optional at runtime: without shellcheck the lint step is SKIPPED and
+says so, and without jq the smoke test is skipped. A skipped check announces
+itself loudly, because a check that quietly no-ops is the failure mode this
+whole file exists to prevent.
+
+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 os
+import re
+import shutil
+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 skip(msg: str) -> None:
+    # Loud on purpose. A check that quietly does nothing is indistinguishable
+    # from a check that passed — the exact confusion that let #2198 survive.
+    print(f"SKIP  {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 ------------------------------------------------------
+
+# --- shellcheck ------------------------------------------------------------
+
+def check_shellcheck() -> None:
+    """Real shell linting, where available.
+
+    The hand-rolled patterns above only know the bugs that already happened.
+    This is what catches the next one.
+    """
+    exe = shutil.which("shellcheck")
+    if not exe:
+        skip("shellcheck not installed — install it to lint the hooks properly")
+        return
+    for script in hook_scripts():
+        proc = subprocess.run(
+            [exe, "--severity=warning", "--shell=bash", str(script)],
+            capture_output=True, text=True,
+        )
+        rel = script.relative_to(ROOT)
+        if proc.returncode != 0:
+            fail(f"{rel}: shellcheck\n{proc.stdout.strip()}")
+        else:
+            ok(f"{rel}: shellcheck")
+
+
+# --- the fail-open contract ------------------------------------------------
+
+# Every hook promises never to break the operator's session: unconfigured or
+# unreachable, it exits 0. Three of them additionally promise SILENCE, because
+# they are pure enrichment. scribe_session_context.sh is the exception by
+# design — it always emits a static behavioural floor that needs no credentials
+# and no network, so "silent" would be the wrong assertion for it.
+#
+# This is the contract that made #2198 invisible for weeks, so it is worth
+# pinning: the bug and the healthy no-results case look identical from outside.
+# Pinning it does NOT make the failure visible; it makes sure the fail-open
+# behaviour is deliberate rather than accidental.
+SMOKE_EVENTS: dict[str, str] = {
+    "scribe_autoinject.sh": json.dumps(
+        {"session_id": "smoke", "cwd": ".", "prompt": "a multi-line\nprompt\nhere"}
+    ),
+    "scribe_prior_art.sh": json.dumps(
+        {"session_id": "smoke", "cwd": ".", "tool_name": "Edit",
+         "tool_input": {"file_path": "src/x.py", "new_string": "def f():\n    pass\n"}}
+    ),
+    "scribe_sync_processes.sh": json.dumps({"source": "startup"}),
+    "scribe_session_context.sh": json.dumps({"source": "startup"}),
+}
+
+# The one hook that legitimately produces output with no credentials.
+STATIC_FLOOR = "scribe_session_context.sh"
+
+
+def _run_hook(script: Path, event: str, env_extra: dict[str, str]) -> subprocess.CompletedProcess:
+    env = {k: v for k, v in os.environ.items()
+           if not k.startswith(("SCRIBE_", "CLAUDE_PLUGIN_OPTION_"))}
+    env.update(env_extra)
+    return subprocess.run(
+        ["bash", str(script)], input=event, capture_output=True, text=True,
+        env=env, timeout=30,
+    )
+
+
+def check_fail_open() -> None:
+    if not shutil.which("jq"):
+        # Without jq every hook bails at its first line, so this would pass
+        # while exercising nothing. Say so rather than bank a green tick.
+        skip("jq not installed — the hooks would exit at line 1, so this "
+             "check would pass without testing anything")
+        return
+
+    scenarios = [
+        ("unconfigured", {}),
+        # Connection refused immediately — exercises the unreachable-instance
+        # path without waiting on a real network timeout.
+        ("unreachable", {"SCRIBE_URL": "http://127.0.0.1:1", "SCRIBE_TOKEN": "x"}),
+    ]
+    for script in hook_scripts():
+        rel = script.relative_to(ROOT)
+        event = SMOKE_EVENTS.get(script.name)
+        if event is None:
+            skip(f"{rel}: no smoke event defined")
+            continue
+        for label, env_extra in scenarios:
+            try:
+                proc = _run_hook(script, event, env_extra)
+            except subprocess.TimeoutExpired:
+                fail(f"{rel} [{label}]: hung — a hook must never block a session")
+                continue
+            if proc.returncode != 0:
+                fail(f"{rel} [{label}]: exited {proc.returncode}, must be 0 — "
+                     f"a recall aid may never fail the operator's action")
+                continue
+            out = proc.stdout.strip()
+            if script.name == STATIC_FLOOR:
+                # Emits its bundled static tier regardless; that floor is the
+                # whole point of the two-tier design.
+                if not out:
+                    fail(f"{rel} [{label}]: emitted nothing — the static "
+                         f"behavioural floor must survive having no credentials")
+                else:
+                    ok(f"{rel} [{label}]: exit 0, static floor present")
+            elif out:
+                fail(f"{rel} [{label}]: emitted output with no working instance:\n"
+                     f"        {out[:200]}")
+            else:
+                ok(f"{rel} [{label}]: exit 0, silent")
+
+
+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 first — `git fetch --depth=1 origin main:refs/remotes/"
+            f"origin/main` is enough, since this diffs two trees and needs no "
+            f"common ancestor — or pass --no-version 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()
+    check_shellcheck()
+    check_fail_open()
+    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())