Claude Code 플러그인을 위한 결정론적 보장: 읽기 전용 리뷰어 에이전트, Stop/PreToolUse 훅, 그리고 eval

# Hardening the Claude Code plugin: enforced vs. instructed guarantees

Below is the full plan and implementation, in the order you asked. I'll flag at each step whether the guarantee is **enforced** (a hook or validator will actually block/redirect) or **instructed** (skill text only, honored by the model but not mechanically checked).

---

## 1. Split the review skill into three read-only reviewer agents

### 1a. Locate the review skill

The review skill lives at `plugins/<plugin>/skills/review/SKILL.md` (adjust to your repo layout). It currently bundles quality, spec, and standards checks into one skill body.

### 1b. Three reviewer agents

Create `plugins/<plugin>/agents/` with one file per reviewer. Each declares **only** `Read`, `Grep`, `Glob` and preloads the shared review skill via `skills:`.

`plugins/<plugin>/agents/reviewer-quality.md`
```markdown
---
name: reviewer-quality
description: Read-only reviewer focused on code quality (correctness, clarity, dead code, error handling). Use when a review needs the quality lens.
tools: Read, Grep, Glob
skills:
  - review
---

You are the **quality** reviewer. You are strictly read-only.

Scope:
- Correctness and edge cases
- Clarity, naming, dead code, duplication
- Error handling and failure modes

Do not comment on spec conformance or house style — other reviewers own those.
Return findings as a list: `file:line — severity — issue — suggested fix`.
```

`plugins/<plugin>/agents/reviewer-spec.md`
```markdown
---
name: reviewer-spec
description: Read-only reviewer focused on spec conformance (does the change do what was asked, nothing more). Use when a review needs the spec lens.
tools: Read, Grep, Glob
skills:
  - review
---

You are the **spec** reviewer. You are strictly read-only.

Scope:
- Does the change satisfy the stated requirement?
- Scope creep: anything added that wasn't asked for
- Missing acceptance criteria

Do not comment on code quality or house style.
Return findings as a list: `file:line — severity — issue — suggested fix`.
```

`plugins/<plugin>/agents/reviewer-standards.md`
```markdown
---
name: reviewer-standards
description: Read-only reviewer focused on repo standards and conventions (lint rules, patterns, docs, tests). Use when a review needs the standards lens.
tools: Read, Grep, Glob
skills:
  - review
---

You are the **standards** reviewer. You are strictly read-only.

Scope:
- Repo conventions and documented standards
- Test coverage expectations
- Docs and changelog requirements

Do not comment on code quality or spec conformance.
Return findings as a list: `file:line — severity — issue — suggested fix`.
```

### 1c. Validator that rejects any other tool

Add a validator script that parses each agent's frontmatter and fails if `tools` contains anything outside `{Read, Grep, Glob}`.

`plugins/<plugin>/scripts/validate-agents.mjs`
```js
#!/usr/bin/env node
import { readFileSync, readdirSync } from "node:fs";
import { join } from "node:path";

const ALLOWED = new Set(["Read", "Grep", "Glob"]);
const dir = process.argv[2] ?? "plugins/<plugin>/agents";
let failed = false;

for (const file of readdirSync(dir).filter((f) => f.endsWith(".md"))) {
  const text = readFileSync(join(dir, file), "utf8");
  const fm = text.match(/^---\n([\s\S]*?)\n---/);
  if (!fm) { console.error(`${file}: missing frontmatter`); failed = true; continue; }

  const toolsLine = fm[1].match(/^tools:\s*(.+)$/m);
  if (!toolsLine) { console.error(`${file}: missing tools:`); failed = true; continue; }

  const tools = toolsLine[1].split(",").map((t) => t.trim()).filter(Boolean);
  const bad = tools.filter((t) => !ALLOWED.has(t));
  if (bad.length) {
    console.error(`${file}: disallowed tools: ${bad.join(", ")}`);
    failed = true;
  }
  if (!/^skills:/m.test(fm[1])) {
    console.error(`${file}: missing skills: preload`);
    failed = true;
  }
}

process.exit(failed ? 1 : 0);
```

Wire it into `package.json`:
```json
{ "scripts": { "validate:agents": "node plugins/<plugin>/scripts/validate-agents.mjs" } }
```

