| name | review-changes |
| description | Perform a deep review of a changeset — invoke when reviewing a set of code changes spanning more than one file, before committing; use review-python-file for a single file in depth. Routes each file to its type-specific review skill, then adds the cross-cutting checks (unrequested surface, design consistency, DRY, coverage) no single-file review can catch |
Review like a senior engineer. Every finding must answer three questions in concrete terms: what is wrong, what is better, why the change is worth its cost. All topics are fair game — correctness, security, design, duplication, clarity, naming, structure, tests. Be conscious of suggestions that do not serve a purpose.
Review the changeset described in the prompt. Common forms:
| Prompt | Git command to get changed files |
|---|
| "uncommitted changes" (default) | git diff HEAD --name-only |
| "changes on this branch" / "vs main" | git diff main...HEAD --name-only |
| "staged changes" | git diff --cached --name-only |
| "the last commit" | git diff HEAD~1 HEAD --name-only |
| "changes vs <branch>" | git diff <branch>...HEAD --name-only |
If no changeset is specified, default to uncommitted changes.
Steps
-
Identify changed files: Run the appropriate git command above to get the full list of changed files. If the changeset touches an area you have not worked in, load ref-project-reference first for the module map — what each package owns and which types cross package boundaries.
-
Review each file based on its type:
| File type | Skill to apply |
|---|
.py | review-python-file |
CLI help-bearing files (cli/_commands/*.py, cli/_help.py, cli/_manifest.py, cli/_errors.py) | review-python-file plus cli-help-accuracy (the surfaced help, error-code registry, and output schema must match the code and docs/CLI.md) |
MCP tool signatures (mcp_systems_server/_tools/*.py) or tool-wrapping CLI commands (cli/_commands/*.py setting wraps_tool/wraps_tools) | review-python-file plus run uv run --extra test pytest tests/cli/test_tool_wrapper_drift.py -q and apply ref-cli-tool-wrapping — a changed tool signature must be reflected in its wrapper's flags / intentionally_unsupported, and vice versa |
.md, .rst | docs-accuracy |
Pydantic config (config/schema/*.py, config/tree.py), *.json5 examples under config-samples/ai/config/, docs/CONFIGURATION.md | ref-configuration-conventions |
docs/ENV.md | ref-configuration-conventions plus docs-accuracy — the canonical environment-variable inventory; verify every entry still matches its reader in the code |
.agents/skills/**/SKILL.md, AGENTS.md | skill-review plus run uv run pytest tests/agents/test_skills_catalog.py — it pins the frontmatter contract, the README row, and every section citation and canonical-implementation pointer across the catalog, none of which markdownlint checks |
.json, .json5, .yaml, .toml (config files outside the schemas above) |
-
Cross-cutting review — assess the changeset as a whole:
- Unrequested surface: List every new environment variable, config field, CLI flag, error code, output field, MCP tool, noun or verb, exit code, public export, config file kind — or any other user-visible contract — in the changeset, and name the requirement each traces to. A surface entailed by an approved change (a requested command's own flags and error codes) traces to that request and is in scope. Flag any that trace to none (
AGENTS.md Scope discipline). A well-executed unrequested feature — typed, tested, documented, fully covered — passes every other check in this list, so this is the only one that catches it.
- New environment variable: Scan the whole diff — any file, including tests and docs — for an
os.environ read, a click envvar=, or a new variable name. Each one must be explicitly requested and must carry its docs/ENV.md entry in the same changeset. A JSON field plus ${env:VAR} templating is the default answer; a new variable is a product decision, not an implementation detail. Apply ref-configuration-conventions, plus cli-command-add's anti-patterns for CLI code.
- Design consistency: Names, abstractions, and patterns introduced in one file match what siblings already use. Flag any new pattern that supersedes an existing one without also retiring the old one.
- Code changes that stale a skill: A changeset that adds, renames, or moves a module, or changes a symbol a skill names as a canonical implementation, staled a skill even when no skill file is in the diff. Check
ref-project-reference (the module map) and grep the catalog for the old path or symbol — grep -rn '<old-name>' .agents/ AGENTS.md. This is the case the per-file routing table cannot catch, because the skill file itself is unchanged.
- DRY across files: Identify any block of substantive logic that appears in two or more changed files; either extract to a shared helper in this changeset or flag for follow-up.
- Test coverage of new code paths: Every new branch, error path, and public function in the changeset has at least one test that exercises it. List by file any new code paths with zero test coverage.
- Public-surface contract: Every new or modified
__all__, __init__.py export, MCP tool registration, CLI command, error code, or config field has its corresponding test pinning the surface (per tests-improve step 1 for ; per step 4 for tool registration; per for CLI; per for config).
Do not remove TODOs without a very good reason.
