feat(pr-reviewer): re-implement 7 safety checks against current architecture

[don-petry]

Background

PR #131 added 7 safety checks to the PR-review agent. It was closed unfixable
(28 merge-conflict sentinels) because its supporting script changes were
restructured out from under it by the dev-lead refactor on main. The check
semantics are still worth landing; reference PR #131 branch
claude/issue-43-20260511-2113, commit ed7d462.

The 7 checks: (1) CI-weakening detection, (2) prompt-injection scanning in
workflows, (3) large-PR gating, (4) PR-description quality scoring,
(5) critical-path tracing, (6) duplication search, (7) dependency-risk
assessment.

---

Revised implementation plan (determinism-first)

Correction to the original premise. The original issue said the checks are
"purely prompt-level additions to agents/pr-reviewer.md, no script changes
needed." That is inaccurate for the operational pipeline. agents/pr-reviewer.md
is only the manually-invoked Copilot profile. The automated cascade
that reviews every PR runs prompts/triage.md (Tier 1, no tools) →
prompts/deep-review.md (Tier 2, has search) → prompts/security-audit.md
(Tier 3), driven by scripts/review-one-pr.sh. To make these checks actually
fire on real PRs they must live in the cascade, not only the Copilot profile.

Design principle (required): deterministic wherever possible, LLM only where
unavoidable. Mechanical, pattern-matchable checks must NOT be left to the LLM
to "eyeball" — they must be computed in shell so they are reproducible and
unit-testable. The LLM is reserved for genuinely semantic judgment.

Determinism split

