Skip to content

ci(v1.x): sync the #91 cancellation migration - #220

Open
heskew wants to merge 3 commits into
v1.xfrom
sync-cancellation-v1.x
Open

ci(v1.x): sync the #91 cancellation migration#220
heskew wants to merge 3 commits into
v1.xfrom
sync-cancellation-v1.x

Conversation

@heskew

@heskew heskew commented Aug 28, 2026

Copy link
Copy Markdown
Member

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

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
@heskew
heskew requested a review from a team as a code owner August 28, 2026 23:05
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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
What: The if: gate for the review job still misses synchronize, opened, and reopened events when the claude-review label is present but vars.CLAUDE_ALWAYS_ON is false.
Why it matters: PRs that opt into AI reviews via the label will not receive automated re-reviews on new pushes (synchronize) or when reopened. The recent addition of ready_for_review to the gate is incomplete and doesn't cover the other common pull request actions for opt-in users.
Suggested fix: Apply the simplified, robust logic proposed in the prior review:

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
What: The if: gate for the review job still misses synchronize, opened, and reopened events when the gemini-review label is present but vars.GEMINI_ALWAYS_ON is false.
Why it matters: Similar to the Claude workflow, opt-in users will miss automated re-reviews on subsequent pushes. The ready_for_review addition does not solve the underlying issue flagged in the prior round.
Suggested fix: Apply the simplified, robust logic proposed in the prior review:

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)

  • .github/workflows/claude-review.yml:36 — Use the simplified if: gate logic proposed in Finding 1 to improve readability and coverage.
  • .github/workflows/gemini-review.yml:44 — Use the simplified if: gate logic proposed in Finding 2 to improve readability and coverage.

heskew and others added 2 commits August 28, 2026 16:21
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')) }}

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.

🔴 BlockerMissing 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')) }}

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.

🔴 BlockerMissing 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')) }}

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.

💡 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')) }}

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.

💡 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.

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.

1 participant