Skip to content

docs(hook-observability): refresh statusMessage adoption (30/31) + define dim-9; cover buffer_stdin timeout & EPOCHREALTIME skip (F4, F3, F2-shared) - #1172

Merged
kyle-sexton merged 1 commit into
mainfrom
docs/1169-hook-observability-adoption
Jul 23, 2026
Merged

docs(hook-observability): refresh statusMessage adoption (30/31) + define dim-9; cover buffer_stdin timeout & EPOCHREALTIME skip (F4, F3, F2-shared)#1172
kyle-sexton merged 1 commit into
mainfrom
docs/1169-hook-observability-adoption

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Closes #1169.

Fleet-scope findings surfaced by the bash-format consumer audit (handoff-inbox item 20260723-095036), re-verified and rescoped against current source. No runtime behavior change — a fleet-doc correction plus shared-lib test coverage.

Changes

  • F4 — statusMessage adoption doc was badly stale. The doc claimed "no hooks.json in the fleet declares statusMessage yet — all 27 wired type: command handlers across 12 plugins need it added." Scripted count of current source (jq over every plugins/*/hooks/hooks.json): 31 command-type handlers across 15 hook-bearing plugins, 30 declare statusMessage; sole holdout plugins/disk-hygiene (a PreToolUse destructive-guard). Doc now records near-complete adoption, names the holdout, and states the feat(conventions): hook-observability fleet convention + fleet adoption #836 close condition.
  • F3 — dim-9 doctrine defined. Grounding refuted the "bash-format-local" premise: the label is fleet vocabulary cited in 9 hook scripts (actionlint, bash-format, biome-format, desktop-notification, eol-normalizer, go-format, powershell-format, ruff-format, typos-format) with no central glossary (verified — no dimensions registry exists). Defined it authoritatively in the visible-skip (systemMessage) section it names, and noted the broader undefined dim-N scheme as a separate follow-up. Not stripped from bash-format — that would fragment a fleet-wide convention (reuse-or-replace).
  • F2-shared — shared-lib test coverage. Added lib/hook-utils.test.sh cases for hook::buffer_stdin's timeout path (return 2 / BLOCKED on stderr) and hook::emit_telemetry's EPOCHREALTIME-absent (Bash < 5.0) skip-fail-open. Shared-lib behavior → tests live centrally, not duplicated into a plugin.

Verification

  • bash lib/hook-utils.test.shPASS=85 FAIL=0 (2 new cases green) on bash 5.3.
  • shellcheck -x -S warning lib/hook-utils.test.sh → clean.
  • Only lib/hook-utils.sh's test was touched (not the SSOT lib itself), so no sync-hook-utils.sh run and no plugin version bumps are needed.

Notes

🤖 Generated with Claude Code

Related

🤖 Generated with Claude Code

…fine dim-9; cover buffer_stdin timeout & EPOCHREALTIME skip (F4, F3, F2-shared)

Fleet-scope findings from the bash-format consumer audit (handoff-inbox item
20260723-095036), re-verified/rescoped against current source. No runtime
behavior change.

- F4: the doc claimed "no hooks.json declares statusMessage yet — all 27 wired
  handlers across 12 plugins need it added". Scripted count of current source
  (jq over every plugins/*/hooks/hooks.json): 31 command-type handlers across 15
  hook-bearing plugins, 30 declare statusMessage, sole holdout
  plugins/disk-hygiene (a PreToolUse destructive-guard). Doc updated to
  near-complete adoption with the holdout named and the #836 close condition.
- F3: "dim-9 doctrine" is fleet vocabulary cited in 9 hook scripts with no
  central glossary (verified: no dimensions registry exists). Defined it
  authoritatively in the visible-skip (systemMessage) section it names, and
  noted the broader undefined dim-N scheme as a separate follow-up. Not stripped
  from bash-format — that would fragment a fleet-wide convention.
