Skip to content

fix: embed altimate-core in standalone binary and rename to altimate (match npm primary)#820

Open
mdesmet wants to merge 9 commits into
mainfrom
fix/curl-install-bundle-altimate-core
Open

fix: embed altimate-core in standalone binary and rename to altimate (match npm primary)#820
mdesmet wants to merge 9 commits into
mainfrom
fix/curl-install-bundle-altimate-core

Conversation

@mdesmet
Copy link
Copy Markdown
Contributor

@mdesmet mdesmet commented May 18, 2026

Convergence round 1 fixes applied

The multi-model code-review pass on this PR surfaced 5 actionable items
(review comment: #820 (comment)).
All five are addressed as separate commits stacked on top of 32a17c015:

Commit Item Summary
18dd4c6f1 C1 install handles altimate.exe on windows — binary_name resolved from $os, install_from_binary preserves caller's basename
4fc156e34 C2 Hard-error on musl + win32-arm64 in install, packages/opencode/bin/altimate, and packages/opencode/script/postinstall.mjs; curl --fail on both download paths
154cb6c96 M1 Hermetic smoke test: cwd: os.tmpdir() in spawnSync; new content-level assertion that the binary embeds exactly one .node (catches a silent onResolve miss); same cd "${RUNNER_TEMP:-/tmp}" shape in release.yml
2564abfef M2 Restore _requiredExports validation in the staged shim — extracted from the upstream loader at build time, hard-throw if the regex stops matching
85dd35164 M3 Replace locatePlatformPackageDir's bun-flat-layout walk with createRequire rooted at the loader

Verification (re-run after all 5 commits)

$ rm -rf packages/opencode/dist
$ bun run packages/opencode/script/build.ts --single --skip-install
building @altimateai/altimate-code-darwin-arm64
Verified 0 required external(s) are in package.json

# Hermetic regression test
$ cd /tmp && env -u NODE_PATH .../bin/altimate --version
0.0.0-fix/curl-install-bundle-altimate-core-202605181342

# Content-level assertion (M1)
$ strings .../bin/altimate | grep -oE 'altimate-core\.(darwin|linux|win32)-[a-z0-9-]+\.node' | sort -u
altimate-core.darwin-arm64-ptxrnv5e.node      # bunfs-embedded entry
altimate-core.darwin-arm64.node               # require() string
# After hash-suffix collapse → 1 distinct .node. No leakage from other platforms.

# C1 .exe plumbing
$ printf '#!/bin/sh\necho fake\n' > /tmp/altimate.exe && chmod +x /tmp/altimate.exe
$ HOME=$SCRATCH bash ./install --binary /tmp/altimate.exe --no-modify-path
$ ls $SCRATCH/.altimate/bin
altimate.exe                                  # ← extension preserved

# C2 musl fail-fast (simulated on darwin host)
$ sed 's|is_musl=false|is_musl=true|' install | bash --version 0.1.0
Alpine Linux (musl) is not currently supported by the standalone install.
altimate-core has no NAPI prebuild for musl yet. Workarounds:
  • apk add gcompat   # run glibc binaries on Alpine
  • Use npm: npm install -g altimate-code
$ echo $?
1

# E2E install via --binary on the real darwin-arm64 binary
$ SCRATCH=$(mktemp -d)
$ HOME=$SCRATCH bash ./install --binary .../bin/altimate --no-modify-path
$ ls $SCRATCH/.altimate/bin
altimate
$ env -u NODE_PATH $SCRATCH/.altimate/bin/altimate --version
0.0.0-fix/curl-install-bundle-altimate-core-202605181342

# Release-critical test suites
$ bun test test/install/ test/branding/
401 pass / 0 fail / 1343 expect() calls
# (was 400 before M1 — the new "binary embeds exactly one .node" test adds 1.)

One review note that needed adjusting in practice

M1's suggested hash-strip regex /-[a-f0-9]+(?=\.node$)/ didn't actually
strip bunfs's suffix. Bun's embedded-resource hash is alphanumeric (not hex)
and the real example darwin-arm64-ptxrnv5e.node contains p/t/x/r/n/v
none of which match [a-f]. Used /-[a-z0-9]{6,}(?=\.node$)/ instead: the
length bound is unambiguous because real platform last-segments
(arm64, x64, gnu, msvc) are all ≤5 chars and Bun's hash is ≥7. The
test was failing 1 vs 2 distinct names before this fix.


Why

The curl install at the repo root produced a binary that crashed on first run with Cannot find module '@altimateai/altimate-core'. Root cause: script/build.ts marked altimate-core as external (NAPI native modules can't live inside Bun's single-file bunfs), and the release archive shipped only the raw Bun binary — no companion node_modules, no NODE_PATH-aware wrapper. The CI smoke tests hid the bug by pre-setting NODE_PATH="$(pwd)/packages/opencode/node_modules:..." against the developer checkout before invoking the binary.

History: commit a15dbfecf originally moved altimate-core to external to avoid 4x binary bloat (bundling brought in 5 platforms' worth of .node files, ~250 MB total). Subsequent commits made altimate-core a runtime npm dep so the npm flow worked. Neither commit accounted for curl-install / Homebrew / AUR.

A second mismatch: the npm package exposes altimate as the primary bin and altimate-code as the alias (packages/opencode/package.json bin block, postinstall banner, README). The curl install was installing only altimate-code. This PR aligns the standalone install with the npm contract.

What

Two halves, in one commit, in this PR.

1. Self-contained binary (build-level fix)

In each per-target build loop:

  1. Resolve @altimateai/altimate-core and locate the target's NAPI prebuild (@altimateai/altimate-core-darwin-arm64 for darwin-arm64, etc.).
  2. Stage a per-target copy of the loader at dist/${name}/.altimate-core-staged/@altimateai/altimate-core/.
  3. Replace the loader's NAPI-RS dispatcher (which references every platform's .node and platform package) with a one-line shim:
    module.exports = require('./altimate-core.<platform>.node')
  4. Co-locate the target's .node file next to the shim.
  5. Add a Bun.build onResolve plugin that redirects @altimateai/altimate-core to the staged shim. Without this, Bun resolves the import via the workspace node_modules and uses the full multi-platform dispatcher.
  6. Drop @altimateai/altimate-core from requiredExternals. Bun statically sees the single require() and embeds that one .node into bunfs.

Result: ~192 MB self-contained binary. strings on it shows only target-platform .node references, no leakage from the other platforms.

Caveat: targets without an altimate-core prebuild

@altimateai/altimate-core has no NAPI prebuilds for linux-musl variants or win32-arm64. Those targets are removed from allTargets in build.ts and from the GitHub Actions release matrix. Re-add if/when altimate-core ships those prebuilds upstream. Dropping was preferred over shipping a broken artifact. Affects Alpine Linux container users and Windows ARM users specifically — should be called out in release notes.

2. Rename curl-install binary altimate-codealtimate

The npm package already exposes altimate as the primary bin. The standalone install was installing only altimate-code, making it the odd one out. Aligned:

  • installAPP=altimate (was altimate-code). That single change cascades to: archive filename altimate-<target>.{zip,tar.gz}, INSTALL_DIR=$HOME/.altimate/bin, mv "$tmp_dir/altimate" "$INSTALL_DIR", every log line / tmp-basename / check_version reference. The final getting-started block matches upstream opencode's shape (cd <project> / altimate). No fallback, no symlink, no alias — users who want altimate-code get it from npm or Homebrew.
  • script/build.ts — archive step renames @altimateai/altimate-code-<target>altimate-<target> and packs exactly one file (the altimate binary), not the sourcemaps and not the altimate-code alias. The altimate-code copy stays inside the platform package dir because the npm wrapper (packages/opencode/bin/altimate) looks for bin/altimate-code to locate the platform binary — npm flow unchanged.
  • script/publish.ts — Homebrew + AUR formula URLs and the sha256sum paths swap to the new altimate-<target> archive names. Brew + AUR still install the binary as altimate AND create an altimate-code alias inside the Cellar / /usr/bin — those package managers keep both names.

Things intentionally NOT changed:

  • GitHub repo URL AltimateAI/altimate-code (the repo isn't renamed)
  • npm package name @altimateai/altimate-code (not renamed)
  • npm wrapper package's bin block (still exposes both altimate and altimate-code)
  • The npm bin wrapper at packages/opencode/bin/altimate and packages/opencode/bin/altimate-code (unchanged — keep working as today)

Files changed

.github/workflows/release.yml                          |  32 ++--
install                                                |  36 ++--
packages/opencode/script/build.ts                      | 196 +++++++++++++++++----
packages/opencode/script/publish.ts                    |  23 +--
packages/opencode/test/branding/branding.test.ts       |  14 +-
packages/opencode/test/install/brew-formula.test.ts    |  24 +--
packages/opencode/test/install/smoke-test-binary.test  |  37 ++--

Verification

All run on darwin-arm64 against this branch:

$ rm -rf packages/opencode/dist
$ OPENCODE_RELEASE=1 OPENCODE_VERSION=v0.0.0-test \
    bun run packages/opencode/script/build.ts --single

$ ls packages/opencode/dist/*.zip
packages/opencode/dist/altimate-darwin-arm64.zip          # archive renamed

$ unzip -l packages/opencode/dist/altimate-darwin-arm64.zip
 192306704  altimate                                       # single file, no altimate-code

# Regression: standalone binary works from /tmp with NODE_PATH unset
$ cd /tmp && env -u NODE_PATH .../bin/altimate --version
0.0.0-test

# The original failing command — altimate . — launches the TUI
$ mkdir -p /tmp/smoke && cd /tmp/smoke
$ env -u NODE_PATH timeout 4 .../bin/altimate .
# TUI launches (terminal control sequences observed)

# install vs upstream opencode — 159 total diff lines (all branding-related)
$ diff -u <(curl -fsSL https://raw.githubusercontent.com/anomalyco/opencode/dev/install) install | wc -l
159

# install vs upstream — non-branding lines only (after filtering brand words)
$ diff -u /tmp/upstream-install install \
    | grep -E '^[-+]' | grep -vE 'opencode|altimate|anomalyco|AltimateAI|OpenCode|Altimate'
--- /tmp/upstream-install   2026-05-18 19:25:59
-echo -e "${MUTED}                    ${NC}             ▄     "
-echo -e "${MUTED}█▀▀█ █▀▀█ █▀▀█ █▀▀▄ ${NC}█▀▀▀ █▀▀█ █▀▀█ █▀▀█"
-echo -e "${MUTED}█░░█ █░░█ █▀▀▀ █░░█ ${NC}█░░░ █░░█ █░░█ █▀▀▀"
-echo -e "${MUTED}▀▀▀▀ █▀▀▀ ▀▀▀▀ ▀  ▀ ${NC}▀▀▀▀ ▀▀▀▀ ▀▀▀▀ ▀▀▀▀"
+echo -e "${MUTED}To start:${NC}"
# → file marker, ASCII banner (4 lines, dropped), cosmetic getting-started line.
# No structural divergence.

# E2E install via --binary
$ SCRATCH=$(mktemp -d)
$ HOME=$SCRATCH bash ./install --binary .../bin/altimate --no-modify-path
$ ls $SCRATCH/.altimate/bin
altimate
$ env -u NODE_PATH $SCRATCH/.altimate/bin/altimate --version
0.0.0-test

Release-critical test suites: 400 pass / 0 fail across test/branding/ + test/install/. Typecheck clean.

Binary contains no other-platform .node refs:

$ strings .../bin/altimate | grep -oE 'altimate-core\.(darwin|linux|win32)-[a-z0-9-]+\.node' | sort -u
altimate-core.darwin-arm64-ptxrnv5e.node      # bunfs-embedded
altimate-core.darwin-arm64.node               # the require() string

Test plan

  • CI: linux-x64 build-time smoke test passes with env -u NODE_PATH.
  • CI: pre-publish smoke test passes with env -u NODE_PATH.
  • After merge: tag a prerelease, run curl -fsSL .../install | bash -s -- --version <prerelease> in a clean container. Expect altimate --version to work; ~/.altimate/bin/altimate is on PATH; no altimate-code symlink (use npm/brew if you need that name).
  • Dogfood npm: npm i -g @altimateai/altimate-code@<prerelease> and verify both altimate --version and altimate-code --version work (npm still exposes both).
  • Dogfood brew: brew install AltimateAI/tap/altimate-code after the prerelease formula is regenerated. Verify both altimate and altimate-code work.
  • Document in release notes: linux-musl + win32-arm64 standalone builds removed; standalone binary renamed altimate-codealtimate; install dir moved from ~/.altimate-code/bin to ~/.altimate/bin. Existing curl-install users will need to re-run install (the old binary in ~/.altimate-code/bin continues to work but won't be upgraded).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Releases and installer now provide a standalone "altimate" binary with a single per-platform native module embedded.
  • Bug Fixes

    • Improved runtime stability: smoke tests now clear NODE_PATH and verify the binary starts without missing-module errors.
  • Chores

    • Updated artifact naming, packaging, installer behaviour (including Alpine/musl and Windows‑ARM64 fail‑fast gating), install paths, and CI release workflow.
  • Tests

    • Updated branding/install templates and hermetic smoke tests (content-level guard for a single embedded native module).

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR embeds a single target .node into each Bun binary, restricts builds to platforms with available prebuilds, renames artifacts from altimate-code-* to altimate-*, adds musl/Win-ARM gating in installer and runtime wrapper, updates publish/formula names, and changes smoke tests to run with NODE_PATH cleared and add a binary-content guard.

Changes

Standalone NAPI Embedding and Binary Release

Layer / File(s) Summary
Build target constraints and NAPI dependency preparation
packages/opencode/script/build.ts
Document excluded targets; prune allTargets; add createRequire import for resolution anchoring.
Locate per-target prebuilds
packages/opencode/script/build.ts
Install/resolve @altimateai/altimate-core prebuilds per OS/arch and map each build target to the expected platform package and .node filename.
Per-target NAPI staging and Bun resolution
packages/opencode/script/build.ts
Stage a minimal @altimateai/altimate-core (package.json, index.d.ts, generated index.js shim) containing the single target .node; register Bun onResolve to redirect @altimateai/altimate-core to the staged shim; clean up staging and create altimate-code compatibility copy.
Release archive generation and SHA mapping
packages/opencode/script/build.ts, packages/opencode/script/publish.ts
Archives now contain only the altimate binary (altimate.exe on Windows) and are named altimate-<target>; SHA256 computations and archive creation updated to new filenames.
Publish templates & package manager URLs
packages/opencode/script/publish.ts, packages/opencode/test/install/brew-formula.test.ts
AUR PKGBUILD and Homebrew formula templates and related tests updated to use altimate-darwin-* and altimate-linux-* artifact filenames (drop altimate-code- prefix).
Installer rename, download, chmod, and musl guard
install, packages/opencode/script/postinstall.mjs
Installer APPaltimate, INSTALL_DIR$HOME/.altimate/bin, update usage/messages/version check/temp names, add curl --fail, preserve installed-binary basename for --binary, chmod installed file, and add Alpine/musl and Windows-ARM64 fail-fast errors in postinstall.
Runtime wrapper platform checks
packages/opencode/bin/altimate
Add isMusl() detection and early exit messages for musl (Alpine) and Windows ARM64; remove musl-specific Linux candidate names.
Smoke tests & binary content guard
.github/workflows/release.yml, packages/opencode/test/install/smoke-test-binary.test.ts
CI and local smoke tests now run the linux-x64 binary with NODE_PATH cleared and assert successful --version without "Cannot find module"; add a non-Windows binary strings check asserting exactly one embedded altimate-core .node reference.
Unit test updates for naming and behavior
packages/opencode/test/branding/branding.test.ts, packages/opencode/test/install/brew-formula.test.ts
Branding tests expect APP=altimate and .altimate/bin; brew formula and publish URL tests updated to altimate-* filenames.
sequenceDiagram
  participant BuildScript
  participant StageDir
  participant BunBuild
  participant BinaryArchive
  participant Publish
  BuildScript->>StageDir: locate platform .node and write staged shim
  StageDir->>BunBuild: register onResolve -> redirect `@altimateai/altimate-core` to shim
  BunBuild->>BinaryArchive: produce single-binary altimate (embed .node)
  BinaryArchive->>Publish: create altimate-<target> artifact and compute SHA256
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

A rabbit packed a tiny node inside,
Hopped platforms safe, with musl denied,
Archives renamed, the installer sings,
Altimate bounds forth on nimble springs—🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is comprehensive and well-structured but is missing the required 'PINEAPPLE' marker at the top that is mandatory for AI-generated contributions per the template, and the Test Plan section lists items as a checklist rather than describing how testing was actually performed. Add 'PINEAPPLE' at the very top of the description before any other content as required by the template for AI-generated contributions. Additionally, expand the Test Plan section to describe actual testing performed rather than listing future test items.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: embedding altimate-core in the standalone binary and renaming the curl-install target from altimate-code to altimate to match the npm primary binary name.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/curl-install-bundle-altimate-core

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mdesmet mdesmet force-pushed the fix/curl-install-bundle-altimate-core branch from 94b714b to 5947f93 Compare May 18, 2026 11:39
@mdesmet mdesmet changed the title fix: bundle altimate-core in release archive so curl install works fix: embed altimate-core .node into Bun binary so curl install works May 18, 2026
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

…(match npm primary)

The curl install at the repo root produced a binary that crashed on first
run with `Cannot find module '@altimateai/altimate-core'`. Root cause:
script/build.ts marked altimate-core as `external` (NAPI native modules
can't live inside Bun's single-file bunfs), and the release archive
shipped only the raw Bun binary — no companion node_modules, no
NODE_PATH-aware wrapper. The CI smoke tests hid the bug by pre-setting
NODE_PATH against the developer checkout before invoking the binary.

The fix has two halves:

1. Self-contained binary (build-level fix)
   - Drop `@altimateai/altimate-core` from requiredExternals.
   - In the per-target build loop, stage a copy of the loader package and
     replace its NAPI-RS dispatcher with a one-line shim:
       module.exports = require('./altimate-core.<platform>.node')
     Co-locate the target's `.node` file. Add a Bun.build onResolve plugin
     that redirects `@altimateai/altimate-core` to the staged shim. Bun
     statically sees a single require() and embeds that one .node into
     bunfs. Result: ~192 MB self-contained binary, no companion files.
   - Ensure every altimate-core NAPI prebuild is installed before the
     target loop via `bun install --os=* --cpu=*`.
   - Drop targets without an altimate-core prebuild from allTargets:
     linux-*-musl and win32-arm64. release.yml's build matrix updated to
     match. Re-add if/when altimate-core publishes those prebuilds.
   - CI smoke tests now run the binary with NODE_PATH explicitly cleared
     (`env -u NODE_PATH ...`). This turns the smoke test into a real
     regression guard for the curl-install class of bug.

2. Rename the curl-install binary altimate-code → altimate
   The npm package already exposes `altimate` as the primary bin and
   `altimate-code` as the alias (see packages/opencode/package.json bin
   block, and the postinstall banner). The curl install was the odd one
   out, installing only `altimate-code`. Align it with the npm contract:
   the standalone install puts `altimate` on $PATH. Drop the alias from
   the curl-install path — users who want `altimate-code` get it from
   npm or Homebrew.

   - `install`: APP=altimate (was altimate-code). Cascades to archive
     filename `altimate-<target>.{zip,tar.gz}`, INSTALL_DIR
     `~/.altimate/bin`, `mv $tmp_dir/altimate $INSTALL_DIR`, and every
     log line / tmp-basename / check_version reference. Final
     getting-started block matches upstream opencode's shape
     (cd <project> / altimate). Branding-only diff against
     anomalyco/opencode's install is now 6 non-branding lines: the
     file-marker line, 4 ASCII banner lines (removed), and one cosmetic
     getting-started line.
   - `script/build.ts`: archive step renames `@altimateai/altimate-code-<target>`
     → `altimate-<target>` and packs exactly one file (the renamed
     `altimate` binary), not the sourcemaps and not the `altimate-code`
     alias. The `altimate-code` copy stays inside the platform package
     dir because the npm wrapper (`packages/opencode/bin/altimate`)
     looks for `bin/altimate-code` to locate the platform binary —
     that flow is unchanged.
   - `script/publish.ts`: Homebrew + AUR formula URLs and the sha256sum
     paths swap to the new `altimate-<target>` archive names. The brew
     formula still installs the binary as `altimate` and creates the
     `altimate-code` alias inside the Cellar — Homebrew users keep both
     names.
   - Tests (`branding.test.ts`, `brew-formula.test.ts`,
     `smoke-test-binary.test.ts`) updated to match the new binary name
     and archive prefix. Things that intentionally did NOT change:
     GitHub repo URL `AltimateAI/altimate-code`, npm package name
     `altimate-code`, the npm wrapper package's `bin` block.

Verification on darwin-arm64:

  $ ls packages/opencode/dist/*.zip
  altimate-darwin-arm64.zip                 # was altimate-code-darwin-arm64.zip

  $ unzip -l packages/opencode/dist/altimate-darwin-arm64.zip
   192306704  altimate                       # single file, no altimate-code

  # Regression test: NODE_PATH unset, from /tmp
  $ cd /tmp && env -u NODE_PATH .../bin/altimate --version
  0.0.0-test

  # E2E install via --binary
  $ HOME=$SCRATCH bash ./install --binary .../bin/altimate
  $ ls $SCRATCH/.altimate/bin
  altimate
  $ env -u NODE_PATH $SCRATCH/.altimate/bin/altimate --version
  0.0.0-test

  # Install diff vs upstream opencode — non-branding lines
  $ diff -u <(curl -s https://raw.githubusercontent.com/anomalyco/opencode/dev/install) install \
      | grep -E '^[-+]' | grep -vE 'opencode|altimate|anomalyco|AltimateAI|OpenCode|Altimate'
  --- /tmp/upstream-install ...
  -echo -e "${MUTED}                    ${NC}             ▄     "
  -echo -e "${MUTED}█▀▀█ █▀▀█ █▀▀█ █▀▀▄ ${NC}█▀▀▀ █▀▀█ █▀▀█ █▀▀█"
  -echo -e "${MUTED}█░░█ █░░█ █▀▀▀ █░░█ ${NC}█░░░ █░░█ █░░█ █▀▀▀"
  -echo -e "${MUTED}▀▀▀▀ █▀▀▀ ▀▀▀▀ ▀  ▀ ${NC}▀▀▀▀ ▀▀▀▀ ▀▀▀▀ ▀▀▀▀"
  +echo -e "${MUTED}To start:${NC}"
  # → file marker + ASCII banner removal + cosmetic "To start:" line.

  $ bun test --timeout 60000 test/branding/ test/install/
  400 pass / 0 fail

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mdesmet mdesmet force-pushed the fix/curl-install-bundle-altimate-core branch from 5947f93 to 0e83773 Compare May 18, 2026 12:09
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@mdesmet mdesmet changed the title fix: embed altimate-core .node into Bun binary so curl install works fix: embed altimate-core in standalone binary and rename to altimate (match npm primary) May 18, 2026
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@mdesmet
Copy link
Copy Markdown
Contributor Author

mdesmet commented May 18, 2026

Local verification on darwin-arm64

Ran from a fresh clone of 0e837734d in the agent's worktree.

1. Build

$ bun run packages/opencode/script/build.ts --single
...
building @altimateai/altimate-code-darwin-arm64
Verified 0 required external(s) are in package.json
$ du -sh packages/opencode/dist/@altimateai/altimate-code-darwin-arm64/bin/altimate
183M
$ ls packages/opencode/dist/@altimateai/altimate-code-darwin-arm64/bin
altimate
altimate-code        # internal copy used by the npm wrapper — not shipped in curl archive
index.js.map
parser.worker.js.map
worker.js.map

The 183 MB size reflects the embedded altimate-core.darwin-arm64.node plus everything else that was already bundled. Trade-off for a self-contained standalone binary.

2. The regression — original failure mode

$ BIN="$(pwd)/packages/opencode/dist/@altimateai/altimate-code-darwin-arm64/bin/altimate"
$ cd /tmp && env -u NODE_PATH "$BIN" --version
0.0.0-fix/curl-install-bundle-altimate-core-202605181212

No Cannot find module '@altimateai/altimate-core' from '/$bunfs/root/src/index.js'. Resolves entirely from the embedded .node.

3. The exact user-reported failing command

$ cd $(mktemp -d) && timeout 3 env -u NODE_PATH "$BIN" .
# (TUI launched — no error output, killed by timeout as expected)

Previously this errored out at startup with the missing-module crash. Now it boots into the TUI.

4. End-to-end install via --binary

$ SCRATCH=$(mktemp -d)
$ HOME="$SCRATCH" bash ./install --binary "$BIN" --no-modify-path
Installing altimate from: .../bin/altimate

To start:

cd <project>  # Open directory
altimate      # Run command

For more information visit https://altimate.ai

$ ls "$SCRATCH/.altimate/bin"
altimate
$ env -u NODE_PATH "$SCRATCH/.altimate/bin/altimate" --version
0.0.0-fix/curl-install-bundle-altimate-core-202605181212

The install dir is ~/.altimate/bin/ (matching the rename to altimate), the binary is named altimate, and it runs cleanly with no NODE_PATH setup. No altimate-code alias is installed by the curl flow — that name continues to exist via npm + Homebrew.

Conclusion

The original curl ... | bashaltimate-code .Cannot find module failure path is closed. The curl install now produces a single altimate binary at ~/.altimate/bin/altimate that works without any companion node_modules or NODE_PATH setup, and matches the upstream opencode install pattern with branding-only differences.

@mdesmet mdesmet requested a review from anandgupta42 May 18, 2026 12:25
@mdesmet mdesmet marked this pull request as ready for review May 18, 2026 12:25
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@mdesmet
Copy link
Copy Markdown
Contributor Author

mdesmet commented May 18, 2026

@claude review

@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/opencode/script/build.ts (1)

249-277: ⚡ Quick win

Use the installed loader version for .bun path resolution.

locatePlatformPackageDir constructs .bun store paths using the dependency spec string from pkg.dependencies. If that spec becomes a range or tag (e.g. ^0.3.1), the lookup will fail even when the package is installed, since Bun stores packages by their resolved installed version, not the spec string.

Proposed fix
-const altimateCoreVersion = pkg.dependencies["`@altimateai/altimate-core`"]
+const altimateCoreVersion = JSON.parse(
+  fs.readFileSync(altimateCoreLoaderPkgJson, "utf8"),
).version as string
🤖 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 `@packages/opencode/script/build.ts` around lines 249 - 277, The code currently
uses the dependency spec string (altimateCoreVersion from pkg.dependencies) when
composing Bun store paths, which fails for ranges/tags; instead read the actual
installed version from the on-disk loader package and use that resolved version
when building candidates. In locatePlatformPackageDir, derive the installed
version by reading/parsing the loader's package.json (located via
altimateCoreLoaderDir, e.g. fs.readFileSync(path.join(altimateCoreLoaderDir,
"package.json")) and JSON.parse(...).version) and replace uses of
altimateCoreVersion with that installedVersion when forming
`${flatName}@${installedVersion}` so the lookup matches Bun's store naming.
🤖 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.

Nitpick comments:
In `@packages/opencode/script/build.ts`:
- Around line 249-277: The code currently uses the dependency spec string
(altimateCoreVersion from pkg.dependencies) when composing Bun store paths,
which fails for ranges/tags; instead read the actual installed version from the
on-disk loader package and use that resolved version when building candidates.
In locatePlatformPackageDir, derive the installed version by reading/parsing the
loader's package.json (located via altimateCoreLoaderDir, e.g.
fs.readFileSync(path.join(altimateCoreLoaderDir, "package.json")) and
JSON.parse(...).version) and replace uses of altimateCoreVersion with that
installedVersion when forming `${flatName}@${installedVersion}` so the lookup
matches Bun's store naming.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 21bd2d09-97fc-4877-a307-ffd25776fb17

📥 Commits

Reviewing files that changed from the base of the PR and between c859b57 and 0e83773.

📒 Files selected for processing (7)
  • .github/workflows/release.yml
  • install
  • packages/opencode/script/build.ts
  • packages/opencode/script/publish.ts
  • packages/opencode/test/branding/branding.test.ts
  • packages/opencode/test/install/brew-formula.test.ts
  • packages/opencode/test/install/smoke-test-binary.test.ts

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 7 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/opencode/script/build.ts Outdated
The `name` builder at build.ts:283 substitutes `win32 → windows` when
constructing the per-target directory name, so the keys in the
`binaries` map (e.g. `@altimateai/altimate-code-windows-x64`) never
contain the literal substring `win32`. The archive step at line 461
was using `key.includes("win32")` to pick `altimate.exe` vs
`altimate`, which evaluated false on every Windows target and caused
the zip step to look for a non-existent `altimate` file on Windows.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, reopen this pull request to trigger a review.

@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

1 similar comment
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/opencode/script/build.ts (1)

92-183: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when target selection resolves to an unsupported standalone build.

After removing musl and win32-arm64 from allTargets, this path can now either pick the wrong Linux artifact or silently build nothing. --single still matches the glibc linux-* entry on Alpine because it only checks process.platform/process.arch, and a stale --target-index (or --single on win32-arm64) collapses to [] and exits 0 after producing no binary. Please reject unsupported host ABIs/arches and targets.length === 0 before entering the build loop.

🤖 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 `@packages/opencode/script/build.ts` around lines 92 - 183, The build selection
can silently produce an empty targets list or pick an incompatible host artifact
(e.g., glibc vs musl) — update the logic around target resolution (look at
allTargets, targetsFlag, targetIndexFlag, targets, singleFlag, baselineFlag) to
validate and fail fast: (1) when singleFlag is used, explicitly verify the host
ABI/arch/OS combination is present in allTargets (reject unsupported host
ABIs/arches such as musl or removed win32-arm64) and exit non‑zero with an error
if not; (2) after computing targets, check if targets.length === 0 and emit a
clear error + non‑zero exit instead of continuing; perform these checks
immediately after targets is computed and before entering the build loop.
🤖 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.

Outside diff comments:
In `@packages/opencode/script/build.ts`:
- Around line 92-183: The build selection can silently produce an empty targets
list or pick an incompatible host artifact (e.g., glibc vs musl) — update the
logic around target resolution (look at allTargets, targetsFlag,
targetIndexFlag, targets, singleFlag, baselineFlag) to validate and fail fast:
(1) when singleFlag is used, explicitly verify the host ABI/arch/OS combination
is present in allTargets (reject unsupported host ABIs/arches such as musl or
removed win32-arm64) and exit non‑zero with an error if not; (2) after computing
targets, check if targets.length === 0 and emit a clear error + non‑zero exit
instead of continuing; perform these checks immediately after targets is
computed and before entering the build loop.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ad407437-8d1f-4b60-84ae-e655fd065818

📥 Commits

Reviewing files that changed from the base of the PR and between 0e83773 and 32a17c0.

📒 Files selected for processing (1)
  • packages/opencode/script/build.ts

@mdesmet
Copy link
Copy Markdown
Contributor Author

mdesmet commented May 18, 2026

Multi-Model Code Review — PR #820

Verdict: REQUEST CHANGES
Critical Issues: 2
Major Issues: 3
Minor Issues: 4
NITs: 3

Critical Issues

C1. Install script doesn't handle the Windows .exe suffix

Category: Bug
Location: install:343–344, install:350–351

mv "$tmp_dir/altimate" "$INSTALL_DIR"
chmod 755 "${INSTALL_DIR}/altimate"

On Windows (Git-Bash / MSYS2 / Cygwin — detected at install:81–85), the archive is altimate-windows-x64.zip containing altimate.exe (correctly produced by build.ts:464 after the win32→windows fix). The install script hard-codes the binary name without .exe, so mv "$tmp_dir/altimate" fails to find the file and the user ends up with an empty ~/.altimate/bin/. Same issue in install_from_binary at line 350.

Fix:

binary_name="$APP"
[ "$os" = "windows" ] && binary_name="$APP.exe"
mv "$tmp_dir/$binary_name" "${INSTALL_DIR}/$binary_name"
chmod 755 "${INSTALL_DIR}/$binary_name"

Plumb through install_from_binary. Add a fixture test that runs bash install --binary fixtures/fake-altimate.exe --no-modify-path and asserts ~/.altimate/bin/altimate.exe exists.

C2. Musl curl-install path produces a cryptic 404 → tar error

Category: Bug / UX
Location: install:117–128, 159–166; packages/opencode/bin/altimate:156–199; packages/opencode/script/postinstall.mjs:50

Two distinct sub-issues, both stemming from the dropped linux-*-musl and win32-arm64 targets:

(a) Install script — musl path is the cryptic-failure path. musl detection passes the combo check (which only filters by OS+arch), then target="$target-musl" is appended at line 165, producing a download URL for altimate-linux-x64-musl.tar.gz that no longer exists. curl (without --fail) writes GitHub's 404 HTML response to disk; tar -xzf then errors with "not in gzip format" — confusing for Alpine users.
(b) Install script — win32-arm64 already fails gracefully. The combo case at install:103–110 lists only the published combos, so windows/arm64 hits the catch-all and exits with Unsupported OS/Arch: windows/arm64. Clear, not a regression. Mentioned only for completeness; this part needs no fix beyond an optional message tweak to point to npm.
(c) npm wrapper still references dropped musl/win32-arm64 packages. The wrapper's names array at packages/opencode/bin/altimate:179 enumerates musl/baseline-musl variants for fallback. Those packages were never published with the new build matrix; findBinary() walks all of them and ends with "It seems your package manager failed to install the right version" — wrong diagnosis. postinstall.mjs:50 likewise doesn't validate that the resolved platform package matches the published set.

Fix: In install, fail early before URL construction:

if [ "$is_musl" = "true" ]; then
    print_message error "Alpine Linux (musl) is not currently supported by the standalone install."
    print_message info  "altimate-core has no NAPI prebuild for musl yet. Workarounds:"
    print_message info  "  • apk add gcompat   # run glibc binaries on Alpine"
    print_message info  "  • Use the npm install path: npm install -g altimate-code"
    exit 1
fi

Add --fail to both curl invocations as defense in depth. In the npm wrapper, drop musl entries from names (or hard-error in findBinary() when musl=true) so the user sees a clean "unsupported" message rather than an inscrutable "failed to install" one. Add a "Removed targets" section to the release-notes block in .github/workflows/release.yml:332.

GPT's correction incorporated: do NOT direct users to npm when npm is itself affected — for now the message is "unsupported until altimate-core ships matching prebuilds." Once the npm wrapper is also cleaned up, the npm path will be a real fallback for non-musl dropped targets only (and currently there are none).

Major Issues

M1. Smoke test isn't hermetic — would have missed the original regression

Category: Testing
Location: packages/opencode/test/install/smoke-test-binary.test.ts:96–121; .github/workflows/release.yml:107–117, 258–266

Diagnosis tightened per Gemini's correction: the NODE_PATH: "" is correctly cleared, so the bug isn't NODE_PATH inheritance. The real non-hermetic mechanism is Bun's runtime filesystem search from the executable's path — if the onResolve plugin in build.ts:348–355 ever silently fails to redirect the import, Bun walks up from the binary's location, finds altimate-core in the worktree's node_modules, and the test passes for the wrong reason.

That's the exact false-positive class that hid the v0.7.0 crash. Even though NODE_PATH is cleared, the filesystem-walk fallback remains.

Fix: Run the smoke test from a directory with no node_modules anywhere upward, and add a content-level assertion that doesn't rely on runtime fallback at all:

const result = spawnSync(binary!, ["--version"], {
  cwd: os.tmpdir(),                          // hermetic — no parent node_modules
  env: { PATH, HOME, OPENCODE_DISABLE_TELEMETRY: "1", NODE_PATH: "" },
  encoding: "utf-8",
  timeout: 15_000,
})

// Independent of runtime resolution: assert exactly one .node embedded
const strings = execFileSync("strings", [binary!], { encoding: "utf-8" })
const refs = [...strings.matchAll(/altimate-core\.(darwin|linux|win32)-[a-z0-9-]+\.node/g)]
  .map((m) => m[0])
  .map((r) => r.replace(/-[a-f0-9]+(?=\.node$)/, ""))   // strip Bun's hash suffix
expect(new Set(refs).size).toBe(1)

Apply the same cd $RUNNER_TEMP && env -u NODE_PATH "$BINARY" --version shape to release.yml:117, 266.

M2. Shim drops _requiredExports validation

Category: Design / Code Quality
Location: packages/opencode/script/build.ts:335–339

The generated shim is one line:

module.exports = require('./altimate-core.<platform>.node')

The original NAPI-RS loader appends a runtime check (_requiredExports.filter(n => typeof nativeBinding[n] !== 'function')) that catches .node artifacts missing expected functions. The shim drops this.

Disagreement resolved: 5 of 6 models flagged this CRITICAL; Gemini argued it's structurally impossible to hit because the shim and the .node are pulled from the same bun install --os="*" --cpu="*" in the same build run. Gemini is right that strict version drift is impossible by design — that kills most of the validation's value. What remains is defense against upstream publish errors (a .node artifact missing exports the JS side expects, e.g., a botched altimate-core release). Net: real loss of defense, but not a runtime blocker today → MAJOR.

Fix: Extract _requiredExports from the real loader at build time and include it in the shim:

const realLoader = await Bun.file(path.join(altimateCoreLoaderDir, "index.js")).text()
const m = realLoader.match(/const _requiredExports = (\[[^\]]+\])/)
const requiredExports = m?.[1] ?? "[]"

fs.writeFileSync(stagedIndexJs,
  `const nativeBinding = require('./${platform.nodeFile}')\n` +
  `const _requiredExports = ${requiredExports}\n` +
  `const _missing = _requiredExports.filter(n => typeof nativeBinding[n] !== 'function')\n` +
  `if (_missing.length > 0) throw new Error(` +
  `  '@altimateai/altimate-core: embedded binary missing ' + _missing.length + ' export(s): ' + _missing.slice(0, 5).join(', '))\n` +
  `module.exports = nativeBinding\n`
)

Note (Kimi): the upstream loader's pattern is module.exports = nativeBinding followed by redundant module.exports.X = nativeBinding.X self-assignments. Since module.exports === nativeBinding after the first line, the named-export concerns raised in some Phase 1 reviews are not real — properties on nativeBinding are already accessible through module.exports. The validation is the only real defensive piece lost.

M3. locatePlatformPackageDir couples to Bun's flat-layout + literal version string

Category: Code Quality / Logic Error
Location: packages/opencode/script/build.ts:249, 256–268

Two related fragilities:

  1. Manually walks for .bun's flat layout (@scope+name@version). Future Bun cache changes or other package managers will fail.
  2. altimateCoreVersion = pkg.dependencies["@altimateai/altimate-core"] is interpolated literally. Today pinned "0.3.1". The day someone bumps to "^0.3.2", the lookup builds @altimateai+altimate-core-darwin-arm64@^0.3.2 and fails with "Could not locate ..." — no hint at the cause.

Fix (Gemini's suggestion):

import { createRequire } from "node:module"

function locatePlatformPackageDir(pkgName: string): string {
  const req = createRequire(path.join(altimateCoreLoaderDir, "index.js"))
  const pkgJsonPath = req.resolve(`${pkgName}/package.json`)
  return fs.realpathSync(path.dirname(pkgJsonPath))
}

Drop altimateCoreVersion from the lookup entirely.

Verify before relying on this: createRequire from the loader's path walks node_modules from there upward. In a bun-hoisted layout (this project's layout) the platform packages are at the workspace root's node_modules/@altimateai/altimate-core-<platform>/, which is reachable via parent walking from inside node_modules/.bun/@altimateai+altimate-core@0.3.1/node_modules/@altimateai/altimate-core/. Confirm with a quick import.meta.resolve log before committing the change.

Minor Issues

m1. CI smoke test only covers linux-x64

Location: .github/workflows/release.yml:108if: matrix.name == 'linux-x64'

darwin-arm64, darwin-x64, win32-x64, and their baseline variants are never smoke-tested. macOS-arm64 is the developer/customer platform; win32-x64 is the brand-new target. The Windows smoke would have caught C1 before this review.

Fix: Add a macOS runner job + Windows runner job that each run the standalone smoke. Minimum: document the gap as a known limitation.

m2. Smoke test only exercises --version — never calls into altimate-core

Category: Testing
Location: packages/opencode/test/install/smoke-test-binary.test.ts; .github/workflows/release.yml

Downgraded from MAJOR to MINOR per GPT + MiniMax convergence feedback. --version is the standard smoke bar for a compiled binary; calling into core.lint / core.schemaFingerprint is integration testing requiring fixtures, network state, and SDK initialization. Still worth doing as a separate test enhancement — would catch a stale-but-loadable .node (which M2 also guards against in a different way).

Fix (follow-up ticket okay): hidden altimate --smoke-internal flag that loads core.lint + core.schemaFingerprint on tiny in-memory fixtures and exits 0/1. Wire into both local smoke and CI smoke.

m3. Dead requiredExternals verification block

Category: Code Quality
Location: packages/opencode/script/build.ts:422–451, with requiredExternals = [] at line 195

The whole block now iterates an empty array and always logs Verified 0 required external(s) are in package.json. The original purpose — catching "marked external but missing from package.json" — is obsolete in the embedded design.

Fix: Delete it, or repurpose to assert post-staging invariants (shim file written, .node copied) in the per-target loop before Bun.build():

if (!fs.existsSync(stagedShimAbs) || !fs.existsSync(path.join(stagedAltimateCoreDir, platform.nodeFile))) {
  throw new Error(`Staging incomplete for ${name}`)
}

m4. Staging cleanup not in try/finally

Category: Code Quality
Location: packages/opencode/script/build.ts:391–392

If Bun.build() throws, dist/${name}/.altimate-core-staged lingers until the next top-level rm -rf dist. Low-impact but cleaner:

try {
  await Bun.build({ ... })
} finally {
  await $`rm -rf dist/${name}/.altimate-core-staged`.nothrow()
}

NITs

  • N1: build.ts:317–318 comment says "~80–100 MB"; local build measured 183 MB. Update or investigate.
  • N2: build.ts:459 archiveName regex ^@altimateai/altimate-code- is hard-coded. Anchor on pkg.name for robustness.
  • N3: Document the intentional strict ^@altimateai/altimate-core$ filter at build.ts:351. Codebase grep confirms zero subpath imports exist today; the strict filter is correct, not a bug. (Several Phase 1 reviews flagged this as a real concern — convergence resolved it to NIT-level documentation.)

Rejected Findings

Positive Observations

  • Correct architectural call: per-target staged shim + onResolve plugin is the right surgical fix to the bloat-vs-fragility tension. Embeds the target platform's .node without bloating to 5×.
  • Per-target staging directory (dist/${name}/.altimate-core-staged/) is concurrency-safe for --target-index=N parallel CI matrix.
  • altimateCorePlatformFor throws loud and clear for unsupported combos instead of silently shipping broken artifacts.
  • Install script is a true branding-only rebrand of upstream opencode — diff vs upstream reduces to color escapes / repo URL / banner. Future upstream sync stays cheap.
  • Homebrew + AUR + npm continue to expose both altimate and altimate-code aliases while the curl install collapses to one primary name. Clean channel separation.
  • CI smoke test stopped pre-setting NODE_PATH (release.yml:117, 266) — even with M1's hermeticity gap, this is a clear improvement over main.
  • Follow-up commit 32a17c015 caught the win32 vs windows archive-key mismatch with a clear inline comment. Good documentation of the trap.

Missing Tests

  1. Hermetic standalone smoke (M1).
  2. Functional smoke against altimate-core exports (m2).
  3. Windows .exe install flow fixture (C1).
  4. Musl install-script rejection path (C2).
  5. macOS + Windows CI smoke runs (m1).
  6. locatePlatformPackageDir against alternative layouts (M3).

Overall Assessment

The architecture is sound. The PR closes the user-reported v0.7.x crash and matches upstream opencode's install pattern with branding-only delta. Two issues block merge: C1 (Windows install actually broken end-to-end) and C2 (musl users get an inscrutable tar error). Both are short fixes.

M1–M3 are defense-in-depth concerns that would have stopped this exact bug class from regressing. M2 is the most subtle — Gemini's "build-time pinning makes version drift impossible" argument is right, but the loss of upstream-publish-error defense still argues for the small extension to the shim.

m1–m4 and N1–N3 are cleanup that can land in a follow-up.


Finding Attribution

Issue Origin Type
C1 — Install script doesn't handle Windows .exe suffix Claude (CRIT), GPT (MAJ), Gemini (MIN) Consensus
C2 — Musl install-script cryptic 404→tar failure GPT, Kimi, Qwen, GLM-5, Gemini, Grok, MiniMax, MiMo Consensus (7 of 8 panelists; severity converged on CRITICAL via GLM-5/Qwen/Gemini)
C2 — npm wrapper still references dropped musl/win32-arm64 package names GPT, MiMo Convergence fix (split out of C2 after MiniMax's correction that the npm wrapper does not construct URLs)
M1 — Smoke test isn't hermetic (Bun's filesystem-walk fallback hides regression) Claude Unique; diagnosis tightened in convergence per Gemini's correction
M2 — Shim drops _requiredExports validation Grok, Qwen, GPT, MiniMax, Kimi (CRIT); Gemini (safe) Consensus with disagreement; downgraded CRIT→MAJOR via convergence on Gemini's "build-time pinning kills version-drift class" argument
M3 — locatePlatformPackageDir couples to Bun flat-layout + literal version string Gemini (MAJ), Claude (MIN, version-only subset) Consensus
m1 — CI smoke covers only linux-x64 Kimi, Grok, MiniMax, Qwen Consensus
m2 — Smoke test only calls --version, not core API Grok, MiniMax (Phase 1 MAJ); demoted to MIN via convergence per GPT + MiniMax round-1 objection Convergence fix
m3 — Dead requiredExternals block Claude, MiniMax Consensus
m4 — Staging cleanup not in try/finally Kimi Unique
N1 — "~80–100 MB" comment wrong (actual 183 MB) Claude Unique
N2 — archiveName regex hard-coded prefix Claude Unique
N3 — Document intentional strict onResolve filter Qwen (CRIT), Grok (MAJ), MiniMax (NIT), MiMo (MIN), Kimi-on-reflection (NIT), Gemini-on-reflection (safe) Convergence fix (downgraded to NIT after grep confirmed zero subpath imports)

Reviewed by 9 models: Claude, GPT 5.4 Codex, Gemini 3.1 Pro, Kimi K2.5, Grok 4 (4.20), MiniMax M2.7, GLM-5, Qwen 3.5 Plus, MiMo V2 Pro. Convergence: 1 round (6 APPROVE / 2 CHANGES NEEDED). All round-1 corrections incorporated textually; no round 2 needed.

mdesmet and others added 5 commits May 18, 2026 21:33
C1 from PR #820 review. The install script hard-coded `mv "$tmp_dir/altimate"`
and `chmod ... "$INSTALL_DIR/altimate"` for every platform, but the Windows
release archive contains `altimate.exe`. Result on Windows: the script tries to
move a file that doesn't exist (download path) or installs a file under the
wrong name with no `.exe` extension (--binary path).

Fix:
- Set `binary_name="$APP"` (with `.exe` suffix on windows) right after
  computing the target triple, then route mv/chmod through `$binary_name`
  in `download_and_install`.
- In `install_from_binary` use the caller's basename so the installed file
  matches what was supplied — also handles the `.exe` case naturally and
  doesn't break the existing non-windows callers.

Local verification (darwin-arm64 host, simulating windows path):

  $ binary_name="$APP"; [ "$os" = "windows" ] && binary_name="$APP.exe"
  $ cp /tmp/c1-test/$binary_name "${INSTALL_DIR}/$binary_name"
  $ ls "${INSTALL_DIR}"
  altimate.exe                 # ← extension preserved end-to-end

  $ bash ./install --binary /tmp/c1-test/altimate.exe --no-modify-path
  $ ls $HOME/.altimate/bin
  altimate.exe                 # ← --binary path preserves the .exe basename

  $ bash ./install --binary /tmp/c1-test/altimate --no-modify-path
  $ ls $HOME/.altimate/bin
  altimate                     # ← non-.exe path unchanged

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
C2 from PR #820 review. The previous PR dropped linux-*-musl and win32-arm64
from the build matrix (no @altimateai/altimate-core NAPI prebuild exists), but
left three runtime paths that still tried to fetch / resolve archives for
those platforms and emitted misleading errors:

1. The curl install script's musl detection sets is_musl=true and constructs
   `altimate-linux-x64-musl.tar.gz`. The release doesn't ship that archive,
   so the download 404s. curl without `--fail` writes the GitHub 404 HTML
   to disk; tar -xzf then dies with "not in gzip format". Fail fast with an
   actionable message (`apk add gcompat`, or use npm) before URL construction.

2. The npm wrapper at packages/opencode/bin/altimate has a names[] array
   that fans out to ${base}-musl / ${base}-baseline-musl as fallbacks. Those
   packages are no longer published. When findBinary() exhausts the list
   it prints "your package manager failed to install the right version of
   the altimate-code CLI" — wrong diagnosis. Hard-error on musl and
   win32-arm64 before findBinary() runs, with the same actionable message.

3. The npm postinstall script's findBinary() emits a generic "Could not
   find package @altimateai/altimate-code-<plat>-<arch>" via the require
   catch path on these systems. Add the same explicit musl/win32-arm64
   detection there so npm install fails with a clear message.

Also add `curl --fail` to both download paths (the trace-enabled progress
fetch and the simpler `-#` fallback). Without it, a future 404 on any
unknown target archive silently writes the error page to disk and bombs
later in tar/unzip.

Verified locally (sed-patched is_musl=true to force the path on a darwin
host):

  $ bash /tmp/install-musl-sim --version 0.1.0
  Alpine Linux (musl) is not currently supported by the standalone install.
  altimate-core has no NAPI prebuild for musl yet. Workarounds:
    • apk add gcompat   # run glibc binaries on Alpine
    • Use npm: npm install -g altimate-code
  $ echo $?
  1

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
M1 from PR #820 review. The previous standalone-mode smoke test spawned
the binary with NODE_PATH="" but inherited the worktree as cwd. Bun's
compiled binary, when resolving an unbundled require, walks the
filesystem from process.execPath upward — it doesn't actually need
NODE_PATH if there's a node_modules anywhere in the parent chain. So if
the staged-shim onResolve plugin ever silently fails to redirect
@altimateai/altimate-core, the binary falls back to filesystem
resolution and finds altimate-core in the worktree's node_modules.
Test passes for the wrong reason.

Two fixes:

1. Make the runtime test hermetic. Set cwd=os.tmpdir() in spawnSync so
   the binary cannot walk upward into the worktree node_modules tree.
   Mirror the change in release.yml smoke tests (build-time and
   pre-publish): resolve $BINARY to an absolute path first, then
   `cd "${RUNNER_TEMP:-/tmp}"` before invoking the binary.

2. Add an independent content-level assertion. Run `strings` against
   the binary and count distinct altimate-core.<platform>.node
   references (after collapsing the bunfs hash suffix). If the staged
   shim ever silently fails and Bun pulls in the upstream multi-
   platform NAPI-RS loader, every platform's .node name leaks into
   bunfs and this test catches it independently of any runtime path.

The bunfs hash-suffix regex needed a wider character class than the
review's `[a-f0-9]+`: Bun uses an alphanumeric (not hex) suffix. With
`[a-f0-9]+` the suffix `darwin-arm64-ptxrnv5e.node` doesn't strip
(contains p/t/x/r/n/v) and the test sees 2 distinct names. Use
`[a-z0-9]{6,}` instead — Bun's hash is 7+ chars; real platform
last-segments (arm64/x64/gnu/msvc) are all ≤5 chars so the length
bound is unambiguous.

Local verification on darwin-arm64:
  $ bun test test/install/smoke-test-binary.test.ts
  4 pass / 0 fail (was 3; added "binary embeds exactly one .node")

  $ strings .../bin/altimate | grep -oE 'altimate-core\\.(...).*\\.node' | sort -u
  altimate-core.darwin-arm64-ptxrnv5e.node    # bunfs entry
  altimate-core.darwin-arm64.node             # require() string
  # → collapse to 1 distinct after hash strip. Test passes.

The Windows path in the content-level assertion no-ops because the
`strings` binutil isn't on a stock Windows runner; Linux + macOS CI
(where the build matrix actually runs) is fully covered.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
M2 from PR #820 review. The single-line shim
(`module.exports = require('./altimate-core.<platform>.node')`) dropped
the upstream NAPI-RS loader's `_requiredExports` correctness check. If
the embedded .node ever ships missing an export (truncated artifact,
mismatched napi version, etc.), the binary would crash later with a
confusing "TypeError: X is not a function" instead of a clear startup
error pointing at the broken binding.

Extract the `_requiredExports` literal from the real loader at build
time and inline it into the generated shim. If the upstream loader
format ever changes and the regex stops matching, abort the build
rather than ship a shim with no validation — replacing the review's
suggested `?? "[]"` fallback with a hard `throw`.

Verification: the embedded `strings` of the compiled binary now show
the _requiredExports array (40 method names: analyzeMigration ...
validate) and the throw branch. The binary still starts cleanly:

  $ env -u NODE_PATH .../bin/altimate --version
  0.0.0-fix/curl-install-bundle-altimate-core-202605181339

  $ strings .../bin/altimate | grep -c _requiredExports
  6   # ← validation present (twice — main chunk + worker chunk)

  $ bun test test/install/smoke-test-binary.test.ts
  4 pass / 0 fail

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
M3 from PR #820 review. The old locatePlatformPackageDir walked the
filesystem hand-checking three different prefix shapes for bun's flat
`.bun/<scope>+<name>@<version>/node_modules/<scope>/<name>` layout and
interpolated `altimateCoreVersion` (read from packages/opencode/package.json
dependencies) into the path literally. Both fragile:

- pkg.dependencies values can carry semver prefixes (`^0.3.1`, `~0.3.1`,
  `>=0.3.1`) — the literal interpolation would miss the on-disk dir,
  which is keyed by the exact resolved version.
- bun could change its flat-layout dir name format at any time; we'd
  silently start failing to find the prebuild.

Replace with `createRequire(path.join(loaderDir, "index.js"))` then
`req.resolve(`${pkgName}/package.json`)`. Node's resolver walks parent
node_modules from the require base, which (in this project's bun-hoisted
layout) finds the prebuild via the loader's `.bun/@AltimateAI+altimate-core@.../node_modules/@altimateai/altimate-core-<platform>`
sibling entries without us having to know what the surrounding store
looks like.

Verified all 5 platform packages resolve to the same `.bun` paths the
old function returned:

  darwin-arm64 -> .../node_modules/.bun/@AltimateAI+altimate-core-darwin-arm64@0.3.1/node_modules/@altimateai/altimate-core-darwin-arm64
  darwin-x64   -> .../node_modules/.bun/@AltimateAI+altimate-core-darwin-x64@0.3.1/node_modules/@altimateai/altimate-core-darwin-x64
  linux-arm64  -> .../node_modules/.bun/@AltimateAI+altimate-core-linux-arm64-gnu@0.3.1/node_modules/@altimateai/altimate-core-linux-arm64-gnu
  linux-x64    -> .../node_modules/.bun/@AltimateAI+altimate-core-linux-x64-gnu@0.3.1/node_modules/@altimateai/altimate-core-linux-x64-gnu
  win32-x64    -> .../node_modules/.bun/@AltimateAI+altimate-core-win32-x64-msvc@0.3.1/node_modules/@altimateai/altimate-core-win32-x64-msvc

Build still succeeds, binary still starts hermetically:

  $ rm -rf packages/opencode/dist && \
      bun run packages/opencode/script/build.ts --single --skip-install
  building @altimateai/altimate-code-darwin-arm64
  $ cd /tmp && env -u NODE_PATH .../bin/altimate --version
  0.0.0-fix/curl-install-bundle-altimate-core-202605181341

  $ bun test test/install/ test/branding/
  401 pass / 0 fail

altimateCoreVersion is no longer used and is dropped.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

1 similar comment
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@mdesmet mdesmet marked this pull request as draft May 18, 2026 13:44
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
install (1)

123-126: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

pipefail breaks musl detection—ldd --version returns exit code 1 on musl systems.

Under set -o pipefail (line 2), when ldd --version exits with code 1 on musl systems (which it does by design), the pipeline fails immediately and grep never executes, even though the output contains "musl". This causes is_musl to remain false on musl systems, skipping the early error message (lines 134–139) and leading to confusing downstream failures when the glibc binary fails to extract.

Suggested fix
     if command -v ldd >/dev/null 2>&1; then
-      if ldd --version 2>&1 | grep -qi musl; then
+      ldd_out="$(ldd --version 2>&1 || true)"
+      if printf '%s' "$ldd_out" | grep -qi musl; then
         is_musl=true
       fi
     fi
🤖 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 `@install` around lines 123 - 126, The musl detection pipeline fails under set
-o pipefail because ldd --version exits with code 1 on musl, preventing grep
from running; update the detection around the ldd check so the ldd output is
captured regardless of ldd's exit status (e.g., run ldd --version and pipe its
stdout/stderr into grep using a subshell or capture output into a variable),
then test that output for "musl" and set is_musl=true accordingly; modify the
block that calls ldd --version and grep (the logic that sets is_musl) so it does
not rely on pipefail-sensitive pipelines and always inspects the command output
before deciding.
🧹 Nitpick comments (1)
packages/opencode/test/install/smoke-test-binary.test.ts (1)

97-111: ⚡ Quick win

Use tmpdir() fixture instead of os.tmpdir() for hermetic cwd.

Switch this test to await using tmp = await tmpdir(...) and set cwd: tmp.path so it follows the repo’s temp-dir lifecycle and cleanup pattern.

As per coding guidelines, packages/opencode/test/**/*.test.ts: "Use the tmpdir function from fixture/fixture.ts to create temporary directories for tests with automatic cleanup" and "Always use await using syntax with tmpdir() for automatic cleanup when the variable goes out of scope".

🤖 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 `@packages/opencode/test/install/smoke-test-binary.test.ts` around lines 97 -
111, Replace the use of os.tmpdir() for the test cwd with the repo fixture
tmpdir(): import and call tmpdir() using the recommended pattern (await using
tmp = await tmpdir(...)) at the start of the "binary succeeds with NODE_PATH
cleared (standalone mode)" runTest block, then set spawnSync's cwd to tmp.path
instead of os.tmpdir(); ensure the tmpdir fixture is imported from
fixture/fixture.ts and used with await using so the directory is automatically
cleaned up when the test scope ends.
🤖 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 `@packages/opencode/script/postinstall.mjs`:
- Around line 58-62: The current detection uses execSync which throws on
non-zero exit and only returns stdout, so replace the execSync call in
isMuslPlatform (postinstall.mjs) with child_process.spawnSync('ldd',
['--version'], { encoding: 'utf8' }) and inspect the spawnSync result
(result.stdout and result.stderr) without relying on thrown errors; treat either
stdout or stderr content (concatenate both) and check
toLowerCase().includes('musl') and return true if found, and fall back to false
on missing output or other failures while preserving the existing try/catch
behavior.

---

Outside diff comments:
In `@install`:
- Around line 123-126: The musl detection pipeline fails under set -o pipefail
because ldd --version exits with code 1 on musl, preventing grep from running;
update the detection around the ldd check so the ldd output is captured
regardless of ldd's exit status (e.g., run ldd --version and pipe its
stdout/stderr into grep using a subshell or capture output into a variable),
then test that output for "musl" and set is_musl=true accordingly; modify the
block that calls ldd --version and grep (the logic that sets is_musl) so it does
not rely on pipefail-sensitive pipelines and always inspects the command output
before deciding.

---

Nitpick comments:
In `@packages/opencode/test/install/smoke-test-binary.test.ts`:
- Around line 97-111: Replace the use of os.tmpdir() for the test cwd with the
repo fixture tmpdir(): import and call tmpdir() using the recommended pattern
(await using tmp = await tmpdir(...)) at the start of the "binary succeeds with
NODE_PATH cleared (standalone mode)" runTest block, then set spawnSync's cwd to
tmp.path instead of os.tmpdir(); ensure the tmpdir fixture is imported from
fixture/fixture.ts and used with await using so the directory is automatically
cleaned up when the test scope ends.
🪄 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: 2b3d22e0-4384-4e96-8d57-645837391f64

📥 Commits

Reviewing files that changed from the base of the PR and between 32a17c0 and 85dd351.

📒 Files selected for processing (6)
  • .github/workflows/release.yml
  • install
  • packages/opencode/bin/altimate
  • packages/opencode/script/build.ts
  • packages/opencode/script/postinstall.mjs
  • packages/opencode/test/install/smoke-test-binary.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • .github/workflows/release.yml
  • packages/opencode/script/build.ts

Comment thread packages/opencode/script/postinstall.mjs Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/release.yml">

<violation number="1" location=".github/workflows/release.yml:263">
P1: Pre-publish smoke test still looks for the old `altimate-code-linux-x64` artifact path, which can break release verification after the binary/archive rename to `altimate`.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

run: |
BINARY=$(find packages/opencode/dist -path '*altimate-code-linux-x64/bin/altimate' -type f | head -1)
# Resolve to an absolute path before we cd away from the workspace.
BINARY=$(find "$(pwd)/packages/opencode/dist" -path '*altimate-code-linux-x64/bin/altimate' -type f | head -1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Pre-publish smoke test still looks for the old altimate-code-linux-x64 artifact path, which can break release verification after the binary/archive rename to altimate.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/release.yml, line 263:

<comment>Pre-publish smoke test still looks for the old `altimate-code-linux-x64` artifact path, which can break release verification after the binary/archive rename to `altimate`.</comment>

<file context>
@@ -256,14 +259,17 @@ jobs:
         run: |
-          BINARY=$(find packages/opencode/dist -path '*altimate-code-linux-x64/bin/altimate' -type f | head -1)
+          # Resolve to an absolute path before we cd away from the workspace.
+          BINARY=$(find "$(pwd)/packages/opencode/dist" -path '*altimate-code-linux-x64/bin/altimate' -type f | head -1)
           if [ -z "$BINARY" ]; then
             echo "::error::No linux-x64 binary found in artifacts — cannot verify release"
</file context>
Suggested change
BINARY=$(find "$(pwd)/packages/opencode/dist" -path '*altimate-code-linux-x64/bin/altimate' -type f | head -1)
BINARY=$(find "$(pwd)/packages/opencode/dist" -path '*altimate-linux-x64/bin/altimate' -type f | head -1)

Tip: Review your code locally with the cubic CLI to iterate faster.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bug — false positive. The path glob matches the dist subdirectory layout, not the release archive name. The build script writes binaries to dist/@altimateai/altimate-code-<target>/bin/altimate (per build.ts:280-291, the per-target dir name is pkg.name + os + arch + ... = @altimateai/altimate-code-<target>). That layout is unchanged by this PR — only the archive filename produced at build.ts:459-468 was renamed to altimate-<target>.{zip,tar.gz}. The find pattern *altimate-code-linux-x64/bin/altimate correctly resolves the dist binary; locally verified with ls packages/opencode/dist/@altimateai/altimate-code-darwin-arm64/bin/ → contains altimate.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! I've saved this as a new learning to improve future reviews.

Comment thread packages/opencode/script/postinstall.mjs Outdated
… musl

execSync throws on non-zero exit AND only returns stdout. ldd --version on
musl exits 1 and prints to stderr — so isMuslPlatform() would silently
return false on every non-Alpine musl distro (Void musl, custom builds),
falling through to the generic "Could not find package" error instead of
the actionable apk-add-gcompat message.

packages/opencode/bin/altimate already used spawnSync correctly; this
brings postinstall.mjs in line with it.

Flagged by cubic-dev-ai (P2) and coderabbitai (Major) in PR #820 review.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@mdesmet
Copy link
Copy Markdown
Contributor Author

mdesmet commented May 18, 2026

Review comment disposition

Walked through every review comment on the PR. Status:

Comment Disposition
cubic-dev-ai P1 — build.ts:461 win32 vs windows ✅ already fixed in 32a17c0
coderabbitai Major — postinstall.mjs:62 execSync misses musl ✅ fixed in bfe430f77 (swap to spawnSync matching bin/altimate:164)
cubic-dev-ai P2 — postinstall.mjs:59 spawnSync over execSync ✅ fixed in bfe430f77 (same fix)
cubic-dev-ai P1 — release.yml:263 stale altimate-code-linux-x64 path ❌ not a bug. The path glob matches the dist subdirectory (@altimateai/altimate-code-<target>, unchanged by this PR), not the renamed release archive. Verified locally — find packages/opencode/dist -path '*altimate-code-linux-x64/bin/altimate' would correctly resolve. Replied inline.
dev-punia-altimate — 15 TS test failures (12:53 UTC) ✅ stale — that snapshot predates the 0e83773→bfe430f77 commit chain. Current TypeScript check is now running green on bfe430f77 (run 26038983426).
github-actions[bot] auto-close pings ⚠️ recurring CI noise — appears to be a quality-check bot misconfigured against draft PRs from this branch. Not blocking. Worth investigating separately.

PR now at HEAD bfe430f77 with all reviewer-flagged blockers + valid suggestions applied. M1/M2/M3 defense-in-depth fixes from the multi-model review are also in (see earlier 154cb6c96, 2564abfef, 85dd35164).

@anandgupta42 anandgupta42 marked this pull request as ready for review May 19, 2026 01:10
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 9 files

Re-trigger cubic

@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

1 similar comment
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="install">

<violation number="1" location="install:500">
P2: Only show the shell-reload hint when PATH was actually persisted; otherwise instruct manual PATH export. Current logic can tell users to reload even when no rc file was updated.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread install Outdated
ash|sh) reload_cmd="exec \$SHELL" ;;
*) reload_cmd="exec \$SHELL" ;;
esac
print_message info "${MUTED}Reload your shell to pick up the updated PATH:${NC}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Only show the shell-reload hint when PATH was actually persisted; otherwise instruct manual PATH export. Current logic can tell users to reload even when no rc file was updated.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install, line 500:

