Skip to content

ci: admit the Claude reviews of one pull request one at a time - #963

Merged
cdeust merged 1 commit into
mainfrom
ci/serialize-review-and-resolver
Nov 10, 2026
Merged

cdeust merged 1 commit into
mainfrom
ci/serialize-review-and-resolver

Conversation

@cdeust

@cdeust cdeust commented Nov 10, 2026

Copy link
Copy Markdown
Owner

Contexte

Le budget de reviews (ADR-1119) compte les reviews terminées. Des pushs rapprochés, dont les runs de review se chevauchent, étaient donc chacun admis comme le même tour, et le plafond de trois pouvait être dépassé. Constat Codex P2 relevé trois fois sur les PR de réplication (ai-architect-mcp-codebase#486, ai-architect-mcp-spec#185, cortex-viz#233).

Changement

  • claude-review.yml : groupe de concurrence par PR, cancel-in-progress: false. Une review en cours n'est jamais annulée ; parmi celles en attente, GitHub garde la plus récente, celle du head courant.
  • Le test du budget importe unittest.mock d'une seule façon (alerte CodeQL relevée sur cortex-viz#233, même fichier).

Métrique

Reviews admises au-delà du plafond par des pushs rapprochés : possible aujourd'hui, 0 après.

Tests

test_reviews_of_one_pr_are_admitted_one_at_a_time ; 33 tests des workflows de review verts en local.

Cette PR modifie claude-review.yml : la review Claude ne tourne pas dessus. Fusion manuelle, autorisée par le propriétaire pour les modifications de CI.


Generated by Claude Code

The budget counts completed reviews, so pushes whose review runs overlapped
were each admitted as the same round and the cap of three (ADR-1119) could be
exceeded. claude-review.yml now has a concurrency group per pull request that
never cancels a run in progress. The budget test also imports unittest.mock one
way (CodeQL py/import-and-import-from, raised on cortex-viz#233).

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T04:51:56.374455Z e57e826 PR opened
🔒 Security Review ✅ Completed 2026-11-10T04:53:19.676155Z e57e826 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.

cdeust pushed a commit to cdeust/zetetic-team-subagents that referenced this pull request Nov 10, 2026
CodeQL py/import-and-import-from (alert 258) on the file taken from Cortex;
the same change is in cdeust/Cortex#963.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>
@cdeust
cdeust merged commit 1b69782 into main Nov 10, 2026
18 checks passed
@cdeust
cdeust deleted the ci/serialize-review-and-resolver branch November 10, 2026 06:34
cdeust added a commit to cdeust/zetetic-team-subagents that referenced this pull request Nov 10, 2026
…e merge (#244)

* ci: the default profile follows Cortex, the Claude verdict decides the merge

The template takes Cortex's resolve and close-issues jobs and its merge
aggregate, and loses its review job (review, repair, auto-merge arming). The
resolver merges a head whose Pipeline succeeded and whose latest Claude
verdict, bound to a review run of that head, approves (Cortex ADR-1121);
low-risk Dependabot updates need no review (ADR-1126); the review's
completion wakes the resolver (ADR-1127). claude-review.yml and its budget
script are Cortex's (ADR-1119). The native profile is unchanged.

pytest tests/: 3994 passed (3975 before). tools/ci_post_merge.py: 99 % lines.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>

* ci: keep the tracked .coverage file this change never meant to touch

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>

* ci: the default render delivers the review its resolver waits for

A consumer rendered with the default profile received the resolver that
holds every merge without a Claude verdict, but not claude-review.yml, its
budget script, the /review-pr command or its reviewers (Codex P1 on #244).
templates/ci/review carries them and prepare_files delivers them.

pytest tests/: 3996 passed.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>

* ci: session-optimizer's uv-compiled requirements pair is a dependency file

Its Dependabot updates touch requirements-dev.in and requirements-dev.txt;
without them in LOW_RISK_FILES every such update waits for a review that
Dependabot runs never get.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>

* ci: the native caller wakes on the Pipeline only

The shared completion caller now listens to PR Review too, but the native
resolver accepts a Pipeline source only, so each native review completion
would have started a failing run (Codex P2 on #244). The renderer keeps the
review wake-up for the default profile alone.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>

* test: import unittest.mock one way in the budget test

CodeQL py/import-and-import-from (alert 258) on the file taken from Cortex;
the same change is in cdeust/Cortex#963.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>

* ci: admit the Claude reviews of one pull request one at a time

Cortex #963: a per-PR concurrency group on claude-review.yml, so
overlapping pushes cannot pass the three-review cap. Same file in
templates/ci/review, which the default render delivers.

Co-Authored-By: Claude Fable 5.2 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Signed-off-by: Claude <[email protected]>

---------

Signed-off-by: Claude <[email protected]>
Co-authored-by: Claude <[email protected]>
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.

2 participants