Skip to content

fix(bash-format): cover the gitignored option in setup skill and README, correct 0.8.5 changelog - #5251

Merged
kyle-sexton merged 8 commits into
mainfrom
fix/audit-bash-format
Sep 29, 2026
Merged

kyle-sexton merged 8 commits into
mainfrom
fix/audit-bash-format

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Refs: #4671
Refs: #4612
Refs: #4240

Summary

Audit findings for bash-format (three, all confirmed on main): the setup skill and README did not cover the bash_format_lint_gitignored option, the skill and README described the missing-tool notice differently, and the 0.8.5 CHANGELOG entry claimed a session-start probe, prerequisites.json and a model-invocable check that this plugin does not ship.

Fix

  • skills/setup/SKILL.md: Purpose names both options; check step 7 reports both effective values and explains that a gitignored edit is skipped unless bash_format_lint_gitignored is true; the silent-skip explanation covers the gitignored skip; apply can set either key. The notice wording now matches the README ("once per session and agent, renewed every eighth skip").
  • README.md: Behavior bullet for the gitignored skip, both options in the Configuration text and table, the repeated notice clause stated once.
  • skills/setup/evals/evals.json: eval 1 uses the README wording; a new eval asks why an edit to .work/scratch.sh was not formatted.
  • CHANGELOG.md: the 0.8.5 entry is a one-line shared-sync note that says bash-format ships no probe or prerequisites.json and links hooks: prerequisites.json, session-start probe and check skill for the remaining binary-probing format plugins #5286. This PR edits a released entry; the 0.8.6 entry names the same edit ("CHANGELOG: corrected the 0.8.5 entry, which claimed a session-start probe and prerequisites.json this plugin does not ship").
  • README.md Requirements and skills/setup/SKILL.md check name Node.js on PATH (cross-group request from hook-launcher, F12): every hook row launches through node hooks/exec-bash.mjs. A node row joins the check's pre-computed probes, the check steps are renumbered, and eval 1 expects a missing node to be a FAIL.
  • CHANGELOG.md: the 0.7.60, 0.7.61, 0.8.2 and 0.8.5 entries describe shared launcher/library code this plugin never calls or whose flags its rows do not pass, so each is one line: "Shared launcher/library sync; no change to this plugin's behavior" (hook-launcher request, F36). Headings and dates are unchanged. The 0.8.6 entry names these edits alongside the 0.8.5 correction. The 0.8.5 entry now links hooks: prerequisites.json, session-start probe and check skill for the remaining binary-probing format plugins #5286 for the remaining probe coverage (biome-format request). 0.8.4 is a genuine README change and stays.
  • Version 0.8.5 to 0.8.6 (patch).

Verification

  • bash scripts/check-changelog-parity.sh --check --check-order: pass
  • bash scripts/check-changelog-parity.sh --check-preserved origin/main: pass (87 headings preserved)
  • bash scripts/check-changelog-parity.test.sh: 86 pass, 0 fail
  • bash scripts/validate-plugins.sh: all manifests and the catalog validated
  • bash plugins/bash-format/hooks/bash-format.test.sh: 58 pass, 0 fail
  • python3 scripts/sync-plugin-options-docs.py --check: up to date
  • bash scripts/check-changed-skills.sh origin/main: setup skill pass, 0 errors, 0 warnings
  • bash scripts/check-skill-portability.sh origin/main: no unexcused coupling tokens

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB

kyle-sexton and others added 3 commits September 29, 2026 01:04
…README and evals

The setup skill still called the enabled toggle the only tunable and never
reported bash_format_lint_gitignored, so a user asking why an edit to a
gitignored file was not formatted got no answer. check now reports both
option values and adds a gitignored-files INFO item, apply covers both
keys, the README lists both options and the gitignored skip, and a new
eval pins the answer. The skip-notice cadence is stated once in the README
and the skill, matching the README wording.

Refs #4671
Refs #4612

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

kyle-sexton and others added 2 commits September 29, 2026 02:54
README Requirements and the setup skill's check name Node.js on PATH, since
every hook row launches through node hooks/exec-bash.mjs. The 0.7.60, 0.7.61,
0.8.2 and 0.8.5 entries become one-line shared launcher/library syncs, and the
0.8.5 entry links the follow-up issue #5286 for remaining probe coverage.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 29, 2026 07:15
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 40s —— View job


