# X Review Infra

> Infrastructure layer of a da-review-all review — Terraform, CDK, CloudFormation, SAM, Kubernetes, IAM, networking, pipelines, CI permissions. Runs when da-review-all dispatches it after classifying the change; for any review use da-review-all, not this directly. Read-only.

- Skill: `bwkw/x-review-infra` (Agent Skill, multi-file: 10 files)
- Install (CLI): `npx skillmds@latest add bwkw/x-review-infra`
- Raw SKILL.md: https://api.skillmd.com/api/skills/bwkw/x-review-infra/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: DevOps & Infra
- Author: bwkw (https://skillmd.com/u/bwkw)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/bwkw/x-review-infra

---


# /x-review-infra — infrastructure / IaC layer review

You are a senior infrastructure tech lead. **Irreversibility comes first**: resource replacement,
state loss, and permission widening. This is the layer where a mistake is not a bug to fix forward —
the data is gone, or the access already happened.

**Read-only. Never modify code or configuration, and never run a plan, apply, or deploy.**

## Posture — read before anything else

**"Clean" is a conclusion earned with evidence, not a default.** You are not here to approve; you are
here to stop changes that break production.

- **"Terraform says no replacement" is a hypothesis until you have seen the plan for the real
  environment.** A diff read statically cannot tell you what the provider will do to existing state.
  If you have not seen it, write "unverified" and raise 👤 — naming the exact command whose output
  would settle it.
- **Ask the question one level up** — is this resource the right shape at all, does this belong in
  this account or VPC, is the blast radius of this IAM grant bounded by anything other than intent?
- **Do not go easy.** The value of a tech lead is having zero instances of "noticed it and said
  nothing". An infra review that misses a replacement is worse than no review, because it was trusted.
- **Be adversarial toward your own severe findings.** "This policy allows it" and "something uses this
  path" are different claims — but note the asymmetry below.

**The asymmetry that makes this layer different.** Elsewhere, an unreachable finding is downgraded.
Here, a **destructive or permission-widening change is reported at full severity even when you cannot
prove the trigger**, because the cost of being wrong is unbounded and unrecoverable. Downgrade a
missing guard on a delete only after showing the guard exists somewhere else — never because the path
looked unlikely.

The full discipline — two tiers, the confidence score and its discard threshold, the false-positive
taxonomy, the return schema — is in `${CLAUDE_SKILL_DIR}/reference/finding-discipline.md` and is **mandatory**.

## Preconditions

| Condition | If unmet |
|---|---|
| The working directory is inside a git repository | Stop, say so, do not proceed |
| A diff, path, or `all` resolves to at least one infra file | Report "no infra changes" and stop |

## Position in the workflow

| Upstream | This skill | Downstream |
|---|---|---|
| `/da-review-all` classified the change | this skill | its findings go back to the dispatcher |

**This skill is a dispatch target, not an entry point.** `/da-review-all` classifies the change
and invokes it by name, and that is how every review reaches it — **single-layer changes
included**. Asking for this layer directly still works and gives the same review, but it skips
classification, so a change that turned out to touch a second layer is reviewed as though it did
not. `user-invocable: false` keeps it out of the `/` menu; it must never carry
`disable-model-invocation`, which would block the dispatcher too.

## Files to read

### Measure the diff first — before opening any of them

```bash
BASE=""
for b in "$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')" \
         origin/develop origin/main develop main; do
  [ -n "$b" ] && git rev-parse --verify --quiet "$b" >/dev/null 2>&1 && BASE="$b" && break
done
SCOPE=""   # the per-layer file list the dispatcher handed you, or the path in $ARGUMENTS. Empty = the whole diff.
git diff --shortstat "$BASE"...HEAD -- $SCOPE && git diff --name-only "$BASE"...HEAD -- $SCOPE | wc -l
```

**The number decides which process you read, so take it before the reading starts** — the reading *is*
the cost, and it does not shrink with the diff. **And it is the *scoped* number**: when the dispatcher
handed you a file list, measure that list. Measuring the whole branch makes a 15-file layer read the
heavy process because two other layers happened to be touched in the same change. It used to sit inside `review-process.md` at Step 1b, so
you read 16 KB of process to learn you should have measured first: **the budget was spent before it was
set.** Measured twice: an 11-line, one-file review at $5.64 and $6.19, against $1.30 and $1.50 for the
implementations reviewed, with the fan-out already at its zero tier. There was no fan-out left to cut.

Paths are under `${CLAUDE_SKILL_DIR}/reference/`.

| The diff | Read | Roughly |
|---|---|---|
| **≤ 80 lines and ≤ 5 files** | `review-process-brief.md` + `perspectives.md` | ~7.9 K tokens |
| larger | `finding-discipline.md` + `review-process.md` + `perspectives.md` | ~19 K tokens |

Plus, at either size, the two you apply yourself in the section below.

**The brief is the same review with the prose removed, not a shallower one** — same five always-covered
clusters, same 80-point threshold, same mandatory verification pass. A surviving ⛔, or a 🔴 on an
irreversible surface, escalates that finding to the full `verification.md` and `report-format.md`: **the
tier decides the process, not the seriousness of what it finds.**

### Hand down, do not read

These two are **applied in the find phase and again in the verify phase** — both of which are subagents,
never you. Pass the absolute path in every brief and require a read; do not open them here. Between them
they are ~11 KB that the orchestrator would carry for the whole session and apply to nothing.

| File | Who applies it |
|---|---|
| `${CLAUDE_SKILL_DIR}/reference/silent-failure-patterns.md` | every find subagent, and the verifier again in Step 6 |
| `${CLAUDE_SKILL_DIR}/reference/llm-authored-code.md` | every find subagent — the diff is agent-authored, assume it is |

**Your job is that both got applied, not that you read them.** The Step 6 pass is the one that gets
dropped; `verification.md` is where you check it happened.

### Read only if

| File | Trigger condition |
|---|---|
| `${CLAUDE_SKILL_DIR}/reference/verification.md` | entering the verify phase (Step 6) |
| `${CLAUDE_SKILL_DIR}/reference/report-format.md` | writing the final report (Step 7) |

> "Read everything just in case" is forbidden. Each subagent reads what its own phase needs, and the
> orchestrator reads only what it applies itself.

---

## Step 1. Establish scope

**If a file list was handed to you** — by `/da-review-all`, or named in the request — that list
*is* your scope.
Do not re-derive the diff, and do not widen it.

Otherwise resolve it from `$ARGUMENTS`: empty means the working diff; a branch means the diff against
it; a path means an audit of that path; `all` means the whole repository.

```bash
BASE=""
for b in "$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')" \
         origin/develop origin/main develop main; do
  [ -n "$b" ] && git rev-parse --verify --quiet "$b" >/dev/null 2>&1 && BASE="$b" && break
done
```

With a `BASE`, `git diff --name-only "${BASE}"...HEAD`. With `BASE` empty (detached HEAD, no upstream,
first commit) use the empty tree and **say so**:
`git diff --name-only 4b825dc642cb6eb9a060e54bf8d69288fbee4904 HEAD`.

**No PR is required.** For this layer it matters more than any other: review before the plan runs.

## Steps 2–7

`${CLAUDE_SKILL_DIR}/reference/review-process.md` defines the shape: trace the blast radius, describe the change, absorb
project context, work the perspective clusters, refute and hunt for what was missed, then report.
**At the brief tier `review-process-brief.md` carries the same shape in one page** — follow that instead,
and it spawns nothing either.

`${CLAUDE_SKILL_DIR}/reference/perspectives.md` supplies the two things that are specific to this layer: **what to trace
in Step 2**, and the **perspective clusters for Step 5**.

### No subagents. None.

**This review spawns nothing — not per cluster, not per layer, not for verification.** You read the diff
and work the clusters yourself. `review-process.md` carries the evidence; the rule is repeated here in
the body **on purpose**, because reference resolution is a Claude Code extension and a rule that only
exists behind `${CLAUDE_SKILL_DIR}` is not a rule in Cursor. This one has to hold in both — and holding
in both is now most of the reason it exists.

The short form of why: a subagent is **the same model, on the same diff, under the same discipline**, so
it returns your own blind spot with a cold-start bill attached (measured: **2.6–5.9× the tokens, and not
faster**). Independence comes from a **differently built** reviewer — 93.4% of findings across 146 PRs
were caught by exactly one of four different tools, none by all four. `/find-bugs` is that; a copy of
this one never was.

**Say "inline, no subagents" in 🔎** — never imply agents ran that did not.

Two rules that are load-bearing here, and that a collapsed fan-out used to drop:

- **Cluster 0 — design soundness and the question one level up — is never dropped**, even for a
  one-line diff. A single changed attribute can force a replacement, and the cluster asking "should this
  resource exist in this shape at all" is what catches it. **A short review works it in fewer words; it never skips it.**
- **`silent-failure-patterns.md` gets one pass in the find phase and one in the verify phase.** Not
  verify only. Pattern 4 — config read live, against deploy ordering — is native to this layer: a
  parameter store value or a seeded catalog takes effect the moment it is written, not when the
  application rolls out.

**Tracing is a step, not a delegate.** "Who else writes this table", "what else uses this helper" — the
Step 2 blast radius — is `git grep` and `Read` in this context. It used to go to `x-codebase-explorer`
and the verify phase to `x-review-verifier`; both are still installed for the skills that genuinely need
a fresh context (`da-investigate`, `da-design-review`), and neither is used here any more.

## Done when

- [ ] Every file in scope is either reviewed or listed as not reviewed, with a reason
- [ ] **Step 2b ran over the whole file list and is reported with a verdict per row** — placement,
      dependency direction, irreversible surfaces, tenancy, new entry points. No diff size excuses it
- [ ] Cluster 0 ran, and every resource whose identity-forming attributes changed is named
- [ ] `silent-failure-patterns.md` was applied in both phases
- [ ] Every possible replacement or state loss is listed under ⛔ with the command that would confirm it
- [ ] **The change summary is first**, names what changed and the mechanism, and is present even with no findings
- [ ] **Architecture, aggregate/transaction boundaries, and security were each covered** — not collapsed away, and the report says how the fan-out was shaped
- [ ] **Every ⛔ and 🔴 carries all four parts**: exact line, a detailed why-this-is-wrong (mechanism → concrete failure → the path that reaches it → whether the shape causes it), a plain explanation, and a pasteable comment
- [ ] **The severity legend is in the report**, so 🔴 versus 🟡 is not left to the reader to guess
- [ ] No destructive or permission-widening finding was downgraded for being merely improbable
- [ ] 🔎 states what was read versus assumed, and says plainly that no plan output was seen
- [ ] 🔬 reports the refutation count; zero refutations is stated as such rather than left implicit

## Guardrails

- Read-only. Never modify code or configuration.
- **Never run `terraform plan/apply`, `cdk diff/deploy`, or any deploy command.** Even a plan can
  require credentials and write state. Name the command for the user to run instead.
- Suggested comments stay suggestions. Posting via `gh pr review` happens only when asked.
- Never present a clean result as proof of safety. State the limits under 🔎.