Skip to content
Open
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
47 changes: 37 additions & 10 deletions agents/code-reviewer.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,45 @@
---
name: code-reviewer
description: Reviews code diffs against section plans. Used by /deep-implement for section review.
description: Adversarial code reviewer for /deep-implement section diffs. Finds issues that tests and linters cannot catch.
tools: Read, Grep, Glob
model: inherit
---

You are a code reviewer for the deep-implement workflow.
You are an adversarial code reviewer. You HATE this code. Your job is to find every issue that automated tests and linters cannot catch.

You will receive two file paths:
1. **Section plan** - The specification describing what should be built and why
2. **Diff file** - The actual code changes as a result of the section plan
2. **Diff file** - The actual code changes

Read both files. Reconcile the implementation and the plan.
Read both files. Then tear the implementation apart.

Pretend you're a senior architect who hates this implementation. What would you criticize? What is missing?
## Universal Checks (all runtimes)

1. **Security** — injection, hardcoded secrets, unsafe deserialization, path traversal, missing auth checks
2. **Error handling** — swallowed errors, missing nil/null checks, errors that propagate without context
3. **Contract compliance** — does the implementation match the plan? Missing requirements? Extra scope?
4. **Scope violations** — files or features not described in the section plan
5. **Missing tests** — untested error paths, edge cases, missing assertions for key behavior
6. **Concurrency** — race conditions, shared mutable state without synchronization
7. **Resource leaks** — unclosed handles, missing defers/finally, connection pool exhaustion

## Go-Specific Checks

When reviewing Go code, also check:

- **Hex-arch imports**: inner layers MUST NOT import outer layers (domain → ports → service → handler → adapter). Flag any `import` of an outer package from an inner one.
- **File sizes by layer**: ports ≤100 lines, domain ≤250, service/adapter ≤300, handler ≤250, test ≤500. Flag violations.
- **Function length**: non-test ≤75 lines, test ≤150 lines. Flag violations.
- **Error wrapping**: errors crossing package boundaries must wrap with `fmt.Errorf("context: %w", err)`. Flag bare `return err` across boundaries.
- **Prohibited patterns**: `panic()` outside init, `os.Exit()` outside main, `log.Fatal` in library code.

## Python-Specific Checks

When reviewing Python code, also check:

- **Type hints** — public functions without type annotations
- **Resource management** — file/connection handles without `with` or explicit close
- **Circular imports** — import patterns that will fail at runtime

## Output Format

Expand All @@ -28,8 +54,9 @@ Return a JSON object with this EXACT structure:

## Rules

1. Return ONLY the JSON object - no preamble or explanation
2. Be specific - reference exact line numbers/function names
3. Prioritize high-severity issues (security, data loss, crashes)
4. Check implementation against the plan's requirements
5. If no issues found, return that the implementation looked good
1. Return ONLY the JSON object — no preamble or explanation
2. Be specific — reference exact file paths, line numbers, and function names
3. **Severity order**: security > correctness > contract compliance > architecture > style
4. Every finding must include: what's wrong, where (file:line or function), and why it matters
5. If the code is genuinely solid, say so — but look harder first
6. Do NOT flag style preferences that linters already catch (formatting, naming conventions)
2 changes: 1 addition & 1 deletion hooks/hooks.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"hooks": [
{
"type": "command",
"command": "uv run ${CLAUDE_PLUGIN_ROOT}/scripts/hooks/capture-session-id.py"
"command": "uv run --script ${CLAUDE_PLUGIN_ROOT}/scripts/hooks/capture-session-id.py"
}
]
}
Expand Down
127 changes: 126 additions & 1 deletion scripts/checks/setup_implementation_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
sys.path.insert(0, str(Path(__file__).parent.parent.parent))

from scripts.lib.config import load_session_config, save_session_config, create_session_config
from scripts.lib.sections import parse_manifest_block, parse_project_config_block, validate_section_file, get_completed_sections
from scripts.lib.sections import parse_manifest_block, parse_project_config_block, parse_section_concerns, sort_sections_by_concern, validate_section_file, get_completed_sections
from scripts.lib.task_storage import TaskToWrite, write_tasks, build_dependency_graph, TaskStatus
from scripts.lib.task_reconciliation import TaskListContext
from scripts.lib.impl_tasks import (
Expand Down Expand Up @@ -377,6 +377,87 @@ def check_pre_commit_hooks(git_root: Path) -> dict:
}


