fix: add release smoke tests to prevent broken binary publishes#463
Merged
anandgupta42 merged 2 commits intomainfrom Mar 25, 2026
Merged
fix: add release smoke tests to prevent broken binary publishes#463anandgupta42 merged 2 commits intomainfrom
anandgupta42 merged 2 commits intomainfrom
Conversation
Closes #462 v0.5.10 shipped with `@altimateai/altimate-core` missing from standalone distributions, crashing on startup. Add three-layer defense: - **CI smoke test**: run the compiled `linux-x64` binary after build and before npm publish — catches runtime crashes that compile fine - **Build-time verification**: validate all `requiredExternals` are in `package.json` `dependencies` (not just `devDependencies`) so they ship in the npm wrapper package - **Local pre-release script**: `bun run pre-release` builds + smoke-tests the binary before tagging — mandatory step in RELEASING.md Also adds `smoke-test-binary.test.ts` with 3 tests: version check, standalone graceful-failure, and `--help` output. Reviewed by 5 AI models (Claude, GPT 5.2 Codex, Gemini 3.1 Pro, Kimi K2.5, MiniMax M2.5). Key fixes from review: - Include workspace `node_modules` in `NODE_PATH` (Gemini) - Restrict dep check to `dependencies` only (GPT, Gemini, Kimi) - Hard-fail pre-publish gate when binary not found (Claude, GPT) - Tighten exit code assertion for signal safety (MiniMax) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds multi-layer release validation: build-time verification of required externals, a pre-release sanity-check script, CI binary smoke tests, an npm Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant Pre as Pre-release Check
participant Build as Local Build
participant CI as CI Workflow
participant Bin as Compiled Binary
participant Pub as NPM Publish
Dev->>Pre: bun run pre-release
Pre->>Pre: verify required externals in dependencies
Pre->>Pre: require.resolve checks
Pre->>Build: bun run build:local (MODELS_DEV_API_JSON=fixture)
Build->>Bin: produce platform binaries (dist/)
Pre->>Bin: execute --version (NODE_PATH set)
Bin-->>Pre: exit 0 / version output
alt all checks pass
Dev->>CI: push tag
CI->>Build: build linux-x64
CI->>Bin: execute --version (NODE_PATH set)
Bin-->>CI: exit 0
CI->>Bin: execute --version (publish-time check)
Bin-->>CI: exit 0
CI->>Pub: proceed to publish
Pub-->>CI: npm package published
else any check fails
CI-->>CI: fail job, block publish
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opencode/script/pre-release-check.ts (1)
41-42: KeeprequiredExternalsin one place.This redefines the list already introduced in
packages/opencode/script/build.ts. If the build list grows and only that file is updated,bun run pre-releasecan still green-light the release becausebuild:localonly warns on missing dependencies outside release mode. Please move the list to a tiny shared module and import it from both scripts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/script/pre-release-check.ts` around lines 41 - 42, The duplicate requiredExternals array should be centralized: create a tiny shared module that exports a named constant requiredExternals (e.g., export const requiredExternals = ["@altimateai/altimate-core"]), then replace the local declaration in pre-release-check.ts and the one in build.ts with imports from that shared module (import { requiredExternals } from "./requiredExternals"), removing the duplicated definition so both scripts use the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 193-202: The pre-publish smoke test's artifact lookup uses find
with pattern "*/linux-x64/*/altimate" which fails because the built binary
resides under
packages/opencode/dist/@altimateai/altimate-code-linux-x64/bin/altimate; update
the BINARY find pattern in the "Pre-publish smoke test" step to match the actual
output layout (e.g., search for paths containing
"altimate-code-linux-x64/bin/altimate" or a wildcard that covers
"*linux-x64*/bin/altimate") so BINARY picks the correct file, then keep the
chmod +x and version check logic as-is (refer to the BINARY assignment and the
subsequent chmod +x "$BINARY" and "$BINARY" --version lines).
In `@packages/opencode/script/pre-release-check.ts`:
- Around line 85-125: searchDist currently only looks for "bin/altimate" and the
smoke-test sets NODE_PATH using ":" which breaks on Windows; update searchDist
(and the equivalent logic in
packages/opencode/test/install/smoke-test-binary.test.ts) to consider both
"bin/altimate" and "bin/altimate.exe" (or use path.basename checks) when probing
dist directories (symbols: function searchDist, variable binaryPath) and replace
the hardcoded ":" with path.delimiter when building NODE_PATH (symbol: NODE_PATH
environment assignment / nodePaths.join(...)); keep behavior otherwise the same
so smokeResult/spawnSync continues to call binaryPath with ["--version"] and
other env vars intact.
---
Nitpick comments:
In `@packages/opencode/script/pre-release-check.ts`:
- Around line 41-42: The duplicate requiredExternals array should be
centralized: create a tiny shared module that exports a named constant
requiredExternals (e.g., export const requiredExternals =
["@altimateai/altimate-core"]), then replace the local declaration in
pre-release-check.ts and the one in build.ts with imports from that shared
module (import { requiredExternals } from "./requiredExternals"), removing the
duplicated definition so both scripts use the single source of truth.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 57482b16-8c9b-4937-ae53-012642c2eb93
📒 Files selected for processing (7)
.github/meta/commit.txt.github/workflows/release.ymldocs/RELEASING.mdpackages/opencode/package.jsonpackages/opencode/script/build.tspackages/opencode/script/pre-release-check.tspackages/opencode/test/install/smoke-test-binary.test.ts
….delimiter`, binary names - Use `*altimate-code-linux-x64/bin/altimate` in pre-publish find pattern for clarity (old pattern worked but was ambiguous) - Use `path.delimiter` instead of hardcoded `:` for NODE_PATH in both `pre-release-check.ts` and `smoke-test-binary.test.ts` - Handle Windows `altimate.exe` binary name in `searchDist()` functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What does this PR do?
Adds three-layer defense to prevent broken binary releases like v0.5.10, where
@altimateai/altimate-corewas marked as external but missing from standalone distributions:linux-x64binary after build AND before npm publishrequiredExternalsare inpackage.jsondependenciesbun run pre-releasebuilds + smoke-tests before taggingType of change
Issue for this PR
Closes #462
How did you verify your code works?
bun run typecheck— cleanbun test test/install/— 123 tests pass (120 existing + 3 new smoke tests)bun run pre-release— all 4 checks pass (deps in package.json, installed, build, smoke test)node_modulesinNODE_PATH(Gemini),dependencies-only check (GPT/Gemini/Kimi), hard-fail pre-publish gate (Claude/GPT), exit code assertion tightening (MiniMax)Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation
Tests