<comment>Only show the shell-reload hint when PATH was actually persisted; otherwise instruct manual PATH export. Current logic can tell users to reload even when no rc file was updated.</comment>

<file context>
@@ -465,6 +475,33 @@ if [ -n "${GITHUB_ACTIONS-}" ] && [ "${GITHUB_ACTIONS}" == "true" ]; then
+            ash|sh) reload_cmd="exec \$SHELL" ;;
+            *)      reload_cmd="exec \$SHELL" ;;
+        esac
+        print_message info "${MUTED}Reload your shell to pick up the updated PATH:${NC}"
+        print_message info "  $reload_cmd       ${MUTED}# or open a new terminal${NC}"
+    fi
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orphaned — this comment was posted on install:500 after I'd already force-pushed away the commit (322788bcd) that introduced the reload hint. The current PR HEAD (2898d9f01) has only 478 lines of install; line 500 doesn't exist. The reload-hint code referenced no longer ships on the branch.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads-up—understood, this comment is stale after the force-push and no longer applies.

@anandgupta42
Copy link
Copy Markdown
Contributor

@mdesmet — circling back on the cubic P2 on install:500 after comparing against upstream.

The finding is real: install:486-503 only checks parent_path_has_install_dir != true before printing the reload hint. It does not check whether add_to_path actually wrote to an rc file. Two paths that hit this bug:

  1. Unknown shell branch — upstream install:432-436 falls through to printing a manual export PATH=... warning without writing any rc. Our copy still prints "Reload your shell to pick up the updated PATH: exec $SHELL" right after that warning, which is misleading — there's nothing to reload.
  2. No config file foundinstall:412-414 prints "No config file found … you may need to manually add to PATH" and again no rc is written. Same misleading reload hint follows.

