Skip to content

fix(biome-format): honor the kill switch on the SessionStart probe; bind notice to manifest - #5284

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

kyle-sexton merged 9 commits into
mainfrom
fix/audit-biome-format

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Contributor

Refs: #4240

Summary

Audit findings against plugins/biome-format for #4240 (missing-tool prerequisites). The SessionStart probe ignored the biome_format_enabled kill switch, the hook's notice text was not bound to prerequisites.json, and the README, setup skill and check skill disagreed on notice wording and left out Node and the probe. Coverage of the other five binary-probing plugins and the two owner questions on #4240 stay open, so this PR only refs the issue.

Fix

  • hooks/hooks.json: the SessionStart row launches with --run-if-unset-or-true BIOME_FORMAT_ENABLED (the form typos-format uses). probe-prerequisite.sh and exec-bash.mjs are untouched; the probe stays byte-identical across biome-format, go-format and markdown-format.
  • hooks/biome-format.test.sh: runs the row as the harness spawns it, from an empty cwd on a PATH without biome. Disabled prints nothing; unset and true print the notice. A missing node fails the suite. A new case binds the hook's missing-binary notice (tool name, check, install line) and the tool's local bin path to prerequisites.json.
  • README, setup and check skills, and their evals: the two latch classes are stated once, Node.js and the probe are documented, the setup check names the probe as a separate resolver, adds a Node row and reports biome_format_lint_gitignored, and the check skill runs every probe through Bash because setup's pre-computed rows are not rendered when it reads the file.
  • Release 0.7.6 with a CHANGELOG entry.
  • plugins/planning/tests/interview-defenses.test.sh: re-pins the interview SKILL.md frontmatter digest that feat(skills): argument-hint house style, gate, and fleet rewrite (#3542) #5042 (argument-hint only) left stale and that failed test-linux on main; planning 0.45.13. Same one-line change as fix(session-flow): audit remediation (observer flag gate, export records, unattended parsing) #5239.
  • CHANGELOG.md, in-place corrections to released entries (each also named in the 0.7.6 entry): 0.7.2 said this plugin's hook rows were unchanged although 0.7.1, the same commit, changed them; 0.6.59 and 0.6.58 described hook::shell_c_operand and hook::bash_parse_segments, which no hook here calls. All three now read "shared launcher/library sync; no change to this plugin's behavior" with their issue links kept. 0.7.3 stays: hook::begin calls hook::repo_relative_path_to. No entry is renumbered or folded.
  • docs/formatter-path-probes.md: the dispatch-completeness block is a pointer at No PostToolUse hook has completed on this machine since 2026-08-12 (formatters silently not running) #3549, and the "do not raise PostToolUse timeouts" guidance is bound to the recheck (the Windows claude --debug result on No PostToolUse hook has completed on this machine since 2026-08-12 (formatters silently not running) #3549) instead of standing as fleet guidance. The four-part record stays in this file.

Not done, owner-reserved: whether the probe may notify in repos with no Biome config (Q1) and whether to keep the per-plugin check skills (Q2). No option is implemented.

Verification

  • bash plugins/biome-format/hooks/biome-format.test.sh: PASS=62 FAIL=0
  • bash scripts/check-prerequisite-probes.test.sh: 18 cases, 0 failed
  • bash scripts/check-changelog-parity.sh --check --check-order: pass
  • bash scripts/validate-plugins.sh: all manifests and the catalog validated
  • check-cross-plugin-source-drift.sh, check-hook-exec-form.sh, check-killswitch-hoist.sh, check-skill-leaf-names.sh, check-purged-em-dashes.sh, check-silent-skips.sh: no findings for this plugin
  • Mutation checks (task runs): reverting the hooks.json flag fails the disabled case; changing the install string in the hook only fails the binding case.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB

kyle-sexton and others added 4 commits September 29, 2026 01:29
…site probe

The SessionStart row launched probe-prerequisite.sh with no option gate, so
a plugin disabled through biome_format_enabled=false still printed the
missing-biome notice. Launch the row with --run-if-unset-or-true
BIOME_FORMAT_ENABLED, the form typos-format uses, so a closed gate exits 0
before bash is resolved. probe-prerequisite.sh stays byte-identical across
biome-format, go-format and markdown-format.

The suite now runs the hooks.json row as the harness spawns it, from an empty
cwd on a PATH without biome: disabled prints nothing and exits 0; unset and
true print the notice naming biome, /biome-format:check and the install line.
A missing node fails the suite. Whether the probe may notify in a repo with no
Biome config is left to the owner (#4240).

Refs #4240

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

Assert the manifest has exactly one tool (biome, node_modules/.bin/biome) and
that the hook's notice call and local-bin walk state its name, check and
install strings, so the two cannot drift apart.

Refs #4240

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
…heck self-sufficient

README Requirements names the two latch classes once, adds Node.js (every hook row
launches through exec-bash.mjs, so a missing node is a launch error, not a skip
notice) and describes the SessionStart probe. The setup check drops its own latch
wording, names the probe as a separate resolver, adds a Node row and reports
biome_format_lint_gitignored. The check skill states that setup's pre-computed rows
are not rendered when it reads the file. Evals cover the new rows.

Refs #4240

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.

…atch guidance to its recheck

Reword the 0.7.2, 0.6.59 and 0.6.58 entries to a one-line shared launcher/library sync note, and name
each edit in the 0.7.6 entry. Shorten the dispatch-completeness block in
docs/formatter-path-probes.md to a pointer at #3549 and tie the timeout guidance to that issue's
recheck.

Refs #4240

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
@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:19:15.435474Z 309a768 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 51s —— View job


Reviewing PR #5284

  • Gather context
  • Understand the request (invoke /review:code-review for this PR)
  • Run /review:code-review skill
  • Post inline findings
  • Post summary

Summary

No high-signal findings — nothing to block on. This is a well-scoped, carefully verified fix.

What I checked against the diff (origin/main...HEAD):

  • Kill-switch fix (hooks.json): the SessionStart row now passes --run-if-unset-or-true BIOME_FORMAT_ENABLED before the probe script. I read exec-bash.mjs's parseLaunchArgs/optionGateOpen (plugins/biome-format/hooks/exec-bash.mjs#L63-L98) and confirmed the gate semantics match the test's expectations exactly: biome_format_enabled=false → gate closed → exit 0 before bash spawns; unset or true → gate open → probe runs. Also confirmed exec-bash.mjs is byte-identical to typos-format's copy, and probe-prerequisite.sh is byte-identical across biome-format/go-format/markdown-format, as claimed.
  • New test cases in biome-format.test.sh: the manifest-binding case (asserting the hook's missing-binary notice text and local-bin walk state the exact name/check/install/local_bin from prerequisites.json) and the probe-gate case (spawning the real SessionStart row via node with a controlled PATH/env) are correctly targeted — I traced the sed ranges used to extract the notice call and the biome_local_bin_here function body, and both correctly bound the multi-line call/function. The run_probe helper's env -u NAME / env NAME=value + pipefail combination correctly captures the spawned node process's real exit code.
  • CHANGELOG corrections: the reworded 0.7.2/0.6.59/0.6.58 entries are consistent with what those commits actually touched (0.7.1 rewrote the hook rows to exec form in the same change 0.7.2 claimed left rows "unchanged"; 0.6.59/0.6.58 described hook::shell_c_operand/hook::bash_parse_segments, which this plugin's hooks don't call).
  • Docs/README/skill wording: cross-checked the setup/check skill wording against the actual hook and probe resolution logic (repo-root -f walk vs. probe's 8-level -x walk) — accurate. Skill/command names (/biome-format:setup check, /biome-format:check) match the actual skill directories.

Not independently verified (sandbox denies bash/script execution in this lane): the author-claimed PASS=62 FAIL=0 test run, the check-changelog-parity.sh, validate-plugins.sh, and cross-plugin drift-check results, and the mutation-check claims. These are author-claimed, not confirmed by me.

Two owner-reserved questions (Q1/Q2 on #4240) are correctly left unimplemented, matching the PR's stated scope.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context (PR diff, changed files)
  • Review changed files for security-relevant issues
  • Check for instruction-surface deletions (CLAUDE.md/AGENTS.md/rules/skills)
  • Post findings

No security issues found.

This PR touches only test scripts, documentation, changelog wording, an eval fixture, a plugin manifest version bump, and a hooks.json argument addition (--run-if-unset-or-true BIOME_FORMAT_ENABLED) that gates the existing probe-prerequisite.sh SessionStart row behind the plugin's own kill switch. Specifics checked:

  • hooks/hooks.json (diff): adds two static string args to the existing SessionStart row. No new command construction, no shell interpolation, no attacker-controlled input. exec-bash.mjs and probe-prerequisite.sh are unchanged, so the launch/parsing path already reviewed for this plugin is unaffected.
  • hooks/biome-format.test.sh: new test-only logic uses jq, sed, grep -qF against a static, checked-in prerequisites.json (one hardcoded tool entry, no external input) and spawns the probe row in an isolated mktemp -d sandbox with a scrubbed PATH. No untrusted data reaches these shell constructs.
  • docs/formatter-path-probes.md: reworded diagnostic/triage prose. Checked against the instruction-surface-deletion lens — the standing guidance ("do not raise in-repo PostToolUse timeouts" until a cause is named) is preserved, just rebound to the #3549 recheck rather than removed or weakened.
  • CHANGELOG.md, README.md, SKILL.md, evals.json, plugin.json: documentation, version bump, and eval fixtures with no executable or trust-boundary surface.

No GitHub Actions files are touched, so zizmor's lane is not implicated here.

@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: 309a7687e9

ℹ️ 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/biome-format/skills/setup/SKILL.md Outdated
Comment thread plugins/biome-format/skills/setup/SKILL.md Outdated
Comment thread plugins/biome-format/skills/setup/SKILL.md Outdated
kyle-sexton and others added 2 commits September 29, 2026 09:49
…rview frontmatter digest

- setup check: Node.js stays FAIL when the toggle is off (the runner starts node
  before the gate runs) and a Bash PASS is scoped to Bash's PATH; the probe's walk
  limit and file tests are read from probe-prerequisite.sh instead of restated.
- planning: re-pin the interview SKILL.md frontmatter digest. #5042 shortened the
  argument-hint key only (the two defenses are body prose, digested separately), and
  the stale digest fails test-linux on main. Same one-line change as #5239.

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

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context (PR diff, changed files)
  • Review changed files for security-relevant issues
  • Check for instruction-surface deletions (CLAUDE.md/AGENTS.md/rules/skills)
  • Post findings

No security issues found.

Reviewed origin/main...HEAD (head 847702e), which carries the two review-response commits on top of what the earlier Claude/Codex reviews already covered at 309a768. All 11 changed files are test scripts, documentation, changelog wording, eval fixtures, and a manifest version bump:

  • hooks/hooks.json: adds two static string args (--run-if-unset-or-true, BIOME_FORMAT_ENABLED) to the existing SessionStart row. No new command construction, no shell interpolation, no attacker-controlled input. exec-bash.mjs and probe-prerequisite.sh are unchanged.
  • hooks/biome-format.test.sh: new test-only logic (jq/sed/grep -qF against a static, checked-in prerequisites.json; the probe-gate case spawns node in an isolated mktemp -d sandbox with a scrubbed PATH). No untrusted input reaches these shell constructs; nothing here differs in security posture from what was already reviewed at 309a768.
  • docs/formatter-path-probes.md: rewording that rebinds the "do not raise in-repo PostToolUse timeouts" guidance to the #3549 recheck. Checked under the instruction-surface-deletion lens — the standing guidance is preserved, not weakened or removed, and it isn't a protected-class guardrail (credential/access-control/etc.) to begin with.
  • skills/setup/SKILL.md, skills/check/SKILL.md: this commit's changes are documentation edits that address the two Codex P1/P2 suggestions from the prior round (Node probed via Bash's PATH with an explicit caveat that a PASS doesn't establish hook-launch PATH resolution, and Node FAIL preserved when the toggle is disabled) plus deferring the SessionStart resolver's exact walk to reading probe-prerequisite.sh at check time rather than restating it. No executable or trust-boundary surface.
  • CHANGELOG.md, README.md, plugin.json, evals/evals.json (both), plugins/planning/tests/interview-defenses.test.sh: prose, version bump, eval fixtures, and a pinned-hash update in an unrelated plugin's test to match this PR's setup/SKILL.md content change. No security surface.

No GitHub Actions files are touched, so zizmor's lane is not implicated.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 13s —— View job


Reviewing PR #5284

  • Gather context
  • Invoke /review:code-review for this PR
  • Post inline findings
  • Post summary

Summary

No high-signal findings on 847702e (origin/main...HEAD). This is a solid, carefully-scoped follow-up that addresses all three prior Codex comments correctly.

What I checked:

  • The three Codex findings from the previous commit (309a768) are all correctly fixed in plugins/biome-format/skills/setup/SKILL.md:
    • Probe 7 (Node.js) now states explicitly that Bash's PATH can diverge from the hook-launch environment (nvm-style), so a PASS there doesn't establish hook availability — matches the exact failure class documented in docs/formatter-path-probes.md.
    • Probe 7 stays FAIL even when the toggle is disabled, with the correct reasoning (the runner must start node before exec-bash.mjs can evaluate the option gate at all) — the disabled-downgrades-to-INFO rule in the preamble (lines 41-45) now explicitly carves this out.
    • Probe 3's SessionStart-resolver description no longer hardcodes the eight-level/-x/-f specifics; it now defers to reading probe-prerequisite.sh directly, closing the stale-copy risk the reviewer flagged.
  • Verified independently, not just read: extracted plugins/planning/skills/interview/SKILL.md's YAML frontmatter and hashed it myself — it matches the new pinned digest (881ecec0f...) in plugins/planning/tests/interview-defenses.test.sh. That file's frontmatter was last touched by e544012 (already on main, pulled in by this branch's merge commit), so the re-pin is a legitimate, correctly-computed consequence of the merge, not a scope-creep edit.
  • Cross-checked README/SKILL.md/evals for consistency: the "walk up to eight directories" wording in README.md matches probe-prerequisite.sh's have() loop (for ((i = 0; i < 8; i++))); the biome_format_lint_gitignored default (false) matches plugin.json; eval IDs in both evals.json files are unique (checked with jq).
  • CHANGELOG 0.7.6 entry and the three corrected historical entries (0.7.2, 0.6.59, 0.6.58) accurately describe what those commits touched, consistent with the prior review's finding.
  • docs/formatter-path-probes.md: the four-part verification record (Claim/Basis/As-of/Recheck) is intact and correctly rebound to the #3549 recheck.

Not independently re-verified (sandbox denies running the test suites in this lane): the author-claimed PASS=62 FAIL=0, check-prerequisite-probes.test.sh, check-changelog-parity.sh, and validate-plugins.sh results — these are author-claimed, not confirmed by me. Everything above was verified by reading the source files directly (and, for the digest, recomputing it by hand).

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context (PR diff, changed files)
  • Review changed files for security-relevant issues
  • Check for instruction-surface deletions (CLAUDE.md/AGENTS.md/rules/skills)
  • Post findings

No security issues found.

Reviewed origin/main...HEAD at head 50dcaf3. This is the same content the three prior Claude/Codex reviews on this PR covered at 309a768 and 847702e; the only file that changed since then is plugins/planning/tests/interview-defenses.test.sh (a re-pinned digest pulled in by the merge from origin/main), which I independently verified below. Findings:

  • plugins/biome-format/hooks/hooks.json: adds two static string args (--run-if-unset-or-true, BIOME_FORMAT_ENABLED) to the existing SessionStart row. No new command construction, no shell interpolation, no attacker-controlled input; exec-bash.mjs and probe-prerequisite.sh are unchanged.
  • plugins/biome-format/hooks/biome-format.test.sh: new test-only logic (jq/sed/grep -qF against the static, checked-in prerequisites.json; the probe-gate case spawns node in an isolated mktemp -d sandbox with a scrubbed PATH, and env values passed to run_probe are literal test constants, never external input). grep -qF (fixed-string, not regex) rules out pattern-injection from the manifest values. No trust-boundary surface.
  • docs/formatter-path-probes.md — checked under the instruction-surface-deletion lens: the "do not raise in-repo PostToolUse timeouts" guardrail is preserved (rebound to the #3549 recheck, not removed or weakened), and it isn't a protected-class guardrail (credential/access-control/injection-defense) to begin with, so no instruction-exception-register match.
  • plugins/planning/tests/interview-defenses.test.sh (diff): this file is a security-relevant tripwire — it pins a SHA-256 digest over plugins/planning/skills/interview/SKILL.md's YAML frontmatter specifically to catch an unauthorized/weakening edit to that always-loaded routing surface. Re-pinning a digest is exactly the kind of change that could mask a real weakening if done carelessly, so I independently recomputed it rather than trusting the PR's claim: I extracted the current frontmatter (SKILL.md lines 2-8) and hashed it myself, which reproduces the new pinned value 881ecec0f... exactly. git log on that file shows its last content change was e544012 (already on main, pulled into this branch only by the merge commit dd3c324), confirming the re-pin reflects a legitimate upstream change (an argument-hint edit unrelated to either pinned defense) and not scope creep introduced by this PR.
  • Manifest/eval JSON files, README/CHANGELOG wording, and version bumps: no executable or trust-boundary surface.

No GitHub Actions files are touched, so zizmor's lane is not implicated.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 24s —— View job


Reviewing PR #5284

  • Gather context
  • Invoke /review:code-review for this PR
  • Post inline findings
  • Post summary

Summary

No high-signal findings on 50dcaf3 (origin/main...HEAD, scoped via the merge base at aebb9bb, since the shallow checkout can't resolve git diff origin/main...HEAD directly — verified this scoping reproduces the PR's exact stated stats: 13 files, +220/-36).

This head only adds a dd3c324 main-sync merge and the 50dcaf3 planning release bump on top of what the prior Claude/Codex reviews already covered at 847702e; no new source changes landed since that pass. I independently re-verified the load-bearing claims rather than re-reading the prior reviews' conclusions:

  • Kill-switch gate (hooks.json#L128-L131, exec-bash.mjs#L84-L96): traced optionGateOpen myself — --run-if-unset-or-true BIOME_FORMAT_ENABLED closes the gate only when the option is explicitly set to something other than true/empty/unset, so biome_format_enabled=false exits before bash spawns, and unset/true runs the probe. Matches the new test's three cases exactly.
  • Manifest-binding test case (biome-format.test.sh#L81-L100): hand-traced the sed ranges against the hook source — hook::tool_missing_notice_to BIOME_NOTICE (lines 126-128 of biome-format.sh) and biome_local_bin_here() (lines 107-111) both correctly contain the manifest's name/check/install/local_bin fields from prerequisites.json. A drift in either would fail the case.
  • Frontmatter digest re-pin (interview-defenses.test.sh#L447): recomputed sha256sum over lines 2-8 of plugins/planning/skills/interview/SKILL.md myself — it equals the new pin (881ecec0f...) exactly. The change is a legitimate consequence of the already-on-main 0.45.11 argument-hint edit reaching this branch via the merge commit, not scope creep from this PR.
  • docs/formatter-path-probes.md: the four-part record (Claim/Basis/As-of/Recheck) is complete and internally consistent; the #3549 issue reference is used consistently with the PR body's description.
  • Setup/check SKILL.md changes correctly defer the SessionStart resolver's walk limit to reading probe-prerequisite.sh at check time (the Codex P1 fix), and keep Node.js as FAIL even when the toggle is disabled (the Codex P2 fix) — both match what kyle-sexton said was fixed in 847702eb8, verified by reading the current file rather than trusting the changelog note.

Not independently run (no execution/network tooling available in this lane): the author-claimed PASS=62 FAIL=0, check-prerequisite-probes.test.sh, check-changelog-parity.sh, and validate-plugins.sh results. Everything above was verified by reading source directly or recomputing (digest, gate logic) by hand.

@kyle-sexton
kyle-sexton merged commit 7b0159b into main Sep 29, 2026
27 checks passed
@kyle-sexton
kyle-sexton deleted the fix/audit-biome-format branch September 29, 2026 14:22
kyle-sexton added a commit that referenced this pull request Sep 29, 2026
…rtability lint

The planning digest repin is dropped from this branch: origin/main now carries
the same pin (#5284).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EugXnFddtpHcY5gTuyEirB
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