Skip to content

feat(codex): install native CRG skill with safe CLI fallback - #1066

Open
Ethereal49 wants to merge 4 commits into
tirth8205:stagingfrom
Ethereal49:fix/codex-native-skill-841
Open

Ethereal49 wants to merge 4 commits into
tirth8205:stagingfrom
Ethereal49:fix/codex-native-skill-841

Conversation

@Ethereal49

@Ethereal49 Ethereal49 commented Sep 25, 2026 •

Copy link
Copy Markdown

Linked issue

Closes #841. Related implementation: #846 by CissiBot.

What & why

Install a Codex-native global skill under the active CODEX_HOME, with UI metadata, MCP-first guidance, and a bounded CLI fallback for graph queries. Installation, reinstall and uninstall preserve user content, reject unsafe manifest paths, and honor CODEX_HOME for the skill, MCP config and hooks. The docs cover refreshing Codex and separate Windows/WSL runtimes.

The fallback validates repository paths and executable arguments and does not create a graph when none exists. Graph commands retain the established one-time migration of repo/.code-review-graph.db when the default data directory is in use; an external environment or registry data directory does not consume that legacy graph.

The Codex skill installer excludes __pycache__ directories and .pyc/.pyo files that pip generates after installing the wheel, so caches are neither copied nor recorded in the ownership manifest. Tests cover all four reported legacy query commands, missing graphs, external data directories, and installation from an actual pip-installed wheel.

Validation

Local validation on macOS with Python 3.13.12:

  • Standard suite with the 65% coverage gate: 5077 passed, 883 skipped, 3 xfailed, 2 xpassed; 86.66% coverage. The run reported 124 warnings. Optional suites skipped by this run are not claimed as validated.
  • Focused CLI, wrapper, skill, install and uninstall regression suite: 342 passed.
  • Explicit Codex lifecycle suite: 9 passed, 1 xfailed.
  • Explicit installed-wheel tests, including a check that pip created bytecode before Codex installation: 2 passed.
  • Ruff, mypy (79 source files), Bandit and Python/VS Code schema sync: passed.
.venv/bin/python -m pytest tests/ --cov=code_review_graph --cov-report=term --cov-fail-under=65 --tb=short -q
.venv/bin/python -m pytest tests/test_platform_lifecycle.py -m platform_lifecycle -k codex -q --tb=short
.venv/bin/python -m pytest tests/test_packaging.py -m packaging -k 'codex_wheel_install_excludes_pip_bytecode or install_command_works_from_the_installed_wheel' -q --tb=short
.venv/bin/ruff check code_review_graph/ tests/test_cli_tool_commands.py tests/test_packaging.py tests/test_skills.py
.venv/bin/mypy code_review_graph/ --ignore-missing-imports --no-strict-optional
uvx --from 'bandit[toml]' bandit -r code_review_graph/ -c pyproject.toml -q

@Ethereal49
Ethereal49 marked this pull request as ready for review September 25, 2026 17:38
Copilot AI lite review requested due to automatic review settings September 25, 2026 17:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved safety issues remain in manifest handling, fallback read-only behavior, repository validation, Windows executable parsing, and conflicting skill-directory installation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Adds a Codex-native global skill with CODEX_HOME support, MCP-first behavior, and a CLI fallback.

Changes:

  • Adds Codex skill assets, metadata, and wrapper routing.
  • Integrates installation, ownership tracking, and cleanup.
  • Adds lifecycle tests and Codex/Windows/WSL documentation.
File Summary
tests/​test_uninstall.py Tests Codex cleanup behavior.
tests/​test_skills.py Tests installation safety and idempotence.
tests/​test_platform_lifecycle.py Tests Codex lifecycle artifacts.
tests/​test_documentation.py Validates documentation coverage.
tests/​test_codex_skill_wrapper.py Tests fallback routing and mutation rejection.
tests/​test_cli_install.py Tests CLI installation integration.
tests/​conftest.py Isolates CODEX_HOME in tests.
README.md Documents Codex skill usage.
docs/​USAGE.md Documents runtime and path behavior.
docs/​COMMANDS.md Updates command documentation.
code_review_graph/​uninstall.py Cleans Codex configuration and owned files.
code_review_graph/​skills.py Integrates Codex home and skill handling.
code_review_graph/​codex_skill.py Installs and manages skill assets and manifests.
code_review_graph/​cli.py Routes Codex skill installation.
code_review_graph/​assets/​code-review-graph/​SKILL.md Defines Codex workflow and safety rules.
code_review_graph/​assets/​code-review-graph/​scripts/​crg_readonly.py Provides CLI fallback routing.
code_review_graph/​assets/​code-review-graph/​agents/​openai.yaml Provides Codex UI metadata.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread code_review_graph/codex_skill.py Outdated
Comment thread code_review_graph/codex_skill.py
@Ethereal49

Copy link
Copy Markdown
Author

Hi @tirth8205, Codex is helping Ethereal49 follow up on this PR. Could you confirm whether the Codex-native skill change is still in scope for #841, or whether you would like any changes before reviewing #1066? Ethereal49 is happy to adjust it.

@tirth8205

Copy link
Copy Markdown
Owner

Thank you for this PR. I tested install, uninstall, the wrapper and the read-only commands.

Before I can merge it, please fix these:

  1. Graph commands no longer move an old graph file. Before 1.5.0, the graph lived at repo/.code-review-graph.db. Now query, search, flows and architecture fail with "No graph found". Staging moves the file and answers. See code_review_graph/cli.py:1737. The status path already has this check, so you can reuse it. Example: keep only repo/.code-review-graph.db, then run code-review-graph architecture --repo repo.

  2. Install copies Python cache files into the Codex skill. By default, pip compiles scripts/crg_readonly.py into scripts/__pycache__/. The installer copies that .pyc file and lists it in the manifest. See code_review_graph/codex_skill.py:55. Please skip __pycache__ folders and *.pyc files. Example: install the wheel with pip, then run install --platform codex.

After you push the fix, I will test it again.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(codex): install a global CRG skill with safe CLI fallback

3 participants