Upstream comparison: sst/opencode (anomaly/dev) install has no reload-hint block at all. The PATH section ends at install:438 and the script jumps straight to the getting-started banner. Verified just now against anomaly/dev.

Recommended fix: delete the block (matches upstream)

Removes lines 478-503 of our install. Single-commit fix, closes the cubic finding, removes one divergence from upstream. The two branches that already print a manual export PATH=... line (--no-modify-path at install:489-491, and upstream-style "unknown shell" / "no config file" fall-throughs) cover the cases where the user genuinely needs to act.

@@ -475,29 +475,3 @@
     print_message info "Added $INSTALL_DIR to \$GITHUB_PATH"
 fi
-
-# Shell-reload hint. The script can modify rc files but cannot mutate the
-# parent shell's environment (UNIX process isolation — `export` in a child
-# never reaches the parent). When the parent's PATH lacks $INSTALL_DIR we
-# print a shell-specific reload command; otherwise altimate is already
-# usable in the current session and no hint is needed.
-#
-# Skip the hint inside GitHub Actions — the $GITHUB_PATH write above is
-# picked up automatically by subsequent steps.
-if [[ "$parent_path_has_install_dir" != "true" ]] \
-   && { [[ -z "${GITHUB_ACTIONS-}" ]] || [[ "${GITHUB_ACTIONS}" != "true" ]]; }; then
-    echo -e ""
-    if [[ "$no_modify_path" == "true" ]]; then
-        print_message info "${MUTED}--no-modify-path was set. Add this to your shell rc, or run it now:${NC}"
-        print_message info "  export PATH=$INSTALL_DIR:\$PATH"
-    else
-        case ${current_shell:-} in
-            fish)   reload_cmd="exec fish" ;;
-            zsh)    reload_cmd="exec zsh" ;;
-            bash)   reload_cmd="exec bash" ;;
-            ash|sh) reload_cmd="exec \$SHELL" ;;
-            *)      reload_cmd="exec \$SHELL" ;;
-        esac
-        print_message info "${MUTED}Reload your shell to pick up the updated PATH:${NC}"
-        print_message info "  $reload_cmd       ${MUTED}# or open a new terminal${NC}"
-    fi
-fi

