Skip to content

feat(ce-doc-review): add opt-in cross_model_require_peer to fail when no peer artifact returns - #1854

Closed
nish3451 wants to merge 2 commits into
EveryInc:mainfrom
Nishfleet:feat/cross-model-require-peer
Closed

nish3451 wants to merge 2 commits into
EveryInc:mainfrom
Nishfleet:feat/cross-model-require-peer

Conversation

@nish3451

@nish3451 nish3451 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1855

Summary

Problem: the cross-model passes in ce-doc-review and ce-code-review are non-blocking by design. If no different-provider peer is reachable, or the peer returns nothing usable, the worker logs a skip and exits 0. The review then looks complete when only one model family actually ran.

This adds an opt-in key, cross_model_require_peer (default false), mapped to CROSS_MODEL_REQUIRE_PEER on the worker's env prefix like the other cross_model_* keys. When true, a pass that publishes no artifact fails: the worker exits 3 and logs

CROSS_MODEL_REQUIRE_PEER=true: lens '<lens>' got no artifact from any different-provider peer: <reason>

Default behavior is unchanged: unset or any value other than true keeps the non-blocking skip. cross_model_review_mode: off stays an explicit opt-out and is not an error. The --emit-adapter and CROSS_MODEL_DRY_RUN diagnostics are unaffected.

Changes

  • cross-model-doc-review.sh, cross-model-adversarial-review.sh: EXIT-trap check that turns an otherwise clean exit with no published artifact into exit 3 when the flag is set.
  • Both references/cross-model-review.md files, ce-doc-review/SKILL.md: key read and env mapping, and the "never an error" / "never fail the review" lines now carry the exception.
  • docs/guides/configuration.md, the two skill guides, config-template.yaml and its config.example.yaml copy.

Validation

  • New tests in both *-cross-model-routes.test.ts files: flag set with no reachable peer exits 3 with the lens and reason; a peer returning garbage exits 3; a peer that publishes still exits 0; unset and false keep exit 0; dry-run unaffected. A template guard in cross-model-review-mode.test.ts pins the new config comment.
  • bun run test: 4531 pass, 1 fail. The failure (skill-eval-cell/catalog.test.ts) needs a git ref that is absent from a shallow clone and does not touch these files. release:validate passes.
  • Manual run of cross-model-doc-review.sh with CROSS_MODEL_REQUIRE_PEER=true: exit 3 with the message when the only candidate peer CLI is absent; exit 0 and an artifact published when an OpenCode peer is reachable.

Security Disclosure

No security-relevant changes. The change only alters the worker's exit code and log text when no peer artifact is produced. It adds no new shell execution, path handling, or egress.

Agent Disclosure

  • Model: Claude Code · claude-sonnet-5-5

… no peer artifact returns

Adds cross_model_require_peer (env CROSS_MODEL_REQUIRE_PEER, default false) for ce-doc-review and ce-code-review. When true, the cross-model workers exit 3 with an error naming the lens and reason if no different-provider peer returned an artifact, instead of skipping with exit 0.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-14T04:30:52.207364Z e743acc PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tmchow

tmchow commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

Thanks for putting this together, and for the tests and docs. I'm going to close it, though, because I don't think a worker exit code is the right layer for this.

A few things I found when I traced it through:

  • A started worker that exits 0 without publishing already gets recorded as failed by the job runner ("worker exited 0 without publishing a non-empty result"). ce-code-review then runs its recovery path (a replacement peer, then the in-process fallback) and records what happened in peer.outcome and in Coverage, e.g. adversarial lens: in-process fallback (cross-model peer not run: <reason>). So the review doesn't present itself as cross-model when only one family ran.
  • Exit 3 doesn't change anything downstream. The runner maps it to failed: worker exited 3, which both skills already treat as non-blocking. A review has no exit status to fail, so "report as an error" comes down to different wording in Coverage.
  • In strict mode the new prose says to report an error on the first no-artifact result, but the recovery reference still says to start a replacement peer, and that peer might succeed. Those two instructions contradict each other.
  • The log line says "no artifact from any different-provider peer", but the worker only runs one fixed route. Picking a replacement is the host's job, so that message would be wrong whenever a replacement succeeds.
  • The case that really is silent is "never started / not run" in mode:agent (and non-interactive ce-doc-review). No job runs there, so the script change never applies.

If you have automation that needs a hard gate on the peer actually running, reading the review's peer outcome is the better hook. If ce-doc-review doesn't expose an equivalent outcome in headless mode, I'd take an issue for that narrower gap.

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.

ce-doc-review: non-interactive mode exposes no peer outcome when the cross-model peer does not run

2 participants