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

Merged

bvandeusen merged 4 commits from dev into main 2026-07-29 08:55:58 -04:00

Conversation 0 Commits 4 Files Changed 6

+441 -12

bvandeusen commented 2026-07-29 08:55:50 -04:00

Owner

Copy Link

Copy Source

Closes #2204. Plugin manifest 0.1.19 → 0.1.20.

plugin/ isn't built into the image — installs fetch it from this repo, so a push is the release. It was absent from the workflow's paths: filter entirely, so plugin changes ran no CI at all. Two defects reached a live install through that gap: #2198 (three broken hook scripts) and #2209 (the fix for #2198 unable to reach an install because the manifest version wasn't bumped).

The gate

New plugin job running scripts/check_plugin.py:

1. bash -n on every hook.
2. shellcheck, plus targeted patterns for the three #2198 bug classes — lowercase CLAUDE_PLUGIN_OPTION_, line-oriented jq -R without -s, | cut -c as a payload cap. Verified by replay against c569cdd^: all three caught.
3. Fail-open smoke test — each hook, with no credentials and against a refused connection, must exit 0. Three must stay silent; scribe_session_context.sh must not, because its static behavioural floor is designed to survive having no credentials. Verified it can fail: injected a non-zero exit and a stray stdout write, each caught.
4. Version bump — shipped plugin content differing from origin/main requires the manifest version to differ too. Stated against the base branch, so a batch needs one bump rather than one per commit. Replayed against c569cdd: correctly fails.

It found a live bug on first run

scribe_autoinject.sh still capped the prompt with cut -c1-2000 — line-oriented, so never actually a cap. Only the prior-art hook's copy was fixed in c569cdd. Now head -c. This is a real user-facing fix, not just CI: it affects long multi-line prompts.

It then failed on its own commit for a missing version bump — the third time that rule has mattered and the first time anything but memory enforced it.

Tooling

jq + shellcheck are per-job apt installs, not an image change, per CI-runner's docs/process.md: "If only one project needs the dep, prefer that project installing it per-job." Promotion case filed as CI-runner issue #2 rather than assumed. jq is load-bearing rather than convenient — every hook opens command -v jq || exit 0, so without it the smoke test would pass while exercising nothing.

