Skip to content

feat: dispatch chromium-upgrade workflow for release-branch Chromium rolls - #214

Open
claude[bot] wants to merge 1 commit into
mainfrom
feat/dispatch-chromium-upgrade-for-release-branches
Open

claude[bot] wants to merge 1 commit into
mainfrom
feat/dispatch-chromium-upgrade-for-release-branches

Conversation

@claude

@claude claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Requested by Samuel Attard · Slack thread

Before: A Chromium roll on a release branch (for example electron/electron#54085, roller/chromium/44-x-y) opened or updated its PR and stopped there. The roller only dispatched chromium-upgrade.yml in electron/agent-workflows for rolls on main, and the dispatch carried no inputs, so the workflow could only ever run against roller/chromium/main.

After: Every Chromium roll that changes DEPS dispatches the workflow with base-ref set to its own roll branch (roller/chromium/<electron branch>), so release-branch rolls get the same agent-driven upgrade, signed push and 🚧 label lifecycle as main rolls.

How: The gate at the end of roll() in src/utils/roll.ts drops the main-only check, and triggerChromiumUpgradeWorkflow now takes the roll branch name and passes inputs: { 'base-ref': rollBranchName } to createWorkflowDispatch. The roll branch name is computed once at the top of roll() and reused by the existing update and create paths. Node rolls, paused PRs (roller/pause) and passes where the DEPS version is unchanged still do not dispatch; tests cover each of those plus the new release-branch dispatch.

No change to electron/agent-workflows is needed: its setup job already validates any roller/* base-ref and looks up that branch's PR, its publish job fast-forwards that same branch, and its concurrency group is keyed on base-ref, so a release-branch run never cancels (or is cancelled by) the main roll. Note that one cron pass may now dispatch one run per supported release branch in addition to main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SmvfDMwrFWz6GBTVqJjs9h


Generated by Claude Code

…rolls

Release-branch Chromium roll PRs (roller/chromium/N-x-y) previously never
triggered the electron/agent-workflows chromium-upgrade.yml workflow, which
only ran for main and always against its default roller/chromium/main base.
Every Chromium roll that changes DEPS now dispatches the workflow with the
roll's own branch as the base-ref input, so release branches get the same
agent-driven upgrade as main.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SmvfDMwrFWz6GBTVqJjs9h
@MarshallOfSound
MarshallOfSound marked this pull request as ready for review September 18, 2026 15:58
@MarshallOfSound
MarshallOfSound requested review from a team as code owners September 18, 2026 15:58

@claude claude Bot left a comment

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.

LGTM. This is a small, well-contained change: the roll branch name is hoisted once and reused, triggerChromiumUpgradeWorkflow now passes it as base-ref, and the dispatch gate correctly drops the main-only check while still gating on didRoll (so paused PRs and unchanged-DEPS passes still skip dispatch). Tests were updated to match the new payload shape and add coverage for the release-branch dispatch, paused-PR, and unchanged-DEPS cases.

Extended reasoning...

Overview

The diff modifies src/utils/roll.ts and its test file. It hoists the rollBranchName computation (roller/<target>/<electron branch>) to the top of roll(), removing a previously duplicated local computation in two branches of the function. triggerChromiumUpgradeWorkflow now accepts this branch name and passes it as inputs: { 'base-ref': rollBranchName } to octokit.actions.createWorkflowDispatch. The dispatch call site's gate was loosened from didRoll && chromium && electronBranch.name === MAIN_BRANCH to didRoll && chromium, so release-branch Chromium rolls now also trigger the workflow (with their own roll branch as base-ref) rather than only main rolls.

Security risks

No new injection, auth, or data-exposure risk. The rollBranchName value passed as base-ref is derived entirely from trusted inputs (rollTarget.name and electronBranch.name, both controlled by the roller code itself, not from any PR or user-supplied field) — consistent with the existing comment in the code about deriving the write target only from this trusted naming rather than from untrusted PR fields. The didRoll flag continues to gate dispatch correctly: it is only set true on the two paths where an actual DEPS change occurred (existing-PR body update, and new-PR creation), so paused PRs and unchanged-DEPS passes still correctly skip the dispatch, which the new tests explicitly verify.

Level of scrutiny

This is a low-to-moderate scrutiny change: it's a behavioral change (widening a dispatch condition) but confined to a single well-understood function, with no new external inputs, no crypto/auth logic, and full test coverage added for the new and edge-case behaviors. I read the full roll() function to confirm the didRoll and branch-naming logic wasn't broken by the hoisting refactor, and traced through both the update-existing-PR and create-new-PR paths.

Other factors

The test diff aligns precisely with the described behavior: existing dispatch assertions now expect inputs: { 'base-ref': ... }, the release-branch test was flipped from asserting no dispatch to asserting a dispatch with the correct base-ref, and two new tests cover the paused-PR and unchanged-DEPS non-dispatch cases. I verified the test's branch fixture (testBranch) is indeed a non-main branch, confirming the release-branch test exercises the intended new code path. A human reviewer (MarshallOfSound) already approved this PR with no outstanding objections. I was unable to run the test suite directly due to sandboxing restrictions, but the code and test diffs are consistent and mechanically verifiable by inspection.

@MarshallOfSound
MarshallOfSound enabled auto-merge (squash) September 18, 2026 16:57

This branch has not been deployed

No deployments
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