perf: parallelize local environment sync - #5256
Conversation
| child.stderr.on('data', chunk => (stderr += chunk)); | ||
| child.once('error', reject); | ||
| child.once('close', code => resolve({ status: code ?? 1, stderr, stdout })); | ||
| child.stdin.end(input); |
There was a problem hiding this comment.
WARNING: Unhandled 'error' event on child.stdin can crash the process and mask the real error
If the spawned wrangler exits before consuming stdin (e.g. it fails fast on an invalid store ID) — or the spawn itself fails — writing/closing stdin emits an EPIPE/stream error on child.stdin. With no error listener attached, Node raises it as an unhandled exception, killing the sync instead of surfacing wrangler's stderr via the result.status !== 0 path in createSecretsStoreSecret. The previous spawnSync implementation handled this internally, so this is a regression. Attach a no-op error handler before ending stdin, e.g.:
| child.stdin.end(input); | |
| child.stdin.on('error', () => {}); | |
| child.stdin.end(input); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe new async Overview
Issue Details (click to expand)WARNING
Files Reviewed (9 files)
The concurrency limiting, per-persistence-dir grouping, cross-process locking, and Fix these issues in Kilo Cloud Reviewed by kimi-k3 · Input: 168.8K · Output: 6K · Cached: 259.7K Review guidance: REVIEW.md from base branch |
Summary
--missing-secrets-onlyfor fast worktree setup without refreshing existing source-backed secretsKILO_ENV_SYNC_CONCURRENCYlimit and document the workflowPerformance
dev:env --check: 20.9s → 7.1s locallySafety
.wranglerdirectory remain serialized and cross-process locked; Wrangler 4.112.0 producedSQLITE_READONLYfailures when tested with concurrent writers to one local storeTesting
node --import tsx --test dev/local/*.test.ts dev/local/env-sync/*.test.ts dev/local/scripts/*.test.ts(212 passed)git diff --check