Hosted Codex PR review support (AI-632)

[alexeyzimarev]

What

Adds hosted Codex PR review support (AI-632). Previously the daemon hard-rejected LaunchKind.Review + vendor: codex with "PR review for Codex is not yet supported." A hosted Codex agent can now review a PR with the same kcap-review MCP context the Claude path gets.

How (spike-grounded)

Verified against codex-cli 0.142.3. The Codex path turned out simpler than Claude's — it needs no temp files:

• MCP injection is ephemeral. The same kcap mcp review stdio server is registered via -c mcp_servers.kcap-review.{command,args,env} overrides — no ~/.codex/config.toml mutation, so nothing to clean up. Confirmed all three forms (command, args array, env inline-table) register via codex mcp get.
• No --system-prompt flag exists. The rendered prompt-review.txt is passed as Codex's initial prompt (positional -- arg), leaving Codex's base instructions intact.

Changes

1. ReviewLaunchBuilder — now vendor-aware and takes an explicit kcap-CLI path; exposes a vendor-neutral ReviewMcpServer descriptor and writes the temp --mcp-config JSON only for Claude.
   • Bug fix (also affected Claude): the builder used Environment.ProcessPath, which inside the daemon resolves to the separate kcap-daemon binary (no mcp review subcommand). The daemon now passes _config.CapacitorPath; the interactive kcap review CLI passes Environment.ProcessPath ?? "kcap".
2. CodexLauncher.BuildArgs — review branch injecting the -c MCP overrides + prompt, with an AOT-safe TomlString encoder. Returns McpConfigPath: null.
3. AgentOrchestrator — removed the Codex-review rejection; Codex reviews now flow through the same validation/launcher path as Claude.
4. README — documents hosted Codex review support.

The Claude review path is unchanged at runtime (only compile-time ! null-forgiving edits); effort handling stays at parity (review branches pass --model/-m only, no --effort).

Test plan

• New unit tests: ReviewLaunchBuilderTests (vendor split, descriptor, CLI-path), CodexLauncherTests review branch (the three -c overrides, prompt-after---, no --system-prompt, null McpConfigPath, TOML escaping).
• Replaced the Codex-review rejection test with a gate-removal test (Codex review now reaches the shared origin validation, not the old rejection).
• Gates: unit 1860/1860, integration 45/45, dotnet publish -c Release clean (no IL3050/IL2026).
• CLI-level verification: codex mcp get kcap-review with the injected -c overrides resolves to stdio / command=kcap / args=mcp review --owner … --pr … / env KCAP_URL.

Remaining manual acceptance (needs a live server + PR, per the spec): launch a hosted Codex review against a real PR and confirm the agent calls get_pr_summary against the PR head ref.

Out of scope

Windows (AI-72), the generic sandbox/approval selector (AI-633), the persistent reviewer loop (AI-774), and a Codex-tailored review prompt.

Design & plan: docs/superpowers/specs/2026-06-29-ai-632-codex-pr-review-support-design.md, docs/superpowers/plans/2026-06-29-ai-632-codex-pr-review-support.md.

Closes AI-632.

🤖 Generated with Claude Code

[linear-code]

AI-632

[qodo-code-review]

PR Summary by Qodo

Add hosted Codex PR review support via kcap-review MCP injection

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Enable hosted Codex agents to run PR reviews using the existing kcap-review MCP context.
• Fix daemon PR-review MCP command to use the kcap CLI path (not kcap-daemon).
• Add unit coverage for vendor-specific review launch args and update user docs.

Diagram

graph TD
  A["LaunchAgentCommand (Review)"] --> B["AgentOrchestrator"] --> C["ReviewLaunchBuilder"] --> D{Vendor}
  D --> E["ClaudeLauncher"] --> F{{"claude"}} --> G[("temp --mcp-config JSON")]
  D --> H["CodexLauncher"] --> I{{"codex"}}
  C --> J{{"kcap CLI"}}

  subgraph Legend
    direction LR
    _svc[Service/Module] ~~~ _dec{Decision} ~~~ _ext{{External CLI}} ~~~ _file[(Temp file)]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use Codex-native review subcommand

