Stage Codex inspection before defect assessment

.claude-plugin/marketplace.json

@@ -8,7 +8,7 @@
       "name": "code-review-ad",
       "source": "./",
       "description": "code-review-ad",
-      "version": "1.0.4"
+      "version": "1.0.5"
     }
   ]
 }

.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "code-review-ad",
-  "version": "1.0.4",
+  "version": "1.0.5",
   "description": "Independent Toolkit, Standards, Spec, and conditional Performance review.",
   "author": {
     "name": "Dmitry Kropachev"

.codex-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "code-review-ad",
-  "version": "1.0.4",
+  "version": "1.0.5",
   "description": "Independent Toolkit, Standards, Spec, and conditional Performance review.",
   "author": {
     "name": "Dmitry Kropachev"

PROVENANCE.json

@@ -22,6 +22,16 @@
         "claude/skills/code-review-ad/SKILL.md"
       ],
       "description": "Shared evidence criteria, broad inspection, truncation recovery and explicit candidate reconciliation; native host methods retained. Model and reasoning-effort configuration belongs to the host or runner for both skills."
+    },
+    {
+      "version": "1.0.5",
+      "paths": [
+        "common/codex/scripts/aspect-driver.py",
+        "common/codex/review.md",
+        "codex/skills/code-review-ad/SKILL.md",
+        "tests/test_aspect_driver.py"
+      ],
+      "description": "Experimental Codex aspect-first inspection followed by driver-issued assessment and structured finding collection; read-only source validation and existing parent reconciliation retained."
     }
   ]
 }

README.md

@@ -1,6 +1,6 @@
 # code-review-ad
 
-Independent MIT plugin for Claude Code and Codex. Version 1.0.4.
+Independent MIT plugin for Claude Code and Codex. Version 1.0.5.
 
 ## Install
 
@@ -59,4 +59,6 @@ Invoke `code-review-ad` with a PR number or URL. Review stays in the current hos
 
 Both hosts use shared evidence criteria for runtime/security, test/build, diagnostic, documentation, rule, maintenance, and explicit acceptance findings. Reviews include concrete maintenance risks supported by named duplicated sites, state coupling, API obligations, or a supported failure mode. Reviewers inspect changed files broadly before deep verification and keep omitted or truncated evidence visible as coverage gaps. Ordinary reports remain concise; candidate ledgers are available for requested diagnostics. Directly related pre-existing issues carry explicit scope labels.
 
+The experimental Codex path starts native reviewers with factual code-aspect inspection. Its read-only `common/codex/scripts/aspect-driver.py` then asks whether those observations reveal problems and collects cited findings before the existing parent verification. Claude retains its current method.
+
 Model and reasoning-effort settings are controlled by the host or runner. Neither host skill sets reviewer overrides.

codex/skills/code-review-ad/SKILL.md

@@ -9,4 +9,6 @@ Model and reasoning-effort settings belong to the host or runner. Do not set rev
 
 Before work, read [the lifecycle procedure](../../../common/lifecycle.md), then run its entry command with `--host codex`. Retain its returned token and follow its structured action. Always run its completion command with that token in a finalization step, including on failure or interruption.
 
+Native reviewers first describe code aspects, then run the bundled driver to assess those observations and collect any supported findings. The shared procedure defines the staged handoff.
+
 Read [the shared procedure](../../../common/codex/procedure.md) and perform the requested workflow. Resolve paths beginning with `PLUGIN_ROOT/` against this plugin's root. Preserve the shared procedure's validation and publication guards.

common/codex/review.md

@@ -89,34 +89,42 @@ reference is not evidence that its checks passed. Verify each applicable asserti
 Launch the three core native Codex agents, named `toolkit`, `standards`, and `spec`, in parallel.
 Launch a fourth, independent Performance subagent only when the user explicitly requests performance review, or both the PR changes a hot path and repository guidance or existing performance tests/benchmarks show that performance matters here. Assess that condition from the frozen diff and repository evidence before launching agents; do not infer it from a generic possibility of slowdown. If launched, name it `performance` and give it the same constraints,
 pins, rules and PR metadata, with only performance-relevant source facts.
-Start each reviewer with a fresh context (`fork_turns: "none"`). Give every pass the same frozen
+Start each reviewer with a fresh context (`fork_turns: "none"`). Give each the same frozen
 PR description/spec, pins, changed-file list, commit metadata, claim inventory, applicable
