feat(settings): add OpenCode update action#585
Conversation
Show the installed OpenCode version in the OpenCode settings section and expose an update button only when the npm registry advertises a newer release. The button includes the target version and reports loading, success, unavailable-instance, and failure states in every supported locale. Resolve the configured binary version on the server, cache registry checks for five minutes, and invoke the generated OpenCode SDK global upgrade command through a running instance that uses the same binary. Existing instances remain uninterrupted while newly started instances use the installed update. Cover update detection, cache reuse, SDK target selection, and missing-instance behavior with focused service tests. Validated with server and UI typechecks plus full UI and server builds.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/29257301316 Artifacts expire in 7 days.
|
pascalandr
left a comment
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper review: CHANGES REQUIRED
Findings
-
High: arbitrary binary probing through GET is unsafe on Windows.
packages/server/src/server/routes/opencode-update.ts:11-12,27-30accepts an arbitrarybinaryquery parameter andpackages/server/src/opencode-update/service.ts:39-42,86-90executes it with--version..cmdpaths are interpolated into acmd.exe /ccommand line bypackages/server/src/workspaces/spawn.ts:82-91, so crafted quotes/metacharacters can become command injection. Because the session cookie isSameSite=Lax, a cross-site top-level GET can carry authentication. Resolve only persisted/validated binaries server-side and do not expose command execution through a parameterized GET. -
High: the normal Windows npm installation cannot show status or update. The service probes the raw configured value, normally
opencode, while workspace launch first resolves it withwhere. Reproduced locally:opencode --versionreturns1.17.18, butprobeBinaryVersion("opencode")returnsspawnSync opencode ENOENT;whereresolvesopencode.cmd. Status and upgrade must use the same canonical resolution as workspace startup. -
High: upgrade inherits an unsuitable 10-second timeout.
createInstanceClientappliesAbortSignal.timeout(10_000)inpackages/server/src/workspaces/instance-client.ts:42-46, and the upgrade uses that client atpackages/server/src/opencode-update/service.ts:145-149. npm/Homebrew/Chocolatey/WSL upgrades routinely exceed ten seconds, potentially reporting failure during a still-running or partial installation. Use an upgrade-specific longer timeout and deterministic in-flight handling. -
Medium: success is not verified against the selected executable. The SDK result is trusted and the UI immediately declares the target version installed. Re-probe the canonical selected binary after upgrade and only report success when its actual version matches the target; otherwise a custom/portable binary may leave a different global installation updated.
-
Medium: registry failure hides the locally installed version. Local probing and the npm request are combined into one failing status operation. Return the installed version independently and model update availability as unavailable when the registry cannot be reached.
-
Medium: concurrent clients can launch overlapping package-manager upgrades. Add a per-canonical-binary in-flight guard that joins or rejects duplicate requests.
-
Medium: WSL binary matching lowercases the Linux path suffix.
packages/server/src/workspaces/manager.ts:81-86lowercases the entire UNC path on Windows, although the Linux path portion is case-sensitive. Preserve Linux path casing when matching WSL binaries. -
Low: tests cover only mocked service behavior. Missing coverage includes route validation, Windows command resolution, timeout behavior, post-upgrade verification, concurrency, WSL identity, and UI states. The focused service test is also not wired into the current package/CI test scripts.
Residual context
All current CI builds are green or completing successfully, but they compile the feature without exercising the runtime paths above.
Verdict: CHANGES REQUIRED
Remove request-controlled binary paths from the update API and resolve the persisted OpenCode binary through the same canonical path logic used for workspace startup. Preserve WSL Linux path casing while retaining case-insensitive matching for Windows paths. Allow the SDK upgrade call to run for up to ten minutes, coalesce concurrent upgrades per binary, and verify the selected executable reports the target version before returning success. Registry failures now preserve the installed version in settings, and stale completions cannot overwrite status after the selected binary changes. Add service, route, Windows path, concurrency, verification, registry-failure, and WSL matching coverage. Validated with ten focused tests, server and UI typechecks, full UI and server builds, and a real Windows resolution from opencode to opencode.cmd.
pascalandr
left a comment
pascalandr
left a comment
There was a problem hiding this comment.
Gatekeeper follow-up: PASS
Commit 30c81867 resolves the blocking findings from the previous review:
- removed request-controlled binary paths from both update endpoints;
- resolved the persisted binary with the same canonical lookup used by workspace startup;
- verified the Windows npm wrapper resolves from
opencodetoopencode.cmdand reports its version; - extended only the upgrade SDK call to a ten-minute timeout;
- re-probed the configured executable after upgrade before reporting success;
- preserved the installed version when the npm registry check fails;
- coalesced concurrent upgrades per canonical binary;
- preserved Linux path case when comparing WSL UNC binaries;
- ignored stale upgrade completions after the configured binary changes;
- added route, service, concurrency, verification, registry-failure, Windows-path, and WSL-path tests.
Validation completed locally:
- 10 focused tests passed;
- server typecheck passed;
- UI typecheck passed;
- UI build passed;
- full server build passed;
- real Windows resolution/probe passed for
opencode.cmd.
Residual non-blocking gaps: there is no dedicated component test for the stale-completion transition, and the focused tests are not yet wired into a dedicated CI job. The full server suite produced 153 passes and one unrelated existing failure caused by the local Git default branch name.
Verdict: PASS
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/29265271400 Artifacts expire in 7 days.
|
1 similar comment
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/29265271400 Artifacts expire in 7 days.
|
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/29331927591 Artifacts expire in 7 days.
|
1 participant
Summary
Server behavior
Validation
The local OpenCode version and npm latest version were both 1.17.18, confirming the up-to-date state hides the update action.