Conversation
Full dual-agent support so Codex CLI users get the same Pilot Shell experience as Claude Code users across all Console features. Sessions: CodexSessionReader parses ~/.codex/sessions/ JSONL files, merged with Claude sessions in Dashboard and Sessions views with agent badge and All/Claude/Codex filter toggle. Usage: Codex cost tracking via codex-parser with model-specific pricing, aggregated into daily/monthly charts with agent filtering. Memories: Hook pipeline fixed for Codex (hookSpecificOutput passthrough instead of systemMessage, context handler enabled, no ugly display), all memory lifecycle hooks added (session-init, observation, summarize, session-end), agent column migration on sdk_sessions, observations API agent filter, Memories UI toggle. Extensions: Codex extensions discovery and display alongside Claude extensions in the Extensions view. Quality hooks: file_checker (with apply_patch format support), tool_token_saver, context_monitor (transcript estimation fallback), spec_stop_guard — all ported to codex_hooks.json. Installer: codex_files step installs hooks, skills, MCP config, and AGENTS.md for Codex CLI. Agent runtime detection for both CLIs. Docs: Codex CLI getting-started guide, updated hooks/extensions/MCP docs for dual-agent coverage.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
<review_stack_artifact_start --> ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Claude finished @maxritter's task in 2m 53s —— View job Code Review: feat: enable Codex CLI as a first-class agent alongside Claude CodeTasks:
SummaryThis is a solid, well-architected PR. The Codex integration is implemented symmetrically to Claude Code across all major surfaces: hooks, config, skills, rules, MCP servers, and shell wrappers. Test coverage is extensive (788 new test lines for A few issues worth addressing before merge: Bugs1. TOML key detection may skip required top-level keys when a same-named key exists inside a section
if not re.search(rf"(?m)^{re.escape(key)}\s*=", existing):The [some_section]
approval_policy = "auto"The installer would conclude m = re.search(r"(?m)^\[", existing)
top_level_scope = existing[:m.start()] if m else existing
if not re.search(rf"(?m)^{re.escape(key)}\s*=", top_level_scope):
...2.
try:
self._install_codex_config(ctx)
self._install_codex_mcp(ctx)
except _TomlStructureError as e:
if ctx.ui:
ctx.ui.warning(f"Skipping Codex TOML config due to structure error: {e}")Dead Code3.
_TASK_SUBAGENT_RE = re.compile(
r"Task\(\s*\n?\s*subagent_type=['\"]([^'\"]+)['\"].*?\)",
re.DOTALL,
)
_AGENT_SUBAGENT_RE = re.compile(
r"Agent\(\s*\n?\s*(?:subagent_type=['\"]([^'\"]+)['\"].*?|.*?subagent_type=['\"]([^'\"]+)['\"].*?)\)",
re.DOTALL,
)Neither is referenced in Minor Issues4. Redundant
def _insert_top_level_key(content: str, key: str, value: str) -> str:
import re # ← already imported at module top level (line 11)Remove the local import. 5.
lines.append(f"")This is equivalent to Design Observations
def _is_pilot_managed_entry(entry: dict[str, Any]) -> bool:
for hook in entry.get("hooks", []):
cmd = hook.get("command", "")
if "/.pilot/" in cmd:
return True
return FalseThe identification of "Pilot-managed" hooks relies on the path substring Codex config installs
This hardcodes GPT-5.5-style reasoning config keys. If OpenAI changes how these keys work across model versions (or if a user is running a different model via Test CoverageThe test suite (
One gap: there's no test for the section-scoped key false-positive bug noted in issue #1 above. Documentation
One clarification worth adding: the doc says |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/docusaurus/docs/getting-started/installation.md (1)
21-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInstaller step list likely stale after CodexFilesStep addition.
The section still says “7 steps” and the table omits a Codex files step, which appears inconsistent with this PR’s documented installer changes. Please update the count and step table so it matches the actual installer flow users will see.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/docusaurus/docs/getting-started/installation.md` around lines 21 - 31, Update the installer overview to reflect the new CodexFilesStep: change "7 steps" to "8 steps" and insert a new table row for the "Codex files" step between the existing "Claude files" and "Config files" rows (or wherever the installer flow places CodexFilesStep); include a brief title "Codex files" and a short description like "Sets up `~/.codex/` assets — models, snippets, and runtime metadata" so the markdown table in installation.md matches the actual installer flow and step naming used in the PR.installer/tests/unit/steps/test_dependencies.py (1)
63-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNewly patched install paths aren’t validated in the core orchestration test.
This test now mocks additional dependency steps but only asserts a subset, so orchestration regressions can pass silently. Please assert the newly introduced calls (and gated non-calls) explicitly.
Suggested assertion additions
mock_claude_code.assert_called_once() mock_nodejs.assert_called_once() mock_uv.assert_called_once() mock_python_tools.assert_called_once() mock_plugin_deps.assert_called_once() _mock_semble.assert_called_once() _mock_rtk.assert_called_once() + _mock_codegraph.assert_called_once() + _mock_better_sqlite3.assert_called_once() + _mock_codex_plugin.assert_called_once() + _mock_lsp_plugins.assert_called_once() + _mock_playwright.assert_called_once() + _mock_codegraph_needs_work.assert_called_once() + _mock_init_codegraph.assert_not_called()As per coding guidelines
**/tests/**: Review test code briefly. Focus on: Test coverage for the feature being tested.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installer/tests/unit/steps/test_dependencies.py` around lines 63 - 117, The test test_dependencies_run_installs_core is missing assertions for several newly mocked dependency calls causing orchestration regressions to be invisible; update the test to assert that each new mock (e.g., _mock_codegraph, _mock_better_sqlite3, _mock_codex_plugin, _mock_lsp_plugins, _mock_playwright, _mock_codegraph_needs_work, _mock_init_codegraph, _mock_prettier, _mock_golangci_lint, _mock_pbt_tools, _mock_agent_browser, _mock_semble, _mock_rtk) is called or explicitly not called as appropriate after step.run(ctx) when running DependenciesStep, and also add negative assertions for steps that should not run in this scenario (use assert_called_once() or assert_not_called() on the mock objects to make the orchestration expectations explicit).
🧹 Nitpick comments (5)
installer/steps/codex_files.py (3)
38-43: 💤 Low valueConsider validating
CODEX_HOMEis an absolute path.
get_claude_config_dir()inclaude_files.pyraisesValueErrorifCLAUDE_CONFIG_DIRis relative, but_get_codex_config_dir()accepts any path. This inconsistency could cause unexpected behavior if a user setsCODEX_HOMEto a relative path.Proposed fix
def _get_codex_config_dir() -> Path: """Resolve the Codex config directory, respecting CODEX_HOME env var.""" env_dir = os.environ.get("CODEX_HOME") if env_dir: - return Path(env_dir) + p = Path(env_dir) + if not p.is_absolute(): + raise ValueError(f"CODEX_HOME must be an absolute path, got: {env_dir}") + return p return Path.home() / ".codex"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installer/steps/codex_files.py` around lines 38 - 43, _in _get_codex_config_dir(), validate that the CODEX_HOME env var is an absolute path like get_claude_config_dir() does: if os.environ.get("CODEX_HOME") is set, convert it to a Path and call is_absolute(); if it is not absolute, raise a ValueError with a clear message referencing CODEX_HOME; otherwise return the Path(env_dir) as before, and keep the fallback to Path.home() / ".codex" when the env var is absent.
413-413: ⚡ Quick winRemove redundant
import restatement.
reis already imported at module level (line 11).Proposed fix
def _insert_top_level_key(content: str, key: str, value: str) -> str: """Insert a key=value pair into the top-level scope of a TOML string. Inserts before the first ``[section]`` header so the key doesn't accidentally land inside an unrelated table. """ - import re - line = f"{key} = {value}\n"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installer/steps/codex_files.py` at line 413, Remove the redundant local "import re" statement (the duplicate import re near the bottom of installer/steps/codex_files.py) since re is already imported at module level; locate the duplicate "import re" and delete that line so only the single top-level import remains.
456-456: ⚡ Quick winRemove unnecessary f-string prefix.
f""has no placeholders; use""instead.Proposed fix
- lines.append(f"") + lines.append("")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installer/steps/codex_files.py` at line 456, Replace the unnecessary f-string usage in the call to lines.append by removing the f prefix so it uses a plain empty string; update the lines.append(f"") expression to lines.append("") where it appears (e.g., in the code that builds the lines list).installer/steps/dependencies.py (2)
1083-1107: 💤 Low valueUnreachable
UNCHANGEDoutcome in success path.When
okisTrue, both the hypothesis and fast-check installs succeeded, meaningany_changedis alwaysTrue. The_OUTCOME_UNCHANGEDbranch on line 1106 is dead code within the success path.If the intent is to track whether tools were already present (vs freshly installed/upgraded), the function would need to check
command_existsbefore the install attempts, similar to other install functions.Simplify by removing unreachable branch
if ok: - _record_outcome(_OUTCOME_UPDATED if any_changed else _OUTCOME_UNCHANGED) + _record_outcome(_OUTCOME_UPDATED) return ok🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installer/steps/dependencies.py` around lines 1083 - 1107, The success path in install_pbt_tools sets any_changed to True whenever _run_bash_with_retry for hypothesis and fast-check succeeds, making the _OUTCOME_UNCHANGED branch unreachable; either remove the unreachable _OUTCOME_UNCHANGED usage or implement pre-checks to detect already-present tools. Fix by adding command-existence checks (e.g., use an existing command_exists helper) before calling _run_bash_with_retry for hypothesis and fast-check (manifest_get), set any_changed only when an install/upgrade is actually performed, and then call _record_outcome(_OUTCOME_UPDATED if any_changed else _OUTCOME_UNCHANGED); alternatively, simply always record _OUTCOME_UPDATED on ok and drop the _OUTCOME_UNCHANGED branch. Ensure you update logic around install_pbt_tools, any_changed, _run_bash_with_retry, manifest_get, _record_outcome, _OUTCOME_UPDATED and _OUTCOME_UNCHANGED accordingly.
208-215: 💤 Low valueOutcome recording inconsistent with other install functions.
Unlike
install_semble,install_typescript_lsp,install_prettier, etc., this function always records_OUTCOME_UPDATEDwithout checking if the tools were previously present. This results in "Python tools updated" even on fresh installs.Suggested fix for consistency
def install_python_tools() -> bool: """Install or upgrade Python development tools.""" tools = ["ruff", "basedpyright"] + any_was_present = any(command_exists(t) for t in tools) for tool in tools: if not _run_bash_with_retry(f"uv tool install --upgrade {tool}"): return False - _record_outcome(_OUTCOME_UPDATED) + _record_outcome(_OUTCOME_UPDATED if any_was_present else _OUTCOME_INSTALLED) return True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@installer/steps/dependencies.py` around lines 208 - 215, The install_python_tools function always records _OUTCOME_UPDATED; change it to mirror the other installers by detecting whether each tool already exists before installing and record _OUTCOME_INSTALLED if none were present or _OUTCOME_UPDATED if at least one was previously installed. Concretely, in install_python_tools use a pre-check (e.g., call _run_bash_with_retry("uv tool show {tool}") or equivalent) to set a flag like had_any_installed = True if any tool exists, then perform the installs with _run_bash_with_retry as now, and after successful installs call _record_outcome(_OUTCOME_UPDATED) if had_any_installed else _record_outcome(_OUTCOME_INSTALLED); keep function name install_python_tools and the existing _run_bash_with_retry/_record_outcome/_OUTCOME_UPDATED symbols so the change is consistent with install_semble, install_typescript_lsp, and install_prettier.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/docusaurus/docs/features/extensions.md`:
- Around line 214-223: The two fenced code blocks showing the directory trees
(the block that starts with "~/.agents/" and the block that starts with
"<project>/") are missing a language tag and trigger MD040; update each opening
triple-backtick to include a language (e.g., add "text" so they read ```text) so
markdownlint is satisfied, ensuring both code blocks (the one containing
"~/.agents/ ... ~/.codex/ ..." and the one containing "<project>/ ... .codex/")
are updated consistently.
In `@docs/docusaurus/docs/getting-started/codex-cli.md`:
- Around line 90-96: Update the outdated Hook support section in the docs to
reflect that Codex now ships quality hooks beyond SessionStart and PreCompact:
remove the claim that "most Claude Code hooks ... are not available in Codex"
and instead list the actual registered hooks in ~/.codex/hooks.json (include
SessionStart, PreCompact plus the quality hooks now supported such as
file-checker, context-monitor, spec-stop-guard, spec-mode-guard,
token-saver/tool-redirect equivalents). Ensure the text describes any
Codex-specific fields or differences (e.g., how
hookSpecificOutput/suppressOutput are handled if applicable) and matches the
PR’s hook rollout so readers see the correct coverage and behavior for functions
like file checker, context monitor, spec stop guard, and token saver.
In `@installer/steps/codex_files.py`:
- Around line 352-360: The loop over decomposed skill dirs currently swallows
all exceptions silently which hides failures; update the try/except around
build_codex_skill_md(skill_dir) to catch specific exceptions (or catch Exception
but log it) and emit a warning including the skill_dir name and the exception
details before continuing, so callers can see which skill failed; reference the
build_codex_skill_md call and the _atomic_write/dest_dir logic and use the
existing logger (or add one) to log an informative message (e.g., "Failed to
build SKILL.md for {skill_dir}: {err}") before continue.
In `@installer/steps/shell_config.py`:
- Around line 32-39: The fish wrapper uses the literal token "%self-(random)" so
session IDs become "%self-<random>" instead of PID-based values; update the
claude and codex function definitions in installer/steps/shell_config.py to
build the session id from $fish_pid (e.g. use "$fish_pid-(random)" when
assigning to local _sid and when setting PILOT_SESSION_ID) and keep the existing
exported vars (PILOT_SESSION_ID and CLAUDE_CODE_TASK_LIST_ID) and command
invocation intact so the functions claude, codex, _sid, PILOT_SESSION_ID and
CLAUDE_CODE_TASK_LIST_ID produce correct PID-RANDOM values.
In `@installer/tests/unit/steps/test_dependencies.py`:
- Around line 1430-1441: The test
test_install_golangci_lint_upgrades_when_already_installed must assert the
installer helper was invoked; after calling install_golangci_lint() add an
assertion that the patched helper was called (use the existing patch parameter
_mock_curl or mock_subproc depending on which helper you want to verify) e.g.
assert _mock_curl.assert_called_once() or assert
mock_subproc.assert_called_with(...) to prove the upgrade path executed the
installer helper; keep the rest of the test and mocks intact.
---
Outside diff comments:
In `@docs/docusaurus/docs/getting-started/installation.md`:
- Around line 21-31: Update the installer overview to reflect the new
CodexFilesStep: change "7 steps" to "8 steps" and insert a new table row for the
"Codex files" step between the existing "Claude files" and "Config files" rows
(or wherever the installer flow places CodexFilesStep); include a brief title
"Codex files" and a short description like "Sets up `~/.codex/` assets — models,
snippets, and runtime metadata" so the markdown table in installation.md matches
the actual installer flow and step naming used in the PR.
In `@installer/tests/unit/steps/test_dependencies.py`:
- Around line 63-117: The test test_dependencies_run_installs_core is missing
assertions for several newly mocked dependency calls causing orchestration
regressions to be invisible; update the test to assert that each new mock (e.g.,
_mock_codegraph, _mock_better_sqlite3, _mock_codex_plugin, _mock_lsp_plugins,
_mock_playwright, _mock_codegraph_needs_work, _mock_init_codegraph,
_mock_prettier, _mock_golangci_lint, _mock_pbt_tools, _mock_agent_browser,
_mock_semble, _mock_rtk) is called or explicitly not called as appropriate after
step.run(ctx) when running DependenciesStep, and also add negative assertions
for steps that should not run in this scenario (use assert_called_once() or
assert_not_called() on the mock objects to make the orchestration expectations
explicit).
---
Nitpick comments:
In `@installer/steps/codex_files.py`:
- Around line 38-43: _in _get_codex_config_dir(), validate that the CODEX_HOME
env var is an absolute path like get_claude_config_dir() does: if
os.environ.get("CODEX_HOME") is set, convert it to a Path and call
is_absolute(); if it is not absolute, raise a ValueError with a clear message
referencing CODEX_HOME; otherwise return the Path(env_dir) as before, and keep
the fallback to Path.home() / ".codex" when the env var is absent.
- Line 413: Remove the redundant local "import re" statement (the duplicate
import re near the bottom of installer/steps/codex_files.py) since re is already
imported at module level; locate the duplicate "import re" and delete that line
so only the single top-level import remains.
- Line 456: Replace the unnecessary f-string usage in the call to lines.append
by removing the f prefix so it uses a plain empty string; update the
lines.append(f"") expression to lines.append("") where it appears (e.g., in the
code that builds the lines list).
In `@installer/steps/dependencies.py`:
- Around line 1083-1107: The success path in install_pbt_tools sets any_changed
to True whenever _run_bash_with_retry for hypothesis and fast-check succeeds,
making the _OUTCOME_UNCHANGED branch unreachable; either remove the unreachable
_OUTCOME_UNCHANGED usage or implement pre-checks to detect already-present
tools. Fix by adding command-existence checks (e.g., use an existing
command_exists helper) before calling _run_bash_with_retry for hypothesis and
fast-check (manifest_get), set any_changed only when an install/upgrade is
actually performed, and then call _record_outcome(_OUTCOME_UPDATED if
any_changed else _OUTCOME_UNCHANGED); alternatively, simply always record
_OUTCOME_UPDATED on ok and drop the _OUTCOME_UNCHANGED branch. Ensure you update
logic around install_pbt_tools, any_changed, _run_bash_with_retry, manifest_get,
_record_outcome, _OUTCOME_UPDATED and _OUTCOME_UNCHANGED accordingly.
- Around line 208-215: The install_python_tools function always records
_OUTCOME_UPDATED; change it to mirror the other installers by detecting whether
each tool already exists before installing and record _OUTCOME_INSTALLED if none
were present or _OUTCOME_UPDATED if at least one was previously installed.
Concretely, in install_python_tools use a pre-check (e.g., call
_run_bash_with_retry("uv tool show {tool}") or equivalent) to set a flag like
had_any_installed = True if any tool exists, then perform the installs with
_run_bash_with_retry as now, and after successful installs call
_record_outcome(_OUTCOME_UPDATED) if had_any_installed else
_record_outcome(_OUTCOME_INSTALLED); keep function name install_python_tools and
the existing _run_bash_with_retry/_record_outcome/_OUTCOME_UPDATED symbols so
the change is consistent with install_semble, install_typescript_lsp, and
install_prettier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e502fa8-7aa7-46b5-97b8-9e5a9dda6fed
⛔ Files ignored due to path filters (136)
console/src/cli/adapters/codex.tsis excluded by!console/**console/src/cli/adapters/index.tsis excluded by!console/**console/src/cli/handlers/session-init.tsis excluded by!console/**console/src/cli/hook-command.tsis excluded by!console/**console/src/services/sqlite/SessionStore.tsis excluded by!console/**console/src/services/sqlite/migrations/runner.tsis excluded by!console/**console/src/services/sqlite/sessions/create.tsis excluded by!console/**console/src/services/worker/CodexSessionReader.tsis excluded by!console/**console/src/services/worker/PaginationHelper.tsis excluded by!console/**console/src/services/worker/http/routes/DataRoutes.tsis excluded by!console/**console/src/services/worker/http/routes/ExtensionRoutes.tsis excluded by!console/**console/src/services/worker/http/routes/ExtensionTypes.tsis excluded by!console/**console/src/services/worker/http/routes/SessionRoutes.tsis excluded by!console/**console/src/services/worker/http/routes/SettingsRoutes.tsis excluded by!console/**console/src/services/worker/http/routes/UsageRoutes.tsis excluded by!console/**console/src/services/worker/http/routes/utils/planFileReader.tsis excluded by!console/**console/src/services/worker/usage/aggregator.tsis excluded by!console/**console/src/services/worker/usage/codex-parser.tsis excluded by!console/**console/src/services/worker/usage/pricing.tsis excluded by!console/**console/src/services/worker/usage/yield.tsis excluded by!console/**console/src/shared/hook-constants.tsis excluded by!console/**console/src/ui/viewer/components/ui/AgentBadge.tsxis excluded by!console/**console/src/ui/viewer/components/ui/index.tsis excluded by!console/**console/src/ui/viewer/hooks/useExtensions.tsis excluded by!console/**console/src/ui/viewer/hooks/useUsage.tsis excluded by!console/**console/src/ui/viewer/views/Changes/index.tsxis excluded by!console/**console/src/ui/viewer/views/Dashboard/RecentSessions.tsxis excluded by!console/**console/src/ui/viewer/views/Extensions/ExtensionDetail.tsxis excluded by!console/**console/src/ui/viewer/views/Extensions/ExtensionsGrid.tsxis excluded by!console/**console/src/ui/viewer/views/Extensions/ExtensionsView.tsxis excluded by!console/**console/src/ui/viewer/views/Memories/index.tsxis excluded by!console/**console/src/ui/viewer/views/Requirements/index.tsxis excluded by!console/**console/src/ui/viewer/views/Sessions/SessionCard.tsxis excluded by!console/**console/src/ui/viewer/views/Sessions/SessionDetail.tsxis excluded by!console/**console/src/ui/viewer/views/Sessions/SessionTimeline.tsxis excluded by!console/**console/src/ui/viewer/views/Sessions/index.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/sections/SpecWorkflowSection.tsxis excluded by!console/**console/src/ui/viewer/views/Settings/sections/_shared.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/SpecHeaderCard.tsxis excluded by!console/**console/src/ui/viewer/views/Spec/index.tsxis excluded by!console/**console/src/ui/viewer/views/Usage/ModelCostBreakdown.tsxis excluded by!console/**console/src/ui/viewer/views/Usage/index.tsxis excluded by!console/**console/src/utils/logger.tsis excluded by!console/**console/tests/cli/codex-adapter.test.tsis excluded by!console/**console/tests/cli/codex-agent-filter.test.tsis excluded by!console/**console/tests/cli/context-handler-plan.test.tsis excluded by!console/**console/tests/ui/ExtensionsGrid.test.tsis excluded by!console/**console/tests/ui/spec-delete-selection.test.tsis excluded by!console/**console/tests/unit/services/worker/usage/codex-parser.test.tsis excluded by!console/**console/tests/worker/extension-routes.test.tsis excluded by!console/**launcher/agent_runtime.pyis excluded by!launcher/**launcher/banner.pyis excluded by!launcher/**launcher/build.pyis excluded by!launcher/**launcher/cli.pyis excluded by!launcher/**launcher/codegraph.pyis excluded by!launcher/**launcher/config.pyis excluded by!launcher/**launcher/model_config.pyis excluded by!launcher/**launcher/session.pyis excluded by!launcher/**launcher/statusline/formatter.pyis excluded by!launcher/**launcher/statusline/widgets.pyis excluded by!launcher/**launcher/tests/unit/test_agent_runtime.pyis excluded by!launcher/**launcher/tests/unit/test_cli.pyis excluded by!launcher/**launcher/tests/unit/test_codegraph.pyis excluded by!launcher/**launcher/tests/unit/test_session.pyis excluded by!launcher/**launcher/tests/unit/test_updater.pyis excluded by!launcher/**launcher/updater.pyis excluded by!launcher/**launcher/wrapper.pyis excluded by!launcher/**pilot/.mcp.jsonis excluded by!pilot/**pilot/hooks/codegraph_init.pyis excluded by!pilot/**pilot/hooks/codex_hooks.jsonis excluded by!pilot/**pilot/hooks/codex_skill_sync.pyis excluded by!pilot/**pilot/hooks/context_monitor.pyis excluded by!pilot/**pilot/hooks/file_checker.pyis excluded by!pilot/**pilot/hooks/hooks.jsonis excluded by!pilot/**pilot/hooks/license_check.pyis excluded by!pilot/**pilot/hooks/session_end.pyis excluded by!pilot/**pilot/hooks/tests/test_codegraph_init.pyis excluded by!pilot/**pilot/hooks/tests/test_codex_skill_sync.pyis excluded by!pilot/**pilot/hooks/tests/test_context_monitor.pyis excluded by!pilot/**pilot/hooks/tests/test_file_checker.pyis excluded by!pilot/**pilot/rules/cli-tools.mdis excluded by!pilot/**pilot/rules/development-practices.mdis excluded by!pilot/**pilot/rules/mcp-servers.mdis excluded by!pilot/**pilot/rules/task-and-workflow.mdis excluded by!pilot/**pilot/scripts/context-generator.cjsis excluded by!pilot/**pilot/scripts/mcp-server.cjsis excluded by!pilot/**pilot/scripts/worker-service.cjsis excluded by!pilot/**pilot/settings.jsonis excluded by!pilot/**pilot/skills/benchmark/orchestrator.mdis excluded by!pilot/**pilot/skills/benchmark/scripts/runner.pyis excluded by!pilot/**pilot/skills/benchmark/steps/03-author-evals.mdis excluded by!pilot/**pilot/skills/benchmark/steps/04-execute.mdis excluded by!pilot/**pilot/skills/benchmark/tests/test_runner.pyis excluded by!pilot/**pilot/skills/create-skill/steps/04-create.mdis excluded by!pilot/**pilot/skills/create-skill/steps/06-test.mdis excluded by!pilot/**pilot/skills/fix/steps/01-investigate.mdis excluded by!pilot/**pilot/skills/fix/steps/06-finalise.mdis excluded by!pilot/**pilot/skills/prd/orchestrator.mdis excluded by!pilot/**pilot/skills/prd/steps/01-understand.mdis excluded by!pilot/**pilot/skills/prd/steps/02-research.mdis excluded by!pilot/**pilot/skills/prd/steps/07-write-prd.mdis excluded by!pilot/**pilot/skills/prd/steps/08-handoff.mdis excluded by!pilot/**pilot/skills/setup-rules/steps/08-sync-mcp.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/01-create-header.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/02-investigation.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/04-write-plan.mdis excluded by!pilot/**pilot/skills/spec-bugfix-plan/steps/06-approval.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/03-verification-scenario.mdis excluded by!pilot/**pilot/skills/spec-bugfix-verify/steps/06-code-review-gate.mdis excluded by!pilot/**pilot/skills/spec-implement/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/01-setup.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/02-tdd-loop.mdis excluded by!pilot/**pilot/skills/spec-implement/steps/03-completion.mdis excluded by!pilot/**pilot/skills/spec-plan/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/02-create-header.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/03-workspace-scan.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/05-exploration.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/09-write-plan.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/10-plan-verification.mdis excluded by!pilot/**pilot/skills/spec-plan/steps/12-approval.mdis excluded by!pilot/**pilot/skills/spec-verify/orchestrator.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/01-launch-review.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/03-collect-results.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/07-e2e-and-final-regression.mdis excluded by!pilot/**pilot/skills/spec-verify/steps/10-review-gate.mdis excluded by!pilot/**pilot/skills/spec/orchestrator.mdis excluded by!pilot/**pilot/skills/spec/steps/00-permission-mode.mdis excluded by!pilot/**pilot/skills/spec/steps/01-parse-route.mdis excluded by!pilot/**pilot/ui/ExtensionsView.jsis excluded by!pilot/**pilot/ui/index.jsis excluded by!pilot/**pilot/ui/index2.jsis excluded by!pilot/**pilot/ui/index3.jsis excluded by!pilot/**pilot/ui/index4.jsis excluded by!pilot/**pilot/ui/viewer-bundle.jsis excluded by!pilot/**pilot/ui/viewer.cssis excluded by!pilot/**
📒 Files selected for processing (43)
.gitignoreREADME.mddocs/docusaurus/docs/features/cli.mddocs/docusaurus/docs/features/extensions.mddocs/docusaurus/docs/features/hooks.mddocs/docusaurus/docs/features/mcp-servers.mddocs/docusaurus/docs/features/open-source-tools.mddocs/docusaurus/docs/features/remote-control.mddocs/docusaurus/docs/getting-started/codex-cli.mddocs/docusaurus/docs/getting-started/installation.mddocs/docusaurus/docs/getting-started/prerequisites.mddocs/docusaurus/docs/intro.mddocs/docusaurus/docs/workflows/prd.mddocs/docusaurus/docs/workflows/quick-mode.mddocs/docusaurus/docusaurus.config.tsdocs/docusaurus/sidebars.tsdocs/site/index.htmldocs/site/public/manifest.jsondocs/site/src/components/DeploymentFlow.tsxdocs/site/src/components/FAQSection.tsxdocs/site/src/components/Footer.tsxdocs/site/src/components/HeroSection.tsxdocs/site/src/components/InstallSection.tsxdocs/site/src/components/Logo.tsxdocs/site/src/components/PricingSection.tsxdocs/site/src/components/SEO.tsxdocs/site/src/components/WhatsInside.tsxdocs/site/src/pages/Index.tsxdocs/site/src/pages/Shared.tsxinstall.shinstaller/cli.pyinstaller/steps/claude_files.pyinstaller/steps/codex_files.pyinstaller/steps/dependencies.pyinstaller/steps/finalize.pyinstaller/steps/shell_config.pyinstaller/tests/unit/steps/test_claude_files.pyinstaller/tests/unit/steps/test_codex_files.pyinstaller/tests/unit/steps/test_dependencies.pyinstaller/tests/unit/steps/test_finalize.pyinstaller/tests/unit/steps/test_shell_config.pyinstaller/ui.pyinstaller/upstreams.yaml
There was a problem hiding this comment.
9 issues found across 179 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pilot/skills/prd/steps/02-research.md">
<violation number="1" location="pilot/skills/prd/steps/02-research.md:26">
P2: Inconsistent MCP tool naming: uses underscores (`mcp__web_search__search`) here but hyphens (`mcp__web-search__search`) in the Deep Tier CODEX block below. Since `.mcp.json` defines the server as `web-search` (with hyphen), the hyphenated form is correct. The agent may fail to find the tool with the underscore variant.</violation>
</file>
<file name="docs/docusaurus/docs/features/remote-control.md">
<violation number="1" location="docs/docusaurus/docs/features/remote-control.md:62">
P2: Inconsistency: this step now says `claude` but the rest of the page (Setup section, Channels section) still uses `pilot`. If `pilot` is the command that loads hooks/rules/MCP servers, users following the SSH path would bypass that environment setup. Should this remain `pilot` for consistency, or should the other sections be updated as well?</violation>
</file>
<file name="installer/steps/shell_config.py">
<violation number="1" location="installer/steps/shell_config.py:33">
P1: `%self` does not expand when concatenated with other text in fish shell — `%self-(random)` will produce the literal string `%self-<number>` instead of `<PID>-<number>`. Use `$fish_pid` (available since fish 3.0) which works correctly in concatenation and is the recommended replacement for the deprecated `%self`.</violation>
</file>
<file name="pilot/hooks/codex_hooks.json">
<violation number="1" location="pilot/hooks/codex_hooks.json:14">
P1: `codegraph_init.py` (120s timeout) runs synchronously without `"async": true`, potentially blocking session start for up to 2 minutes. In `hooks.json` this is explicitly async to avoid blocking. If Codex supports the `async` property, consider adding it here to match the non-blocking behavior.</violation>
</file>
<file name="docs/docusaurus/docs/getting-started/codex-cli.md">
<violation number="1" location="docs/docusaurus/docs/getting-started/codex-cli.md:95">
P3: This statement is outdated: Codex hooks in this PR already register `file_checker`, `context_monitor`, `spec_stop_guard`, and `tool_token_saver`. Update the matrix text so setup expectations match the shipped Codex hook coverage.</violation>
</file>
<file name="pilot/hooks/license_check.py">
<violation number="1" location="pilot/hooks/license_check.py:48">
P2: The `else: _allow()` branch is unreachable dead code. By the time execution reaches line 43, `data.get("valid", False)` is guaranteed to be falsy (line 38 would have returned otherwise), so the `elif not data.get("valid", False)` on line 46 is always `True`. This makes the logic confusing and could mask a missed intent (e.g., did you mean to check a different condition in the elif?).
Consider simplifying to remove the redundant check and dead branch:</violation>
</file>
<file name="installer/steps/codex_files.py">
<violation number="1" location="installer/steps/codex_files.py:355">
P2: Avoid swallowing all exceptions while building Codex skills. Silent `except Exception: continue` can skip skill installation with no user-visible signal, leaving Codex workflows partially installed.</violation>
</file>
<file name="docs/docusaurus/docs/features/hooks.md">
<violation number="1" location="docs/docusaurus/docs/features/hooks.md:9">
P2: Hook count is incorrect: tables list 18 hooks total but the text claims 19. If the 2 command-scoped Stop hooks mentioned in prose are included, the count should be 20.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
Re-trigger cubic
| }, | ||
| { | ||
| "type": "command", | ||
| "command": "uv run --no-project python \"$HOME/.pilot/hooks/codegraph_init.py\"", |
There was a problem hiding this comment.
P1: codegraph_init.py (120s timeout) runs synchronously without "async": true, potentially blocking session start for up to 2 minutes. In hooks.json this is explicitly async to avoid blocking. If Codex supports the async property, consider adding it here to match the non-blocking behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pilot/hooks/codex_hooks.json, line 14:
<comment>`codegraph_init.py` (120s timeout) runs synchronously without `"async": true`, potentially blocking session start for up to 2 minutes. In `hooks.json` this is explicitly async to avoid blocking. If Codex supports the `async` property, consider adding it here to match the non-blocking behavior.</comment>
<file context>
@@ -0,0 +1,143 @@
+ },
+ {
+ "type": "command",
+ "command": "uv run --no-project python \"$HOME/.pilot/hooks/codegraph_init.py\"",
+ "timeout": 120
+ },
</file context>
- PaginationHelper: use COALESCE(s.agent, 'claude') so orphan observations aren't dropped when agent filter is applied (LEFT JOIN → INNER JOIN bug) - file_checker: strip trailing whitespace/CR from regex-captured paths to handle Windows line endings in apply_patch content - Memories UI: wrap JSON.parse of facts/concepts in safe helper to prevent render crash on malformed data - hook-command: restore suppressOutput:true in error handler else branch - codex_hooks.json: remove no-op context hook (handler early-returns for codex) - installer codex_files: restrict TOML key detection to top-level scope, catch _TomlStructureError, remove dead regexes, cleanup
|
Claude finished @maxritter's task in 3m 26s —— View job Incremental Code Review: Push to feat/Codex CLI PRTasks:
✅ Resolved from Prior Review1. TOML key detection bug (section-scoped false positive) — 2. 3. Dead code: 4. Redundant local 5. 6. 7. New Issues in This Push1. Stale The docstring still says: But this push removed the Still Open (No Change in This Push)
Code Quality Notes (New in Push)
The prior two-level alert system (65% inform, 75% autocompact) has been replaced with a single alert at 90%. The
The |
There was a problem hiding this comment.
1 issue found across 20 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/docusaurus/docs/features/remote-control.md">
<violation number="1" location="docs/docusaurus/docs/features/remote-control.md:62">
P2: Inconsistency: this step now says `claude` but the rest of the page (Setup section, Channels section) still uses `pilot`. If `pilot` is the command that loads hooks/rules/MCP servers, users following the SSH path would bypass that environment setup. Should this remain `pilot` for consistency, or should the other sections be updated as well?</violation>
</file>
<file name="pilot/hooks/codex_hooks.json">
<violation number="1" location="pilot/hooks/codex_hooks.json:14">
P1: `codegraph_init.py` (120s timeout) runs synchronously without `"async": true`, potentially blocking session start for up to 2 minutes. In `hooks.json` this is explicitly async to avoid blocking. If Codex supports the `async` property, consider adding it here to match the non-blocking behavior.</violation>
</file>
<file name="pilot/hooks/context_monitor.py">
<violation number="1" location="pilot/hooks/context_monitor.py:290">
P2: Unreachable dead code: the two preceding conditions (`< THRESHOLD_AUTOCOMPACT` and `>= THRESHOLD_AUTOCOMPACT`) are exhaustive, so this `save_cache` call and `return 0` can never execute. Remove these lines or restructure the logic if a third branch was intended.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
- Fix CI: add command_exists early-exit to install_pbt_tools - Fix fish shell %self not expanding in concatenation (use $fish_pid) - Fix silent exception swallowing in codex_files skill builder - Add CODEX_HOME absolute path validation - Remove unreachable else branch in license_check.py - Fix HeroSection whitespace-nowrap clipping on mobile - Fix MCP tool naming (underscores → hyphens) in PRD skill - Fix test_codex_rules local_mode mock (was truthy, skipping Path.home path) - Add missing test assertions for dependency orchestration - Update installation.md step count (7 → 8) with Codex files row - Update codex-cli.md hooks section to match actual coverage - Fix hooks.md count (19 → 18) to match table rows - Add language tags to fenced code blocks in extensions.md
|
Claude finished @maxritter's task in 4m 4s —— View job Incremental Code Review: Push to feat/Codex CLI PRTasks:
✅ Resolved from Prior ReviewsThe following issues from prior Claude, CodeRabbit, and Cubic reviews are now addressed:
New Issue
if command_exists("hypothesis") and command_exists("fast-check"):
_record_outcome(_OUTCOME_UNCHANGED)
return True
The prior implementation correctly used def _is_npm_package_installed(package: str) -> bool:
try:
result = subprocess.run(
["npm", "list", "-g", package, "--depth=0"],
capture_output=True, text=True, timeout=15,
)
return result.returncode == 0 and package in result.stdout
except Exception:
return False
if command_exists("hypothesis") and _is_npm_package_installed("fast-check"):
_record_outcome(_OUTCOME_UNCHANGED)
return TrueStill Open (not in this push)
|
There was a problem hiding this comment.
3 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/docusaurus/docs/features/remote-control.md">
<violation number="1" location="docs/docusaurus/docs/features/remote-control.md:62">
P2: Inconsistency: this step now says `claude` but the rest of the page (Setup section, Channels section) still uses `pilot`. If `pilot` is the command that loads hooks/rules/MCP servers, users following the SSH path would bypass that environment setup. Should this remain `pilot` for consistency, or should the other sections be updated as well?</violation>
</file>
<file name="pilot/hooks/codex_hooks.json">
<violation number="1" location="pilot/hooks/codex_hooks.json:14">
P1: `codegraph_init.py` (120s timeout) runs synchronously without `"async": true`, potentially blocking session start for up to 2 minutes. In `hooks.json` this is explicitly async to avoid blocking. If Codex supports the `async` property, consider adding it here to match the non-blocking behavior.</violation>
</file>
<file name="pilot/hooks/context_monitor.py">
<violation number="1" location="pilot/hooks/context_monitor.py:290">
P2: Unreachable dead code: the two preceding conditions (`< THRESHOLD_AUTOCOMPACT` and `>= THRESHOLD_AUTOCOMPACT`) are exhaustive, so this `save_cache` call and `return 0` can never execute. Remove these lines or restructure the logic if a third branch was intended.</violation>
</file>
<file name="installer/steps/codex_files.py">
<violation number="1" location="installer/steps/codex_files.py:44">
P2: Unhandled `ValueError` will crash the installer if `CODEX_HOME` is set to a relative path. Neither the `run()` method nor the step orchestrator catch this exception — only `_TomlStructureError` and `KeyboardInterrupt` are handled. Wrap the early `_get_codex_config_dir()` call in `run()` with a `try/except ValueError` that reports via `ctx.ui.warning()` and returns gracefully.</violation>
</file>
<file name="installer/steps/dependencies.py">
<violation number="1" location="installer/steps/dependencies.py:1095">
P2: `command_exists("fast-check")` will always be False because `fast-check` is an npm library without a CLI binary — this guard never triggers. If the intent is to skip reinstallation when already present, `fast-check` needs a different existence check (e.g., checking the npm global `node_modules` directory). Also, skipping the `--upgrade` flag when the tools are already present breaks the installer's idempotent upgrade contract.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
…ction - Installer post-steps: show all 6 Codex skills (was 3), remove Claude Code Only section - Enable mem-search MCP for Codex: remove _CODEX_SKIP_MCP entirely - Enable memory context injection for Codex: remove early-return in context handler, restore context hook in codex_hooks.json - codex-cli.md: remove Console Views table (all ✅), update MCP list (all 7 servers), remove memory/context from Claude Code-only table, update hooks section - README: update Codex skill list (all 6), remove persistent memory from limitations - prerequisites.md: update Codex description - FAQSection.tsx: remove persistent memory from limitations - Test updates: add benchmark/setup-rules/create-skill to Codex skill safety tests, update invocation regex, fix context handler test to expect injection - create-skill: simplify allowed-tools examples to pass Codex safety checks - Codex rules audit: all rules properly adapted via CC-ONLY/CODEX blocks
|
Claude finished @maxritter's task in 3m 48s —— View job Incremental Code Review: Push to feat/Codex CLI PRTasks:
✅ Resolved from Prior Reviews
New Issue in This Push
The doc now says in two places:
The table row still shows Codex using "Codex built-in memories ( Fix: update the Codex column of the Memory row to reflect both: | Memory | Pilot Console (mem-search MCP) | Pilot Console (mem-search MCP) + Codex built-in memories (`features.memories`) |or, if Pilot Console memory fully supersedes the Codex built-in for Pilot users: | Memory | Pilot Console (mem-search MCP) | Pilot Console (mem-search MCP) |Still Open (not in this push)
|
…ificOutput
Codex CLI doesn't support Claude Code's additionalContext protocol — returning
hookSpecificOutput causes 'invalid session start JSON output' error. Instead:
- Return { continue: true } (valid Codex JSON)
- Write compact one-liner to stderr: '[pilot] Memory loaded for [project]'
- mem-search MCP still available for on-demand memory lookups
|
Claude finished @maxritter's task in 3m 36s —— View job Incremental Code Review: Push to feat/Codex CLI PRTasks:
✅ Resolved in This Push
New Issue in This Push
The new context handler correctly differentiates platforms in the main path: // ✅ Correct: platform-aware in main path
let l=(await o.text()).trim();
if(t.platform==="codex"){
// ...
return {continue: true}
}
return {hookSpecificOutput:{hookEventName:"SessionStart",additionalContext:l}}But the early-return when // ❌ Missing platform check
if(process.env.CLAUDE_PILOT_NO_CONTEXT==="1"||...)
return {hookSpecificOutput:{hookEventName:"SessionStart",additionalContext:""}}For a Codex user with if(process.env.CLAUDE_PILOT_NO_CONTEXT==="1"||...) {
if(t.platform==="codex") return {continue: true};
return {hookSpecificOutput:{hookEventName:"SessionStart",additionalContext:""}}
}Still Open (no change in this push)
|
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pilot/scripts/worker-service.cjs">
<violation number="1">
P2: The condition `p>0` is always true because `split("\n")` returns at least one element even on an empty string. When the context API returns empty content, users will still see "Memory loaded... — 1 lines" which is misleading. The check should guard against empty content (e.g. `l.length > 0`) rather than line count.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
- Add CC-ONLY/CODEX blocks to 5 Skill() calls in spec-verify and spec-bugfix-verify that would break the verify→implement loop in Codex - Add orchestrator-level Codex AskUserQuestion handling to fix, prd, benchmark, and setup-rules skills - Add CODEX blocks to mcp-servers.md (ToolSearch → tool_search) and browser-automation.md (Chrome tool detection) - Fix wrong CodeGraph tool names in spec-implement (trace_call_path → codegraph_callers/callees, search_graph → codegraph_search) - Add CODEX blocks to spec-plan step 00 and setup-rules step 01 for AskUserQuestion consistency
|
Claude finished @maxritter's task in 5m 12s —— View job Incremental Code Review: Latest Push to PR #153Tasks:
Summary of This PushThis push wires up full Codex compatibility across the benchmark runner, create-skill, setup-rules, fix, prd, spec-verify, spec-bugfix-verify, and spec-implement skills. The main patterns used are correct: CC-ONLY/CODEX-START conditional blocks for platform-divergent instructions, AskUserQuestion → plain-text numbered options for Codex, and Bug1. Grader model sentinel
grader_model: str = args.grader_model or (
CODEX_DEFAULT_MODEL if grader_agent == "codex" and agent != "codex" else executor_model
)When
Then claude -p --output-format text --model codex-default
Fix in grader_model: str = args.grader_model or (
CODEX_DEFAULT_MODEL if grader_agent == "codex"
else executor_model if executor_model != CODEX_DEFAULT_MODEL
else resolve_executor_model(target)
)This covers the cross-agent case: Codex executor + Claude grader falls back to the skill's declared model (same fallback Claude uses when run alone). Minor Issue2. CC-ONLY/CODEX-START markers placed inside fenced code blocks
In both files, the conditional markers appear as content inside a |
There was a problem hiding this comment.
1 issue found across 30 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pilot/hooks/codex_hooks.json">
<violation number="1" location="pilot/hooks/codex_hooks.json:14">
P1: `codegraph_init.py` (120s timeout) runs synchronously without `"async": true`, potentially blocking session start for up to 2 minutes. In `hooks.json` this is explicitly async to avoid blocking. If Codex supports the `async` property, consider adding it here to match the non-blocking behavior.</violation>
</file>
<file name="installer/steps/codex_files.py">
<violation number="1" location="installer/steps/codex_files.py:44">
P2: Unhandled `ValueError` will crash the installer if `CODEX_HOME` is set to a relative path. Neither the `run()` method nor the step orchestrator catch this exception — only `_TomlStructureError` and `KeyboardInterrupt` are handled. Wrap the early `_get_codex_config_dir()` call in `run()` with a `try/except ValueError` that reports via `ctx.ui.warning()` and returns gracefully.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
- README + Docusaurus restructured: prerequisites, examples, and feature pages treat Claude Code and Codex as equals (drop Claude-only framing). - Console: dashboard / sessions / memories / requirements show consistent agent badges; settings page agent styling matches across the app. - Console Help page renamed to Documentation and moved below Settings; vim-style G-sequence shortcuts removed; sidebar ordering: Usage → Settings → Documentation. - Console screenshot assets renamed help.webp → documentation.webp (full, _sm, thumbs) and surfaced in marketing site. - Codex usage cost calculation fixed — parser now reads model from JSONL turn_context.payload.model instead of relying on the brittle ~/.codex/state_5.sqlite lookup. Previously all Codex sessions resolved to model='unknown' and total cost = $0. - Codex skill migration: skills, hooks, and installer support both agents via shared .pilot/hooks layout and Codex agent skills under ~/.agents/skills/. uninstall.sh cleans Codex artifacts too. - Statusline simplified to surface only Pilot Shell updates (Claude/Codex CLI updates removed — agents manage their own update prompts). - Website SEO updated: index.html keywords, HowTo schema, FAQ schema, noscript fallback, SEO component defaults, and Docusaurus config (blog/RSS descriptions, metadata keywords) all include Codex/GPT-5. - pilot/scripts and pilot/ui bundles rebuilt against the new console source.
|
Claude finished @maxritter's task in 3m 35s —— View job Incremental Code Review: Latest Push to PR #153Tasks:
✅ Resolved by This Push
New Issue in This Push
- **`/prd`** — brainstorm ideas into clear requirements through with optional deep research"through with" is ungrammatical. Based on the fuller - **`/prd`** — brainstorm ideas into clear requirements with optional deep researchor - **`/prd`** — brainstorm ideas into clear requirements through optional deep researchStill Open (no change in this push)
|
There was a problem hiding this comment.
4 issues found across 186 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.md">
<violation number="1" location="README.md:46">
P3: Fix the wording typo here (`through with`) to keep the `/prd` description clear and grammatically correct.</violation>
</file>
<file name="docs/docusaurus/docs/features/remote-control.md">
<violation number="1" location="docs/docusaurus/docs/features/remote-control.md:66">
P2: Contradicts the warning at the top of the page. The SSH section is about starting sessions visible via Remote Control, but the warning states Remote Control is not available with Codex CLI. Suggesting `codex` here will confuse users.</violation>
</file>
<file name="pilot/skills/prd/steps/04-clarify.md">
<violation number="1" location="pilot/skills/prd/steps/04-clarify.md:9">
P2: The Codex-specific block contradicts the unconditional instruction above it ("⛔ ALWAYS use the `AskUserQuestion` tool — never list numbered questions in plain text"). When CC-ONLY blocks are stripped for Codex, the agent sees both "never use plain text" and "ask one bundled plain-text prompt." Either wrap the unconditional line in `` with a Codex alternative, or rephrase the Codex block to avoid contradicting the shared rule.</violation>
</file>
<file name="docs/docusaurus/docs/workflows/create-skill.md">
<violation number="1" location="docs/docusaurus/docs/workflows/create-skill.md:36">
P2: Inconsistency: Phase 3 only mentions `~/.agents/skills/` (global) for Codex, but the info box at the bottom lists both `.agents/skills/` (project) and `~/.agents/skills/` (global). The Phase 3 description should include both paths for Codex to match.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
Fix all with cubic | Re-trigger cubic
…them
- Add cmd_install agent prerequisite gate (EXIT 1 with README links when
neither Claude Code nor Codex CLI detected)
- Remove install_claude_code + upstreams.yaml entry: installer no longer
installs either agent (README contract)
- Three sibling file steps with clear separation:
* PilotFilesStep — always; ~/.pilot/* runtime + canonical skill source +
~/.pilot/rules/ raw sources read by Codex adapter
* ClaudeFilesStep — gated on is_claude_installed(); ~/.claude/{rules,
agents,settings.json} + Claude post-install merges
* CodexFilesStep — gated on is_codex_installed(); ~/.agents/skills/ +
~/.codex/*, per-category output mirroring Claude
- Fix is_claude_installed paths to launcher canonical set
(~/.claude/local/bin/claude + Claude.app bundle)
- Unify agent detection: dependencies._init_rtk and codex_files now use
platform_utils.is_*_installed
- PilotFilesStep switches inheritance → composition over ClaudeFilesStep
to avoid LSP violation
- CodexFilesStep output via _CodexReport accumulator + label formatters
- Promote cache keys to named constants; remove dead
_post_install_processing and dead _get_cached_pilot_files fallback
- Update README, Docusaurus installation guide, installer patterns rule
|
Claude finished @maxritter's task in 4m 21s —— View job Claude finished @maxritter's task in 4m 11s —— View job Incremental Code Review: Latest Push to PR #153Tasks:
Summary of This PushMajor installer refactor: the monolithic The new tests are thorough: ✅ Resolved from Prior Reviews
New Issue
- return True # fail-open: exception → allow skill sync
+ return False # fail-closed: exception → remove all Codex skillsWhen This affects licensed users with transient except (OSError, subprocess.TimeoutExpired, json.JSONDecodeError, ValueError):
return None # sentinel: "unknown" → preserve existing skills, don't sync or removeThen in the caller: valid = _check_license()
if valid is None:
print(json.dumps({"continue": True})) # unknown: leave skills as-is
returnDesign Note (No Action Required)
This works in normal mode because Still Open (no change in this push)
|
There was a problem hiding this comment.
2 issues found across 74 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pilot/hooks/codex_hooks.json">
<violation number="1" location="pilot/hooks/codex_hooks.json:14">
P1: `codegraph_init.py` (120s timeout) runs synchronously without `"async": true`, potentially blocking session start for up to 2 minutes. In `hooks.json` this is explicitly async to avoid blocking. If Codex supports the `async` property, consider adding it here to match the non-blocking behavior.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
Fix all with cubic | Re-trigger cubic
|
Claude finished @maxritter's task in 3m 29s —— View job Claude finished @maxritter's task in 4m 30s —— Incremental Code Review: Latest Push to PR #153Tasks:
✅ Resolved from Prior Reviews
New Issue in This PushTest name is misleading and the new rules-path
The new test The test name says "during_rules_install" but is actually testing "during_agents_install." The rules-specific handler (the newly added Still Open (no change in this push)
|
# [9.1.0](v9.0.6...v9.1.0) (2026-05-28) ### Features * enable Codex CLI as a first-class agent alongside Claude Code ([#153](#153)) ([fdbe276](fdbe276))
|
🎉 This PR is included in version 9.1.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Full dual-agent support so Codex CLI users get the same Pilot Shell experience as Claude Code users across all Console features.
What changed
Sessions: CodexSessionReader parses
~/.codex/sessions/JSONL files, merged with Claude sessions in Dashboard and Sessions views with agent badge and All/Claude/Codex filter toggle.Usage: Codex cost tracking via codex-parser with model-specific pricing, aggregated into daily/monthly charts with agent filtering.
Memories: Hook pipeline fixed for Codex (
hookSpecificOutputpassthrough instead ofsystemMessage, context handler enabled, no ugly display), all memory lifecycle hooks added (session-init, observation, summarize, session-end), agent column migration onsdk_sessions, observations API agent filter, Memories UI toggle.Extensions: Codex extensions discovery and display alongside Claude extensions in the Extensions view.
Quality hooks:
file_checker(withapply_patchformat support),tool_token_saver,context_monitor(transcript estimation fallback),spec_stop_guard— all ported tocodex_hooks.json.Installer:
codex_filesstep installs hooks, skills, MCP config, andAGENTS.mdfor Codex CLI. Agent runtime detection for both CLIs.Docs: Codex CLI getting-started guide, updated hooks/extensions/MCP docs for dual-agent coverage.
Stats
Summary by cubic
Enable Codex CLI as a first-class agent alongside Claude Code. Running
claudeorcodexnow launches the same Pilot Shell with shared sessions, memories, MCP servers, extensions, and usage tracking.New Features
~/.codex/sessions/*.jsonlwith agent badges and All/Claude/Codex filters; Codex cost parser with model pricing and correct aggregation.mem-search.CC-ONLY/CODEXblocks; orchestrator-level AskUserQuestion handling infix,prd,benchmark, andsetup-rules..pilot/hooksand Codex skills under~/.agents/skills; Console getsAgentBadge+ filters and a new Documentation view; status line simplified.Bug Fixes
turn_context.payload.modelso charts show correct costs.COALESCE(..., 'claude')to keep orphan observations; safe JSON parsing for memory facts/concepts.file_checkertrims CR/LF inapply_patchpaths; restoresuppressOutput: true; correct CodeGraph tool names; fix MCP tool naming in docs.CODEX_HOMEvalidation; unified agent detection; uninstall cleans Codex artifacts; Python tools upgraded viauv.{ continue: true }and emit a compact “[pilot] Memory loaded …” to stderr to avoid invalid JSON at startup.Written for commit 492c6fa. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
New Features
claude/codex; Pilot Shell auto-loads. Hooks pipeline expanded (license checks, CodeGraph init, spec handoff/resume).Documentation
Chores
Tests