Fix runner local build ABI validation#791
Conversation
📝 WalkthroughWalkthroughAdds two new Bash scripts: ChangesRunner Build Pipeline
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
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 docstrings
🧪 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 `@scripts/build/build-runner-binary.sh`:
- Around line 27-30: The make command is invoked on line 38 but is not validated
in the prerequisite dependency checks (lines 27-30). Add a require_command call
for make in the same pattern as the existing require_command calls for git, go,
tar, and sha256sum to ensure users receive a guided error message if make is not
installed rather than a generic command not found error.
🪄 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: 6a1c8237-115a-4152-a655-d74f70fddb08
📒 Files selected for processing (6)
.github/workflows/build-runner-binary.ymlmake/dist.mkmake/help.mkscripts/build/build-runner-binary.shscripts/build/check-libboxlite-abi.shscripts/deploy/runner-update-binary.sh
| require_command git "Install git" | ||
| require_command go "Install Go" | ||
| require_command tar "Install tar" | ||
| require_command sha256sum "Install coreutils" |
There was a problem hiding this comment.
Add an explicit make prerequisite check.
Line 38 invokes make, but make is not validated in the dependency checks. On minimal hosts this fails with a generic command not found instead of a guided error.
Suggested patch
require_command git "Install git"
+require_command make "Install GNU Make"
require_command go "Install Go"
require_command tar "Install tar"
require_command sha256sum "Install coreutils"Also applies to: 38-38
🤖 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 `@scripts/build/build-runner-binary.sh` around lines 27 - 30, The make command
is invoked on line 38 but is not validated in the prerequisite dependency checks
(lines 27-30). Add a require_command call for make in the same pattern as the
existing require_command calls for git, go, tar, and sha256sum to ensure users
receive a guided error message if make is not installed rather than a generic
command not found error.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbf3cc587d
ℹ️ 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".
| fi | ||
|
|
||
| print_section "Building and validating Go SDK static library" | ||
| make -C "$REPO_ROOT" dist:go |
There was a problem hiding this comment.
Gate dist:runner to amd64 before staging libboxlite
On Linux arm64 or any non-amd64 host, this make dist:go invocation builds and smoke-tests sdks/go/libboxlite.a for the host architecture, but the script later forces the cgo runner build with GOARCH=amd64 (go help environment confirms GOARCH is the target architecture). The ABI probe also runs natively, so it can pass while the subsequent amd64 cgo link consumes an arm64 archive and fails; either reject non-amd64 hosts here or build the C archive for the same target as the runner.
Useful? React with 👍 / 👎.
Problem
Dev box creation failed in the runner before VM startup with errors like
failed to create box: add port 33689:2280 failed with code 402665120. A direct C probe against the existing dev-machine static libraries returned non-BoxLite error codes, while a freshorigin/mainbuild returnednew=0 add_port=0.That points to a stale or bad
sdks/go/libboxlite.abeing linked into locally built runner binaries. The Go workspace was already pointing runner at the local Go SDK; the missing guard was rebuilding and validating the static library before runner builds.Changes
scripts/build/check-libboxlite-abi.sh, a minimal C ABI smoke test forboxlite_options_newplusboxlite_options_add_port.make dist:gorebuildboxlite-c, fix Go symbols, smoke-testtarget/release/libboxlite.a, copy it intosdks/go/libboxlite.a, and clear the Go build cache.make dist:runner/scripts/build/build-runner-binary.shfor a Linux dev-machine runner artifact build that does not deploy or replace any EC2 binary.libboxlite.a.scripts/deploy/runner-update-binary.shstill downloads published release assets; unreleased main builds should be produced withmake dist:runner.Verification
bash -n scripts/build/check-libboxlite-abi.sh && bash -n scripts/build/build-runner-binary.sh.git diff --check.make help | rg "dist:(go|runner)".fbf3cc58in/home/brian/work/boxlite/repos/boxliteand ranOUTPUT_DIR=/home/brian/work/boxlite/tmp/runner-build-fbf3cc58 make dist:runner.target/release/libboxlite.asmoke test returnednew=0 add_port=0.sdks/go/libboxlite.asmoke test returnednew=0 add_port=0./home/brian/work/boxlite/tmp/runner-build-fbf3cc58/boxlite-runner, sha256ab29caef2764b7359af630c537790b26338f38104b694568d15f9a3185e55ab8./home/brian/work/boxlite/tmp/runner-build-fbf3cc58/boxlite-runner-v0.9.5-linux-amd64-fbf3cc58.tar.gz, sha256f51281dbe0aba47096c150b10e62693432eb43565b221cb672cf2aab1b93f93d.Scope
No SST deploy, no SSM command, no EC2 replacement, and no live runner binary replacement was performed.
The dashboard image/template visibility issue is separate. Current code removed the create-box image picker and hides the Images route, while the API allowlist still exists. That should be a follow-up product/API PR rather than part of this runner build-chain fix.
Summary by CodeRabbit