Skip to content

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

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

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

melodic-automation-lanes Bot commented Nov 3, 2026 •

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.

a pr-unblock lane stopped its head (pr-unblock/address-feedback is failure), which clears when the
  lane runs again

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 0e7d7adc241e416c79f2c182b747ce162d920960: 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 every hunk of all 10 changed files in .claude-lane/pr.diff. The new head (0e7d7adc) only merges main into the branch. The PR's own change matches what the previous round reviewed at 92711b72, and that round reported no findings.

What I checked

  • The workflow gets a value it accepts. fanout-sweep.js takes only lens or model (L35). When the setting resolves to codex, three places in run-everything-mode.md all launch the sweep with lens: the Launch bullet, the new codex diversity bullet, and the coverage-parity fallback. So codex never reaches the workflow.
  • The panel's status lines match the script. codex-review.sh prints codex: ran, codex: skipped (<reason>) or codex: failed (<reason>) (L11-L13). The new panel: rules are built from exactly those strings.
  • No other list of allowed values is stale. In plugins/review, the only text still listing just lens, model is the workflow's own error message. That is correct, because the workflow really accepts only those two. The schema enum, the setup test header, case 26 and eval 48's WARN text all include codex.
  • 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. codex is an opt-in 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 (expects panel: codex ran) and 51 (expects panel: codex skipped (...)). Both evals fail if that behaviour breaks.

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

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

Prior findings

The prior-findings file lists one earlier summary comment with no findings, no inline findings, 0 rounds with findings, and a cap of 3 (not reached). Nothing is repeated.

For the owner (not a finding)

Step 3 of run-everything already runs Codex under lens. So codex runs the same reviewers lens runs; it only adds the panel: report line and rules out the model tiers. The PR description says this itself. Whether that satisfies rule (4) of #12151 is the owner question the PR names, and this review does not judge it.

Not verified

  • I ran nothing. These checks come from the author's run and I did not confirm them: the changelog and changed-skills checks, check-affected, and the evals quality check.
  • Nobody has run the new case 26 test lines or evals 50 and 51 yet, the authoring lane included. The PR leaves them to CI.

Usage: when Codex is installed, codex spends Codex and subscription usage on two codex-review.sh runs. lens already makes those same runs, so this adds no new spend.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on 0e7d7adc241e416c79f2c182b747ce162d920960; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

"lens",
"model"
"model",
"codex"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 the default-deny rule flags it. I read the code and skill text that use this key, and the change widens nothing:

  • Not a guard. fanout_diversity only chooses how /review:fanout run-everything spreads its leaf reviews. It does not gate, cap, confirm or charge for anything.
  • Default unchanged. "default": "lens" (line 47) is the same as before, so installed users who never set the key behave the same after they update.
  • No new code runs. Under codex, the sweep still 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 only adds a panel: report line.
  • Repository layer. A pull request that edits docs/conventions/review.yaml still cannot set its own reviewers. That layer is skipped for such a change set (reference/config.md, unchanged rule).

severity: IMPORTANT

claude-lane: claude-security-review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pr-unblock / address-feedback: UNCERTAIN, left open

Evidence: plugins/review/.claude-plugin/plugin.json:47, plugins/review/.claude-plugin/plugin.json:51, plugins/review/skills/fanout/context/run-everything-mode.md:11, plugins/review/skills/fanout/context/run-everything-mode.md:16, plugins/review/skills/fanout/context/run-everything-mode.md:17, plugins/review/skills/fanout/SKILL.md:64

I read the code and every claim in this flag checks out. The default stays lens (plugin.json:47), so users who never set the key behave as before. Under codex, the sweep still launches review:fanout-sweep with args.diversity set to lens (run-everything-mode.md:16-17). The Codex panel is step 3's plain and --adversarial codex-review.sh runs, which already run in every diversity mode (run-everything-mode.md:11), so the new value adds only a panel report line and runs no new code. A change set that edits docs/conventions/review.yaml still skips the repository layer for this key (fanout/SKILL.md:63-64), so a pull request cannot pick its own value. No code change is needed. This is a security-review manifest flag on a userConfig options list, and the lane's rules give that kind of flag to a person: only a manifest dependencies flag is one this lane may close on code evidence. So I am leaving the thread open for a maintainer to accept the new codex option.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

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

Security review: PR #12636 (fanout_diversity: codex)

No security issues found beyond the manifest flag below. I read every hunk of all 10 changed files. The head is a merge of main into the branch, and this PR's own diff is the same as it was in the previous round. It adds codex, an opt-in third value for fanout_diversity, across prose, schema, eval and test files.

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. IMPORTANT. It is posted inline at plugin.json#L51. The previous round posted it as a summary with no line anchor, so this round repeats it.
    • The key is not a guard.
    • Its default is unchanged: "default": "lens" at plugin.json#L47.
    • Under codex the sweep launches with diversity: lens (run-everything-mode.md#L16-L17).
    • codex-review.sh already runs at step 3 in every mode (run-everything-mode.md#L11).
    • No hooks, mcpServers, lspServers, monitors, dependencies or other key that installs code 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, and the config.md row keeps that rule.
  • Standing instructions. No rule was deleted, narrowed or softened. The changes to fanout/SKILL.md, run-everything-mode.md and setup/SKILL.md only add text.
  • Trust controls. None of the changed files is a trust control, a merge-hold file or a trusted path. plugins/review/skills/security-review/** is not touched.
  • PR context. It contains no instructions aimed at this lane.

Spend (not a finding): nothing new. The Codex runs that codex reports on already happen at step 3 under lens, and codex never fans out to model tiers.

critical-findings: 0

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on 0e7d7ad.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 3, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the lane left a review finding open and no adjudicator verdict settled it: none was recorded, it found the finding does not hold where the lane had not disputed it (an UNCERTAIN or VALID-defer reply), which only an INCORRECT reply lets a verdict resolve, it found the finding asks only for tests whose covering check runs had not all passed when the round began, or it raised a security question on a finding the security-review lane rated below CRITICAL; so a session builder takes it. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=12636 -f activity=address-feedback

@kyle-sexton

Copy link
Copy Markdown
Member

Hold: replaced by #12858, which merges this head and fixes the open review threads. Releases when: #12858 merges, then this PR is closed.

@kyle-sexton

Copy link
Copy Markdown
Member

Superseded by #12858, which merged.

@kyle-sexton kyle-sexton closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Hard merge gate: do not merge while applied. needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop.

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