- F2-shared: added lib/hook-utils.test.sh coverage for hook::buffer_stdin's
  timeout path (return 2 / BLOCKED) and hook::emit_telemetry's
  EPOCHREALTIME-absent (Bash < 5.0) skip-fail-open. Shared-lib behavior, so the
  tests live centrally, not duplicated into a plugin.

Grounding note: F1's "add a membership-guard test" (filed under sibling #1168)
was already satisfied by lib/hook-utils.test.sh Test 12; see #1168.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 34s —— View job


Code Review

  • Gather context and understand the request
  • Review PR diff
  • Read changed files
  • Analyze findings
  • Post review feedback

Overall: Clean, well-scoped doc correction + test additions. No runtime behavior changes. Two minor observations and one question worth addressing before merge.


docs/conventions/hook-observability/README.md

F4 count update — accurate and well-grounded. The transition from the stale "27 handlers / 12 plugins / 0 adopters" to "31 handlers / 15 plugins / 30 adopters / 1 holdout" is clearly sourced and the #836 close condition is precisely stated.

F3 dim-9 definition — well-placed co-location with the rule it names. The parenthetical noting the broader dim-N scheme lacks a central registry is helpful context. One minor note:

Note

The dim-8 / dim-11 examples embedded in the parenthetical are themselves undefined to a reader unfamiliar with the fleet's history. They illustrate that dim-N is informal, but a reader landing on this doc cold gets no meaning from dim-8 = uniform setup-skill wave. If the follow-up item to document the full dim-N scheme is tracked, consider just noting the label count ("the scheme covers at least 11 numbered dimensions") without naming undefined dimensions. Not blocking, but worth a thought.


lib/hook-utils.test.sh

Both tests are structurally sound and correctly exercise the target paths.

Test 18 (hook::buffer_stdin timeout):

permalink

{ printf '{"incomplete":'; sleep 1; } | {
  CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT=0.4 hook::buffer_stdin >/dev/null 2>"$bs_err_file"
  echo "$?" >"$bs_rc_file"
}

