[codex] Gate desktop changelogs and release tags#8371
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc175febee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ref is None: | ||
| output = git(["ls-files", "desktop/macos"]) | ||
| else: | ||
| output = git(["diff", "--name-only", "--diff-filter=ACM", f"{ref}..HEAD", "--", "desktop/macos"]) |
There was a problem hiding this comment.
Include deletions in desktop release planning
When the only desktop app change since the latest tag is a deletion (for example removing a Swift source or bundled resource), this --diff-filter=ACM diff returns no paths, so changes is empty and the workflow writes should_release=false. The push workflow is still triggered by a deleted desktop/macos/** path, but no macOS tag is created, so that removal never reaches Codemagic until a later non-delete desktop change happens.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
3 issues found across 5 files
Confidence score: 2/5
- In
.github/scripts/plan-desktop-release.py, the Codemagic status check fails open on API errors, so the workflow can create a new release tag while the prior release state is unknown; this could produce overlapping or out-of-order desktop releases—treat API/read failures as blocking before merge. - In
.github/scripts/plan-desktop-release.py, release planning misses deleted or renamed desktop files, which can setshould_release=falseeven when real app changes happened; that risks silently skipping needed desktop releases—include delete/rename events in the change-detection logic before merging. - In
.github/workflows/lint.yml, substring-based label matching can mis-detect labels (for example, matchingnot-no-changelog-needed) and incorrectly skip changelog requirements; this is low impact but noisy governance risk—switch to exact array membership matching in the workflow expression.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a154a18af2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "${{ contains(github.event.pull_request.labels.*.name, 'no-changelog-needed') }}" = "true" ]; then | ||
| SKIP_FLAG="--skip" |
There was a problem hiding this comment.
Trigger lint when the skip label changes
The new no-changelog-needed escape hatch is evaluated only when this lint workflow runs, but this workflow still uses the default pull_request activity types, so adding the label after a failed changelog check will not start a new run, and removing the label after a skipped check will not invalidate the green status. In those PRs the label state and required check can stay out of sync until someone pushes another commit; add labeled/unlabeled to the PR trigger types if the label is meant to control enforcement.
Useful? React with 👍 / 👎.
| def changed_files(base_ref: str, head_ref: str) -> list[str]: | ||
| output = run_git(["diff", "--name-only", "--diff-filter=ACM", f"{base_ref}...{head_ref}"]) |
There was a problem hiding this comment.
Include deletions in changelog enforcement
The release planner now considers deletions, but this PR lint guard still filters changed files with --diff-filter=ACM, which drops deleted desktop paths. A PR that only removes a user-facing desktop file or resource under desktop/macos/ will therefore leave requiring_changelog empty and pass without either an unreleased changelog entry or the no-changelog-needed label.
Useful? React with 👍 / 👎.
Summary
desktop/macos/CHANGELOG.jsonunreleased entry, with ano-changelog-neededlabel escape hatch for internal-only changes.AGENTS.mddesktop guidance to the existing desktop changelog authoring instructions.desktop_backend_auto_dev.ymlremains responsible for Backend-Rust dev deploys.Validation
python3 -m py_compile .github/scripts/check-desktop-changelog.py .github/scripts/plan-desktop-release.pypython3 -m json.tool desktop/macos/CHANGELOG.json/Users/dazheng/go/1.23.4/bin/actionlint .github/workflows/desktop_auto_release.yml .github/workflows/lint.ymlgit diff --checkshould_release=falsewhilev0.11.533+11533-macosCodemagic release wasin_progress.force_releasemode returnedshould_release=truefor the same pending desktop changes.