feat: extract shared hook-utils.sh to an authoring-time SSOT

[kyle-sexton]

Summary

Four plugins carry byte-identical hooks/hook-utils.sh copies — the playbook's Rule-of-Three trigger. Per the decision reached with fresh-docs research (plugins-reference + plugin-dependencies, fetched 2026-07-03), the shared lib becomes an authoring-time SSOT with plain runtime copies:

• lib/hook-utils.sh is the canonical source; scripts/sync-hook-utils.sh propagates it into every carrying plugin (sync / --check / --check-bump <ref> modes).
• New hook-utils-sync CI lane (wired into ci-status): fails on copy drift, and on PRs fails when the lib changed without a version bump in every carrying plugin — the plugin version is the update cache key, so an unbumped plugin never delivers the change.
• Runtime is untouched: each installed plugin stays self-contained under cache isolation; no cross-plugin coupling; one-plugin install UX unchanged.
• Rejected/deferred alternatives are recorded in the playbook with citations and revisit triggers: a dependency plugin is not viable (no documented mechanism exposes a dependency's install path to another plugin's hook; cache dirs are version- and SHA-suffixed by design), and marketplace-internal symlinks are deferred (skipped for --plugin-dir/local installs; Windows-fragile).

All four plugin versions bumped (patch): the shared header no longer claims markdown-formatter heritage and the sourcing guard drops the _MDFMT prefix.

Stacked on #23 (feat/ruff-format-plugin) — do not merge into #23; retarget to main after #23 merges.

Verification

• scripts/sync-hook-utils.sh --check passes; all copies byte-identical to source (sha1 e207414).
• Bump gate exercised in all three paths: lib-unchanged → pass; changed+bumped → pass; changed+stale → exit 1 naming the manifest.
• All four hook-utils.test.sh suites pass (37/37 ×3, 43/43).
• claude plugin validate ×4 and --strict catalog validation pass; shellcheck, markdownlint, actionlint, editorconfig-checker clean locally.

🤖 Generated with Claude Code

https://claude.ai/code/session_012NNwc8U9MbgWsd5sbrfpPh

---

Note

Medium Risk
Touches shared hook runtime utilities used on every PostToolUse edit (path scoping and telemetry), with mandatory plugin version bumps so consumers actually receive the lib change.

Overview
Centralizes duplicated hooks/hook-utils.sh across four formatter/lint plugins into lib/hook-utils.sh, with scripts/sync-hook-utils.sh to copy or verify plugin copies (--check, --check-bump <ref>).

A new hook-utils-sync CI job (included in ci-status) fails PRs when copies drift from the lib or when the lib changes without a patch bump in every carrying plugin’s plugin.json (version is the consumer update key). docs/MIGRATION-PLAYBOOK.md records this SSOT-plus-copies model and why dependency plugins / marketplace symlinks were rejected or deferred.

Synced plugin copies pick up shared headers/guard (_HOOK_UTILS_LOADED) and a behavioral change: hook::physical_path plus hook::read_file_path now resolve symlinks before CLAUDE_PROJECT_DIR membership checks. Carrying plugins are bumped (bash-lint 0.1.2, biome/ruff 0.1.1, markdown-formatter 0.1.3).

^{Reviewed by Cursor Bugbot for commit 7bcb499. Bugbot is set up for automated code reviews on this repo. Configure here.}

[claude]

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

---

PR #24 Review — feat: extract shared hook-utils.sh to an authoring-time SSOT

• Gather context from PR description and changed files list
• Read lib/hook-utils.sh (canonical source)
• Read scripts/sync-hook-utils.sh (sync/check script)
• Read .github/workflows/ci.yml changes
• Read docs/MIGRATION-PLAYBOOK.md updates
• Read plugin manifests and hook-utils copies
• Compile and post review

---

Summary