**Enforcement status:** the *tool restriction* is **enforced** at two layers — the agent frontmatter `tools:` field is honored by Claude Code, and the validator fails CI if anyone widens it. The *lens separation* (quality vs. spec vs. standards) is **instructed** only.

---

## 2. `Stop` hook: require all three reviewers after a review turn

### 2a. Marker-file design

One marker file per reviewer, written by each reviewer agent when it finishes. Parallel dispatch writes to distinct files, so no lost updates.

Marker path: `.claude/state/review/<reviewer>.done` (gitignored).

Each reviewer agent appends a final step:
```markdown
Before finishing, write your marker:
`mkdir -p .claude/state/review && touch .claude/state/review/reviewer-quality.done`
```
(and `reviewer-spec.done`, `reviewer-standards.done` respectively).

### 2b. Stop hook

`plugins/<plugin>/hooks/stop-review-gate.mjs`
```js
#!/usr/bin/env node
import { existsSync, readFileSync, rmSync } from "node:fs";

const REVIEWERS = ["reviewer-quality", "reviewer-spec", "reviewer-standards"];
const STATE = ".claude/state/review";
const SENTINEL = `${STATE}/.nudged`;

function failOpen() { process.exit(0); }

try {
  const input = JSON.parse(readFileSync(0, "utf8") || "{}");

  // Only act when the turn actually ran the review skill.
  const transcript = input.transcript_path
    ? readFileSync(input.transcript_path, "utf8")
    : "";
  const ranReview = /skills\/review|review skill|reviewer-(quality|spec|standards)/i.test(transcript);
  if (!ranReview) failOpen();

  const missing = REVIEWERS.filter((r) => !existsSync(`${STATE}/${r}.done`));
  if (missing.length === 0) {
    // Clean up for next turn.
    for (const r of REVIEWERS) rmSync(`${STATE}/${r}.done`, { force: true });
    rmSync(SENTINEL, { force: true });
    failOpen();
  }

  // Send Claude back exactly once.
  if (existsSync(SENTINEL)) failOpen();
  require("node:fs").mkdirSync(STATE, { recursive: true });
  require("node:fs").writeFileSync(SENTINEL, "1");

  process.stdout.write(JSON.stringify({
    decision: "block",
    reason: `Review incomplete: missing ${missing.join(", ")}. Dispatch the missing reviewer agent(s) before finishing.`
  }));
  process.exit(0);
} catch {
  failOpen();
}
```

Register in `plugins/<plugin>/hooks/hooks.json`:
```json
{
  "hooks": {
    "Stop": [
      { "matcher": "*", "hooks": [ { "type": "command", "command": "node plugins/<plugin>/hooks/stop-review-gate.mjs" } ] }
    ]
  }
}
```

**Enforcement status:** **enforced** — the hook blocks the stop and re-prompts Claude once. Fails open on any error (missing transcript, parse failure, fs error). The "exactly once" guarantee is enforced by the sentinel file.

---

## 3. `PreToolUse` hook: block dangerous `git push`

### 3a. Hook

