Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/recipe-evidence-check.sh:
- Around line 266-279: Update the protected/others partitioning in
recipe-evidence-check.sh so deleted evidence pointers still count as protected:
when building the protected set in the affected-recipes loop, check whether
recipes/evidence/${slug}.yaml exists in either the BASE commit or the current
HEAD working tree, not just HEAD. Use the existing protected/others jq
accumulation logic and the slug loop, but adjust the file-presence test so
pointer deletions remain in protected and continue to be verified.
- Around line 397-443: The verifier handling in recipe-evidence-check.sh is
treating exit 1 as a generic verify error, but the contract says valid bundles
with recorded phase failures should be classified as Exit 1 and invalid bundles
as Exit 2. Update the JSON parsing in the verification block to read the `.exit`
field from the verifier output, then branch on that value in the `case`/status
mapping so Exit 1 produces the phase-failure informational path and Exit 2
remains the invalid-bundle path; keep the existing `signed_digest`, `pending`,
and `failureCause` handling in the same flow and reference the `verify_json`,
`verify_exit`, and `verify_cell` logic when making the change.

📋 Review — multi-persona, meta-confirmed

Method: four persona passes (shell-correctness, security, CI/DX, domain-correctness), each finding independently confirmed/refuted by a senior meta-reviewer against the resolved code. Tier legend: 🔴 Blocker · 🟠 Major · 🟡 Minor · 🔵 Nitpick.

Overall — no blockers; two Majors to resolve or defer in writing

Clean, well-commented tuning of a warning-only, never-blocking gate, and the noise-reduction direction is right. The privilege split (unprivileged pull_request build + workflow_run comment with no PR checkout) is implemented correctly — the most important control here, and it's sound. Nothing in this PR can break CI or merge a bad artifact.

Two Major items keep it from a clean approve (both inline):

1. The component-scoped cascade walks a different inheritance graph than the one the digest is computed over → systematic false-negatives on registry-only edits to shared/base/wildcard components (the exact case the feature targets).
2. The classified-cause/pending rendering reads verifier JSON fields that don't exist yet → graceful dead code that the ADR nonetheless marks "(implemented)".

🟡 Minor (not anchorable inline — file not in this diff)

Stale "Known limitations" comment in .github/workflows/recipe-evidence.yaml L32–35 still says discovery "walks spec.base ancestors but not spec.mixins" and "promote-all only matches registry.yaml or base.yaml" — both made false by this PR. The script header was updated; the duplicated workflow copy was not.

✅ Confirmed non-issues (examined and cleared)

recipe_components() has its own base-chain cycle guard (seen[]); removing the [[ ! -f "$pointer" ]] branch is safe (non-pointer recipes route to the others partition; a vanished pointer fails-closed to :x: verify error); mktemp files are cleaned on every continue; comm -12 | grep -q is correct under pipefail; the new protected=/other_affected= stdout keys break no consumer; the truncation row was correctly updated to 3 columns; the lower MAX_ROWS header comment was updated; trigger-model split is correct.

Summary

Recommendation: not a blocker for a warning-only gate, but please either fix the cascade graph mismatch (it defeats the feature's core purpose for registry edits) or scope it down + document the gap, and reconcile the dead-code vs. "implemented" ADR before merge.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/recipe-evidence-check.sh:
- Around line 157-159: The failure handling in recipe_components currently
treats any aicr recipe timeout, resolver error, or jq parse failure as an empty
component list, which can cause Rule 5 to skip a recipe instead of
conservatively including it. Update recipe_components to fail open by surfacing
resolution/parsing failures in a way that Rule 5 can detect, and adjust the Rule
5 logic that consumes recipe_components so unresolved recipes are included
rather than silently skipped; use the existing recipe_components function and
the Rule 5 check in this script as the main touchpoints.

Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/recipe-evidence-check.sh:
- Line 280: The component-resolution warning in the recipe evidence check is
writing the PR-controlled overlay value directly into a GitHub workflow command.
Update the warning path in recipe-evidence-check.sh to pass the overlay through
the existing sanitizer before emitting the ::warning:: message, using the same
helper used elsewhere for workflow-command output, so the warning text cannot be
interpreted as a command.
- Around line 121-122: The component lookup in the recipe evidence script uses
env(CN), which can coerce scalar-like names and miss matches; update the yq
selections in the lookup logic to use strenv(CN) instead. Apply this change
consistently in the component-name matching expressions for both the base and
head registry queries in the script.

✅ Approve with notes

Re-reviewed the force-push (e3dad1f → 1894bfc) and verified each fix against the code. 8 of 10 findings fixed, both Majors addressed substantively — approving. The two Majors in particular:

• Cascade graph mismatch — fully fixed: recipe_components() now resolves via aicr recipe --format json (the same criteria-resolution engine the digest uses); jq path confirmed correct against RecipeResult (pkg/recipe/metadata.go:415). The base/wildcard false-negative is closed.
• Classified causes / pending — improved (now branches on the real VerifyResult.Exit rather than OS-exit heuristics, no regression), but the pending/failureCause fields still aren't emitted by the verifier, so that rendering stays inert until the #1437 Go half. See inline.

Also confirmed fixed: cosmetic-edit banner guard, double-counted warnings (verify_ok), Markdown escaping (md_escape), yq env(CN), OTHER_MAX cap, log_sanitize on ::warning:: echoes, ADR "matches" wording. Bonus: the de-protection handling (BASE-pointer present, deleted at HEAD → :warning: evidence pointer removed) is a good catch beyond the review.

Two non-blocking items remain (fine as follow-ups)

1. 🟠 ADR overstates classified causes as "(implemented)" while the verifier doesn't emit failureCause/pending yet — land the Go fields or soften the wording (inline).
2. 🟡 Stale comment in .github/workflows/recipe-evidence.yaml:32-33 still says discovery "walks spec.base ancestors but not spec.mixins" and "promote-all only matches registry.yaml" — both now false (resolution is via aicr; registry is component-scoped). The script's own header note was correctly removed; this duplicated copy was missed.

None of these block a warning-only gate. Thanks for the thorough turnaround.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/recipe-evidence-check.sh:
- Around line 72-77: PR-controlled text is still reaching workflow commands
unsanitized in the annotation paths, specifically where `overlay` and `pointer`
are embedded in `::warning::` output. Update the warning/error construction in
`recipe-evidence-check.sh` so every PR-derived value goes through `log_sanitize`
before being echoed, and extend `log_sanitize` to collapse carriage returns as
well as newlines while neutralizing `::`. Use the existing `log_sanitize`,
`digest_err`, and `verify_err` handling as the pattern for the remaining
annotation messages.

✅ Approve

Re-reviewed 1894bfc → fb91a2d. Script-only delta, and every change is hardening with no regressions (bash -n clean):

• log_sanitize now strips \r as well as \n — fully closes the ::warning:: injection nitpick.
• strenv(CN) instead of env(CN) — fixes a scalar-typing edge case beyond the original ask (inline).
• recipe_components() now fails open on resolver timeout/error (include + warn) instead of treating a hiccup as "references no changed component" — correct direction for a protection gate, and it neatly resolves the perf/robustness note from my last pass (inline).
• log_sanitize applied consistently to overlay/name/parent/pointer in the remaining ::warning:: echoes.

Approving. The two non-blocking follow-ups from the prior review remain open and are fine to land separately: the ADR's "Classified verify failures" still sits under "(implemented)" while the verifier doesn't emit failureCause/pending yet, and the stale comment in .github/workflows/recipe-evidence.yaml:32-33. Neither blocks a warning-only gate. Nice iteration.