The architecture decision is correct and well-justified: a single authoring-time SSOT with plain runtime copies is the right tradeoff given the constraints (cache isolation makes cross-plugin file references unviable; symlinks are skipped by --plugin-dir). The decision record in the playbook is thorough and citable. All five copies are byte-identical (sha1 e207414, confirmed locally). No blocking issues. Two medium findings worth addressing before merge.

---

Medium

1. --check-bump: $manifest at HEAD is not guarded before jq reads it

scripts/sync-hook-utils.sh L61:

head_version=$(jq -r '.version // empty' "$manifest")

If a plugin has a hooks/hook-utils.sh copy but its manifest is missing or at a non-standard path, jq errors and set -euo pipefail aborts the whole check — silently skipping the remaining plugins. The base-ref lookup two lines up is already guarded (|| true; empty = skip). HEAD should get the same treatment:

if [[ ! -f "$manifest" ]]; then
  echo "MISSING MANIFEST: $manifest" >&2
  stale=1
  continue
fi
head_version=$(jq -r '.version // empty' "$manifest")

Fix this →

---

2. _HOOK_UTILS_LOADED guard rename needs a justification comment

lib/hook-utils.sh L12–13:

[[ -n "${_HOOK_UTILS_LOADED:-}" ]] && return 0
readonly _HOOK_UTILS_LOADED=1