-repository/user rules, and exact diff command. Supply absolute paths to this procedure, its
-pass brief, and the selected check references. Do not copy this procedure or the references
-again into the handoff; the agent reads the applicable files in focused instruction reads,
-separate from patch output. Include the shared reporting and inspection paths in every handoff.
-Do not pass parent hypotheses, other reviewers' findings, or the parent conversation.
-
-- Toolkit reads [its brief](passes/toolkit.md).
-- Standards reads [its brief](passes/standards.md).
-- Spec reads [its brief](passes/spec.md).
-- When selected, Performance reads [its brief](passes/performance.md).
-
-Each prompt must explicitly forbid edits, formatters, posting, model subprocesses, further
-agents, and Git checkout/switch/reset/stash/worktree. Keep those prohibitions in prompt text,
-not shell commands. Read the assigned briefs/checks separately from the pinned diff.
-Use focused file or hunk reads, checking outer and nested output for omitted or truncated
-ranges and retrieving them before marking coverage complete. Inspect every changed file
-and relevant metadata broadly before deep candidate verification, without repeating a full scan. Unfinished checks
-are named coverage gaps, not grounds to claim no findings. Do not run
-tests, builds, linters, or formatters. Return findings promptly.
-
-For each candidate, return its issue category, exact claim, changed line, source-backed trigger and consequence,
-base/head distinction, and relevant contrary evidence considered. Include the required state,
-transition, interleaving, and numerical tables from selected checks. Add a compact coverage
-note for each applicable requested check: source evidence plus finding reference, supported
-no-finding conclusion, or unexamined gap. Include test assertions/failure cleanup and commit
-requirements. Unexamined means a gap, never no finding. Do not drop low-severity verified facts.
+repository/user rules, and exact diff command. Do not pass parent hypotheses, other agents'
+findings, or the parent conversation.
+
+Use the bundled [aspect driver](scripts/aspect-driver.py) in each native agent's own session.
+The initial task is to look at the assigned aspects and describe the implementation. Give the
+agent the driver command and the inspection boundary, without asking it to find bugs or sending
+this review procedure, reporting criteria, pass brief, or check contents in that initial handoff.
+Supply selected check paths as driver arguments for later use. The driver is read-only and
+prints its next instruction as JSON; no scratch or state files are needed.
+
+1. Run `python3 PLUGIN_ROOT/common/codex/scripts/aspect-driver.py --phase inspect
+   --pass ROLE --base BASE --head HEAD`, replacing ROLE with `toolkit`, `standards`, `spec`,
+   or the selected `performance` role. Follow the printed aspect prompts and inspect every
+   changed file broadly, then trace the relevant behavior. Record factual observations with
+   pinned file/line citations and explicit gaps. Use focused reads and recover needed truncated
+   ranges. Do not read the pass brief or check contents yet.
+2. In that same agent session, run the driver with `--phase assess`, the same role and pins,
+   `--observations` containing JSON `{summary,evidence:[{path,line,observation}],gaps:[]}`,
+   and one `--check PATH` per selected reference. Quote JSON as literal shell arguments.
+   The driver's output asks whether the observed behavior has any problems. Read its `readNow`
+   and applicable `checks` resources at this point, then follow its assessment instructions.
+   Apply the same substantive checks, evidence criteria, and coverage obligations as before.
+3. Run `--phase collect` with the same role, pins and observations and `--findings` containing
+   the driver's requested finding array (or `[]`). Return the collected findings, required
+   evidence tables and coverage gaps to the parent. The driver validates citations and
+   structure; the parent's existing verification still decides which assertions are supported.
+   If Spec has no meaningful specification, return `No spec available` with source observations
+   and an empty findings array. An unfinished required check remains a coverage gap.
+
+Each handoff must explicitly forbid edits, formatters, posting, model subprocesses, further
+agents, and Git checkout/switch/reset/stash/worktree. Do not run tests, builds, or linters.
+Keep those prohibitions in prompt text, not shell commands. The driver requires Python 3.11+
+from the plugin requirements. If it cannot run, disclose the incomplete staged inspection;
+do not silently substitute a direct bug-finding prompt or claim that the protocol completed.
 
 Do not share interim findings or read more source in the parent while reviewers run. Wait
 on their native handles, using a 60000 ms observation timeout where supported so empty polls

common/codex/scripts/aspect-driver.py

