implement-review: wire the reserved `/implement-review mcp` channel via codex mcp-server (Windows antivirus caveat)

[yzhao062]

Summary

The implement-review skill reserves a /implement-review mcp trigger as a "forward-compat slot" but does not implement it (skills/implement-review/SKILL.md:165: an MCP-based Codex channel, "currently out of scope"). On Windows the codex mcp-server MCP transport now drives a Codex review fully automatically, sidestepping the fragile codex exec subprocess path. This issue tracks promoting MCP to a real, preferred auto channel, with the important caveat that the MCP/Codex path has previously tripped Windows antivirus and is currently working for reasons not yet understood.

Current channels

SKILL.md supports three paths to Codex:

• Terminal-relay (default, manual copy-paste)
• Auto-terminal (codex exec subprocess via dispatch-codex.{ps1,sh}, plus a Copilot backend dispatch-copilot.{ps1,sh} for the agent-fungibility case)
• Plugin (user-initiated)

MCP is only a reserved trigger slot, not a channel.

What works now (Windows)

• claude mcp list shows the Codex MCP server connected:
  codex: .../npm/codex mcp-server -c approval_policy=on-failure - Connected
• In a session where its tools are exposed, Claude can call the Codex MCP tool directly: Codex reviews in-process and returns findings. No terminal spawn, no codex exec, no copy-paste. A recent watchlist-monitor review used this path successfully.

Why this was deferred: Windows antivirus

Previously, exercising the /implement-review mcp / codex mcp-server path tripped the system antivirus on Windows (maintainer-observed). That interaction is a likely reason MCP was kept out of scope. It is working again now for unknown reasons (AV definitions update, an added exclusion, or a changed binary). The intermittency is the core risk: an AV false-positive that blocks codex mcp-server would silently break the channel.

Reliability gotcha: "connected" is not "callable"

