diff --git a/README.md b/README.md index 2bb6ae43..2db4e096 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ code-review-graph install # auto-detects and configures all supported p code-review-graph build # parse your codebase ``` -One command sets up everything. `install` detects which AI coding tools you have, writes the correct MCP configuration for each one, installs platform-native hooks/skills where supported, and injects graph-aware instructions into your platform rules. It auto-detects whether you installed via `uvx` or `pip`/`pipx` and generates the right config. Restart your editor/tool after installing. +One command sets up everything. `install` detects which AI coding tools you have, writes the correct MCP configuration for each one, installs platform-native hooks/skills where supported, and injects graph-aware instructions into your platform rules. It auto-detects whether you installed via `uvx` or `pip`/`pipx` and generates the right config. Restart or refresh your editor/tool after installing so its MCP and Skill inventory is reloaded.

One Install, Every Platform: auto-detects Codex, Claude Code, CodeBuddy Code, Cursor, Windsurf, Zed, Continue, OpenCode, Antigravity, Gemini CLI, Qwen, Qoder, Kiro, GitHub Copilot, and GitHub Copilot CLI @@ -88,6 +88,14 @@ code-review-graph install --platform copilot-cli # configure only GitHub Copilo code-review-graph install --platform codebuddy # configure only CodeBuddy Code ``` +For Codex, the installer also places a global `code-review-graph` Skill under +`$CODEX_HOME/skills/code-review-graph/` (default: +`~/.codex/skills/code-review-graph/`). It prefers CRG MCP tools and falls back +to the bundled read-only CLI when MCP is unavailable; a missing, empty, or +stale graph never triggers an implicit build or update. WSL and Windows Codex +runtimes have separate `CODEX_HOME` directories and tool inventories, so +install and refresh the runtime that will execute the task. + Requires Python 3.10+. For the best experience, install [uv](https://docs.astral.sh/uv/) (the MCP config will use `uvx` if available, otherwise falls back to the `code-review-graph` command directly). To remove CRG from a Git or SVN project, use the symmetric uninstall command diff --git a/code_review_graph/assets/code-review-graph/SKILL.md b/code_review_graph/assets/code-review-graph/SKILL.md new file mode 100644 index 00000000..427e76a1 --- /dev/null +++ b/code_review_graph/assets/code-review-graph/SKILL.md @@ -0,0 +1,60 @@ +--- +name: code-review-graph +description: Use the local code-review-graph knowledge graph for compact repository exploration, architecture questions, code review, debugging, dependency tracing, impact analysis, and refactoring safety checks. Use when the user mentions CRG/code-review-graph, asks to use its graph, or an existing graph can materially narrow repository work; do not invoke it repeatedly for an ordinary repository with no graph and no graph request. +--- + +# Code Review Graph + +Use CRG as a local structural index, not as a replacement for source inspection. The workflow is safe by default: read-only inspection may use an existing graph, while graph creation or maintenance always requires explicit user intent. + +## Decide whether to invoke CRG + +- Invoke this skill when the user names CRG/code-review-graph, asks for graph-backed exploration/review/debugging/impact analysis, or the repository already contains a usable CRG graph and graph context would reduce broad scanning. +- For an ordinary repository with no graph and no request to use or build CRG, do not call the CLI or MCP merely to keep checking. A single first-task health check is allowed only after this skill has been triggered by the conditions above. +- “Use the graph” and “understand this code” do not authorize building a missing graph. Ask before any mutating CRG command. + +## Entry workflow + +1. Resolve the active repository root from the workspace or `git rev-parse --show-toplevel`. Never embed the checkout used during installation. +2. Inspect the current tool inventory for CRG MCP tools. Prefer MCP when its tools are actually exposed. +3. On the first CRG task for a repository, perform exactly one read-only health check. With MCP, use `get_minimal_context_tool(task=, repo_root=)` or the available graph-stats tool. Without MCP, run the bundled helper’s `status --json` command. Reuse the result; do not poll status on every turn. +4. If the graph is healthy and fresh, use the smallest relevant MCP query or CLI read-only query to narrow the work. If an MCP call fails, retry at most once, then use the CLI helper. +5. If the graph is missing, empty, or stale, report that once and stop using CRG for this task unless the user explicitly authorizes maintenance. Continue with the smallest useful source/test inspection. + +## Health and mutation boundary + +- A graph is not ready when its database is missing, `nodes` or `files` is zero, or `last_updated` is null. It is stale when the built commit does not match the current repository commit. +- Never run `build`, `update`, `postprocess`, `embed`, or `watch` unless the user explicitly asks to create/maintain the graph or has already granted that authority for this task. Verify status once after an authorized mutation. +- Never launch `serve` as a one-shot query or implicitly use `uvx`; `serve` is a long-running MCP process. Use the installed CLI executable for fallback queries. +- Keep all CRG operations local. Do not send credentials, private keys, or unrelated source to external services. +- Graph results only select a smaller reading scope. Read the actual implementation and relevant tests before making or reporting behavioral conclusions; if graph and source disagree, trust source. + +## Route by task + +- Explore/architecture: start with minimal context or `architecture --detail-level minimal`, then search symbols and trace only relevant callers, callees, imports, tests, communities, or flows. +- Review changes: use change detection, affected flows, impact radius, and `tests_for`; request source snippets only for changed/high-risk areas. +- Debug: search the suspected symbols/terms, trace callers and callees, inspect one relevant flow, and verify hypotheses against source, logs, and tests. +- Refactor/rename: preview impact and tests first; never apply a graph-backed refactor without the user’s explicit edit request. + +## CLI fallback + +Use the bundled read-only wrapper (replace `` with this skill’s directory): + +```bash +python3 /scripts/crg_readonly.py status --repo "" +python3 /scripts/crg_readonly.py architecture --repo "" +python3 /scripts/crg_readonly.py search "" --repo "" +python3 /scripts/crg_readonly.py query callers_of "" --repo "" +python3 /scripts/crg_readonly.py impact --repo "" +python3 /scripts/crg_readonly.py detect-changes --brief --repo "" +``` + +The wrapper resolves/validates the repository, passes arguments without shell interpolation, and exposes only read-only commands. Set `CRG_BIN` only when the installed executable has a non-standard name/path. Preserve and report its errors; do not turn a failed fallback into permission to build. + +## WSL and Windows scope + +Codex processes in WSL and Windows have separate homes and configuration/tool inventories. Resolve the `CODEX_HOME` of the runtime that is actually running the task, and restart or refresh Codex after installing a skill or MCP entry. A Windows-mounted path such as `/mnt/e/...` is not by itself a failure: verify that the selected runtime can execute the CLI and access that path. Do not assume a WSL MCP registration is visible to a Windows Codex session, or vice versa. + +## Output discipline + +State whether evidence came from CRG MCP or the CLI fallback, include the repository root and graph readiness/freshness when material, and keep graph-derived relationships concise. Do not claim CRG was used if its tools and fallback both failed. diff --git a/code_review_graph/assets/code-review-graph/agents/openai.yaml b/code_review_graph/assets/code-review-graph/agents/openai.yaml new file mode 100644 index 00000000..c6150c6e --- /dev/null +++ b/code_review_graph/assets/code-review-graph/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Code Review Graph" + short_description: "Safe graph context with CLI fallback" + default_prompt: "Use $code-review-graph to explore this repository with safe graph context." diff --git a/code_review_graph/assets/code-review-graph/scripts/crg_readonly.py b/code_review_graph/assets/code-review-graph/scripts/crg_readonly.py new file mode 100644 index 00000000..6d15194d --- /dev/null +++ b/code_review_graph/assets/code-review-graph/scripts/crg_readonly.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python3 +"""Run safe, read-only code-review-graph CLI queries for one repository.""" + +from __future__ import annotations + +import argparse +import os +import shlex +import shutil +import subprocess +import sys +from pathlib import Path + +QUERY_PATTERNS = ( + "callers_of", + "callees_of", + "imports_of", + "importers_of", + "children_of", + "tests_for", + "inheritors_of", + "file_summary", +) +SEARCH_KINDS = ("File", "Class", "Function", "Type", "Test") +READ_ONLY_COMMANDS = { + "status", + "search", + "query", + "impact", + "detect-changes", + "architecture", + "flows", + "flow", + "communities", + "community", + "large-functions", + "refactor", +} + + +def _resolve_repo(raw: str | None) -> Path: + if raw: + root = Path(raw).expanduser().resolve() + if not root.is_dir(): + raise ValueError(f"Repository directory does not exist: {root}") + return root + try: + result = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + cwd=Path.cwd(), + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=5, + check=False, + ) + except (FileNotFoundError, OSError, subprocess.TimeoutExpired) as exc: + raise ValueError("Could not resolve the repository root with Git") from exc + if result.returncode != 0 or not result.stdout.strip(): + raise ValueError("Current directory is not inside a Git repository; pass --repo") + return Path(result.stdout.strip()).expanduser().resolve() + + +def _crg_command() -> list[str]: + override = os.environ.get("CRG_BIN", "").strip() + if override: + parts = shlex.split(override) + if not parts: + raise ValueError("CRG_BIN is empty") + return parts + binary = shutil.which("code-review-graph") + if binary: + return [binary] + raise FileNotFoundError( + "code-review-graph is not on PATH; install it or set CRG_BIN to its executable" + ) + + +def _add_common(parser: argparse.ArgumentParser) -> None: + parser.add_argument("--repo", default=None, help="Repository root (auto-detected)") + + +def _build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + description="Run read-only code-review-graph queries against a repository" + ) + sub = parser.add_subparsers(dest="command", required=True) + + status = sub.add_parser("status", help="Show graph statistics as JSON") + _add_common(status) + search = sub.add_parser("search", help="Search graph entities") + search.add_argument("query") + search.add_argument("--kind", choices=SEARCH_KINDS, default=None) + search.add_argument("--limit", type=int, default=20) + _add_common(search) + query = sub.add_parser("query", help="Query graph relationships") + query.add_argument("pattern", choices=QUERY_PATTERNS) + query.add_argument("target") + _add_common(query) + impact = sub.add_parser("impact", help="Analyze change blast radius") + impact.add_argument("--files", nargs="+", default=None) + impact.add_argument("--depth", type=int, default=2) + impact.add_argument("--max-results", type=int, default=500) + impact.add_argument("--base", default="HEAD~1") + _add_common(impact) + detect = sub.add_parser("detect-changes", help="Analyze changed files") + detect.add_argument("--base", default="HEAD~1") + detect.add_argument("--brief", action="store_true") + detect.add_argument("--churn", action="store_true") + detect.add_argument("--verify", action="store_true") + _add_common(detect) + architecture = sub.add_parser("architecture", help="Show architecture overview") + architecture.add_argument( + "--detail-level", choices=("minimal", "standard"), default="minimal" + ) + _add_common(architecture) + flows = sub.add_parser("flows", help="List stored execution flows") + flows.add_argument( + "--sort", + choices=("criticality", "depth", "node_count", "file_count", "name"), + default="criticality", + ) + flows.add_argument("--limit", type=int, default=50) + flows.add_argument("--kind", default=None) + _add_common(flows) + flow = sub.add_parser("flow", help="Show one stored flow") + selector = flow.add_mutually_exclusive_group(required=True) + selector.add_argument("--id", type=int) + selector.add_argument("--name") + flow.add_argument("--source", action="store_true") + _add_common(flow) + communities = sub.add_parser("communities", help="List graph communities") + communities.add_argument( + "--sort", choices=("size", "cohesion", "name"), default="size" + ) + communities.add_argument("--min-size", type=int, default=0) + _add_common(communities) + community = sub.add_parser("community", help="Show one graph community") + selector = community.add_mutually_exclusive_group(required=True) + selector.add_argument("--id", type=int) + selector.add_argument("--name") + community.add_argument("--members", action="store_true") + _add_common(community) + large = sub.add_parser("large-functions", help="Find oversized graph nodes") + large.add_argument("--min-lines", type=int, default=50) + large.add_argument( + "--kind", choices=("Function", "Class", "File", "Test"), default=None + ) + large.add_argument("--path", default=None) + large.add_argument("--limit", type=int, default=50) + _add_common(large) + refactor = sub.add_parser("refactor", help="Preview graph-backed refactors") + refactor.add_argument("mode", choices=("rename", "dead_code", "suggest")) + refactor.add_argument("--old-name", default=None) + refactor.add_argument("--new-name", default=None) + refactor.add_argument("--kind", choices=("Function", "Class"), default=None) + refactor.add_argument("--path", default=None) + _add_common(refactor) + return parser + + +def _command_args(args: argparse.Namespace, root: Path) -> list[str]: + command = args.command + result = [command] + if command == "status": + result.append("--json") + elif command == "search": + result.append(args.query) + if args.kind: + result.extend(("--kind", args.kind)) + result.extend(("--limit", str(args.limit))) + elif command == "query": + result.extend((args.pattern, args.target)) + elif command == "impact": + if args.files: + result.extend(("--files", *args.files)) + result.extend( + ( + "--depth", + str(args.depth), + "--max-results", + str(args.max_results), + "--base", + args.base, + ) + ) + elif command == "detect-changes": + result.extend(("--base", args.base)) + for flag in ("brief", "churn", "verify"): + if getattr(args, flag): + result.append(f"--{flag}") + elif command == "architecture": + result.extend(("--detail-level", args.detail_level)) + elif command == "flows": + result.extend(("--sort", args.sort, "--limit", str(args.limit))) + if args.kind: + result.extend(("--kind", args.kind)) + elif command == "flow": + result.extend( + ("--id", str(args.id)) + if args.id is not None + else ("--name", args.name) + ) + if args.source: + result.append("--source") + elif command == "communities": + result.extend(("--sort", args.sort, "--min-size", str(args.min_size))) + elif command == "community": + result.extend( + ("--id", str(args.id)) + if args.id is not None + else ("--name", args.name) + ) + if args.members: + result.append("--members") + elif command == "large-functions": + result.extend(("--min-lines", str(args.min_lines), "--limit", str(args.limit))) + if args.kind: + result.extend(("--kind", args.kind)) + if args.path: + result.extend(("--path", args.path)) + elif command == "refactor": + result.append(args.mode) + for option, value in ( + ("--old-name", args.old_name), + ("--new-name", args.new_name), + ("--kind", args.kind), + ("--path", args.path), + ): + if value: + result.extend((option, value)) + result.extend(("--repo", str(root))) + return result + + +def main() -> int: + args = _build_parser().parse_args() + if args.command not in READ_ONLY_COMMANDS: + print(f"Unsupported command: {args.command}", file=sys.stderr) + return 2 + try: + root = _resolve_repo(args.repo) + command = _crg_command() + _command_args(args, root) + except (FileNotFoundError, ValueError) as exc: + print(f"CRG fallback unavailable: {exc}", file=sys.stderr) + return 127 + try: + completed = subprocess.run(command, cwd=str(root), check=False) + except (FileNotFoundError, OSError) as exc: + print(f"CRG fallback unavailable: {exc}", file=sys.stderr) + return 127 + return completed.returncode + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/code_review_graph/cli.py b/code_review_graph/cli.py index 68e7fb01..b39f9eed 100644 --- a/code_review_graph/cli.py +++ b/code_review_graph/cli.py @@ -311,6 +311,7 @@ def _handle_init(args: argparse.Namespace) -> None: install_codebuddy_hooks, install_codebuddy_skills, install_codex_hooks, + install_codex_skill, install_cursor_hooks, install_gemini_cli_hooks, install_gemini_cli_skills, @@ -319,8 +320,14 @@ def _handle_init(args: argparse.Namespace) -> None: install_opencode_plugin, install_qoder_skills, ) - if not skip_skills: + codex_detected = target == "codex" or ( + target == "all" and PLATFORMS["codex"]["detect"]() + ) + if codex_detected: + codex_skill_dir = install_codex_skill() + print(f"Installed Codex skill in {codex_skill_dir}") + # Claude Code skills are only relevant for Claude (or full install). if target in ("claude", "all"): skills_dir = generate_skills(repo_root) diff --git a/code_review_graph/codex_skill.py b/code_review_graph/codex_skill.py new file mode 100644 index 00000000..ca8f8a2c --- /dev/null +++ b/code_review_graph/codex_skill.py @@ -0,0 +1,248 @@ +"""Install the bundled Code Review Graph skill for Codex. + +The skill is kept as package data so ``code-review-graph install`` works from +both a source checkout and a wheel. Installation is deliberately conservative: +an existing directory that was not created by CRG is never overwritten. +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import os +import shutil +from pathlib import Path +from typing import Any + +logger = logging.getLogger(__name__) + +CODEX_SKILL_NAME = "code-review-graph" +MANIFEST_NAME = ".code-review-graph-managed.json" +_RESOURCE_ROOT = Path(__file__).resolve().parent / "assets" / "code-review-graph" + + +def codex_home() -> Path: + """Return the active Codex state directory. + + Codex defaults to ``~/.codex`` but accepts ``CODEX_HOME``. Looking up the + environment variable at call time matters for installers and tests that + select a different Codex runtime after importing this module. + """ + + configured = os.environ.get("CODEX_HOME", "").strip() + if configured: + return Path(configured).expanduser().resolve() + return (Path.home() / ".codex").resolve() + + +def codex_skill_dir() -> Path: + """Return the user-level directory where Codex discovers this skill.""" + + return codex_home() / "skills" / CODEX_SKILL_NAME + + +def bundled_skill_dir() -> Path: + """Return the read-only skill resources shipped in the Python package.""" + + return _RESOURCE_ROOT + + +def _iter_resource_files(root: Path) -> dict[str, Path]: + if not root.is_dir(): + raise FileNotFoundError(f"bundled Codex skill is missing: {root}") + files: dict[str, Path] = {} + for path in sorted(root.rglob("*")): + if path.is_file(): + files[path.relative_to(root).as_posix()] = path + if "SKILL.md" not in files: + raise FileNotFoundError(f"bundled Codex skill has no SKILL.md: {root}") + return files + + +def bundled_skill_hashes() -> dict[str, str]: + """Return SHA-256 hashes for the files shipped with the skill.""" + + return { + relative: hashlib.sha256(path.read_bytes()).hexdigest() + for relative, path in _iter_resource_files(bundled_skill_dir()).items() + } + + +def _read_manifest(skill_dir: Path) -> dict[str, str] | None: + path = skill_dir / MANIFEST_NAME + if not path.is_file(): + return None + try: + parsed: Any = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return None + files = parsed.get("files") if isinstance(parsed, dict) else None + if not isinstance(files, dict) or not all( + isinstance(key, str) + and isinstance(value, str) + and _is_safe_relative(key) + for key, value in files.items() + ): + return None + return dict(files) + + +def _is_safe_relative(relative: str) -> bool: + """Return whether a manifest path stays below the skill directory.""" + + path = Path(relative) + return ( + not path.is_absolute() + and ".." not in path.parts + and path.as_posix() == relative + and relative not in {"", "."} + ) + + +def _safe_target(root: Path, target: Path) -> bool: + """Reject symlinked path components and paths outside ``root``.""" + + try: + target.resolve(strict=False).relative_to(root.resolve(strict=False)) + except (OSError, RuntimeError, ValueError): + return False + current = target + while current != root: + try: + if current.is_symlink(): + return False + except OSError: + return False + current = current.parent + return True + + +def _write_manifest(skill_dir: Path, hashes: dict[str, str]) -> None: + payload = {"version": 1, "files": dict(sorted(hashes.items()))} + (skill_dir / MANIFEST_NAME).write_text( + json.dumps(payload, indent=2, ensure_ascii=False) + "\n", encoding="utf-8" + ) + + +def _file_hash(path: Path) -> str | None: + try: + return hashlib.sha256(path.read_bytes()).hexdigest() + except OSError: + return None + + +def install_codex_skill() -> Path: + """Install or update the bundled skill and return its destination. + + A manifest records files owned by CRG. Reinstall updates only those files + and preserves unrelated files. If a pre-existing unmarked skill conflicts + with a bundled file, installation is skipped instead of clobbering it. + """ + + source_files = _iter_resource_files(bundled_skill_dir()) + destination = codex_skill_dir() + if destination.is_symlink() or not _safe_target(codex_home(), destination): + logger.warning("Cannot install Codex skill through symlink: %s", destination) + return destination + destination.mkdir(parents=True, exist_ok=True) + + manifest = _read_manifest(destination) + if (destination / MANIFEST_NAME).exists() and manifest is None: + logger.warning( + "Existing Codex skill manifest is invalid; leaving %s unchanged", destination + ) + return destination + + # A manually installed copy may predate the manifest. Adopt it only when + # every overlapping file is byte-identical; never overwrite user content. + if manifest is None: + conflicts = [ + relative + for relative, source in source_files.items() + if (destination / relative).exists() + and _file_hash(destination / relative) + != hashlib.sha256(source.read_bytes()).hexdigest() + ] + if conflicts: + logger.warning( + "Existing unmarked Codex skill differs (%s); leaving it unchanged", + ", ".join(conflicts), + ) + return destination + manifest = {} + + for relative in (*source_files, *manifest): + if not _is_safe_relative(relative) or not _safe_target( + destination, destination / relative + ): + logger.warning("Unsafe Codex skill path; leaving %s unchanged", destination) + return destination + + current_hashes: dict[str, str] = {} + preserved_files: set[str] = set() + for relative, source in source_files.items(): + target = destination / relative + target.parent.mkdir(parents=True, exist_ok=True) + source_hash = hashlib.sha256(source.read_bytes()).hexdigest() + old_hash = manifest.get(relative) + if ( + target.exists() + and old_hash is not None + and _file_hash(target) not in {old_hash, source_hash} + ): + logger.warning("Preserving user-edited Codex skill file: %s", target) + preserved_files.add(relative) + continue + shutil.copy2(source, target) + current_hashes[relative] = source_hash + + # Remove obsolete files that the previous CRG manifest owned, but only if + # they still match the recorded bytes. User edits and unknown files stay. + for relative, old_hash in manifest.items(): + if relative in source_files: + continue + old_path = destination / relative + if old_path.is_file() and _file_hash(old_path) == old_hash: + old_path.unlink() + + # Keep a user-edited file out of the next manifest so uninstall will not + # mistake the edited bytes for installer-owned content. + _write_manifest( + destination, + { + **current_hashes, + **{key: manifest[key] for key in preserved_files}, + }, + ) + logger.info("Installed Codex skill in %s", destination) + return destination + + +def manifest_path(skill_dir: Path | None = None) -> Path: + """Return the management manifest path for a destination skill.""" + + return (skill_dir or codex_skill_dir()) / MANIFEST_NAME + + +def owned_skill_files(skill_dir: Path | None = None) -> list[Path]: + """Return installed files that still match CRG-owned content. + + This lets uninstall remove a generated skill without deleting a user's + edits or unrelated files in the shared user skill directory. + """ + + destination = skill_dir or codex_skill_dir() + manifest = _read_manifest(destination) + if manifest is None: + return [] + expected = manifest + owned: list[Path] = [] + for relative, expected_hash in expected.items(): + path = destination / relative + if path.is_file() and _file_hash(path) == expected_hash: + owned.append(path) + manifest_file = manifest_path(destination) + if manifest is not None and manifest_file.is_file(): + owned.append(manifest_file) + return owned diff --git a/code_review_graph/skills.py b/code_review_graph/skills.py index 8088e127..c50dc1bb 100644 --- a/code_review_graph/skills.py +++ b/code_review_graph/skills.py @@ -130,9 +130,10 @@ def _opencode_config_path(repo_root: Path) -> Path: PLATFORMS: dict[str, dict[str, Any]] = { "codex": { "name": "Codex", - "config_path": lambda root: Path.home() / ".codex" / "config.toml", + "config_path": lambda root: _codex_home() / "config.toml", "key": "mcp_servers", - "detect": lambda: (Path.home() / ".codex").exists(), + "detect": lambda: bool(os.environ.get("CODEX_HOME", "").strip()) + or _codex_home().exists(), "format": "toml", "needs_type": True, }, @@ -257,6 +258,14 @@ def _opencode_config_path(repo_root: Path) -> Path: } +def _codex_home() -> Path: + """Return Codex's active state directory, honoring ``CODEX_HOME``.""" + + from .codex_skill import codex_home + + return codex_home() + + def _in_poetry_project() -> bool: """Return True when the running interpreter is a Poetry-managed virtualenv. @@ -881,7 +890,7 @@ def generate_hooks_config(repo_root: Path) -> dict[str, Any]: def generate_codex_hooks_config(repo_root: Path) -> dict[str, Any]: - """Generate native Codex hooks configuration for ~/.codex/hooks.json.""" + """Generate native Codex hooks configuration for the active Codex home.""" return { "hooks": { "PostToolUse": [ @@ -1075,13 +1084,13 @@ def install_codebuddy_hooks(repo_root: Path) -> Path: def install_codex_hooks(repo_root: Path) -> Path: - """Write native Codex hooks config to ~/.codex/hooks.json. + """Write native Codex hooks config to the active Codex home. Merges code-review-graph hook entries into any existing hooks.json, preserving user-defined hook entries and other top-level settings. A backup of the original file is created before modifications. """ - codex_dir = Path.home() / ".codex" + codex_dir = _codex_home() codex_dir.mkdir(parents=True, exist_ok=True) hooks_path = codex_dir / "hooks.json" @@ -1132,6 +1141,14 @@ def install_codex_hooks(repo_root: Path) -> Path: return hooks_path +def install_codex_skill() -> Path: + """Install the bundled global Codex skill in the active Codex home.""" + + from .codex_skill import install_codex_skill as _install + + return _install() + + _CLAUDE_MD_SECTION_MARKER = "" _CLAUDE_MD_SECTION = f"""{_CLAUDE_MD_SECTION_MARKER} diff --git a/code_review_graph/uninstall.py b/code_review_graph/uninstall.py index 233b35cd..8cc59825 100644 --- a/code_review_graph/uninstall.py +++ b/code_review_graph/uninstall.py @@ -899,11 +899,19 @@ def _remove_gitignore( ) -def _scope_for_config(path: Path, repo_root: Path, home: Path) -> tuple[str, Path] | None: +def _scope_for_config( + path: Path, + repo_root: Path, + home: Path, + extra_user_boundaries: Sequence[Path] = (), +) -> tuple[str, Path] | None: if _is_lexical_child(path, repo_root): return "repo", repo_root if _is_lexical_child(path, home): return "user", home + for boundary in extra_user_boundaries: + if _is_lexical_child(path, boundary): + return "user", boundary return None @@ -917,6 +925,13 @@ def _process_platform_configs( platforms: frozenset[str] | None = None, ) -> None: seen: set[tuple[Path, str, str]] = set() + active_codex_home: Path | None = None + if platforms is None or "codex" in platforms: + from .codex_skill import codex_home + + active_codex_home = codex_home() + else: + active_codex_home = None for platform_name, spec in skills.PLATFORMS.items(): if platforms is not None and platform_name not in platforms: continue @@ -929,7 +944,12 @@ def _process_platform_configs( f"{platform_name} config (invalid platform specification: {exc})" ) continue - destination = _scope_for_config(path, repo_root, home) + destination = _scope_for_config( + path, + repo_root, + home, + (active_codex_home,) if platform_name == "codex" and active_codex_home else (), + ) if destination is None: if path.exists(): report.skipped_paths.append( @@ -1156,14 +1176,28 @@ def _process_user( else: _remove_tree(user_data, home, report, dry_run=dry_run) + from .codex_skill import codex_home, codex_skill_dir, owned_skill_files + + active_codex_home = codex_home() + # ``_process_platform_configs`` already handles the active CODEX_HOME via + # the live platform registry. Keep hook/skill cleanup in the same directory + # and use that directory itself as the deletion boundary. _remove_hooks( - home / ".codex" / "hooks.json", + active_codex_home / "hooks.json", _commands(skills.generate_codex_hooks_config(reference_repo)) | _legacy_codex_hook_commands(), - home, + active_codex_home, report, dry_run=dry_run, ) + skill_dir = codex_skill_dir() + for owned_path in owned_skill_files(skill_dir): + _remove_skill_file( + owned_path, + active_codex_home, + report, + dry_run=dry_run, + ) _remove_hooks( home / ".cursor" / "hooks.json", _commands(skills.generate_cursor_hooks_config()), diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 9f8dc930..d3002be7 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -300,7 +300,7 @@ base: str = "HEAD~1" # Setup code-review-graph install # Configure detected AI coding platforms (alias: init) code-review-graph install --dry-run # Preview without writing files -code-review-graph install --platform codex # Configure one platform +code-review-graph install --platform codex # Configure Codex MCP, hooks, and global Skill code-review-graph uninstall # Remove all CRG configs, hooks, skills, and data code-review-graph uninstall --platform codex # Unbind one platform (keeps graph data + others) diff --git a/docs/USAGE.md b/docs/USAGE.md index b99059d3..dd3ebaa3 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -10,7 +10,7 @@ code-review-graph install # auto-detects and configures all supported platfor code-review-graph build # parse your codebase ``` -`install` detects which AI coding tools you have, writes the correct MCP configuration for each one, and installs platform-native hooks where supported. Restart your editor/tool after installing. +`install` detects which AI coding tools you have, writes the correct MCP configuration for each one, and installs platform-native hooks/skills where supported. Restart or refresh your editor/tool after installing so its MCP and Skill inventory is reloaded. To target a specific platform instead of auto-detecting all: @@ -25,7 +25,7 @@ code-review-graph install --platform codebuddy | Platform | Config file | |----------|-------------| -| **Codex** | `~/.codex/config.toml` + `~/.codex/hooks.json` | +| **Codex** | `$CODEX_HOME/config.toml` + `$CODEX_HOME/hooks.json` + `$CODEX_HOME/skills/code-review-graph/` (defaults to `~/.codex`) | | **Claude Code** | `.mcp.json` + `.claude/settings.json` | | **CodeBuddy Code** | `.mcp.json` + `CODEBUDDY.md` + `.codebuddy/settings.json` + `.codebuddy/skills//SKILL.md` | | **Cursor** | `.cursor/mcp.json` | @@ -48,6 +48,13 @@ The CodeBuddy project layout follows its official documentation for merged with JSONC awareness, while hook commands resolve the repository at runtime so committed settings do not contain one developer's checkout path. +Codex installs the CRG Skill in the active user Skill root and uses MCP first, +with a bundled read-only CLI fallback when MCP is unavailable. The Skill checks +graph health once per repository/task; a missing, empty, or stale graph never +causes an implicit build or update. WSL and Windows Codex processes have +separate `CODEX_HOME` directories and tool inventories, so install CRG in the +runtime that will actually run the task and restart/refresh that runtime. + ## Core Workflow ### 1. Build the graph (first time only) diff --git a/tests/conftest.py b/tests/conftest.py index 64c2ca1b..e8d6e80c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -33,4 +33,7 @@ def isolated_crg_home(tmp_path_factory, monkeypatch): """ home = tmp_path_factory.mktemp("crg-home") monkeypatch.setenv("CRG_HOME", str(home)) + # Codex's user-level installer must never write to the developer's active + # runtime during tests. Individual Codex tests opt in with their own value. + monkeypatch.delenv("CODEX_HOME", raising=False) return home diff --git a/tests/test_cli_install.py b/tests/test_cli_install.py index 9ac3b5b2..0dc1ee7a 100644 --- a/tests/test_cli_install.py +++ b/tests/test_cli_install.py @@ -112,7 +112,12 @@ def test_handle_init_codex_skips_claude_skills(monkeypatch, tmp_path, capsys): lambda repo_root, target, dry_run=False: ["Codex"], ) - called = {"generate_skills": False, "codex_hooks": False, "git_hook": False} + called = { + "generate_skills": False, + "codex_skill": False, + "codex_hooks": False, + "git_hook": False, + } def _generate_skills(repo_root): called["generate_skills"] = True @@ -122,11 +127,16 @@ def _install_codex_hooks(repo_root): called["codex_hooks"] = True return Path("/tmp/fake-codex-hooks.json") + def _install_codex_skill(): + called["codex_skill"] = True + return tmp_path / "codex" / "skills" / "code-review-graph" + def _install_git_hook(repo_root): called["git_hook"] = True return repo_root / ".git" / "hooks" / "pre-commit" monkeypatch.setattr("code_review_graph.skills.generate_skills", _generate_skills) + monkeypatch.setattr("code_review_graph.skills.install_codex_skill", _install_codex_skill) monkeypatch.setattr("code_review_graph.skills.install_codex_hooks", _install_codex_hooks) monkeypatch.setattr("code_review_graph.skills.install_git_hook", _install_git_hook) @@ -134,11 +144,45 @@ def _install_git_hook(repo_root): out = capsys.readouterr().out assert called["generate_skills"] is False + assert called["codex_skill"] is True assert called["codex_hooks"] is True assert called["git_hook"] is True assert "Installed Codex hooks" in out +def test_handle_init_all_skips_codex_skill_when_not_detected( + monkeypatch, tmp_path +): + monkeypatch.setattr( + "code_review_graph.incremental.ensure_repo_gitignore_excludes_crg", + lambda repo_root: "created", + ) + monkeypatch.setattr( + "code_review_graph.skills.install_platform_configs", + lambda repo_root, target, dry_run=False: [], + ) + monkeypatch.setitem( + skills.PLATFORMS, + "codex", + {**skills.PLATFORMS["codex"], "detect": lambda: False}, + ) + called = False + + def _install_codex_skill(): + nonlocal called + called = True + return tmp_path / "codex" / "skills" / "code-review-graph" + + monkeypatch.setattr("code_review_graph.skills.install_codex_skill", _install_codex_skill) + args = _args(tmp_path, "all") + args.no_hooks = True + args.no_instructions = True + + _handle_init(args) + + assert called is False + + def test_handle_init_cursor_installs_cursor_hooks(monkeypatch, tmp_path, capsys): monkeypatch.setattr( "code_review_graph.incremental.find_repo_root", diff --git a/tests/test_documentation.py b/tests/test_documentation.py index 985c4eea..5bd810b0 100644 --- a/tests/test_documentation.py +++ b/tests/test_documentation.py @@ -23,6 +23,7 @@ USER_DOC_FILES = README_FILES + ( "docs/COMMANDS.md", "docs/FAQ.md", + "docs/USAGE.md", "code_review_graph/docs/LLM-OPTIMIZED-REFERENCE.md", "docs/TROUBLESHOOTING.md", ) @@ -75,3 +76,13 @@ def test_codebuddy_install_docs_cover_project_artifacts(): ".codebuddy/skills//SKILL.md", ): assert artifact in usage + + +def test_codex_install_docs_cover_global_skill_and_runtime_scope(): + readme = (ROOT / "README.md").read_text(encoding="utf-8") + usage = (ROOT / "docs/USAGE.md").read_text(encoding="utf-8") + for content in (readme, usage): + assert "$CODEX_HOME" in content + assert "skills/code-review-graph" in content + assert "WSL" in content or "Windows" in content + assert "read-only CLI" in content diff --git a/tests/test_skills.py b/tests/test_skills.py index dd54eca9..6e69bbce 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -16,6 +16,7 @@ import tomli as tomllib from code_review_graph import skills as skills_module +from code_review_graph.codex_skill import install_codex_skill from code_review_graph.skills import ( _CLAUDE_MD_SECTION_MARKER, PLATFORMS, @@ -560,6 +561,63 @@ def test_creates_hooks_file(self, tmp_path, monkeypatch): assert "PostToolUse" in data["hooks"] assert "SessionStart" in data["hooks"] + def test_installs_global_skill_under_codex_home(self, tmp_path, monkeypatch): + codex_home = tmp_path / "codex-home" + monkeypatch.setenv("CODEX_HOME", str(codex_home)) + destination = install_codex_skill() + + assert destination == codex_home / "skills" / "code-review-graph" + assert (destination / "SKILL.md").is_file() + assert (destination / "scripts" / "crg_readonly.py").is_file() + assert (destination / "agents" / "openai.yaml").is_file() + assert (destination / ".code-review-graph-managed.json").is_file() + + def test_global_skill_install_is_idempotent_and_preserves_unrelated_files( + self, tmp_path, monkeypatch + ): + monkeypatch.setenv("CODEX_HOME", str(tmp_path / "codex-home")) + destination = install_codex_skill() + custom = destination / "notes.txt" + custom.write_text("keep\n", encoding="utf-8") + first = { + path.relative_to(destination): path.read_bytes() + for path in destination.rglob("*") + if path.is_file() + } + + install_codex_skill() + + second = { + path.relative_to(destination): path.read_bytes() + for path in destination.rglob("*") + if path.is_file() + } + assert second == first + assert custom.read_text(encoding="utf-8") == "keep\n" + + def test_unmarked_conflicting_skill_is_not_overwritten(self, tmp_path, monkeypatch): + codex_home = tmp_path / "codex-home" + destination = codex_home / "skills" / "code-review-graph" + destination.mkdir(parents=True) + skill = destination / "SKILL.md" + skill.write_text("user-owned\n", encoding="utf-8") + monkeypatch.setenv("CODEX_HOME", str(codex_home)) + + install_codex_skill() + + assert skill.read_text(encoding="utf-8") == "user-owned\n" + + def test_codex_platform_config_honors_codex_home(self, tmp_path, monkeypatch): + codex_home = tmp_path / "codex-home" + monkeypatch.setenv("CODEX_HOME", str(codex_home)) + + install_platform_configs(tmp_path, target="codex") + + assert (codex_home / "config.toml").is_file() + assert "[mcp_servers.code-review-graph]" in ( + codex_home / "config.toml" + ).read_text(encoding="utf-8") + def test_merges_with_existing(self, tmp_path, monkeypatch): # Path.home() ignores HOME on Windows; patch it like the cursor tests do. monkeypatch.setattr("code_review_graph.skills.Path.home", lambda: tmp_path) @@ -2407,6 +2465,12 @@ def test_claude_install_creates_skills(self, tmp_path): def test_all_target_creates_skills(self, tmp_path): assert self._run_install(tmp_path, "all") is True + def test_codex_install_creates_global_skill(self, tmp_path, monkeypatch): + codex_home = tmp_path / "codex-home" + monkeypatch.setenv("CODEX_HOME", str(codex_home)) + self._run_install(tmp_path, "codex") + assert (codex_home / "skills" / "code-review-graph" / "SKILL.md").is_file() + class TestNonAsciiConfigPreservation: """#497: json.dumps(..., indent=2) defaults to ensure_ascii=True, so any diff --git a/tests/test_uninstall.py b/tests/test_uninstall.py index af794b5b..31fa0e0b 100644 --- a/tests/test_uninstall.py +++ b/tests/test_uninstall.py @@ -852,6 +852,59 @@ def test_platform_scoped_unbind_targets_user_scope_toml( assert "code-review-graph" in claude_config.read_text(encoding="utf-8") +def test_uninstall_uses_custom_codex_home_and_preserves_user_skill( + fake_repo: Path, + fake_home: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + codex_home = fake_home / "wsl-codex" + skill_dir = codex_home / "skills" / "code-review-graph" + skill_dir.mkdir(parents=True) + skill_file = skill_dir / "SKILL.md" + skill_file.write_text("generated\n", encoding="utf-8") + notes = skill_dir / "notes.txt" + notes.write_text("user\n", encoding="utf-8") + (skill_dir / ".code-review-graph-managed.json").write_text( + json.dumps({"version": 1, "files": {"SKILL.md": "not-the-file-hash"}}), + encoding="utf-8", + ) + config = codex_home / "config.toml" + _write_codex_config(config) + monkeypatch.setenv("CODEX_HOME", str(codex_home)) + + report = uninstall.run(repo=fake_repo, keep_data=True) + + assert report.errors == [] + config_text = config.read_text(encoding="utf-8") + assert "[mcp_servers.code-review-graph]" not in config_text + assert "[mcp_servers.other]" in config_text + assert skill_file.exists() + assert notes.exists() + + +def test_uninstall_removes_only_managed_codex_skill_files( + fake_repo: Path, + fake_home: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + from code_review_graph.codex_skill import install_codex_skill + + codex_home = fake_home / "wsl-codex" + monkeypatch.setenv("CODEX_HOME", str(codex_home)) + skill_dir = install_codex_skill() + notes = skill_dir / "notes.txt" + notes.write_text("user\n", encoding="utf-8") + + report = uninstall.run(repo=fake_repo, keep_data=True) + + assert report.errors == [] + assert notes.read_text(encoding="utf-8") == "user\n" + assert not (skill_dir / "SKILL.md").exists() + assert not (skill_dir / "agents" / "openai.yaml").exists() + assert not (skill_dir / "scripts" / "crg_readonly.py").exists() + assert not (skill_dir / ".code-review-graph-managed.json").exists() + + def test_cli_uninstall_platform_scopes_to_one_binding( fake_repo: Path, fake_home: Path,