Skip to content

feat(review): add a codex fanout_diversity value that puts the panel on the Codex reviewer - #12636

Open
melodic-automation-lanes[bot] wants to merge 2 commits into
mainfrom
work-resolve-issue/issue-12504-41563977457-1
Open

melodic-automation-lanes[bot] wants to merge 2 commits into
mainfrom
work-resolve-issue/issue-12504-41563977457-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #12504

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Adds a third fanout_diversity value, codex, that is opt-in through the plugin option or docs/conventions/review.yaml. The default stays lens. Under codex, /review:fanout run-everything launches review:fanout-sweep with args.diversity lens, so no same-vendor model tiers run. The panel is step 3's two codex-review.sh runs (plain and --adversarial), reported as panel: codex ran, or panel: codex skipped|failed (<reason>) when Codex is absent or fails. The lens sweep still runs either way. Choice of means: I added an enum value instead of re-pointing model, so model mode stays available and nothing changes for existing configs. The workflow still accepts only lens and model, because Codex runs on the main thread (SKILL.md 'Orchestrator plugins'). #12495 had already landed as review 4.0.1 (codex-review.sh). The retraction comment on the issue leaves #12151 rule (4) as approved and records no decision, so I built the issue body as written. Note for the owner: run-everything already runs Codex at step 3 under lens, so codex starts the same surfaces lens does. What it adds is the explicit panel report, and it never fans out to model tiers.

Not verified

  • setup-apply.test.sh (the new case 26 lines) and the fanout skill evals were not executed in this lane, because Bash runs only the named wrappers; left to the PR's CI
  • Tidy pass 5 (ai-slop) ran its judgment rubric only: the lane does not permit running detect.sh or digest-guard.sh, so the deterministic detector and guarded fix flow were skipped. Passes 1-4 ran and made no edits
  • The CI-only checks check-affected lists (evals-schema, plugin-schema, cant-fail-scan, purged-em-dashes and the others) run on the PR
  • Owner question, not a blocker: per #12151 comment 6593273287, the owner decides whether the X-arm result changes this issue. This PR builds the body as written, behind configuration, with the default unchanged
  • Untracked files .bash_profile, .bashrc, .gitconfig, .idea, .mcp.json, .profile, .ripgreprc, .vscode, .zprofile and .zshrc are in the checkout root but were not created by this run; they were left alone and are not part of this change

Fix

  • plugins/review/schemas/review.schema.json: fanout_diversity enum gains codex; description says what it does
  • plugins/review/.claude-plugin/plugin.json: fanout_diversity userConfig options gain codex; description shortened to stay under the 300-character option budget
  • plugins/review/README.md: configuration prose names codex; generated options row synced with plugin.json
  • plugins/review/reference/config.md: fanout_diversity row lists codex and what it does
  • plugins/review/skills/fanout/SKILL.md: fanout_diversity argument paragraph describes codex
  • plugins/review/skills/fanout/context/run-everything-mode.md: new codex diversity bullet (launch with lens, Codex runs as the panel, panel: line rules including timeout and permission-denied), Launch bullet exception, fallback roster for codex
  • plugins/review/skills/fanout/evals/evals.json: eval cases 50 (Codex present: runs codex-review.sh, reports panel: codex ran, launches lens) and 51 (Codex absent: panel: codex skipped, not under Ran, sweep continues); eval 48's quoted WARN text lists codex
  • plugins/review/skills/setup/SKILL.md: apply step's recommendation names codex
  • plugins/review/skills/setup/scripts/setup-apply.test.sh: case 26 writes and checks fanout_diversity=codex; header lists the new value
  • .changes/review/work-resolve-issue-issue-12504-41563977457-1-35444ae0.md: minor changelog fragment for the review plugin

Verification

The runner ran each check from the base copy against 92711b7:

  • scripts/check-changelog-fragments.sh --check: passed
  • scripts/check-changed-skills.sh --merge-base: passed