• ➕ Avoids MCP server injection complexity
• ➕ May produce more “review-shaped” Codex UX (depending on Codex CLI features)
• ➖ Does not naturally integrate the existing kcap-review MCP tools/context used for rich PR data
• ➖ Appears focused on local diffs rather than hosted PR-head context and tool calls
• ➖ Would diverge behavior from Claude path and likely require more product/design work

2. Write a temporary Codex `config.toml` (or patch `~/.codex/config.toml`)

• ➕ Could reuse a single configuration mechanism across invocations
• ➖ Requires safe mutation + robust cleanup on crashes
• ➖ Higher risk of leaving user config in a bad state
• ➖ More complex than ephemeral -c overrides that Codex already supports

3. Always generate a temp MCP config artifact for both vendors

• ➕ One artifact shape for both vendors (conceptually simpler)
• ➕ Potentially easier to debug by inspecting a single file
• ➖ Codex doesn’t accept Claude’s --mcp-config JSON shape; would still need vendor-specific translation
• ➖ Adds unnecessary filesystem lifecycle to Codex path (the PR intentionally avoids this)

Recommendation: Keep the current approach: build a vendor-neutral MCP descriptor once, then adapt it per vendor (Claude: temp --mcp-config JSON; Codex: ephemeral -c mcp_servers.kcap-review.*). It minimizes user side effects (no persistent config changes), keeps Claude behavior stable, and fixes a real daemon bug by explicitly threading the kcap CLI path.

Files changed (11) +1050 / -71

Enhancement (2) +50 / -7

AgentOrchestrator.csAllow Codex PR review launches and pass vendor/CLI path to builder +1/-7

Allow Codex PR review launches and pass vendor/CLI path to builder

• Removes the hard rejection for 'LaunchKind.Review' with 'vendor=codex'. Updates review launch construction to call 'ReviewLaunchBuilder.BuildAsync(cmd.Vendor, _config.CapacitorPath, ...)' so the injected MCP server uses the correct 'kcap' binary.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

CodexLauncher.csAdd review-mode args: MCP via '-c' overrides and prompt after '--' +49/-0

Add review-mode args: MCP via '-c' overrides and prompt after '--'

• Adds a review branch that injects the 'kcap-review' MCP server via '-c mcp_servers.kcap-review.{command,args,env}' overrides and passes the rendered review prompt as Codex’s initial prompt. Introduces a small TOML basic-string encoder for AOT-safe escaping.

src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs

Bug fix (1) +48 / -32

ReviewLaunchBuilder.csMake ReviewLaunchBuilder vendor-aware and thread explicit kcap CLI path +48/-32

Make ReviewLaunchBuilder vendor-aware and thread explicit kcap CLI path

• Refactors the builder to return a vendor-neutral 'ReviewMcpServer' descriptor plus the rendered review prompt. Writes a temp MCP JSON file only for Claude and fixes the daemon path by using an explicit 'cliPath' instead of 'Environment.ProcessPath'.

src/Capacitor.Cli.Core/Commands/ReviewLaunchBuilder.cs

Refactor (2) +5 / -4

ClaudeLauncher.csAssert non-null MCP config path for Claude reviews +1/-1

Assert non-null MCP config path for Claude reviews

• Updates the review argv construction to use 'launch.McpConfigPath!', reflecting the now-nullable field for non-Claude vendors while preserving Claude’s runtime behavior.

src/Capacitor.Cli.Daemon/Services/ClaudeLauncher.cs

ReviewCommand.csUpdate interactive review command to use new builder API +4/-3

Update interactive review command to use new builder API

• Updates the interactive 'kcap review' command to call the new vendor-aware builder API and to non-null-assert the MCP config path for Claude. Keeps best-effort temp-file cleanup behavior intact.

src/Capacitor.Cli/Commands/ReviewCommand.cs

Tests (3) +131 / -27

AgentOrchestratorVendorTests.csReplace Codex review rejection test with gate-removal validation test +34/-27

Replace Codex review rejection test with gate-removal validation test

• Reworks the Codex review test to assert the vendor gate is removed by ensuring a Codex review proceeds to shared origin validation (and fails there for a repo without an origin remote). Confirms no PTY spawn occurs in this failure mode.