Observed this session: claude mcp list reports the codex server Connected, yet its tools were not exposed to the agent (ToolSearch could not surface a codex tool; it was absent from the session's deferred-tool list). A connected server does not guarantee the channel is usable in a given session. This argues against making MCP the only auto path.

Proposed design

Per AGENTS.md "Agent Fungibility" (keep alternatives reachable):

• Implement the reserved trigger pattern: /implement-review mcp slash arg plus plain phrases (use mcp, mcp mode) under the existing negation guard (SKILL.md:165).
• Dispatch via the Codex MCP tool instead of codex exec.
• Make MCP the preferred auto channel on Windows when the server is reachable.
• Keep codex exec (Auto-terminal) and the Copilot backend as fallbacks: when the MCP server is not running, not exposed in the session, or AV-blocked; and for the fungibility case (Claude absent, Codex cannot self-review, so Copilot reviews).

Open questions / to investigate

1. AV root cause and mitigation: what exactly did the antivirus flag (the codex mcp-server process, a dispatch script, a temp file)? Document a recommended AV exclusion, or detection-and-graceful-fallback so an AV block degrades to codex exec / terminal-relay instead of hanging.
2. Tool-exposure reliability: detect when the codex MCP tools are connected-but-not-callable and fall back automatically.
3. Dispatch implementation: how the skill invokes the MCP tool (the MCP call replaces the dispatch-codex subprocess), including the prompt/response file contract the existing health-check and review-loop machinery rely on.
4. Cross-agent parity: the same MCP-as-reviewer idea for the Claude / Copilot backends.

Acceptance criteria

• /implement-review mcp (and use mcp / mcp mode) selects an MCP-dispatched Codex review.
• A bare /implement-review auto prefers MCP when reachable, else falls back to codex exec, else terminal-relay, with the chosen channel logged.
• The AV interaction is documented (exclusion or graceful degradation), so an AV block does not hang the loop.
• Fallback channels remain reachable per fungibility.

[yzhao062]

Decision: build it, scoped as a standby (start tomorrow)

Scoping down from the "preferred channel" vision in the body to what we actually want first. codex exec (Auto-terminal) is running well right now, so MCP is being added as a standby / insurance channel, not a replacement.

Tomorrow's bar — minimal /implement-review mcp opt-in

• Wire the reserved trigger (/implement-review mcp slash arg + use mcp / mcp mode plain phrases, under the existing negation guard at SKILL.md:165) to dispatch a Codex review through the connected codex mcp-server instead of codex exec.
• Done = it reliably runs and returns a review when invoked. That is the whole bar for v1.
• codex exec stays the default primary; it is not being touched or replaced.

Why we are doing it (positioning)

MCP is the validated alternate path for when codex exec regresses. It has been intermittently broken on Windows (the precipitating "codex exec still broken" note, plus the run of Windows-specific fixes in git history), so having a second channel that is known to work means the review gate is not a single point of failure. The point is option value, not a speed upgrade (the Codex review compute dominates either way).

Cheap safety worth doing in the same pass (should, not a hard gate)

If the MCP server is unreachable, AV-blocked, or connected-but-not-callable (the two failure modes observed: a Windows-AV history, and claude mcp list showing Connected while the tools were not exposed to the session), surface a clear error pointing at codex exec rather than hanging. Because v1 is a manual opt-in, "notice and fall back by hand" is acceptable, so this is a should, not a must.

Deferred to a later phase (NOT tomorrow)

• Promoting MCP to the preferred/default auto channel.
• The full auto-detect-and-degrade ladder (MCP -> codex exec -> terminal-relay) as the default path. That robustness becomes a must only if/when MCP is promoted to default; the acceptance criteria in the issue body describe that later phase, not v1.

Plan

Start tomorrow. First concrete step: a dispatch-codex-mcp path (mirroring how dispatch-codex is invoked) that calls the MCP tool and writes the same review-output contract the loop already consumes, so the rest of the skill's machinery is unchanged.

[yzhao062]

Agreed plan (phased) — plan of record

Splitting into two changes to keep blast radius low. Phase 1 is additive (a new opt-in path); Phase 2 edits the shared path-selection that every backend flows through. Do them separately so a routing change cannot break the already-working channels.

Phase 1 — MCP channel only (tomorrow)

Additive opt-in. Does not change any default or routing.

• Wire /implement-review mcp (plus use mcp / mcp mode, existing negation guard, SKILL.md:165) to dispatch a Codex review through codex mcp-server instead of codex exec. First step: a dispatch-codex-mcp path that mirrors dispatch-codex's invocation and writes the same review-output contract the loop already consumes, so health-check / auto-watch / stall-watch need no change.
• Codex backend only. Bar: it reliably runs and returns a review when invoked. codex exec stays the default; terminal-relay stays the fallback.
• Hard invariant — do not touch the cross-vendor backends. dispatch-claude must keep its --strict-mcp-config --mcp-config <empty> isolation (SKILL.md:142): a user-level Codex MCP server must NOT reach the Claude reviewer, or it recurses into Codex and hangs with empty output. mcp mode must never select or leak into the Claude / Copilot backends.
• Cheap safety (should, not a gate): on unreachable / AV-blocked / connected-but-not-callable, surface a clear error pointing at codex exec rather than hang.
• AV precedent: the _claude_guard.ps1 split that cleared Bitdefender's AMSI block (SKILL.md:142) is the template for any MCP-side AV mitigation.

Phase 2 — routing change (later, separate)

Higher risk: edits the path-selection logic all backends share.

• Default channel becomes auto via a config knob (e.g. IMPLEMENT_REVIEW_DEFAULT_CHANNEL=auto); the shipped default stays terminal-relay (safe for consumers without codex, or with strict AV).
• Caveats to handle here, not in Phase 1: the --sandbox danger-full-access trust posture (a bare invocation would auto-grant a full-access Codex), and the codex-on-PATH assumption (the existing downgrade at SKILL.md:98 already covers it).
• The preference ladder (auto then terminal-relay, with mcp as a Codex-backend rung that can slot ahead of codex exec once proven) and any auto-detect-and-degrade promotion live here.

Backend selection is unchanged by either phase: the self-review guard (SKILL.md:130 / 163) keeps a bare auto selecting the Codex reviewer, with Copilot / Claude opt-in.

[yzhao062]

Phase 1 implementation notes — dispatch contract + the design call to make first

Pulled the real contract from dispatch-codex.sh so tomorrow starts from fact, not SKILL.md prose. Correction to my earlier "mirror dispatch-codex, reuse health-check / auto-watch / stall-watch unchanged": not quite right. The MCP path is in-process, so it is structurally closer to the dispatch-claude relay-save model than to dispatch-codex's subprocess-self-save model.

dispatch-codex contract (reference)

• Args: --prompt-file <path>, --round <N>, --expected-review-file <name>.
• Env: CODEX_BIN (default codex), CODEX_DISPATCH_SANDBOX (default danger-full-access), TMPDIR.
• Stdout: exactly one line, STATE-DIR <abs-path>, printed before Codex runs.
• State-dir files: pre-mtime (review-file mtime before the run, for auto-watch), timestamp (start epoch), tail (codex combined stdout+stderr).
• Invocation: codex exec --sandbox <mode> - < prompt-file > state-dir/tail 2>&1 — prompt on stdin, byte-identical to terminal-relay.
• Review file: Codex self-writes Review-Codex.md per the prompt's save contract; auto-watch detects it via mtime vs pre-mtime. Exit code propagated; stall-watch launched in background.

Why MCP does not "mirror dispatch-codex"

The Codex MCP is a tool Claude calls in-process (the codex mcp-server Claude is connected to), not a CLI a shell script can exec. So there is no subprocess to launch, no STATE-DIR on stdout, and the result is synchronous in Claude's hand. The subprocess-monitoring machinery (auto-watch = poll for file appearance; stall-watch = detect a hung subprocess) is therefore not applicable — it exists to babysit an async subprocess writing a file.

Two designs — pick this first

• A (recommended): in-process tool call + relay-save. Claude calls the codex MCP tool with the byte-identical assembled prompt, gets the review back, writes it to Review-Codex.md with the <!-- Round N --> marker — exactly the relay-save dispatch-claude already does (SKILL.md:142). No subprocess, no STATE-DIR, no auto-watch / stall-watch. This is the whole reason MCP is smoother; keep it in-process.
• B: subprocess MCP-client script that speaks JSON-RPC to codex mcp-server, writes the full state-dir contract, emits STATE-DIR. Preserves the existing machinery unchanged but re-spawns a subprocess and needs a JSON-RPC client — it throws away MCP's smoothness. Only worth it if Phase 2 intake must stay byte-identical.

Recommend A.

Reuse vs not (under Design A)

• Reuse: the assembled prompt (byte-identical invariant), the save contract (Review-Codex.md + <!-- Round N -->), Phase 2.0 content health-check, the /implement-review mcp trigger slot (SKILL.md:165).
• Not applicable: auto-watch, stall-watch, the STATE-DIR stdout line, and the tail-based Check 7 / Check 8 (those detect the codex exec sandbox CreateProcessAsUserW 1312 failure — there is no exec sandbox on the MCP path).

Phase 1 checklist

1. Decide A vs B (recommend A).
2. Add /implement-review mcp + use mcp / mcp mode triggers in Path selection (SKILL.md:165), under the existing negation guard, Codex backend only.
3. MCP branch in Phase 1c: call the codex MCP tool with the assembled prompt; relay-save the returned review to Review-Codex.md with the round marker.
4. Run Phase 2.0 content health-check on the saved file (skip the exec-sandbox tail checks). May need a minimal synthesized state-dir, or a health-check flag to skip the subprocess/tail checks.
5. Failure handling: MCP unreachable / AV-blocked / connected-but-not-callable → clear error pointing at codex exec, no hang.
6. Hard invariant: do not touch dispatch-claude's --strict-mcp-config --mcp-config <empty> isolation (SKILL.md:142); mcp must not reach the Claude / Copilot backends.

Open (needs a window where the codex MCP tools are exposed): confirm the exact codex MCP tool name and its input / return shape. This session showed codex Connected via claude mcp list but did not expose its tools to the agent.

[yzhao062]

Status: deferred (2026-06-03). Auto-terminal channel is working with no issues in practice, so there is no urgency to add MCP. Forward-compat slot kept (SKILL.md:165). Resume only when a real fungibility need appears (Codex cannot self-review AND Copilot is absent).

Root cause of the "connected ≠ callable" blocker

This was the Phase 1 precondition (confirm the codex MCP tool name + input/return shape). It could not run because the tool is not reachable in-session. Why:

• codex is listed in the agent-config project's disabledMcpServers array in ~/.claude.json. The codex MCP server is disabled for this project's sessions.
• claude mcp list still shows codex ✓ Connected because that command health-pings every configured server regardless of per-project disable state. Connection health ≠ tool exposure.
• Confirmed in-session: ToolSearch +codex returns nothing; no mcp__codex__* tool is in the session registry.

Precondition to resume Phase 1 (in order)

1. Re-enable codex MCP for the project (via the /mcp menu, or remove "codex" from that project's disabledMcpServers). Consider why it was disabled first — it may have been turned off deliberately alongside the dispatch-claude --strict-mcp-config isolation (SKILL.md:142), which exists precisely to keep a user-level Codex MCP server out of the Claude reviewer path.
2. Start a fresh Claude Code session. MCP tools are enumerated at session start, so re-enabling mid-session does not expose them.
3. Then run the one live probe: call the codex MCP tool once, capture its exact tool name + input/return shape. That resolves the only true unknown for Design A.

No code changed. Channel slot remains reserved.