Repository navigation
ci: reload-build runner input as free-form string - #6360
lawrencecchen wants to merge 2 commits into
Conversation
…ilder [skip ci] reload-cloud.sh / reload-cloud-ios.sh --builder blacksmith dispatch this to build a tagged dev macOS app or unsigned iOS archive on a Blacksmith macOS runner and upload it for local download. workflow_dispatch only, so it never joins the push/PR CI fan-out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
[skip ci] Lets the cloud reload scripts dispatch the build onto any macOS runner label (Blacksmith, our self-hosted cmux-macos-26 fleet, warp, depot) without enumerating choices, so a Blacksmith-vs-self-hosted queue/build timing comparison can target both through the same workflow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new Changesreload-build CI Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 80fe9e5. Configure here.
| name: reload-${{ inputs.tag }}-${{ inputs.platform }} | ||
| path: artifact/ | ||
| retention-days: 3 | ||
| if-no-files-found: error |
There was a problem hiding this comment.
Artifact upload skipped on failure
Medium Severity
When a build step fails, Write timings.json still runs via always(), but Upload artifact uses the default success() condition, so it is skipped whenever any earlier step failed. Timing data from failed runs is never published, which undercuts runner comparisons on failed or partial builds.
Reviewed by Cursor Bugbot for commit 80fe9e5. Configure here.
| "deps_seconds": $(( t1 > t0 ? t1 - t0 : 0 )), | ||
| "build_seconds": $(( t1 > 0 ? now - t1 : 0 )), | ||
| "post_checkout_total_seconds": $(( now - t0 )) | ||
| } |
There was a problem hiding this comment.
Checkout failure skews timing totals
Low Severity
Write timings.json runs under always(), but t0 comes from Start timer, which never runs if checkout fails. An empty t0 is treated as zero in shell arithmetic, so post_checkout_total_seconds becomes roughly the current Unix epoch instead of a meaningful duration.
Reviewed by Cursor Bugbot for commit 80fe9e5. Configure here.
Greptile SummaryThis PR adds a new
Confidence Score: 3/5Merging introduces a working CI workflow, but the macOS build step and the timings step both embed free-form user inputs directly into shell commands rather than through env vars, leaving live command-injection paths in code that will be invoked by the cloud reload scripts. The iOS step already demonstrates the correct pattern, so the fix is straightforward, but the same pattern needs to be applied to two more steps before the workflow is safe to run in production dispatch. The injection surfaces are only reachable by users with write access, so the risk is bounded, but they are present in new code on the critical build path. .github/workflows/reload-build.yml — specifically the "Build tagged macOS app" step (line 107) and the "Write timings.json" step (lines 166–185).
|
| Filename | Overview |
|---|---|
| .github/workflows/reload-build.yml | New workflow_dispatch-only build workflow. Runner input correctly widened to free-form string, but two steps embed free-form inputs directly in shell via GHA expressions rather than env vars, creating command injection vectors in the macOS build step and the timings heredoc/echo. |
Reviews (1): Last reviewed commit: "ci: make reload-build runner input a fre..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - name: Build tagged macOS app | ||
| if: ${{ inputs.platform == 'macos' }} | ||
| id: build_macos | ||
| run: | | ||
| set -euo pipefail | ||
| ./scripts/reload.sh --tag "${{ inputs.tag }}" --swift-frontend-workaround 2>&1 | tee /tmp/reload.log |
There was a problem hiding this comment.
${{ inputs.tag }} is expanded by the GitHub Actions expression engine and spliced into the shell script as a raw string before the shell sees the code. If the tag contains $(…) or backticks, those are executed as command substitution inside the double-quoted argument. The iOS build step already uses the correct pattern (env: BUILD_TAG: ${{ inputs.tag }} + "$BUILD_TAG" in the run script) — this step should do the same.
| - name: Build tagged macOS app | |
| if: ${{ inputs.platform == 'macos' }} | |
| id: build_macos | |
| run: | | |
| set -euo pipefail | |
| ./scripts/reload.sh --tag "${{ inputs.tag }}" --swift-frontend-workaround 2>&1 | tee /tmp/reload.log | |
| - name: Build tagged macOS app | |
| if: ${{ inputs.platform == 'macos' }} | |
| id: build_macos | |
| env: | |
| INPUT_TAG: ${{ inputs.tag }} | |
| run: | | |
| set -euo pipefail | |
| ./scripts/reload.sh --tag "$INPUT_TAG" --swift-frontend-workaround 2>&1 | tee /tmp/reload.log |
| - name: Write timings.json | ||
| if: ${{ always() }} | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p artifact | ||
| now=$(date +%s) | ||
| t0=${{ steps.t0.outputs.epoch }} | ||
| t1=${{ steps.t1.outputs.epoch || 0 }} | ||
| cat > artifact/timings.json <<JSON | ||
| { | ||
| "tag": "${{ inputs.tag }}", | ||
| "platform": "${{ inputs.platform }}", | ||
| "runner": "${{ inputs.runner }}", | ||
| "ref": "${{ inputs.ref }}", | ||
| "deps_seconds": $(( t1 > t0 ? t1 - t0 : 0 )), | ||
| "build_seconds": $(( t1 > 0 ? now - t1 : 0 )), | ||
| "post_checkout_total_seconds": $(( now - t0 )) | ||
| } | ||
| JSON | ||
| { | ||
| echo "### reload-build timings" | ||
| echo "" | ||
| echo "- runner: \`${{ inputs.runner }}\`" | ||
| echo "- platform: \`${{ inputs.platform }}\`" | ||
| echo "- deps: $(( t1 > t0 ? t1 - t0 : 0 ))s" | ||
| echo "- build: $(( t1 > 0 ? now - t1 : 0 ))s" | ||
| echo "- post-checkout total: $(( now - t0 ))s" | ||
| } >> "$GITHUB_STEP_SUMMARY" |
There was a problem hiding this comment.
Write timings.json step — two injection surfaces
The heredoc delimiter <<JSON is unquoted, so the shell processes its body for command substitution. ${{ inputs.tag }}, ${{ inputs.runner }}, and ${{ inputs.ref }} are all pre-expanded by GHA before the shell runs, meaning a tag or runner label containing $(…) or backticks will be executed as commands inside the heredoc body. runner is now a free-form string (the whole point of this PR), making this a concrete path.
The echo "- runner: \${{ inputs.runner }}`"line on the step-summary block has the same issue —runneris double-quote-interpolated intoecho`, so command substitution fires there too.
The recommended fix is to bind all three inputs to step-level env vars (INPUT_TAG, INPUT_RUNNER, INPUT_REF) and reference those variables throughout both the heredoc body and the echo lines. The shell arithmetic $(( … )) expressions are unaffected by that change.
This branch was successfully deployed
1 participant
Follow-up to #6354. Makes the
runnerinput a free-form string so the cloud reload scripts can dispatch the build onto any macOS runner label (Blacksmith, our self-hostedcmux-macos-26fleet, warp, depot) without enumerating choices. Enables a Blacksmith-vs-self-hosted queue/build timing comparison through one workflow. workflow_dispatch only; committed[skip ci].Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
CI-only, manually dispatched workflow with read-only repo permissions; it does not run on push/PR and does not change app runtime or release pipelines.
Overview
Adds a workflow_dispatch-only
reload-buildworkflow so cloud reload scripts can build tagged macOS apps or unsigned iOS archives on a remote Mac runner and download the artifact—without triggering on push/PR.The
runnerinput is a free-form string (defaultblacksmith-6vcpu-macos-26) so dispatchers can target Blacksmith, self-hosted labels, warp, or depot without maintaining a fixed choice list; the label is echoed intimings.jsonand the job summary for queue/build comparisons.Dispatch inputs cover tag, optional ref, platform (
macos/ios), and a nonce inrun-nameso callers can find the run. Concurrency cancels in-flight builds per tag+platform. macOS builds use./scripts/reload.sh; iOS runs an unsignedxcodebuild archivewith dev bundle id/display name from the tag.Reviewed by Cursor Bugbot for commit 80fe9e5. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a dispatch-only
reload-buildGitHub Actions workflow to build a tagged dev macOS app or an unsigned iOS archive and upload the artifact. Therunnerinput is now a free-form string so scripts can target any macOS runner label (e.g.,blacksmith-6vcpu-macos-26,cmux-macos-26) and compare queue/build times in one workflow.timings.jsonand a step summary; uploads artifacts with 3-day retention.Written for commit 80fe9e5. Summary will update on new commits.
Summary by CodeRabbit