The run reports:

  • check-changelog-fragments: all 15 fragments valid
  • check-changed-skills: fanout and setup PASS with 0 errors (warnings are size and Gotchas warnings that apply to the whole skill)
  • check-affected: first run failed validate-plugins because the option description was 382 characters, over the 300 budget; after shortening it, 12 passed, 6 skipped, 0 failed (run again after tidy and after the review fixes)
  • check-evals-quality plugins/review/skills/fanout/evals/evals.json: PASS, 2 warnings on cases 39 and 40, which this change did not touch
  • read https: //github.com/melodic-software/claude-code-plugins/issues /12151: rule (4) moves the panel's placement from fanout's model mode to the Codex reviewer if M does not beat L; comment 6593273287 says rule (3) is not met (Codex stays out of the default sweep) and rule (4) stays as approved, wi
  • local review (fresh-context subagent over the whole diff): no high-severity bug; fixed the Launch-bullet ordering, the panel line for a missing status line or denied call, and the stale value lists in the setup test header and eval 48; documented that codex runs the same surfaces as lens plus the pa
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41563977457

Left out: the items under Not verified.

Related

#12504

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 92711b72a4fe8d48abf0c0c48869b04d47518e51: read 10 of 10 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #12636, adds codex as a fanout_diversity value

No blocking or advisory findings. I read all 10 changed files in .claude-lane/pr.diff, every hunk of each.

What I checked

  • The workflow gets a value it accepts. plugins/review/workflows/fanout-sweep.js takes only lens or model for diversity and returns an error for anything else (L32-L36). When the setting resolves to codex, the Launch bullet and the new codex diversity bullet in run-everything-mode.md both pass args.diversity: lens, so the workflow never receives codex. The fallback path (when Workflows are unavailable) also says to use the lens roster.
  • setup-apply accepts the new value. setup-apply.mjs checks values against schemas/review.schema.json, and this PR adds codex to that schema's enum. So writing and checking fanout_diversity=codex works through the schema. New test lines in case 26 cover it.
  • No other place lists the old values as the complete set. The WARN text in eval 48 was updated. The remaining lens or model string is in the workflow, which is correct: the workflow really does take only those two.
  • Closing issue feat(review): move the review panel's placement to the Codex reviewer once fanout can run Codex #12504.
    • "After fix(review): dispatch fanout's Codex surface through a path the model may run #12495 lands, fanout's panel placement uses the Codex reviewer where it used model mode, behind configuration": met. It is an opt-in enum value, and the default stays lens.
    • "A test or eval case shows the panel runs Codex when present and reports it skipped when not": met by fanout evals 50 (Codex present, reports panel: codex ran) and 51 (Codex absent, reports panel: codex skipped (...)). Both can fail.
  • No settled threads file was given, and the prior-findings file lists no earlier posts (0 rounds, cap 3, not past the cap).

Plugin manifest (plugins/review/.claude-plugin/plugin.json)

  • manifest userConfig.fanout_diversity.default: pass: still lens.
  • manifest userConfig.fanout_diversity.options: pass: the field already had options before this PR. Adding a third value doesn't require a newer Claude Code client.
  • manifest userConfig.fanout_diversity (reader): pass: skills/fanout/SKILL.md reads ${user_config.fanout_diversity}.
  • manifest version: pass: the changelog fragment declares bump: minor, which fits an added option value.
  • manifest description/keywords: pass: the plugin's description and keywords don't list diversity values, so nothing there is out of date.

For the owner (not a finding)

In run-everything, step 3 already runs Codex under lens. So codex starts the same reviews as lens. All it adds is the panel: report line, and it rules out the model tiers. The PR description says this itself. Whether that is enough for #12151 rule (4) is the open owner question the PR also names. I'm not judging it here.

