Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions .claude/commands/coding-review-agent-loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@ Parse the arguments to extract:
- **flow** — `issue <N>` (maps to `--flow plan`) or `pr <N>` (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. **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 "<text>"` 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`.
63 changes: 63 additions & 0 deletions SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,69 @@ for validation.

---

## Reversed roles (Codex as coder, Claude as reviewer)

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

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 `<!-- AGENT_PLAN_STATE: approved -->` 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 `<!-- AGENT_PLAN_STATE: ... -->` footer.

Validate, render, and post as in the normal reviewer flow (Step 6), using
`--reviewer Claude` and `--agent Claude` in the respective commands.
This ensures `build-resume --reviewers claude` recognizes the review on resume.

---

## Billing and terms note

This skill runs Claude turns inside your active interactive Claude Code session.
Expand Down
31 changes: 26 additions & 5 deletions helpers/run_external.py
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -10,6 +12,7 @@
--prompt-file PATH \\
--output PATH \\
--workdir PATH \\
[--role {reviewer,coder}] \\
[--cmd PATH] \\
[--dry-run]
"""
Expand Down Expand Up @@ -43,8 +46,19 @@
"\n<!-- AGENT_PLAN_STATE: approved -->\n-- Codex (dry-run stub)\n"
)

_CANNED_PLAN_STATE = """\
## Plan (dry-run stub)

def _build_dry_run_response() -> str:
1. Implement the requested changes.

<!-- AGENT_PLAN_STATE: approved -->
-- 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


Expand All @@ -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.")
Expand All @@ -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:
Expand Down
39 changes: 39 additions & 0 deletions tests/test_skill_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading