fix(desktop): skip the macOS integration test on merge queue batches - #96546
Conversation
Trunk's merge queue opens a real PR from a trunk-merge/** branch rather than using GitHub's native merge queue, so the `github.event_name != 'merge_group'` arm on the integration-test job never matches there. The job requested a depot-macos-26 runner on every batch, where the wait holds the queue. Every batched PR already ran this suite in full, so the batch run is redundant. The skip is scoped to same-repo heads, matching the idiom already used by desktop-agent-release-verify.yml, desktop-react-doctor.yml, and the backend-coupling job in desktop-ci.yml. Generated-By: PostHog Desktop Task-Id: 0fa7390b-35a9-4614-a4ac-8e91c87e5eee
webjunkie
added
the
skip-agent-review
|
😎 Merged manually by force-merge-posthog[bot] - details. |
|
/trunk merge --priority high |
🤖 CI report
|
There was a problem hiding this comment.
🟢 Approval recommended
The workflow logic change is small and targeted; only a minor comment cleanup was flagged.
Pull request overview
This PR updates the Desktop CI workflow to avoid running the macOS integration-test job on Trunk merge-queue batch PRs (trunk-merge/**), preventing merge-queue batches from consuming scarce depot-macos-26 capacity.
Changes:
- Extend the
integration-testjob condition to skip on same-repotrunk-merge/**heads (in addition to the existingmerge_groupskip). - Preserve existing change-detection gating so normal PRs still run
integration-testwhen relevant files change.
File summaries
| File | Description |
|---|---|
| .github/workflows/desktop-test.yml | Update integration-test job if: guard to skip on same-repo trunk-merge/** batch PR heads. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Fail closed: if change detection itself failed, run instead of skipping. | ||
| if: ${{ !cancelled() && github.event_name != 'merge_group' && (needs.changes.result != 'success' || needs.changes.outputs.code == 'true' || needs.changes.outputs.workflow == 'true') }} | ||
| # | ||
| # Also skipped on trunk-merge/** heads. Trunk's queue opens a real PR, so | ||
| # the merge_group arm never matches and this ran on every batch. Every | ||
| # batched PR already ran it in full, and depot-macos-26 is the suite's only |
|
Warning A break-glass force-merge has been requested for this PR. Initiated by: Julian (Slack) Branch protection requirements will be bypassed. If this was unexpected, contact #team-security. |
|
✅ Force-merge completed. |
Problem
integration-testjob runs for about 5 minutes but waits for adepot-macos-26slot, and recent master runs waited 0, 0, 2, 3, 5, 5, 12, 57, and 76 minutes for one.desktop-test.ymltries to skip the job on the merge queue withgithub.event_name != 'merge_group', but master lands through Trunk, which opens a real pull request from atrunk-merge/**branch. The event ispull_request, so the guard never matches and the job runs on every batch.Changes
e2ejob currently needsintegration-test, so it skips on those heads too. That is the same cascade themerge_grouparm already relies on, andDesktop Tests Passcounts a skipped dependency as a pass.unit-test, typecheck, quality, and build still run on batches, where they are cheap and can catch a conflict the batch introduces.trunk-merge/**. This matchesdesktop-agent-release-verify.yml,desktop-react-doctor.yml, and thebackend-couplingjob indesktop-ci.yml.This narrows demand on the macOS pool rather than fixing allocation, so it stacks with the other work in flight rather than replacing it.
flowchart LR A["trunk-merge/** batch"] --> B["changes"] B --> C["unit-test<br/>(linux, ~1m)"] B --> D["integration-test<br/>(depot-macos-26, ~5m run,<br/>up to ~76m queued)"] C --> E["e2e (linux)"] D --> E E --> F["Desktop Tests Pass"] style D fill:#F54E00,stroke:#1D4AFF,color:#fffflowchart LR A["trunk-merge/** batch"] --> B["changes"] B --> C["unit-test<br/>(linux, ~1m)"] B --> D["integration-test<br/>skipped"] C --> E["e2e<br/>skipped (needs integration-test)"] D --> E E --> F["Desktop Tests Pass"] style D fill:#30ABC6,stroke:#1D4AFF,color:#fff style E fill:#30ABC6,stroke:#1D4AFF,color:#fffHow did you test this code?
if:expression.bin/hogli lint:workflowspasses (9 checks, 134 workflows), andactionlint .github/workflows/desktop-test.ymlis clean.bin/hogli ci:preflight --fixreports 0 failures.trunk-merge/pr-96071/...head printsExpanded: (!false && ('pull_request' != 'merge_group') && ...), thenResult: true, thenRequested labels: depot-macos-26.desktop-test.ymlruns on master.Automatic notifications
Docs update
No docs update is needed. The change affects CI scheduling only.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Raised from a Slack thread that started as a question about failing and slow desktop tests. Those turned out to be two unrelated things: the four failing assertions are already fixed by #96427, and the wall clock is this macOS queue. This PR addresses only the second.
Invoked
/authoring-ci-workflowsand/writing-pr-descriptions. Used read, bash, and edit; no subagents.Two open drafts already target the macOS pool, and neither covers this. #95852 moves the web E2E suite to its own Linux job and drops
integration-testfrome2e'sneeds; #94509 caps the smoke step and evicts cache-warming runs in the release workflows. Both reduce time spent on a macOS runner; neither stops the batch from asking for one. If #95852 lands,e2eno longer cascades and would run on batches again — the comment in the diff is worded for that, and guardinge2eseparately would be a follow-up decision, not part of this fix. #95852 also editsdesktop-test.yml, but in the job's steps rather than its condition.The diff contains no customer data or other non-public material.