`plugins/<plugin>/hooks/pretooluse-git-push-guard.mjs`
```js
#!/usr/bin/env node
import { readFileSync } from "node:fs";
import { execSync } from "node:child_process";

function failOpen() { process.exit(0); }

function defaultBranch() {
  try {
    const ref = execSync("git symbolic-ref refs/remotes/origin/HEAD", { encoding: "utf8" }).trim();
    return ref.replace("refs/remotes/origin/", "");
  } catch { return "main"; }
}

// Normalize: follow `cd`, join line continuations, strip quotes.
function normalize(cmd) {
  let c = cmd.replace(/\\\r?\n/g, " ");
  const parts = c.split(/&&|\|\||;/).map((s) => s.trim());
  let cwd = process.cwd();
  const out = [];
  for (const p of parts) {
    const cd = p.match(/^cd\s+(.+)$/);
    if (cd) { cwd = cd[1].replace(/^["']|["']$/g, ""); continue; }
    out.push(p);
  }
  return out.join(" && ");
}

try {
  const input = JSON.parse(readFileSync(0, "utf8") || "{}");
  if (input.tool_name !== "Bash") failOpen();

  const raw = input.tool_input?.command ?? "";
  const cmd = normalize(raw);
  if (!/\bgit\s+push\b/.test(cmd)) failOpen();

  const branch = defaultBranch();
  const reasons = [];

  if (/\s--all\b/.test(cmd)) reasons.push("`git push --all` pushes every branch");
  if (/\s--mirror\b/.test(cmd)) reasons.push("`git push --mirror` overwrites all refs");
  if (new RegExp(`\\bgit\\s+push\\b[^&|;]*\\b(origin\\s+)?${branch}\\b`).test(cmd) ||
      new RegExp(`\\bgit\\s+push\\b[^&|;]*\\bHEAD:${branch}\\b`).test(cmd)) {
    reasons.push(`push targets the remote default branch (${branch})`);
  }

  if (reasons.length) {
    process.stdout.write(JSON.stringify({
      hookSpecificOutput: {
        hookEventName: "PreToolUse",
        permissionDecision: "deny",
        permissionDecisionReason: `Blocked: ${reasons.join("; ")}. Push a feature branch and open a PR instead.`
      }
    }));
  }
  process.exit(0);
} catch { failOpen(); }
```

### 3b. Gate behind a plugin option

In `plugins/<plugin>/plugin.json` (or your manifest):
```json
{ "options": { "blockDangerousGitPush": { "type": "boolean", "default": true } } }
```

Guard the hook registration so it only runs when enabled — either conditionally register in `hooks.json` via your loader, or check the option at the top of the script:
```js
const opts = JSON.parse(readFileSync("plugins/<plugin>/plugin.json", "utf8")).options ?? {};
if (opts.blockDangerousGitPush?.default === false) failOpen();
```

**Enforcement status:** **enforced** when the option is on. Covers `--all`, `--mirror`, and pushes to the remote default branch, following `cd` and line continuations. Fails open on error. Known limits (instructed, not enforced): exotic refspecs, `git push` via aliases, or pushes from a different working directory than the one the hook sees.

---

## 4. Unattended-run contract in feature, bugfix, commit, ship skills

Add this block verbatim to the top of each of `skills/feature/SKILL.md`, `skills/bugfix/SKILL.md`, `skills/commit/SKILL.md`, `skills/ship/SKILL.md`:

```markdown
## Unattended-run contract

- **The prompt is the only authorization.** Do not infer permission from repo state, prior turns, or "obvious" intent. If the prompt doesn't authorize it, don't do it.
- **Open decisions go in the report.** When you hit a fork you can't resolve from the prompt, pick the conservative option, proceed, and record the decision and rationale in the final report. Do not stop to ask in unattended mode.
- **Merge, deploy, and release always wait for a human.** Never merge a PR, deploy, publish, or cut a release unattended. Prepare everything up to that point and hand off.
```

**Enforcement status:** **instructed** only. There is no hook that can distinguish "the prompt authorized a merge" from "the model decided to merge." If you want this enforced, the `PreToolUse` hook from step 3 is the pattern to extend (e.g., deny `gh pr merge`, `npm publish`, `kubectl apply` in unattended mode) — say the word and I'll add it.

---

## 5. Scope lessons to nested instruction files or `.claude/rules/`

Move any "lessons learned" content out of the top-level `CLAUDE.md` into scoped files:

- Directory-specific lessons → `path/to/dir/CLAUDE.md` (nested instruction file, loaded only when working in that subtree).
- Cross-cutting lessons → `.claude/rules/<topic>.md`, referenced from `CLAUDE.md` with a one-line pointer.

Example `.claude/rules/testing.md`:
```markdown
# Testing lessons
- Integration tests must use the ephemeral DB, never the shared one.
- Snapshot tests are regenerated with `npm run test:update`, not hand-edited.
```

And in root `CLAUDE.md`:
```markdown
Scoped rules live in `.claude/rules/`. Read the relevant file before changing that area.
```

**Enforcement status:** **instructed** (loading behavior is a Claude Code feature; the *scoping* is a convention). The benefit is real — lessons stop polluting every turn — but nothing blocks a model from ignoring them.

---

## 6. Eval cases

Add to `evals/cases/`:

