Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"name": "code-review-ad",
"source": "./",
"description": "code-review-ad",
"version": "1.0.5"
"version": "1.0.6"
}
]
}
2 changes: 1 addition & 1 deletion .claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "code-review-ad",
"version": "1.0.5",
"version": "1.0.6",
"description": "Independent Toolkit, Standards, Spec, and conditional Performance review.",
"author": {
"name": "Dmitry Kropachev"
Expand Down
2 changes: 1 addition & 1 deletion .codex-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "code-review-ad",
"version": "1.0.5",
"version": "1.0.6",
"description": "Independent Toolkit, Standards, Spec, and conditional Performance review.",
"author": {
"name": "Dmitry Kropachev"
Expand Down
11 changes: 11 additions & 0 deletions PROVENANCE.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@
"common/reporting.md"
],
"description": "Require simple, direct and concise review language without filler words for both hosts and every native reviewer."
},
{
"version": "1.0.6",
"paths": [
"common/codex/scripts/aspect-driver.py",
"common/codex/review.md",
"common/codex/passes",
"codex/skills/code-review-ad/SKILL.md",
"tests/test_aspect_driver.py"
],
"description": "Codex issue discovery from the first prompt and one role-specific deeper-look driver call in the same session, without intermediate observation or finding-collection payloads; read-only pins and parent verification retained."
}
]
}
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# code-review-ad

Independent MIT plugin for Claude Code and Codex. Version 1.0.5.
Independent MIT plugin for Claude Code and Codex. Version 1.0.6.

## Install

Expand Down Expand Up @@ -60,3 +60,5 @@ 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.

Model and reasoning-effort settings are controlled by the host or runner. Neither host skill sets reviewer overrides.

Codex reviewers find issues immediately, then call the read-only driver once for a role-specific deeper-look prompt before returning findings for parent verification. Both looks use the same native agent session and reuse inspected evidence; there are no intermediate observation or finding-collection payloads. Claude retains its current method.
2 changes: 2 additions & 0 deletions codex/skills/code-review-ad/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 find issues immediately, then use the bundled driver for one role-specific deeper look in the same session before returning supported findings.

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.
2 changes: 2 additions & 0 deletions common/codex/passes/performance.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Performance pass

Find concrete performance regressions in the selected hot paths immediately.

Inspect changed code and relevant call sites for PR-introduced runtime, memory, I/O, or contention regressions. Prioritize paths exercised per request, item, or retry; derive the work or allocation from the changed operation and a realistic input or concurrency level. Compare base and head, and report only a concrete, consequential regression at a changed line. Do not run benchmarks or expand into general correctness, style, or spec review. Return `No findings` when no performance-sensitive change warrants a finding.
2 changes: 2 additions & 0 deletions common/codex/passes/spec.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Spec pass

Find unmet explicit requirements and contradictions with the supplied specification immediately.

Compare the changed behavior against the PR description, linked issue, and explicit acceptance criteria. Compare the PR body's claimed scope and validation with every commit's actual changes, including tests and observable error/logging behavior. Trace important paths and tests, checking whether reordered early returns skip work formerly performed before them. For a new failure return before a moved state update, check what remains stale while failures continue; a later successful retry does not erase that interval. Compare the base and head for changes to when a public call completes, including new waits, cancellation points, and timeout behavior. If a new wait makes close or stop explicitly depend on earlier queued work, check whether the PR describes that completion order; report a documentation gap even if a previous flush could also block and no new hang is proved. If a wait can remain pending on a peer, I/O, or a promise, check whether the PR describes that observable behavior and whether a test covers it. If none provides a meaningful specification, return `No spec available` for this axis; do not invent one.
2 changes: 2 additions & 0 deletions common/codex/passes/standards.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Standards pass

Find applicable repository-rule violations and evidenced maintenance risks immediately.

