diff --git a/.archon/checklists/cold-pass-default-checklist.md b/.archon/checklists/cold-pass-default-checklist.md new file mode 100644 index 0000000000..d5c7fe38b0 --- /dev/null +++ b/.archon/checklists/cold-pass-default-checklist.md @@ -0,0 +1,63 @@ +# Cold-Pass Default Checklist (Universal) + +**Reviewer instruction:** Walk each `[ ]` item literally. Output PASS / FAIL / N/A +with a file:line reference for every FAIL. Do not summarize. Do not skip. + +The #1 heuristic: **one-sided parity.** For every producer touched in the diff, +identify its consumers. For every filter added, identify every sibling endpoint +that needs matching behavior. Multi-agent reviewers miss this because they read +files in isolation — your job is to read them together. + +--- + +## ONE-SIDED PARITY (read/write drift) +[ ] For every function the diff modifies, which other functions call it? Does + each call site still pass the correct arguments? +[ ] For every new parameter or field: is it threaded through every path that + reaches the modified function, or only the one the PR author tested? +[ ] For every writer touched: find its readers. Still in sync? +[ ] For every reader touched: find its writers. Still in sync? +[ ] "Sibling endpoint" parity: if PR adds filter/field on endpoint A, do + siblings (list, detail, count, summary, inventory) need matching behavior? + +## CROSS-FILE STATE MACHINES +[ ] If the PR advances a state in one table/record, are all linked states + advanced together? (Approval workflows, order fulfillment, multi-step + transitions.) +[ ] Are invariants maintained across the cross-table transition, or only + per-table? + +## MIGRATION / SCHEMA HYGIENE +[ ] Migrations have explicit BEGIN/COMMIT wrappers or transaction guarantees? +[ ] UPDATE-only migrations on existing rows: do they silent no-op on fresh DBs + where the rows don't exist yet? +[ ] Numeric prefix / filename collisions with existing migrations? +[ ] Schema changes referenced consistently across all code paths (ORM + raw + SQL + serializers)? + +## RESOURCE / FAILURE MODES +[ ] `query[0]` / `.first()` on a multi-row query without ORDER BY + (non-deterministic on Postgres, MySQL, most engines)? +[ ] New parameter handled for empty string, None/null, special characters, + very large input? +[ ] Bypasses existing auth, rate-limit, or validation middleware? +[ ] Error paths return the same shape as success paths? (Silent-200, + partial-failure hidden behind 200 OK.) + +## TEST COVERAGE BLIND SPOTS +[ ] Every branch added in the diff maps to at least one test assertion? +[ ] Tests exercise the REAL write path, not direct SQL insert / mock that + skips triggers, validators, or middleware? +[ ] If the PR introduces multiple backends / modes / providers, does at least + one test cover each? + +## DOWNSTREAM CONSISTENCY +[ ] If public API shape changes, are consumers updated or version-gated? +[ ] Config, environment variables, feature flags: documented and plumbed + through all environments? + +## HARDCODED VALUES +[ ] Hardcoded identifiers (IDs, names, keys, domains) that should be + config-driven? +[ ] Defaults that only work for the author's happy path but break other + legitimate inputs? diff --git a/.archon/commands/defaults/archon-cold-pass-review.md b/.archon/commands/defaults/archon-cold-pass-review.md new file mode 100644 index 0000000000..d8e96737eb --- /dev/null +++ b/.archon/commands/defaults/archon-cold-pass-review.md @@ -0,0 +1,97 @@ +--- +description: Independent cold second-pass PR review — single reviewer, repo-local checklist +argument-hint: (none — reads PR number from ARTIFACTS_DIR/.pr-number) +--- + +# Cold Pass Review + +## Your Role + +You are the **first reviewer** of this PR. That is not metaphor — for your purposes, +no one has looked at this code before. You are walking through it fresh, with a +checklist of structural concerns this repo's maintainers have learned to watch for. + +Archon's 5-agent review has already run and posted its findings as a PR comment. +You will glance at those findings ONCE, at the end, for deduplication only. They +must not shape where you look during your review. Correlated-attention blind spots +are the entire reason you exist: if you let Archon's report anchor your search, you +become a 6th Archon agent and add no value. + +## Phase 1: LOAD + +1. Read `$ARTIFACTS_DIR/.pr-number` → `PR` +2. Read `$ARTIFACTS_DIR/.checklist-source` → `SOURCE` (`default` or `repo+default`) +3. Read `$ARTIFACTS_DIR/cold-pass-checklist.md` → the layered checklist. It always contains the universal default; when `SOURCE=repo+default`, the repo-local extension is appended after a `---` separator. Walk both in order. +4. `gh pr diff $PR` → the changes +5. For every file in the diff, `cat ` in full (not just the diff context) +6. Do NOT read Archon's artifacts. Do NOT read `$ARTIFACTS_DIR/review/*`. + +**Phase 1 checkpoint:** +- [ ] PR number loaded +- [ ] Checklist source loaded +- [ ] Layered checklist loaded (universal default present; repo-local present if `SOURCE=repo+default`) +- [ ] Diff read +- [ ] All modified files read in full +- [ ] Archon artifacts NOT consulted + +## Phase 2: ANALYZE + +Walk the checklist literally — check each `[ ]` box, one at a time, out loud. +The literal walk is the discipline. Never summarize the checklist. Never skip +an item because it "looks fine." + +For each item: `PASS` / `FAIL` / `N/A`. For every `FAIL`, include a file:line +reference and a one-sentence explanation. + +**Special attention to one-sided parity.** For every producer/writer touched by +the diff, identify its consumers/readers (grep the codebase if needed) and verify +they are still in sync. This is the #1 shape to hunt for. + +**Phase 2 checkpoint:** +- [ ] Every checklist item walked +- [ ] FAILs have file:line refs +- [ ] One-sided parity checks done for every modified writer + +## Phase 3: GENERATE + +Write `$ARTIFACTS_DIR/cold-pass-findings.md`: + +``` +# 🧊 COLD PASS: PR # + +**Reviewer model:** +**Checklist source:** <`default` or `repo+default` — read from `$ARTIFACTS_DIR/.checklist-source`> +**Verdict:** + +## HIGH +- — + +## MEDIUM +- — + +## LOW +- — + +## Deduplication note + +After reading `gh pr view --comments`, I removed findings Archon already +caught at the same or higher severity. Findings retained below are either new, +or graded higher than Archon did (with reasoning). + +## Summary + + +``` + +**Phase 3 checkpoint:** +- [ ] Findings file exists +- [ ] Verdict line present +- [ ] Dedup note present (even if empty) + +## Phase 4: VALIDATE + +Check that `cold-pass-findings.md`: +- Has a Verdict line (APPROVE or REQUEST_CHANGES) +- Has a Summary section +- Has a Deduplication note +- Uses file:line refs for all FAILs