feat(bash-lint): add shell lint/format-on-edit plugin

[kyle-sexton]

Summary

Ports medley's bash-lint hook into the marketplace as the second plugin (after markdown-formatter), as a repo-agnostic, plugin-form-safe capability.

Behavior — PostToolUse hook on Write|Edit of *.sh/*.bash:

• ShellCheck lints always (warning+), non-mutating, advisory — the hook always exits 0.
• shfmt formats only when the consumer opts in with an .editorconfig (walking up to the repo root). With none present, the file is left untouched rather than rewritten to shfmt's defaults — the plugin never imposes a style the repo didn't choose. Run with no formatting flags so .editorconfig stays authoritative.
• Findings surface via additionalContext plus the marketplace hook::emit_telemetry envelope. Reads the consumer's own .shellcheckrc/.editorconfig; ships no rules. Kill switch: HOOK_BASH_LINT_ENABLED.

Generalized from medley (not a byte-copy)

• Stripped medley-policy prose (lefthook/CI "hard gate" etc.) — repo-agnostic.
• Dropped the markdownlint-era cd — shfmt/shellcheck resolve config from the file's path, so no working-directory assumption is needed.
• Gated shfmt on .editorconfig opt-in vs medley's unconditional shfmt -w.
• Used the marketplace's hook::emit_telemetry envelope over medley's older emit_timed_event.
• hook-utils.sh is byte-identical to the markdown-formatter copy (git blob 6342c502…).

Verification (all green locally)

• claude plugin validate --strict — plugin manifest and catalog manifest
• shellcheck · editorconfig-checker · typos · markdownlint (0 errors) · comment-hygiene
• exec-bit modes (755 scripts / 644 lib) · LF endings · final newline
• Tests: hook-utils.test.sh 37/0, bash-lint.test.sh 28/0 (includes shfmt gate ON/OFF and telemetry envelope shape)
• Runtime wiring smoke: invoked via the exact hooks.json command + ${CLAUDE_PLUGIN_ROOT} with an envelope on stdin — formatted the file and surfaced SC2154, exit 0

🤖 Generated with Claude Code

[claude]

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

---

PR Review: feat(bash-lint)

Tasks

• Gather context and understand the request
• Read plugin manifest and marketplace registration
• Review hook script (bash-lint.sh)
• Review hook utilities (hook-utils.sh)
• Review test files
• Review hooks.json wiring
• Check CLAUDE.md compliance (fresh-docs mandate, design rules)
• Post final review

---

Overall assessment

This is a solid, well-structured plugin port. The architecture decisions are sound: ShellCheck-always / shfmt-opt-in, file-anchored config discovery, advisory exit 0, and proper telemetry envelope. Test coverage is broad (37 + 28 cases). A few issues are worth addressing before merge.

---

Issues

1. hook-utils.sh has wrong identity — copied verbatim from markdown-formatter

hook-utils.sh line 3 says:

"Minimal hook utility library for the markdown-formatter plugin."

And line 8–9:

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

The PR description intentionally preserves byte-identity with the markdown-formatter copy (git blob 6342c502), but this creates a semantic problem: the guard variable _MDFMT_HOOK_UTILS_LOADED is named after the wrong plugin, and the file-header comment is misleading for anyone reading the bash-lint plugin in isolation. Each hook runs in its own subprocess so the guard collision has no runtime impact today — but if hooks ever share a shell (possible with future Claude Code changes), both plugins sourcing this file in the same process would break. The byte-identity argument trades correctness for DRY, and neither plugin directly benefits from DRY in isolated subprocesses.

Recommend: Update the header comment and guard variable to _BASH_LINT_HOOK_UTILS_LOADED (or a generic _HOOK_UTILS_LOADED if shared-library semantics are intended). Fix this →

---

2. shfmt errors are silently discarded — hook reports "ok" when formatting fails

bash-lint.sh line 97:

shfmt -w "$FILE" 2>/dev/null
ran_any=1

shfmt exits non-zero when it encounters a parse error (e.g., a syntax-broken .sh file). The exit code is discarded and ran_any=1 is set regardless, so a failed shfmt run results in a telemetry status of "ok" with findings:[]. The consumer sees no signal that formatting silently failed. This matters most when a CI gate also runs shfmt -d and finds a mismatch the hook should have caught.

Recommend: Capture the exit code and either surface a finding or set status to "error" when shfmt fails:

if ! shfmt -w "$FILE" 2>/tmp/shfmt_err; then
  # surface shfmt stderr in additionalContext
fi

At minimum, don't suppress stderr for errors. Fix this →

---

3. additionalContext shows basename instead of repo-relative path

bash-lint.sh line 109:

hook::ctx_append "bash-lint: $(basename "$FILE") has ShellCheck findings:"

basename strips the directory, so in a repo with multiple files named utils.sh the message is ambiguous. FILE_REL is already computed by this point (line 49–58) and gives the repo-relative path, which is exactly what Claude needs to locate the file. Fix this →

---

4. No test for shfmt error path (broken syntax file)

bash-lint.test.sh tests the shfmt gate on .editorconfig presence/absence but not on a file that shfmt cannot parse (e.g., a heredoc with a bash syntax error). Given issue #2 above, a test like this would both catch the silent-failure bug and provide a regression guard once it's fixed.

---

5. CLAUDE_PROJECT_DIR membership guard is not integration-tested

All bash-lint.test.sh invocations explicitly unset CLAUDE_PROJECT_DIR. The membership guard in hook::read_file_path is unit-tested in hook-utils.test.sh via normalize_path, but there is no end-to-end test that sets CLAUDE_PROJECT_DIR to a repo and verifies the hook rejects a file in a sibling directory. This is low-risk since the logic lives entirely in the shared util, but worth noting.

---

Minor notes

• hooks.json timeout: 15 seconds is reasonable for a single file. When shellcheck runs with -x on a deeply-sourced hierarchy it can be slow. No action needed, just be aware.
• README.md install commands: The @melodic-software scoping syntax matches the marketplace pattern from the existing markdown-formatter README — consistent.
• plugin.json: No hooks field is required (consistent with markdown-formatter; the plugin system auto-discovers hooks/hooks.json).
• CLAUDE.md fresh-docs mandate: Applies to the PR author rather than reviewer. The validation (claude plugin validate --strict) is mentioned in the PR description but the canonical doc URLs aren't cited. For future PRs, citing the fetched URL in the description satisfies the mandate explicitly.

---

Summary

Two issues need attention before merge: the silent shfmt error suppression (#2) and the misleading hook-utils.sh identity (#1). Issue #3 (basename vs. FILE_REL) is a usability improvement worth doing in the same pass. The rest can be deferred. The test suite and architecture are otherwise clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2faa75ca03

ℹ️ 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".

[claude]

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

---

PR Review (post-43dd32d) — Updated

Tasks

• Gather context and understand the request
• Read updated bash-lint.sh (43dd32d)
• Read hook-utils.sh
• Check which prior issues are resolved vs. still open
• Post updated review

---

What 43dd32d fixed

The latest commit addressed both Codex P2 findings cleanly:

• Bash < 5 portability — start=$EPOCHREALTIME → start=${EPOCHREALTIME:-} plus an emit_tel() guard wrapper. The hook now lints and formats on Bash 3.2/4.x instead of aborting under set -u before the advisory exit 0. ✅
• --apply-ignore — shfmt -w "$FILE" → shfmt --apply-ignore -w "$FILE". Repos with ignore = true editorconfig sections now have their opt-out respected even on direct-file invocations. ✅
• Regression test — REPO_IGN / gen.sh case in bash-lint.test.sh (lines 200–221) guards this. ✅

---

Still open from the previous review

Three issues from the first review pass were not addressed in 43dd32d.

1. hook-utils.sh carries markdown-formatter identity — misleading, latent guard collision

hook-utils.sh line 2:

# Minimal hook utility library for the markdown-formatter plugin.

hook-utils.sh lines 8–9:

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

hook-utils.sh lines 71–73 also still references markdownlint in the hook::repo_root docstring — stale markdown-formatter prose.

The guard collision is latent today (each hook runs in its own subprocess), but the wrong-plugin comment is an active correctness hazard for anyone reading or auditing the bash-lint plugin in isolation. These are mechanical fixes.

Fix this →

---

2. shfmt failure is silent — hook reports "ok" when formatting aborts

bash-lint.sh line 111:

shfmt --apply-ignore -w "$FILE" 2>/dev/null
ran_any=1

The exit code is still discarded and stderr is still suppressed. When shfmt encounters a parse error (e.g. #!/usr/bin/env bash\nif true; then with no fi) it exits non-zero without rewriting the file — but the hook sets ran_any=1, reports status "ok", and emits findings:[]. The consumer sees a clean telemetry envelope when formatting silently failed.

Minimum fix: capture exit code and surface it when non-zero:

shfmt_err=$(mktemp)
if ! shfmt --apply-ignore -w "$FILE" 2>"$shfmt_err"; then
  hook::ctx_reset
  hook::ctx_append "bash-lint: shfmt failed on $FILE_REL:"
  hook::ctx_append "  $(cat "$shfmt_err")"
  hook::ctx_flush PostToolUse
fi
rm -f "$shfmt_err"
ran_any=1

Fix this →

---

3. additionalContext header uses basename — ambiguous when multiple files share a name

bash-lint.sh line 123:

hook::ctx_append "bash-lint: $(basename "$FILE") has ShellCheck findings:"

FILE_REL is already computed on lines 59–68 and gives the repo-relative path. In any repo with more than one utils.sh the basename message is ambiguous. One-character fix:

hook::ctx_append "bash-lint: $FILE_REL has ShellCheck findings:"

Fix this →

---

Summary

43dd32d cleanly resolves the two Codex findings (--apply-ignore, Bash < 5 portability) and adds a focused regression test. Three items from the first review pass remain: the hook-utils.sh identity mislabeling (#1), the silent shfmt failure (#2), and the basename-vs-FILE_REL usability nit (#3). Issues #1 and #3 are mechanical and low-risk; #2 is the one that affects consumer-observable behavior.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43dd32dc2e

ℹ️ 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".

[claude]

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

---

PR Review (post-149090e) — Updated

Tasks

• Gather context and understand the request
• Read updated bash-lint.sh (149090e + 9bc923f)
• Read hook-utils.sh
• Check which prior issues are resolved vs. still open
• Check new Codex findings addressed by 9bc923f and 149090e
• Post updated review

---

What 9bc923f + 149090e fixed

Both new Codex P2 findings from the last review pass are cleanly addressed:

• shfmt --apply-ignore version gate (9bc923f) — line 142 now falls back to shfmt -w "$FILE" when --apply-ignore is unknown (shfmt < 3.8). The README-only minimum-version requirement is gone; the hook degrades gracefully. ✅
• Shell EditorConfig precision (149090e) — section_applies_to_shell() + shell_editorconfig_opt_in() replace the old any-editorconfig-present gate. A repo whose .editorconfig only governs [*.md] no longer triggers shell formatting. New tests for both [*.md]-only (no format) and [*] catch-all (format). ✅

---

Still open from previous reviews

Three items from the prior review passes were not addressed in either new commit.

1. hook-utils.sh carries markdown-formatter identity — misleading, latent guard collision

hook-utils.sh line 2:

# Minimal hook utility library for the markdown-formatter plugin.

hook-utils.sh lines 8–9:

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

hook-utils.sh lines 71–72:

# markdownlint config auto-discovery is CWD-anchored, so the hook cd's here
# before linting.

The guard variable and the two comments are markdown-formatter-specific. The guard collision is latent today (isolated subprocesses), but the wrong-plugin comments are an active readability and audit hazard for anyone reading the bash-lint plugin in isolation. All three are mechanical one-liners.

Fix this →

---

2. shfmt failure is still silent — hook reports "ok" when formatting aborts

bash-lint.sh line 142:

shfmt --apply-ignore -w "$FILE" 2>/dev/null || shfmt -w "$FILE" 2>/dev/null
ran_any=1

The || chain is now a version-fallback (correct), but if both calls fail (e.g., the file contains a parse error that shfmt cannot process), ran_any=1 is still set unconditionally and all exit codes are discarded. The hook then reports status "ok" with findings:[], giving the consumer no signal that formatting silently failed. This diverges from the CI gate (shfmt -d) that would catch the same parse error.

shfmt_err=$(mktemp)
if ! { shfmt --apply-ignore -w "$FILE" 2>"$shfmt_err" || shfmt -w "$FILE" 2>"$shfmt_err"; }; then
  hook::ctx_reset
  hook::ctx_append "bash-lint: shfmt failed on $FILE_REL:"
  hook::ctx_append "  $(cat "$shfmt_err")"
  hook::ctx_flush PostToolUse
fi
rm -f "$shfmt_err"
ran_any=1

Fix this →

---

3. additionalContext header uses basename — ambiguous in repos with duplicate filenames

bash-lint.sh line 154:

hook::ctx_append "bash-lint: $(basename "$FILE") has ShellCheck findings:"

FILE_REL (computed lines 59–68) already gives the repo-relative path. Replace with:

hook::ctx_append "bash-lint: $FILE_REL has ShellCheck findings:"

Fix this →

---

New minor note on section_applies_to_shell

bash-lint.sh line 92:

[[ "$h" =~ \*\.(sh|bash)([^[:alnum:]]|$) ]]

The [^[:alnum:]] lookahead makes *.sh.tmpl match (. is non-alphanumeric), so a [*.sh.tmpl] section in an editorconfig would incorrectly opt shell files in. This is a narrow edge case — .sh.tmpl editorconfig sections are rare — but worth noting. A tighter pattern like \*\.(sh|bash)([^[:alnum:]._]|$) or anchoring on ] would avoid the false positive. Not a blocker.

Test coverage for the brace syntax path ([*.{sh,bash}]) is also absent; the regex ([{,](sh|bash)[,}]) handles it, but a single case in the test suite would provide a regression guard.

---

Summary

9bc923f and 149090e close all remaining Codex findings cleanly. The section_applies_to_shell implementation is well-reasoned and the new tests cover the critical cases. Three items from the first two review passes remain unaddressed: the hook-utils.sh identity mislabeling (#1), the silent shfmt failure (#2), and the basename-vs-FILE_REL usability nit (#3). Issues #1 and #3 are one-liners; #2 is the one that affects consumer-observable behavior.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 149090eed2

ℹ️ 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]

Match EditorConfig globs against the edited path

When a repo uses a path-scoped shell section such as [scripts/*.sh] or [generated/*.sh], this check returns true for every edited .sh file even if that section does not match the file being edited. EditorConfig path globs containing / are relative to the .editorconfig directory and only matching sections apply, so editing tools/foo.sh in that repo will still run bare shfmt and can rewrite a file the repo did not opt in; this is fresh evidence beyond the earlier non-shell-section comment because the current fix still checks only for a shell-looking header, not whether it matches FILE_REL.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Add SCRIPTDIR to ShellCheck source lookup

For scripts that source sibling files with paths like . ./lib.sh, this invocation depends on Claude Code's hook working directory rather than the edited script's directory. ShellCheck's docs for source-path say sourced files are searched from the working directory by default and that SCRIPTDIR is the special value for the checked script's directory, so edits made while the hook runs from the repo root or another cwd will surface false SC1091 findings for valid relative sources unless the user has a compensating .shellcheckrc.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Do not fall back over shfmt ignore rules

On systems with shfmt older than 3.8, --apply-ignore is an unknown flag, so this || shfmt -w fallback formats the file directly without applying .editorconfig ignore = true rules. The shfmt manual says ignore rules are skipped for direct file formatting without --apply-ignore, so a generated/vendor script that the repo explicitly ignored can still be rewritten; this is fresh evidence after the earlier ignore-rule thread because the current fallback reintroduces the direct-file behavior specifically on older shfmt installs.

Useful? React with 👍 / 👎.