Review against applicable, documented repository rules and established conventions supported by specific files. Compare every new test's setup and teardown with nearby tests and contributing guidance, including tracing or logging initialization. Compare a new error variant's display style with adjacent variants and every existing variant that wraps the same inner error; report a pattern mismatch only when it violates an applicable rule or causes a concrete issue. Inspect the PR body and each commit for concrete rule violations, unsupported attribution or validation claims, broken local issue references, a fix split from its test into a later commit, and later fixup commits. For each commit, compare its subject and body with all behavior, public types, and ordering it changes; a subject about one fix may conceal a moved check or another bundled change. If user-level guidance forbids attribution in work authored on the user's behalf, check the PR body and commits for that attribution and cite the guidance; existing older commits do not waive the user's explicit rule. Also inspect changed code for concrete maintenance risks: duplicated nearby explanations, identical adjacent `if` conditions, or scaffolding; loops that repeatedly call the same metric update when one counted operation would suffice; bounded polling sleeps that could use an existing or new notification, or drive the handler directly in a test; new test helpers that repeat existing builders; classifiers that must stay synchronized; unclear test-helper names; fragile references into containers that later edits could invalidate; and new state checks callers could forget. A timeout bounds a poll but does not make its observation deterministic, and lack of an existing signal does not itself make polling the best test structure. For each new lifecycle flag, enumerate every writer of the guarded state, including setup-only paths, and check whether each is guarded or relies only on a comment about call order; report a current failure, applicable rule violation, or maintenance risk meeting the shared evidence criteria. Check whether send-like paths or existing predicates omit the flag. Compare newly added or rewrapped documentation lines with the surrounding file's line-length convention, reporting a concrete violation of an applicable documented rule. When a new documentation block sits above an older comment on the same declaration, check for duplicated descriptions and whether the old comment separates the doc block from the declaration. For each new test wrapper, name the behavior it adds and check whether its class name communicates that behavior; report unclear names separately from duplicated forwarding methods. Report rule violations as findings. Currently safe code may still create a maintenance risk when exact duplicated sites, coupled state, API obligations, or a supported failure mode are demonstrated. Report at proportionate severity; omit matters of taste and generic future warnings.
2 changes: 2 additions & 0 deletions common/codex/passes/toolkit.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Toolkit pass

Find bugs and other supported issues immediately within this pass's scope.

In one pass, inspect comments and documentation for factual mismatch; test behavior and missing cases; error handling and swallowed failures; type and API design; correctness, maintainability, and security. For a new error path that discards a resource and schedules a retry, trace the stored state used by the next attempt: whether failure clears or changes it, whether repeated retries can succeed without external change, and what happens as existing resources expire. Scheduling another retry is not itself recovery, but retaining requested state can be intentional. Report the mechanism only when its consequence violates an applicable contract; do not invent a requirement to roll back a failed request or admit a resource with the wrong configuration. For a changed error log level, compare base and head and check whether repeated failures remain visible to operators or callers at default verbosity; report the visibility consequence separately from a commit-message mismatch. Compare a new error variant's display text and doc comment with every path that can produce it, not just the first call site. For changed tests, check that assertions enforce the claimed behavior, including whether they already hold before the changed action, every wait has a failure deadline, and cleanup still runs after an assertion or unexpected exception. A wait without a deadline and cleanup skipped by an exception are distinct hazards even when both can hang the test; cite both mechanisms. Map each changed production branch and client/server path to an actual test stimulus; do not count a test merely because it calls the surrounding method. When a test creates fixtures for both sides, check whether each captured hook is actually invoked and asserted. For a newly added override, compare the base declaration and nearby implementations under supported compile-time guards; a method that overrides only in the default configuration can leave another configuration abstract or fail to compile. For a new wait on queued work, trace every operation needed to settle that work, including peer I/O and shutdown or abort actions sequenced after the wait. Compare base and head ordering before concluding an older flush had the same liveness behavior. For changed docs and test comments, compare the stated consequence or platform coverage with the actual error, assertion, abort, or supported build path. A process-terminating assertion is materially more severe than an ordinary recoverable error; flag documentation that hides that consequence. For a comment claiming no other path waits on a future or state member, inspect later reads of that member. Inspect comments claiming coverage on a particular architecture against source constraints and build configuration; if read-only evidence is insufficient, mark the claim unproven. Check newly added comments against adjacent existing comments for duplication or contradiction. Combine overlapping checks into one finding. Report demonstrated defects, applicable rule violations, and maintenance risks meeting the shared evidence criteria.
59 changes: 32 additions & 27 deletions common/codex/review.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,34 +89,39 @@ 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.

