fix(markdown-formatter): harden hook + simplify plumbing

.github/workflows/ci.yml

@@ -202,20 +202,10 @@ jobs:
     timeout-minutes: 15
     steps:
       - name: Aggregate lane results
+        # Derived from the needs graph above — the single source of truth for the
+        # lane list. Adding a lane to needs automatically extends this check.
         env:
-          RESULTS: >-
-            ${{ needs.markdown.result }}
-            ${{ needs.typos.result }}
-            ${{ needs.gitleaks.result }}
-            ${{ needs.editorconfig.result }}
-            ${{ needs.shellcheck.result }}
-            ${{ needs.actionlint.result }}
-            ${{ needs.jsonschema.result }}
-            ${{ needs.exec-bit.result }}
-            ${{ needs.machine-specific-paths.result }}
-            ${{ needs.eol-renormalize.result }}
-            ${{ needs.comment-hygiene.result }}
-            ${{ needs.zizmor.result }}
+          RESULTS: ${{ join(needs.*.result, ' ') }}
         run: |
           for r in $RESULTS; do
             case "$r" in

plugins/markdown-formatter/hooks/hook-utils.sh

@@ -18,16 +18,29 @@ hook::check_enabled() {
   fi
 }
 
-# Normalize a path: backslashes → forward slashes, then POSIX/lowercase drive
-# letter → uppercase Windows drive letter (idempotent). On macOS/Linux the
-# regex does not match (multi-char first segments) — no-op.
+# Normalize a path for the membership comparison below: backslashes → forward
+# slashes, and — only on Windows/MSYS, whose filesystem is case-insensitive —
+# fold a leading drive (POSIX `/c/...` or `c:/...`) to an upper-case drive
+# letter + lower-cased remainder so the byte-exact comparison is effectively
+# case-insensitive. The fold is gated on the host (OSTYPE), NOT on the path
+# shape: on a case-sensitive POSIX filesystem a real single-letter top-level
+# directory such as `/c/Repo` must pass through unchanged, otherwise it would
+# collapse with `/c/repo` and the membership guard would admit a sibling
+# outside CLAUDE_PROJECT_DIR. The result is used ONLY for comparison; the
+# emitted path is always the caller's original.
 hook::normalize_path() {
   local p="${1//\\//}"
-  if [[ "$p" =~ ^/([a-zA-Z])/ || "$p" =~ ^([a-zA-Z]):/ ]]; then
-    printf '%s' "${BASH_REMATCH[1]^}:${p:2}"
-  else
-    printf '%s' "$p"
-  fi
+  case "${OSTYPE:-}" in
+    msys* | cygwin* | win32)
+      if [[ "$p" =~ ^/([a-zA-Z])/ || "$p" =~ ^([a-zA-Z]):/ ]]; then
+        local rest="${p:2}"
+        printf '%s' "${BASH_REMATCH[1]^}:${rest,,}"
+        return
+      fi
+      ;;
+    *) ;; # POSIX hosts: case-sensitive FS, no drive fold — pass through below
+  esac
+  printf '%s' "$p"
 }
 
 # Parse file_path from PostToolUse JSON on stdin; validate existence and (when
