feat(ci): silent-skip gate for hook prerequisite visibility

[kyle-sexton]

Summary

Deferred CI-gate backlog item from the prerequisite-visibility wave: a grep-level tripwire that fails CI when a hook entry script quietly no-ops on a missing CLI prerequisite.

• scripts/check-silent-skips.sh scans plugins/*/hooks/*.sh (entry scripts only — hook-utils.sh lib copies have their own sync gate; *.test.sh excluded) for two shapes: the same-line command -v X … || exit 0 / || return 0 / || <skip-named helper> guard, and the if ! command -v X block that reaches exit/return 0. A site passes when the skip is visible (hook::emit_skip_notice, hook::emit_system_message, hook::notice_once, hook::require_jq, or a stderr write) or carries a documented # silent-skip-ok: <reason> quiet classification at the site.
• scripts/check-silent-skips.test.sh: 14 fixture-based unit tests (annotation scoping, nested-fi depth tracking, non-skip guards never flagged, exclusions).
• New silent-skip-gate ci.yml lane, added to the ci-status needs graph.

The gate's first catch

guardrails/flag-commit-pr-skill-bypass.sh had a genuinely silent jq guard (if ! command -v jq … exit 0 with no notice) while every sibling guardrails hook emits a one-line stderr notice — a prerequisite-visibility wave miss. Fixed to match the siblings (guardrails 0.7.1).

Sanctioned quiet sites, now recorded at the site

• claude-ops 0.13.1 — hook-telemetry-sink.sh: fire-and-forget sink; the producer discards stdout+stderr, so no notice channel exists (producer owns visibility).
• powershell-format 0.4.1 — absent pwsh is the documented not-applicable quiet classification; telemetry still records the skip.

Comment-only annotation changes for those two; behavior unchanged.

Related

Part of the epic's deferred CI-gate backlog: #313 (epic stays open — remaining backlog items land separately).

No linked issue: this PR closes nothing; it is tracked under epic #313.

🤖 Generated with Claude Code

[claude]

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

---

PR Review: feat(ci): silent-skip gate for hook prerequisite visibility

• Gather context and understand the PR
• Review scripts/check-silent-skips.sh (core gate logic)
• Review scripts/check-silent-skips.test.sh (unit tests)
• Review .github/workflows/ci.yml changes
• Review plugin hook changes (guardrails, claude-ops, powershell-format)
• Review plugin.json and CHANGELOG.md bumps
• Synthesize findings and post review

---

Verdict: Approve with minor suggestions. The implementation is correct, the test suite is solid, and the CI integration is clean. Two minor gaps are worth noting for future-proofing but are not blocking.

---

Core gate logic (scripts/check-silent-skips.sh)

The awk implementation correctly handles both shapes and all the annotation scoping rules. A few specifics worth confirming:

pending_annot semantics are correct. The variable is only set to 1 (never reset within a comment block), which achieves "annotation anywhere in the contiguous comment block" correctly. A non-comment line is the only reset trigger. Traced through all the test scenarios — the logic holds.

annotated_above capture timing is right. annotated_above = pending_annot is captured before the comment/non-comment branch runs, so the guard line itself sees the pre-reset value. This is the subtle correct-ordering that makes same-guard-line and above-the-guard annotation both work. ✓

(el)?if pattern and depth tracking. The block entry pattern ^[[:space:]]*(el)?if[[:space:]]+!.*command -v correctly catches elif ! command -v at top level. Inside a block, depth only increments on ^[[:space:]]*if[[:space:]] (not elif), which is correct — elif doesn't require its own fi. One acknowledged limitation: if an elif ! command -v branch appears mid-if/elif/fi, the tracker starts a new synthetic block and the outer fi is consumed by it, which could leave the outer block "unterminated" from the tracker's perspective. Not an issue in the current corpus, and the grep-level scope is documented.

Potential false-positive concern ruled out. 2>&1 does not match />&2/ — those are different redirect forms. >&2 (stdout to stderr) only matches actual >&2 stderr writes. ✓

The emit_skipped case in powershell-format.sh is correctly flagged. Because awk ~ does substring matching, emit_skipped matches [A-Za-z_][A-Za-z0-9_:]*[Ss]kip via the emit_skip substring — confirming the annotation on that file is load-bearing for CI, not just documentation. ✓

Cross-check against the full corpus. Reviewed all plugins/*/hooks/*.sh entry scripts (excluding hook-utils.sh and *.test.sh). All if command -v X positive-form guards and || return 1 / || continue guards are correctly left unflagged. Only powershell-format.sh's || emit_skipped and the now-fixed flag-commit-pr-skill-bypass.sh block were flaggable, and both are handled. ✓

One minor gap: no test for || return 0 variant. The gate handles it at check-silent-skips.sh:93, but all same-line fixtures use || exit 0 or || <helper>. A future regex change could silently drop return 0 coverage. Consider adding →

One minor gap: no regression test for positive-form guards. The PR description correctly notes that if command -v X; then ... else exit 0; fi is intentionally not flagged (the current corpus uses the else-branch for its visible notice). There's no fixture preventing a future regression where this form starts being flagged. Low risk since the awk block entry pattern requires ! before command -v. Worth a one-liner fixture if the test file is ever touched again.

---

Hook fixes

guardrails/flag-commit-pr-skill-bypass.sh fix (lines 57–60) is correct and consistent with the sibling hooks' one-line stderr pattern. The notice accurately describes the degraded state ("advisory disabled — install jq to enable") without blocking. Matches the surrounding doc comment ("Fail OPEN when it is absent, but make the degraded state visible"). ✓

claude-ops/hook-telemetry-sink.sh annotation (lines 30–32) — the rationale is sound and well-stated: the sink runs in a backgrounded subshell with stdout+stderr → /dev/null, so there is genuinely no notice channel available to the sink itself. The producer side is the correct place for prerequisite visibility. Comment-only; behavior unchanged. ✓

powershell-format/hooks/powershell-format.sh annotation (lines 155–157) — the not-applicable classification is correct: pwsh absence is a platform N/A (contributor box or cloud session without PowerShell), not a missing optional tool. Telemetry still fires via emit_skipped. Comment-only; behavior unchanged. ✓

---

CI integration (.github/workflows/ci.yml)

The silent-skip-gate job (lines 234–245) is correctly wired:

• Pinned checkout SHA matches the other script-only jobs
• No fetch-depth: 0 (correct — no git history needed here)
• persist-credentials: false — consistent
• timeout-minutes: 15 — consistent
• Added to ci-status needs graph (line 366) — the fail-closed !cancelled() aggregation correctly extends to the new lane automatically

Step order (gate first, then tests) is the right call — a real corpus failure surfaces before test infrastructure questions.

---

Version bumps

All three plugin bumps are patch level:

• guardrails 0.7.0 → 0.7.1: Correct — a behavior fix (jq-absent skip now visible). Patch. ✓
• claude-ops 0.13.0 → 0.13.1: Correct — comment-only annotation, but a compliance fix for the new CI gate warrants a bump so consumers updating off the pin see the annotation in their copy. ✓
• powershell-format 0.4.0 → 0.4.1: Same reasoning. ✓

CHANGELOG entries are clear and accurate.

---

Summary

Two low-severity test gaps to consider (no fixture for || return 0, no fixture for the positive-form guard non-flagging invariant) — neither affects current correctness. Everything else is well-designed and well-tested. The gate closes a real visibility gap that the prerequisite-visibility wave missed, and the sanctioned quiet sites are correctly classified with documented reasoning at the site.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c25ea88955

ℹ️ 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 2s —— View job

---

PR Review: feat(ci): silent-skip gate for hook prerequisite visibility

• Gather context (PR updated with permission-fix commit 471f556)
• Verify the executable permission fix
• Review CI yml changes
• Review scripts/check-silent-skips.sh and check-silent-skips.test.sh
• Review plugin hook changes (guardrails, claude-ops, powershell-format)
• Synthesize findings and post review

---

Verdict: Approve. The Codex P1 blocker is correctly resolved, the gate logic and test suite are sound, and the plugin fixes are accurate. The two low-severity test gaps noted in the earlier review remain open but are non-blocking.

---

Permission fix (471f556) — Codex P1 resolved ✓

Both scripts are now 100755 in the git index (confirmed via git ls-files -s). The CI step run: scripts/check-silent-skips.sh will execute without Permission denied on the Ubuntu runner. The sibling test step already used bash scripts/check-silent-skips.test.sh so it was safe before; the main gate step now matches.

---

CI yml (.github/workflows/ci.yml)

The silent-skip-gate job is correctly structured:

• Pinned checkout SHA (9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0) matches sibling jobs
• No fetch-depth: 0 — correct, no git history needed
• persist-credentials: false — consistent
• timeout-minutes: 15 — consistent
• Step order correct: gate first (real corpus check), then unit tests
• Added to ci-status needs graph (line 366) — the !cancelled() fail-closed aggregation automatically extends to the new lane ✓

---

Gate logic (scripts/check-silent-skips.sh)

The awk implementation handles both shapes correctly:

pending_annot semantics — set to 1 on any annotated comment line, reset to 0 on the first non-comment line. annotated_above = pending_annot is captured before the reset branch runs, so the guard line itself sees the pre-reset value. Annotation-above and same-line-annotation both work correctly. ✓

Block entry pattern — ^[[:space:]]*(el)?if[[:space:]]+!.*command -v correctly catches if and elif. Inside a block, only ^[[:space:]]*if[[:space:]] increments depth (not elif), which is correct — elif doesn't open a new fi-terminated scope. ✓

is_visible pattern — >&2 matches only actual stderr redirects; 2>&1 (stderr→stdout) doesn't match, so no false-passes from that direction. ✓

END block — correctly handles a file ending inside an unterminated guard block. ✓

emit_skipped case — awk ~ does substring matching, so emit_skipped matches /[A-Za-z_][A-Za-z0-9_:]*[Ss]kip/ via the emit_skip substring. The annotation on powershell-format.sh is therefore load-bearing for CI, not just documentation. ✓

Still-open minor gap (non-blocking): No test exercises the || return 0 variant of the same-line guard (the gate handles it at line 93, but all same-line fixtures use || exit 0 or || <helper>). A future regex change could silently drop return 0 coverage. Consider adding →

---

Test suite (scripts/check-silent-skips.test.sh) — 14 fixtures ✓

All 14 tests are correctly structured and cover the key invariants: both shapes flagged, all annotation scopes (above, multi-line block above, same-line, in-block), annotation scoping leak prevention, nested fi depth tracking, non-skip guard forms never flagged, and exclusion rules for hook-utils.sh / *.test.sh. The fixture approach (copy the real script into a temp dir so cd "$(dirname "$0")/.." resolves correctly) is clean and portable.

Still-open minor gap (non-blocking): No fixture asserts the positive-form if command -v X; then ... else exit 0 pattern is not flagged. Low risk since the block entry pattern requires ! before command -v, but a one-liner guard would prevent future regression.

---

Plugin hook changes ✓

guardrails/flag-commit-pr-skill-bypass.sh (lines 57–60) — stderr notice correctly describes the degraded state without blocking. Consistent with all sibling guardrails hooks. Patch version bump (0.7.0 → 0.7.1) is appropriate for a behavior fix. ✓

claude-ops/hook-telemetry-sink.sh (lines 30–32) — annotation rationale is sound: the sink runs in a backgrounded subshell with stdout+stderr → /dev/null; no notice channel is available to the sink. Comment-only, behavior unchanged. Patch bump (0.13.0 → 0.13.1) appropriate so consumers see the annotation in their copy. ✓

powershell-format/hooks/powershell-format.sh (lines 155–157) — not-applicable classification is correct: pwsh absence is a platform N/A, not a missing optional tool. Telemetry still fires via emit_skipped. Comment-only, behavior unchanged. Patch bump (0.4.0 → 0.4.1) appropriate. ✓

---

CLAUDE.md alignment ✓

• Repo-agnostic: gate script uses plugins/*/hooks/*.sh glob relative to project root, no hardcoded paths
• Plugin-form-safe: no ../ reach-outs in gate or hook code
• Versioned: all three affected plugins carry explicit semver bumps
• CHANGELOG entries are accurate and clear

---

Summary

The Codex P1 (non-executable scripts → exit 126) is fully resolved by 471f556. Two minor test coverage gaps from the prior review remain open but do not affect current correctness. The gate closes a real visibility gap, all sanctioned quiet sites are correctly classified with documented reasoning at the site, and the CI integration is clean.