From a0a8912ea71eb79a5499ce551baebf1e99015543 Mon Sep 17 00:00:00 2001 From: Wild Wind Date: Mon, 8 Jun 2026 01:33:05 -0700 Subject: [PATCH 1/2] feat(skill): add --coder codex reversed-roles support (issue #285) Add --role coder to run_external.py so Codex can be invoked for plan writing turns. Document the reversed-roles flow in SKILL.md and update the slash command to accept --coder codex. Add a dry-run test covering the new coder role path. Closes #285 Co-Authored-By: Claude Sonnet 4.6 --- .claude/commands/coding-review-agent-loop.md | 10 ++-- SKILL.md | 48 ++++++++++++++++++++ helpers/run_external.py | 31 +++++++++++-- tests/test_skill_helpers.py | 39 ++++++++++++++++ 4 files changed, 119 insertions(+), 9 deletions(-) diff --git a/.claude/commands/coding-review-agent-loop.md b/.claude/commands/coding-review-agent-loop.md index 18d94e0..750b9af 100644 --- a/.claude/commands/coding-review-agent-loop.md +++ b/.claude/commands/coding-review-agent-loop.md @@ -7,14 +7,16 @@ Parse the arguments to extract: - **flow** — `issue ` (maps to `--flow plan`) or `pr ` (maps to `--flow pr`) - **reviewers** — `--reviewers codex`, `gemini`, or both (default: gemini) - **plan-first** — present if the user passes `--plan-first` (only relevant for issue flow) +- **coder** — `--coder claude` (default) or `--coder codex` If any required argument is missing, ask the user before proceeding. Then follow the orchestration steps in `SKILL.md` from Step 1. +When `--coder codex` is parsed, follow the **Reversed roles** section of `SKILL.md` +instead of the default Claude-as-coder flow: Codex handles Step 2 (plan writing) +via `run_external --role coder`, and Claude performs Step 6 (review turn) directly +in the session. + Note: `task ""` is not supported in skill mode. Direct the user to the headless CLI (`agent-loop task "..." --repo OWNER/REPO`) for task-based flows. - -**Current limitation**: this skill runs Claude as the coder. Reversed roles -(Codex as coder, Claude as reviewer) are not yet supported in skill mode, -though the headless `agent-loop` CLI supports `--coder codex --reviewer claude`. diff --git a/SKILL.md b/SKILL.md index 551f621..70dbdbe 100644 --- a/SKILL.md +++ b/SKILL.md @@ -216,6 +216,54 @@ for validation. --- +## Reversed roles (Codex as coder, Claude as reviewer) + +When `--coder codex` is requested, replace Step 2 and Step 6 as follows. +All other steps (1, 3, 4, 5, 7, 8) are unchanged. + +### Step 2 (coder turn) — Codex writes the plan + +Instead of Claude writing the plan directly, invoke Codex: + +```bash +python -m helpers.run_external \ + --agent codex \ + --role coder \ + --prompt-file /tmp/agent-loop-skill/{session-id}/coder-prompt.md \ + --output /tmp/agent-loop-skill/{session-id}/plan.md \ + --workdir /path/to/codex/checkout +``` + +The prompt file must contain the issue context and plan-format instructions +(including the `` footer requirement). + +### Step 3 — Validate the plan (same as normal) + +```bash +python -m helpers.validate_response \ + --file /tmp/agent-loop-skill/{session-id}/plan.md \ + --kind plan_state +``` + +### Steps 4–5 — Attach metadata and post (same as normal) + +Use `--agent Codex` in the `attach-metadata` call instead of `--agent Claude`. + +### Step 6 (review turn) — Claude writes the structured review + +Claude (the session host) writes the structured JSON review directly to a temp file: + +``` +/tmp/agent-loop-skill/{session-id}/claude-review.md +``` + +The file must be a valid `plan_review` JSON followed by the `` footer. + +Validate, render, and post as in the normal reviewer flow (Step 6), using +`--reviewer Claude` and `--agent Claude` in the respective commands. + +--- + ## Billing and terms note This skill runs Claude turns inside your active interactive Claude Code session. diff --git a/helpers/run_external.py b/helpers/run_external.py index 934a90b..cabdfde 100644 --- a/helpers/run_external.py +++ b/helpers/run_external.py @@ -1,7 +1,9 @@ """ -Run an external agent (Codex or Gemini) for one review turn. +Run an external agent (Codex or Gemini) for one reviewer or coder turn. -In --dry-run mode, writes a canned approved plan_review stub to --output and exits 0. +In --dry-run mode, writes a canned approved stub to --output and exits 0: + --role reviewer → plan_review stub + --role coder → plan_state stub In live mode, invokes the agent CLI and writes the response to --output. Usage: @@ -10,6 +12,7 @@ --prompt-file PATH \\ --output PATH \\ --workdir PATH \\ + [--role {reviewer,coder}] \\ [--cmd PATH] \\ [--dry-run] """ @@ -43,8 +46,19 @@ "\n\n-- Codex (dry-run stub)\n" ) +_CANNED_PLAN_STATE = """\ +## Plan (dry-run stub) -def _build_dry_run_response() -> str: +1. Implement the requested changes. + + +-- Codex (dry-run stub) +""" + + +def _build_dry_run_response(role: str) -> str: + if role == "coder": + return _CANNED_PLAN_STATE return _CANNED_PLAN_REVIEW + _CANNED_PLAN_REVIEW_FOOTER @@ -54,6 +68,12 @@ def main() -> None: parser.add_argument("--prompt-file", required=True, help="Path to prompt text file.") parser.add_argument("--output", required=True, help="Path to write the agent response.") parser.add_argument("--workdir", required=True, help="Working directory for the agent.") + parser.add_argument( + "--role", + default="reviewer", + choices=["reviewer", "coder"], + help="Turn role: 'reviewer' (default) or 'coder' (Codex writes the plan).", + ) parser.add_argument("--cmd", default=None, help="Agent CLI command (overrides default).") parser.add_argument("--diff-file", default=None, help="Path to a pre-fetched PR diff to embed in the prompt.") parser.add_argument("--dry-run", action="store_true", help="Write a canned stub and exit.") @@ -63,8 +83,9 @@ def main() -> None: output_path.parent.mkdir(parents=True, exist_ok=True) if args.dry_run: - output_path.write_text(_build_dry_run_response(), encoding="utf-8") - print(f"dry-run: wrote canned plan_review stub to {output_path}") + stub_kind = "plan_state" if args.role == "coder" else "plan_review" + output_path.write_text(_build_dry_run_response(args.role), encoding="utf-8") + print(f"dry-run: wrote canned {stub_kind} stub to {output_path}") return try: diff --git a/tests/test_skill_helpers.py b/tests/test_skill_helpers.py index 6ac6642..428030d 100644 --- a/tests/test_skill_helpers.py +++ b/tests/test_skill_helpers.py @@ -407,3 +407,42 @@ def test_dry_run_exits_zero_and_writes_valid_stub(self) -> None: # The dry-run stub must contain a valid plan_review JSON and AGENT_PLAN_STATE marker assert "AGENT_PLAN_STATE: approved" in content assert '"state": "approved"' in content + + def test_dry_run_coder_role_exits_zero_and_writes_plan_state_stub(self) -> None: + with tempfile.NamedTemporaryFile("w", suffix=".md", delete=False, encoding="utf-8") as pf: + pf.write("Implement the feature.") + prompt_path = pf.name + with tempfile.NamedTemporaryFile("w", suffix=".md", delete=False, encoding="utf-8") as of: + output_path = of.name + + result = _run( + "helpers.run_external", + "--agent", + "codex", + "--role", + "coder", + "--prompt-file", + prompt_path, + "--output", + output_path, + "--workdir", + "/tmp", + "--dry-run", + ) + assert result.returncode == 0 + content = Path(output_path).read_text(encoding="utf-8") + # Coder dry-run stub must be a valid plan_state (no JSON, just markdown + marker) + assert "AGENT_PLAN_STATE: approved" in content + # Must NOT be a plan_review JSON blob + assert '"kind": "plan_review"' not in content + + # Confirm the stub passes plan_state validation + stub_path = _write_tmp(content) + validate_result = _run( + "helpers.validate_response", + "--file", + stub_path, + "--kind", + "plan_state", + ) + assert "validation passed: plan_state" in validate_result.stdout From 88627b24e5747c7d5e17ce1a2528e8a5e2c1d73c Mon Sep 17 00:00:00 2001 From: Wild Wind Date: Mon, 8 Jun 2026 15:57:56 -0700 Subject: [PATCH 2/2] fix(skill): specify --reviewers claude for reversed-roles build-resume Codex's blocking review (PR #286) identified that SKILL.md documented the reversed-roles flow without specifying --reviewers claude in the build-resume call. Without that, build-resume ignores Claude's posted review on any subsequent resume attempt, causing the loop to repeat review turns. Co-Authored-By: Claude Sonnet 4.6 --- .claude/commands/coding-review-agent-loop.md | 4 +++- SKILL.md | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/.claude/commands/coding-review-agent-loop.md b/.claude/commands/coding-review-agent-loop.md index 750b9af..448210f 100644 --- a/.claude/commands/coding-review-agent-loop.md +++ b/.claude/commands/coding-review-agent-loop.md @@ -16,7 +16,9 @@ Then follow the orchestration steps in `SKILL.md` from Step 1. When `--coder codex` is parsed, follow the **Reversed roles** section of `SKILL.md` instead of the default Claude-as-coder flow: Codex handles Step 2 (plan writing) via `run_external --role coder`, and Claude performs Step 6 (review turn) directly -in the session. +in the session. **Important**: pass `--reviewers claude` (not `codex`/`gemini`) +in the `build-resume` Step 1 call so Claude's completed review is tracked correctly +for resume. Note: `task ""` is not supported in skill mode. Direct the user to the headless CLI (`agent-loop task "..." --repo OWNER/REPO`) for task-based flows. diff --git a/SKILL.md b/SKILL.md index 70dbdbe..52c66b3 100644 --- a/SKILL.md +++ b/SKILL.md @@ -218,8 +218,22 @@ for validation. ## Reversed roles (Codex as coder, Claude as reviewer) -When `--coder codex` is requested, replace Step 2 and Step 6 as follows. -All other steps (1, 3, 4, 5, 7, 8) are unchanged. +When `--coder codex` is requested, the following steps differ from the normal flow. + +**Critical**: `build-resume` tracks completion by matching the posted review's +`--agent` value against the `--reviewers` list. In reversed-roles mode Claude +is the reviewer, so **pass `--reviewers claude`** in Step 1 (not `codex` or +`gemini`). Using the wrong reviewers list causes `build-resume` to ignore +Claude's completed review on any subsequent resume. + +### Step 1 — build-resume (reversed-roles variant) + +```bash +python -m helpers.state_manager build-resume \ + --issue ISSUE --repo OWNER/REPO \ + --reviewers claude \ + --flow plan +``` ### Step 2 (coder turn) — Codex writes the plan @@ -261,6 +275,7 @@ The file must be a valid `plan_review` JSON followed by the `