Skip to content

ci: identify the merge-group fail-fast watcher's source workflow by file, not display name - #13793

Merged
teamleaderleo merged 5 commits into
mainfrom
ci/fail-fast-workflow-identity
Sep 23, 2026
Merged

teamleaderleo merged 5 commits into
mainfrom
ci/fail-fast-workflow-identity

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

.github/workflows/merge-group-fail-fast.yml decides which workflow may start it by matching a human-readable display name: on.workflow_run.workflows: [Merge-group policy checks]. That string is the name: of merge-group-policy-checks.yml. Anyone who renames that workflow for clarity disables the watcher, and nothing reports it — the watcher simply never triggers again, so a merge group whose first macOS job fails runs the rest of its shards to the end and every queue entry behind it waits. The merge queue is disabled on this repository right now (flaky and cancelled runs clogged it), so the watcher is dormant and this can rot unnoticed until the queue is turned back on. Three more workflows depend on another workflow's display name the same way: persistent-macos-router.yml (CI), update-homebrew.yml (Release macOS app) and cmux-tui-release-delivery.yml (cmux-tui release binaries).

Resulting behavior

on.workflow_run.workflows: accepts only a workflow's display name, so the name stays in the trigger. It is no longer the only copy of the pairing:

  • Each of the four consumers declares env.SOURCE_WORKFLOW_PATHS, the file that must carry the name it waits for. tests/test_ci_workflow_run_sources.py reads that file's own name: and fails when the two disagree, so a rename fails the pull request that renames it instead of silently switching a guard off.
  • merge-group-fail-fast.yml confirms the identity at run time as well. The workflow_run payload carries path and workflow_id next to the presentation name, and display names are not unique, so the watcher now refuses to cancel anything unless it was started by .github/workflows/merge-group-policy-checks.yml. The CI run it then watches was already located by file (actions/workflows/ci.yml/runs), and the test asserts every such lookup names a workflow that exists.

API evidence for the fields used, from a real merge-group run of the source workflow:

$ gh api "repos/manaflow-ai/cmux/actions/workflows/merge-group-policy-checks.yml/runs?per_page=1" \
    --jq '.workflow_runs[0] | {id, name, path, workflow_id, event}'
{"event":"merge_group","id":35503457744,"name":"Merge-group policy checks",
 "path":".github/workflows/merge-group-policy-checks.yml","workflow_id":362434528}

Sweep for the same shape

Every other place in .github/ and scripts/ci/ that matches a job, check or workflow by display text:

Where Matches Status
merge-group-fail-fast.yml, persistent-macos-router.yml, update-homebrew.yml, cmux-tui-release-delivery.yml another workflow's name: fixed here
update-homebrew.yml:58 select(.name == "build-sign-notarize") a job in release.yml already pinned by tests/test_release_homebrew_gate.py, which also asserts the job carries no name: override
ios-testflight.yml:159,377 job.name === 'Upload to TestFlight' its own upload job's name: pinned here, derived from that job
tests/test_ci_merge_queue_required_checks.py REQUIRED_CHECKS branch-protection check names unchanged. The source of truth is the GitHub API, not the tree; reconciling needs administration: read, which PR CI must not hold
tests/test_tui_publish_workflow_security.py:1604 cmux-tui release binaries, written out again left as is; the new test now derives the same name from the file, so a rename fails there first
ci.yml:458 release_only_jobs ci-macos.yml job ids not this class. Job ids are identity, and the step parses them out of the file

The jobs API has no stable alternative for the job matches: its objects expose name and workflow_name, never the YAML job id (gh api .../jobs --jq '.jobs[0] | keys' on release run 35225939389). That is why those stay name matches with a test pinning them.

Validation and remaining gap

  • python3 tests/test_ci_workflow_run_sources.py fails on the first commit and passes on the second.
  • Four negative controls, each applied to the tree and reverted: renaming merge-group-policy-checks.yml, renaming the TestFlight upload job, replacing the run-time path check with name, and pointing the CI lookup at a file that does not exist. Each is reported as a distinct failure.
  • actionlint clean on all five changed workflows. test_ci_change_areas.py (which asserts this watcher's shape), test_release_homebrew_gate.py, test_tui_publish_workflow_security.py, test_ci_persistent_mac_compile.py, test_ci_merge_queue_required_checks.py, test_ci_guard_workflow_structure.py, test_ci_linux_guard_routing.py, test_ci_reusable_workflow_permissions.py and the execution-registry validator all pass. Two test_ci_change_areas.py failures (test_workflow_self_change_guard_runs_before_detector_imports, test_router_change_with_app_source_uses_trusted_base_product_routing) reproduce unchanged on a clean checkout of the base commit 95e843a and are unrelated to this branch.
  • Remaining gap: the merge queue is off, so the run-time identity check has not executed against a live merge group. Nothing here runs until the queue is re-enabled; the static test is what holds in the meantime. REQUIRED_CHECKS still duplicates branch protection with no way to derive it.

Part of #13095. Row 2 of docs/ci/derived-not-declared.md.

🤖 Generated with Claude Code


Summary by cubic

Pins four workflow_run consumers to the workflow files they expect, while retaining GitHub's display-name trigger filter. Renames now fail CI instead of silently disabling consumers, and the merge-group watcher verifies the triggering file before cancelling runs.

  • Adds SOURCE_WORKFLOW_PATHS validation and runs it in CI guards, covering missing workflows, mismatched names, and file-based run lookups.
  • Makes the fail-fast watcher locate CI runs by workflow file and reject unexpected source workflows or events.
  • Updates the review-fabric test to validate router behavior and explicit ownership instead of grepping stale path literals.
  • Bounds the suite-coverage job with a 5-minute timeout.
  • Part of [RFC] CI structure: thin router, reusable platform workflows, merge queue, failure ratchet #13095.

Written for commit c1865e5. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 2 commits September 22, 2026 19:34
`on.workflow_run.workflows:` matches a workflow's display `name:`. Renaming a
workflow for clarity stops every consumer from triggering, and nothing turns
red: the consumer simply never runs again. Four workflows depend on a name this
way, and `merge-group-fail-fast.yml` is the expensive one -- when it stops
firing, a doomed merge group runs its macOS jobs to the end and every queue
entry behind it waits.

This test requires each consumer to declare the workflow file it means, derives
the expected name from that file, and requires the fail-fast watcher to confirm
`github.event.workflow_run.path` before it cancels anything. It fails until the
next commit adds those declarations.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each workflow_run consumer now declares the file whose display name its
`workflows:` filter spells out, so the pairing is written down in terms that do
not change when someone renames a workflow for clarity.

`merge-group-fail-fast.yml` also checks that identity at run time. The
workflow_run object carries `path` next to the presentation `name`, and display
names are not unique, so the watcher confirms it was started by
merge-group-policy-checks.yml before it cancels a CI run. The CI run it then
watches was already located by workflow file rather than by name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 46d94241-9040-4613-90c8-df30ca4bf9f5

📥 Commits

Reviewing files that changed from the base of the PR and between 83a6294 and c1865e5.

📒 Files selected for processing (8)
  • .github/workflows/ci-guards.yml
  • .github/workflows/ci.yml
  • .github/workflows/cmux-tui-release-delivery.yml
  • .github/workflows/merge-group-fail-fast.yml
  • .github/workflows/persistent-macos-router.yml
  • .github/workflows/update-homebrew.yml
  • tests/test-execution.toml
  • tests/test_ci_workflow_run_sources.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@blacksmith-sh

This comment has been minimized.

…text (#13788)

test_ci_executes_review_fabric_contracts grepped
scripts/ci/detect_linux_guard_changes.py for each review-fabric path as a
literal string. #13775 made the guard routes derived from PATH_OWNERS and from
ci-guards.yml's run: lines, so those literals are no longer in the file and the
test fails on a clean main -- taking preflight, Guard status, linux-preflight,
tests and ci-status down with it on every pull request routing that lane.

The routing is intact; only the check was stale. Assert the behaviour instead:
the path must route linux_guard_tests, and groups_for_path must explicitly own
it with preflight. That second assertion uses groups_for_path rather than
classify_test_groups because classify_test_groups falls open to every group for
an unknown path, and so would keep passing if ownership were dropped.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 23, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

* ci: bound the suite coverage gate

* ci: align timeout placement with in-flight branch fixes
@teamleaderleo
teamleaderleo merged commit ee107f7 into main Sep 23, 2026
51 of 52 checks passed
teamleaderleo added a commit that referenced this pull request Sep 23, 2026
Three claims in the inventory were wrong, found while implementing the
rows they describe.

Row 2 listed `ci.yml` as a display-name dependency of
merge-group-fail-fast.yml. It is not: that reference is
`actions/workflows/ci.yml/runs`, a file path, which is already stable.
Only the `workflow_run` trigger names a workflow by display name. The
row also said deriving it was impossible; a test can pin the trigger
against the producer's own `name:`, which #13793 does.

Row 8 called `release_only_jobs` a set of job names. They are job ids:
the surrounding code partitions on `\njobs:\n` and splits on the YAML
keys, so a display name never participates.

Row 4's deferral reason estimated that syncing the path lists would
make "nearly every push to main" run the workflow. Measured against
this checkout it is 11 of 67 merges, and the expensive half stays
gated. #13789 implements the row.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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