def check_github_remote(git_root: Path) -> dict:
"""
Check for GitHub remote and parse owner/repo.

Args:
git_root: Git repository root

Returns:
{"has_remote": bool, "remote_url": str | None, "owner_repo": str | None}
"""
try:
result = subprocess.run(
["git", "remote", "get-url", "origin"],
cwd=git_root,
capture_output=True,
text=True,
)
if result.returncode == 0:
url = result.stdout.strip()
# Parse owner/repo from various URL formats:
# https://github.com/owner/repo.git
# git@github.com:owner/repo.git
# https://github.com/owner/repo
match = re.search(r"github\.com[:/]([^/]+/[^/\s]+?)(?:\.git)?$", url)
if match:
return {
"has_remote": True,
"remote_url": url,
"owner_repo": match.group(1),
}
# Non-GitHub remote
return {"has_remote": True, "remote_url": url, "owner_repo": None}
except Exception:
pass
return {"has_remote": False, "remote_url": None, "owner_repo": None}


def check_gh_cli() -> dict:
"""
Check if GitHub CLI is installed and authenticated.

Returns:
{"installed": bool, "authenticated": bool, "gh_version": str | None}
"""
# Check installation
try:
version_result = subprocess.run(
["gh", "--version"],
capture_output=True,
text=True,
)
if version_result.returncode != 0:
return {"installed": False, "authenticated": False, "gh_version": None}
except FileNotFoundError:
return {"installed": False, "authenticated": False, "gh_version": None}

# Parse version string (first line: "gh version X.Y.Z (date)")
gh_version = None
first_line = version_result.stdout.strip().split("\n")[0]
version_match = re.search(r"(\d+\.\d+\.\d+)", first_line)
if version_match:
gh_version = version_match.group(1)

# Check authentication
try:
auth_result = subprocess.run(
["gh", "auth", "status"],
capture_output=True,
text=True,
)
authenticated = auth_result.returncode == 0
except Exception:
authenticated = False

return {
"installed": True,
"authenticated": authenticated,
"gh_version": gh_version,
}


def detect_section_review_state(state_dir: Path, section_name: str) -> dict:
"""
Detect the code review state for a specific section.
Expand Down Expand Up @@ -718,6 +799,17 @@ def main():
sections = validation["sections"]
project_config = validation["project_config"]

# Concern-based ordering (opt-in via PROJECT_CONFIG)
concern_ordering = project_config.get("concern_ordering", "").lower() == "true"
index_content = (sections_dir / "index.md").read_text()
section_concerns = {}
execution_order = list(sections) # default: manifest order
if concern_ordering:
section_concerns = parse_section_concerns(index_content)
if section_concerns:
execution_order = sort_sections_by_concern(sections, section_concerns)
sections = execution_order

# State directory (sibling to sections) for session config and reviews
state_dir = sections_dir.parent / "implementation"

Expand Down Expand Up @@ -745,6 +837,21 @@ def main():
# Check pre-commit hooks
pre_commit = check_pre_commit_hooks(git_root)

# Check GitHub availability
github_remote = check_github_remote(git_root)
gh_cli = check_gh_cli()
github_available = (
github_remote.get("owner_repo") is not None
and gh_cli.get("installed", False)
and gh_cli.get("authenticated", False)
)
github_info = {
"available": github_available,
"owner_repo": github_remote.get("owner_repo"),
"gh_installed": gh_cli.get("installed", False),
"gh_authenticated": gh_cli.get("authenticated", False),
}

# Infer session state
state = infer_session_state(sections_dir, state_dir, git_root)

Expand Down Expand Up @@ -785,13 +892,27 @@ def main():
session_id_matched = (context_session_id == env_session_id)

# Context values for task generation
# GitHub state is populated later by SKILL.md after user opt-in;
# on resume, read from existing config
existing_config = load_session_config(state_dir) if state["mode"] == "resume" else None
github_enabled = "false"
github_issue = "none"
if existing_config and existing_config.get("github", {}).get("enabled"):
github_enabled = "true"
issue_num = existing_config["github"].get("issue_number")
if issue_num:
github_issue = str(issue_num)

context_values = {
"plugin_root": str(plugin_root),
"sections_dir": str(sections_dir),
"target_dir": str(target_dir),
"state_dir": str(state_dir),
"runtime": project_config["runtime"],
"test_command": project_config["test_command"],
"github_enabled": github_enabled,
"github_issue": github_issue,
"concern_ordering": str(concern_ordering).lower(),
}

# Generate implementation tasks
Expand Down Expand Up @@ -834,11 +955,15 @@ def main():
"dirty_files": working_tree["dirty_files"],
"commit_style": commit_style,
"pre_commit": pre_commit,
"github": github_info,
"project_config": project_config,
"sections": sections,
"completed_sections": state["completed_sections"],
"resume_from": state["resume_from"],
"resume_section_state": state.get("resume_section_state"),
"concern_ordering": concern_ordering,
"section_concerns": section_concerns,
"execution_order": execution_order,
"tasks_written": write_result.tasks_written if write_result else 0,
"task_write_error": task_write_error,
# Session ID diagnostics
Expand Down
Loading