test/Capacitor.Cli.Tests.Unit/AgentOrchestratorVendorTests.cs

CodexLauncherTests.csAdd unit tests for Codex review argv composition and TOML escaping +54/-0

Add unit tests for Codex review argv composition and TOML escaping

• Adds review-context helpers and tests ensuring review launches inject the three MCP '-c' overrides, place the prompt after '--', omit '--system-prompt', return 'McpConfigPath=null', and properly escape backslashes/quotes in TOML strings.

test/Capacitor.Cli.Tests.Unit/Codex/CodexLauncherTests.cs

ReviewLaunchBuilderTests.csAdd unit tests for ReviewLaunchBuilder vendor split and descriptor output +43/-0

Add unit tests for ReviewLaunchBuilder vendor split and descriptor output

• Adds tests verifying Claude writes a temp MCP config file and populates the MCP descriptor, while Codex writes no file but still returns the fully populated descriptor and rendered prompt placeholders.

test/Capacitor.Cli.Tests.Unit/ReviewLaunchBuilderTests.cs

Documentation (3) +816 / -1

README.mdDocument hosted Codex PR review support +1/-1

Document hosted Codex PR review support

• Updates the README to state that hosted Codex agents can now perform PR reviews. Clarifies that both Claude and Codex use the same injected 'kcap-review' MCP context, while sandbox/approval selectors remain a follow-up.

README.md

2026-06-29-ai-632-codex-pr-review-support.mdAdd implementation plan for hosted Codex PR reviews +594/-0

Add implementation plan for hosted Codex PR reviews

• Introduces a detailed step-by-step implementation plan describing vendor-aware review launch building, Codex '-c' MCP injection, orchestrator gate removal, and verification steps (including AOT constraints and test commands).

docs/superpowers/plans/2026-06-29-ai-632-codex-pr-review-support.md

2026-06-29-ai-632-codex-pr-review-support-design.mdAdd design spec for Codex hosted PR review flow +221/-0

Add design spec for Codex hosted PR review flow

• Adds a design doc covering the Codex CLI spike results (ephemeral MCP injection and lack of '--system-prompt'), current architecture touchpoints, the CLI-path bug in the daemon, and the approved testing strategy.

docs/superpowers/specs/2026-06-29-ai-632-codex-pr-review-support-design.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Incomplete TOML escaping ✓ Resolved 🐞 Bug ≡ Correctness

Description

CodexLauncher.TomlString only escapes backslashes and double-quotes, so control characters (e.g.,
tab/newline) in injected values can produce invalid TOML in the -c mcp_servers.kcap-review.*
overrides and cause Codex review launches to fail at config-parse time.

Code

src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs[R142-145]

+    /// Encode a value as a TOML basic string: wrap in double quotes and escape
+    /// backslashes and double quotes (covers Windows paths and arbitrary URLs).
+    static string TomlString(string value) =>
+        "\"" + value.Replace("\\", "\\\\").Replace("\"", "\\\"") + "\"";

Evidence

Codex review-mode args are constructed by embedding TOML fragments via -c, and the encoder used
for command/args/env values only escapes \\ and ". The design spec for this change explicitly
requires escaping control chars as well, and the daemon config validation does not constrain
CapacitorPath (used as the injected MCP command) to exclude such characters, so malformed TOML is
possible with certain inputs.

src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs[103-145]
docs/superpowers/specs/2026-06-29-ai-632-codex-pr-review-support-design.md[133-138]
src/Capacitor.Cli.Daemon/DaemonConfig.cs[43-76]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`CodexLauncher` builds Codex review-mode MCP injection using TOML `-c` overrides. The current `TomlString` helper only escapes `\\` and `"`, but TOML basic strings also require escaping control characters (e.g. `\n`, `\t`, `\r`, and other `U+0000..U+001F`). If any injected value contains these characters (misconfigured CLI path, unexpected metadata, etc.), the override string becomes invalid TOML and Codex can fail before starting the session.

### Issue Context
The design spec for AI-632 explicitly calls for escaping control chars in the TOML encoder.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/CodexLauncher.cs[120-145]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---