ci: add mac-build-smoke job — catch submodule-side breakage on every PR#806
Conversation
`mac-release.yml` is workflow_dispatch only, so a rename or path move inside the owletto submodule (Mac source lives at packages/web/apps/mac/) can silently break the release pipeline and only surface the next time someone triggers a release. This job runs xcodebuild build (unsigned, no archive) against the pinned submodule SHA on PRs that touch: - packages/web (the submodule pointer) - .github/workflows/mac-release.yml - .github/actions/setup-submodule Closes #793. Note: `github.event.pull_request.changed_files` from the issue's snippet is an integer in the webhook payload (a count, not a list), so the `contains()` filter there would never match. This implementation uses a git diff against the PR base to compute changed paths instead.
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a path-filtered ChangesMac Build Smoke Test
Sequence DiagramsequenceDiagram
participant GitHub as GitHub Actions
participant Filter as mac-build-smoke-filter
participant Smoke as mac-build-smoke
participant Submodule as setup-submodule
participant Xcode as xcodebuild
GitHub->>Filter: checkout history\ncompute PR base SHA\ndiff paths -> run=true/false
Filter->>Smoke: emit run flag
Smoke->>Submodule: setup owletto submodule (with deploy key)
Submodule->>Smoke: submodule present / stubbed info
Smoke->>Xcode: run unsigned xcodebuild on Lobu.xcodeproj\n(CODE_SIGNING_ALLOWED=NO)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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/ci.yml:
- Around line 406-409: The mac-build-smoke job is inheriting default
GITHUB_TOKEN scopes; add an explicit least-privilege permissions block to the
job (or at the workflow root) to limit token capabilities—for example add a
permissions entry that sets contents: read (and any other minimal permissions
needed by mac-build-smoke) so the GITHUB_TOKEN scope is restricted for the
mac-build-smoke job.
🪄 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 Plus
Run ID: c024be43-7cfe-45fa-8f49-aab26b5235c3
📒 Files selected for processing (1)
.github/workflows/ci.yml
| mac-build-smoke: | ||
| runs-on: macos-15 | ||
| timeout-minutes: 15 | ||
| steps: |
There was a problem hiding this comment.
Set explicit least-privilege GITHUB_TOKEN permissions for this job.
mac-build-smoke currently inherits default token scopes. Please add an explicit permissions block (at workflow or job level), e.g. contents: read, to reduce CI token blast radius.
Suggested hardening change
jobs:
+ mac-build-smoke:
+ permissions:
+ contents: read
- mac-build-smoke:
runs-on: macos-15
timeout-minutes: 15📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mac-build-smoke: | |
| runs-on: macos-15 | |
| timeout-minutes: 15 | |
| steps: | |
| mac-build-smoke: | |
| permissions: | |
| contents: read | |
| runs-on: macos-15 | |
| timeout-minutes: 15 | |
| steps: |
🤖 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/ci.yml around lines 406 - 409, The mac-build-smoke job is
inheriting default GITHUB_TOKEN scopes; add an explicit least-privilege
permissions block to the job (or at the workflow root) to limit token
capabilities—for example add a permissions entry that sets contents: read (and
any other minimal permissions needed by mac-build-smoke) so the GITHUB_TOKEN
scope is restricted for the mac-build-smoke job.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc206102bb
ℹ️ 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".
| # when the submodule pointer, the release workflow, or the submodule | ||
| # setup action changes. | ||
| mac-build-smoke: | ||
| runs-on: macos-15 |
There was a problem hiding this comment.
Avoid scheduling macOS runners before filtering paths
Because the path gate runs as the first step inside mac-build-smoke, GitHub still schedules a macos-15 runner and performs checkout for every PR before this script can decide to skip. In this CI workflow, pull_request triggers all PRs, so backend/docs-only changes still consume scarce macOS runner startup time and can wait on macOS capacity; move the path decision to a job/workflow-level gate or a cheap Linux prefilter job before starting the macOS build job.
Useful? React with 👍 / 👎.
| # there, so skip the rest gracefully rather than fail. Same-repo PRs | ||
| # have the key, so a stub here means the secret is misconfigured. | ||
| - name: Fail fast if submodule is stubbed (same-repo PRs only) | ||
| if: steps.filter.outputs.run == 'true' && steps.submodule.outputs.stubbed == 'true' && github.event.pull_request.head.repo.full_name == github.repository |
There was a problem hiding this comment.
Treat push runs as same-repo when the submodule is stubbed
When this job runs on push to main, the filter defaults to run=true but github.event.pull_request is absent. If OWLETTO_WEB_DEPLOY_KEY is accidentally missing, setup-submodule writes the stub and this same-repo guard does not fire, so the build is skipped and CI can stay green until mac-release.yml fails later; make non-PR events fail on stubbed == 'true' and reserve the graceful skip only for actual fork PRs.
Useful? React with 👍 / 👎.
…tighten push stub-check pi review findings on the mac-build-smoke job: 1. Skipped PRs allocated a macOS runner just to evaluate the diff. Split into a cheap ubuntu `mac-build-smoke-filter` job + the macOS `mac-build-smoke` job gated on `needs.mac-build-smoke-filter.outputs.run`. The macOS runner is now only allocated when the filter says run. 2. `.gitmodules` was missing from the path filter — changes there can break submodule resolution without touching `packages/web`. Added. 3. The fork-skip condition used a bare `pull_request.head.repo.full_name` compare, which on push events (where the PR object is absent) silently landed in the wrong branch. Tightened both stub-handling steps to gate on `github.event_name == 'pull_request'` first, so push events with a stubbed submodule now fail loudly as intended.
| runs-on: ubuntu-latest | ||
| outputs: | ||
| run: ${{ steps.filter.outputs.run }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| # Need the PR base commit so we can diff against it. The default | ||
| # fetch-depth=1 only has HEAD. | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Decide whether to run the Mac smoke build | ||
| id: filter | ||
| env: | ||
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
| run: | | ||
| set -euo pipefail | ||
| if [ -z "${BASE_SHA:-}" ]; then | ||
| # push to main (or any non-PR trigger) — always run. | ||
| echo "run=true" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| changed=$(git diff --name-only "$BASE_SHA"...HEAD) | ||
| if echo "$changed" | grep -E '^(packages/web($|/)|\.github/workflows/mac-release\.yml$|\.github/actions/setup-submodule(/|$)|\.gitmodules$)' >/dev/null; then | ||
| echo "run=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "run=false" >> "$GITHUB_OUTPUT" | ||
| echo "::notice::No paths affecting the Mac build changed — skipping mac-build-smoke." | ||
| fi | ||
|
|
||
| # Cheap end-to-end Xcode build for the Mac app. `mac-release.yml` is | ||
| # workflow_dispatch only, so a rename or path move inside the owletto | ||
| # submodule (Mac source lives at `packages/web/apps/mac/`) can silently | ||
| # break the release pipeline and only surface the next time someone | ||
| # triggers a release. Build-only (no archive, no signing, no notarize) | ||
| # is enough to prove the Xcode project + submodule layout still compose. | ||
| # | ||
| # Runner cost is bounded by the filter job above — the macOS runner is | ||
| # only allocated when the submodule pointer, the release workflow, the | ||
| # submodule setup action, or `.gitmodules` changes. | ||
| mac-build-smoke: |
| needs: mac-build-smoke-filter | ||
| if: needs.mac-build-smoke-filter.outputs.run == 'true' | ||
| runs-on: macos-15 | ||
| timeout-minutes: 15 | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - id: submodule | ||
| uses: ./.github/actions/setup-submodule | ||
| with: | ||
| deploy-key: ${{ secrets.OWLETTO_WEB_DEPLOY_KEY }} | ||
|
|
||
| # Forks (and any run where the secret isn't available) get a stub | ||
| # package.json from setup-submodule. There's no Mac source to build | ||
| # there, so skip the rest gracefully rather than fail. Same-repo PRs | ||
| # and push events both have the key, so a stub there means the secret | ||
| # is misconfigured — fail loudly. | ||
| - name: Fail fast if submodule is stubbed (same-repo PRs and push) | ||
| if: steps.submodule.outputs.stubbed == 'true' && !(github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) | ||
| run: | | ||
| echo "::error::Mac source unreachable — OWLETTO_WEB_DEPLOY_KEY missing or invalid." | ||
| exit 1 | ||
|
|
||
| - name: Skip on fork PRs without submodule access | ||
| if: steps.submodule.outputs.stubbed == 'true' && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository | ||
| run: echo "::notice::Submodule stubbed on fork PR — skipping Mac smoke build." | ||
|
|
||
| - name: Build (unsigned, no archive) | ||
| if: steps.submodule.outputs.stubbed != 'true' | ||
| run: | | ||
| xcodebuild \ | ||
| -project packages/web/apps/mac/Lobu.xcodeproj \ | ||
| -scheme Lobu -configuration Release build \ | ||
| CODE_SIGNING_ALLOWED=NO |
Summary
Adds a
mac-build-smokejob to.github/workflows/ci.ymlthat runsxcodebuild build(unsigned, no archive) against the pinnedpackages/websubmodule SHA on PRs that could affect the Mac build.Closes #793.
What triggers it
The job only runs when the diff touches a path that can break the Mac release pipeline:
packages/web— the submodule pointer (Mac source lives atpackages/web/apps/mac/).github/workflows/mac-release.yml— the release workflow whose contract this smokes.github/actions/setup-submodule— the composite action both workflows depend onEverything else short-circuits with a notice and uses zero macOS-runner minutes.
What it does
setup-submodulecomposite action.OWLETTO_WEB_DEPLOY_KEYis missing/invalid — same posture asmac-release.yml's release contract).xcodebuild ... -scheme Lobu -configuration Release build CODE_SIGNING_ALLOWED=NO. No archive, no signing, no notarization, no DMG.Note on the path filter
The issue's snippet uses
contains(github.event.pull_request.changed_files || '', ...). That expression doesn't work —changed_filesin the PR webhook payload is an integer (the count of changed files), not a list of paths, socontains()over it never matches and the job would run on every PR.This implementation computes the changed-file list via
git diff "$BASE_SHA"...HEADin a setup step and gates the rest of the job on its output. Push events (no PR base SHA) default to running the job.Runner cost
macos-15is the most expensive runner tier, but the filter keeps it to PRs that actually touch the relevant paths — likely a handful per month, in line with how often the submodule pointer bumps.Test plan
.github/workflows/ci.yml, not the three trigger paths) — should see themac-build-smokejob exist but log "No paths affecting the Mac build changed — skipping" and pass with noxcodebuildinvocation.packages/websubmodule pointer, confirm the smoke build actually runs and passes against the current owletto SHA.OWLETTO_WEB_DEPLOY_KEY) hits the "submodule stubbed on fork PR" notice rather than failing.Summary by CodeRabbit