Skip to content

fix(markdown-formatter): guard EPOCHREALTIME for Bash < 5 portability#17

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/markdown-formatter-epochrealtime
Jun 29, 2026
Merged

fix(markdown-formatter): guard EPOCHREALTIME for Bash < 5 portability#17
kyle-sexton merged 1 commit into
mainfrom
fix/markdown-formatter-epochrealtime

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Propagation follow-up from #16's Codex review: markdown-formatter carried the identical latent bug.

EPOCHREALTIME is Bash 5.0+. The hook's start=$EPOCHREALTIME under set -u aborts on Bash 3.2/4.x (e.g. stock macOS) before the advisory exit 0 — failing every Write/Edit, not just markdown ones (the capture runs before the file-type filter).

Fix

  • Default the stamp: start=${EPOCHREALTIME:-} (no set -u abort).
  • emit_tel wrapper skips telemetry when the stamp is empty, so the hook still formats on older bash; the high-res envelope is simply omitted there.
  • hook-utils.sh stays byte-identical to the bash-lint copy (the guard lives in the hook, not the lib).
  • Version 0.1.0 → 0.1.1 so already-installed consumers receive the fix.

Verification (all green locally)

  • shellcheck · editorconfig · typos · markdownlint (0 errors) · comment-hygiene
  • claude plugin validate --strict passed
  • Tests: markdown-format.test.sh 41/0, hook-utils.test.sh 37/0
  • Confirmed hook-utils.sh byte-identical to plugins/bash-lint/hooks/hook-utils.sh

🤖 Generated with Claude Code

EPOCHREALTIME is Bash 5.0+; `start=$EPOCHREALTIME` under set -u aborts on Bash
3.2/4.x before the advisory exit 0, failing every edit. Default the stamp to
empty and skip telemetry when it is unavailable, so the hook still formats on
older bash. Mirrors the bash-lint fix; hook-utils.sh stays byte-identical.
Bumps version to 0.1.1 so installed consumers receive the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WWt8nQzh8BAoZXRZRt2ARU
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

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


PR Review: fix(markdown-formatter): guard EPOCHREALTIME for Bash < 5 portability

  • Gather context and understand the request
  • Read PR diff and changed files
  • Review plugin.json version bump
  • Review markdown-format.sh changes
  • Review README.md changes
  • Post comprehensive review feedback

Summary

This is a correct, minimal, and well-scoped bug fix. The root cause was that start=$EPOCHREALTIME under set -uo pipefail would abort the entire hook on Bash < 5 (e.g. macOS stock Bash 3.2) — before the file-type filter ran — causing every Write/Edit to silently fail. The fix is a faithful propagation of the same pattern applied to bash-lint in PR #16.

Verdict: approve — no blockers.


markdown-format.sh — Core Fix

The guard (line 25):

start=${EPOCHREALTIME:-}

Correct. ${VAR:-} is portable back to POSIX sh; it silences set -u by providing an empty default when the variable is unset. On Bash < 5, start becomes the empty string, then the emit_tel wrapper short-circuits before any hook::emit_telemetry call that would itself hit local now=$EPOCHREALTIME under set -u.

The wrapper (lines 30–33):

emit_tel() {
  [[ -n "$start" ]] || return 0
  hook::emit_telemetry "$@"
}

Clean approach for three call sites. Using a closure over the outer-scope $start is idiomatic here since start is a script-level variable — no ambiguity. Checked all three call sites are correctly updated (lines 86, 93, 121); no stray hook::emit_telemetry direct calls remain.

hook-utils.sh byte-identical: confirmed with diff — no divergence from the bash-lint copy, which is the right constraint to maintain for future syncs.

Pattern fidelity: the implementation matches bash-lint.sh exactly (confirmed with grep). This is correct propagation, not a novel approach.


plugin.json — Version Bump

0.1.0 → 0.1.1 is the right semver increment for a bug fix. Already-installed consumers will receive the patch on next update. ✓


README.md — Docs

The added paragraph clearly communicates the compatibility baseline and the graceful degradation behavior. It belongs in the Requirements section. ✓


Minor Observation (non-blocking)

The comment block at lines 27–29 slightly echoes the comment already at lines 21–24 and also what the emit_tel code itself expresses. Given the existing comment density in the file this is a judgment call — the extra comment does no harm and follows the style established in the bash-lint copy.


Checklist

Concern Status
set -u abort on Bash < 5 fixed
No regression on Bash 5+ (empty guard is a no-op when EPOCHREALTIME is set)
All three hook::emit_telemetry call sites replaced
hook-utils.sh byte-identical to bash-lint copy
Pattern matches bash-lint fix
Version bump correct
README updated

@kyle-sexton
kyle-sexton merged commit 37be40a into main Jun 29, 2026
16 checks passed
@kyle-sexton
kyle-sexton deleted the fix/markdown-formatter-epochrealtime branch June 29, 2026 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant