feat: define shared code review contracts

CHANGELOG.md

@@ -6,7 +6,9 @@ summary: Chronological history of repository and skill changes.
 
 ## 2026-07-19 — Epic workflow and review contract cleanup
 
+- feat: define shared code review contracts
 - feat: add epic sequence implementation skill
+  (06bd81f4293a24e12cde1f0e466596b41095e8f4)
 - revert: remove modular code review contract
   (b889fe4dc313dc50320dcb20f98980b993062c9a)
 

README.md

@@ -5,6 +5,8 @@ A personal monorepo for agent skills and supporting scripts.
 ## Repository Layout
 
 - `skills/` — skill folders, each containing a `SKILL.md` and bundled resources
+- `review-suite/` — canonical code-review contracts, validators, and raw
+  evaluation fixtures shared by repository-owned review skills
 - `justfile` — common tasks for testing, validation, and formatting
 
 Current skills:
@@ -26,6 +28,13 @@ Run skill-specific tests:
 
 ```bash
 just test-prepare-changesets
+just test-review-suite
+```
+
+Validate a review packet and result together:
+
+```bash
+python3 review-suite/scripts/validate.py pair packet.json result.json
 ```
 
 Run deterministic local evals (no Codex required):

justfile

@@ -19,8 +19,15 @@ test:
   done; \
   if [ "$found" -eq 0 ]; then \
     echo "No skill tests found under {{skills_dir}}/*/scripts/tests"; \
+  fi; \
+  if [ -d review-suite/scripts/tests ]; then \
+    echo "Running tests in review-suite/scripts/tests"; \
+    python3 -m unittest discover -s review-suite/scripts/tests -p 'test_*.py'; \
   fi
 
+test-review-suite:
+  python3 -m unittest discover -s review-suite/scripts/tests -p 'test_*.py'
+
 test-prepare-changesets:
   python3 -m unittest discover -s {{skills_dir}}/prepare-changesets/scripts/tests -p 'test_*.py'
 

review-suite/CONTRACT.md

@@ -0,0 +1,114 @@
+# Code review suite contract
+
+This directory is the canonical, non-skill foundation for the repository-owned
+code review suite. Review skills consume these contracts; they must not copy or
+silently redefine them.
+
+## Contract ownership
+
+- `contracts/review-packet.schema.json` owns the packet shape.
+- `contracts/review-result.schema.json` owns the finding and result shapes.
+- This document owns the cross-field semantics that JSON Schema cannot express
+  clearly.
+- `scripts/validate.py` enforces both schemas and these semantic rules without
+  third-party dependencies.
+- `fixtures/` contains raw review inputs and separate expected material outcomes
+  for deterministic tests and independent forward tests.
+
+## Review packet
+
+A review packet binds the review to one candidate and states what that candidate
+must accomplish. Required evidence is deliberately distinct from optional
+context.
+
+Required packet sections are:
+
+1. `repository`: repository identity and base branch.
+2. `candidate`: the captured head, the comparison base or merge base, and the
+   complete candidate diff.
+3. `change_contract`: observable goal, non-empty acceptance criteria, explicit
+   non-goals, and behavior or invariants to preserve.
+4. `sources`: applicable repository instructions, named design or contract
+   documents, and representative nearby patterns. Arrays may be empty only when
+   the caller has established that no such source applies.
+5. `validation`: at least one `focused` and one `full` validation entry, with
+   every required command's exact result or an explicit reason that the command
+   is unavailable.
+
+Optional `context` records public API, data, authorization, compatibility, and
+operational concerns when applicable. Optional `worktree` records tracked,
+staged, unstaged, untracked, and ignored state when candidate integrity depends
+on it. Optional `base_drift` records why evidence was retained or reset after
+the base advanced.
+
+Do not infer missing intent. Missing repository identity, goal, acceptance
+criteria, candidate identity, a complete diff, or required validation evidence
+prevents a trustworthy review and must yield a `blocked` result.
+
+## Finding semantics
+
+Every material finding contains:
+
+- a stable identifier;
+- its owning lens;
+- severity and confidence;
+- the requirement, non-goal, invariant, or repository rule involved;
+- concrete evidence;
+- the concern and material impact;
+- the smallest sufficient proposed change; and
+- the expected behavioral or complexity effect.
+
+Use these severities:
+
+- `blocking`: a demonstrated correctness, security, authorization, acceptance,
+  architecture, compatibility, or validation failure that prevents merge.
+- `strong_recommendation`: a material, tractable, ticket-scoped improvement
+  supported by concrete evidence and a sufficiently specified correction.
+- `defer`: a real concern intentionally outside the active ticket, dependent on
+  an unresolved decision, or not justified strongly enough to change the
+  candidate.
+
+Do not emit aesthetic preferences, praise, generic resources, numerical quality
+scores, imagined compatibility needs, speculative hardening, or abstractions
+that merely move complexity behind another name.
+
+## Verdict semantics
+
+- `clean`: no `blocking` or `strong_recommendation` finding remains. Deferred
+  findings may be retained without failing the gate.
+- `changes_required`: at least one actionable `blocking` or
+  `strong_recommendation` finding remains.
+- `blocked`: essential evidence or a product or architecture decision is
+  missing, so no trustworthy merge verdict is possible. Include at least one
+  concrete `blocking_reason`.
+
+`clean` and `changes_required` results must include complete candidate identity
+and must not include `blocking_reasons`. A `blocked` result may omit candidate
+fields that the caller could not establish and may preserve already-demonstrated
+findings, but those findings do not convert the blocked review into a merge
+verdict.
+
+## Candidate identity and base drift
+
+Bind every result to the packet's captured head and comparison base. Any edit,
+rebase, conflict resolution, or update operation that changes the head
+invalidates head-bound evidence and requires a new packet.
+
+When only the base advances, inspect the effective merge candidate. Retain prior
+head-bound evidence only when all of these are true:
+
+- the effective diff is unchanged;
+- the resulting tree is unchanged;
+- no conflict exists;
+- no relevant base code overlaps the candidate; and
+- repository policy does not require a complete reset.
+
+Record the decision and reason in `base_drift`. Otherwise reset affected or all
+evidence as repository policy and the changed candidate require.
+
+## Fixture use
+
+Each fixture keeps `expected.json` separate from `prompt.md` and `packet.json`.
+Give a forward-testing reviewer only the prompt and raw packet. Do not expose
+the expected outcome, implementation transcript, prior conclusions, or suspected
+finding.

review-suite/contracts/review-packet.schema.json

@@ -0,0 +1,166 @@
+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "$id": "https://github.com/shaug/agent-scripts/review-suite/contracts/review-packet.schema.json",
+  "title": "Review packet",
+  "type": "object",
+  "additionalProperties": false,
+  "required": [
+    "schema_version",
+    "repository",
+    "candidate",
+    "change_contract",
+    "sources",
+    "validation"
+  ],
+  "properties": {
+    "schema_version": {
+      "const": "1.0"
+    },
+    "repository": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": ["identity", "base_branch"],
+      "properties": {
+        "identity": {"type": "string", "minLength": 1},
+        "base_branch": {"type": "string", "minLength": 1}
+      }
+    },
+    "candidate": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": ["head_sha", "comparison_base_sha", "diff"],
+      "properties": {
+        "head_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+        "comparison_base_sha": {
+          "type": "string",
+          "pattern": "^[0-9a-f]{40}$"
+        },
+        "diff": {
+          "type": "object",
+          "additionalProperties": false,
+          "required": ["format", "complete", "content"],
+          "properties": {
+            "format": {"enum": ["unified_diff"]},
+            "complete": {"const": true},
+            "content": {"type": "string", "minLength": 1}
+          }
+        }
+      }
+    },
+    "change_contract": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": [
+        "goal",
+        "acceptance_criteria",
+        "non_goals",
+        "preserved_behaviors"
+      ],
+      "properties": {
+        "goal": {"type": "string", "minLength": 1},
+        "acceptance_criteria": {
+          "type": "array",
+          "minItems": 1,
+          "items": {"type": "string", "minLength": 1}
+        },
+        "non_goals": {
+          "type": "array",
+          "items": {"type": "string", "minLength": 1}
+        },
+        "preserved_behaviors": {
+          "type": "array",
+          "items": {"type": "string", "minLength": 1}
+        }
+      }
+    },
+    "sources": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": [
+        "repository_instructions",
+        "named_documents",
+        "nearby_patterns"
+      ],
+      "properties": {
+        "repository_instructions": {
+          "type": "array",
+          "items": {"type": "object", "required": ["label", "location"], "properties": {"label": {"type": "string", "minLength": 1}, "location": {"type": "string", "minLength": 1}, "summary": {"type": "string"}}, "additionalProperties": false}
+        },
+        "named_documents": {
+          "type": "array",
+          "items": {"type": "object", "required": ["label", "location"], "properties": {"label": {"type": "string", "minLength": 1}, "location": {"type": "string", "minLength": 1}, "summary": {"type": "string"}}, "additionalProperties": false}
+        },
+        "nearby_patterns": {
+          "type": "array",
+          "items": {"type": "object", "required": ["label", "location"], "properties": {"label": {"type": "string", "minLength": 1}, "location": {"type": "string", "minLength": 1}, "summary": {"type": "string"}}, "additionalProperties": false}
+        }
+      }
+    },
+    "validation": {
+      "type": "array",
+      "minItems": 1,
+      "items": {
+        "type": "object",
+        "additionalProperties": false,
+        "required": ["name", "command", "scope", "status"],
+        "properties": {
+          "name": {"type": "string", "minLength": 1},
+          "command": {"type": "string", "minLength": 1},
+          "scope": {"enum": ["focused", "full"]},
+          "status": {"enum": ["passed", "failed", "unavailable"]},
+          "result": {"type": "string", "minLength": 1},
+          "reason": {"type": "string", "minLength": 1}
+        }
+      }
+    },
+    "context": {
+      "type": "object",
+      "additionalProperties": false,
+      "properties": {
+        "public_api": {"type": "array", "items": {"type": "string", "minLength": 1}},
+        "data": {"type": "array", "items": {"type": "string", "minLength": 1}},
+        "authorization": {"type": "array", "items": {"type": "string", "minLength": 1}},
+        "compatibility": {"type": "array", "items": {"type": "string", "minLength": 1}},
+        "operational": {"type": "array", "items": {"type": "string", "minLength": 1}}
+      }
+    },
+    "worktree": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": ["tracked", "staged", "unstaged", "untracked", "ignored"],
+      "properties": {
+        "tracked": {"type": "array", "items": {"type": "string"}},
+        "staged": {"type": "array", "items": {"type": "string"}},
+        "unstaged": {"type": "array", "items": {"type": "string"}},
+        "untracked": {"type": "array", "items": {"type": "string"}},
+        "ignored": {"type": "array", "items": {"type": "string"}}
+      }
+    },
+    "base_drift": {
+      "type": "object",
+      "additionalProperties": false,
+      "required": [
+        "captured_base_sha",
+        "current_base_sha",
+        "effective_diff_changed",
+        "resulting_tree_changed",
+        "conflict",
+        "relevant_overlap",
+        "repository_requires_reset",
+        "decision",
+        "reason"
+      ],
+      "properties": {
+        "captured_base_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+        "current_base_sha": {"type": "string", "pattern": "^[0-9a-f]{40}$"},
+        "effective_diff_changed": {"type": "boolean"},
+        "resulting_tree_changed": {"type": "boolean"},
+        "conflict": {"type": "boolean"},
+        "relevant_overlap": {"type": "boolean"},
+        "repository_requires_reset": {"type": "boolean"},
+        "decision": {"enum": ["retain", "reset_affected", "reset_all"]},
+        "reason": {"type": "string", "minLength": 1}
+      }
+    }
+  }
+}