Not verified

  • I ran nothing. The PR's own verification (changelog and changed-skills checks, check-affected, the eval-quality check) comes from the author's run and I did not confirm it.
  • The new case 26 lines in setup-apply.test.sh and the fanout evals were not run in the authoring lane either. CI should cover them.

Usage: when Codex is present, codex spends Codex and subscription usage on the two codex-review.sh runs. Run-everything already spends that under lens, so the extra cost is nothing.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 92711b72a4fe8d48abf0c0c48869b04d47518e51: read 10 of 10 changed file(s); 1 finding(s), 0 as inline comments.

Security review: PR #12636 (fanout_diversity: codex)

No security issues found. I read every hunk of all 10 changed files. Apart from the manifest flag below, the change is prose, schema, eval and test-only edits that add an opt-in third value, codex, to fanout_diversity.

Manifest

  • manifest userConfig.fanout_diversity.description: pass. A userConfig description is inert.
  • manifest userConfig.fanout_diversity.options: flag, a third accepted value codex is added to an existing userConfig key. This is IMPORTANT, and the reason is only that a non-inert field changed:
    • The key is not a guard, and its default is unchanged ("default": "lens", plugin.json#L24).
    • Under codex, nothing runs that lens does not already run. The workflow launches with diversity: lens (run-everything-mode.md#L16-L17), and codex-review.sh already runs at step 3 in every mode (run-everything-mode.md#L11).
    • No hooks, mcpServers, dependencies or other code-installing manifest key changed.

What I checked

  • Trust boundary. A pull request still cannot choose its own reviewers through this key: the repository layer is skipped when the reviewed change edits docs/conventions/review.yaml (config.md#L12), and that rule is unchanged.
  • Standing instructions. No rule was deleted or softened. The changes to the skill bodies (fanout/SKILL.md, run-everything-mode.md, setup/SKILL.md) only add text.
  • Trust controls. None of the changed files is a trust control or merge-hold file. The trusted-path glob plugins/review/skills/security-review/** is not touched.
  • PR context. No embedded instructions aimed at this lane.

Spend (not a finding): none beyond what exists today. The Codex runs the codex value reports on already happen at step 3 under lens, and codex never fans out to model tiers.

critical-findings: 0

Findings not posted inline (their lines are not in the diff):

  • plugins/review/.claude-plugin/plugin.json:29 (RIGHT): manifest userConfig.fanout_diversity.options: flag, a third accepted value codex is added to an existing userConfig key

The options list is not in the inert manifest set, so this is flagged by the default-deny rule. I checked the code and skill text that read the key, and the change does not widen anything:

  • Not a guard. fanout_diversity only chooses how /review:fanout run-everything spreads its leaf reviews. It does not gate, cap, confirm or cost anything new.
  • Default unchanged. "default": "lens" (line 24) is untouched, so installed users who never set the key get the same behaviour after updating.
  • No new code runs. Under codex, the sweep launches with args.diversity set to lens (skills/fanout/context/run-everything-mode.md:16-17). The Codex panel is step 3's two codex-review.sh runs, and step 3 already makes those runs in every mode (run-everything-mode.md:11). The new value adds only a panel: report line.
  • Repository layer. A codex set in docs/conventions/review.yaml is skipped when the reviewed change edits that file (reference/config.md:12). A pull request therefore cannot pick its own reviewer through this key.

I found no exploit path. The line is recorded only because a non-inert field changed.

severity: IMPORTANT

@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 3, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 92711b72a4fe8d48abf0c0c48869b04d47518e51.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 3, 2026
@kyle-sexton kyle-sexton added do-not-merge Hard merge gate: do not merge while applied. and removed do-not-merge Hard merge gate: do not merge while applied. labels Nov 3, 2026
Lane-Activity: pr-unblock/resolve-conflicts
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 0e7d7adc241e416c79f2c182b747ce162d920960. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate refuses its verifier verdict (verify:missing)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): move the review panel's placement to the Codex reviewer once fanout can run Codex

1 participant