From d6868342726c52a17b5863efca1842eccbd01469 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 01:01:22 +0000 Subject: [PATCH 01/21] refactor(claude-ops): resolve the skill-usage store through one policy for writer and reader The pair-cooccurrence reader hard-coded the repo-scope store path and read neither skill_usage_scope nor skill_usage_dir, so under the user or data-dir scope the hook wrote where the reader never looked. The reader now sources claude-ops-paths.sh from its own plugin root and calls the same claude_ops::resolve_skill_usage_dir the writer uses, with --scope, --dir, --data-root and --print-store; --store stays as the explicit override, and a missing store names the scope. The data-dir scope requires --data-root and never falls back to an inherited CLAUDE_PLUGIN_DATA. Deepening candidate 12 of the architecture review. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011SQkHipoF2M8rTtnkbFKKP --- .../skills/audit-skill-visibility/SKILL.md | 22 +++ .../reference/pair-cooccurrence.md | 39 ++++- .../scripts/skill-pair-cooccurrence.sh | 155 ++++++++++++++++-- .../scripts/skill-pair-cooccurrence.test.sh | 132 +++++++++++++++ 4 files changed, 334 insertions(+), 14 deletions(-) diff --git a/plugins/claude-ops/skills/audit-skill-visibility/SKILL.md b/plugins/claude-ops/skills/audit-skill-visibility/SKILL.md index 8540444111..bcb7bead1a 100644 --- a/plugins/claude-ops/skills/audit-skill-visibility/SKILL.md +++ b/plugins/claude-ops/skills/audit-skill-visibility/SKILL.md @@ -136,6 +136,28 @@ the same engine. It is not needed to get a report. Python 3.11+ is the only requirement. No third-party packages, matching `inventory.py` and `install_state.py`. +### The skill-usage store the hooks write + +The hooks write `skill-usage.jsonl` where the `skill_usage_scope` and `skill_usage_dir` options +say. Neither script here guesses that location: `audit_skill_visibility.py` takes it as +`--skill-usage`, and `scripts/skill-pair-cooccurrence.sh` resolves it through the resolver the +hooks themselves call. Hand it the rendered option values (a skill subprocess inherits no +`CLAUDE_PLUGIN_OPTION_*` mirror), resolve once, pass the one path to both: + +```bash +STORE="$(bash "${CLAUDE_PLUGIN_ROOT}/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.sh" \ + --scope "${user_config.skill_usage_scope}" --dir "${user_config.skill_usage_dir}" --print-store)" +python3 "${CLAUDE_PLUGIN_ROOT}/skills/audit-skill-visibility/scripts/audit_skill_visibility.py" \ + --skill-usage "$STORE" +bash "${CLAUDE_PLUGIN_ROOT}/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.sh" \ + --store "$STORE" --pair , +``` + +An empty or unrendered option value reads as its default, as it does in the hooks. The +`data-dir` scope also needs `--data-root `, never taken +from `CLAUDE_PLUGIN_DATA`. Flags, that reason, and missing-store behavior: +[reference/pair-cooccurrence.md](reference/pair-cooccurrence.md). + ## Reading the output Three independent fields per skill; a single flat verdict would collapse diff --git a/plugins/claude-ops/skills/audit-skill-visibility/reference/pair-cooccurrence.md b/plugins/claude-ops/skills/audit-skill-visibility/reference/pair-cooccurrence.md index c7098835a8..259234613f 100644 --- a/plugins/claude-ops/skills/audit-skill-visibility/reference/pair-cooccurrence.md +++ b/plugins/claude-ops/skills/audit-skill-visibility/reference/pair-cooccurrence.md @@ -12,14 +12,47 @@ scripts/skill-pair-cooccurrence.sh --pair a:b,c:d --json # machine-readable | Flag | Meaning | |---|---| -| `--store PATH` | store to read; defaults to `/.claude/observability/skill-usage.jsonl` | +| `--store PATH` | store to read; an explicit override that skips the scope resolution below | +| `--scope SCOPE` | the plugin's `skill_usage_scope` option: `repo` (default), `user`, or `data-dir` | +| `--dir REL` | the plugin's `skill_usage_dir` option, a contained relative directory under the scope root (default `.claude/observability`; `data-dir` ignores it) | +| `--data-root PATH` | the plugin data root the hooks write under; required by the `data-dir` scope, ignored by the others | +| `--print-store` | print the resolved store path and exit; hand it to `audit_skill_visibility.py --skill-usage` so both scripts read one store | | `--pair A,B` | ordered pair, caller first | | `--floor-days N` | minimum observed span before any rate is reportable (default 30) | | `--floor-groups N` | minimum caller-bearing groups before any rate is reportable (default 5) | | `--json` | one JSON object instead of prose | -Exit `0` for a reading (verdict **or** withheld), `2` for a missing/unreadable store, `3` for -bad arguments. +Exit `0` for a reading (verdict **or** withheld) or a printed store path, `2` for a missing or +unreadable store or a destination that cannot be resolved, `3` for bad arguments. + +## The default store is the writer's store + +Without `--store` the script sources the hooks' own resolver +(`hooks/claude-ops-paths.sh`, `claude_ops::resolve_skill_usage_dir`) and gives it the same +three inputs the writers use: the scope, the relative dir, and the project root +(`CLAUDE_PROJECT_DIR`, else the working directory). So the file it opens is the file the hooks +wrote, in every scope. A default restated here would be one branch of that policy, correct +only until the policy moved. + +The hooks read their options from the `CLAUDE_PLUGIN_OPTION_*` mirrors in the hook +environment; a skill subprocess inherits none of those. The skill body therefore passes the +rendered `${user_config.skill_usage_scope}` and `${user_config.skill_usage_dir}` values through +`--scope` and `--dir`. An empty or unrendered value reads as the option's default, the same rule +the hooks apply. An unknown scope falls back to `repo` with a notice on stderr, again as the +hooks do, so the reader lands on the store they wrote rather than refusing it. + +The `data-dir` root is the one input with no default. A skill subprocess can inherit an +unrelated plugin's `CLAUDE_PLUGIN_DATA` +([plugin-data keying convention](https://github.com/melodic-software/claude-code-plugins/blob/main/docs/conventions/plugin-data-report-keying/README.md) +rule 2), so this script never reads that variable and the sibling pruner +`skills/observability/scripts/clean.sh` refuses to either. Pass the claude-ops plugin data +directory through `--data-root`; without it the `data-dir` scope exits `2` rather than reading a +path no writer chose. + +A missing store names the scope it was looked for in (`(scope user)`), because a store written +under one scope and read under another is the other common reason for that message. An +unresolvable destination (a traversal `--dir`, `data-dir` with no data root) exits `2` with the +scope named and is never reported as "nothing observed". ## It is a proxy — do not strip the caveat diff --git a/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.sh b/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.sh index 2faeca72ad..2b1ffb2f2b 100755 --- a/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.sh +++ b/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.sh @@ -28,9 +28,27 @@ # rather than a small number: a store younger than the exposure floor cannot # distinguish "CALLEE never fired" from "nothing was observed yet". # +# WHERE THE STORE IS — the hooks decide, this script asks them: +# +# The writers select the store through claude_ops::resolve_skill_usage_dir +# in ../../../hooks/claude-ops-paths.sh (skill_usage_scope: repo, user or +# data-dir; skill_usage_dir under the scope root). Without --store this script +# sources that same resolver and feeds it the same options, so the file it +# opens is the file the hooks wrote for every scope. A restated default here +# would be one branch of that policy, correct only until the policy moved. +# The hooks read their options from CLAUDE_PLUGIN_OPTION_* in the hook +# environment; a skill subprocess inherits none of those, so the skill body +# passes the rendered ${user_config.*} values through --scope / --dir. The +# data-dir root arrives the same way, through --data-root: a skill +# subprocess was observed carrying an UNRELATED plugin's CLAUDE_PLUGIN_DATA +# (docs/conventions/plugin-data-report-keying/README.md rule 2), so this +# script never reads that variable and the sibling pruner +# (skills/observability/scripts/clean.sh) refuses to either. +# # Exit: -# 0 a reading was produced — a VERDICT or an honest WITHHELD -# 2 the store is missing or unreadable +# 0 a reading was produced — a VERDICT or an honest WITHHELD — or, with +# --print-store, the resolved store path was printed +# 2 the store is missing, unreadable, or its destination cannot be resolved # 3 invoked with bad arguments set -uo pipefail @@ -38,11 +56,35 @@ EX_OK=0 EX_NO_STORE=2 EX_USAGE=3 -USAGE='usage: skill-pair-cooccurrence.sh [--store PATH] [--pair CALLER,CALLEE] - [--floor-days N] [--floor-groups N] [--json] +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# The plugin is cache-isolated and the hooks ship inside it, so the resolver is +# reachable by a path relative to this script: skills//scripts -> hooks. +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" + +DEFAULT_SCOPE="repo" +DEFAULT_REL_DIR=".claude/observability" +STORE_FILE="skill-usage.jsonl" + +# shellcheck disable=SC2016 # the ${...} tokens are literal help text, never expansions +USAGE='usage: skill-pair-cooccurrence.sh [--store PATH | --scope SCOPE --dir REL --data-root PATH] + [--pair CALLER,CALLEE] [--floor-days N] + [--floor-groups N] [--json] [--print-store] - --store PATH skill-usage.jsonl to read. Default: the repo-scope store, - /.claude/observability/skill-usage.jsonl + --store PATH skill-usage.jsonl to read; an explicit override that skips the + scope resolution below + --scope SCOPE skill_usage_scope the hooks write under: repo (default), user, + or data-dir. Resolved by the resolver the hooks themselves + call, so the store read is the store written. Empty or an + unrendered ${user_config.*} placeholder reads as the default, + as it does in the hooks + --dir REL skill_usage_dir: the contained relative directory under the + scope root (default .claude/observability; ignored by data-dir) + --data-root PATH the plugin data root the hooks write under. REQUIRED by the + data-dir scope and ignored by the others; never read from + CLAUDE_PLUGIN_DATA, which a skill subprocess was observed + carrying for an unrelated plugin + --print-store print the resolved store path and exit 0. Hand it to + audit_skill_visibility.py --skill-usage so both read ONE store --pair A,B ordered pair. Default: implementation:implement,tdd:principles --floor-days N minimum observed span before any rate is reportable (default 30, matching audit_skill_visibility.py exposure_floor_days) @@ -55,7 +97,16 @@ die_usage() { exit "$EX_USAGE" } +# An option the skill body passed through unrendered (`${user_config.x}`) or +# empty is an unset option, and reads as its default. +# shellcheck disable=SC2016 # the literal placeholder text is the thing matched +unset_value() { [[ -z "$1" || "$1" == '${user_config.'* ]]; } + STORE="" +SCOPE="" +REL_DIR="" +DATA_ROOT="" +PRINT_STORE=0 PAIR="implementation:implement,tdd:principles" FLOOR_DAYS=30 FLOOR_GROUPS=5 @@ -72,6 +123,25 @@ while [[ $# -gt 0 ]]; do STORE="$2" shift 2 ;; + --scope) + [[ $# -ge 2 ]] || die_usage "--scope needs a value" + SCOPE="$2" + shift 2 + ;; + --dir) + [[ $# -ge 2 ]] || die_usage "--dir needs a value" + REL_DIR="$2" + shift 2 + ;; + --data-root) + [[ $# -ge 2 ]] || die_usage "--data-root needs a value" + DATA_ROOT="$2" + shift 2 + ;; + --print-store) + PRINT_STORE=1 + shift + ;; --pair) [[ $# -ge 2 ]] || die_usage "--pair needs a value" PAIR="$2" @@ -111,15 +181,78 @@ command -v jq >/dev/null 2>&1 || { exit "$EX_NO_STORE" } -if [[ -z "$STORE" ]]; then - TOPLEVEL="$(git rev-parse --show-toplevel 2>/dev/null || printf '.')" - STORE="$TOPLEVEL/.claude/observability/skill-usage.jsonl" +# Resolve the store the way the writers do. Sets STORE and STORE_ORIGIN (the +# phrase the missing-store message names, so a wrong-scope run says which scope +# it looked in). Exits 2 when the destination itself cannot be resolved: that +# is a configuration answer, not "nothing observed". +resolve_store_from_scope() { + local project_dir store_dir rc + # shellcheck source=../../../hooks/hook-utils.sh + . "$PLUGIN_ROOT/hooks/hook-utils.sh" + # shellcheck source=../../../hooks/claude-ops-paths.sh + . "$PLUGIN_ROOT/hooks/claude-ops-paths.sh" + + unset_value "$SCOPE" && SCOPE="$DEFAULT_SCOPE" + unset_value "$REL_DIR" && REL_DIR="$DEFAULT_REL_DIR" + # DATA_ROOT has no environment fallback on purpose: an inherited + # CLAUDE_PLUGIN_DATA in a skill subprocess can name another plugin's data + # directory, so an unpassed --data-root is an unanswerable data-dir scope + # rather than a guess at one. + unset_value "$DATA_ROOT" && DATA_ROOT="" + case "$SCOPE" in + repo | user | data-dir) ;; + *) + # The same fallback the writers apply to an unknown scope, so the reader + # still lands on the file they wrote. + printf 'skill-pair-cooccurrence.sh: unknown skill_usage_scope "%s" (valid: repo, user, data-dir); reading the default %s scope, as the hooks write to it\n' \ + "$SCOPE" "$DEFAULT_SCOPE" >&2 + SCOPE="$DEFAULT_SCOPE" + ;; + esac + + # Same project root the writers key on: CLAUDE_PROJECT_DIR in a hook or + # skill subprocess, the working directory otherwise. An unresolved root + # (not a git checkout) falls back to the hint, as it does for the writers. + project_dir=$(hook::repo_root "${CLAUDE_PROJECT_DIR:-.}") || true + store_dir=$(CLAUDE_PLUGIN_DATA="$DATA_ROOT" claude_ops::resolve_skill_usage_dir "$SCOPE" "$project_dir" "$REL_DIR") + rc=$? + case "$rc" in + 0) ;; + 1) + printf 'skill-pair-cooccurrence.sh: the skill-usage destination is invalid for scope "%s": skill_usage_dir "%s" must be a contained relative path (no absolute, drive, UNC, traversal, or escaping symlink path), so the hooks write nothing there either\n' \ + "$SCOPE" "$REL_DIR" >&2 + exit "$EX_NO_STORE" + ;; + *) + if [[ "$SCOPE" == "data-dir" ]]; then + printf 'skill-pair-cooccurrence.sh: scope "data-dir" needs the plugin data root: pass --data-root . It is not taken from CLAUDE_PLUGIN_DATA, which a skill subprocess can carry for an unrelated plugin\n' >&2 + else + printf 'skill-pair-cooccurrence.sh: scope "%s" needs HOME to name an existing directory\n' "$SCOPE" >&2 + fi + exit "$EX_NO_STORE" + ;; + esac + STORE="${store_dir}/${STORE_FILE}" + STORE_ORIGIN="scope ${SCOPE}" +} + +if [[ -n "$STORE" ]]; then + STORE_ORIGIN="explicit --store" +else + resolve_store_from_scope +fi + +if ((PRINT_STORE)); then + printf '%s\n' "$STORE" + exit "$EX_OK" fi if [[ ! -r "$STORE" ]]; then # Absent store is not a crash: it is the commonest state on a fresh install, - # and the honest answer is "nothing observed", said out loud. - printf 'skill-pair-cooccurrence.sh: no readable skill-usage store at %s\n' "$STORE" >&2 + # and the honest answer is "nothing observed", said out loud, with the scope + # it was said about — a store written under another scope is the other + # common reason for this branch. + printf 'skill-pair-cooccurrence.sh: no readable skill-usage store at %s (%s)\n' "$STORE" "$STORE_ORIGIN" >&2 printf 'Nothing has been observed. This is the normal state before the claude-ops skill-usage hooks have run in this repo; it is not evidence about %s or %s.\n' \ "$CALLER" "$CALLEE" >&2 exit "$EX_NO_STORE" diff --git a/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.test.sh b/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.test.sh index a5d301d36d..907c5010f6 100755 --- a/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.test.sh +++ b/plugins/claude-ops/skills/audit-skill-visibility/scripts/skill-pair-cooccurrence.test.sh @@ -18,6 +18,13 @@ # - a malformed row is skipped, not fatal # - a record with no project_id/branch still groups instead of vanishing # - argument validation exits 3; an absent store exits 2 and says so +# - without --store the reader opens the file the hooks WROTE, for each of +# the three skill_usage_scope values, driven through the shared resolver +# (the real writer hook runs, then the reader is pointed at the same +# options); an unrendered ${user_config.*} placeholder reads as the +# default; an unknown scope falls back to repo exactly as the writer does; +# an unresolvable destination exits 2 and a missing store names its scope; +# an inherited CLAUDE_PLUGIN_DATA never stands in for --data-root # # Every case runs in THIS shell, never a `( … )` subshell: an assertion inside a # subshell increments a copy of the failure counter, and the run would report @@ -27,6 +34,8 @@ set -uo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" SUT="$SCRIPT_DIR/skill-pair-cooccurrence.sh" +PLUGIN_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" +WRITER="$PLUGIN_ROOT/hooks/skill-usage-audit.sh" FAILED=0 CASE_NUM=0 @@ -230,6 +239,129 @@ assert_status "an unknown flag exits 3" "$?" 3 err="$(bash "$SUT" --store "$TMP/does-not-exist.jsonl" 2>&1)" assert_status "an absent store exits 2" "$?" 2 assert_contains "…and says nothing was observed rather than implying a zero" "$err" "Nothing has been observed" +assert_contains "…and names the explicit override as where it looked" "$err" "(explicit --store)" + +# --- the default store is the writer's store, in every scope ----------------- +# The writer (the skill-usage-audit hook) selects its destination through +# claude_ops::resolve_skill_usage_dir; the reader must open that same file for +# each scope, not a restated copy of one branch. Each case runs the REAL writer +# with the hook-environment option mirrors, then the reader with the rendered +# option values the skill body passes, and asserts the two meet: the reader's +# --print-store answer is the resolver's answer, and the reader sees the row. +# HOME, CLAUDE_PROJECT_DIR and CLAUDE_PLUGIN_DATA are all isolated under $TMP, +# so a store on this machine cannot stand in for the one the case wrote. +# shellcheck source=../../../hooks/hook-utils.sh +. "$PLUGIN_ROOT/hooks/hook-utils.sh" +# shellcheck source=../../../hooks/claude-ops-paths.sh +. "$PLUGIN_ROOT/hooks/claude-ops-paths.sh" + +PROJECT="$TMP/project" +FAKE_HOME="$TMP/home" +DATA_ROOT="$TMP/plugin-data" +mkdir -p "$PROJECT" "$FAKE_HOME" "$DATA_ROOT" +git -C "$PROJECT" init -q 2>/dev/null || true +WRITE_INPUT='{"tool_name":"Skill","tool_input":{"skill":"/tdd:principles"}}' + +# run_writer : one SkillUse row through the real hook. +run_writer() { + env -u HOOK_TELEMETRY_SINK \ + HOME="$FAKE_HOME" CLAUDE_PROJECT_DIR="$PROJECT" CLAUDE_PLUGIN_DATA="$DATA_ROOT" \ + CLAUDE_PLUGIN_OPTION_SKILL_USAGE_SCOPE="$1" CLAUDE_PLUGIN_OPTION_SKILL_USAGE_DIR="$2" \ + bash "$WRITER" <<<"$WRITE_INPUT" >/dev/null 2>&1 +} + +# run_reader : the reader in the skill-subprocess shape (no +# CLAUDE_PLUGIN_OPTION_* mirrors, no CLAUDE_PLUGIN_DATA), options by flag. +run_reader() { + env -u CLAUDE_PLUGIN_DATA -u CLAUDE_PLUGIN_OPTION_SKILL_USAGE_SCOPE -u CLAUDE_PLUGIN_OPTION_SKILL_USAGE_DIR \ + HOME="$FAKE_HOME" CLAUDE_PROJECT_DIR="$PROJECT" \ + bash "$SUT" "$@" +} + +# expected_store : the writer's destination, from the resolver. +expected_store() { + local dir + dir="$(HOME="$FAKE_HOME" CLAUDE_PLUGIN_DATA="$DATA_ROOT" claude_ops::resolve_skill_usage_dir "$1" "$PROJECT" "$2")" || return 1 + printf '%s/skill-usage.jsonl' "$dir" +} + +for scope in repo user data-dir; do + rel_dir="telemetry/skills" + run_writer "$scope" "$rel_dir" + expected="$(expected_store "$scope" "$rel_dir")" + if [[ -s "$expected" ]]; then + pass "writer wrote the $scope-scope store where the resolver says" + else + fail "writer wrote the $scope-scope store where the resolver says" "no store at $expected" + fi + case "$scope" in + data-dir) resolved="$(run_reader --scope "$scope" --dir "$rel_dir" --data-root "$DATA_ROOT" --print-store)" ;; + *) resolved="$(run_reader --scope "$scope" --dir "$rel_dir" --print-store)" ;; + esac + if [[ "$resolved" == "$expected" ]]; then + pass "reader resolves the $scope-scope store through the shared resolver" + else + fail "reader resolves the $scope-scope store through the shared resolver" "expected $expected, got $resolved" + fi + case "$scope" in + data-dir) jout="$(run_reader --scope "$scope" --dir "$rel_dir" --data-root "$DATA_ROOT" --pair tdd:principles,x --json)" ;; + *) jout="$(run_reader --scope "$scope" --dir "$rel_dir" --pair tdd:principles,x --json)" ;; + esac + assert_contains "reader reads the row the writer put in the $scope-scope store" "$jout" '"events_read":1' +done + +# The rendered option values arrive unset in two shapes, and both mean "the +# default": the writer resolves an unset option to .claude/observability under +# the repo, so the reader must land there too. +run_writer "" "" +expected="$(expected_store repo .claude/observability)" +# shellcheck disable=SC2016 # the literal placeholder text is the input under test +resolved="$(run_reader --scope '${user_config.skill_usage_scope}' --dir '${user_config.skill_usage_dir}' --print-store)" +if [[ "$resolved" == "$expected" ]]; then + pass "an unrendered \${user_config.*} placeholder reads as the default scope and dir" +else + fail "an unrendered \${user_config.*} placeholder reads as the default scope and dir" "expected $expected, got $resolved" +fi +resolved="$(run_reader --scope '' --dir '' --print-store)" +if [[ "$resolved" == "$expected" ]]; then + pass "empty --scope and --dir read as the default scope and dir" +else + fail "empty --scope and --dir read as the default scope and dir" "expected $expected, got $resolved" +fi + +# An unknown scope: the writer falls back to repo with an advisory, so the +# reader does the same, and says so, rather than refusing the store it wrote. +err="$(run_reader --scope bogus --print-store 2>&1 >/dev/null)" +resolved="$(run_reader --scope bogus --print-store 2>/dev/null)" +if [[ "$resolved" == "$expected" ]]; then + pass "an unknown scope falls back to the repo store, as the writer does" +else + fail "an unknown scope falls back to the repo store, as the writer does" "expected $expected, got $resolved" +fi +assert_contains "…and the fallback is said out loud" "$err" 'unknown skill_usage_scope "bogus"' + +# An unresolvable destination is a configuration answer, not "nothing observed". +err="$(run_reader --scope user --dir ../outside 2>&1)" +assert_status "a traversal dir exits 2" "$?" 2 +assert_contains "…naming the scope it failed in" "$err" 'invalid for scope "user"' +assert_not_contains "…and never claims nothing was observed" "$err" "Nothing has been observed" +err="$(run_reader --scope data-dir 2>&1)" +assert_status "data-dir without a data root exits 2" "$?" 2 +assert_contains "…naming the scope and the missing root" "$err" 'scope "data-dir" needs the plugin data root' +# An inherited CLAUDE_PLUGIN_DATA in a skill subprocess can name ANOTHER +# plugin's data directory, so it is not a substitute for --data-root: the run +# stops rather than reading a path no claude-ops writer chose. +err="$(env HOME="$FAKE_HOME" CLAUDE_PROJECT_DIR="$PROJECT" CLAUDE_PLUGIN_DATA="$TMP/other-plugin" \ + bash "$SUT" --scope data-dir 2>&1)" +assert_status "an inherited CLAUDE_PLUGIN_DATA does not stand in for --data-root" "$?" 2 +assert_not_contains "…and no other plugin's data directory is resolved" "$err" "other-plugin" + +# A store missing in the scope asked about names that scope: a store written +# under one scope and read under another is the case this line exists for. +err="$(run_reader --scope user --dir never/written 2>&1)" +assert_status "a store missing in the named scope exits 2" "$?" 2 +assert_contains "…and the message names the scope it looked in" "$err" "(scope user)" +assert_contains "…and still says nothing was observed" "$err" "Nothing has been observed" printf '\n%d case(s), %d failure(s)\n' "$CASE_NUM" "$FAILED" [[ "$FAILED" -eq 0 ]] From 28db30fe64e735572fd8524fdeb8f398e6d9907d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 01:08:27 +0000 Subject: [PATCH 02/21] feat(ci): prove every continue-on-error gate step is fed to the ci-status aggregate After the six-job collapse the lanes live as steps, and the lane-coverage gate still proved reachability for jobs only, so a gate step whose id was left out of the aggregator feed turned nothing red. The gate now asserts set equality at both levels from one awk pass: jobs against ci-status.needs, and per job the steps carrying continue-on-error with an id against the ids the feed reads, minus the opt-outs declared in scripts/lane-coverage-step-opt-outs.txt (the resolver's own detect and match steps). Both directions fail: an unfed gate, an unreadable gate with no id, and a dangling feed row. The opt-out list is read through scripts/lib/read-list.sh. Deepening candidate 15 of the architecture review. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011SQkHipoF2M8rTtnkbFKKP --- .github/workflows/ci.yml | 2 +- scripts/check-lane-coverage.sh | 321 ++++++++++++++++++++++-- scripts/check-lane-coverage.test.sh | 256 +++++++++++++++++-- scripts/lane-coverage-step-opt-outs.txt | 24 ++ 4 files changed, 563 insertions(+), 40 deletions(-) create mode 100644 scripts/lane-coverage-step-opt-outs.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 46549875c7..249e8a398a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1027,7 +1027,7 @@ jobs: - name: Test the lane-coverage gate if: needs.changes.outputs.run_shell == 'true' run: bash scripts/check-lane-coverage.test.sh - - name: Check every ci.yml job is reachable from the ci-status aggregate + - name: Check every ci.yml job and gate step is fed to the ci-status aggregate id: lane_coverage continue-on-error: true run: scripts/check-lane-coverage.sh --check diff --git a/scripts/check-lane-coverage.sh b/scripts/check-lane-coverage.sh index 39083d4eb2..0b08a08929 100755 --- a/scripts/check-lane-coverage.sh +++ b/scripts/check-lane-coverage.sh @@ -1,10 +1,12 @@ #!/usr/bin/env bash -# Gate: every job defined in the CI workflow must be reachable from the required -# aggregate's `needs` graph, or carry a written reason for staying out of it. +# Gate: every lane defined in the CI workflow must be able to turn the required +# aggregate red, at BOTH levels the lanes live at, or carry a written reason +# for staying out of it. # -# scripts/check-lane-coverage.sh --check [ []] +# scripts/check-lane-coverage.sh --check [ [ []]] # -# Defaults: .github/workflows/ci.yml and the `ci-status` aggregate. +# Defaults: .github/workflows/ci.yml, the `ci-status` aggregate, and +# scripts/lane-coverage-step-opt-outs.txt. # # WHY. `ci-status` is the single check the org ci-gate ruleset keys on, and its # own comment calls its `needs` list "the single source of truth for the lane @@ -20,7 +22,17 @@ # dead enforcement — with the added twist that the surface is the merge gate # itself. # -# WHAT IS CHECKED (all four directions, so the two sets are provably equal): +# The same shape exists one level down. Most lanes are no longer jobs: they are +# STEPS of one job, each carrying `continue-on-error: true` so that one failure +# does not hide the others, and an `id` so that an aggregator feed at the foot of +# the job can read `steps..outcome` and turn the job red on any non-success. +# `continue-on-error` absorbs the failure by design, so a gate step whose id the +# feed does not read fails silently: the job stays green, `ci-status` stays +# green, and the lane is decoration. The job-level check cannot see that, because +# the job IS in `needs`. So this gate proves set equality at step level too. +# +# WHAT IS CHECKED AT JOB LEVEL (all four directions, so the two sets are +# provably equal): # 1. UNGATED LANE — a job defined in the workflow, not in the aggregate's # `needs`, and not annotated. The class #2856 filed. # 2. DANGLING NEED — a `needs` entry naming no defined job. GitHub rejects @@ -34,9 +46,42 @@ # this gate exists to deny, so it fails rather than passing # as "annotated". # -# THE OPT-OUT. A job that legitimately does not belong in the required aggregate -# — advisory by design, or driven by an event the merge gate never sees — -# records that decision inline: +# WHAT IS CHECKED AT STEP LEVEL. A GATE STEP is a step carrying a literal +# `continue-on-error: true`; the AGGREGATOR FEED of a job is every row of the +# form `=${{ ... steps..outcome }}` inside a block scalar of that job +# (the `CHECK_RESULTS` env block of the aggregate step, in this repo). Within +# each job the set of gate steps, minus the declared opt-outs, must EQUAL the +# set of ids the feed reads: +# 5. UNFED GATE — a gate step with an id that no feed row in its job +# reads, and no opt-out. Its failure is absorbed and +# nothing turns red. The class this level exists for. +# 6. UNREADABLE GATE — a gate step with no `id` at all. It cannot be fed, so +# it is the same defect with nothing to point at. +# 7. DANGLING FEED — a feed row reading `steps..outcome` for an id no +# step in that job carries with `continue-on-error: +# true`. An undefined id evaluates to an empty string; +# an id on a step without the flag is a row the feed +# does not need and that would mask the flag's absence +# if one were added later. +# 8. STALE STEP OPT-OUT — a listed step that is not a gate step, or that the +# feed reads anyway. An exemption must not outlive what +# it excuses. +# 9. BARE STEP OPT-OUT — a listed step with no reason written beside it. +# Steps of a job that is itself annotated `# lane-coverage-ok:` are exempt from +# 5 and 6: an informational lane cannot gate a merge whatever its steps do. +# Check 7 applies everywhere, because a feed row that reads nothing is wrong in +# any job. +# +# A `steps..outcome` read OUTSIDE a block scalar — a step-level `if:`, say — +# is not a feed row. This gate models ONE aggregation shape, the one this +# workflow uses, and a gate step reachable only some other way reports UNFED +# rather than passing on a shape nobody checked. That is the fail-closed +# direction: a false alarm names a step and a fix, a missed one is the silent +# lane this gate exists to deny. +# +# THE JOB OPT-OUT. A job that legitimately does not belong in the required +# aggregate — advisory by design, or driven by an event the merge gate never +# sees — records that decision inline: # # # lane-coverage-ok: # some-job: @@ -51,8 +96,25 @@ # path exists so that a future advisory lane is a decision on the record instead # of an omission nobody notices. # -# The aggregate job itself is exempt by construction — it cannot depend on -# itself — via AGGREGATE below, not via a silent filter. +# THE STEP OPT-OUT. A step that carries `continue-on-error: true` for a reason +# other than aggregation (the docs-only resolver's own steps, whose failure must +# fall through to a fail-open default instead of failing their job) is listed in +# scripts/lane-coverage-step-opt-outs.txt, one entry per line: +# +# / +# +# The reason rides on the entry line, so it cannot drift away from what it +# excuses, and the list is checked in both directions (8 and 9 above), so it +# cannot rot into a silent allowlist. It is a list rather than the inline +# annotation the job level uses because a job key is a 2-space line with one +# possible meaning, while a step is a sequence item whose comment could sit above +# `- name:`, `uses:` or `with:` — attributing it would be exactly the +# shape-guessing this gate refuses to do. The list is parsed by the shared reader +# every scripts/*.txt list uses (scripts/lib/read-list.sh, `leading` mode), not +# by a private parser. +# +# The aggregate job itself is exempt from the job-level check by construction — +# it cannot depend on itself — via AGGREGATE below, not via a silent filter. # # FAIL CLOSED ON SHAPE. This reads the workflow structurally with awk rather # than through a YAML library (the repo ships no root YAML dependency, and the @@ -60,8 +122,12 @@ # org-owned standards materialization this repo does not edit). Every YAML shape # it does not recognize — a flow-sequence `needs: [a, b]`, a scalar `needs: x`, # an aggregate with no `needs:` at all, a 2-space key under `jobs:` that is not a -# plain job id — exits 2 (inconclusive), never 0. Returning an empty lane set on -# an unparsed file would be this gate committing the very defect it detects. +# plain job id, an expression-valued `continue-on-error`, a `steps..outcome` +# read inside a block scalar that is not a single-step feed row — exits 2 +# (inconclusive), never 0. Returning an empty lane set on an unparsed file would +# be this gate committing the very defect it detects. Block-scalar bodies are +# read as data, never as structure, so a script line can never be mistaken for +# a step key. # # Exit: 0 covered; 1 a coverage defect; 2 usage, missing file, or unrecognized # workflow shape. @@ -71,31 +137,92 @@ if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then echo "check-lane-coverage: not inside a git work tree" >&2 exit 2 fi +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || exit 2 cd "$(git rev-parse --show-toplevel)" || exit 2 +# shellcheck source=lib/read-list.sh +. "$SCRIPT_DIR/lib/read-list.sh" || exit 2 usage() { - echo "usage: $(basename "$0") --check [ []]" >&2 + echo "usage: $(basename "$0") --check [ [ []]]" >&2 exit 2 } [[ "${1:-}" == "--check" ]] || usage WORKFLOW="${2:-.github/workflows/ci.yml}" AGGREGATE="${3:-ci-status}" -[[ $# -le 3 ]] || usage +STEP_OPT_OUTS="${4:-scripts/lane-coverage-step-opt-outs.txt}" +[[ $# -le 4 ]] || usage if [[ ! -f "$WORKFLOW" ]]; then echo "check-lane-coverage: workflow not found: $WORKFLOW" >&2 exit 2 fi +if [[ ! -f "$STEP_OPT_OUTS" ]]; then + echo "check-lane-coverage: step opt-out list not found: $STEP_OPT_OUTS" >&2 + exit 2 +fi -# One structural pass. Emits three record kinds on stdout: -# JOB A = annotated opt-out (reason may be empty) +# One structural pass. Emits these record kinds on stdout: +# JOB A = annotated opt-out (reason may be empty) # NEED +# STEPNAME the step's display name, for messages +# STEPID +# COE step carries a literal continue-on-error: true +# FEED a feed row reads steps..outcome # ERR parsed="$( awk -v agg="$AGGREGATE" ' function reset_ann() { ann = 0; reason = "" } - BEGIN { injobs = 0; seen_jobs = 0; job = ""; needs_state = 0; reset_ann() } + function trim(s) { sub(/^[[:blank:]]+/, "", s); sub(/[[:blank:]]+$/, "", s); return s } + function indent_of(s, t) { t = s; sub(/[^[:blank:]].*$/, "", t); return length(t) } + # The column the KEY of a line sits at: past the "- " of a sequence item. + function key_indent(s, i) { i = indent_of(s); if (substr(s, i + 1, 2) == "- ") { return i + 2 } return i } + function uncommented(s, l) { + l = trim(s) + if (substr(l, 1, 1) == "#") { return "" } + sub(/[[:blank:]]+#.*$/, "", l) + return l + } + + # A block scalar body is data. The only fact read from it is the aggregator + # feed: a row `=${{ ... steps..outcome }}` names exactly one + # step. Any other read of a step outcome inside a scalar is a shape this + # gate does not model, so it is refused rather than guessed at. + function scan_scalar( l, id, rest) { + l = trim($0) + if (l !~ /steps\.[A-Za-z_][A-Za-z0-9_-]*\.outcome/) { return } + if (l ~ /^[A-Za-z0-9_-]+=[$][{][{] .*steps\.[A-Za-z_][A-Za-z0-9_-]*\.outcome [}][}]$/) { + match(l, /steps\.[A-Za-z_][A-Za-z0-9_-]*\.outcome [}][}]$/) + id = substr(l, RSTART + 6, RLENGTH - 6 - 11) + rest = substr(l, 1, RSTART - 1) + if (rest ~ /steps\./) { + print "ERR feed row in job " job " reads more than one step: " l + return + } + print "FEED " job " " id + return + } + print "ERR step outcome read in job " job " in a shape this gate does not model: " l + } + + BEGIN { + injobs = 0; seen_jobs = 0; job = ""; needs_state = 0; reset_ann() + step = 0; scalar_key = -1; scalar_body = -1 + } + + # --- block scalars: their content is data, never structure ------------- + # The body is every following line indented deeper than the key that + # opened it, at or beyond the indent its first line established. A line + # shallower than that closes it and is real structure again. + scalar_key >= 0 { + if ($0 ~ /^[[:blank:]]*$/) { next } + if (indent_of($0) > scalar_key && (scalar_body < 0 || indent_of($0) >= scalar_body)) { + if (scalar_body < 0) { scalar_body = indent_of($0) } + scan_scalar() + next + } + scalar_key = -1; scalar_body = -1 + } # The jobs: mapping opens at column 0 and closes at the next column-0 key. !injobs && /^jobs:[[:blank:]]*$/ { injobs = 1; seen_jobs = 1; next } @@ -138,6 +265,7 @@ parsed="$( job = $0 sub(/:.*$/, "", job) sub(/^ /, "", job) + step = 0 trail = $0 if (trail ~ /#[[:blank:]]*lane-coverage-ok:/) { sub(/^.*#[[:blank:]]*lane-coverage-ok:[[:blank:]]*/, "", trail) @@ -162,10 +290,38 @@ parsed="$( print "ERR " agg " needs: is not a block sequence: " $0 next } + if (uncommented($0) ~ /:[[:blank:]]*[|>][-+0-9]*$/) { scalar_key = key_indent($0) } next } - { reset_ann() } + # --- steps: boundaries, ids, and the absorb flag --- + { + reset_ann() + if ($0 ~ /^ - /) { step = step + 1 } + if ($0 ~ /^ - name:/) { + nm = $0 + sub(/^ - name:[[:blank:]]*/, "", nm) + print "STEPNAME " job " " step " " uncommented(nm) + } + if ($0 ~ /^ id:/) { + sid = $0 + sub(/^ id:[[:blank:]]*/, "", sid) + print "STEPID " job " " step " " uncommented(sid) + } + # `continue-on-error` decides whether a failure is absorbed, so an + # expression-valued one cannot be judged without evaluating it. Refuse + # rather than guess: a wrong guess is a silent gate either way. + if ($0 ~ /^ continue-on-error:/) { + coe = $0 + sub(/^ continue-on-error:[[:blank:]]*/, "", coe) + coe = uncommented(coe) + if (coe == "true") { print "COE " job " " step } + else if (coe != "false") { + print "ERR continue-on-error in job " job " step " step " is not a literal true/false: " coe + } + } + if (uncommented($0) ~ /:[[:blank:]]*[|>][-+0-9]*$/) { scalar_key = key_indent($0) } + } END { if (!seen_jobs) print "ERR no jobs: mapping found" } ' "$WORKFLOW" @@ -202,14 +358,48 @@ if [[ -z "$needs_all" ]]; then exit 2 fi +# --- the step opt-out list -------------------------------------------------- +# +# One `/ ` per line, read through the shared reader +# scripts/lib/read-list.sh in `leading` mode: a whole-line `#` is a comment, and +# everything after the id on an ENTRY line is that entry's reason. Carrying the +# reason on the entry line rather than in a comment block above it is what keeps +# the two from drifting apart — there is no adjacency rule to get wrong, and the +# check below can hold each entry to having one. Read before any verdict, so a +# malformed list is inconclusive rather than a pass over an empty set. +optout_entries=() +read_list::into optout_entries "$STEP_OPT_OUTS" --comments leading || exit 2 + +step_optouts="" # "/" per line +step_optout_reasons="" # "/" per line +TAB=$'\t' +for optout_line in ${optout_entries[@]+"${optout_entries[@]}"}; do + entry="${optout_line%%[[:blank:]]*}" + if [[ ! "$entry" =~ ^[A-Za-z_][A-Za-z0-9_-]*/[A-Za-z_][A-Za-z0-9_-]*$ ]]; then + echo "check-lane-coverage: malformed entry in $STEP_OPT_OUTS: '$optout_line' (expected '/ ')" >&2 + exit 2 + fi + entry_reason="${optout_line#"$entry"}" + entry_reason="${entry_reason#"${entry_reason%%[![:blank:]]*}"}" + step_optouts+="$entry"$'\n' + step_optout_reasons+="${entry}${TAB}${entry_reason}"$'\n' +done + +# Newline-delimited membership test without forking. +has_line() { case $'\n'"$1" in *$'\n'"$2"$'\n'*) return 0 ;; *) return 1 ;; esac } + errors=0 report() { echo "$1" >&2 errors=$((errors + 1)) } +# --- job level -------------------------------------------------------------- + +annotated_jobs="" while read -r _ job flag reason; do [[ -n "$job" ]] || continue + [[ "$flag" == "A" ]] && annotated_jobs+="$job"$'\n' [[ "$job" != "$AGGREGATE" ]] || continue in_needs=1 @@ -235,11 +425,104 @@ while IFS= read -r need; do report "DANGLING NEED: ${AGGREGATE}.needs names '$need', which is not a job defined in $WORKFLOW." done <<<"$needs_all" +# --- step level ------------------------------------------------------------- + +rec_stepid="$(printf '%s\n' "$parsed" | grep '^STEPID ' || true)" +rec_coe="$(printf '%s\n' "$parsed" | grep '^COE ' || true)" +rec_feed="$(printf '%s\n' "$parsed" | grep '^FEED ' || true)" +rec_stepname="$(printf '%s\n' "$parsed" | grep '^STEPNAME ' || true)" + +# step_id_of : the id a step declares, or empty. +step_id_of() { + local _ j o id + while read -r _ j o id; do + [[ "$j" == "$1" && "$o" == "$2" ]] || continue + printf '%s' "$id" + return + done <<<"$rec_stepid" +} + +# step_name_of : the display name, for a step with no id. +step_name_of() { + local _ j o name + while read -r _ j o name; do + [[ "$j" == "$1" && "$o" == "$2" ]] || continue + printf '%s' "$name" + return + done <<<"$rec_stepname" +} + +# Every gate step as "/"; every feed read the same way; every id +# declared anywhere, so a dangling feed row can say WHICH half is missing. +gate_steps="" +all_step_ids="" +while read -r _ j o id; do + [[ -n "$j" ]] || continue + all_step_ids+="$j/$id"$'\n' +done <<<"$rec_stepid" + +while read -r _ j o; do + [[ -n "$j" ]] || continue + id="$(step_id_of "$j" "$o")" + if [[ -z "$id" ]]; then + has_line "$annotated_jobs" "$j" && continue + report "UNREADABLE GATE: job '$j' step #$o ('$(step_name_of "$j" "$o")') carries 'continue-on-error: true' but no 'id', so its outcome cannot be read and its failure turns nothing red. Give it an id and feed it to the aggregator, or drop continue-on-error so it fails the job directly." + continue + fi + gate_steps+="$j/$id"$'\n' +done <<<"$rec_coe" + +feed_reads="" +while read -r _ j id; do + [[ -n "$j" ]] || continue + feed_reads+="$j/$id"$'\n' +done <<<"$rec_feed" + +# 5. UNFED GATE, and the fed half of 8. +while IFS= read -r gs; do + [[ -n "$gs" ]] || continue + job="${gs%%/*}" + if has_line "$step_optouts" "$gs"; then + if has_line "$feed_reads" "$gs"; then + report "STALE STEP OPT-OUT: $STEP_OPT_OUTS lists '$gs' as not an aggregator gate, but a feed row in job '$job' reads steps.${gs#*/}.outcome. Drop the entry." + fi + continue + fi + has_line "$feed_reads" "$gs" && continue + has_line "$annotated_jobs" "$job" && continue + report "UNFED GATE: step '${gs#*/}' in job '$job' carries 'continue-on-error: true' but no aggregator feed row in that job reads steps.${gs#*/}.outcome, so its failure is absorbed and turns nothing red. Add a row '=\${{ steps.${gs#*/}.outcome }}' to the feed, or list '$gs' in $STEP_OPT_OUTS with a reason." +done <<<"$gate_steps" + +# 7. DANGLING FEED. +while IFS= read -r fr; do + [[ -n "$fr" ]] || continue + has_line "$gate_steps" "$fr" && continue + job="${fr%%/*}" + if has_line "$all_step_ids" "$fr"; then + report "DANGLING FEED: a feed row in job '$job' reads steps.${fr#*/}.outcome, but step '${fr#*/}' does not carry 'continue-on-error: true'. A step without it fails the job on its own; the row is not mirroring a gate. Add the flag, or drop the row." + else + report "DANGLING FEED: a feed row in job '$job' reads steps.${fr#*/}.outcome, but no step in that job declares 'id: ${fr#*/}'. The read evaluates to an empty string. Fix the id, or drop the row." + fi +done <<<"$feed_reads" + +# 8 (unlisted half) and 9. +while IFS="$TAB" read -r entry oreason; do + [[ -n "$entry" ]] || continue + if [[ -z "$oreason" ]]; then + report "BARE STEP OPT-OUT: $STEP_OPT_OUTS lists '$entry' with nothing after it. Write the reason on the entry line: '$entry '. An opt-out without a written reason is not a documented opt-out." + fi + if ! has_line "$gate_steps" "$entry"; then + report "STALE STEP OPT-OUT: $STEP_OPT_OUTS lists '$entry', but no step with that id in that job carries 'continue-on-error: true' in $WORKFLOW. Drop the entry." + fi +done <<<"$step_optout_reasons" + if [[ "$errors" -ne 0 ]]; then echo "check-lane-coverage: $errors coverage defect(s) in $WORKFLOW" >&2 exit 1 fi covered="$(printf '%s\n' "$needs_all" | grep -c . || true)" -echo "check-lane-coverage: $WORKFLOW — all $covered lane(s) reachable from ${AGGREGATE}.needs" +fed="$(printf '%s' "$feed_reads" | grep -c . || true)" +opted="$(printf '%s' "$step_optouts" | grep -c . || true)" +echo "check-lane-coverage: $WORKFLOW — all $covered lane(s) reachable from ${AGGREGATE}.needs; all $fed gate step(s) fed to the aggregator, $opted opted out" exit 0 diff --git a/scripts/check-lane-coverage.test.sh b/scripts/check-lane-coverage.test.sh index aa4f5da1b9..e06392a3b5 100755 --- a/scripts/check-lane-coverage.test.sh +++ b/scripts/check-lane-coverage.test.sh @@ -7,6 +7,11 @@ # `git -C config user.*`, and the un-scoped form of that command writes the # test identity into the CALLER's repo config (claude-code-plugins#2839). No git # state means the class cannot recur here. +# +# Every fixture run passes its OWN step opt-out list. The repository's real list +# names steps of the real ci.yml, and an entry naming a step no fixture defines +# is a stale opt-out by construction — so a fixture checked against it would fail +# for a reason that has nothing to do with the case under test. set -uo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" @@ -22,6 +27,9 @@ fail() { scratch="$(mktemp -d)" trap 'rm -rf "$scratch"' EXIT +NONE="$scratch/no-optouts.txt" +: >"$NONE" + # Runs the gate and asserts exit code plus (optionally) a substring of output. expect() { local label="$1" want_rc="$2" want_text="$3" @@ -87,24 +95,100 @@ needs_of() { for n in "$@"; do printf ' - %s\n' "$n"; done } +# The step-level shape: a `lint` job whose lanes are `continue-on-error` steps, +# with an aggregator feed reading their outcomes back out of a block scalar. The +# caller supplies the steps and the feed rows so each case can break exactly one +# side of the pairing. A `noop` lane rides along so `ci-status.needs` is never +# empty, which is its own exit-2 shape. +# +# An annotation on `lint` also takes `lint` OUT of needs, because an annotated +# job that IS in needs is a stale opt-out — a different defect from the one such +# a fixture is built to exercise. +write_step_workflow() { + local path="$1" steps_block="$2" feed_block="$3" job_annotation="${4:-}" + local needs_rows=' - noop' + [[ -n "$job_annotation" ]] || needs_rows+=' + - lint' + cat >"$path" <:` spec per step: +# gate — `continue-on-error: true` plus an id, the pairable shape +# plain — an id but no absorb flag, so the step fails its job on its own +# anon — the absorb flag with no id, so nothing can read its outcome +steps_of() { + local spec kind id + for spec in "$@"; do + kind="${spec%%:*}" + id="${spec#*:}" + printf ' - name: Run the %s gate\n' "$id" + [[ "$kind" == anon ]] || printf ' id: %s\n' "$id" + [[ "$kind" == plain ]] || printf ' continue-on-error: true\n' + printf ' run: scripts/%s.sh --check\n' "$id" + done +} + +# The aggregator feed rows for the named ids, in the shape ci.yml uses. +# shellcheck disable=SC2016 # deliberate: ${{ }} is workflow syntax, not a shell expansion +feed_of() { + local id + for id in "$@"; do printf ' %s=${{ steps.%s.outcome }}\n' "$id" "$id"; done +} + # --- usage / input errors --------------------------------------------------- expect "bare invocation exits 2 with usage" 2 "usage:" expect "unknown mode exits 2 with usage" 2 "usage:" --verify expect "missing workflow exits 2" 2 "workflow not found" --check "$scratch/nope.yml" -expect "excess arguments exit 2" 2 "usage:" --check "$scratch/nope.yml" ci-status extra +expect "excess arguments exit 2" 2 "usage:" --check "$scratch/nope.yml" ci-status "$NONE" extra + +write_workflow "$scratch/list-missing.yml" "" "$(needs_of alpha beta)" +expect "missing step opt-out list exits 2" 2 "step opt-out list not found" \ + --check "$scratch/list-missing.yml" ci-status "$scratch/no-such-list.txt" printf 'name: ci\non:\n pull_request:\n' >"$scratch/nojobs.yml" -expect "workflow with no jobs mapping exits 2" 2 "no jobs: mapping found" --check "$scratch/nojobs.yml" +expect "workflow with no jobs mapping exits 2" 2 "no jobs: mapping found" \ + --check "$scratch/nojobs.yml" ci-status "$NONE" -# --- the defect this gate exists to catch ----------------------------------- +# --- the defect this gate exists to catch, at JOB level --------------------- write_workflow "$scratch/ungated.yml" "" "$(needs_of alpha)" expect "job absent from needs fails and names the job" 1 "UNGATED LANE: job 'beta'" \ - --check "$scratch/ungated.yml" + --check "$scratch/ungated.yml" ci-status "$NONE" write_workflow "$scratch/covered.yml" "" "$(needs_of alpha beta)" -expect "every job in needs passes" 0 "all 2 lane(s) reachable" --check "$scratch/covered.yml" +expect "every job in needs passes" 0 "all 2 lane(s) reachable" \ + --check "$scratch/covered.yml" ci-status "$NONE" # --- a lane that fans out across a matrix ----------------------------------- # @@ -127,7 +211,7 @@ write_workflow "$scratch/sharded.yml" " gamma: run: echo \"leg \$LEG of \$LEGS\" " "$(needs_of alpha beta gamma)" expect "a lane carrying a strategy matrix parses and stays covered" 0 "all 3 lane(s) reachable" \ - --check "$scratch/sharded.yml" + --check "$scratch/sharded.yml" ci-status "$NONE" write_workflow "$scratch/sharded-static.yml" " gamma: runs-on: ubuntu-24.04 @@ -140,9 +224,9 @@ write_workflow "$scratch/sharded-static.yml" " gamma: run: echo leg " "$(needs_of alpha beta gamma)" expect "a literal matrix block parses too" 0 "all 3 lane(s) reachable" \ - --check "$scratch/sharded-static.yml" + --check "$scratch/sharded-static.yml" ci-status "$NONE" -# --- the opt-out path ------------------------------------------------------- +# --- the job opt-out path --------------------------------------------------- write_workflow "$scratch/optout.yml" "" "$(needs_of alpha)" # Annotate `beta` via the contiguous comment block immediately above its key. @@ -153,22 +237,25 @@ annotate_above() { ' "$file" >"$file.tmp" && mv "$file.tmp" "$file" } annotate_above "$scratch/optout.yml" beta " # lane-coverage-ok: advisory lane, findings routed to code scanning" -expect "annotated opt-out passes" 0 "all 1 lane(s) reachable" --check "$scratch/optout.yml" +expect "annotated opt-out passes" 0 "all 1 lane(s) reachable" \ + --check "$scratch/optout.yml" ci-status "$NONE" write_workflow "$scratch/bare-optout.yml" "" "$(needs_of alpha)" annotate_above "$scratch/bare-optout.yml" beta " # lane-coverage-ok:" -expect "opt-out with no reason fails" 1 "BARE OPT-OUT: job 'beta'" --check "$scratch/bare-optout.yml" +expect "opt-out with no reason fails" 1 "BARE OPT-OUT: job 'beta'" \ + --check "$scratch/bare-optout.yml" ci-status "$NONE" write_workflow "$scratch/trailing-optout.yml" "" "$(needs_of alpha)" awk '{ sub(/^ beta:$/, " beta: # lane-coverage-ok: deliberately advisory"); print }' \ "$scratch/trailing-optout.yml" >"$scratch/trailing-optout.yml.tmp" && mv "$scratch/trailing-optout.yml.tmp" "$scratch/trailing-optout.yml" -expect "trailing-comment opt-out passes" 0 "all 1 lane(s) reachable" --check "$scratch/trailing-optout.yml" +expect "trailing-comment opt-out passes" 0 "all 1 lane(s) reachable" \ + --check "$scratch/trailing-optout.yml" ci-status "$NONE" write_workflow "$scratch/stale-optout.yml" "" "$(needs_of alpha beta)" annotate_above "$scratch/stale-optout.yml" beta " # lane-coverage-ok: no longer true" expect "opt-out on a job that IS in needs fails as stale" 1 "STALE OPT-OUT: job 'beta'" \ - --check "$scratch/stale-optout.yml" + --check "$scratch/stale-optout.yml" ci-status "$NONE" # An annotation separated from its job key by a blank line must NOT carry over. write_workflow "$scratch/detached-optout.yml" "" "$(needs_of alpha)" @@ -178,40 +265,169 @@ awk ' ' "$scratch/detached-optout.yml" >"$scratch/detached-optout.yml.tmp" && mv "$scratch/detached-optout.yml.tmp" "$scratch/detached-optout.yml" expect "non-contiguous annotation does not exempt the job" 1 "UNGATED LANE: job 'beta'" \ - --check "$scratch/detached-optout.yml" + --check "$scratch/detached-optout.yml" ci-status "$NONE" # --- dangling needs --------------------------------------------------------- write_workflow "$scratch/dangling.yml" "" "$(needs_of alpha beta gamma)" -expect "needs entry with no defined job fails" 1 "DANGLING NEED" --check "$scratch/dangling.yml" +expect "needs entry with no defined job fails" 1 "DANGLING NEED" \ + --check "$scratch/dangling.yml" ci-status "$NONE" # --- unrecognized shapes must be inconclusive, never green ------------------ write_workflow "$scratch/flow-needs.yml" "" " needs: [alpha, beta]" -expect "flow-sequence needs exits 2, not 0" 2 "not a block sequence" --check "$scratch/flow-needs.yml" +expect "flow-sequence needs exits 2, not 0" 2 "not a block sequence" \ + --check "$scratch/flow-needs.yml" ci-status "$NONE" write_workflow "$scratch/scalar-needs.yml" "" " needs: alpha" -expect "scalar needs exits 2, not 0" 2 "not a block sequence" --check "$scratch/scalar-needs.yml" +expect "scalar needs exits 2, not 0" 2 "not a block sequence" \ + --check "$scratch/scalar-needs.yml" ci-status "$NONE" write_workflow "$scratch/no-needs.yml" "" " permissions: contents: read" -expect "aggregate with no needs block exits 2" 2 "declares no needs: block" --check "$scratch/no-needs.yml" +expect "aggregate with no needs block exits 2" 2 "declares no needs: block" \ + --check "$scratch/no-needs.yml" ci-status "$NONE" write_workflow "$scratch/empty-needs.yml" "" " needs: " -expect "aggregate with an empty needs block exits 2" 2 "empty needs: block" --check "$scratch/empty-needs.yml" +expect "aggregate with an empty needs block exits 2" 2 "empty needs: block" \ + --check "$scratch/empty-needs.yml" ci-status "$NONE" write_workflow "$scratch/odd-key.yml" " &anchor-not-a-job: runs-on: ubuntu-24.04 " "$(needs_of alpha beta)" -expect "unmodelled 2-space key exits 2" 2 "unrecognized key under jobs" --check "$scratch/odd-key.yml" +expect "unmodelled 2-space key exits 2" 2 "unrecognized key under jobs" \ + --check "$scratch/odd-key.yml" ci-status "$NONE" write_workflow "$scratch/missing-agg.yml" "" "$(needs_of alpha beta)" -expect "unknown aggregate job exits 2" 2 "is not defined" --check "$scratch/missing-agg.yml" no-such-job +expect "unknown aggregate job exits 2" 2 "is not defined" \ + --check "$scratch/missing-agg.yml" no-such-job "$NONE" + +# --- STEP level: the gate set and the feed set must be equal ---------------- +# +# This is the half that survived the six-job collapse. A lane is now a step +# carrying `continue-on-error: true`; the flag absorbs its failure, so the only +# thing that turns anything red is the aggregator reading `steps..outcome` +# back. A gate the feed does not read is decoration, and the job-level check +# above cannot see it, because the JOB is in `needs`. + +write_step_workflow "$scratch/steps-paired.yml" \ + "$(steps_of gate:shellcheck gate:typos)" "$(feed_of shellcheck typos)" +expect "every gate step fed to the aggregator passes" 0 "all 2 gate step(s) fed" \ + --check "$scratch/steps-paired.yml" ci-status "$NONE" + +write_step_workflow "$scratch/steps-unfed.yml" \ + "$(steps_of gate:shellcheck gate:typos)" "$(feed_of shellcheck)" +expect "a gate step missing from the feed fails and names it" 1 \ + "UNFED GATE: step 'typos' in job 'lint'" \ + --check "$scratch/steps-unfed.yml" ci-status "$NONE" + +write_step_workflow "$scratch/steps-anonymous.yml" \ + "$(steps_of gate:shellcheck anon:typos)" "$(feed_of shellcheck)" +expect "a gate step with no id fails as unreadable" 1 \ + "UNREADABLE GATE: job 'lint' step #2 ('Run the typos gate')" \ + --check "$scratch/steps-anonymous.yml" ci-status "$NONE" + +write_step_workflow "$scratch/steps-dangling-feed.yml" \ + "$(steps_of gate:shellcheck)" "$(feed_of shellcheck typos)" +expect "a feed row naming no step fails" 1 "no step in that job declares 'id: typos'" \ + --check "$scratch/steps-dangling-feed.yml" ci-status "$NONE" + +write_step_workflow "$scratch/steps-feed-without-flag.yml" \ + "$(steps_of gate:shellcheck plain:typos)" "$(feed_of shellcheck typos)" +expect "a feed row for a step without continue-on-error fails" 1 \ + "does not carry 'continue-on-error: true'" \ + --check "$scratch/steps-feed-without-flag.yml" ci-status "$NONE" + +# The feed carries an override on a docs-only diff; the row still names exactly +# one step, and that is the only part this gate reads. (Which overrides are +# sanctioned is scripts/check-docs-only-gate.sh's question, not this one.) +write_step_workflow "$scratch/steps-overridden-feed.yml" \ + "$(steps_of gate:shellcheck)" \ + " shellcheck=\${{ needs.changes.outputs.run_full == 'false' && 'success' || steps.shellcheck.outcome }}" +expect "an overridden feed row still pairs with its gate step" 0 "all 1 gate step(s) fed" \ + --check "$scratch/steps-overridden-feed.yml" ci-status "$NONE" + +# Steps of a job that is itself an annotated opt-out cannot gate a merge +# whatever they do, so the step-level check does not second-guess them. +write_step_workflow "$scratch/steps-annotated-job.yml" \ + "$(steps_of gate:shellcheck gate:typos)" "$(feed_of shellcheck)" \ + " # lane-coverage-ok: advisory lane, findings routed to code scanning +" +expect "an unfed gate step in an annotated job is exempt" 0 "all 1 gate step(s) fed" \ + --check "$scratch/steps-annotated-job.yml" ci-status "$NONE" + +# --- STEP level: shapes this gate refuses to guess at ----------------------- + +write_step_workflow "$scratch/steps-expr-flag.yml" \ + " - name: Run the shellcheck gate + id: shellcheck + continue-on-error: \${{ github.event_name == 'pull_request' }} + run: scripts/shellcheck.sh --check" \ + "$(feed_of shellcheck)" +expect "an expression-valued continue-on-error exits 2" 2 "is not a literal true/false" \ + --check "$scratch/steps-expr-flag.yml" ci-status "$NONE" + +write_step_workflow "$scratch/steps-odd-feed.yml" \ + "$(steps_of gate:shellcheck)" \ + " shellcheck=\${{ steps.shellcheck.outcome == 'success' }}" +expect "a feed row in an unmodelled shape exits 2" 2 "a shape this gate does not model" \ + --check "$scratch/steps-odd-feed.yml" ci-status "$NONE" + +write_step_workflow "$scratch/steps-two-in-a-row.yml" \ + "$(steps_of gate:shellcheck gate:typos)" \ + " both=\${{ steps.shellcheck.outcome || steps.typos.outcome }}" +expect "a feed row naming two steps exits 2" 2 "reads more than one step" \ + --check "$scratch/steps-two-in-a-row.yml" ci-status "$NONE" + +# --- STEP level: the opt-out list, checked in both directions --------------- + +optout_list() { + local path="$1" + shift + printf '%s\n' "$@" >"$path" + printf '%s' "$path" +} + +write_step_workflow "$scratch/steps-optout.yml" \ + "$(steps_of gate:shellcheck gate:resolver)" "$(feed_of shellcheck)" +expect "an opted-out gate step passes" 0 "1 opted out" \ + --check "$scratch/steps-optout.yml" ci-status \ + "$(optout_list "$scratch/ol-good.txt" \ + "# A whole-line comment is prose about the list, not a reason." \ + "lint/resolver its failure must fall through to a fail-open default, not turn the job red")" + +expect "an opt-out with no reason fails" 1 "BARE STEP OPT-OUT" \ + --check "$scratch/steps-optout.yml" ci-status \ + "$(optout_list "$scratch/ol-bare.txt" "lint/resolver")" + +# A whole-line comment is prose about the list. It never stands in as the reason +# for the entry under it, so the reason cannot drift away from what it excuses. +expect "a comment line above an entry is not its reason" 1 "BARE STEP OPT-OUT" \ + --check "$scratch/steps-optout.yml" ci-status \ + "$(optout_list "$scratch/ol-above.txt" "# resolver's own step, fails open by design" "lint/resolver")" + +write_step_workflow "$scratch/steps-paired-again.yml" \ + "$(steps_of gate:shellcheck gate:typos)" "$(feed_of shellcheck typos)" +expect "an opt-out for a step the feed reads anyway fails as stale" 1 \ + "STALE STEP OPT-OUT: " \ + --check "$scratch/steps-paired-again.yml" ci-status \ + "$(optout_list "$scratch/ol-fed.txt" "lint/typos no longer true")" + +expect "an opt-out for a step that is not a gate fails as stale" 1 \ + "no step with that id in that job carries 'continue-on-error: true'" \ + --check "$scratch/steps-paired-again.yml" ci-status \ + "$(optout_list "$scratch/ol-gone.txt" "lint/deleted the step this excused was deleted")" + +expect "a malformed opt-out entry exits 2" 2 "malformed entry in" \ + --check "$scratch/steps-paired-again.yml" ci-status \ + "$(optout_list "$scratch/ol-malformed.txt" "lint:typos the separator is a slash")" # --- the real workflow ------------------------------------------------------ expect "the repository's own ci.yml is fully covered" 0 "reachable from ci-status.needs" --check +expect "every gate step in the repository's own ci.yml is fed or opted out" 0 \ + "gate step(s) fed to the aggregator" --check if [[ $failures -eq 0 ]]; then echo "ALL PASS" diff --git a/scripts/lane-coverage-step-opt-outs.txt b/scripts/lane-coverage-step-opt-outs.txt new file mode 100644 index 0000000000..6bd8c28077 --- /dev/null +++ b/scripts/lane-coverage-step-opt-outs.txt @@ -0,0 +1,24 @@ +# Steps that carry `continue-on-error: true` for a reason OTHER than feeding the +# aggregator, each paired with that reason. Read by +# scripts/check-lane-coverage.sh, which otherwise requires every such step to be +# read back as `steps..outcome` by a feed row in its own job — because +# `continue-on-error` absorbs the failure, so a gate step nothing reads turns +# nothing red. +# +# Format: one `/ ` per line, matched against +# .github/workflows/ci.yml. The reason is the rest of the line, and an entry with +# nothing after it FAILS, the same way a bare `# lane-coverage-ok:` annotation on +# a job does. Whole-line `#` comments and blanks are ignored; the file is parsed +# by scripts/lib/read-list.sh in `leading` mode, like every other list here. +# +# Checked in BOTH directions: an entry naming a step that does not carry +# `continue-on-error: true`, or one the feed reads anyway, fails as stale. The +# list cannot rot into a silent allowlist. +# +# ADDING A NEW GATE? Do not reach for this list. A gate step missing from the +# aggregator feed should FAIL here; that failure is the tool working. This list +# is only for a step whose `continue-on-error` exists to let a FAILURE FALL +# THROUGH to a fail-open default, which is the opposite of a gate. + +changes/detect the resolver's own step: a failed detector must leave its outputs unset so every consumer resolves toward RUNNING its lane, and feeding that outcome to an aggregator would red the run for the one case it exists to survive +changes/match the resolver's own filter-group step, on the same fall-through contract as changes/detect; the pair is gated instead by the non-continue-on-error detector self-test above them and by scripts/check-docs-only-gate.sh From b3eb2aad5e7c2937c8c23c2779c6ad4cc23784f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 01:09:52 +0000 Subject: [PATCH 03/21] refactor(claude-ops): dispatch seven audit rows from one event emitter Seven of the nine audit hooks were the same fifteen-to-thirty-line shape, differing only in the event they listened to and the fields they projected, so every change to the shape was seven edits and the row table lived only in a test. One script, audit-event-emitter.sh, is now registered on those seven events and dispatches on hook_event_name to a per-row projection. Each row keeps its own _enabled switch and emits the same hook name, event, status and data fields as before, so telemetry readers see byte-identical envelopes. hook-failure-audit.sh and skill-usage-audit.sh keep their own files: the first is earned behavior, the second sits on the PostToolUse hot path where the kill-switch hoist gate requires a single-switch predicate. The session_id extraction is done once. The seven per-hook suites are replaced by one table-driven suite at the emitter's interface. Deepening candidate 10 of the architecture review. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011SQkHipoF2M8rTtnkbFKKP --- .../claude-config/skills/audit-pass/SKILL.md | 2 +- plugins/claude-ops/README.md | 36 ++- plugins/claude-ops/hooks/api-error-audit.sh | 45 --- .../claude-ops/hooks/api-error-audit.test.sh | 48 --- .../claude-ops/hooks/audit-event-emitter.sh | 264 +++++++++++++++++ .../hooks/audit-event-emitter.test.sh | 276 ++++++++++++++++++ .../claude-ops/hooks/audit-session-id.test.sh | 43 +-- plugins/claude-ops/hooks/claude-ops-paths.sh | 2 +- .../hooks/claude-ops-test-helpers.sh | 35 +++ .../claude-ops/hooks/config-change-audit.sh | 44 --- .../hooks/config-change-audit.test.sh | 41 --- plugins/claude-ops/hooks/hooks.json | 16 +- .../hooks/instructions-loaded-audit.sh | 84 ------ .../hooks/instructions-loaded-audit.test.sh | 76 ----- .../hooks/permission-denied-audit.sh | 58 ---- .../hooks/permission-denied-audit.test.sh | 62 ---- plugins/claude-ops/hooks/pre-compact-audit.sh | 44 --- .../hooks/pre-compact-audit.test.sh | 41 --- .../hooks/skill-usage-expansion-audit.sh | 78 ----- .../hooks/skill-usage-expansion-audit.test.sh | 145 --------- .../claude-ops/hooks/tool-failure-audit.sh | 55 ---- .../hooks/tool-failure-audit.test.sh | 49 ---- .../scripts/skill-pair-cooccurrence.sh | 3 +- 23 files changed, 643 insertions(+), 904 deletions(-) delete mode 100755 plugins/claude-ops/hooks/api-error-audit.sh delete mode 100755 plugins/claude-ops/hooks/api-error-audit.test.sh create mode 100755 plugins/claude-ops/hooks/audit-event-emitter.sh create mode 100644 plugins/claude-ops/hooks/audit-event-emitter.test.sh delete mode 100755 plugins/claude-ops/hooks/config-change-audit.sh delete mode 100755 plugins/claude-ops/hooks/config-change-audit.test.sh delete mode 100755 plugins/claude-ops/hooks/instructions-loaded-audit.sh delete mode 100755 plugins/claude-ops/hooks/instructions-loaded-audit.test.sh delete mode 100755 plugins/claude-ops/hooks/permission-denied-audit.sh delete mode 100755 plugins/claude-ops/hooks/permission-denied-audit.test.sh delete mode 100755 plugins/claude-ops/hooks/pre-compact-audit.sh delete mode 100755 plugins/claude-ops/hooks/pre-compact-audit.test.sh delete mode 100755 plugins/claude-ops/hooks/skill-usage-expansion-audit.sh delete mode 100755 plugins/claude-ops/hooks/skill-usage-expansion-audit.test.sh delete mode 100755 plugins/claude-ops/hooks/tool-failure-audit.sh delete mode 100755 plugins/claude-ops/hooks/tool-failure-audit.test.sh diff --git a/plugins/claude-config/skills/audit-pass/SKILL.md b/plugins/claude-config/skills/audit-pass/SKILL.md index 78ca1a4c1a..f8f7e8ad8c 100644 --- a/plugins/claude-config/skills/audit-pass/SKILL.md +++ b/plugins/claude-config/skills/audit-pass/SKILL.md @@ -158,7 +158,7 @@ name it in `skipped`. Then `/memory`, `/skills`, `/hooks`, `/mcp`, `/permissions **`InstructionsLoaded` is normally UNAVAILABLE, and the run says so rather than requiring it.** This plugin wires no `InstructionsLoaded` hook, the only producer in this marketplace -(`claude-ops/hooks/instructions-loaded-audit.sh`) is optional, is a no-op without a telemetry sink, +(the InstructionsLoaded row of `claude-ops/hooks/audit-event-emitter.sh`) is optional, is a no-op without a telemetry sink, and drops `session_start` events by default, and the startup events this skill would need have already fired before it is invoked, so there is nothing to subscribe to at dispatch time even where a producer exists. Requiring data the plugin never records would make the memory-layer liveness diff --git a/plugins/claude-ops/README.md b/plugins/claude-ops/README.md index 91df67924c..4c1a7a8184 100644 --- a/plugins/claude-ops/README.md +++ b/plugins/claude-ops/README.md @@ -47,13 +47,35 @@ Claude Code's native OTEL cannot see. ## The audit hooks -Eight advisory `*-audit` hooks, spread across nine hook scripts because `skill-usage-audit` has two -producers, emit the marketplace +Eight advisory `*-audit` hooks, registered as nine rows because +`skill-usage-audit` has two producers, emit the marketplace [hook-telemetry envelope](../../docs/conventions/hook-telemetry/README.md). One JSON event per run carrying that hook's own `duration_ms`, outcome, and a privacy-safe subject. Each is independently toggleable via its own `userConfig` boolean (default **on**; see [Per-hook kill switches](#per-hook-kill-switches)). -The six pure emitters are a no-op until a consumer wires a sink (below); + +Three scripts serve those nine rows. `hooks/audit-event-emitter.sh` carries +seven of them and picks the row from the payload's `hook_event_name`, the way +`session-event-log.sh` next to it serves about thirty events from one file; the +seven events are distinct, so the event alone selects the row. Each row still +reads its own `_enabled` switch and emits the same telemetry `hook` id, +`hook_event`, `status` and `data` fields it emitted as a standalone script, so +no downstream reader can tell the difference. The two rows with earned behavior +of their own keep their files: `skill-usage-audit.sh`, whose kill switch is +inlined above its library `source` because it sits on the hot `PostToolUse` +path, and `hook-failure-audit.sh` (below). + +The [hook budget](../../docs/conventions/hook-budget/README.md) is accounted per +`hooks.json` entry, and the collapse changes no entry: the same nine audit rows, +on the same events, with the same matchers, and one event still spawns exactly +one process. Per-entry cost is unchanged for an enabled row (the same library, +the same `jq` passes, plus one bash pattern match to read the event) and lower +when every switch is off, because the emitter reads all seven switches before it +parses the library. A row that is off on its own pays one buffered payload it +did not pay before, because the row is only known once the event has been read; +it parsed the same library then as now, and no extra process runs either way. + +The six pure emitter rows are a no-op until a consumer wires a sink (below); `skill-usage-audit` is one exception. Both its producers also write the shared `skill-usage.jsonl` second store unconditionally (disable the whole feature with `skill_usage_audit_enabled=false`; pick the store's home with `skill_usage_scope` @@ -116,10 +138,10 @@ None captures a command body, absolute path, error message, or argument body, on the repo-relative path of the loaded rule file. The InstructionsLoaded row carries no matcher on purpose. That event's matcher selects on load -reason, and `instructions-loaded-audit.sh` passes every reason through verbatim into its subject, so +reason, and the row passes every reason through verbatim into its subject, so scoping to the full documented set would skip nothing and would silently drop any reason a later release adds. Scoping below that set is worse: the only reason worth excluding for cost is -`session_start`, which the script already drops at write time, and it drops it behind the +`session_start`, which the row already drops at write time, and it drops it behind the `instructions_loaded_audit_log_session_start` option. A matcher that excluded `session_start` would stop the hook from ever spawning on it, leaving that option switched on but unable to log anything. The row therefore stays unscoped until the option is retired. @@ -129,7 +151,9 @@ The row therefore stays unscoped until the option is retired. Each audit hook is toggled by its own `userConfig` boolean (default **on**; set to `false` for a clean no-op). Disable one hook without touching the others. The hooks read them through the native `CLAUDE_PLUGIN_OPTION_` hook-process -mirror. +mirror. Seven rows share `audit-event-emitter.sh`, which reads the switch of the +row the event selected: sharing a script does not share a switch, and turning one +row off leaves the other six emitting. | Hook | Option | |---|---| diff --git a/plugins/claude-ops/hooks/api-error-audit.sh b/plugins/claude-ops/hooks/api-error-audit.sh deleted file mode 100755 index da39ef8ffa..0000000000 --- a/plugins/claude-ops/hooks/api-error-audit.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/usr/bin/env bash -# StopFailure hook: emit a telemetry envelope for API-level turn failures -# (rate_limit, billing_error, server_error, ...). Feeds a consumer's -# rate-limit velocity tracking (e.g. the observability skill). -# -# ADVISORY: StopFailure output and exit code are ignored — always exit 0. -# The subject is the StopFailure `error` type only: `error_details` may carry -# prompt fragments or session metadata, so it is never captured (privacy-safe). -# Pure telemetry emitter: no sink wired (HOOK_TELEMETRY_SINK unset) → no-op. -# Kill switch: CLAUDE_PLUGIN_OPTION_API_ERROR_AUDIT_ENABLED=false. - -set -uo pipefail -# Hook directory by parameter expansion, never `dirname`. GNU Bash forks a -# subshell for every command substitution even when the body is a builtin -# (Command Substitution, Bash Reference Manual). On Windows Git Bash that -# fork is a process. `${BASH_SOURCE[0]%/*}` equals dirname for every shape -# BASH_SOURCE takes; the fallback covers a bare filename, where the strip is a -# no-op and dirname answers `.`. -HOOK_DIR="${BASH_SOURCE[0]%/*}" -[[ "$HOOK_DIR" == "${BASH_SOURCE[0]}" ]] && HOOK_DIR=. - -# shellcheck source=hook-utils.sh -source "$HOOK_DIR/hook-utils.sh" -hook::check_enabled "API_ERROR_AUDIT" -hook::telemetry_enabled || exit 0 - -START=${EPOCHREALTIME:-} - -hook::buffer_stdin_to INPUT || exit 0 - -# data.session_id (additive, hook-telemetry rule 1): the sink routes an -# envelope carrying one into the per-session log beside session-event-log.sh. -# A bash match over the buffered payload, no extra process; empty when the -# payload carries none, and the key is then left out of data. -SESSION_ID="" -[[ "$INPUT" =~ \"session_id\"[[:space:]]*:[[:space:]]*\"([A-Za-z0-9._-]+)\" ]] && SESSION_ID="${BASH_REMATCH[1]}" - -ERROR_TYPE=$(hook::jq_field "$INPUT" '.error') || exit 0 - -DATA=$(jq -nc --arg session_id "$SESSION_ID" --arg subject "$ERROR_TYPE" '{subject: $subject} + (if $session_id == "" then {} else {session_id: $session_id} end)') - -hook::emit_telemetry "api-error-audit" "StopFailure" "error" \ - "$START" "$DATA" "${CLAUDE_PROJECT_DIR:-}" - -exit 0 diff --git a/plugins/claude-ops/hooks/api-error-audit.test.sh b/plugins/claude-ops/hooks/api-error-audit.test.sh deleted file mode 100755 index d171ee5b2d..0000000000 --- a/plugins/claude-ops/hooks/api-error-audit.test.sh +++ /dev/null @@ -1,48 +0,0 @@ -#!/usr/bin/env bash -# Contract test for api-error-audit.sh (claude-ops plugin). Black-box. -set -uo pipefail - -HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -HOOK="$HOOK_DIR/api-error-audit.sh" -TEST_TMPDIR="$(mktemp -d)" -trap 'rm -rf "$TEST_TMPDIR"' EXIT - -# shellcheck source=claude-ops-test-helpers.sh -source "$HOOK_DIR/claude-ops-test-helpers.sh" -unset CLAUDE_PROJECT_DIR - -INPUT='{"session_id":"sess-api","error":"rate_limit"}' - -# --- Emits the envelope when a sink is wired ------------------------------- -TEL="$TEST_TMPDIR/tel.json" -SINK="$(make_sink "$TEL")" -env HOOK_TELEMETRY_SINK="$SINK" bash "$HOOK" <<<"$INPUT" >/dev/null 2>&1 -if wait_for_sink "$TEL"; then - assert_eq "hook id" "api-error-audit" "$(jq -r '.hook' "$TEL")" - assert_eq "hook_event" "StopFailure" "$(jq -r '.hook_event' "$TEL")" - assert_eq "status" "error" "$(jq -r '.status' "$TEL")" - assert_eq "schema_version" "1.1" "$(jq -r '.schema_version' "$TEL")" - assert_eq "spine session_id from the payload (1.1)" "sess-api" "$(jq -r '.session_id' "$TEL")" - assert_eq "data.session_id still sent for a 1.0 sink" "sess-api" "$(jq -r '.data.session_id' "$TEL")" - assert_eq "data.subject" "rate_limit" "$(jq -r '.data.subject' "$TEL")" -else - bad "no envelope written when sink wired" -fi - -# --- No-op (silent, exit 0) when no sink is wired -------------------------- -OUT=$(bash "$HOOK" <<<"$INPUT" 2>&1); RC=$? -assert_exit "unwired → exit 0" 0 "$RC" -assert_silent "unwired → silent" "$OUT" - -# --- Kill switch suppresses emission --------------------------------------- -TELK="$TEST_TMPDIR/telk.json"; SINKK="$(make_sink "$TELK")" -env HOOK_TELEMETRY_SINK="$SINKK" CLAUDE_PLUGIN_OPTION_API_ERROR_AUDIT_ENABLED=false \ - bash "$HOOK" <<<"$INPUT" >/dev/null 2>&1 -assert_file_absent "kill switch → no envelope" "$TELK" - -# --- Missing error field is a silent skip ---------------------------------- -TELM="$TEST_TMPDIR/telm.json"; SINKM="$(make_sink "$TELM")" -env HOOK_TELEMETRY_SINK="$SINKM" bash "$HOOK" <<<'{}' >/dev/null 2>&1 -assert_file_absent "missing error → no envelope" "$TELM" - -report diff --git a/plugins/claude-ops/hooks/audit-event-emitter.sh b/plugins/claude-ops/hooks/audit-event-emitter.sh new file mode 100755 index 0000000000..25dd5669e2 --- /dev/null +++ b/plugins/claude-ops/hooks/audit-event-emitter.sh @@ -0,0 +1,264 @@ +#!/usr/bin/env bash +# One emitter for the audit telemetry rows that differ only in an event, a kill +# switch and a payload projection. Registered in hooks.json on each of those +# events and dispatching on the payload's `hook_event_name`, the same shape +# session-event-log.sh uses to serve ~30 events from one script. +# +# Sharing a script shares no identity and no switch. Each row emits its own +# telemetry `hook` id, `hook_event`, `status` and `data` fields, and reads its +# own `_enabled` userConfig boolean from +# CLAUDE_PLUGIN_OPTION__ENABLED, so a downstream reader sees independent +# producers and an operator turns one off without touching the rest. +# +# event | hook id | switch +# StopFailure | api-error-audit | API_ERROR_AUDIT +# ConfigChange | config-change-audit | CONFIG_CHANGE_AUDIT +# PreCompact | pre-compact-audit | PRE_COMPACT_AUDIT +# PostToolUseFailure | tool-failure-audit | TOOL_FAILURE_AUDIT +# PermissionDenied | permission-denied-audit | PERMISSION_DENIED_AUDIT +# InstructionsLoaded | instructions-loaded-audit | INSTRUCTIONS_LOADED_AUDIT +# UserPromptExpansion | skill-usage-audit | SKILL_USAGE_AUDIT +# +# The seven events are distinct, so the event alone selects the row and no +# matcher field is read to disambiguate. +# +# NON-BLOCKING on every row: exit 0 always, never a decision or a retry. +# Privacy is per row, and no row captures a command body, an error message, an +# absolute path, or an argument body. +# Pure telemetry emitter on six rows: no sink wired (HOOK_TELEMETRY_SINK unset) +# → no-op. The UserPromptExpansion row also writes the skill-usage.jsonl second +# store, which is unconditional and needs no sink. + +set -uo pipefail + +# All seven rows off means nothing below can run, so leave before parsing the +# library. One disabled row still reaches the library and buffers the payload: +# the row is only known once the event is read, and the payload is the only +# place the event arrives. +_any_on=0 +for _opt in API_ERROR_AUDIT CONFIG_CHANGE_AUDIT INSTRUCTIONS_LOADED_AUDIT \ + PERMISSION_DENIED_AUDIT PRE_COMPACT_AUDIT SKILL_USAGE_AUDIT TOOL_FAILURE_AUDIT; do + _var="CLAUDE_PLUGIN_OPTION_${_opt}_ENABLED" + if [[ "${!_var:-true}" == "true" ]]; then + _any_on=1 + break + fi +done +((_any_on)) || exit 0 + +# Hook directory by parameter expansion, never `dirname`. GNU Bash forks a +# subshell for every command substitution even when the body is a builtin +# (Command Substitution, Bash Reference Manual). On Windows Git Bash that +# fork is a process. `${BASH_SOURCE[0]%/*}` equals dirname for every shape +# BASH_SOURCE takes; the fallback covers a bare filename, where the strip is a +# no-op and dirname answers `.`. +HOOK_DIR="${BASH_SOURCE[0]%/*}" +[[ "$HOOK_DIR" == "${BASH_SOURCE[0]}" ]] && HOOK_DIR=. + +# shellcheck source=hook-utils.sh +source "$HOOK_DIR/hook-utils.sh" + +START=${EPOCHREALTIME:-} + +hook::buffer_stdin_to INPUT || exit 0 + +# The row selector. A bash match over the buffered payload, no extra process. +# `hook_event_name` is a common field on every hook payload; a payload without +# one names no row and is a silent skip. +EVENT="" +[[ "$INPUT" =~ \"hook_event_name\"[[:space:]]*:[[:space:]]*\"([A-Za-z]+)\" ]] && EVENT="${BASH_REMATCH[1]}" + +# data.session_id (additive, hook-telemetry rule 1): the sink routes an +# envelope carrying one into the per-session log beside session-event-log.sh. +# The library also puts it on the envelope spine; both are sent, because a sink +# on contract 1.0 reads only data. Extracted once here for every row. +# Empty when the payload carries none, and the key is then left out of data. +SESSION_ID="" +[[ "$INPUT" =~ \"session_id\"[[:space:]]*:[[:space:]]*\"([A-Za-z0-9._-]+)\" ]] && SESSION_ID="${BASH_REMATCH[1]}" + +# emit::subject_row +# The rows whose whole projection is one payload field as the subject. An +# absent or empty field is a silent skip. +emit::subject_row() { + local subject data + subject=$(hook::jq_field "$INPUT" "$1") || return 0 + data=$(jq -nc --arg session_id "$SESSION_ID" --arg subject "$subject" \ + '{subject: $subject} + (if $session_id == "" then {} else {session_id: $session_id} end)') + hook::emit_telemetry "$2" "$3" "$4" "$START" "$data" "${CLAUDE_PROJECT_DIR:-}" +} + +# emit::tool_row +# The rows keyed on a tool call. Privacy-safe subject convention: +# Bash → "Bash:" (e.g. "Bash:git", "Bash:dotnet") +# Write|Edit → tool_name only (no file_path leak) +# other → tool_name only +# Full command strings, error messages, file paths and stdin are NEVER captured. +# +# Both payload fields in ONE jq process (hook::jq_fields), not two: a jq spawn is +# ~140 ms of fork() emulation on Windows Git Bash. A missing jq or an unparsable +# payload returns non-zero here and skips, the same silent skip an absent +# tool_name takes below; an absent `.tool_input.command` arrives as the empty +# string the subject helper tolerates. +emit::tool_row() { + local tool cmd subject data + hook::jq_fields "$INPUT" '.tool_name' '.tool_input.command' || return 0 + tool="${HOOK_JQ_FIELDS[0]}" + [[ -n "$tool" ]] || return 0 + cmd="${HOOK_JQ_FIELDS[1]}" + subject=$(hook::extract_bash_subject "$tool" "$cmd") + data=$(jq -nc --arg session_id "$SESSION_ID" --arg subject "$subject" --arg tool "$tool" \ + '{subject: $subject, tool: $tool} + (if $session_id == "" then {} else {session_id: $session_id} end)') + hook::emit_telemetry "$1" "$2" "$3" "$START" "$data" "${CLAUDE_PROJECT_DIR:-}" +} + +# The InstructionsLoaded row. Its subject is ":" +# so query analysis can group loads by reason without parsing extra fields. +# +# Write-time filter: session_start loads are deterministic (the same always-load +# files fire every boot) and high-volume, so they are dropped by default. Opt +# back in for one-off debugging with +# CLAUDE_PLUGIN_OPTION_INSTRUCTIONS_LOADED_AUDIT_LOG_SESSION_START=true. +emit::instructions_loaded_row() { + local file_path load_reason file_disp project_dir subject data + # Both payload fields in ONE jq process; see emit::tool_row for why. + hook::jq_fields "$INPUT" '.file_path' '.load_reason' || return 0 + file_path="${HOOK_JQ_FIELDS[0]}" + load_reason="${HOOK_JQ_FIELDS[1]}" + + # Need at least one of the two; a pure missing payload is a silent skip. + [[ -n "$file_path$load_reason" ]] || return 0 + + if [[ "$load_reason" == "session_start" && + "${CLAUDE_PLUGIN_OPTION_INSTRUCTIONS_LOADED_AUDIT_LOG_SESSION_START:-false}" != "true" ]]; then + return 0 + fi + + # Privacy: InstructionsLoaded `file_path` is absolute, so logging it verbatim + # would leak local usernames / private directory names into the shared + # observability store. Reduce to a repo-relative path when the file is under + # the project root, else to its basename — never the absolute prefix. + file_disp="" + if [[ -n "$file_path" ]]; then + project_dir=$(hook::repo_root "${CLAUDE_PROJECT_DIR:-.}") + case "$file_path" in + "$project_dir"/*) file_disp="${file_path#"$project_dir"/}" ;; + *) file_disp="${file_path##*/}" ;; + esac + fi + + subject="${file_disp}:${load_reason}" + data=$(jq -nc --arg session_id "$SESSION_ID" --arg subject "$subject" \ + '{subject: $subject} + (if $session_id == "" then {} else {session_id: $session_id} end)') + hook::emit_telemetry "instructions-loaded-audit" "InstructionsLoaded" "ok" \ + "$START" "$data" "${CLAUDE_PROJECT_DIR:-}" +} + +# The UserPromptExpansion row: user-typed slash-command / MCP-prompt +# invocations, the second producer of the skill-usage-audit signal. The +# PostToolUse/Skill producer (skill-usage-audit.sh, its own file on the hot +# per-tool-call path) only fires when the MODEL invokes the Skill tool, so a +# user who types "/skill" directly is missed by that path. The two events are +# disjoint, so no dedup is required; each carries a `source` field (`tool` vs +# `expansion`) so consumers can tell the paths apart, and both share one +# telemetry `hook` id and one store. +# +# Two outputs: +# 1. skill-usage.jsonl (SkillUse events), written UNCONDITIONALLY at the +# scope-selected destination (skill_usage_scope: repo | user | data-dir; +# skill_usage_dir, else .claude/observability, for the repo/user scopes). +# 2. The telemetry envelope, only when a consumer wires HOOK_TELEMETRY_SINK. +# +# Registered with NO matcher, so it fires for every expanded command; emission +# keys on command_name (always present). expansion_type (slash_command vs +# mcp_prompt) is recorded when present so the distinction is preserved +# downstream, but is never gated on — a CC build that omits it must still record +# the user-typed path rather than silently drop it. +# +# Captures the command name only — no argument body. +emit::skill_expansion_row() { + local skill exp_type data + skill=$(hook::jq_field "$INPUT" '.command_name') || return 0 + skill="${skill#/}" + + # Fed through `printf | jq` rather than a here-string: bash fills a + # here-string's pipe itself, so a payload at or above the pipe capacity blocks + # before jq runs. + exp_type=$(printf '%s' "$INPUT" | jq -r '(.expansion_type // empty) | gsub("\r";"")' 2>/dev/null) + + claude_ops::record_skill_use "UserPromptExpansion" "skill-usage-expansion-audit" \ + "$INPUT" "$skill" "expansion" "$exp_type" + + hook::telemetry_enabled || return 0 + data=$(jq -nc --arg session_id "$SESSION_ID" --arg subject "Skill:$skill" --arg skill "$skill" --arg exp "$exp_type" \ + '{subject: $subject, skill: $skill, source: "expansion"} + + (if $exp != "" then {expansion_type: $exp} else {} end) + (if $session_id == "" then {} else {session_id: $session_id} end)') + hook::emit_telemetry "skill-usage-audit" "UserPromptExpansion" "ok" \ + "$START" "$data" "${CLAUDE_PROJECT_DIR:-}" +} + +case "$EVENT" in +StopFailure) + # ADVISORY: StopFailure output and exit code are ignored. The subject is the + # `error` type only: `error_details` may carry prompt fragments or session + # metadata, so it is never captured. + hook::is_enabled "API_ERROR_AUDIT" || exit 0 + hook::telemetry_enabled || exit 0 + emit::subject_row '.error' "api-error-audit" "StopFailure" "error" + ;; +ConfigChange) + # Settings/skills mutations. Matcher-scoped upstream to + # user_settings|project_settings|local_settings|skills; policy_settings is + # excluded there (cannot be blocked, low signal). The subject is the source + # identifier, which is a path-free label. + hook::is_enabled "CONFIG_CHANGE_AUDIT" || exit 0 + hook::telemetry_enabled || exit 0 + emit::subject_row '.source' "config-change-audit" "ConfigChange" "ok" + ;; +PreCompact) + # A compaction trigger (manual|auto), for diagnosing autocompact-loop + # regressions and validating a CLAUDE_CODE_AUTO_COMPACT_WINDOW threshold. + hook::is_enabled "PRE_COMPACT_AUDIT" || exit 0 + hook::telemetry_enabled || exit 0 + emit::subject_row '.trigger' "pre-compact-audit" "PreCompact" "ok" + ;; +PostToolUseFailure) + # Write/Edit/Bash failures. Complements the success-only PostToolUse + # write-side hooks. + hook::is_enabled "TOOL_FAILURE_AUDIT" || exit 0 + hook::telemetry_enabled || exit 0 + emit::tool_row "tool-failure-audit" "PostToolUseFailure" "error" + ;; +PermissionDenied) + # The auto-mode classifier blocking a tool call (distinct from + # PermissionRequest, which fires on all permission dialogs). Never returns + # retry:true — denials warrant human review, which is the whole point of the + # classifier blocking the action. + hook::is_enabled "PERMISSION_DENIED_AUDIT" || exit 0 + hook::telemetry_enabled || exit 0 + emit::tool_row "permission-denied-audit" "PermissionDenied" "blocked" + ;; +InstructionsLoaded) + # ADVISORY: InstructionsLoaded exit code is ignored, so this never gates a + # load. Lets a consumer validate that `paths:` frontmatter is matching and + # @-includes resolve (which rules actually load). + hook::is_enabled "INSTRUCTIONS_LOADED_AUDIT" || exit 0 + hook::telemetry_enabled || exit 0 + emit::instructions_loaded_row + ;; +UserPromptExpansion) + # The only row with an output that does not need a sink, so it runs before + # the telemetry gate. claude-ops-paths.sh is sourced here, not at the top: + # the other six rows never touch the second store and must not parse it. + hook::is_enabled "SKILL_USAGE_AUDIT" || exit 0 + # shellcheck source=claude-ops-paths.sh + source "$HOOK_DIR/claude-ops-paths.sh" + emit::skill_expansion_row + ;; +*) + # An event this script names no row for: a payload carrying no + # hook_event_name, or a hooks.json entry registered ahead of its row. A + # silent skip, like every other unmatched payload here. + ;; +esac + +exit 0 diff --git a/plugins/claude-ops/hooks/audit-event-emitter.test.sh b/plugins/claude-ops/hooks/audit-event-emitter.test.sh new file mode 100644 index 0000000000..5b3a905e23 --- /dev/null +++ b/plugins/claude-ops/hooks/audit-event-emitter.test.sh @@ -0,0 +1,276 @@ +#!/usr/bin/env bash +# Contract test for audit-event-emitter.sh (claude-ops plugin). Black-box. +# +# The emitter serves seven audit rows from one script, dispatching on the +# payload's hook_event_name, so the suite is a table over those rows: each row's +# telemetry `hook` id, `hook_event`, `status` and `data.subject` are asserted +# from one loop, and so are its kill switch, its unwired behavior and its +# missing-required-field skip. What a row does BEYOND that table — the +# privacy reductions, the InstructionsLoaded session_start filter, and the +# UserPromptExpansion second store — follows the table in its own section. +set -uo pipefail + +HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +HOOK="$HOOK_DIR/audit-event-emitter.sh" +TEST_TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TEST_TMPDIR"' EXIT + +# shellcheck source=claude-ops-test-helpers.sh +source "$HOOK_DIR/claude-ops-test-helpers.sh" +unset CLAUDE_PROJECT_DIR + +# A non-git project root, so hook::repo_root answers PROJ itself and the +# InstructionsLoaded row's path reduction is deterministic. The +# UserPromptExpansion row writes its second store under a per-case project. +PROJ="$TEST_TMPDIR/proj" +mkdir -p "$PROJ/.claude/rules" +: >"$PROJ/.claude/rules/x.md" + +# --- The row table ---------------------------------------------------------- +# Tab-separated: id, switch suffix, payload, hook id, hook_event, status, +# data.subject, and a payload that reaches the row with its required field +# missing. `%PROJ%` expands to the fixture project root. +ROWS=( + $'api-error\tAPI_ERROR_AUDIT\t{"session_id":"sess-1","hook_event_name":"StopFailure","error":"rate_limit"}\tapi-error-audit\tStopFailure\terror\trate_limit\t{"hook_event_name":"StopFailure"}' + $'config-change\tCONFIG_CHANGE_AUDIT\t{"session_id":"sess-1","hook_event_name":"ConfigChange","source":"project_settings"}\tconfig-change-audit\tConfigChange\tok\tproject_settings\t{"hook_event_name":"ConfigChange"}' + $'pre-compact\tPRE_COMPACT_AUDIT\t{"session_id":"sess-1","hook_event_name":"PreCompact","trigger":"auto"}\tpre-compact-audit\tPreCompact\tok\tauto\t{"hook_event_name":"PreCompact"}' + $'tool-failure\tTOOL_FAILURE_AUDIT\t{"session_id":"sess-1","hook_event_name":"PostToolUseFailure","tool_name":"Bash","tool_input":{"command":"FOO=bar dotnet build"}}\ttool-failure-audit\tPostToolUseFailure\terror\tBash:dotnet\t{"hook_event_name":"PostToolUseFailure","tool_input":{"command":"ls"}}' + $'permission-denied\tPERMISSION_DENIED_AUDIT\t{"session_id":"sess-1","hook_event_name":"PermissionDenied","tool_name":"Bash","tool_input":{"command":"git push --force origin main"}}\tpermission-denied-audit\tPermissionDenied\tblocked\tBash:git\t{"hook_event_name":"PermissionDenied","tool_input":{"command":"ls"}}' + $'instructions-loaded\tINSTRUCTIONS_LOADED_AUDIT\t{"session_id":"sess-1","hook_event_name":"InstructionsLoaded","file_path":"%PROJ%/.claude/rules/x.md","load_reason":"path_glob_match"}\tinstructions-loaded-audit\tInstructionsLoaded\tok\t.claude/rules/x.md:path_glob_match\t{"hook_event_name":"InstructionsLoaded"}' + $'skill-expansion\tSKILL_USAGE_AUDIT\t{"session_id":"sess-1","hook_event_name":"UserPromptExpansion","command_name":"/research","expansion_type":"slash_command"}\tskill-usage-audit\tUserPromptExpansion\tok\tSkill:research\t{"hook_event_name":"UserPromptExpansion","expansion_type":"slash_command"}' +) + +n=0 +for row in "${ROWS[@]}"; do + n=$((n + 1)) + IFS=$'\t' read -r id switch payload hook_id event status subject missing <<<"$row" + payload="${payload//%PROJ%/$PROJ}" + # Each row gets its own project root so the UserPromptExpansion second store + # cannot be confused with another case's. + rowproj="$TEST_TMPDIR/rowproj-$id" + mkdir -p "$rowproj/.claude/rules" + : >"$rowproj/.claude/rules/x.md" + + # --- the envelope this row emits ----------------------------------------- + TEL="$TEST_TMPDIR/$id.json" + if emit_envelope "$HOOK" "$payload" "$TEL" CLAUDE_PROJECT_DIR="$PROJ"; then + assert_eq "$id: hook id" "$hook_id" "$(jq -r '.hook' "$TEL")" + assert_eq "$id: hook_event" "$event" "$(jq -r '.hook_event' "$TEL")" + assert_eq "$id: status" "$status" "$(jq -r '.status' "$TEL")" + assert_eq "$id: data.subject" "$subject" "$(jq -r '.data.subject' "$TEL")" + assert_eq "$id: schema_version" "1.1" "$(jq -r '.schema_version' "$TEL")" + assert_eq "$id: spine session_id (1.1)" "sess-1" "$(jq -r '.session_id' "$TEL")" + assert_eq "$id: data.session_id still sent for a 1.0 sink" "sess-1" "$(jq -r '.data.session_id' "$TEL")" + else + bad "$id: no envelope written when a sink is wired" + fi + + # --- this row's kill switch, and only this row's ------------------------- + expect_no_envelope "$id: kill switch → no envelope" \ + "$HOOK" "$payload" "$TEST_TMPDIR/$id.killed.json" \ + CLAUDE_PROJECT_DIR="$rowproj" "CLAUDE_PLUGIN_OPTION_${switch}_ENABLED=false" + + # --- unwired: exit 0, and nothing on stdout or stderr -------------------- + OUT=$(env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR="$rowproj" bash "$HOOK" <<<"$payload" 2>&1) + RC=$? + assert_exit "$id: unwired → exit 0" 0 "$RC" + assert_silent "$id: unwired → silent" "$OUT" + + # --- the required field is missing: a silent skip ------------------------ + expect_no_envelope "$id: missing required field → no envelope" \ + "$HOOK" "$missing" "$TEST_TMPDIR/$id.missing.json" CLAUDE_PROJECT_DIR="$rowproj" +done +assert_eq "every row in the table was driven" 7 "$n" + +# --- One row's switch does not silence another ------------------------------ +TELX="$TEST_TMPDIR/cross-switch.json" +if emit_envelope "$HOOK" \ + '{"session_id":"sess-1","hook_event_name":"PreCompact","trigger":"auto"}' "$TELX" \ + CLAUDE_PROJECT_DIR="$PROJ" CLAUDE_PLUGIN_OPTION_API_ERROR_AUDIT_ENABLED=false; then + assert_eq "another row's switch leaves this one emitting" "pre-compact-audit" "$(jq -r '.hook' "$TELX")" +else + bad "a disabled sibling row suppressed pre-compact-audit" +fi + +# --- No row for the event → silent skip ------------------------------------- +expect_no_envelope "unknown event → no envelope" \ + "$HOOK" '{"session_id":"sess-1","hook_event_name":"SessionStart","source":"startup"}' \ + "$TEST_TMPDIR/unknown-event.json" CLAUDE_PROJECT_DIR="$PROJ" +expect_no_envelope "no hook_event_name → no envelope" \ + "$HOOK" '{"session_id":"sess-1","error":"rate_limit"}' \ + "$TEST_TMPDIR/no-event.json" CLAUDE_PROJECT_DIR="$PROJ" + +# --- Privacy: the tool rows never carry a command body ---------------------- +for probe in "PostToolUseFailure tool-failure" "PermissionDenied permission-denied"; do + read -r pev pid <<<"$probe" + TELP="$TEST_TMPDIR/$pid.privacy.json" + if emit_envelope "$HOOK" \ + "{\"hook_event_name\":\"$pev\",\"tool_name\":\"Bash\",\"tool_input\":{\"command\":\"TOKEN=\\\"secret value\\\" curl https://x\"}}" \ + "$TELP" CLAUDE_PROJECT_DIR="$PROJ"; then + assert_eq "$pid: quoted assignment → bare Bash subject" "Bash" "$(jq -r '.data.subject' "$TELP")" + assert_absent "$pid: no value fragment leaked" "$(cat "$TELP")" "secret" + assert_absent "$pid: no value fragment leaked (2)" "$(cat "$TELP")" "value" + assert_absent "$pid: no command body leaked" "$(cat "$TELP")" "curl" + else + bad "$pid: no envelope for a quoted-assignment command" + fi + + TELW="$TEST_TMPDIR/$pid.nonbash.json" + if emit_envelope "$HOOK" \ + "{\"hook_event_name\":\"$pev\",\"tool_name\":\"Write\",\"tool_input\":{\"file_path\":\"secret.env\"}}" \ + "$TELW" CLAUDE_PROJECT_DIR="$PROJ"; then + assert_eq "$pid: non-Bash subject = tool name" "Write" "$(jq -r '.data.subject' "$TELW")" + assert_eq "$pid: data.tool" "Write" "$(jq -r '.data.tool' "$TELW")" + assert_absent "$pid: file_path not captured" "$(cat "$TELW")" "secret.env" + else + bad "$pid: no envelope for a non-Bash tool" + fi +done + +# --- InstructionsLoaded: path reduction and the session_start filter -------- +TELB="$TEST_TMPDIR/il-outside.json" +OUTSIDE="$TEST_TMPDIR/elsewhere/private-dir/CLAUDE.md" +if emit_envelope "$HOOK" \ + "$(MSYS_NO_PATHCONV=1 jq -nc --arg fp "$OUTSIDE" '{hook_event_name:"InstructionsLoaded", file_path:$fp, load_reason:"include"}')" \ + "$TELB" CLAUDE_PROJECT_DIR="$PROJ"; then + assert_eq "instructions-loaded: out-of-repo → basename subject" "CLAUDE.md:include" "$(jq -r '.data.subject' "$TELB")" + assert_absent "instructions-loaded: no parent dir leaked" "$(cat "$TELB")" "private-dir" +else + bad "instructions-loaded: no envelope for an out-of-repo path" +fi + +TELA="$TEST_TMPDIR/il-inrepo.json" +if emit_envelope "$HOOK" \ + "$(MSYS_NO_PATHCONV=1 jq -nc --arg fp "$PROJ/.claude/rules/x.md" '{hook_event_name:"InstructionsLoaded", file_path:$fp, load_reason:"path_glob_match"}')" \ + "$TELA" CLAUDE_PROJECT_DIR="$PROJ"; then + assert_absent "instructions-loaded: no absolute prefix leaked" "$(cat "$TELA")" "$PROJ" +else + bad "instructions-loaded: no envelope for an in-repo path" +fi + +SS_PAYLOAD="$(MSYS_NO_PATHCONV=1 jq -nc --arg fp "$PROJ/CLAUDE.md" '{hook_event_name:"InstructionsLoaded", file_path:$fp, load_reason:"session_start"}')" +expect_no_envelope "instructions-loaded: session_start filtered by default" \ + "$HOOK" "$SS_PAYLOAD" "$TEST_TMPDIR/il-ss.json" CLAUDE_PROJECT_DIR="$PROJ" + +TELO="$TEST_TMPDIR/il-ss-opt-in.json" +if emit_envelope "$HOOK" "$SS_PAYLOAD" "$TELO" CLAUDE_PROJECT_DIR="$PROJ" \ + CLAUDE_PLUGIN_OPTION_INSTRUCTIONS_LOADED_AUDIT_LOG_SESSION_START=true; then + assert_eq "instructions-loaded: session_start opt-in subject" "CLAUDE.md:session_start" "$(jq -r '.data.subject' "$TELO")" +else + bad "instructions-loaded: session_start opt-in did not emit" +fi + +# --- UserPromptExpansion: the second store, written without a sink ---------- +EXP='{"hook_event_name":"UserPromptExpansion","command_name":"/research","expansion_type":"slash_command"}' + +PROJS="$TEST_TMPDIR/exp-store" +mkdir -p "$PROJS" +env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR="$PROJS" bash "$HOOK" <<<"$EXP" >/dev/null 2>&1 +STORE="$PROJS/.claude/observability/skill-usage.jsonl" +if [[ -s "$STORE" ]]; then + assert_eq "second store event" "SkillUse" "$(jq -r '.event' "$STORE")" + assert_eq "second store skill (slash stripped)" "research" "$(jq -r '.skill' "$STORE")" + assert_eq "second store hook (unified)" "skill-usage-audit" "$(jq -r '.hook' "$STORE")" + assert_eq "second store source" "expansion" "$(jq -r '.source' "$STORE")" + assert_eq "second store expansion_type" "slash_command" "$(jq -r '.expansion_type' "$STORE")" +else + bad "second store not written (unconditional, no sink)" +fi + +# expansion_type is optional: recorded when present, never gated on. +PROJX="$TEST_TMPDIR/exp-noexp" +mkdir -p "$PROJX" +env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR="$PROJX" \ + bash "$HOOK" <<<'{"hook_event_name":"UserPromptExpansion","command_name":"deploy"}' >/dev/null 2>&1 +STOREX="$PROJX/.claude/observability/skill-usage.jsonl" +if [[ -s "$STOREX" ]]; then + assert_eq "no-expansion_type skill" "deploy" "$(jq -r '.skill' "$STOREX")" + assert_eq "no-expansion_type source" "expansion" "$(jq -r '.source' "$STOREX")" + assert_eq "expansion_type key absent" "false" "$(jq -r 'has("expansion_type")' "$STOREX")" +else + bad "second store not written when expansion_type is absent" +fi + +PROJM="$TEST_TMPDIR/exp-mcp" +mkdir -p "$PROJM" +env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR="$PROJM" \ + bash "$HOOK" <<<'{"hook_event_name":"UserPromptExpansion","command_name":"ask","expansion_type":"mcp_prompt"}' >/dev/null 2>&1 +assert_eq "mcp_prompt recorded" "mcp_prompt" \ + "$(jq -r '.expansion_type' "$PROJM/.claude/observability/skill-usage.jsonl" 2>/dev/null)" + +PROJ2="$TEST_TMPDIR/exp-dir" +mkdir -p "$PROJ2" +env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR="$PROJ2" \ + CLAUDE_PLUGIN_OPTION_SKILL_USAGE_DIR="telemetry/skills" \ + bash "$HOOK" <<<"$EXP" >/dev/null 2>&1 +assert_eq "skill_usage_dir override used" "research" \ + "$(jq -r '.skill' "$PROJ2/telemetry/skills/skill-usage.jsonl" 2>/dev/null)" + +# An invalid override is visible and cannot escape the project. CLAUDE_PLUGIN_DATA +# is isolated because the advisory goes through hook::notice_once, which persists +# a once-per-session marker there. +PROJB="$TEST_TMPDIR/exp-bad" +mkdir -p "$PROJB" +BADCFG_DATA="$TEST_TMPDIR/badcfg-data" +mkdir -p "$BADCFG_DATA" +INVALID_OUTPUT=$(env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR="$PROJB" \ + CLAUDE_PLUGIN_OPTION_SKILL_USAGE_DIR="../outside" \ + CLAUDE_PLUGIN_DATA="$BADCFG_DATA" \ + bash "$HOOK" <<<"$EXP" 2>/dev/null) +assert_file_absent "traversal override cannot write outside the project" \ + "$TEST_TMPDIR/outside/skill-usage.jsonl" +assert_contains "invalid override emits a visible advisory" "$INVALID_OUTPUT" \ + "claude-ops skipped skill-usage logging" +assert_eq "invalid override advisory uses the hook protocol" "UserPromptExpansion" \ + "$(jq -r '.hookSpecificOutput.hookEventName' <<<"$INVALID_OUTPUT" 2>/dev/null)" +assert_contains "invalid override advisory is user-visible (systemMessage)" \ + "$(jq -r '.systemMessage // empty' <<<"$INVALID_OUTPUT" 2>/dev/null)" \ + "claude-ops skipped skill-usage logging" + +PROJU="$TEST_TMPDIR/exp-user" +HOMEU="$TEST_TMPDIR/exp-home" +mkdir -p "$PROJU" "$HOMEU" +env -u HOOK_TELEMETRY_SINK CLAUDE_PROJECT_DIR="$PROJU" HOME="$HOMEU" \ + CLAUDE_PLUGIN_OPTION_SKILL_USAGE_SCOPE="user" \ + bash "$HOOK" <<<"$EXP" >/dev/null 2>&1 +assert_eq "user scope writes under HOME" "research" \ + "$(jq -r '.skill' "$HOMEU/.claude/observability/skill-usage.jsonl" 2>/dev/null)" +assert_eq "user-scope row carries the project field" "exp-user" \ + "$(jq -r '.project' "$HOMEU/.claude/observability/skill-usage.jsonl" 2>/dev/null)" +assert_file_absent "user scope leaves the project tree untouched" \ + "$PROJU/.claude/observability/skill-usage.jsonl" + +# The shared switch suppresses BOTH of this row's outputs. +PROJK="$TEST_TMPDIR/exp-killed" +mkdir -p "$PROJK" +expect_no_envelope "skill-expansion: kill switch → no envelope (again, with the store)" \ + "$HOOK" "$EXP" "$TEST_TMPDIR/exp-killed.json" \ + CLAUDE_PROJECT_DIR="$PROJK" CLAUDE_PLUGIN_OPTION_SKILL_USAGE_AUDIT_ENABLED=false +assert_file_absent "skill-expansion: kill switch → no second store" \ + "$PROJK/.claude/observability/skill-usage.jsonl" + +# A missing command_name skips both outputs. +PROJN="$TEST_TMPDIR/exp-nocmd" +mkdir -p "$PROJN" +expect_no_envelope "skill-expansion: no command_name → no envelope" \ + "$HOOK" '{"hook_event_name":"UserPromptExpansion","expansion_type":"slash_command"}' \ + "$TEST_TMPDIR/exp-nocmd.json" CLAUDE_PROJECT_DIR="$PROJN" +assert_file_absent "skill-expansion: no command_name → no second store" \ + "$PROJN/.claude/observability/skill-usage.jsonl" + +# --- Every switch off: the emitter leaves before parsing the library -------- +PROJA="$TEST_TMPDIR/all-off" +mkdir -p "$PROJA" +expect_no_envelope "all switches off → no envelope" \ + "$HOOK" "$EXP" "$TEST_TMPDIR/all-off.json" CLAUDE_PROJECT_DIR="$PROJA" \ + CLAUDE_PLUGIN_OPTION_API_ERROR_AUDIT_ENABLED=false \ + CLAUDE_PLUGIN_OPTION_CONFIG_CHANGE_AUDIT_ENABLED=false \ + CLAUDE_PLUGIN_OPTION_INSTRUCTIONS_LOADED_AUDIT_ENABLED=false \ + CLAUDE_PLUGIN_OPTION_PERMISSION_DENIED_AUDIT_ENABLED=false \ + CLAUDE_PLUGIN_OPTION_PRE_COMPACT_AUDIT_ENABLED=false \ + CLAUDE_PLUGIN_OPTION_SKILL_USAGE_AUDIT_ENABLED=false \ + CLAUDE_PLUGIN_OPTION_TOOL_FAILURE_AUDIT_ENABLED=false +assert_file_absent "all switches off → no second store" \ + "$PROJA/.claude/observability/skill-usage.jsonl" + +report diff --git a/plugins/claude-ops/hooks/audit-session-id.test.sh b/plugins/claude-ops/hooks/audit-session-id.test.sh index 9b7c356e01..014aa785e0 100755 --- a/plugins/claude-ops/hooks/audit-session-id.test.sh +++ b/plugins/claude-ops/hooks/audit-session-id.test.sh @@ -1,13 +1,13 @@ #!/usr/bin/env bash -# The nine claude-ops audit hooks put the payload's session_id into their +# The nine claude-ops audit rows put the payload's session_id into their # envelope `data` (additive, docs/conventions/hook-telemetry rule 1) so the # reference sink can route the line into the per-session log. One suite for -# all of them: each hook is driven black-box with a minimal payload that +# all of them: each row is driven black-box with a minimal payload that # carries a session_id, and once without one, and the captured envelope is -# read back. Covers: api-error-audit.sh config-change-audit.sh -# instructions-loaded-audit.sh permission-denied-audit.sh pre-compact-audit.sh -# skill-usage-audit.sh skill-usage-expansion-audit.sh tool-failure-audit.sh -# hook-failure-audit.sh +# read back. Seven rows are served by audit-event-emitter.sh, which dispatches +# on hook_event_name, so every payload here carries that key as Claude Code +# sends it; skill-usage-audit.sh and hook-failure-audit.sh keep their own +# scripts. set -uo pipefail HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -27,30 +27,39 @@ mkdir -p "$DATA_DIR" TRANSCRIPT="$TEST_TMPDIR/transcript.jsonl" printf '{"attachment":{"type":"hook_non_blocking_error","hookName":"PreToolUse:demo","toolUseID":"toolu_x","hookEvent":"PreToolUse","stderr":"boom","stdout":"","exitCode":1,"command":"bash demo.sh","durationMs":2},"type":"attachment","uuid":"u","session_id":"s"}\n' >"$TRANSCRIPT" -# hook -> the payload members (without session_id) that make it emit +# row -> the payload members (without session_id) that make it emit, +# hook_event_name included members() { case "$1" in - api-error-audit) printf '"error":"rate_limit"' ;; - config-change-audit) printf '"source":"project_settings"' ;; - instructions-loaded-audit) printf '"file_path":"%s/.claude/rules/x.md","load_reason":"path_glob_match"' "$PROJ" ;; - permission-denied-audit) printf '"tool_name":"Bash","tool_input":{"command":"git push"}' ;; - pre-compact-audit) printf '"trigger":"auto"' ;; - skill-usage-audit) printf '"tool_name":"Skill","tool_input":{"skill":"/research"}' ;; - skill-usage-expansion-audit) printf '"command_name":"/research","expansion_type":"slash_command"' ;; - tool-failure-audit) printf '"tool_name":"Bash","tool_input":{"command":"dotnet build"}' ;; + api-error-audit) printf '"hook_event_name":"StopFailure","error":"rate_limit"' ;; + config-change-audit) printf '"hook_event_name":"ConfigChange","source":"project_settings"' ;; + instructions-loaded-audit) printf '"hook_event_name":"InstructionsLoaded","file_path":"%s/.claude/rules/x.md","load_reason":"path_glob_match"' "$PROJ" ;; + permission-denied-audit) printf '"hook_event_name":"PermissionDenied","tool_name":"Bash","tool_input":{"command":"git push"}' ;; + pre-compact-audit) printf '"hook_event_name":"PreCompact","trigger":"auto"' ;; + skill-usage-audit) printf '"hook_event_name":"PostToolUse","tool_name":"Skill","tool_input":{"skill":"/research"}' ;; + skill-usage-expansion-audit) printf '"hook_event_name":"UserPromptExpansion","command_name":"/research","expansion_type":"slash_command"' ;; + tool-failure-audit) printf '"hook_event_name":"PostToolUseFailure","tool_name":"Bash","tool_input":{"command":"dotnet build"}' ;; hook-failure-audit) printf '"transcript_path":"%s","hook_event_name":"Stop"' "$TRANSCRIPT" ;; *) printf '' ;; esac } -# drive +# row -> the script that serves it +script_for() { + case "$1" in + skill-usage-audit | hook-failure-audit) printf '%s.sh' "$1" ;; + *) printf 'audit-event-emitter.sh' ;; + esac +} + +# drive drive() { local sink : >"$3" sink="$(make_sink "$3")" env HOOK_TELEMETRY_SINK="$sink" CLAUDE_PROJECT_DIR="$PROJ" CLAUDE_PLUGIN_DATA="$DATA_DIR" \ CLAUDE_PLUGIN_OPTION_SKILL_USAGE_SCOPE=data-dir \ - bash "$HOOK_DIR/$1.sh" <<<"$2" >/dev/null 2>&1 + bash "$HOOK_DIR/$(script_for "$1")" <<<"$2" >/dev/null 2>&1 } for hook in api-error-audit config-change-audit instructions-loaded-audit permission-denied-audit \ diff --git a/plugins/claude-ops/hooks/claude-ops-paths.sh b/plugins/claude-ops/hooks/claude-ops-paths.sh index 0949d64ad3..ed53d0dd83 100644 --- a/plugins/claude-ops/hooks/claude-ops-paths.sh +++ b/plugins/claude-ops/hooks/claude-ops-paths.sh @@ -157,7 +157,7 @@ claude_ops::ensure_git_exclude() { # Append one SkillUse line to the scope-selected skill-usage.jsonl store. The # shared body of the two producers (skill-usage-audit.sh on PostToolUse/Skill, -# skill-usage-expansion-audit.sh on UserPromptExpansion): resolve the configured +# audit-event-emitter.sh's UserPromptExpansion row): resolve the configured # destination, re-verify it after mkdir, keep git status clean in the repo # scope, and write the row. Best-effort throughout; every skip is surfaced once # per session via hook::notice_once markers keyed "-badscope / diff --git a/plugins/claude-ops/hooks/claude-ops-test-helpers.sh b/plugins/claude-ops/hooks/claude-ops-test-helpers.sh index a54992334a..6130ba2fe1 100644 --- a/plugins/claude-ops/hooks/claude-ops-test-helpers.sh +++ b/plugins/claude-ops/hooks/claude-ops-test-helpers.sh @@ -70,6 +70,41 @@ make_sink() { printf '%s' "$s" } +# drive_with_sink [KEY=VALUE ...] -> run +# one hook black-box with a stub sink wired, and return as soon as the hook +# exits (the sink is fire-and-forget, so the capture file may still be empty). +# Trailing KEY=VALUE arguments join the hook's environment, which is where a +# kill switch, a userConfig option or CLAUDE_PROJECT_DIR goes. +drive_with_sink() { + local __hook="$1" __payload="$2" __cap="$3" __sink + shift 3 + : >"$__cap" + __sink="$(make_sink "$__cap")" + env HOOK_TELEMETRY_SINK="$__sink" "$@" bash "$__hook" <<<"$__payload" >/dev/null 2>&1 +} + +# emit_envelope [KEY=VALUE ...] -> drive +# the hook and block until its envelope arrives. Returns non-zero when none +# did, so a caller writes `if emit_envelope …; then assert …; else bad …; fi`. +emit_envelope() { + drive_with_sink "$@" + wait_for_sink "$3" +} + +# expect_no_envelope