The rename from _MDFMT_HOOK_UTILS_LOADED (a copy-paste artifact from ruff-format's original) to a shared, non-namespaced variable is correct — hooks run in isolated subprocesses, so there is no cross-plugin contamination risk, and within a single process all copies are byte-identical so any one loaded = all loaded. But without that reasoning inline, a future contributor will see a suspiciously global variable and be tempted to re-namespace it, reintroducing divergence. A one-line comment is worth adding:

# Non-namespaced: all copies are byte-identical; hooks run in isolated subprocesses.
[[ -n "${_HOOK_UTILS_LOADED:-}" ]] && return 0

Fix this →

---

Low / Observations (no action required)

3. Sync mode: no early existence check on $src

scripts/sync-hook-utils.sh L16 — set -euo pipefail will catch a missing lib/hook-utils.sh at the cp call, but the error message will be a raw cp failure rather than a clear "source not found." Harmless in practice; easy to add if the script grows more modes.

4. Deferred test consolidation is correctly scoped

The playbook correctly records the 4 divergent hook-utils.test.sh suites as a known follow-up, deferred to the next behavioral lib change. No action needed here.

5. EPOCHREALTIME in hook::emit_telemetry is bash 5.0+

lib/hook-utils.sh L178–179 — On bash < 5.0 (macOS stock shell), EPOCHREALTIME is empty and the duration arithmetic would produce 0 or error. This was already the case in the pre-existing plugin copies; this PR doesn't regress it, and the telemetry function is opt-in and fail-open. Noting for awareness if bash < 5 support ever becomes explicit.

---

Positive callouts

• Byte-identity confirmed: all 5 copies at sha1 e207414 ✓
• Guard variable fix: _MDFMT_HOOK_UTILS_LOADED in ruff-format was a copy-paste error (that guard name belongs to markdown-formatter); the rename to _HOOK_UTILS_LOADED fixes a latent inconsistency ✓
• CI bump gate is correctly scoped: if: github.event_name == 'pull_request' prevents the gate from running on push-to-main where the base ref concept doesn't apply ✓
• fetch-depth: 0 is correctly required and documented in the YAML ✓
• New-plugin opt-in path is documented and handled: the script skips plugins absent at the base ref; their first release already carries the new lib ✓
• Telemetry fire-and-forget pattern: ("$sink" >/dev/null 2>&1) & correctly prevents the fd1 inheritance issue; the comment explaining the "C1 fd1-inheritance blocker" is the right level of detail ✓
• Decision record in the playbook is exemplary: alternatives weighed with citations, revisit triggers named, known follow-ups scoped ✓

[claude]

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

---

PR #24 Review — feat: extract shared hook-utils.sh to an authoring-time SSOT

• Check what changed since last review (merge commit)
• Read current scripts/sync-hook-utils.sh — verify medium finding chore: initialize marketplace scaffold #1 status
• Read current lib/hook-utils.sh — verify medium finding fix: correct plugin-system inaccuracies and harden references #2 status
• Inspect merge conflict resolution (ruff-format files)
• Verify byte-identity of all hook-utils.sh copies
• Post updated review

---

What changed in this sync

This synchronize event was a merge-back: 7bcb499 pulled origin/main (the now-merged PR #23 feat/ruff-format-plugin) into this branch, resolving conflicts in the ruff-format plugin. No new commits addressing the prior review findings were pushed.

Merge resolution is correct:

• All five hook-utils.sh copies remain byte-identical at sha1 e207414 ✓
• plugins/ruff-format/.claude-plugin/plugin.json correctly lands at 0.1.1 (this branch's version bump took precedence over the 0.1.0 introduced by PR feat(ruff-format): add Ruff format/lint-on-edit plugin #23) ✓
• ruff-format.sh and ruff-format.test.sh are intact as introduced by PR feat(ruff-format): add Ruff format/lint-on-edit plugin #23 ✓

---

Medium (both still open from prior review)

1. --check-bump: $manifest at HEAD is not guarded before jq reads it

scripts/sync-hook-utils.sh L61:

head_version=$(jq -r '.version // empty' "$manifest")

The base-ref path two lines up is guarded (|| true; empty → continue). The HEAD path is not: if $manifest is missing, jq errors and set -euo pipefail aborts the whole check — silently dropping the remaining plugins. Suggested fix:

if [[ ! -f "$manifest" ]]; then
  echo "MISSING MANIFEST: $manifest" >&2
  stale=1
  continue
fi
head_version=$(jq -r '.version // empty' "$manifest")

Fix this →

---

2. _HOOK_UTILS_LOADED guard rename still lacks a justification comment

lib/hook-utils.sh L11–13:

# Guard against double-sourcing.
[[ -n "${_HOOK_UTILS_LOADED:-}" ]] && return 0
readonly _HOOK_UTILS_LOADED=1

The rename from _MDFMT_HOOK_UTILS_LOADED to a non-namespaced variable is correct, but the why is absent. Without it, a future contributor will see a suspiciously global variable and re-namespace it, introducing divergence again. Suggested:

# Non-namespaced: all copies are byte-identical; hooks run in isolated subprocesses.
[[ -n "${_HOOK_UTILS_LOADED:-}" ]] && return 0

Fix this →

---

Positive callouts

• Merge resolution is clean — no drift introduced; byte-identity holds across all five copies ✓
• ruff-format plugin version correctly resolved — 0.1.1 from this branch wins over 0.1.0 from the base ✓
• ruff-format.sh is well-implemented — advisory exit 0 throughout, EPOCHREALTIME guarded (start=${EPOCHREALTIME:-}), proper --no-unsafe-fixes, --force-exclude, and opt-in via config discovery ✓
• Test suite is comprehensive — 43 cases covering no-config opt-out, pyproject.toml variants, F401 guard, unsafe-fixes protection, excluded files, kill switch, PATH fallback, and telemetry envelope fields ✓
• All low/observations from the prior review stand unchanged — no action required

---

Verdict

This PR is well-architected and the merge conflict was resolved correctly. The two medium findings remain open and should be addressed before merge — they are both small, self-contained fixes.

[claude]

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

---

PR #24 Review — feat: extract shared hook-utils.sh to an authoring-time SSOT

• Gather context from prior reviews and PR state
• Read scripts/sync-hook-utils.sh — verify medium finding chore: initialize marketplace scaffold #1 status
• Read lib/hook-utils.sh — verify medium finding fix: correct plugin-system inaccuracies and harden references #2 status
• Compile and post final review

---

Status of Prior Findings

This PR merged with both medium findings from prior review rounds still open. They are now in main.

---

Medium (both unresolved — now in main)

1. --check-bump: $manifest at HEAD unguarded before jq

scripts/sync-hook-utils.sh L61:

head_version=$(jq -r '.version // empty' "$manifest")

The base-ref lookup two lines up at L57 is guarded with || true (failure → empty string → continue). The HEAD path has no such guard: if $manifest is absent, jq errors and set -euo pipefail aborts the entire script — silently skipping any plugins that follow in the loop. Suggested fix:

if [[ ! -f "$manifest" ]]; then
  echo "MISSING MANIFEST: $manifest" >&2
  stale=1
  continue
fi
head_version=$(jq -r '.version // empty' "$manifest")

Fix this →

---

2. _HOOK_UTILS_LOADED guard still lacks a justification comment

lib/hook-utils.sh L11–13:

# Guard against double-sourcing.
[[ -n "${_HOOK_UTILS_LOADED:-}" ]] && return 0
readonly _HOOK_UTILS_LOADED=1

The comment explains what but not why the variable is intentionally non-namespaced. Without the reasoning, a future contributor will see a suspiciously global variable and re-namespace it (e.g. back to _MDFMT_HOOK_UTILS_LOADED), which would reintroduce divergence across the four copies. Suggested replacement:

# Non-namespaced: all copies are byte-identical; hooks run in isolated subprocesses.
[[ -n "${_HOOK_UTILS_LOADED:-}" ]] && return 0

Fix this →

---

Observations (unchanged from prior reviews)

• $src existence check in sync mode: missing but harmless (cp will fail clearly enough); low priority.
• EPOCHREALTIME requires bash 5.0+: pre-existing, opt-in, fail-open; not a regression.
• Test consolidation: correctly deferred in the playbook.

---

Positive callouts (carry-forward)

All architecture decisions and implementation quality notes from prior reviews stand:

• Byte-identity across all five copies at sha1 e207414 ✓
• CI bump gate scoped correctly to pull_request events ✓
• fetch-depth: 0 required and documented ✓
• Fire-and-forget telemetry pattern with C1 fd1-inheritance blocker ✓
• Decision record in playbook is exemplary ✓

---

Verdict

The two medium findings are small, self-contained fixes. Finding #2 (lib/hook-utils.sh comment) requires propagating the change to all four plugin copies via scripts/sync-hook-utils.sh and bumping each manifest version — the sync CI lane will enforce this. Recommended to address both in a follow-up patch before any behavioral lib change lands.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 7bcb499. Configure here.}

[cursor]

Bad ref skips bump gate

Medium Severity

In --check-bump, a failed git diff against the base ref is treated like a lib change, and carriers whose git show for plugin.json fails are skipped like new plugins. With an unresolvable base ref or unreadable base manifests, the script can exit 0 without detecting stale versions even when lib/hook-utils.sh changed and carrying plugins were not bumped.

 

^{Reviewed by Cursor Bugbot for commit 7bcb499. Configure here.}

[cursor]

Risk: medium. Not approving: Cursor Bugbot completed as skipped and reported 1 unresolved finding in scripts/sync-hook-utils.sh. The PR is already merged; no reviewers were assigned.

  

_{Sent by Cursor Approval Agent: Pull Request Router and Approver}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7bcb499929

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Fail closed when the bump base is unreadable

When the supplied base ref is missing or otherwise unreadable, this git diff --quiet runs inside an if, so Git's fatal exit is treated the same as “the lib changed”; the later git show "$base:$manifest" ... || true then makes every base version empty and skips all manifests. I verified scripts/sync-hook-utils.sh --check-bump no-such-ref prints Git's fatal error but still exits 0, which means a PR whose base ref was not fetched/resolved could pass the version-bump gate with stale plugin versions instead of failing closed.

Useful? React with 👍 / 👎.