#	Check	Mechanism	Where
1	CI weakening	Deterministic — grep diff for skipped/deleted tests (skip, only, xit, it.skip, test.skip, .todo, @Ignore), if: false, continue-on-error: true, commented-out CI steps, and lowered numeric thresholds (compare -/+ lines in coverage/CI config). Hard-stop.	new scripts/lib/safety-checks.sh
2	Prompt injection	Deterministic — scan changed .github/workflows/*.yml for ${{ github.event.* }} used inside a run: step, LLM output interpolated into shell, over-broad token perms, pull_request_target + PR-head checkout without trust gate. Hard-stop.	scripts/lib/safety-checks.sh
3	Large-PR gate	Deterministic — numeric from PR_METADATA (changedFiles, additions, deletions) + regex for an implementation-plan/breakdown section in the body.	scripts/lib/safety-checks.sh
4	Description quality	Deterministic — regex presence of the 5 required sections (problem statement, risk category, test plan, rollback, monitoring). Count missing; ≥3 missing → escalate.	scripts/lib/safety-checks.sh
7	Dependency risk	Hybrid — deterministic parse of lockfile/manifest diff hunks (added deps, unpinned/^/~/latest ranges) in the lib; LLM narrative / CVE interpretation in deep-review.	lib + prompts/deep-review.md
5	Critical-path trace	LLM (semantic) — data-flow tracing, permission checks on all auth branches, boundary-condition coverage.	prompts/deep-review.md
6	Duplication search	Hybrid — Tier 2 has the search tool: deterministic candidate gathering, LLM adjudicates true duplication.	prompts/deep-review.md

Architecture (mirror the existing downstream-impact feature)

There is already a proven pattern for injecting a deterministic, pre-computed
block into the LLM cascade. Follow it exactly:

• scripts/lib/downstream-impact.sh → assemble_downstream_impact() writes a
  structured block; review-one-pr.sh inlines it into the triage prompt via
  downstream_impact_triage_section; gated behind DOWNSTREAM_IMPACT_ENABLED
  so that when the flag is off the triage prompt is byte-identical to
  pre-feature behavior. Tests: tests/test_downstream_impact_*.bats,
  tests/test_downstream_impact_triage_inline.bats.

Do the same for safety checks:

1. New scripts/lib/safety-checks.sh — pure functions that take PR_METADATA
   (JSON from gh pr view) and PR_DIFF and emit a structured SAFETY_CHECKS
   block: per-check findings with severity, file:line, and two hard-stop
   flags CI_WEAKENING_DETECTED / PROMPT_INJECTION_DETECTED. No network, no
   gh calls — deterministic and side-effect-free so it is unit-testable.
2. Wire into scripts/review-one-pr.sh — compute the block before the Tier 1
   prompt is assembled and inline it into TRIAGE_PROMPT_FILE (next to the
   DOWNSTREAM_IMPACT / ADVISORY_BOT_FEEDBACK sections). Also write it to a
   file (mirror ADVISORY_BOT_FEEDBACK_FILE) so Tier 2/3 can read it. Gate
   behind a SAFETY_CHECKS_ENABLED flag; default it ON (these are
   safety-critical), but keep the flag for rollback and holdout-eval stability
   (scripts/lib/holdout-guard.sh). When off, the prompt must be byte-identical.
3. prompts/triage.md — consume the SAFETY_CHECKS block: treat
   CI_WEAKENING_DETECTED and PROMPT_INJECTION_DETECTED as forced
   escalate:true and never-approve; large-PR gate and description-missing≥3
   → escalate. The prompt must NOT re-derive these mechanically (no tools in
   Tier 1) — it consumes the pre-computed verdicts.
4. prompts/deep-review.md — add the LLM-only checks: critical-path trace
   (5), duplication adjudication using the search tool (6), dependency-risk
   narrative (7). Reference the deterministic hard-stops as blocking.
5. agents/pr-reviewer.md — document all 7 checks in the human-facing
   Copilot profile so the manually-invoked agent stays consistent with the
   cascade (this satisfies the literal original ask; the operational teeth are
   in the lib + prompts).

Testing (required — the whole point of determinism)

• tests/test_safety_checks.bats — unit-test each deterministic check with
  fixture diffs/metadata: CI-weakening true/false (incl. threshold lowering),
  prompt-injection true/false, large-PR boundary (just under/over), description
  scoring (0/3/5 sections present), dependency parsing (pinned vs unpinned).
• Triage-inlining test mirroring tests/test_downstream_impact_triage_inline.bats:
  block present when enabled, and prompt byte-identical when the flag is off.
• Register new test files in the bats job list / scripts/run-bats.sh
  invocation so lint CI runs them. shellcheck must pass on the new lib.

Guardrails

• Do NOT modify thin caller-stub workflows (dev-lead.yml, pr-review.yml
  header contract, etc.) beyond what AGENTS.md allows. Changes are confined to
  scripts/lib/safety-checks.sh (new), scripts/review-one-pr.sh,
  prompts/triage.md, prompts/deep-review.md, agents/pr-reviewer.md, and
  tests/.
• Flag-off path must be byte-identical to current triage output (protects
  holdout evals).
• Keep the lib free of gh/network calls so it stays deterministic and testable.

Acceptance criteria

• scripts/lib/safety-checks.sh computes all deterministic checks with no network calls; shellcheck clean.
• SAFETY_CHECKS block inlined into Tier 1 prompt when SAFETY_CHECKS_ENABLED; byte-identical when off.
• prompts/triage.md forces escalate/never-approve on the two hard-stops and consumes (does not re-derive) the deterministic verdicts.
• prompts/deep-review.md covers critical-path tracing, duplication adjudication, dependency-risk narrative.
• agents/pr-reviewer.md documents all 7 checks for the manual Copilot profile.
• tests/test_safety_checks.bats + triage-inlining test added and wired into the bats CI job; all green.

🤖 Generated with Claude Code

[don-petry]

Dev-Lead Implementation Plan

Issue: #305 — feat(pr-reviewer): re-implement 7 safety checks against current architecture

Scope

Add the 7 PR-review safety checks determinism-first: a new pure scripts/lib/safety-checks.sh computes the mechanical checks (CI-weakening, prompt-injection, large-PR, description-quality, dependency parsing) as a SAFETY_CHECKS block inlined into the Tier-1 triage prompt (mirroring downstream-impact), with the semantic checks (critical-path, duplication, dependency-risk narrative) added to the deep-review prompt, all documented in the human-facing Copilot profile.

Implementation Checklist

• scripts/lib/safety-checks.sh (new) — pure functions over PR_METADATA (JSON) + PR_DIFF; emits a SAFETY_CHECKS block with per-check findings (severity, file:line) and two hard-stop flags CI_WEAKENING_DETECTED / PROMPT_INJECTION_DETECTED; no gh/network. Checks 1–4 + dependency parse (7). Plus a safety_checks_triage_section gated behind SAFETY_CHECKS_ENABLED (default ON) that inlines the block, byte-identical when off.
• scripts/review-one-pr.sh — compute the block before the triage prompt is assembled, inline via safety_checks_triage_section next to the DOWNSTREAM_IMPACT / ADVISORY_BOT_FEEDBACK sections, and write to SAFETY_CHECKS_FILE for Tier 2/3.
• prompts/triage.md — consume the pre-computed block: CI_WEAKENING_DETECTED/PROMPT_INJECTION_DETECTED force escalate:true + never-approve; large-PR gate and description-missing≥3 → escalate. Do NOT re-derive (no tools in Tier 1).
• prompts/deep-review.md — add LLM-only checks: critical-path trace (5), duplication adjudication via search (6), dependency-risk narrative (7); reference the deterministic hard-stops as blocking.
• agents/pr-reviewer.md — document all 7 checks for the manual Copilot profile.
• .github/workflows/lint.yml — register new test files in the bats job list.

Tests to Write

• tests/test_safety_checks.bats — unit-test each deterministic check with fixture diffs/metadata: CI-weakening true/false (incl. threshold lowering), prompt-injection true/false, large-PR boundary (just under/over), description scoring (0/3/5 sections), dependency parsing (pinned vs unpinned).
• tests/test_safety_checks_triage_inline.bats — block present when enabled; prompt byte-identical when the flag is off (mirrors test_downstream_impact_triage_inline.bats).

Test Command

bats tests/test_safety_checks.bats tests/test_safety_checks_triage_inline.bats (+ full scripts/run-bats.sh list); bash .dev-lead/scripts/dev-lead-lint.sh