mastermind-critical-review

Mastermind Critical Review

Challenge a proposed design or completed change before it becomes accepted work. The goal is not to be negative — it's to prevent confident wrongness.

This is the rubric; the spawnable agent that applies it is the mastermind-critic subagent in independent context.

When to use

• Before approving a non-trivial task spec.
• Before accepting a design that touches auth, billing, migrations, public APIs, data loss, or rollback complexity.
• When a planner asks a critic to stress-test an approach.
• When an auditor needs to judge whether executor claims are supported.
• When a design "sounds reasonable" but hasn't been checked against failure modes.

Do NOT use for raw fact gathering — use [[mastermind-codegraph-research]] first when symbol existence, callers, imports, file paths, or blast radius are unknown. Do NOT use to implement fixes; this produces critique, not code.

Security scope: if the review surfaces security-sensitive scope (auth, tools, secrets, delegation, supply chain, prompt injection), don't go deep inline — spawn mastermind-security-auditor and fold its verdict into the critique.

Inputs

• Proposal — the design, spec, report, or plan under review.
• Evidence — codegraph facts, files, test results, logs, or an explicit "evidence unavailable".
• Scope — what the review may challenge.
• Lens (optional) — security, performance, simplicity, migration safety, API compatibility, or testing.

If evidence is missing, say so. Missing mmcg alone does not invalidate a review when source, test, or runtime evidence establishes the required facts. An unsupported claim is unknown; a claim contradicted by evidence is a finding.

Review protocol

Run in order.

1. Evidence — Which claims are backed by local evidence vs assumption? Which named files/symbols/callers/contracts are unverified? Is the design relying on memory or guesswork? Code structure named without evidence is a finding.
2. Contract — Does this change an API, data shape, permission boundary, event/CLI/config contract, or persisted behavior? Are existing consumers covered? Is backward compatibility explicit? Are error shapes and edge cases preserved? Hidden contract changes are high severity.
3. Failure mode — What happens on retry, partial failure, concurrency, stale data, empty/malformed/duplicated/out-of-order input, or a slow/missing/misbehaving dependency? Report only the plausible ones.
4. Scope — Solving the stated problem or adding future-proofing? Is every abstraction justified by ≥ 2 current use cases? Is unrelated cleanup mixed in? Is the path larger than the problem requires? Scope creep is a finding even when the extra work seems useful.
5. Test & verification — Would the proposed tests fail before the fix? Do they cover the contract that can break? Are integration paths covered where unit tests aren't enough? Are VERIFY commands concrete and runnable? "Run tests" is not a sufficient plan for behavior-changing work.
6. Rollback & observability — Safely revertible? Does it need a migration/backfill/feature flag? Would production failure be visible? Require observability only when the risk justifies it.