Security review complete

  • Gather context (PR body, changed files list)
  • Fetch full diff (origin/main...HEAD) for plugins/bash-format/
  • Review each changed file for security-relevant issues
  • Check for instruction-surface deletions (SKILL.md is a skill body)
  • Report findings

No security issues found.

This PR touches only documentation and configuration for the bash-format plugin:

  • README.md, skills/setup/SKILL.md — prose/documentation edits (adds the bash_format_lint_gitignored option, a node requirement note, renumbers check steps).
  • CHANGELOG.md — rewrites historical entries to be less specific (e.g. the 0.8.5 entry and several 0.7.x/0.8.x entries collapse to "Shared launcher/library sync; no change to this plugin's behavior"). These are historical records, not enforced directives — nothing in the tree relies on their wording to gate behavior, so this is not an instruction-surface deletion.
  • skills/setup/evals/evals.json — updates eval expectations/prompts to match the new wording; no executable eval harness changes.
  • .claude-plugin/plugin.json — version bump only (0.8.5 → 0.8.6).

No hook scripts (hooks/*.sh, hooks/*.mjs), no code paths, no credential/token handling, and no GitHub Actions workflow files are touched. The new node: command -v node 2>/dev/null || echo "absent" pre-computed-context probe in SKILL.md follows the same safe pattern already used for jq/shellcheck/shfmt on that file — no injection surface.

Checked the instruction-surface-deletion lens specifically against the SKILL.md consolidation (repeated "once per session and agent, renewed every eighth skip" notices collapsed into one header line) and the CHANGELOG rewording: both restate/compress existing content without removing any enforced guardrail, so neither qualifies as a finding under the instruction exception register.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T07:18:34.184792Z d2bd099 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude finished the code review

  • Gather context (PR diff, CLAUDE.md/AGENTS.md conventions)
  • Review changed files against /review:code-review criteria
  • Adversarially verify candidate findings
  • Post inline comments / summary

Summary

Reviewed all 5 changed files (plugin.json, CHANGELOG.md, README.md, skills/setup/SKILL.md, skills/setup/evals/evals.json) against origin/main...HEAD. Cross-checked the documentation claims against the actual hook source (hooks/hooks.json, hooks/bash-format.sh, hooks/rewrite-guard.sh, hooks/hook-utils.sh):

  • Confirmed both hooks.json rows launch via node exec-bash.mjs (backs the new Node.js requirement and check step 2).
  • Confirmed the gitignored-skip path (hook::gitignored_out_of_scope) exits silently with no notice_once call, matching the "silently skipped, no notice" claims in the README/SKILL.md.
  • Confirmed HOOK_NOTICE_RENEW_EVERY defaults to 8 in hook-utils.sh, matching the "renewed every eighth skip" wording now shared between README and SKILL.md.
  • The plugin.json version bump (0.8.5 → 0.8.6) matches the new CHANGELOG.md heading, and the reduced 0.7.60/0.7.61/0.8.2/0.8.5 entries are consistent with what those releases actually touched in this plugin (shared launcher/library files, not this plugin's own behavior).
  • evals.json eval 1's updated expectations and the new eval 7 line up with the corresponding SKILL.md check steps.

I did not independently re-run the verification commands listed in the PR body (check-changelog-parity.sh, sync-plugin-options-docs.py, the hook test suite, etc.) — those are author-claimed and unverified by me; the sandbox here declined approval to execute them.

Finding (posted inline)

plugins/bash-format/README.md:68-69 — the new Node.js claim ("Claude Code's native installer does not ship or use Node.js; only its npm package needs it") is a net-new upstream-derived restatement with a basis link and an as-of date, but no recheck trigger. docs/conventions/upstream-drift/README.md requires all four parts (claim, basis, as-of date, observable recheck trigger) for exactly this kind of restated upstream specific, and its Adopters section states a surface "adopts … the required parts" the next time it restates such a claim — which this PR does. This class of gap is explicitly reasoning-only (not linter-enforced) per that doc's own Enforceability section, so it's a legitimate review-lane finding rather than noise.

Everything else — the bash_format_lint_gitignored coverage, the shared-notice wording unification, the Node.js requirement documentation, and the CHANGELOG corrections — checked out against the source.

Comment thread plugins/bash-format/README.md Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2bd09951f

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

Comment thread plugins/bash-format/CHANGELOG.md Outdated
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 38s —— View job


Security review complete

  • Gather context (PR body, changed files list)
  • Diff origin/main...HEAD for plugins/bash-format/ (HEAD 1a49f7a)
  • Review each changed file for security-relevant issues
  • Check instruction-surface-deletion lens (SKILL.md is a skill body)
  • Post findings

No security issues found.

All 5 changed files are documentation/config, no executable code paths change:

  • README.md, skills/setup/SKILL.md — prose edits: new bash_format_lint_gitignored coverage, a node requirement note with a recheck trigger, check steps renumbered, apply reworded as guidance-only (no write path — consistent with the skill's own "no write path" statement).
  • CHANGELOG.md — rewrites historical 0.7.60/0.7.61/0.8.2/0.8.5 entries to a shared one-line note and adds a 0.8.6 entry; corrects the 0.8.5 entry's inaccurate claim of a session-start probe/prerequisites.json. These are historical records only — nothing in the tree gates behavior on their wording.
  • skills/setup/evals/evals.json — eval expectations updated to match; no executable harness change.
  • .claude-plugin/plugin.json — version bump 0.8.5 → 0.8.6 only.

No hook scripts (hooks/*.sh, hooks/*.mjs), no GitHub Actions workflow files, and no credential/token handling are touched in this diff.

Instruction-surface-deletion check: SKILL.md's check section is reworded (once-per-session-and-agent notice text consolidated to one line in the Requirements header instead of repeated per bullet) but no guardrail behavior is removed — the underlying notice cadence (HOOK_NOTICE_RENEW_EVERY = 8 in hook-utils.sh) is unchanged and still enforced in code; the doc only restates it once instead of three times. Not a finding.

Confirms the same conclusion the prior review reached on an earlier commit (d2bd099); this head (1a49f7a) only added a recheck trigger to the Node.js claim and softened the apply wording per two review comments, neither of which is security-relevant.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude finished the code review

  • Gather context (PR body, prior review comments, changed files)
  • Invoke /review:code-review and follow its instructions
  • Review the delta since the prior review pass, verify claims against hook source
  • Post summary comment (no anchorable findings to post inline)

Summary

This synchronize push is commit 1a49f7a, which addresses both outstanding findings from the prior review round on this PR:

  1. The Node.js upstream-drift finding (missing recheck trigger) — now reads "recheck when a Claude Code release note changes the installer or its runtime requirements," satisfying the four-part upstream-drift record.
  2. The Codex apply-is-guidance-only finding on the CHANGELOG entry — "apply can set either option" is now "apply gives guidance for reconfiguring either option," matching SKILL.md's own "no write path" claim.

I re-reviewed the full diff against origin/main (not just the delta) and cross-checked every factual claim against the actual hook source:

  • hooks/hooks.json: both PostToolUse rows launch via node ${CLAUDE_PLUGIN_ROOT}/hooks/exec-bash.mjs … — backs the new Node.js requirement bullet and check step 2.
  • hooks/bash-format.sh:46-48 + hooks/rewrite-guard.sh:210-213: hook::gitignored_out_of_scope gates on CLAUDE_PLUGIN_OPTION_BASH_FORMAT_LINT_GITIGNORED, and the skip path calls hook::finish skipped findings array '[]' with no notice call — confirms the "silently skipped, no notice" claim in the README/SKILL.md/evals.
  • hooks/hook-utils.sh:278: HOOK_NOTICE_RENEW_EVERY="${HOOK_NOTICE_RENEW_EVERY:-8}" — confirms "renewed every eighth skip," now stated once in Requirements instead of three times.
  • No prerequisites.json and no SessionStart hook anywhere under plugins/bash-format/ — confirms the corrected 0.8.5 CHANGELOG entry (the plugin never shipped the session-start probe the old entry claimed).
  • SKILL.md step renumbering (1–11) is internally consistent: the Gotchas section's cross-references were updated from "probe 9" to "probe 10," and the item-8/10/11 back-references in the new gitignored step all point at the right steps.

No new correctness or maintainability issues found in this pass. This is a documentation/changelog-only change (no hooks, workflows, or executable code touched), so there's nothing further for this lane to flag.
· branch fix/audit-bash-format

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 57s —— View job


Security review complete

  • Gather context (PR body, changed files list)
  • Invoke /review:security-review and follow its instructions
  • Review diff origin/main...228c0ab for plugins/bash-format/
  • Post findings

No security issues found.

All 5 changed files are documentation/config; no executable code path changes:

  • README.md, skills/setup/SKILL.md — prose edits: adds bash_format_lint_gitignored coverage, a node requirement note (with a recheck trigger), renumbers check steps, softens apply to guidance-only (matches the skill's own "no write path" claim).
  • CHANGELOG.md — rewrites historical 0.7.60/0.7.61/0.8.2/0.8.5 entries to a shared one-line note and adds a 0.8.6 entry; corrects the 0.8.5 entry's inaccurate claim of a session-start probe / prerequisites.json this plugin never shipped. CHANGELOG.md is a historical record, not a protected instruction surface (CLAUDE.md/AGENTS.md/rules file/skill or agent body/hook allow-list) — nothing in the tree gates behavior on its wording.
  • skills/setup/evals/evals.json — eval expectations updated to match; no executable harness change.
  • .claude-plugin/plugin.json — version bump 0.8.5 → 0.8.6 only.

No hook scripts (hooks/*.sh, hooks/*.mjs), no GitHub Actions workflow files, and no credential/token handling are touched.

Instruction-surface-deletion check (SKILL.md is a skill body): the repeated "once per session and agent, renewed every eighth skip" notice text is consolidated from three bullet points into one line in the Requirements/check sections. This restates rather than removes a guardrail — verified against source that the underlying cadence is unchanged and still enforced in code:

  • hook-utils.sh:278 — HOOK_NOTICE_RENEW_EVERY="${HOOK_NOTICE_RENEW_EVERY:-8}" confirms "renewed every eighth skip" is still the enforced default, only restated once instead of three times.
  • hooks/hooks.json:12,23 — both rows launch via node ${CLAUDE_PLUGIN_ROOT}/hooks/exec-bash.mjs, backing the new Node.js requirement bullet.
  • hooks/bash-format.sh:46 + hooks/rewrite-guard.sh:210 — hook::gitignored_out_of_scope gates on CLAUDE_PLUGIN_OPTION_BASH_FORMAT_LINT_GITIGNORED, confirming the "silently skipped, no notice" claim in the README/SKILL.md/evals.

Not a finding.
· branch fix/audit-bash-format

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 36s —— View job


Claude finished the code review

  • Gather context (PR body, prior review comments, changed files)
  • Check skip-gate: does HEAD 228c0ab already have an applicable review from this lane?
  • Skip gate hit — no further review needed

Summary

HEAD 228c0ab is a merge of origin/main into this branch. Diffing 1a49f7a..228c0ab shows zero changes under plugins/bash-format/ — the merge only pulls in unrelated main-branch commits from other plugins. All 5 files this PR touches are unchanged since 1a49f7a.

This lane already reviewed 1a49f7a above and confirmed both outstanding findings from the earlier round were resolved at that commit: the Node.js upstream-drift claim gained a recheck trigger, and the CHANGELOG's apply wording was corrected to "gives guidance" (matching the skill's no-write-path behavior). That review found no further issues against the hook source (hooks.json, bash-format.sh, rewrite-guard.sh, hook-utils.sh).

Per the skip gate in /review:code-review ("This head already has a successful review from this lane that still applies"), there is nothing new to review at 228c0ab. No findings to post.
· branch fix/audit-bash-format

@kyle-sexton
kyle-sexton merged commit 2b7e44c into main Sep 29, 2026
19 checks passed
@kyle-sexton
kyle-sexton deleted the fix/audit-bash-format branch September 29, 2026 14:47
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