Fix CLI models snapshot release validation - #10949
Conversation
| return null | ||
| }) | ||
| if (!sourceBinPath) return | ||
| const sourceSnapshotPath = snapshotForBinary(sourceBinPath) |
There was a problem hiding this comment.
WARNING: Variable shadowing — sourceSnapshotPath is already declared at module scope on line 35 (pointing to packages/opencode/src/provider/models-snapshot.json for the source-wrapper path). This inner const shadows it with the sidecar path next to the built binary, which is the correct value for this path, but the shadowing is confusing and will likely be flagged by the ESLint no-shadow rule.
Consider renaming the local to snapshotPath (consistent with the same helper in watch-cli.ts) or remove the module-level sourceSnapshotPath constant since it is only used inside writeSourceWrapper() where existsSync(sourceSnapshotPath) is called — that use could just inline the join(opencodeDir, "src", "provider", snapshotName) call or compute it from snapshotForBinary with a dev-mode sentinel.
|
|
||
| await $`mkdir -p ${targetBinDir}` | ||
| await $`cp ${source} ${targetBinPath}` | ||
| await $`cp ${snapshotPath(source)} ${join(targetBinDir, snapshotName)}` |
There was a problem hiding this comment.
SUGGESTION: The snapshot is copied without first checking that the file exists. If the build produced the binary but somehow not the sidecar (e.g. a partial failure), cp will throw an ENOENT which is caught by the outer try/catch and logged as a generic ERROR. The binary will then be in place at targetBinPath without the accompanying snapshot.
Adding an existence check before the copy (similar to how local-bin.ts does it at line 105) would surface a clearer diagnostic. This is a dev-only watcher so the blast radius is small, but the resulting state (binary present, no snapshot) would cause the VS Code extension to fail to start on the next run with a confusing error.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Both issues from the previous review have been resolved in this commit:
The copy order change (snapshot before binary) in both files is consistent and intentional. Files Reviewed (2 files, incremental)
Fix these issues in Kilo Cloud Reviewed by claude-4.6-sonnet-20260217 · 267,579 tokens Review guidance: REVIEW.md from base branch |
Fix CLI models snapshot release validation
Issue
No linked issue. This addresses a release-build regression where an invalid or broken generated models snapshot could pass publication build steps and only fail when the packaged CLI starts.
Context
The CLI build previously generated a giant mutable
models-snapshot.jsmodule frommodels.dev/api.json. Bun 1.3.14 can emit broken standalone binaries from that generated JS, and the publish workflow did not validate all packaged CLI binaries before the VS Code extension consumed them.Implementation
The build now downloads, parses, validates, and canonicalizes the models.dev snapshot once before compiling CLI targets. The snapshot is stored as a JSON sidecar next to each packaged
kilobinary instead of being compiled as a JS object literal.The runtime model loader reads the sidecar or dev snapshot JSON when model metadata is needed. VS Code and JetBrains packaging paths copy/extract the sidecar alongside the CLI. Local VS Code development also refreshes the sidecar and falls back to a Bun source wrapper if local standalone compilation fails, so extension testing can still start the backend.
The publish workflow now validates supported Unix and Windows CLI artifacts with
--versionand--pure models anthropicbefore building VSIX packages.Screenshots / Video
N/A, no visual changes.
How to Test
Manual/local verification
bun test ./test/kilocode/provider/models-snapshot.test.tsfrompackages/opencodepassed.bun run typecheckfrompackages/opencodepassed.bun run check-types:extensionfrompackages/kilo-vscodepassed../gradlew typecheckfrompackages/kilo-jetbrainspassed.bun run script/check-workflows.tspassed..github/workflows/publish.ymlwith the local Nodeyamlparser successfully.packages/kilo-vscode/bin/kilo --versionreturnedlocal.packages/kilo-vscode/bin/kilo serve --port 0started and printed a listening URL.bun turbo typecheckhook successfully.Reviewer test steps
bun test ./test/kilocode/provider/models-snapshot.test.tsfrompackages/opencode.bun script/local-bin.ts --forcefrompackages/kilo-vscodeand confirmbin/kilo --versionworks.publishworkflow graph and confirmbuild-vscodedepends onvalidate-cli-unixandvalidate-cli-windows.models anthropicwith model fetching disabled.Blocked checks and substitute verification
bun run script/build.ts --singlefrompackages/opencodecould not complete locally because the compiled Linux x64 standalone binary exits with code 139 during the existing--versionsmoke test in this Nix/Bun environment. Substitute verification wasbun run typecheck, targeted snapshot tests, VS Code extension typecheck, JetBrains typecheck, local VS Code source-wrapper backend startup, and the pre-pushbun turbo typecheckhook.Checklist
Get in Touch
@catrielmuller