@@ -0,0 +1,128 @@
+#!/usr/bin/env python3
+"""Read-only, two-stage source inspection and finding collection for native agents."""
+import argparse
+import hashlib
+import json
+from pathlib import Path, PurePosixPath
+import re
+import subprocess
+import sys
+
+
+ASPECTS = {
+    "toolkit": [
+        "Describe inputs, outputs, error propagation, and public type/API behavior.",
+        "Trace ownership, state transitions, callbacks, waits, retries, and shutdown.",
+        "Describe test stimuli, assertions, deadlines, and cleanup paths.",
+        "Relate factual comments and documentation to the implementation.",
+        "Map changed branches and repeated logic to their callers and tests.",
+    ],
+    "standards": [
+        "Identify documented repository rules and their artifact-specific scope.",
+        "Compare changed setup, teardown, helpers, and declarations with nearby conventions.",
+        "Describe duplicated sites and state or API obligations that require coordination.",
+        "Map commit contents, messages, attribution, and validation claims to the changes.",
+    ],
+    "spec": [
+        "Identify explicit promises in the PR description and supplied specification.",
+        "Map those promises to base/head behavior, including completion and failure ordering.",
+        "Describe how changed tests stimulate the promised behavior.",
+    ],
+    "performance": [
+        "Describe work, allocation, I/O, and contention in changed hot paths.",
+        "Compare base/head costs for concrete inputs and concurrency levels.",
+    ],
+}
+ROOT = Path(__file__).resolve().parents[3]
+
+
+def git(*args):
+    return subprocess.check_output(["git", *args], text=True, stderr=subprocess.PIPE).strip()
+
+
+def citation(row, head):
+    path, line = row.get("path"), row.get("line")
+    if (not isinstance(path, str) or not path or "\\" in path
+            or PurePosixPath(path).is_absolute() or ".." in path.split("/")):
+        raise ValueError("citation needs a repository-relative path")
+    if type(line) is not int or line < 1:
+        raise ValueError("citation needs a positive integer line")
+    if line > len(git("show", f"{head}:{path}").splitlines()):
+        raise ValueError("citation line is outside the pinned head file")
+
+
+def observations(value, head):
+    if not isinstance(value, dict) or not isinstance(value.get("summary"), str) or not value["summary"].strip():
+        raise ValueError("observations need a nonempty factual summary")
+    evidence, gaps = value.get("evidence"), value.get("gaps")
+    if not isinstance(evidence, list) or not isinstance(gaps, list) or any(not isinstance(g, str) or not g.strip() for g in gaps):
+        raise ValueError("observations need evidence and explicit gap lists")
+    if not evidence and not gaps:
+        raise ValueError("supply source evidence or disclose the unexamined scope")
+    for row in evidence:
+        citation(row, head)
+        if not isinstance(row.get("observation"), str) or not row["observation"].strip():
+            raise ValueError("each source citation needs a factual observation")
+    return value
+
+
+def drive(args):
+    if not all(re.fullmatch(r"[a-f0-9]{40}", pin) for pin in (args.base, args.head)):
+        raise ValueError("full base/head OIDs are required")
+    if git("rev-parse", "HEAD") != args.head:
+        raise ValueError("checkout HEAD differs from the pinned head")
+    if git("merge-base", args.base, args.head) != args.base:
+        raise ValueError("base must be the pinned merge-base")
+    common = {"schemaVersion": 1, "pass": args.role, "base": args.base, "head": args.head}
+    if args.phase == "inspect":
+        return {**common, "phase": "inspect", "aspects": ASPECTS[args.role],
+                "instruction": "Look at these aspects across every changed file and relevant metadata. Describe what the code does, with source citations, and record unexamined scope. Then run this driver with --phase assess and --observations JSON containing summary, evidence [{path,line,observation}], and gaps. Stay in this same native agent session."}
+    seen = observations(json.loads(args.observations or "null"), args.head)
+    digest = hashlib.sha256(json.dumps(seen, sort_keys=True).encode()).hexdigest()
+    common.update(observations=seen, observationSha256=digest)
+    if args.phase == "assess":
+        return {**common, "phase": "assess",
+                "question": "Having looked at those aspects, do you see any problems there? No problems is a valid answer. Assess the observed behavior against the applicable contracts and consider contrary evidence; investigate a concern further only as needed to support its actual assertion.",
+                "readNow": [str(ROOT / "common" / "reporting.md"),
+                            str(ROOT / "common" / "codex" / "passes" / f"{args.role}.md")],
+                "checks": args.check,
+                "instruction": "Apply the supplied pass brief and applicable check references now. Collect supported findings with --phase collect, the same --observations, and --findings JSON. Each finding needs id, category, claim, path, line, trigger, consequence, baseHead, and contraryEvidence. An empty array is valid. Preserve material coverage gaps; do not create files, run tests/builds, delegate, or publish."}
+    findings = json.loads(args.findings or "null")
+    if not isinstance(findings, list):
+        raise ValueError("findings must be a JSON array; use [] for no findings")
+    ids = set()
+    for row in findings:
+        if not isinstance(row, dict):
+            raise ValueError("each finding must be an object")
+        for field in ("id", "category", "claim", "trigger", "consequence", "baseHead", "contraryEvidence"):
+            if not isinstance(row.get(field), str) or not row[field].strip():
+                raise ValueError(f"finding needs nonempty {field}")
+        if row["id"] in ids:
+            raise ValueError("finding IDs must be unique within a pass")
+        ids.add(row["id"])
+        citation(row, args.head)
+    return {**common, "phase": "collected", "findings": findings,
+            "coverageGaps": seen["gaps"],
+            "instruction": "Return these findings and coverage gaps to the parent for its existing independent verification. Collection validates structure and citations, not semantic truth."}
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--phase", choices=("inspect", "assess", "collect"), required=True)
+    parser.add_argument("--pass", dest="role", choices=ASPECTS, required=True)
+    parser.add_argument("--base", required=True)
+    parser.add_argument("--head", required=True)
+    parser.add_argument("--observations")
+    parser.add_argument("--findings")
+    parser.add_argument("--check", action="append", default=[])
+    args = parser.parse_args()
+    try:
+        print(json.dumps(drive(args), ensure_ascii=False))
+    except (ValueError, TypeError, AttributeError, subprocess.CalledProcessError) as exc:
+        print(f"aspect-driver: {exc}", file=sys.stderr)
+        return 1
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/test_aspect_driver.py

