From 7f6c7683632edbed4a5d077e5c12210cb491df84 Mon Sep 17 00:00:00 2001 From: goonobu-dot Date: Mon, 15 Jun 2026 20:17:19 +0900 Subject: [PATCH] feat: add Codex change review command --- CHANGELOG.md | 2 + README.md | 14 +- .../plans/2026-06-15-review-command.md | 51 +++++ examples/CODEX_REVIEW.example.md | 36 +++ src/codex_maintainer_kit/cli.py | 21 ++ src/codex_maintainer_kit/review.py | 207 ++++++++++++++++++ tests/test_cli.py | 15 ++ tests/test_review.py | 64 ++++++ 8 files changed, 408 insertions(+), 2 deletions(-) create mode 100644 docs/superpowers/plans/2026-06-15-review-command.md create mode 100644 examples/CODEX_REVIEW.example.md create mode 100644 src/codex_maintainer_kit/review.py create mode 100644 tests/test_review.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 144bb28..052effa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add `codex-maintainer-kit review` for human maintainer review briefs from current working-tree changes. + ## 0.2.0 - 2026-06-15 - Add `codex-maintainer-kit audit` for OSS maintenance health scoring and prioritized next actions. diff --git a/README.md b/README.md index e1d45a6..54613f5 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,7 @@ Codex can help with that work, but it needs clear repository context and human r - Creates starter maintainer files with `init`. - Turns readiness gaps into `CODEX_TASKS.md`, JSON, and GitHub issue Markdown files with `tasks`. - Adds suggested labels, verification commands, and maintainer review checklists to generated issue drafts. +- Generates `CODEX_REVIEW.md` from the current working-tree changes so maintainers can review AI-assisted diffs before merge. ## Installation @@ -74,6 +75,12 @@ Generate Codex-ready maintenance tasks: codex-maintainer-kit tasks /path/to/repo --output CODEX_TASKS.md ``` +Generate a human maintainer review brief for current changes: + +```bash +codex-maintainer-kit review /path/to/repo --output CODEX_REVIEW.md +``` + Generate machine-readable tasks: ```bash @@ -129,6 +136,7 @@ See: - [examples/MAINTAINER_BRIEF.generated.md](examples/MAINTAINER_BRIEF.generated.md), generated from this repository - [examples/CODEX_TASKS.example.md](examples/CODEX_TASKS.example.md) - [examples/CODEX_TASKS.generated.md](examples/CODEX_TASKS.generated.md), generated from this repository +- [examples/CODEX_REVIEW.example.md](examples/CODEX_REVIEW.example.md) - [examples/codex-maintainer-kit.toml](examples/codex-maintainer-kit.toml) - [schema/codex-tasks.schema.json](schema/codex-tasks.schema.json) @@ -159,8 +167,9 @@ Codex Maintainer Kit focuses on a narrower workflow: creating a practical mainte 4. Run `codex-maintainer-kit tasks`. 5. Convert the generated task file or issue drafts into scoped maintenance work. 6. Ask Codex to make the smallest useful change. -7. Run tests and inspect the diff. -8. Merge only after human review. +7. Run `codex-maintainer-kit review` to create a focused review brief for the current diff. +8. Run tests and inspect the diff. +9. Merge only after human review. ## Development @@ -176,6 +185,7 @@ Run the CLI without installing: PYTHONPATH=src python3 -m codex_maintainer_kit.cli brief . --output /tmp/maintainer-brief.md PYTHONPATH=src python3 -m codex_maintainer_kit.cli audit . --output /tmp/oss-maintenance-audit.md PYTHONPATH=src python3 -m codex_maintainer_kit.cli tasks . --output /tmp/codex-tasks.md +PYTHONPATH=src python3 -m codex_maintainer_kit.cli review . --output /tmp/codex-review.md ``` ## License diff --git a/docs/superpowers/plans/2026-06-15-review-command.md b/docs/superpowers/plans/2026-06-15-review-command.md new file mode 100644 index 0000000..ede5e42 --- /dev/null +++ b/docs/superpowers/plans/2026-06-15-review-command.md @@ -0,0 +1,51 @@ +# Review Command Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a `review` command that turns the current working-tree changes into a human maintainer review brief. + +**Architecture:** Reuse `scan_repository()` for changed files and repository hints. Add `review.py` for risk classification, checklist generation, and Markdown rendering. Wire the CLI in `cli.py` with optional configured verification commands. + +**Tech Stack:** Python 3.9+, argparse, pytest. + +--- + +### Task 1: Review Report Contract + +**Files:** +- Create: `tests/test_review.py` +- Modify: `tests/test_cli.py` + +- [ ] Write failing tests for clean repositories, source+test changes, workflow/security-sensitive changes, Markdown rendering, and CLI file output. +- [ ] Run targeted tests and confirm they fail because `codex_maintainer_kit.review` and the CLI command do not exist. + +### Task 2: Review Implementation + +**Files:** +- Create: `src/codex_maintainer_kit/review.py` +- Modify: `src/codex_maintainer_kit/cli.py` + +- [ ] Add dataclasses for changed-file review items and review reports. +- [ ] Classify changed files into docs, tests, source, config, CI, and security-sensitive groups. +- [ ] Generate a risk level, focused checklist, verification command, and Codex review prompt. +- [ ] Add `codex-maintainer-kit review` with optional `--output`. + +### Task 3: Public Documentation + +**Files:** +- Modify: `README.md` +- Modify: `CHANGELOG.md` +- Create: `examples/CODEX_REVIEW.example.md` + +- [ ] Document the `review` command and where it fits in the maintainer workflow. +- [ ] Add an example review report that demonstrates a realistic changed-file set. +- [ ] Update changelog with the unreleased review feature. + +### Task 4: Verification + +**Files:** +- All changed files. + +- [ ] Run `python3 -m pytest -p no:cacheprovider tests -q`. +- [ ] Run `codex-maintainer-kit review` against this repository. +- [ ] Run `git diff --check` and inspect `git status --short`. diff --git a/examples/CODEX_REVIEW.example.md b/examples/CODEX_REVIEW.example.md new file mode 100644 index 0000000..662b178 --- /dev/null +++ b/examples/CODEX_REVIEW.example.md @@ -0,0 +1,36 @@ +# Codex Change Review + +Repository: `/path/to/example-repo` +Git state: `dirty` +Risk level: **high** + +3 changed file(s) detected across: ci, security-sensitive, tests. + +## Changed Files + +| File | Category | Review note | +| --- | --- | --- | +| `.github/workflows/tests.yml` | ci | Confirm the workflow does not require unexpected secrets or broad permissions. | +| `src/auth/token_store.py` | security-sensitive | Inspect auth, token, permission, or secret-handling behavior carefully. | +| `tests/test_token_store.py` | tests | Confirm tests cover behavior and would fail for a real regression. | + +## Maintainer Review Checklist + +- [ ] Read the actual diff, not only this generated summary. +- [ ] Confirm the change is scoped to one maintenance goal. +- [ ] Run the verification command or explain why it cannot be run. +- [ ] Tests were changed: confirm they would fail for the bug or missing behavior. +- [ ] CI or security-sensitive files changed: confirm permissions, secrets, tokens, and external calls. +- [ ] Do not auto-merge AI-generated changes without human review. + +## Verification Commands + +- `python3 -m pytest -p no:cacheprovider tests -q` + +## Suggested Codex Review Prompt + +Review the changed files above as an OSS maintainer. Focus on correctness, scope control, tests, documentation accuracy, security-sensitive changes, and whether the diff is small enough for human review. Do not approve the change unless the verification commands are practical and the remaining risks are named. + +## Human Review Rule + +This report is a review aid, not an approval. A human maintainer must inspect the actual diff before merge. diff --git a/src/codex_maintainer_kit/cli.py b/src/codex_maintainer_kit/cli.py index ec4b49c..78d2cdc 100644 --- a/src/codex_maintainer_kit/cli.py +++ b/src/codex_maintainer_kit/cli.py @@ -7,6 +7,7 @@ from codex_maintainer_kit.audit import build_audit_report, render_audit_markdown from codex_maintainer_kit.config import load_config from codex_maintainer_kit.renderer import render_maintenance_brief +from codex_maintainer_kit.review import build_review_report, render_review_markdown from codex_maintainer_kit.scanner import scan_repository from codex_maintainer_kit.tasks import build_tasks, render_issue_markdown, render_tasks_json, render_tasks_markdown @@ -63,6 +64,8 @@ def main(argv: list[str] | None = None) -> int: return _brief(args) if args.command == "init": return _init(args) + if args.command == "review": + return _review(args) if args.command == "tasks": return _tasks(args) @@ -90,6 +93,10 @@ def _build_parser() -> argparse.ArgumentParser: init.add_argument("--dry-run", action="store_true", help="List files that would be written.") init.add_argument("--force", action="store_true", help="Overwrite existing files.") + review = subparsers.add_parser("review", help="Generate a human maintainer review brief for current changes.") + review.add_argument("repo", nargs="?", default=".", help="Repository path to inspect.") + review.add_argument("--output", "-o", help="Write Markdown to this file instead of stdout.") + tasks = subparsers.add_parser("tasks", help="Generate Codex-ready maintenance tasks.") tasks.add_argument("repo", nargs="?", default=".", help="Repository path to inspect.") tasks.add_argument("--output", "-o", help="Write task output to this file instead of stdout.") @@ -142,6 +149,20 @@ def _init(args: argparse.Namespace) -> int: return 0 +def _review(args: argparse.Namespace) -> int: + scan = scan_repository(args.repo) + config = load_config(scan.root) + report = build_review_report(scan, verification_command=config.verification_command) + markdown = render_review_markdown(report) + if args.output: + output = Path(args.output) + output.parent.mkdir(parents=True, exist_ok=True) + output.write_text(markdown, encoding="utf-8") + else: + print(markdown) + return 0 + + def _tasks(args: argparse.Namespace) -> int: scan = scan_repository(args.repo) config = load_config(scan.root) diff --git a/src/codex_maintainer_kit/review.py b/src/codex_maintainer_kit/review.py new file mode 100644 index 0000000..0ee09ae --- /dev/null +++ b/src/codex_maintainer_kit/review.py @@ -0,0 +1,207 @@ +from __future__ import annotations + +from dataclasses import dataclass + +from codex_maintainer_kit.scanner import RepositoryScan + + +@dataclass(frozen=True) +class ChangedFileReview: + path: str + category: str + review_note: str + + +@dataclass(frozen=True) +class ReviewReport: + repository: str + git_state: str + risk_level: str + summary: str + changed_files: list[ChangedFileReview] + review_checklist: list[str] + verification_commands: list[str] + + +def build_review_report(scan: RepositoryScan, verification_command: str | None = None) -> ReviewReport: + changed_files = [_classify_changed_file(path) for path in scan.git_state.changed_files] + risk_level = _risk_level(changed_files) + review_checklist = _review_checklist(changed_files) + verification_commands = _verification_commands(scan, verification_command) + + return ReviewReport( + repository=str(scan.root), + git_state=scan.git_state.status, + risk_level=risk_level, + summary=_summary(changed_files), + changed_files=changed_files, + review_checklist=review_checklist, + verification_commands=verification_commands, + ) + + +def render_review_markdown(report: ReviewReport) -> str: + lines = [ + "# Codex Change Review", + "", + f"Repository: `{report.repository}`", + f"Git state: `{report.git_state}`", + f"Risk level: **{report.risk_level}**", + "", + report.summary, + "", + "## Changed Files", + "", + ] + + if report.changed_files: + lines.extend(["| File | Category | Review note |", "| --- | --- | --- |"]) + for item in report.changed_files: + lines.append(f"| `{item.path}` | {item.category} | {item.review_note} |") + else: + lines.append("No changed files detected.") + + lines.extend(["", "## Maintainer Review Checklist", ""]) + lines.extend(f"- [ ] {item}" for item in report.review_checklist) + + lines.extend(["", "## Verification Commands", ""]) + lines.extend(f"- `{command}`" for command in report.verification_commands) + + lines.extend( + [ + "", + "## Suggested Codex Review Prompt", + "", + "Review the changed files above as an OSS maintainer. Focus on correctness, scope control, tests, documentation accuracy, security-sensitive changes, and whether the diff is small enough for human review. Do not approve the change unless the verification commands are practical and the remaining risks are named.", + "", + "## Human Review Rule", + "", + "This report is a review aid, not an approval. A human maintainer must inspect the actual diff before merge.", + "", + ] + ) + return "\n".join(lines) + + +def _classify_changed_file(path: str) -> ChangedFileReview: + normalized = path.lower() + if _is_ci_path(normalized): + return ChangedFileReview(path, "ci", "Confirm the workflow does not require unexpected secrets or broad permissions.") + if _is_security_sensitive_path(normalized): + return ChangedFileReview(path, "security-sensitive", "Inspect auth, token, permission, or secret-handling behavior carefully.") + if _is_test_path(normalized): + return ChangedFileReview(path, "tests", "Confirm tests cover behavior and would fail for a real regression.") + if _is_docs_path(normalized): + return ChangedFileReview(path, "docs", "Confirm documentation matches shipped behavior and does not overpromise.") + if _is_config_path(normalized): + return ChangedFileReview(path, "config", "Confirm dependency, package, release, or tool configuration changes are intentional.") + if _is_source_path(normalized): + return ChangedFileReview(path, "source", "Review behavior, edge cases, and whether tests cover the changed path.") + return ChangedFileReview(path, "other", "Confirm the change is expected and belongs in this maintenance task.") + + +def _risk_level(changed_files: list[ChangedFileReview]) -> str: + categories = {item.category for item in changed_files} + if not changed_files: + return "low" + if categories & {"ci", "security-sensitive"}: + return "high" + if categories & {"source", "config"}: + return "medium" + return "low" + + +def _summary(changed_files: list[ChangedFileReview]) -> str: + if not changed_files: + return "No changed files detected." + categories = sorted({item.category for item in changed_files}) + return f"{len(changed_files)} changed file(s) detected across: {', '.join(categories)}." + + +def _review_checklist(changed_files: list[ChangedFileReview]) -> list[str]: + if not changed_files: + return ["Confirm the working tree is clean before starting new maintenance work."] + + categories = {item.category for item in changed_files} + checklist = [ + "Read the actual diff, not only this generated summary.", + "Confirm the change is scoped to one maintenance goal.", + "Run the verification command or explain why it cannot be run.", + ] + if "source" in categories: + checklist.append("Source files changed: confirm behavior, edge cases, and tests.") + if "tests" in categories: + checklist.append("Tests were changed: confirm they would fail for the bug or missing behavior.") + if "docs" in categories: + checklist.append("Docs changed: confirm examples and claims match the current code.") + if "config" in categories: + checklist.append("Config changed: confirm dependencies, package metadata, and release settings are intentional.") + if categories & {"ci", "security-sensitive"}: + checklist.append("CI or security-sensitive files changed: confirm permissions, secrets, tokens, and external calls.") + checklist.append("Do not auto-merge AI-generated changes without human review.") + return checklist + + +def _verification_commands(scan: RepositoryScan, verification_command: str | None) -> list[str]: + if verification_command: + return [verification_command] + if scan.files.get("tests", False): + if "python" in scan.project_hints: + return ["python3 -m pytest -p no:cacheprovider tests -q"] + if "javascript" in scan.project_hints: + return ["npm test"] + if "go" in scan.project_hints: + return ["go test ./..."] + if "rust" in scan.project_hints: + return ["cargo test"] + if "ruby" in scan.project_hints: + return ["bundle exec rake test"] + if "java" in scan.project_hints: + return ["mvn test"] + if "swift" in scan.project_hints: + return ["swift test"] + return ["No automated test command detected. Perform manual review and add tests before risky changes."] + + +def _is_ci_path(path: str) -> bool: + return path.startswith(".github/workflows/") or path in {".gitlab-ci.yml", "azure-pipelines.yml", ".circleci/config.yml"} + + +def _is_security_sensitive_path(path: str) -> bool: + markers = ["auth", "token", "secret", "credential", "permission", "security", ".env"] + return any(marker in path for marker in markers) + + +def _is_test_path(path: str) -> bool: + return path.startswith(("tests/", "test/", "__tests__/", "spec/")) or any( + marker in path for marker in ["test_", "_test.", ".test.", ".spec."] + ) + + +def _is_docs_path(path: str) -> bool: + return path.endswith((".md", ".rst", ".txt")) or path.startswith(("docs/", "examples/")) + + +def _is_config_path(path: str) -> bool: + names = [ + "pyproject.toml", + "package.json", + "package-lock.json", + "pnpm-lock.yaml", + "yarn.lock", + "cargo.toml", + "go.mod", + "go.sum", + "gemfile", + "composer.json", + "pom.xml", + "build.gradle", + "setup.py", + ] + return path in names or path.endswith((".toml", ".yaml", ".yml", ".json", ".ini", ".cfg")) + + +def _is_source_path(path: str) -> bool: + return path.startswith(("src/", "lib/", "app/")) or path.endswith( + (".py", ".js", ".ts", ".tsx", ".jsx", ".go", ".rs", ".rb", ".php", ".java", ".cs", ".swift") + ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 55378a3..b759668 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -56,6 +56,21 @@ def test_audit_command_writes_markdown_file(tmp_path: Path) -> None: assert "## Prioritized Next Actions" in text +def test_review_command_writes_markdown_file(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + (repo / ".git").mkdir() + output = tmp_path / "CODEX_REVIEW.md" + + exit_code = main(["review", str(repo), "--output", str(output)]) + + assert exit_code == 0 + text = output.read_text(encoding="utf-8") + assert text.startswith("# Codex Change Review") + assert "Risk level:" in text + assert "## Maintainer Review Checklist" in text + + def test_tasks_command_writes_json_file(tmp_path: Path) -> None: repo = tmp_path / "repo" repo.mkdir() diff --git a/tests/test_review.py b/tests/test_review.py new file mode 100644 index 0000000..d85bee2 --- /dev/null +++ b/tests/test_review.py @@ -0,0 +1,64 @@ +from pathlib import Path +from typing import Optional + +from codex_maintainer_kit.review import build_review_report, render_review_markdown +from codex_maintainer_kit.scanner import GitState, RepositoryScan + + +def _scan(changed_files: list[str], project_hints: Optional[list[str]] = None, has_tests: bool = True) -> RepositoryScan: + return RepositoryScan( + root=Path("/repo/demo"), + files={ + "readme": True, + "license": True, + "contributing": True, + "code_of_conduct": True, + "security": True, + "changelog": True, + "agents": True, + "issue_templates": True, + "ci": True, + "tests": has_tests, + }, + project_hints=project_hints or ["python"], + git_state=GitState(status="dirty" if changed_files else "clean", changed_files=changed_files), + ) + + +def test_build_review_report_handles_clean_repository() -> None: + report = build_review_report(_scan([])) + + assert report.risk_level == "low" + assert report.summary == "No changed files detected." + assert report.changed_files == [] + assert report.review_checklist == ["Confirm the working tree is clean before starting new maintenance work."] + + +def test_build_review_report_classifies_source_and_test_changes() -> None: + report = build_review_report(_scan(["src/demo.py", "tests/test_demo.py", "README.md"])) + + assert report.risk_level == "medium" + assert [item.category for item in report.changed_files] == ["source", "tests", "docs"] + assert "python3 -m pytest -p no:cacheprovider tests -q" in report.verification_commands + assert any("Tests were changed" in item for item in report.review_checklist) + + +def test_build_review_report_flags_ci_and_security_sensitive_changes() -> None: + report = build_review_report(_scan([".github/workflows/tests.yml", "src/auth/token_store.py"])) + + assert report.risk_level == "high" + assert [item.category for item in report.changed_files] == ["ci", "security-sensitive"] + assert any("CI or security-sensitive files changed" in item for item in report.review_checklist) + + +def test_render_review_markdown_is_human_review_ready() -> None: + report = build_review_report(_scan(["src/demo.py", "pyproject.toml"])) + + markdown = render_review_markdown(report) + + assert markdown.startswith("# Codex Change Review") + assert "Risk level: **medium**" in markdown + assert "## Changed Files" in markdown + assert "| `src/demo.py` | source |" in markdown + assert "## Maintainer Review Checklist" in markdown + assert "## Suggested Codex Review Prompt" in markdown