fix(cli): isolate release target builds - #10935
Conversation
| return true | ||
| }) | ||
| : allTargets | ||
| if (isolated) targets.splice(0, targets.length, allTargets[selection]!) // kilocode_change - select one target in child |
There was a problem hiding this comment.
SUGGESTION: targets is a direct reference to allTargets when singleFlag is false (line 223: : allTargets), so targets.splice(...) mutates allTargets itself. This doesn't cause a bug today — the child process exits after one build and never reaches allTargets.indexOf(item) — but it's fragile. Consider using a fresh array to avoid the alias:
if (isolated) targets.splice(0, targets.length, allTargets[selection]!)Could be rewritten as:
const targets = isolated
? [allTargets[selection]!]
: singleFlag
? allTargets.filter(...)
: allTargetsor simply targets.length = 0; targets.push(allTargets[selection]!) if keeping the current structure.
|
|
||
| // kilocode_change start - isolate Bun's shared-chunk state between cross-platform targets | ||
| if (!isolated && targets.length > 1) { | ||
| await $`${process.execPath} run script/build.ts --target=${allTargets.indexOf(item)} --skip-install` |
There was a problem hiding this comment.
SUGGESTION: The child subprocess only receives --skip-install. Other flags like --sourcemaps and --baseline are not forwarded. For release builds this is intentional (sourcemaps are disabled via Script.release), but if someone invokes build.ts --sourcemaps on a multi-target build (non---single), the child processes will silently ignore it.
Consider forwarding extra flags explicitly or documenting that multi-target builds don't support --sourcemaps:
const extraFlags = sourcemapsFlag ? ["--sourcemaps"] : []
await $`${process.execPath} run script/build.ts --target=${allTargets.indexOf(item)} --skip-install ${extraFlags}`| path: packages/kilo-vscode/out | ||
|
|
||
| # kilocode_change start - execute the cross-compiled Apple Silicon binary before publishing it | ||
| validate-darwin-arm64: |
There was a problem hiding this comment.
SUGGESTION: The validate-darwin-arm64 job (and validate-linux below) is missing if: github.repository == 'Kilo-Org/kilocode' that every other job in this file has (version, build-cli, build-vscode, smoke-test, publish). In practice it doesn't matter since this workflow is only triggered from the main repo, but it breaks the established pattern and could cause confusion in forks.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Minor suggestions only — safe to merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (3 files)
The core fix is well-reasoned. The root cause analysis in the PR description is solid — process-level state isolation in Bun's bundler is the right approach. The validation gates in CI close a real gap that allowed broken cross-compiled artifacts to ship. Fix these issues in Kilo Cloud Reviewed by claude-4.6-sonnet-20260217 · 1,165,457 tokens Review guidance: REVIEW.md from base branch |
…-state fix(cli): isolate release target builds
What changed
Root cause
Incorrect root cause, we will unwind the split build changes but keep the kilo CLI arch --version tests which are valuable.