The exit-code capture idiom (write to a temp file from within the pipe's right-side subshell) is correct under pipefail — the right-side subshell exits 0 from echo, so the pipeline doesn't trip the error trap.

[!QUESTION]
The test comment notes that jq must be present for return 2 rather than return 1 ("without it the branch fails open to return 1"). Should there be an explicit jq availability guard at the top of this test, or is jq availability guaranteed by the CI environment contract? If a future CI runner drops jq, this test will silently report a different failure path without a clear diagnosis. Consider adding a guard: command -v jq >/dev/null || skip "jq absent — timeout-path test requires jq" (or whatever the file's skip idiom is). Fix this →

Test 19 (emit_telemetry EPOCHREALTIME-absent):

permalink

rc19=$(
  unset EPOCHREALTIME
  start=${EPOCHREALTIME:-}
  HOOK_TELEMETRY_SINK="$sink19" hook::emit_telemetry "t" "PostToolUse" "ok" "$start" '{"tool":"x","file":"y","findings":[]}'
  echo $?
)
sleep 0.1

The early-return guard in emit_telemetry ([[ -n "$start_epoch" && -n "$now" ]] || return 0) fires before any background dispatch when $start is empty, making the sleep 0.1 reliable rather than racy — the 100 ms is dead time but not a flakiness risk. This is fine.

The $() subshell captures echo $? (stdout) while stderr goes to the terminal — there's no stderr redirection here, so any unexpected stderr output from emit_telemetry would be visible in CI logs. That's fine for a test.

Note

Minor consistency nit: earlier tests in this file use echo "$?" >"$some_rc_file" with a quoted $?; this test uses echo $? (unquoted). The value is always numeric so it doesn't matter, but the inconsistency stands out. Not blocking.


Summary

Finding Severity Verdict
Test 18 missing jq guard Minor / Low Worth addressing
Unquoted $? in Test 19 Cosmetic Not blocking
Dim-8/dim-11 examples in parenthetical Cosmetic Author's call

No correctness bugs, no security issues, no CLAUDE.md violations. The shared-lib test approach (central coverage, no plugin duplication) is correct per the repo's structure. The documentation state is now accurate and the dim-9 anchor is a genuine improvement for future readers.

Ready to merge after addressing (or explicitly deferring) the jq guard question.

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 30s —— View job


Security Review — PR #1172

  • Gather context (PR diff, changed files)
  • Review docs/conventions/hook-observability/README.md
  • Review lib/hook-utils.test.sh
  • Report findings

No security issues found.

This PR touches two files. Neither introduces exploitable risk.

docs/conventions/hook-observability/README.md

Pure documentation — updates adoption counts, names the disk-hygiene holdout, and authoritatively defines the dim-9 label in the visible-skip section. No code execution, no configuration, no secrets, no attack surface.

lib/hook-utils.test.sh (Test 18, lines 816–837; Test 19, lines 839–861)

Test 18 — hook::buffer_stdin timeout path:

  • Temp files (bs_rc_file, bs_err_file) are created via mktemp — race-free and safely quoted throughout.
  • The pipe producer { printf '{"incomplete":'; sleep 1; } is a static string literal with no interpolation.
  • CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT=0.4 is a literal value scoped to the command invocation.
  • Cleanup via rm -f "$bs_rc_file" "$bs_err_file" is correct.

Test 19 — hook::emit_telemetry EPOCHREALTIME-absent skip:

  • tel19 and sink19 are both mktemp-sourced paths — no metacharacter risk.
  • unset EPOCHREALTIME and the start= assignment are scoped inside a command-substitution subshell; they do not leak into the surrounding suite.
  • HOOK_TELEMETRY_SINK="$sink19" is an env-var override for a single command invocation, and $sink19 is a mktemp-derived absolute path with no special characters.
  • rm -f "$tel19" "$sink19" correctly removes both the output file and the generated sink script.

Pre-existing make_sink note (out of scope for this PR): The existing helper embeds its argument directly into an unquoted heredoc, which would allow injection if called with an attacker-controlled path. All callers — including the two new tests — pass mktemp output exclusively, so no path is exploitable. This predates the PR and is flagged only for awareness.

@kyle-sexton
kyle-sexton merged commit 95c7449 into main Jul 23, 2026
26 of 27 checks passed
@kyle-sexton
kyle-sexton deleted the docs/1169-hook-observability-adoption branch July 23, 2026 18:26
kyle-sexton added a commit that referenced this pull request Jul 23, 2026
…/shfmt-fallback test coverage (F1, F2, F6) (#1171)

Closes #1168.

Consumer-side audit polish for `bash-format@0.6.1` (handoff-inbox item
`20260723-095036`), findings **F1, F2 (bash-format-local), F6**. **No
behavioral change** — docs + tests only. Version `0.6.2`.

## Changes

- **F1 (docs).** Documented the in-project-only scope: the hook acts
only on shell files under `CLAUDE_PROJECT_DIR` (symlink-resolved
membership guard in the shared library); files written outside the
project are silently skipped. Added a README "Scope" note and a
project-scope probe to the setup skill's `check` (which no longer
reports "fully operational" without the caveat).
- **Grounding note:** F1's original "add a membership-guard test"
sub-item is **already satisfied** — `lib/hook-utils.test.sh` Test 12
thoroughly tests the guard (in-project accept, prefix-sibling reject,
escaping-symlink reject, symlinked-root). The producer only read
`bash-format.test.sh` (which deliberately unsets `CLAUDE_PROJECT_DIR`).
So F1 reduces to docs.
- **F2 (bash-format-local tests).** Added `bash-format.test.sh` coverage
for the `[*.{sh,bash}]` brace-list and `[**/*.sh]` path-prefixed
`.editorconfig` opt-in forms, and the `shfmt < 3.8` `--apply-ignore`
fallback (`|| shfmt -w`) via a stub shfmt.
- **F6.** Added a `## Gotchas` section to the setup skill (cache-path
`ENAMETOOLONG`, `check`-PASS-≠-full-coverage, opt-in-conditional `shfmt`
FAIL).

## Verification

- `bash plugins/bash-format/hooks/bash-format.test.sh` → **PASS=42
FAIL=0** (3 new cases green).
- `shellcheck -x -S warning` on the test file → clean.
- `plugin.json` valid; version bumped `0.6.1` → `0.6.2`; CHANGELOG
updated.

## Scope moved out of this PR

- **F3** (`dim-9 doctrine` label) → **#1169**. Grounding refuted the
"bash-format-local" premise: the label is fleet vocabulary cited in 9
hook scripts with no shared glossary. Stripping it from bash-format
alone would fragment the convention; the fix is a shared-doc definition
(fleet-scope).
- **Shared-lib F2 gaps** (`buffer_stdin` timeout, `EPOCHREALTIME`
degradation) → **#1169** (belong in `lib/hook-utils.test.sh`).
- **F5** (`--config` fresh-install claim) → **#1170** (HITL — verifying
mutates shared machine install state).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Related

- #1172 — sibling PR for the fleet-scope findings (F2-shared, F3, F4)
from the same audit item.
- #1170 — HITL follow-up (F5, `--config` fresh-install claim), not
closed here.
- Handoff-inbox item
`20260723-095036-bash-format-audit-docs-tests-polish` (producer SW2030).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 23, 2026
…eralize CRLF gotcha; correct install_new render doc (F1-F3) (#1177)

Closes #1176.

Consumer-audit fixes for the claude-ops `plugins` sync skill
(handoff-inbox item `20260723-155027`), re-verified against current
source (0.19.0). Version `0.19.0` → `0.19.1`.

## Changes

- **F1 (bug).** `fleet-state.sh`'s `resolve_default_marketplace`
exact-matched the running plugin root against the version-pinned
`installPath`, so any skew between the session's loaded version and the
installed one — a marketplace `autoUpdate` after session start, or
`sync`'s own Step-3 self-update of claude-ops — broke the primary
no-`--marketplace` path with "could not resolve the default
marketplace". Added a **version-agnostic fallback** matching the
version-stripped `…/cache/<marketplace>/<plugin>` prefix (exact match
still tried first; the marketplace stays distinguishable via the path),
plus a clearer error that prints the searched root and names the skew
cause.
- **F2a (doc).** Generalized `context/gotchas.md`'s CRLF entry from
"this host's `jq` build" to **any captured Windows value** (`python`
print, PowerShell interop, `git config`, CRLF file read): a trailing
`\r` on a constructed `claude plugin` id makes the CLI report `Plugin
"<name>" not found` with the full id passed (marketplace suffix
corrupted), colliding with the bare-name symptom — documented and
cross-referenced both ways.
- **F3a (doc).** `plugins/SKILL.md` now reads an unset
`${user_config.install_new}` — which renders the literal placeholder,
since the manifest `default` is not substituted for an unset key
(verified vs CC 2.1.218) — as the **expected unset state → default
`ask`**, not an invalid value; only an explicitly-set unsupported value
is flagged.

## Verification

- `bash plugins/claude-ops/skills/plugins/scripts/fleet-state.test.sh` →
**28 cases, 0 failed** (new version-skew case green).
- `shellcheck -x -S warning fleet-state.sh` → clean.
- `scripts/check-changed-skills.sh origin/main` → PASS for the `plugins`
skill (runs the fleet-state test).

## Deferred (noted on #1176, not in this PR)

- **F2b:** ship a `sync-sweep.sh` sibling owning the Step 2+3 update
loop (CR-stripping, fail-loud, structured per-id JSON) so `sync.md`
shrinks to "run this, report from its output". Bigger net-new surface
with its own test.
- **F3b:** verify upstream whether a declared `userConfig` default is
*intended* to substitute for an unset key; file a CC docs issue if docs
and behavior disagree. F3a is correct regardless.

## Related

- Handoff-inbox item `20260723-155027-claude-ops-plugins-sync-audit`
(producer: KyleSexton workstation, CC 2.1.218).
- Prior inbox items landed this session: #1171 (bash-format), #1172
(hook-observability + hook-utils).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant