fix: add .altimate-code path detection + use actual choco error message#274
Open
dev-punia-altimate wants to merge 1 commit intomainfrom
Open
fix: add .altimate-code path detection + use actual choco error message#274dev-punia-altimate wants to merge 1 commit intomainfrom
dev-punia-altimate wants to merge 1 commit intomainfrom
Conversation
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Author
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.
Once credits are available, comment @claude review on this pull request to trigger a review.
dev-punia-altimate
force-pushed
the
qa-autopilot/fix-installation-issues
branch
from
56d802b to
b08c4aa
Compare
dev-punia-altimate
changed the title
Author
Sentry Bot Findings — ResponseL331 (Scoop URL): Pre-existing on main — this PR doesn't change the scoop URL. The current code correctly points to L320 (Choco empty results): Valid finding but pre-existing on main — this PR doesn't change the choco version check code. Adding an empty results guard is a good improvement but should be a separate fix. This PR's actual diff (2 lines):
|
dev-punia-altimate
force-pushed
the
qa-autopilot/fix-installation-issues
branch
from
b08c4aa to
00705ef
Compare
📝 WalkthroughWalkthroughRebrands uninstall/install flows from Opencode to Altimate Code, adds dual legacy/current shell-marker and bin-path detection for PATH cleanup, replaces several direct fetches with a 10s fetchWithTimeout, and hardens upgrade/latest logic and curl-based installer selection/validation. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (uninstall)
participant PackageMgr as Package Manager
participant Shell as User Shell (rc file)
participant FS as File System
CLI->>PackageMgr: run uninstall (e.g. `@altimateai/altimate-code`, `altimate-code`)
PackageMgr-->>CLI: respond success/failure
CLI->>Shell: read shell config (search `SHELL_MARKERS`)
Shell-->>CLI: return matched file(s)
CLI->>Shell: remove PATH lines referencing `BIN_PATHS` (legacy & current)
CLI->>FS: remove binary directories (`.altimate-code/bin`, `.opencode/bin`) if present
FS-->>CLI: confirm removals
CLI->>CLI: emit post-uninstall message
sequenceDiagram
participant CLI as CLI (upgrade/latest)
participant Fetch as fetchWithTimeout (10s)
participant Remote as Remote Registry/API
participant OS as Operating System
CLI->>Fetch: request release/installer
Fetch->>Remote: HTTP GET
Remote-->>Fetch: response (200/other + body)
Fetch-->>CLI: return response (status + body) or timeout
CLI->>CLI: validate response (status, JSON parse, not-HTML script)
alt valid & non-Windows curl path
CLI->>OS: run curl-based installer
else invalid or Windows
CLI-->>CLI: fallback/error (PowerShell/npm instructions or alternate source)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/opencode/src/cli/cmd/uninstall.ts (2)
218-220:⚠️ Potential issue | 🟡 MinorBinary directory check doesn't account for new install path.
Line 218 only checks for
.opencodein the binary path, but new curl installations use.altimate-code. This means the directory cleanup hint won't be shown for users with the new install location.Proposed fix
const binDir = path.dirname(targets.binary) - if (binDir.includes(".opencode")) { + if (binDir.includes(".opencode") || binDir.includes(".altimate-code")) { prompts.log.info(` rmdir "${binDir}" 2>/dev/null`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/uninstall.ts` around lines 218 - 220, The cleanup hint currently only logs rmdir when binDir.includes(".opencode"); update the condition in uninstall.ts to also detect the new curl install path by checking for ".altimate-code" (e.g., replace the single includes check with a logical OR or a regex that matches either ".opencode" or ".altimate-code") so prompts.log.info(` rmdir "${binDir}" 2>/dev/null`) is shown for both install locations.
269-272:⚠️ Potential issue | 🟡 MinorShell config cleanup doesn't handle
.altimate-code/binpaths.The
getShellConfigFilefunction only searches for".opencode/bin"and"# opencode"markers. Users who installed via the new curl installer (which uses~/.altimate-code/bin) won't have their shell config cleaned up properly during uninstall.Consider extending the detection to include both patterns:
Proposed fix
const content = await Filesystem.readText(file).catch(() => "") - if (content.includes("# opencode") || content.includes(".opencode/bin")) { + if (content.includes("# opencode") || content.includes(".opencode/bin") || + content.includes("# altimate") || content.includes(".altimate-code/bin")) { return file }Similar updates would be needed in
cleanShellConfig(lines 287-304) to handle both# altimatecomment markers and.altimate-code/binpaths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/uninstall.ts` around lines 269 - 272, Update getShellConfigFile and cleanShellConfig to detect and remove entries for the new installer by checking for both ".altimate-code/bin" and the altimate comment marker in addition to the existing ".opencode/bin" and "# opencode" checks; specifically modify getShellConfigFile (the block that currently looks for content.includes("# opencode") || content.includes(".opencode/bin")) to also test for content.includes("# altimate") and content.includes(".altimate-code/bin"), and update cleanShellConfig's pattern matching/replace logic (the function that trims out the opencode comment block and path entries) to also handle the "# altimate" comment marker and ".altimate-code/bin" path so uninstall will clean both installer variants.
🧹 Nitpick comments (8)
script/smoke-download-connections.ts (4)
66-68: JSON parse errors will surface as cryptic failures.Both
readJsonFileandloadExistingConnectionsuseJSON.parsewithout try-catch. If the file contains malformed JSON, the error message won't indicate which file failed. Consider wrapping with context:♻️ Suggested improvement
function readJsonFile(filepath: string) { - return JSON.parse(fs.readFileSync(filepath, "utf-8")) as Record<string, unknown> + try { + return JSON.parse(fs.readFileSync(filepath, "utf-8")) as Record<string, unknown> + } catch (e) { + throw new Error(`Failed to parse JSON from ${filepath}: ${e instanceof Error ? e.message : e}`) + } }Apply similar handling to
loadExistingConnections.Also applies to: 100-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/smoke-download-connections.ts` around lines 66 - 68, Wrap JSON.parse calls in readJsonFile and loadExistingConnections with try/catch so malformed JSON errors include the file path/context; specifically, catch the parse error inside readJsonFile (and mirror the same pattern in loadExistingConnections) and rethrow or log a new Error that includes the filepath and original error message/stack (e.g., "Failed to parse JSON in <filepath>: <original error>") to make failures actionable.
51-64: Naive YAML parser may fail on complex configs.
readSimpleYamlonly handles flatkey: valuelines. It won't correctly parse multi-line values, quoted strings containing colons, arrays, or nested structures. This is acceptable if the Snowflake/Databricks credential files are guaranteed to be simple, but consider adding a comment documenting this limitation or using a proper YAML library if configs evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/smoke-download-connections.ts` around lines 51 - 64, The current readSimpleYaml function is a fragile, line-based parser that breaks on multi-line values, quoted strings containing colons, arrays, and nested structures; replace it with a robust YAML parser (e.g., use js-yaml's load on fs.readFileSync(filepath, "utf-8")) and have readSimpleYaml return the parsed object as Record<string, unknown>, adding try/catch to surface parse errors, or if you prefer to keep the naive parser, add a clear comment above readSimpleYaml describing these limitations and when it's safe to use it; reference the readSimpleYaml function to find where to change and update imports to include the chosen YAML library if used.
168-169: Clarify the purpose ofchdirbefore dynamic import.The
process.chdir(opencodeDir)changes the current working directory, but the subsequent dynamic import uses a path relative toimport.meta.dir(the script's location), notprocess.cwd(). Thechdirdoesn't affect the import resolution.If the
chdiris needed forDispatcherinternals (e.g., config file discovery), consider adding a comment to clarify. Otherwise, it may be unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/smoke-download-connections.ts` around lines 168 - 169, The call to process.chdir(opencodeDir) appears unrelated to the subsequent dynamic import of ../packages/opencode/src/altimate/native/index.ts because dynamic imports resolve relative to the script location, not process.cwd(); either remove the process.chdir(opencodeDir) if it’s unnecessary, or if Dispatcher (or its internals) relies on the working directory for config discovery, retain it but add a clarifying comment above the chdir explaining that Dispatcher expects process.cwd() to be opencodeDir; locate the change near the process.chdir and the dynamic import and adjust accordingly for either removal or documenting why it must remain.
171-173: Consider validating that expected warehouses are configured.The
warehouse.listresult is logged but not validated. Per the type definitions, it returns{ warehouses: WarehouseInfo[] }. You could verify that all smoke check connections are actually present before running tests:♻️ Suggested validation
const listed = await Dispatcher.call("warehouse.list", {}) console.log("\nConfigured warehouses:") console.log(JSON.stringify(listed, null, 2)) const configuredNames = new Set(listed.warehouses.map((w: { name: string }) => w.name)) for (const check of smokeChecks) { if (!configuredNames.has(check.name)) { console.error(`Warning: ${check.name} not found in configured warehouses`) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@script/smoke-download-connections.ts` around lines 171 - 173, The code logs the result of Dispatcher.call("warehouse.list") into the variable listed but doesn't validate that the expected smokeChecks are present; add a validation step after calling Dispatcher.call("warehouse.list" ) that builds a Set of configured warehouse names from listed.warehouses (each WarehouseInfo.name) and then iterate the smokeChecks array, emitting a console.error or console.warn for each check whose name is missing (e.g., refer to variables listed and smokeChecks and use their name property), so missing configured warehouses are reported before tests run.packages/opencode/src/installation/index.ts (2)
283-293: Scoop upgrade constraint is reasonable but could improve UX.The validation that
targetmust matchlatestScoopmakes sense given scoop's manifest-based versioning. However, users attempting to upgrade to a specific version will get a somewhat cryptic error.Consider clarifying in the error message that scoop doesn't support pinned version upgrades:
Suggested improvement
if (target !== latestScoop) { throw new Error( - `Scoop can only upgrade to the latest published manifest (${latestScoop}). ` + - `Requested ${target}.`, + `Scoop doesn't support upgrading to specific versions. ` + + `Latest available: ${latestScoop}, requested: ${target}. ` + + `Use 'scoop update opencode' to get the latest, or switch to npm for version pinning.`, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/installation/index.ts` around lines 283 - 293, The error thrown when target !== latestScoop in the "scoop" case is too cryptic; update the message produced by the check in Installation.latest / the "scoop" case to clearly explain that Scoop only installs from the latest published manifest and does not support upgrading to a pinned/older manifest version, include both the requested value (target) and the allowed value (latestScoop) in the message, and keep the existing behavior (throwing an Error) before calling Process.run(["scoop","update","opencode"], { nothrow: true } so callers see a user-friendly explanation referencing target and latestScoop.
83-91: Consider using shell.ts's robust bash detection approach in installation code.The
which("bash")utility is properly available and correctly handles Windows with explicit PATH/PATHEXT variable handling. However,packages/opencode/src/shell/shell.tsalready implements a more robust bash detection pattern that first checks the standard Git Bash location (C:\Program Files\Git\bin\bash.exe) before falling back towhich("bash"). This approach is more reliable for users with Git Bash installed in standard locations. The installation code should consider adopting this same pattern for consistency and better Windows compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/installation/index.ts` around lines 83 - 91, Replace the simple which("bash") check in upgradeCurl with the same robust bash-detection helper used in packages/opencode/src/shell/shell.ts (the helper that first checks the standard Git Bash location on Windows before falling back to which), by importing and calling that exported function (or, if it's not exported, copy its logic) so upgradeCurl uses the Git Bash-aware resolution; update the const bash assignment in upgradeCurl to use that helper and keep the existing error message flow if no bash is found.packages/opencode/test/cli/upgrade-command.test.ts (1)
1-72: Good mock setup, consider expanding test coverage.The mock infrastructure is solid. Consider adding tests for edge cases like:
- When
Installation.VERSION === target(skip path)- When
Installation.upgrade()throwsUpgradeFailedError- When method is "unknown" and user declines install
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/cli/upgrade-command.test.ts` around lines 1 - 72, Add new tests exercising the skipped-upgrade, upgrade-failure, and unknown-method decline paths: create a test where Installation.VERSION equals latestMock() to assert upgrade is skipped (use latestMock and Installation.VERSION), a test where Installation.upgrade is made to throw the mocked UpgradeFailedError class and assert errorMock/other handling is invoked, and a test where methodMock returns "unknown" and the `@clack/prompts.select` mock is set to decline to assert the install/upgrade flow is aborted; reuse the existing mocks (methodMock, latestMock, upgradeMock, UpgradeFailedError, select mock, and spinner/log mocks) and reset them in afterEach so each scenario is isolated.install (1)
476-478: Consider cleaning upaltimateon--binaryinstalls too.A machine that previously used the full installer can keep the old
altimateshim alongside the newaltimate-codebinary, which leaves the two commands on different versions.🔧 Small cleanup
- rm -f "${INSTALL_DIR}/altimate-code" "${INSTALL_DIR}/altimate-code.exe" + rm -f "${INSTALL_DIR}/altimate" "${INSTALL_DIR}/altimate-code" "${INSTALL_DIR}/altimate-code.exe"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install` around lines 476 - 478, The install script currently only removes the old altimate-code shims; update the cleanup in the --binary install path to also remove the legacy altimate shim so a previous full-installer leftover doesn't remain. In the block using INSTALL_DIR, remove both "${INSTALL_DIR}/altimate" and "${INSTALL_DIR}/altimate.exe" (in addition to the existing altimate-code removals) before copying "$binary_path" to "${INSTALL_DIR}/$target_name" and setting chmod 755; reference the existing symbols INSTALL_DIR, altimate-code, altimate, "$binary_path" and "$target_name" to locate and modify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install`:
- Around line 455-466: The script currently deletes the existing shims and
node_modules before copying new files, risking leaving the CLI broken if any
cp/chmod fails; instead, implement a staging swap: create a temporary staging
directory (e.g. "$INSTALL_ROOT.tmp"), copy and chmod all new artifacts there
(use the same operations referencing wrapper_dir, platform_dir, core_dir,
core_native_dir and the package name vars platform_package_name and
core_native_package_name), verify the copies succeeded, then atomically
move/rename the staged directory over "$INSTALL_ROOT" and update/install the
shims in "$INSTALL_DIR" (remove or replace originals only after the staged
install is confirmed); ensure error checking so rm -rf on the live install only
runs after a successful staged swap.
- Around line 426-436: The curl fallbacks and the download helper don't use
--fail/--show-error, so HTTP errors can appear as successful downloads; update
the curl invocations that write to "$wrapper_tgz", "$platform_tgz", "$core_tgz",
and "$core_native_tgz" to include --fail and --show-error (e.g., curl --fail
--show-error -# -L -o ...) and also add those flags inside the
download_with_progress implementation so it propagates non-2xx HTTP failures
instead of producing tar extraction errors.
In `@packages/opencode/src/cli/cmd/pr.ts`:
- Around line 85-95: The current session-detection regex only matches the old
opncd.ai shortlink; replace the literal-domain match in the PR handler (the
prInfo.body.match call that produces sessionMatch) with a domain-agnostic
pattern (for example match https?://<any-host>/s/<id> e.g.
/https?:\/\/[^\/\s]+\/s\/([A-Za-z0-9_-]+)/) so Altimate rebranded URLs are
detected while avoiding hardcoded opncd.ai strings; keep using sessionMatch[0]
for sessionUrl and leave the subsequent UI.println and
Process.text(["altimate-code","import", sessionUrl], { nothrow: true }) calls
unchanged.
In `@packages/opencode/test/branding/branding.test.ts`:
- Around line 143-150: The test "network defaults use altimate local domain"
currently asserts that networkContent does not contain "opencode.local" but
misses the same negative assertion for mdnsContent; update the test to add
expect(mdnsContent).not.toContain("opencode.local") (alongside the existing
expect(mdnsContent).toContain("altimate.local") and
expect(mdnsContent).toContain("altimate-")) so the mdns.ts source is also
checked for removal of the stale domain.
In `@packages/opencode/test/install/github-action.test.ts`:
- Around line 8-11: The test "installs from the repo-hosted shell script instead
of altimate.ai" currently asserts a moving-branch URL
(https://raw.githubusercontent.com/AltimateAI/altimate-code/main/install);
change the assertion to expect a stable ref (e.g. a tag or commit SHA) instead
of /main/, and update the action.yml that ACTION_YML points to so the installer
URL uses the same stable ref (the action's tag or commit SHA) — locate the test
by the test name and the ACTION_YML constant, and modify the action file’s
installer URL to be pinned to the action’s own version (tag or SHA) so the test
and action remain consistent.
In `@packages/opencode/test/install/root-installer.test.ts`:
- Around line 10-31: Replace the manual temp-dir management (makeHome, homes
array, and afterEach cleanup) with the test fixture tmpdir: change tests to
allocate a temp home via "await using tmp = await tmpdir(...)" and use tmp.path
(realpath-resolved) when setting HOME in runInstaller's env; remove makeHome,
homes, and afterEach, and update any callers of makeHome to use tmp.path so the
fixture handles automatic cleanup and realpath guarantees.
---
Outside diff comments:
In `@packages/opencode/src/cli/cmd/uninstall.ts`:
- Around line 218-220: The cleanup hint currently only logs rmdir when
binDir.includes(".opencode"); update the condition in uninstall.ts to also
detect the new curl install path by checking for ".altimate-code" (e.g., replace
the single includes check with a logical OR or a regex that matches either
".opencode" or ".altimate-code") so prompts.log.info(` rmdir "${binDir}"
2>/dev/null`) is shown for both install locations.
- Around line 269-272: Update getShellConfigFile and cleanShellConfig to detect
and remove entries for the new installer by checking for both
".altimate-code/bin" and the altimate comment marker in addition to the existing
".opencode/bin" and "# opencode" checks; specifically modify getShellConfigFile
(the block that currently looks for content.includes("# opencode") ||
content.includes(".opencode/bin")) to also test for content.includes("#
altimate") and content.includes(".altimate-code/bin"), and update
cleanShellConfig's pattern matching/replace logic (the function that trims out
the opencode comment block and path entries) to also handle the "# altimate"
comment marker and ".altimate-code/bin" path so uninstall will clean both
installer variants.
---
Nitpick comments:
In `@install`:
- Around line 476-478: The install script currently only removes the old
altimate-code shims; update the cleanup in the --binary install path to also
remove the legacy altimate shim so a previous full-installer leftover doesn't
remain. In the block using INSTALL_DIR, remove both "${INSTALL_DIR}/altimate"
and "${INSTALL_DIR}/altimate.exe" (in addition to the existing altimate-code
removals) before copying "$binary_path" to "${INSTALL_DIR}/$target_name" and
setting chmod 755; reference the existing symbols INSTALL_DIR, altimate-code,
altimate, "$binary_path" and "$target_name" to locate and modify the code.
In `@packages/opencode/src/installation/index.ts`:
- Around line 283-293: The error thrown when target !== latestScoop in the
"scoop" case is too cryptic; update the message produced by the check in
Installation.latest / the "scoop" case to clearly explain that Scoop only
installs from the latest published manifest and does not support upgrading to a
pinned/older manifest version, include both the requested value (target) and the
allowed value (latestScoop) in the message, and keep the existing behavior
(throwing an Error) before calling Process.run(["scoop","update","opencode"], {
nothrow: true } so callers see a user-friendly explanation referencing target
and latestScoop.
- Around line 83-91: Replace the simple which("bash") check in upgradeCurl with
the same robust bash-detection helper used in
packages/opencode/src/shell/shell.ts (the helper that first checks the standard
Git Bash location on Windows before falling back to which), by importing and
calling that exported function (or, if it's not exported, copy its logic) so
upgradeCurl uses the Git Bash-aware resolution; update the const bash assignment
in upgradeCurl to use that helper and keep the existing error message flow if no
bash is found.
In `@packages/opencode/test/cli/upgrade-command.test.ts`:
- Around line 1-72: Add new tests exercising the skipped-upgrade,
upgrade-failure, and unknown-method decline paths: create a test where
Installation.VERSION equals latestMock() to assert upgrade is skipped (use
latestMock and Installation.VERSION), a test where Installation.upgrade is made
to throw the mocked UpgradeFailedError class and assert errorMock/other handling
is invoked, and a test where methodMock returns "unknown" and the
`@clack/prompts.select` mock is set to decline to assert the install/upgrade flow
is aborted; reuse the existing mocks (methodMock, latestMock, upgradeMock,
UpgradeFailedError, select mock, and spinner/log mocks) and reset them in
afterEach so each scenario is isolated.
In `@script/smoke-download-connections.ts`:
- Around line 66-68: Wrap JSON.parse calls in readJsonFile and
loadExistingConnections with try/catch so malformed JSON errors include the file
path/context; specifically, catch the parse error inside readJsonFile (and
mirror the same pattern in loadExistingConnections) and rethrow or log a new
Error that includes the filepath and original error message/stack (e.g., "Failed
to parse JSON in <filepath>: <original error>") to make failures actionable.
- Around line 51-64: The current readSimpleYaml function is a fragile,
line-based parser that breaks on multi-line values, quoted strings containing
colons, arrays, and nested structures; replace it with a robust YAML parser
(e.g., use js-yaml's load on fs.readFileSync(filepath, "utf-8")) and have
readSimpleYaml return the parsed object as Record<string, unknown>, adding
try/catch to surface parse errors, or if you prefer to keep the naive parser,
add a clear comment above readSimpleYaml describing these limitations and when
it's safe to use it; reference the readSimpleYaml function to find where to
change and update imports to include the chosen YAML library if used.
- Around line 168-169: The call to process.chdir(opencodeDir) appears unrelated
to the subsequent dynamic import of
../packages/opencode/src/altimate/native/index.ts because dynamic imports
resolve relative to the script location, not process.cwd(); either remove the
process.chdir(opencodeDir) if it’s unnecessary, or if Dispatcher (or its
internals) relies on the working directory for config discovery, retain it but
add a clarifying comment above the chdir explaining that Dispatcher expects
process.cwd() to be opencodeDir; locate the change near the process.chdir and
the dynamic import and adjust accordingly for either removal or documenting why
it must remain.
- Around line 171-173: The code logs the result of
Dispatcher.call("warehouse.list") into the variable listed but doesn't validate
that the expected smokeChecks are present; add a validation step after calling
Dispatcher.call("warehouse.list" ) that builds a Set of configured warehouse
names from listed.warehouses (each WarehouseInfo.name) and then iterate the
smokeChecks array, emitting a console.error or console.warn for each check whose
name is missing (e.g., refer to variables listed and smokeChecks and use their
name property), so missing configured warehouses are reported before tests run.
🪄 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: 0a14f105-69b4-425a-8169-783e37c0dbaf
📒 Files selected for processing (18)
github/action.ymlinstallpackages/opencode/src/cli/cmd/pr.tspackages/opencode/src/cli/cmd/tui/thread.tspackages/opencode/src/cli/cmd/uninstall.tspackages/opencode/src/cli/cmd/upgrade.tspackages/opencode/src/cli/network.tspackages/opencode/src/config/config.tspackages/opencode/src/installation/index.tspackages/opencode/src/server/mdns.tspackages/opencode/test/branding/branding.test.tspackages/opencode/test/cli/upgrade-command.test.tspackages/opencode/test/install/github-action.test.tspackages/opencode/test/install/root-installer.test.tspackages/opencode/test/installation/upgrade-methods.test.tspackages/opencode/test/installation/upgrade.test.tspackages/opencode/test/installation/version.test.tsscript/smoke-download-connections.ts
Author
Updated Findings from Installation Claim MatrixLatest run: https://github.com/AltimateAI/altimate-qa/actions/runs/23295037235 Supported channels (from CLI
|
| Channel | Ubuntu | macOS | Windows | Issue |
|---|---|---|---|---|
| npm | ✅ | ✅ | ✅ | — |
| bun | ✅ | ✅ | ❌ | bun global install on Windows (bun limitation) |
| pnpm | ✅ | ❌ | ✅ | macOS: upgrade installs 0.5.1 instead of 0.5.2 |
| curl | ❌ | ❌ | — | chmod: dangling symlink — install script creates symlink to non-existent binary |
| brew | — | ❌ | — | Formula/release asset issue |
| scoop | — | — | ❌ | Uses upstream "opencode" package |
| choco | — | — | ❌ | Uses upstream "opencode" package |
New bugs found:
1. curl install: dangling symlink (CRITICAL)
chmod: cannot operate on dangling symlink '/tmp/.altimate-code/bin/altimate-code'
Install script creates ~/.altimate-code/bin/altimate-code as symlink before the binary is downloaded.
2. pnpm upgrade version mismatch (HIGH)
upgrade version check did not include 0.5.2: 0.5.1
pnpm install -g @altimateai/altimate-code@0.5.2 installs 0.5.1 on macOS. May be pnpm cache or resolution issue.
3. scoop/choco use upstream "opencode" package
These install the upstream opencode (v1.2.27), not altimate-code (v0.5.2). Users on these channels get the wrong product. Need to either publish altimate-code packages or remove these channels from the CLI.
What this PR fixes vs what needs separate action:
| Issue | This PR? | Action needed |
|---|---|---|
| .altimate-code path detection | ✅ Fixed | — |
| Hardcoded choco error | ✅ Fixed | — |
| HTML guard on curl | ✅ Fixed | Server needs to serve script |
| Windows curl guard | ✅ Fixed | — |
| Fetch timeout | ✅ Fixed | — |
| Empty results guards | ✅ Fixed | — |
| Brew 404 fallback | ✅ Fixed | Tap formula needs update to v0.5.2 |
| Local channel skip | ✅ Fixed | — |
| curl dangling symlink | ❌ | Install script bug (server-side) |
| pnpm version mismatch | ❌ | Needs investigation (pnpm resolution) |
| scoop/choco wrong product | ❌ | Publish altimate-code packages or remove channels |
dev-punia-altimate
force-pushed
the
qa-autopilot/fix-installation-issues
branch
from
fb26b4f to
fce42af
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/installation/index.ts`:
- Around line 265-269: The scoop branch currently always runs
Process.run(["scoop", "update", "opencode"], { nothrow: true }) which ignores
any provided target version; update the case "scoop" handling to respect the
target parameter by either (a) when a target is passed, run the appropriate
scoop install syntax (e.g., "scoop install opencode@<target>" if your buckets
support it) or explicitly return/throw/log that scoop cannot pin to a specific
version and fall back to "scoop update" only when no target is provided; modify
the Process.run call(s) and the surrounding logic that sets result to handle
these two flows and add a clear inline comment explaining scoop’s limitation if
you choose the latter approach so callers know target versions are not
supported.
🪄 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: 043d16b5-3328-4f11-9752-83523bb5d039
📒 Files selected for processing (2)
packages/opencode/src/cli/cmd/uninstall.tspackages/opencode/src/installation/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opencode/src/cli/cmd/uninstall.ts
dev-punia-altimate
force-pushed
the
qa-autopilot/fix-installation-issues
branch
from
fce42af to
7c2185b
Compare
Author
Clarification on install matrix resultsAll channels were tested. Here's what the matrix found: Working (code is correct):
Code fixed in this PR, but infrastructure still broken:
Upstream packages (not altimate-published):
Needs investigation:
The install matrix is reusable — run it after each infra fix to verify. |
dev-punia-altimate
force-pushed
the
qa-autopilot/fix-installation-issues
branch
from
7c2185b to
4c5e3ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/opencode/src/installation/index.ts (1)
265-268:⚠️ Potential issue | 🟠 Major
targetis still ignored for Scoop upgrades.This branch never uses
target, soupgrade(method, target)can succeed after installing whatever version the bucket serves rather than the version the caller requested. If pinning isn't supported here, fail fast with a clear error instead of pretending the request was honored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/installation/index.ts` around lines 265 - 268, The "scoop" upgrade branch ignores the target argument, allowing upgrade(method, target) to appear successful while not honoring a requested version; update the case in the upgrade function (the "scoop" branch that calls Process.run(["scoop", "update", "opencode"], ...)) to check the target parameter and if a specific target/version is provided, fail fast with a clear error (or throw) indicating pinning/targeting isn't supported for scoop; if no target is provided, proceed with the existing Process.run(["scoop", "update", "opencode"], { nothrow: true }) path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/installation/index.ts`:
- Around line 51-74: The new preflight checks in installUrl/fetchWithTimeout
(body HTML guard and Windows guard) throw raw Errors which bypass the upgrade()
telemetry and its UpgradeFailedError handling; update upgradeCurl() (or the
calling upgrade() flow) to catch these specific failures and rethrow or wrap
them as UpgradeFailedError (including the original error/message and non-zero
exit/result code), or alternatively return a synthetic non-zero result from
upgradeCurl() so upgrade() sees a failure and reports it via its existing
telemetry path; refer to installUrl, fetchWithTimeout, upgradeCurl(), and
upgrade() to locate where to add the try/catch and wrapping/return behavior.
---
Duplicate comments:
In `@packages/opencode/src/installation/index.ts`:
- Around line 265-268: The "scoop" upgrade branch ignores the target argument,
allowing upgrade(method, target) to appear successful while not honoring a
requested version; update the case in the upgrade function (the "scoop" branch
that calls Process.run(["scoop", "update", "opencode"], ...)) to check the
target parameter and if a specific target/version is provided, fail fast with a
clear error (or throw) indicating pinning/targeting isn't supported for scoop;
if no target is provided, proceed with the existing Process.run(["scoop",
"update", "opencode"], { nothrow: true }) path.
🪄 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: a692b1e3-5aed-433a-8e64-15c1e45eab9d
📒 Files selected for processing (2)
packages/opencode/src/cli/cmd/uninstall.tspackages/opencode/src/installation/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opencode/src/cli/cmd/uninstall.ts
Comprehensive fix for installation/upgrade reliability across all platforms.
1. **HTML guard on curl install** — altimate.ai/install currently returns HTML.
Now detects HTML response and gives actionable error with npm workaround.
2. **Windows curl guard** — curl/bash upgrade on Windows now fails fast with
PowerShell alternative command.
3. **Fetch timeout (10s)** — all fetch() calls now use AbortController timeout
to prevent hanging on network issues.
4. **.altimate-code/bin path detection** — method() now detects curl installs
in ~/.altimate-code/bin/ (not just ~/.opencode/bin/).
5. **Scoop upgrade command** — changed from `scoop install opencode@version`
to `scoop update opencode` (correct command for upgrades).
6. **Choco error message** — removed hardcoded "not running from elevated shell",
now shows actual stderr from choco process.
7. **Choco empty results guard** — data.d.results[0].Version no longer crashes
when Chocolatey returns empty results array.
8. **Scoop missing version guard** — checks data.version exists before accessing.
9. **GitHub releases null guard** — checks data.tag_name exists before .replace().
10. **Brew JSON parse safety** — JSON.parse wrapped in try/catch for non-JSON output.
11. **Brew core formula 404** — removed fetch to formulae.brew.sh/altimate-code.json
(doesn't exist in homebrew core, only in AltimateAI/tap). Falls through to
GitHub releases instead.
12. **Local channel skip** — npm/bun/pnpm version check returns current VERSION
when CHANNEL is "local" instead of fetching /local from registry (404).
| Registry | Package | Exists? |
|----------|---------|---------|
| npm | @altimateai/altimate-code | ✅ v0.4.9 |
| scoop | opencode | ✅ v1.2.27 (upstream) |
| choco | opencode | ✅ v1.2.27 (upstream) |
| brew | AltimateAI/tap/altimate-code | ✅ formula exists, v0.3.1 |
| GitHub | AltimateAI/altimate-code | ✅ v0.4.9 with all platform binaries |
- altimate.ai/install returns HTML homepage (needs server config fix)
- Brew tap formula pinned to v0.3.1 (needs tap update)
- No altimate-code packages on scoop/choco (uses upstream opencode)
Install matrix (55 combinations): https://github.com/AltimateAI/altimate-qa/actions/runs/23263489002
dev-punia-altimate
force-pushed
the
qa-autopilot/fix-installation-issues
branch
from
4c5e3ac to
5694fca
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opencode/src/cli/cmd/uninstall.ts (1)
136-146:⚠️ Potential issue | 🟡 MinorDisplay commands don't match actual uninstall commands.
The
cmdsrecord here shows legacy package names (e.g.,opencode-ai,opencode) to users, butexecuteUninstallat lines 188-195 runs commands with the new package names (@altimateai/altimate-code,altimate-code). This inconsistency could confuse users if they try to run these commands manually.🔧 Proposed fix to align display commands
const cmds: Record<string, string> = { - npm: "npm uninstall -g opencode-ai", - pnpm: "pnpm uninstall -g opencode-ai", - bun: "bun remove -g opencode-ai", - yarn: "yarn global remove opencode-ai", - brew: "brew uninstall opencode", + npm: "npm uninstall -g `@altimateai/altimate-code`", + pnpm: "pnpm uninstall -g `@altimateai/altimate-code`", + bun: "bun remove -g `@altimateai/altimate-code`", + yarn: "yarn global remove `@altimateai/altimate-code`", + brew: "brew uninstall altimate-code", choco: "choco uninstall opencode", scoop: "scoop uninstall opencode", }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/cli/cmd/uninstall.ts` around lines 136 - 146, The displayed uninstall commands in the cmds map are using old package names (e.g., opencode-ai/opencode) and must be updated to match the actual commands run by executeUninstall; update the cmds Record in uninstall.ts so entries for npm, pnpm, bun, yarn use the new package scope/name used by executeUninstall (e.g., `@altimateai/altimate-code` or altimate-code), and update brew/choco/scoop to the new package names as well so prompts.log.info(` ✓ Package: ${cmds[method] || method}`) shows the same commands the code executes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/opencode/src/cli/cmd/uninstall.ts`:
- Around line 136-146: The displayed uninstall commands in the cmds map are
using old package names (e.g., opencode-ai/opencode) and must be updated to
match the actual commands run by executeUninstall; update the cmds Record in
uninstall.ts so entries for npm, pnpm, bun, yarn use the new package scope/name
used by executeUninstall (e.g., `@altimateai/altimate-code` or altimate-code), and
update brew/choco/scoop to the new package names as well so prompts.log.info(`
✓ Package: ${cmds[method] || method}`) shows the same commands the code
executes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3485b73c-507c-4fb4-a84d-90dfb9da22e4
📒 Files selected for processing (2)
packages/opencode/src/cli/cmd/uninstall.tspackages/opencode/src/installation/index.ts
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.
Evidence Matrix
Test Results
Full matrix run: https://github.com/AltimateAI/altimate-qa/actions/runs/23263489002
Issues Found
Fixes in This PR
9 renames of stale "opencode" references to "altimate-code" in installation/index.ts:
Not Fixed (needs separate action)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores