perf(markdown-formatter): gate telemetry payload behind the sink opt-in

[kyle-sexton]

Summary

When HOOK_TELEMETRY_SINK is unset (telemetry not wired — the default), the markdown-format producer built its telemetry payload unconditionally before hook::emit_telemetry's opt-in guard could short-circuit: the tool_name jq parse, the FILE_REL block (2× cygpath on Windows), and data_json (1× jq). That is ~4 telemetry-only subprocesses per .md edit on the unwired path (~120ms on Windows Git Bash at ~30ms per fork).

Change

• lib/hook-utils.sh: new hook::telemetry_enabled probe — a pure-shell [[ -n "${HOOK_TELEMETRY_SINK:-}" ]], no subprocess. hook::emit_telemetry still re-checks the sink itself, so callers that skip the probe stay correct, just slower.
• markdown-format.sh: TOOL + FILE_REL construction is gated on the probe, and the data_json build is folded into emit_tel after both guards (start-stamp and sink opt-in). The unwired path now spawns zero telemetry-only subprocesses. REPO_ROOT stays unconditional — it also anchors markdownlint config discovery.
• Plugin copies synced via scripts/sync-hook-utils.sh; all four carrying plugins bump their patch version so consumers receive the lib change (the plugin version is the update cache key).

TOOL was gated in addition to the two items enumerated in the issue: its jq parse feeds only the envelope's data.tool, so leaving it would have kept one telemetry-only subprocess on the unwired path.

Tests

• lib/hook-utils.test.sh: probe verdicts for unset / empty / non-empty sink.
• markdown-format.test.sh: counts subprocess spawns via cygpath/jq PATH shims — unwired runs make zero cygpath -lm calls and exactly one jq call (the file_path parse); a wired positive control proves the shims observe the telemetry spawns (2× cygpath -lm, payload jq calls, envelope delivered).

Ran locally (Windows Git Bash): lib suite 46/46; markdown-format, bash-lint, ruff-format suites pass (biome suite skips without a Biome binary); shellcheck clean on all changed scripts; sync-hook-utils.sh --check and --check-bump pass. One pre-existing, unrelated local flake: the C1 slow-sink timing bound (<2000ms) also fails on unmodified main on this machine when the suite runs end-to-end (in-suite elapsed 5–7s vs ~1.8s standalone, i.e. suite-load noise, not an fd1 leak — the slow-vs-fast sink delta standalone is ~640ms).

Closes #6

🤖 Generated with Claude Code

https://claude.ai/code/session_017K9RRMdUKD7HLExmHwMXAe

---

Note

Low Risk
Performance-only gating with unchanged hook semantics when telemetry is wired; hook::emit_telemetry still enforces the sink guard for correctness.

Overview
Adds hook::telemetry_enabled to the shared hook lib (synced to all four formatter/lint plugins) so producers can skip telemetry-only work when HOOK_TELEMETRY_SINK is unset—the default unwired path.

markdown-format.sh now builds tool_name, repo-relative FILE_REL (including Windows cygpath -lm), and the telemetry data JSON only when that probe passes; emit_tel centralizes the start-stamp + sink guards and folds build_data_json in so unwired edits avoid extra jq/cygpath subprocesses. Formatting behavior is unchanged; REPO_ROOT stays unconditional for markdownlint discovery.

Tests cover the probe in hook-utils.test.sh and PATH-shim subprocess counts in markdown-format.test.sh (unwired vs wired positive control). Carrying plugins get patch version bumps so consumers pick up the lib update.

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

[claude]

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

---

PR Review: perf(markdown-formatter): gate telemetry payload behind the sink opt-in

• Gather context and understand the PR
• Read changed files
• Review lib/hook-utils.sh and lib/hook-utils.test.sh
• Review markdown-format.sh and markdown-format.test.sh
• Review plugin syncs and version bumps
• Post final review

---

Summary: This PR is well-designed and correctly implemented. The telemetry gate eliminates ~4 unnecessary subprocess forks per .md edit on the unwired (default) path. I have a few observations, none of which are blocking.

---

lib/hook-utils.sh — hook::telemetry_enabled

The new probe is correct and appropriately minimal — a pure-shell [[ -n "${HOOK_TELEMETRY_SINK:-}" ]]. The :- guard ensures set -u cannot abort on an unset variable. The comment accurately documents the double-check design (the caller probes early; hook::emit_telemetry re-checks itself as a correctness backstop).