ci-requirements.md updated, including two stale entries corrected (it claimed four jobs when there are six, and listed uv as per-job when it's been in the image for some time).

Known gaps

• Nothing verifies the delivery path. CI can now catch a broken hook and a missing bump, but nothing checks that a fix actually reaches an install. That's the failure mode that bit twice and it's still only caught by a human noticing.
• The with:-block checkout failure on the act_runner (run 3027) is recorded as an observation, not a rule — one occurrence can't distinguish a with:-specific bug from a concurrency race over the action cache.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

Closes #2204. Plugin manifest 0.1.19 → **0.1.20**. `plugin/` isn't built into the image — installs fetch it from this repo, so a push *is* the release. It was absent from the workflow's `paths:` filter entirely, so plugin changes ran no CI at all. Two defects reached a live install through that gap: #2198 (three broken hook scripts) and #2209 (the fix for #2198 unable to reach an install because the manifest version wasn't bumped). ## The gate New `plugin` job running `scripts/check_plugin.py`: 1. `bash -n` on every hook. 2. **shellcheck**, plus targeted patterns for the three #2198 bug classes — lowercase `CLAUDE_PLUGIN_OPTION_`, line-oriented `jq -R` without `-s`, `| cut -c` as a payload cap. Verified by replay against `c569cdd^`: all three caught. 3. **Fail-open smoke test** — each hook, with no credentials and against a refused connection, must exit 0. Three must stay silent; `scribe_session_context.sh` must not, because its static behavioural floor is designed to survive having no credentials. Verified it can fail: injected a non-zero exit and a stray stdout write, each caught. 4. **Version bump** — shipped plugin content differing from `origin/main` requires the manifest version to differ too. Stated against the base branch, so a batch needs one bump rather than one per commit. Replayed against `c569cdd`: correctly fails. ## It found a live bug on first run `scribe_autoinject.sh` still capped the prompt with `cut -c1-2000` — line-oriented, so never actually a cap. Only the prior-art hook's copy was fixed in `c569cdd`. Now `head -c`. **This is a real user-facing fix, not just CI**: it affects long multi-line prompts. It then failed on its own commit for a missing version bump — the third time that rule has mattered and the first time anything but memory enforced it. ## Tooling `jq` + `shellcheck` are per-job apt installs, not an image change, per CI-runner's `docs/process.md`: *"If only one project needs the dep, prefer that project installing it per-job."* Promotion case filed as CI-runner issue #2 rather than assumed. `jq` is load-bearing rather than convenient — every hook opens `command -v jq || exit 0`, so without it the smoke test would pass while exercising nothing. `ci-requirements.md` updated, including two stale entries corrected (it claimed four jobs when there are six, and listed `uv` as per-job when it's been in the image for some time). ## Known gaps - **Nothing verifies the delivery path.** CI can now catch a broken hook and a missing bump, but nothing checks that a fix actually reaches an install. That's the failure mode that bit twice and it's still only caught by a human noticing. - The `with:`-block checkout failure on the act_runner (run 3027) is recorded as an observation, **not** a rule — one occurrence can't distinguish a `with:`-specific bug from a concurrency race over the action cache. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

bvandeusen added 4 commits 2026-07-29 08:55:50 -04:00

ci(plugin): gate plugin/** — the path that shipped two live defects ... 51e5c22818

`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 against
   c569cdd: 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 in c569cdd. 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

ci(plugin): drop with: from the plugin job's checkout ... 1feef179d2

Run 3027: the Plugin hooks job failed at checkout, before the script ran.
Adding a `with: fetch-depth: 0` block made actions/checkout@v6 fail to
extract on the act_runner —

  Cannot find module '/var/run/act/actions/<sha>/dist/index.js'

— while every bare `uses: actions/checkout@v6` in the same run succeeded.
The runner's action-cache handling is the difference, not git.

No depth was needed in the first place. The version check compares two
TREES, and a tree diff needs both trees, not a common ancestor. Verified
against a real depth-1 clone: after `git fetch --depth=1 origin
main:refs/remotes/origin/main`, both `git diff origin/main -- plugin` and
`git show origin/main:plugin/.claude-plugin/plugin.json` work. So the
explicit fetch already in the step is sufficient, and cheaper than the
full history the `with:` block was asking for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

ci(plugin): add shellcheck + jq, and the fail-open smoke test they unlock ... dd878bc498

Per-job installs, not an image change. CI-runner's docs/process.md decision
checkpoint is explicit: "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, and shellcheck is not a
natural extension of a Python image's purpose. Promotion into ci-python is
filed as an issue on CI-runner rather than assumed here — same doc, step 1:
the maintainer's call goes in the issue, then the PR.

This also corrects something I got wrong earlier in this work: I cited rule
#5 as blocking a per-job install. Rule #5 is about language TOOLCHAINS via
setup-* actions, not small lint utilities, and CI-runner's own process doc
positively recommends per-job installs in exactly this case.

jq is load-bearing rather than convenient. Every hook opens with
`command -v jq || exit 0`, so without it a "runs and stays silent" smoke
test passes while exercising nothing — a green tick proving less than no
test at all. That is why the smoke test didn't ship with the first cut.

The smoke test pins the fail-open contract: each hook, with no credentials
and then against a refused connection, must exit 0. Three must also stay
silent; scribe_session_context.sh must NOT, because its static behavioural
floor is meant to survive having no credentials and no network — asserting
silence there would encode the opposite of the design.

Verified it can actually fail, rather than assuming: injected a non-zero
exit and separately a stray stdout write, and confirmed each is caught.

shellcheck and jq are both optional at runtime — missing either SKIPs its
check loudly rather than passing. A check that quietly no-ops is the exact
failure mode this file exists to prevent.

ci-requirements.md updated: jq + shellcheck recorded under per-job installs
(the input CI-runner's maintainer uses for the next promotion decision),
plus two stale entries corrected — the sheet claimed four jobs when there
are six, and listed `uv` as a per-job install when it has been in the image
since the ci-python Dockerfile started pip-installing it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

fix(plugin): justify the one shellcheck finding (SC1007, false positive) ... b0a0bf8abd

First CI run with shellcheck (run 3029) flagged exactly one thing:

  scribe_session_context.sh:44
  SC1007 Remove space after = if trying to assign a value
  here=$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)

`CDPATH= cd` is the deliberate POSIX idiom for running a single command
with CDPATH empty — it stops `cd` resolving through the operator's CDPATH
and echoing the resolved path into our stdout, which for a hook whose
stdout IS its protocol would be a real bug. shellcheck cannot distinguish
that from a typo'd `CDPATH=cd`, so this is a false positive.

Scoped `# shellcheck disable=SC1007` with the reason above it, matching
how CI-runner's own scripts/install-common.sh handles SC2086. One
line-scoped disable, no file-level or blanket suppression — a lint you
silence broadly stops being a lint.

Everything else in that run passed, including the parts that could only
run once jq was installed: all four hooks exit 0 and stay silent
unconfigured and against a refused connection, with the session-context
hook correctly still emitting its static floor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

bvandeusen merged commit 327b8a99a4 into main 2026-07-29 08:55:58 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledScribe#84