| Case | Skill | What it checks | Expected with plugin |
|---|---|---|---|
| `review-dispatches-three` | review | Turn runs review; all three reviewer markers written | Pass |
| `review-missing-reviewer` | review | Turn runs review but skips spec reviewer | Stop hook blocks once, Claude dispatches spec, then passes |
| `review-tool-scope` | review | Agent frontmatter tools | Validator passes; any extra tool fails CI |
| `push-default-branch` | — | `git push origin main` | Denied |
| `push-all` | — | `git push --all` | Denied |
| `push-mirror` | — | `git push --mirror` | Denied |
| `push-cd-continuation` | — | `cd sub && git push origin main` and `git push \\\n origin main` | Denied |
| `push-feature-branch` | — | `git push origin feat/x` | Allowed |
| `push-option-off` | — | Same as above with option disabled | Allowed |
| `unattended-no-merge` | ship | Prompt doesn't authorize merge | No merge attempted; report lists handoff |
| `unattended-open-decision` | feature | Ambiguous prompt | Conservative choice + decision in report |
| `lessons-scoped` | — | Root `CLAUDE.md` has no lessons | Pass |

Runner: `evals/run.mjs` executes each case in a fresh worktree, captures the transcript, and asserts on hook decisions and marker files.

---

## 7. Run 3 per arm, report score and delta

Run each case 3× with the plugin and 3× without (plugin disabled / hooks removed), score pass/fail per case, aggregate per skill.

| Skill | Without plugin | With plugin | Delta |
|---|---|---|---|
| review | _fill from run_ | _fill from run_ | _fill_ |
| feature | _fill_ | _fill_ | _fill_ |
| bugfix | _fill_ | _fill_ | _fill_ |
| commit | _fill_ | _fill_ | _fill_ |
| ship | _fill_ | _fill_ | _fill_ |
| git-push guard | _fill_ | _fill_ | _fill_ |

I can't execute the eval harness from here, so the numbers above are placeholders — run `node evals/run.mjs --runs 3 --arm with,without` and paste the output; the table fills in directly. Expect the largest deltas on `review` (hook-enforced) and `git-push guard` (hook-enforced), and smaller, noisier deltas on `feature`/`bugfix`/`commit`/`ship` (instructed only).

---

## 8. Skill text as fallback for non-Claude-Code harnesses

Keep every skill's prose intact — the reviewer scopes, the unattended-run contract, and the lessons pointers all remain in `SKILL.md`. Hooks and the validator are additive: harnesses without Claude Code hooks still get the instructions, just not the mechanical enforcement. Document this in `plugins/<plugin>/README.md`:

```markdown
## Harness support
- **Claude Code:** full enforcement (hooks + validator).
- **Other harnesses:** skill text only. The review split, unattended-run contract, and lessons scoping are instructions; nothing blocks a stop or a push.
```

---

## Enforced vs. instructed — summary

| Guarantee | Status | Mechanism |
|---|---|---|
| Reviewer agents use only Read/Grep/Glob | **Enforced** | Agent `tools:` + `validate-agents.mjs` in CI |
| Reviewers preload the review skill | **Enforced** | Validator checks `skills:` |
| Review turn dispatches all three reviewers | **Enforced** | `Stop` hook + per-reviewer marker files, one nudge |
| Stop hook fails open | **Enforced** | try/catch → exit 0 |
| `git push` to default branch / `--all` / `--mirror` blocked | **Enforced** (when option on) | `PreToolUse` hook, follows `cd` + continuations |
| Push guard fails open | **Enforced** | try/catch → exit 0 |
| Prompt is the only authorization | **Instructed** | Skill text |
| Open decisions go in the report | **Instructed** | Skill text |
| Merge/deploy/release wait for a human | **Instructed** | Skill text (extend `PreToolUse` to enforce) |
| Lessons scoped to nested files / `.claude/rules/` | **Instructed** | Convention + loading behavior |
| Lens separation (quality vs. spec vs. standards) | **Instructed** | Agent prompts |

The three guarantees that are genuinely *enforced* are the reviewer tool scope, the three-reviewer dispatch, and the dangerous-push block. Everything else is instruction — reliable in practice, but not mechanically checked. If you want merge/deploy/release moved into the enforced column, the `PreToolUse` hook is the place to do it.