@@ -43,7 +56,11 @@ hook::read_file_path() {
     local norm_file norm_project
     norm_file=$(hook::normalize_path "$file")
     norm_project=$(hook::normalize_path "${CLAUDE_PROJECT_DIR}")
-    if [[ "$norm_file" != "$norm_project"* ]]; then
+    norm_project="${norm_project%/}"
+    # Anchor on a path-segment boundary: accept the project root itself or a
+    # child under it, but not a sibling whose name merely shares the prefix
+    # (e.g. /c/repo must not admit /c/repo-backup/...).
+    if [[ "$norm_file" != "$norm_project" && "$norm_file" != "$norm_project"/* ]]; then
       return 1
     fi
   fi

plugins/markdown-formatter/hooks/hook-utils.test.sh

@@ -39,6 +39,20 @@ EOF
   printf '%s' "$s"
 }
 
+# wait_for_sink <file> [max_polls] → block until <file> is non-empty (the
+# fire-and-forget sink has flushed) or the bound elapses. Polls in 20ms steps so
+# the assertion fires as soon as the write lands instead of racing a fixed sleep
+# (sink dispatch is a freshly-spawned process; spawn latency varies, especially
+# on Windows Git Bash). Returns non-zero on timeout so negative cases can assert.
+wait_for_sink() {
+  local f="$1" tries="${2:-150}"
+  while (( tries-- > 0 )); do
+    [[ -s "$f" ]] && return 0
+    sleep 0.02
+  done
+  return 1
+}
+
 # --- Test 1: HOOK_TELEMETRY_SINK unset → returns 0, no output ----------------
 unset HOOK_TELEMETRY_SINK 2>/dev/null || true
 out=$(hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"foo.md","findings":[]}' 2>/dev/null)
@@ -54,22 +68,20 @@ else
   fail "sink unset: unexpected output: $out"
 fi
 
-# --- Test 2: jq absent → emit skipped, no error, returns 0 ------------------
-# Shadow jq with a function that returns 127 to simulate absence.
-# The HOOK_TELEMETRY_SINK env var and the jq shadow are scoped to the subshell
-# so they never pollute the outer test context.
+# --- Test 2: jq absent → fail-open (returns 0, no output) -------------------
+# Make `command -v jq` genuinely fail by running with a PATH that contains no
+# jq. A shell-function shadow does NOT exercise the guard: `command -v jq`
+# reports a defined function as present, so the absence branch is never taken.
+# emit_telemetry uses only shell builtins until its jq calls, so an empty PATH
+# is sufficient. Scoped to the command so PATH/sink never leak into the suite.
+EMPTY_BIN="$(mktemp -d)"
 # shellcheck disable=SC2030,SC2031
 out_nojq=$(
   export HOOK_TELEMETRY_SINK="cat"
-  (
-    unset _MDFMT_HOOK_UTILS_LOADED 2>/dev/null || true
-    jq() { return 127; }
-    export -f jq
-    source "$HOOK_DIR/hook-utils.sh"
-    hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"foo.md","findings":[]}' 2>/dev/null
-  )
+  PATH="$EMPTY_BIN" hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"foo.md","findings":[]}' 2>/dev/null
 )
 rc_nojq=$?
+rmdir "$EMPTY_BIN" 2>/dev/null || true
 if [[ $rc_nojq -eq 0 ]]; then
   ok "jq absent: returns 0"
 else
@@ -93,8 +105,7 @@ export HOOK_TELEMETRY_SINK
 data_json='{"tool":"Write","file":"docs/foo.md","findings":["docs/foo.md:12 MD013/line-length"]}'
 start=$EPOCHREALTIME
 hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$start" "$data_json" 2>/dev/null
-# Allow the background process to finish
-sleep 0.2
+wait_for_sink "$SINK_FILE"
 unset HOOK_TELEMETRY_SINK
 
 if [[ -s "$SINK_FILE" ]]; then
@@ -165,10 +176,12 @@ if [[ -s "$SINK_FILE" ]]; then
   # We verify by checking the TZ=UTC prefix actually produces UTC
   utc_h=$(TZ=UTC date +%H)
   ts_h="${ts:11:2}"
-  # Allow ±1h boundary tolerance
-  h_diff=$(( ${ts_h#0} - ${utc_h#0} ))
-  h_diff=${h_diff#-}
-  if [[ $h_diff -le 1 ]]; then
+  # Circular ±1h tolerance to absorb an hour-boundary straddle between the emit
+  # and this check — including the 23→00 midnight wrap, where a linear distance
+  # would read 23 and false-fail once per day. (#0 strips the leading zero so
+  # 00–09 are not misread as octal inside (( )).)
+  h_diff=$(( ( ${ts_h#0} - ${utc_h#0} + 24 ) % 24 ))
+  if [[ $h_diff -le 1 || $h_diff -ge 23 ]]; then
     ok "timestamp: UTC hour aligns with system UTC"
   else
     fail "timestamp: hour drift vs UTC: ts_h=$ts_h utc_h=$utc_h diff=$h_diff"
@@ -198,7 +211,7 @@ SINK_FILE2="$(mktemp)"
 HOOK_TELEMETRY_SINK="$(make_sink "$SINK_FILE2")"
 export HOOK_TELEMETRY_SINK
 hook::emit_telemetry "markdown-format" "PostToolUse" "skipped" "$EPOCHREALTIME" '{"tool":"","file":"","findings":[]}' 2>/dev/null
-sleep 0.2
+wait_for_sink "$SINK_FILE2"
 unset HOOK_TELEMETRY_SINK
 
 if [[ -s "$SINK_FILE2" ]]; then
@@ -227,7 +240,7 @@ EOF
 chmod +x "$ROOT7/$REL7"
 export HOOK_TELEMETRY_SINK="$REL7"
 hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"x.md","findings":[]}' "$ROOT7" 2>/dev/null
-sleep 0.2
+wait_for_sink "$OUT7"
 unset HOOK_TELEMETRY_SINK
 if [[ -s "$OUT7" ]] && [[ "$(jq -r '.hook' "$OUT7")" == "markdown-format" ]]; then
   ok "relative sink: resolved against repo_root arg, envelope delivered"
@@ -247,7 +260,7 @@ EOF
 chmod +x "$ROOT8/.claude/hooks/sink.sh"
 export HOOK_TELEMETRY_SINK=".claude/hooks/sink.sh"
 CLAUDE_PROJECT_DIR="$ROOT8" hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"x.md","findings":[]}' 2>/dev/null
-sleep 0.2
+wait_for_sink "$OUT8"
 unset HOOK_TELEMETRY_SINK
 if [[ -s "$OUT8" ]]; then
   ok "relative sink: resolved against CLAUDE_PROJECT_DIR fallback"
@@ -279,7 +292,7 @@ OUT10="$(mktemp)"
 ABS10="$(make_sink "$OUT10")" # mktemp path is absolute
 export HOOK_TELEMETRY_SINK="$ABS10"
 hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$EPOCHREALTIME" '{"tool":"Write","file":"x.md","findings":[]}' "/some/ignored/root" 2>/dev/null
-sleep 0.2
+wait_for_sink "$OUT10"
 unset HOOK_TELEMETRY_SINK
 if [[ -s "$OUT10" ]] && [[ "$(jq -r '.hook' "$OUT10")" == "markdown-format" ]]; then
   ok "absolute sink: passes through unchanged (repo_root ignored)"
@@ -288,6 +301,43 @@ else
 fi
 rm -f "$ABS10" "$OUT10"
 
+# --- Test 11: hook::normalize_path is host-gated on OSTYPE --------------------
+# The drive-letter fold is for Windows/MSYS only (case-insensitive FS). On a
+# case-sensitive POSIX host a real single-letter top dir like /c/Repo must pass
+# through unchanged, or the membership guard collapses it with /c/repo and
+# admits a sibling outside CLAUDE_PROJECT_DIR. normalize_path reads the shell's
+# OSTYPE, so override it in a subshell per case (the global stays intact).
+norm_as() { ( OSTYPE="$1"; hook::normalize_path "$2" ); }
+assert_norm() { # <ostype> <input> <expected> <desc>
+  local got
+  got="$(norm_as "$1" "$2")"
+  if [[ "$got" == "$3" ]]; then
+    ok "normalize_path[$1]: $4"
+  else
+    fail "normalize_path[$1]: $4 — expected '$3', got '$got'"
+  fi
+}
+
+# Windows/MSYS: fold both the POSIX /c/ and colon c:/ drive forms.
+assert_norm msys   "/c/Repo"        "C:/repo"        "msys folds /c/Repo → C:/repo"
+assert_norm msys   "C:/Repo"        "C:/repo"        "msys folds C:/Repo → C:/repo"
+assert_norm msys   "/c/Repo/Sub"    "C:/repo/sub"    "msys folds nested path"
+assert_norm msys   'C:\Repo\x'      "C:/repo/x"      "msys: backslashes → slashes then fold"
+assert_norm cygwin "/c/Repo"        "C:/repo"        "cygwin folds like msys"
+
+# POSIX: NO fold — single-letter top dirs stay distinct (the regression guard).
+assert_norm linux-gnu "/c/Repo"     "/c/Repo"        "linux leaves /c/Repo unchanged"
+assert_norm linux-gnu "/c/repo"     "/c/repo"        "linux leaves /c/repo unchanged"
+assert_norm linux-gnu "/opt/App/Sub" "/opt/App/Sub"  "linux leaves normal path unchanged"
+assert_norm linux-gnu 'a\b'         "a/b"            "linux still converts backslashes"
+
+# Explicit guard: on POSIX the two casings must NOT collapse to one value.
+if [[ "$(norm_as linux-gnu /c/Repo)" != "$(norm_as linux-gnu /c/repo)" ]]; then
+  ok "normalize_path[linux-gnu]: /c/Repo and /c/repo stay distinct"
+else
+  fail "normalize_path[linux-gnu]: /c/Repo and /c/repo collapsed (membership-guard regression)"
+fi
+
 echo
 echo "PASS=$PASS FAIL=$FAIL"
 [[ $FAIL -eq 0 ]]

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

@@ -31,7 +31,6 @@ case "$FILE" in
 esac
 
 TOOL=$(printf '%s' "$INPUT" | jq -r '.tool_name // empty' 2>/dev/null)
-TOOL="${TOOL:-}"
 
 # Resolve repo root early — needed for CWD-anchored config discovery and for
 # computing the schema-required repo-relative path in data.file.
@@ -53,29 +52,34 @@ else
   FILE_REL="${FILE#"$REPO_ROOT"/}"
 fi
 
+# Build the telemetry data object for the current TOOL/FILE_REL. $1 is the
+# findings JSON array. jq is authoritative; the string fallback keeps telemetry
+# best-effort if jq fails (findings collapse to [] since they can't be re-quoted
+# safely without jq).
+build_data_json() {
+  jq -n \
+    --arg tool     "$TOOL" \
+    --arg file     "$FILE_REL" \
+    --argjson findings "$1" \
+    '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null \
+    || printf '{"tool":"%s","file":"%s","findings":[]}' "$TOOL" "$FILE_REL"
+}
+
 MDLINT=()
 if command -v markdownlint-cli2 >/dev/null 2>&1; then
   MDLINT=(markdownlint-cli2)
 elif command -v npx >/dev/null 2>&1; then
   MDLINT=(npx markdownlint-cli2)
 else
   # markdownlint unavailable — emit skipped telemetry then exit cleanly.
-  data_json=$(jq -n \
-    --arg tool     "$TOOL" \
-    --arg file     "$FILE_REL" \
-    --argjson findings '[]' \
-    '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null) || data_json="{\"tool\":\"$TOOL\",\"file\":\"$FILE_REL\",\"findings\":[]}"
+  data_json=$(build_data_json '[]')
   hook::emit_telemetry "markdown-format" "PostToolUse" "skipped" "$start" "$data_json" "$REPO_ROOT"
   exit 0
 fi
 
 if FIX_OUTPUT=$(cd "$REPO_ROOT" && "${MDLINT[@]}" --fix "$FILE" 2>&1); then
   # Clean after fix — emit ok with empty findings.
-  data_json=$(jq -n \
-    --arg tool     "$TOOL" \
-    --arg file     "$FILE_REL" \
-    --argjson findings '[]' \
-    '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null) || data_json="{\"tool\":\"$TOOL\",\"file\":\"$FILE_REL\",\"findings\":[]}"
+  data_json=$(build_data_json '[]')
   hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$start" "$data_json" "$REPO_ROOT"
   exit 0
 fi
@@ -88,19 +92,21 @@ fi
 # Violation lines are identified by the " MD<digits>/" rule-code pattern.
 hook::ctx_reset
 hook::ctx_append "markdown-format: $(basename "$FILE") has markdownlint findings:"
-FINDINGS_JSON='[]'
+findings_raw=""
 while IFS= read -r line; do
   hook::ctx_append "  $line"
   if [[ "$line" =~ [[:space:]]MD[0-9]+/ ]]; then
-    FINDINGS_JSON=$(printf '%s' "$FINDINGS_JSON" | jq --arg l "$line" '. + [$l]' 2>/dev/null) || true
+    findings_raw+="$line"$'\n'
   fi
 done <<<"$FIX_OUTPUT"
 hook::ctx_flush PostToolUse
 
-data_json=$(jq -n \
-  --arg tool     "$TOOL" \
-  --arg file     "$FILE_REL" \
-  --argjson findings "$FINDINGS_JSON" \
-  '{tool:$tool,file:$file,findings:$findings}' 2>/dev/null) || data_json="{\"tool\":\"$TOOL\",\"file\":\"$FILE_REL\",\"findings\":[]}"
+# Build the findings array in one jq pass (one JSON string per matched line).
+FINDINGS_JSON='[]'
+if [[ -n "$findings_raw" ]]; then
+  FINDINGS_JSON=$(printf '%s' "$findings_raw" | jq -R . | jq -s . 2>/dev/null) || FINDINGS_JSON='[]'
+fi
+
+data_json=$(build_data_json "$FINDINGS_JSON")
 hook::emit_telemetry "markdown-format" "PostToolUse" "ok" "$start" "$data_json" "$REPO_ROOT"
 exit 0

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

@@ -49,6 +49,19 @@ make_sink() {
   printf '%s' "$s"
 }
 
+# wait_for_sink <file> [max_polls] → block until <file> is non-empty (the
+# fire-and-forget sink has flushed) or the bound elapses, polling in 20ms steps.
+# Replaces a fixed sleep so delivery assertions fire as soon as the write lands
+# instead of racing variable process-spawn latency (notably on Windows Git Bash).
+wait_for_sink() {
+  local f="$1" tries="${2:-150}"
+  while (( tries-- > 0 )); do
+    [[ -s "$f" ]] && return 0
+    sleep 0.02
+  done
+  return 1
+}
+
 # --- Build the throwaway consumer repo --------------------------------------
 REPO="$WORK/consumer"
 mkdir -p "$REPO"
@@ -214,7 +227,7 @@ printf '# Doc T\n\n## Section\n\ntext\n\n## Section\n\nmore text\n' >"$REPO/fixt
 _OUT_T="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureT.md" \
   | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$STUB_SINK" bash "$HOOK")"
 RC_T=$?
-sleep 0.3  # allow background sink to flush
+wait_for_sink "$TEL_FILE"
 
 if [[ $RC_T -eq 0 ]]; then ok "telemetry/stub-sink: hook exit 0"; else fail "telemetry/stub-sink: hook exit $RC_T"; fi
 
@@ -288,7 +301,7 @@ printf '# Clean Doc\n\nSome text.\n' >"$REPO/fixtureClean.md"
 _OUT_CLEAN="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureClean.md" \
   | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$STUB_CLEAN" bash "$HOOK")"
 RC_CLEAN=$?
-sleep 0.3
+wait_for_sink "$TEL_CLEAN"
 
 if [[ $RC_CLEAN -eq 0 ]]; then ok "telemetry/clean: hook exit 0"; else fail "telemetry/clean: hook exit $RC_CLEAN"; fi
 if [[ -s "$TEL_CLEAN" ]]; then
@@ -312,7 +325,7 @@ printf '# Failing Sink Doc\n\nSome text.\n' >"$REPO/fixtureFailSink.md"
 _OUT_FS="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureFailSink.md" \
   | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$FAIL_SINK" bash "$HOOK")"
 RC_FS=$?
-sleep 0.3
+wait_for_sink "$FAIL_SINK_FILE"
 
 if [[ $RC_FS -eq 0 ]]; then ok "telemetry/fail-sink: hook exit 0 despite sink failure"; else fail "telemetry/fail-sink: hook exit $RC_FS, expected 0"; fi
 rm -f "$FAIL_SINK_FILE"
@@ -348,7 +361,7 @@ LEAK_SINK="$(make_sink "cat >\"$TEL_LEAK\"")"
 printf '# Leak Doc\n\n## Section\n\ntext\n\n## Section\n\nmore text\n' >"$REPO/fixtureLeakCheck.md"
 OUT_LEAK="$(cd "$UNRELATED" && printf '{"tool_input":{"file_path":"%s"},"tool_name":"Write"}' "$REPO/fixtureLeakCheck.md" \
   | env -u CLAUDE_PROJECT_DIR HOOK_MARKDOWN_FORMAT_ENABLED=true HOOK_TELEMETRY_SINK="$LEAK_SINK" bash "$HOOK")"
-sleep 0.3
+wait_for_sink "$TEL_LEAK"
 
 # The hook stdout must NOT contain the telemetry envelope's top-level keys
 if printf '%s' "$OUT_LEAK" | jq -e '.schema_version' >/dev/null 2>&1; then