Add Safari build coverage to pull request CI - #1023
Merged
Merged
Conversation
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request updates Safari build scripting and its optional DMG dependency, configures Python and Xcode in tagged releases, and adds macOS CI coverage that builds and verifies Safari app and DMG artifacts. ChangesSafari build and release integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant SafariBuild
participant SafariConverter
participant Xcodebuild
participant ArtifactChecks
GitHubActions->>SafariBuild: run npm run build:safari
SafariBuild->>SafariConverter: convert generated extension
SafariConverter->>Xcodebuild: archive and export Safari app
SafariBuild-->>GitHubActions: create Safari app and DMG
GitHubActions->>ArtifactChecks: verify bundle, codesign, and DMG
ArtifactChecks-->>GitHubActions: pass or fail build
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
PR Summary by QodoAdd Safari macOS build + artifact verification to PR CI
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Contributor
Code Review by Qodo
1. Mutable action tags
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds Safari packaging validation to the PR CI workflow so macOS/Xcode-specific regressions surface earlier, without involving release signing credentials.
Changes:
- Extend PR/push path filters to include
safari/**. - Add a
macos-15safari-buildjob that runsnpm run build:safari. - Add artifact verification for exactly one exported
.appbundle plus DMG integrity checks.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: maxim-lobanov/setup-xcode@v1 | ||
| with: | ||
| xcode-version: 16.2 | ||
| - run: npm run build:safari |
PeterDaveHello
force-pushed
the
addSafariMacCi
branch
from
July 21, 2026 17:02
2f2e92a to
677219c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
safari/build.sh (1)
15-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider using
npx appdmgfor direct script execution.Because
appdmgwas moved topackage.json'soptionalDependencies, its executable will reside innode_modules/.bin. If this script is executed directly (./safari/build.sh) instead of via an npm script (e.g.,npm run build:safari), theappdmgcommand will not be found.Using
npx --no-install appdmgensures that the local binary is reliably resolved regardless of how the script is executed.♻️ Proposed fix
rm -f ./build/safari.dmg -appdmg ./safari/appdmg.json ./build/safari.dmg +npx --no-install appdmg ./safari/appdmg.json ./build/safari.dmg🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@safari/build.sh` around lines 15 - 16, Update the appdmg invocation in the Safari build script to use npx --no-install, ensuring the locally installed optional dependency is resolved when the script runs directly. Keep the existing input and output arguments unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tagged-release.yml:
- Around line 43-46: Remove the event-name condition from the
actions/setup-python@v6 step in the tagged release workflow so Python 3.10 is
configured for every workflow trigger, including manual runs, before the
existing npm ci step executes.
In `@safari/build.sh`:
- Around line 1-16: Make the patch application flow in the build script
rerunnable: update the handling around git apply safari/project.pre.patch and
git apply --allow-empty safari/project.patch so already-applied patches are
safely reverted, skipped, or otherwise guarded without triggering set -e
failures. Preserve the existing build, conversion, archive, export, and
packaging sequence.
---
Nitpick comments:
In `@safari/build.sh`:
- Around line 15-16: Update the appdmg invocation in the Safari build script to
use npx --no-install, ensuring the locally installed optional dependency is
resolved when the script runs directly. Keep the existing input and output
arguments unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b7bf8ea-ad0c-4675-b781-bfe5ade2ba1d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/pr-tests.yml.github/workflows/tagged-release.ymlpackage.jsonsafari/build.shsafari/project.pre.patch
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pr-tests.yml
Contributor
|
Code review by qodo was updated up to the latest commit 677219c |
PeterDaveHello
force-pushed
the
addSafariMacCi
branch
from
July 21, 2026 19:16
677219c to
c266ff0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tagged-release.yml:
- Around line 43-48: Update the release job’s macOS runner from macos-14 to a
supported runner, using macos-15-intel if the workflow requires x86_64. Verify
that the existing Xcode setup via maxim-lobanov/setup-xcode and Python 3.10
configuration for appdmg remain compatible with the selected runner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 867d0d88-70a5-4e20-862f-160f00aeaf4b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/pr-tests.yml.github/workflows/tagged-release.ymlpackage.jsonsafari/build.shsafari/project.pre.patch
🚧 Files skipped from review as they are similar to previous changes (3)
- safari/project.pre.patch
- safari/build.sh
- .github/workflows/pr-tests.yml
Comment on lines
+43
to
+48
| - uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: '3.10' # for appdmg | ||
| - uses: maxim-lobanov/setup-xcode@v1 | ||
| with: | ||
| xcode-version: 16.2 |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' .github/workflows/tagged-release.ymlRepository: ChatGPTBox-dev/chatGPTBox
Length of output: 3914
🌐 Web query:
GitHub Actions runner images macos-14 deprecation support end arm64 macos-14-large macos-15-intel current documentation
💡 Result:
The macOS 14-based GitHub Actions runner images are currently in the deprecation process and will reach their end of support on November 2, 2026 [1][2]. Key details regarding this deprecation: - Deprecation process: The deprecation of macOS 14 and macOS 14 arm64 images began on July 6, 2026 [1][2]. - End of Support: These images will be fully unsupported for both GitHub Actions and Azure DevOps starting November 2, 2026 [1][2]. - Affected labels: Workflows utilizing the macos-14, macos-14-large, or macos-14-xlarge labels are affected [1]. These jobs will be terminated with an error after the retirement date [1]. Recommended Migration: Users are advised to update their workflows to use more recent image labels [1]. Current alternatives include: - For general use: macos-latest, macos-15, or macos-26 [1]. - For large runners: macos-latest-large, macos-15-large, or macos-26-large [1]. - For xlarge runners: macos-latest-xlarge, macos-15-xlarge, or macos-26-xlarge [1]. Note on macos-15-intel: While the user inquired about macos-15-intel, current documentation identifies support for macOS 15 and macOS 26 environments, with specific Intel-based runner options available as macos-15-intel and macos-26-intel [3]. Users should monitor the official runner-images repository for specific image availability and version updates [4][5].
Citations:
- 1: [macOS] The macOS 14 Sonoma based runner images will begin deprecation on July 6th and will be fully unsupported by November 2nd for GitHub Actions and Azure DevOps actions/runner-images#13518
- 2: https://eosl.date/eol/product/github-actions-runner-images/
- 3: https://docs.github.com/en/actions/reference/runners/github-hosted-runners
- 4: https://github.com/actions/runner-images
- 5: https://github.com/actions/runner-images/releases
Migrate this release job off macos-14. macos-14 is in deprecation and will stop being supported on Nov 2, 2026, so this release workflow will eventually fail. Move to a supported runner (for example macos-15-intel if x86_64 is required) and recheck Xcode 16.2 and appdmg compatibility.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/tagged-release.yml around lines 43 - 48, Update the
release job’s macOS runner from macos-14 to a supported runner, using
macos-15-intel if the workflow requires x86_64. Verify that the existing Xcode
setup via maxim-lobanov/setup-xcode and Python 3.10 configuration for appdmg
remain compatible with the selected runner.
Source: MCP tools
Contributor
|
Code review by qodo was updated up to the latest commit c266ff0 |
PeterDaveHello
force-pushed
the
addSafariMacCi
branch
from
July 21, 2026 19:35
c266ff0 to
8016c44
Compare
Comment on lines
+43
to
+48
| - uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: '3.10' # for appdmg | ||
| - uses: maxim-lobanov/setup-xcode@v1 | ||
| with: | ||
| xcode-version: 16.2 |
Comment on lines
+49
to
+54
| safari-build: | ||
| runs-on: macos-14 | ||
| timeout-minutes: 20 | ||
| permissions: | ||
| contents: read | ||
|
|
| contents: read | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v7 |
Contributor
There was a problem hiding this comment.
1. Mutable action tags 🐞 Bug ⛨ Security
The workflows use mutable GitHub Action tags (e.g., @v7, @v6, @v1) rather than immutable commit SHAs, so CI/release behavior can change without any repository change if those tags move. This weakens build reproducibility and increases CI/CD supply-chain risk, particularly for the Safari packaging path that produces distributable artifacts.
Agent Prompt
## Issue description
Workflows reference actions by mutable tags (e.g. `actions/checkout@v7`, `maxim-lobanov/setup-xcode@v1`). Tags can be moved, which can change CI/release behavior without any change in this repo.
## Issue Context
This PR adds/updates workflow steps for Safari build coverage and release setup. Pinning actions to a full commit SHA makes CI runs reproducible and reduces supply-chain exposure.
## Fix Focus Areas
- .github/workflows/pr-tests.yml[40-67]
- .github/workflows/tagged-release.yml[36-48]
## Implementation notes
- Replace each `uses: owner/action@vN` with `uses: owner/action@<full_sha>`.
- Optionally keep the human-readable version as a comment, e.g. `# v7`.
- Apply consistently to the actions introduced/modified by this PR (and ideally across all workflows for consistency).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
|
Code review by qodo was updated up to the latest commit 8016c44 |
Make Safari packaging fail on intermediate errors and install appdmg from the lockfile after configuring the macOS toolchain. Apply the Safari-only name change to generated Firefox output so repeated builds leave tracked manifests and Firefox archives intact. Remove the obsolete empty Xcode project patch.
Run Safari packaging on macOS for pull requests and master updates that can affect release output. Verify the app bundle signature and disk image without release credentials, and require Safari validation before badge updates.
PeterDaveHello
force-pushed
the
addSafariMacCi
branch
from
July 21, 2026 20:01
8016c44 to
1bc18b8
Compare
Contributor
|
Code review by qodo was updated up to the latest commit 1bc18b8 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
macos-14for pullrequests and
masterupdates that can affect release outputsigning credentials
Firefox release artifacts
appdmg@0.6.6from the committed lockfileWhy
Safari packaging previously had no macOS coverage in pull request CI.
The existing build script also installed
appdmgdynamically andcontinued after failed intermediate commands, making its CI signal
non-reproducible and susceptible to false success.
The new job matches the current release runner and exercises the same
locked, fail-fast packaging path without release signing credentials.
The tagged-release workflow only changes toolchain setup ordering and
removes version-substitution steps that became no-ops; publishing and
release signing behavior are unchanged.
Validation
master(89cf0a4)npm run prettynpm run lintnpm test(893 tests passed)npm run buildfirefox.zipretains theChatGPTBoxnameshellcheck safari/build.shbash -n safari/build.shgit diff --checkactionable issues
testspassed on Linuxsafari-buildpassed on macOS, including the Xcodearchive, ad-hoc app signature, and DMG integrity validation
Manual browser smoke testing was skipped because no extension runtime
files changed.