Alternative: gate on actual rc-file write

If you want to keep the hint, the correct gating is to track whether add_to_path actually appended to a file and only print the reload command in that case:

  • Set path_persisted=false at script start.
  • In add_to_path (or right after each call site at install:418-430), set path_persisted=true on success.
  • Wrap the reload-message print at install:500-501 with if [[ "$path_persisted" == "true" ]].
  • Drop the --no-modify-path branch from the reload-hint block — that path already prints its own manual export line and isn't a reload situation.

Happy to push either variant. The delete-the-block option is simpler and removes a divergence, so I'd lean that way unless there's a specific reason to keep it.

@mdesmet mdesmet force-pushed the fix/curl-install-bundle-altimate-core branch from 322788b to bfe430f Compare May 19, 2026 04:19
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@mdesmet
Copy link
Copy Markdown
Contributor Author

mdesmet commented May 19, 2026

@anandgupta42 I have removed the latest commit.

…leanup

Three issues surfaced by coderabbitai's outside-diff review.

1. install: capture ldd output before grep
   `set -o pipefail` (line 2) made `ldd --version 2>&1 | grep -qi musl` evaluate
   false on every non-Alpine musl distro — ldd exits 1 by design, pipefail
   propagates that, the if-block never fires, and is_musl stays false. The C2
   fail-fast logic at line 134-139 then doesn't trigger, and Void/Adelie/custom
   musl users hit the cryptic 404→tar failure C2 was supposed to fix.
   Capture ldd output into a variable with `|| true`, grep on the captured
   string. Sanity-tested with `false` simulating musl ldd — new form returns
   is_musl=true, old form returns false.