No issues.

---

plugins/markdown-formatter/hooks/markdown-format.sh

Correctness of the refactor — clean. The argument order to hook::emit_telemetry is preserved exactly:

hook_id="markdown-format"  hook_event="PostToolUse"  status=$1  start_epoch=$start  data_json=$(build_data_json $2)  repo_root=$REPO_ROOT

All three call sites (skipped, ok after fix, ok after findings) now go through the shared emit_tel wrapper, which applies both guards before the payload subshell. No call-site drift possible.

build_data_json defined after emit_tel — not a bug in bash (functions resolve at call time, not definition time), and build_data_json can only be reached via emit_tel which is called after line 91 where build_data_json is defined. But the ordering is mildly surprising for a reader who skims top-down. Worth a note if this file is audited in future, but not a correctness issue.

TOOL="" / FILE_REL="$FILE" initialized unconditionally — correct. Prevents set -u errors in the (unreachable) case where build_data_json is called without going through emit_tel. The assignments are pure-shell, zero cost.

REPO_ROOT unconditional — also correct and intentional: it's needed for markdownlint config discovery regardless of telemetry, and serves as the repo_root anchor for relative sink path resolution in hook::emit_telemetry.

One subtle edge case to be aware of (not a regression — pre-existing): if REPO_ROOT comes back empty from hook::repo_root and a relative HOOK_TELEMETRY_SINK is configured, hook::emit_telemetry fail-opens and drops the envelope. The || return 0 guard in hook::emit_telemetry handles this safely. No action needed here.

---

lib/hook-utils.test.sh — Test 13

Three cases: unset, empty string, non-empty value. Coverage is complete for the function's contract. Cleanup (unset HOOK_TELEMETRY_SINK) at the end is correct — avoids leaking the test value into downstream tests.

---

plugins/markdown-formatter/hooks/markdown-format.test.sh — subprocess-count tests

The shim design is the right approach for this kind of assertion on a black-box subprocess.

count_lm is robust for its purpose. grep -c returns the number of matching lines; since the cygpath shim writes printf '%s\n' "$*" (one line per invocation), this equals the invocation count. Filtering on -lm correctly excludes any cygpath -w calls from npm/npx launchers.

${!#} in the cygpath shim — correct bash idiom for "last positional argument." cygpath -lm /foo/bar → echoes /foo/bar. This is intentional: the shim mimics cygpath's output so the hook's normalization logic runs and produces a non-empty FILE_REL even on Linux. Smart.

REAL_JQ="$(command -v jq)" — if jq is absent, this is empty and the shim would exec "", failing. The test implicitly requires jq. That's an acceptable test dependency given the rest of the suite assumes jq.

Positive-control design — the wired test immediately follows the unwired test and uses the same shim infrastructure. This prevents the zero-spawn assertions from passing vacuously (a shim that never fires would always show zero). The PR author explicitly calls this out in the comment. Good test discipline.

JQ_WIRED -gt 1 (not -eq N) — intentionally loose. The exact count of jq calls on the wired path is an implementation detail (it includes file_path parse + tool_name parse + data_json build + envelope build). Using > 1 avoids brittle coupling to the exact internal count while still proving the gate works. Reasonable.

Minor: _OUT_GW is declared with # shellcheck disable=SC2034 — the variable is captured for timing correctness (ensuring wait_for_sink has enough time to observe the background sink write). The suppression is correct and the comment explains the intent.

---

Plugin syncs and version bumps

All four hook-utils.sh copies are byte-for-byte identical to lib/hook-utils.sh (confirmed by SHA in the changed-files list: all show 1c003435abd05af4944215989dcdcf65bbf62546). Version bumps are consistent with the project's stated update-cache-key contract. No issues.

---

Overall

This is a clean, well-scoped perf fix. The design is sound: the probe is a no-op predicate with no side effects, the callers that skip it remain correct (just slower), and the tests cover both the negative path (zero spawns) and the positive control (spawns are observable) to avoid vacuous passes. Approved — no blocking issues.

[cursor]

Risk: low. Cursor Bugbot completed with no findings; CI checks are green. Approved — telemetry gating is a narrow perf improvement with test coverage and limited blast radius.

  

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