ci(v1.x): sync the #91 cancellation migration - #220
Conversation
Content matches fleet-cancellation-migration (#219) — byte-identical to main once that merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Reviewed; 2 blockers found. Prior 2 finding(s) remain; new findings below. 1. Missing events in opt-in gate (Claude)File: .github/workflows/claude-review.yml:36 if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'claude-review') || (github.event.action != 'labeled' && (vars.CLAUDE_ALWAYS_ON == 'true' || contains(github.event.pull_request.labels.*.name, 'claude-review'))) }}2. Missing events in opt-in gate (Gemini)File: .github/workflows/gemini-review.yml:44 if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'gemini-review') || (github.event.action != 'labeled' && (vars.GEMINI_ALWAYS_ON == 'true' || contains(github.event.pull_request.labels.*.name, 'gemini-review'))) }}Suggestions (non-blocking)
|
Review feedback on oauth#219 — consistency with the PR's permissions- ceiling goal, applied fleet-wide. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
The retained note claiming the label is matched only in the reusable contradicted the caller gate two lines below (it now names the label); reworded to state the deliberate duplication. Narrating comments that restated the adjacent declaration are dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt
| # Note: the `claude-review` label name is deliberately duplicated — | ||
| # the caller gate names it (below) AND the reusable's authorize | ||
| # matches it; renaming the label means changing both. | ||
| if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'claude-review') || (github.event.action != 'labeled' && vars.CLAUDE_ALWAYS_ON == 'true') || (github.event.action == 'ready_for_review' && contains(github.event.pull_request.labels.*.name, 'claude-review')) }} |
There was a problem hiding this comment.
🔴 Blocker — Missing events in opt-in gate (Claude)
The if-gate still misses synchronize, opened, and reopened for labeled PRs when ALWAYS_ON is false. Use (github.event.action != 'labeled' && (vars.CLAUDE_ALWAYS_ON == 'true' || contains(github.event.pull_request.labels.*.name, 'claude-review'))) to cover all actions.
| # Note: the `gemini-review` label name is deliberately duplicated — | ||
| # the caller gate names it (below) AND the reusable's authorize | ||
| # matches it; renaming the label means changing both. | ||
| if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'gemini-review') || (github.event.action != 'labeled' && vars.GEMINI_ALWAYS_ON == 'true') || (github.event.action == 'ready_for_review' && contains(github.event.pull_request.labels.*.name, 'gemini-review')) }} |
There was a problem hiding this comment.
🔴 Blocker — Missing events in opt-in gate (Gemini)
The if-gate still misses synchronize, opened, and reopened for labeled PRs when ALWAYS_ON is false. Use (github.event.action != 'labeled' && (vars.GEMINI_ALWAYS_ON == 'true' || contains(github.event.pull_request.labels.*.name, 'gemini-review'))) to cover all actions.
| # Note: the `claude-review` label name is deliberately duplicated — | ||
| # the caller gate names it (below) AND the reusable's authorize | ||
| # matches it; renaming the label means changing both. | ||
| if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'claude-review') || (github.event.action != 'labeled' && vars.CLAUDE_ALWAYS_ON == 'true') || (github.event.action == 'ready_for_review' && contains(github.event.pull_request.labels.*.name, 'claude-review')) }} |
There was a problem hiding this comment.
💡 Suggestion (non-blocking) — Simplify if-gate logic
Use the proposed simplified logic to improve readability and ensure all relevant PR events are covered for both always-on and opt-in modes.
| # Note: the `gemini-review` label name is deliberately duplicated — | ||
| # the caller gate names it (below) AND the reusable's authorize | ||
| # matches it; renaming the label means changing both. | ||
| if: ${{ (github.event.action == 'labeled' && github.event.label.name == 'gemini-review') || (github.event.action != 'labeled' && vars.GEMINI_ALWAYS_ON == 'true') || (github.event.action == 'ready_for_review' && contains(github.event.pull_request.labels.*.name, 'gemini-review')) }} |
There was a problem hiding this comment.
💡 Suggestion (non-blocking) — Simplify if-gate logic
Use the proposed simplified logic to improve readability and ensure all relevant PR events are covered for both always-on and opt-in modes.
Brings the reusable-owned-cancellation migration (#219) to
v1.x— byte-identical to main once #219 merges, so merge #219 first, then this (caller validation compares against the default branch).🤖 Generated with Claude Code
https://claude.ai/code/session_01S94XethbGXpAb4DRKMD4kt