Ask each reviewer to find issues immediately within its assigned role. Supply paths to its
pass brief (`passes/ROLE.md`), the shared reporting and inspection criteria, and selected checks;
read them before inspecting source. Do not copy their contents into the handoff.

After the initial broad inspection and candidate verification, each reviewer runs this command
exactly once in the same session, even if it found no issues:

`python3 PLUGIN_ROOT/common/codex/scripts/aspect-driver.py --pass ROLE --base BASE --head HEAD`

Use `toolkit`, `standards`, `spec`, or the selected `performance` role and the pinned merge-base
and head. Follow the printed role-specific deeper-look prompt. Reuse prior evidence; make
focused reads for new candidates or unresolved gaps without repeating the full scan or
reloading unchanged instructions. The driver only supplies this follow-up: no observation
serialization, finding-collection call, scratch files, or extra agent is needed.

Return supported findings from both looks with issue category, exact claim, changed line,
source-backed trigger and consequence, base/head distinction, and contrary evidence.
Include required tables and a compact coverage note for each applicable check: evidence plus
a finding, supported no-finding conclusion, or unexamined gap. There is no finding quota.
If Spec has no meaningful specification, return `No spec available`. Unfinished checks remain
coverage gaps; do not drop low-severity verified facts.

Each handoff must explicitly forbid edits, tests, builds, linters, formatters, posting,
model subprocesses, further agents, and Git checkout/switch/reset/stash/worktree.
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 driver workflow;
do not claim that the follow-up completed. Do not add another discovery round
in the parent; its remaining work is verification and reconciliation below.

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
Expand Down
47 changes: 47 additions & 0 deletions common/codex/scripts/aspect-driver.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env python3
"""Print one role-specific deeper-look prompt for an ongoing native review."""
import argparse
import re
import subprocess
import sys


PROMPTS = {
"toolkit": "Look deeper for additional bugs and other supported issues beyond your first candidates. Challenge boundary inputs, failure and cleanup paths, ownership, and async ordering; check whether tests can pass while the claimed behavior is broken.",
"standards": "Look deeper for additional repository-rule violations or evidenced maintenance risks. Check overlooked rule scope and exceptions, duplicated sites, and state or API obligations across callers. Support each concern with the exact applicable rule or demonstrated coordination invariant.",
"spec": "Look deeper for additional gaps in the explicit promises. Challenge failure, completion, and alternate input paths; check whether tests distinguish the promised behavior. Stay within the stated conditions and do not invent requirements. If no meaningful specification exists, retain No spec available.",
"performance": "Look deeper for additional performance regressions. Challenge scaling, repeated work, allocation, and contention under concrete input sizes or concurrency. Support the cost difference with source evidence and the repository's performance requirements.",
}


def git(*args):
return subprocess.check_output(["git", *args], text=True, stderr=subprocess.PIPE).strip()


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")
return (PROMPTS[args.role] + "\n\n"
"Take this deeper look once in the same session, whether or not you found issues initially. Reuse inspected evidence; make only focused reads for new candidates or unresolved gaps. Preserve earlier supported findings and consider contrary evidence. There is no finding quota; no additional issues is valid. Return findings and coverage gaps to the parent after this follow-up.")


def main():
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--pass", dest="role", choices=PROMPTS, required=True)
parser.add_argument("--base", required=True)
parser.add_argument("--head", required=True)
args = parser.parse_args()
try:
print(drive(args))
except (ValueError, subprocess.CalledProcessError) as exc:
print(f"aspect-driver: {exc}", file=sys.stderr)
return 1
return 0


if __name__ == "__main__":
sys.exit(main())
Loading
Loading