ci(security-review): adopt always-report shape — paths input replaces workflow-level filter

[kyle-sexton]

What

Phase II steps 1–2 caller adoption (enforcement rail, #509 — not closed here; step 3 remains):

• .github/workflows/claude-security-review.yml: drop the workflow-level on.pull_request.paths filter; pass the same 26 security-sensitive patterns as the reusable workflow's new paths input; re-pin to the ci-workflows#189 merge SHA (99cb082). The workflow now triggers on every PR; the reusable workflow's changes job decides relevance and a not-applicable PR yields a name-stable SKIPPED security-review / security-review check a required-check ruleset reads as success.
• docs/adr/0002: ordering-correction addendum — ci-workflows-first, because a caller-side if: on the uses: job registers a DIFFERENT check name when skipped (community discussion 72708: child context stays "Expected" and wedges prose PRs), and caller-first against the old pin would run a full security pass on every prose PR in the interim. Ruleset flip stays LAST (unchanged constraint).
• docs/topics/ladder-climb-roadmap/PLAN.md: Phase II tagged DOING + dated scope-change note.

Phase II step 0 record (per plan sanity check)

• Consumer inventory: direct contents-API enumeration of all 11 active melodic-software repos + the proving-ground scratch repo (gh search code returned a false empty and was not trusted) — exactly one consumer of the reusable security workflow: this repo's caller. Full sweep archived in session records.
• Captured check context (live check runs 2026-07-21): security-review / security-review; caller job id and reusable job id unchanged by this PR, so the context is stable pre/post restructure.
• Upstream verification: workflow-level path skip leaves a required check Pending (wedge); job-level conditional skip reports Success — troubleshooting-required-status-checks. Inner-job skip name-stability confirmed empirically on this repo (run 29793157393: skip-actors skip reported security-review / security-review conclusion=skipped).

Verification on THIS PR

This PR touches .github/** and docs: expect security-review / changes to run and security-review / security-review to run a real review (relevant=true). The docs-only skip path gets verified by a follow-up test PR before the step-3 ruleset gate.

No linked issue (this PR closes nothing; #509 completes at step 3).

Related

• autonomy pipeline: no dedicated security-review pass — automated code review only #509 — enforcement ruling (Phase II of the ladder-climb roadmap)
• feat(claude-security-review): always-report shape — paths input + job-level relevance gate ci-workflows#189 — reusable workflow always-report reshape (merged, 99cb082)
• ADR 0002 addenda (autonomy pipeline: no dedicated security-review pass — automated code review only #509 ruling; ordering correction)
• https://github.com/orgs/community/discussions/72708

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[claude]

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

---

PR Review: ci(security-review) always-report shape adoption

Tasks

• Gather context and understand the PR
• Read changed files and diff
• Review workflow changes
• Review ADR addendum
• Review PLAN.md update
• Post findings

---

Summary

Clean, well-scoped Phase II step 2. No blocking findings. The core mechanics are correct and the documentation is thorough. Notes below.

---

.github/workflows/claude-security-review.yml

Pattern preservation — exact. All 26 patterns from the old on.pull_request.paths filter are present in the new paths input block, in the same order. No patterns were dropped or added. This is the highest-risk part of the migration (a missed pattern would silently shrink the security surface) and it's correct.

SHA pin. Updated from 0a5fd4c578f2a6278fdc0e586b8e08392e64299a (2026-07-20) to 99cb082ebb942b9da08e8345a851be5bf252ad79 (2026-07-21). The inline short-SHA comment matches the ci-workflows#189 merge SHA cited in the PR body. Correct.

cancel-in-progress: false — preserved and correct. Cancelling an in-flight security pass on a quick force-push would leave the check in a stale state; this setting prevents that.

Secret handling. One named secret (CLAUDE_CODE_OAUTH_TOKEN), no secrets: inherit. Least-privilege posture is maintained.

Header comment is accurate and precise — it explains the "no workflow-level path filter" decision and the wedge risk in plain terms. Appropriate for a future reader who might wonder why the filter was removed.

---

docs/adr/0002-default-on-ai-review-advisory-with-earned-promotion.md

The ordering correction addendum is well-reasoned. Two points:

1. Sequence consistency. The first addendum (autonomy pipeline: no dedicated security-review pass — automated code review only #509) stated the sequence as "caller restructure → workflow always-report shape → required check." The second addendum (this PR) explicitly corrects that to ci-workflows-first. Reading both addenda in document order is unambiguous. No change needed — the correction is the point.

2. Link. The trailing troubleshooting-required-status-checks link resolves to the GitHub docs page. The evidence for the skipped-uses:-job check-name behavior (community discussion 72708) is referenced by discussion number; it's not linked inline, but the PR body and plan note carry the evidence. Acceptable.

---

docs/topics/ladder-climb-roadmap/PLAN.md

Phase II tagged [DOING] with a dated scope-change note — correct.

Nit: The step list under Phase II still shows the original ordering (step 1 = caller, step 2 = ci-workflows), with the swap explained only in the prose block above. A reader skimming the numbered list won't immediately see that ci-workflows#189 is already done or that these steps ran in reverse of the list. Adding a (done — ci-workflows#189) note beside the ci-workflows step and a (this PR) note beside the caller step would let the list self-document its state without requiring a cross-read of the scope-change note. This is a suggestion only — the prose note is sufficient.

---

Verification gap (noted, not blocking)

The PR body explicitly defers the skip-path (docs-only PR → not-applicable verdict) test to a follow-up PR before the step-3 ruleset gate. That's the right call — locking step 3 on an observed skip is safer than assuming it works. The sanity check in PLAN.md (gh pr checks on a docs-only test PR showing Success with not-applicable) captures the acceptance bar.

---

Overall: Correct implementation, complete pattern migration, good documentation trail. The deferred skip-path verification is the remaining open item before step 3.

[claude]

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

---

Tasks

• Gather context and read changed files
• Verify pattern migration completeness
• Audit security posture and SHA pin
• Review ADR addendum
• Review PLAN.md update
• Post findings

---

PR Review: ci(security-review) always-report shape adoption

Verdict: approve with one observation and one nit. The core mechanics are correct, the security surface is preserved, and the documentation trail is solid.

---

.github/workflows/claude-security-review.yml

Pattern migration — verified 26 for 26. Every pattern from the old on.pull_request.paths block appears in the new paths input, in the same order:

#	Pattern
1–3	.github/**, .claude/**, scripts/**
4–9	plugins/*/hooks/** through plugins/*/commands/**
10–13	.claude-plugin/**, REVIEW.md, CLAUDE.md, AGENTS.md
14–15	plugins/*/src/**, plugins/*/dist/**
16–22	**/*.sh, **/*.ps1, **/*.mjs, **/*.ts, **/*.js, **/*.cjs, **/.mcp.json
23–26	**/mcp.json, **/package.json, **/package-lock.json, plugins/*/.claude-plugin/plugin.json

The format change from YAML-quoted list items (".github/**") to unquoted multiline block scalar (|) is correct — with: inputs accept newline-separated strings, not YAML sequence syntax, and the reusable workflow receives these as a single multiline string.

SHA pin — correct. 99cb082 matches ci-workflows#189's merge SHA. Short-SHA inline comment matches the full SHA. ✓

Security posture — unchanged and correct:

• secrets: inherit not used ✓
• Single named secret (CLAUDE_CODE_OAUTH_TOKEN) ✓
• Workflow-level permissions: contents: read as a least-privilege default; job-level permissions add only what the security-review job needs (pull-requests: write, id-token: write) ✓
• cancel-in-progress: false preserved ✓

Header comment accurately describes the deliberate absence of a workflow-level path filter and the wedge risk. Good for future maintainers who might otherwise "restore" the filter.

Observation — billing posture change (expected, justified, but worth noting explicitly):
The old workflow-level paths filter prevented GitHub Actions from allocating a run for non-security-relevant PRs entirely — zero minutes consumed. Now every PR allocates a workflow run; the filtering happens inside the reusable workflow. The trade-off (token overhead on every run vs. name-stable SKIPPED check enabling required-check enforcement) is correctly justified in the PR body and header comment. This is not a concern to block on — just flagging the change for the step-3 required-check cost model.

---

docs/adr/0002-default-on-ai-review-advisory-with-earned-promotion.md

The ordering correction addendum is logically tight. The key claim — that a skipped uses: job registers a different check name, leaving the child context "Expected" and wedging prose PRs — is cited to community discussion 72708 and corroborated by the GitHub troubleshooting link. The causal chain (ci-workflows-first → caller adoption → ruleset flip) follows directly.

Nit: ADR Decision §1 (line 29) still reads "a PATH FILTER over security-sensitive surfaces" — now a paths input evaluated inside the reusable workflow, not a caller-side filter. ADR immutability convention means the original decision text is appropriately left in place, and the addenda capture the evolution. A future reader reading Decision → Addendum 1 → Addendum 2 in order gets the full picture. No change needed, but awareness: if someone reads only the Decision section they'll see the old framing.

---

docs/topics/ladder-climb-roadmap/PLAN.md

[DOING] tag and the scope-change note are correct. The note captures the causal constraint ("caller-side if: on the uses: job registers a different check name") and cross-references the ADR addendum. The step-0 outcome (sole consumer = this repo's caller; captured context security-review / security-review) is recorded inline.

Nit (same as prior review): The numbered step list under Phase II still reflects the original ordering (caller first, ci-workflows second). The scope-change prose block above the list corrects this, but a reader skimming the list sees the old order. Adding a (done — ci-workflows#189) annotation to the ci-workflows step and a (this PR) to the caller step would make the list self-documenting without requiring a cross-read. Suggestion only — the prose block is sufficient.

---

Verification deferred (correct call)

The docs-only skip path (not-applicable verdict → name-stable security-review / security-review conclusion=skipped reads as Success under the required check) is deferred to a follow-up test PR before the step-3 ruleset gate. That's the right sequencing — empirical confirmation before flipping the ruleset.

---

Overall: Correct implementation. Pattern migration complete, security posture maintained, documentation trail sound. The billing observation is the only new item vs. the prior review; both nits are carry-forwards. Ready to merge once the skip-path verification PR passes.