diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index e851ac5..fc4986c 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -129,6 +129,19 @@ jobs: # 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 diff --git a/ci-requirements.md b/ci-requirements.md index 3f7aad2..217dbf4 100644 --- a/ci-requirements.md +++ b/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 diff --git a/scripts/check_plugin.py b/scripts/check_plugin.py index c67c92f..9fc42e2 100755 --- a/scripts/check_plugin.py +++ b/scripts/check_plugin.py @@ -17,13 +17,18 @@ 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. +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 @@ -33,7 +38,9 @@ from __future__ import annotations import argparse import json +import os import re +import shutil import subprocess import sys from pathlib import Path @@ -59,6 +66,12 @@ 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")) @@ -132,6 +145,114 @@ def check_patterns() -> None: # --- 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 @@ -221,6 +342,8 @@ def main() -> int: check_syntax() check_patterns() + check_shellcheck() + check_fail_open() if not args.no_version: check_version_bump(args.base)