@@ -0,0 +1,85 @@
+"""Exercise staged collection against real pinned Git objects without source edits."""
+import json
+from pathlib import Path
+import subprocess
+import sys
+
+import pytest
+
+
+DRIVER = Path(__file__).resolve().parents[1] / "common/codex/scripts/aspect-driver.py"
+
+
+@pytest.fixture
+def repo(tmp_path):
+    def git(*args):
+        return subprocess.check_output(["git", "-C", str(tmp_path), *args], text=True).strip()
+    git("init", "-q")
+    git("config", "user.email", "test@example.invalid")
+    git("config", "user.name", "Test")
+    (tmp_path / "code.py").write_text("value = 1\n")
+    git("add", ".")
+    git("commit", "-qm", "base")
+    base = git("rev-parse", "HEAD")
+    (tmp_path / "code.py").write_text("value = 2\n")
+    git("commit", "-qam", "head")
+    return tmp_path, base, git("rev-parse", "HEAD")
+
+
+def run(repo, phase, **inputs):
+    directory, base, head = repo
+    args = [sys.executable, str(DRIVER), "--phase", phase, "--pass", "toolkit",
+            "--base", base, "--head", head]
+    for key, value in inputs.items():
+        args.extend(["--" + key, json.dumps(value)])
+    return subprocess.run(args, cwd=directory, capture_output=True, text=True)
+
+
+def seen(**updates):
+    return {"summary": "The assigned value changes.", "evidence": [
+        {"path": "code.py", "line": 1, "observation": "Value is now two."}],
+        "gaps": [], **updates}
+
+
+def finding(**updates):
+    return {"id": "t1", "category": "runtime", "claim": "Example claim",
+            "path": "code.py", "line": 1, "trigger": "Example input",
+            "consequence": "Example consequence", "baseHead": "One becomes two",
+            "contraryEvidence": "No alternate assignment", **updates}
+
+
+def test_stages_collect_without_editing_checkout(repo):
+    assert run(repo, "inspect").returncode == 0
+    assessment = run(repo, "assess", observations=seen())
+    assert assessment.returncode == 0, assessment.stderr
+    assessed = json.loads(assessment.stdout)
+    assert assessed["observations"] == seen()
+    collected = run(repo, "collect", observations=seen(), findings=[finding()])
+    assert collected.returncode == 0, collected.stderr
+    value = json.loads(collected.stdout)
+    assert value["findings"] == [finding()]
+    assert value["observationSha256"] == assessed["observationSha256"]
+    assert subprocess.check_output(["git", "status", "--porcelain"], cwd=repo[0]) == b""
+
+
+def test_no_findings_preserves_unexamined_scope(repo):
+    result = run(repo, "collect", observations=seen(gaps=["Shutdown not inspected"]), findings=[])
+    assert result.returncode == 0, result.stderr
+    assert json.loads(result.stdout)["coverageGaps"] == ["Shutdown not inspected"]
+
+
+@pytest.mark.parametrize("update", [{"path": "../code.py"}, {"path": "missing.py"},
+                                   {"line": 200}, {"line": True}, {"claim": ""}])
+def test_rejects_invalid_claims_and_citations(repo, update):
+    assert run(repo, "collect", observations=seen(), findings=[finding(**update)]).returncode == 1
+
+
+def test_does_not_assess_without_observations(repo):
+    assert run(repo, "assess").returncode == 1
+    assert run(repo, "assess", observations=seen(evidence=[])).returncode == 1
+
+
+def test_rejects_duplicate_ids_and_moved_checkout(repo):
+    assert run(repo, "collect", observations=seen(), findings=[finding(), finding()]).returncode == 1
+    directory, base, head = repo
+    assert run((directory, base, base), "inspect").returncode == 1