2. build.ts: refuse to silently produce no artifact + reject musl host for --single
   `--target-index=N` for an index that no longer exists after the musl/
   win32-arm64 cull yielded `[allTargets[N]].filter(Boolean) === []`, which
   ran the build loop zero times and exited 0 — looked like success in CI.
   `--single` on Alpine matches process.platform=linux/x64, builds the glibc
   target, and produces a binary the host can't load.
   Add a targets.length === 0 guard with a reason-specific error, and a musl
   host check for --single mode.

3. smoke-test-binary.test.ts: use the repo's tmpdir() fixture
   `cwd: os.tmpdir()` works but doesn't auto-clean. Repo coding guidelines
   require `await using tmp = await tmpdir(...)` for tests. Functional behavior
   unchanged; cleanup is now deterministic when the scope closes.

Tests: 397 pass / 0 fail / 5 skip (the local-binary smoke tests skip when no
build is present). typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@mdesmet
Copy link
Copy Markdown
Contributor Author

mdesmet commented May 19, 2026

Round 2 disposition — coderabbit outside-diff review

The earlier inline reviews are resolved (commit bfe430f77 and replies). coderabbit also posted three out-of-diff items that I missed on first pass; addressed in 2898d9f01:

Item Fix
coderabbit Nitpick — build.ts use installed loader version (instead of pkg spec) Already obsolete after the M3 commit 85dd351locatePlatformPackageDir now uses createRequire and doesn't read the version string at all. Cleaner than the proposed fix.
coderabbit Major — install:123-126 pipefail defeats musl detection Fixed in 2898d9f01. set -o pipefail + ldd --version (which exits 1 on musl) made the pipeline-based if-block always false on non-Alpine musl. Captured ldd output into a variable with || true, then grep. Sanity-tested with a stub ldd.
coderabbit Major — build.ts:92-183 no fail-fast on unsupported host / empty targets Fixed in 2898d9f01. --target-index=N of a removed index silently produced no artifact; --single on musl built the glibc target. Added a targets.length === 0 guard and a musl-host rejection for --single.
coderabbit Nitpick — smoke test should use repo tmpdir() fixture Fixed in 2898d9f01. Switched cwd: os.tmpdir() to await using tmp = await tmpdir() from test/fixture/fixture.ts.
cubic-dev-ai P2 — install:500 reload-hint logic Orphaned. cubic posted this on a commit (322788bcd) that was force-pushed away; install is now 478 lines so line 500 doesn't exist. Replied inline.

PR HEAD: 2898d9f01. Tests 397/0/5skip. Typecheck clean.

@dev-punia-altimate
Copy link
Copy Markdown

❌ Tests — Failures Detected

TypeScript — 15 failure(s)

  • baseline [0.49ms]
  • baseline [0.30ms]
  • baseline [0.07ms]
  • baseline [0.24ms]
  • connection_refused [0.26ms]
  • timeout [0.06ms]
  • permission_denied [0.04ms]
  • parse_error [0.05ms]
  • oom [0.04ms]
  • network_error [0.05ms]
  • auth_failure [0.03ms]
  • rate_limit [0.07ms]
  • internal_error [0.04ms]
  • empty_error [0.05ms]
  • connection_refused [0.06ms]

Next Step

Please address the failing cases above and re-run verification.

cc @mdesmet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants