fix(desktop): gate the release tag on master Desktop Tests - #96677
Conversation
🤖 CI report
|
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
There was a problem hiding this comment.
🟡 Changes recommended
The new wait-for-check step likely can’t read check runs with the current job permissions (missing checks: read), which would cause the tag workflow to time out and hold releases indefinitely.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR prevents PostHog Desktop releases from being cut off a red master by gating desktop-tag.yml on the Desktop Tests Pass check for the most recent desktop-touching commit, and strengthens release validation by running the full Electron e2e suite against the signed macOS build.
Changes:
- Add an
ignore_master_cibreak-glass input and a “wait for Desktop Tests Pass” hold in.github/workflows/desktop-tag.yml. - Expand
desktop-release.yml’s post-signing validation from a smoke spec to the full Playwright Electron e2e suite. - Document the new “hold” behavior and break-glass dispatch input in
products/desktop/docs/UPDATES.md.
File summaries
| File | Description |
|---|---|
| products/desktop/docs/UPDATES.md | Documents the new tag hold behavior and the ignore_master_ci override. |
| .github/workflows/desktop-tag.yml | Gates tagging on Desktop Tests Pass via wait-for-check, with timeout/PR comment behavior and a manual bypass. |
| .github/workflows/desktop-release.yml | Runs the full Electron e2e suite on the packaged/signed app instead of only the smoke spec. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Stacked PR 96679 failed testing in the merge queue. Please investigate the failure and re-submit the stack. |
desktop-tag.yml read no CI signal at all, so a red master shipped at the next twice-daily schedule. It now waits for the Desktop Tests Pass check on the last commit that touched products/desktop/ through the shared wait-for-check action, leaves the release to the next schedule when the run has not finished, and holds the tag when the check failed. A manual dispatch can set ignore_master_ci for a break-glass release. The release job already packages and signs the app on macOS, so it runs the whole Electron suite there instead of the smoke spec. That costs no extra runner allocation and keeps macOS boot and IPC coverage in the pipeline. Claude-Session: https://claude.ai/code/session_01AAfrGhk4pdw6Y4X5pEnb9f
wait-for-check reads the check-runs API with github.token, and the job's permissions block did not grant checks: read, so every poll would 403 and hold the tag. Also state the reason for the gate precisely: the queue does run the suites, but on a batch branch, not on the master commit. Claude-Session: https://claude.ai/code/session_01AAfrGhk4pdw6Y4X5pEnb9f
Master no longer runs Desktop Tests on a push, so a gate that waits for the check on the last desktop commit would hold every release. The tag job now dispatches the workflow on master, waits for that run, and tags the commit it tested. Linux only, twice a day, about fifteen minutes. Claude-Session: https://claude.ai/code/session_01AAfrGhk4pdw6Y4X5pEnb9f
2e99b03 to
1306d62
Compare
|
This pull request was merged into |
Problem
desktop-tag.ymltags master twice a day after a quiet period and never reads a CI result.Changes
wait-for-checkaction, and tags the commit it tested.ignore_master_ciskips the check, for a break-glass release.Note
Trunk lanes merge in parallel, so the master tree can differ from any batch the queue tested. This run is the only test of the exact tree that ships.
How did you test this code?
hogli lint:workflows,actionlintandshellcheckon the new step pass locally.wait-for-checkaction, already used byci-hobby.yml.Automatic notifications
Docs update
products/desktop/docs/UPDATES.mddescribes the hold and the break-glass input.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Fable 5.1 orchestrating, Opus subagent implementing). Skills invoked: /stacking-prs, /authoring-ci-workflows, /writing-code-comments, /writing-pr-descriptions, /simplify. Middle layer of a stack on the paths filter fix. First design waited for the master push run of Desktop Tests. #96633 removed that push run while this PR was in review, so the job now dispatches the workflow itself. A hand-rolled poll was replaced by the existing
wait-for-checkaction in the simplify pass.https://claude.ai/code/session_01AAfrGhk4pdw6Y4X5pEnb9f