fix(ci): fail closed when Codemagic desktop preview dies after dispatch (#10145) - #11171
Conversation
…ch (BasedHardware#10145) Dispatch returning a buildId was treated as success while Codemagic could die at startup with no artifact, so macos.omi.me/preview/<slug> silently fell back to stable. Poll the exact build to a terminal status and keep durable evidence. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…0145) Reject non-alphanumeric build IDs before GITHUB_OUTPUT, pass observe args via env (not ${{ }} in run) to avoid shell injection from provider data, and drop the unused sys import for Ruff. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed both cubic findings: P1 — shell injection: dispatch now rejects build IDs outside P3 — Ruff: removed unused
|
Git-on-my-level
added
workflow-review
|
Thanks — this looks like a useful fail-closed improvement for the desktop preview lane. I reviewed the observer script, workflow wiring, release-process guard update, and the new behavioral tests. The important pieces look sound: the dispatcher now preserves the Codemagic build ID through Validation I ran locally on the PR head:
I’m not formally approving because this touches the GitHub/Codemagic preview publishing path and should get a maintainer pass before merge, but my technical read is positive. One small follow-up for the human maintainer to consider: if Codemagic’s single-build endpoint can ever omit workflow/env metadata, the observer currently treats those identity fields as best-effort rather than required; that seems acceptable for the exact build ID returned by dispatch, but it is worth confirming against the live API shape. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
|
@Git-on-my-level need human response — code-owner APPROVE. Author-side clear: CI green, MERGEABLE, cubic P1/P3 addressed Branch protection is specifically waiting on code owner review from you ( Noted for maintainers: observer treats Codemagic workflow/env identity fields as best-effort if the single-build endpoint omits them — acceptable for the exact |
|
Thanks for the ping. I re-checked the current head and the workflow-side fix still looks technically sound: the Codemagic build ID is validated before entering I also reran the local validation on
I’m still not leaving a formal code-owner approval from automation because this changes the GitHub/Codemagic preview publishing lane and should get an explicit human maintainer/code-owner sign-off before merge. My maintainer signal remains positive pending that human review. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
|
@Git-on-my-level final approval reminder: PR #11171 is fully CI-green at a1b3480. Your latest technical re-review reran 9 tests, release-process guards, and diff checks with no blocker; the Codemagic build-ID validation and fail-closed observer wiring are sound. The only remaining gate is formal workflow code-owner approval; #10221 secret provisioning remains separate maintainer ops. |
Keep Codemagic preview fail-closed observer tip current for workflow CO.
|
@Git-on-my-level need human response — formal workflow/code-owner APPROVE is the only merge blocker on #11171. Author-clear:
Please human / workflow-review APPROVE (or explicit hold) when you can. Branch protection is waiting on code owner review from you ( |
|
@Git-on-my-level need human response — formal workflow/code-owner APPROVE is still the only merge blocker on #11171. Author-clear on tip
CI re-running on the main merge. Please human / |
|
Thanks — I re-reviewed the current head after the main-branch refresh. I still do not see a code blocker in the PR-specific changes, but this remains a workflow-sensitive desktop preview publishing path, so I’m leaving final workflow/code-owner sign-off to a human maintainer rather than formal approval. File-specific notes:
Validation I ran locally on this head:
Final maintainer note: the implementation looks like a useful fail-closed improvement for #10145, but because it changes CI/release-preview behavior and — Automated maintainer review by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
|
@Git-on-my-level need human response — formal workflow/code-owner APPROVE is still the only merge blocker on #11171. Author-clear on tip
Please human / |
|
Thanks for the refresh. I reviewed the current head ( File-specific notes:
Validation I ran on this head:
Maintainer note: the technical direction looks like the right fail-closed fix for #10145, but because it changes the GitHub/Codemagic desktop preview publishing lane under — Automated maintainer review by AI on behalf of David — for human maintainer review because this changes workflow-controlled desktop preview publishing. |
|
@Git-on-my-level final merge-gate ping for #11171. Current head |
|
@Git-on-my-level @kodjima33 Workflow review ping: tip Still current and still author-side clear. Verified against |
…emagic-observe # Conflicts: # .github/checks-manifest.yaml
3 participants
Summary
desktop_publish_preview.ymltreated a returnedbuildIdas success and ended. With unprovisioneddesktop_preview_secrets(or any early Codemagic failure), GH Actions stayed green whilemacos.omi.me/preview/<slug>silently fell back to stable.finishedpass;failed/canceled/timeout fail), write durable observation evidence, retain as artifact. Release-process guards + checks-manifest keep the wire from regressing.Test plan
python3 .github/scripts/test_observe_codemagic_preview_build.py→ 9 passedcheck_desktop_preview_publishing()→ 0 errorsFailure-Class: none
Fixes #10145