Fix #1024: bump CLI preflight timeout to 5s + add configurable override#1026
Merged
Conversation
…verride The 400ms cap on the `codev --version` startup probe was too tight against legitimate latency sources (remote SSH, WSL, nvm/fnm/volta shim re-resolution, AV scanning, network filesystems), producing a false `missing` status that opened the 'Get started with Codev' walkthrough and no-op'd guarded commands despite a healthy install. - Default timeout bumped 400ms -> 5000ms (DEFAULT_VERSION_TIMEOUT_MS). - New setting codev.cliVersionTimeoutMs (number, default 5000, min 100, max 60000) overrides it per-workspace, surviving extension upgrades. - runCodevVersion now reports a timedOut flag; the OutputChannel logs a [Preflight] line naming the timeout value and recovery action when the cap fires, so a slow-env false-negative is diagnosable instead of silent. - Relocated the vscode-free runCodevVersion probe + a pure resolveVersionTimeout helper into preflight-core.ts so they are unit-testable under vitest. - Added unit tests covering the explicit-timeoutMs (positive) and setting-unset default (negative) cases.
codex + claude both noted comments still naming the old 400ms cap. Make them version-agnostic: extension.ts now points at the codev.cliVersionTimeoutMs budget; the #983 Tower-divergence comment drops the stale ~400ms figure. Comment-only; no behavior change.
Per architect review at the pr gate: resolveVersionTimeout + MIN/MAX constants
were over-built for a constant bump. VSCode already returns the package.json
default for an unset setting and enforces minimum/maximum in its settings UI,
so the helper's unset-fallback branch was dead at runtime and only the marginal
clamp/non-number guard ran live.
- Read the setting with the codebase idiom:
getConfiguration('codev').get<number>(KEY, DEFAULT_VERSION_TIMEOUT_MS),
matching overviewRefreshSeconds et al.
- Delete resolveVersionTimeout, MIN_VERSION_TIMEOUT_MS, MAX_VERSION_TIMEOUT_MS;
the 100/60000 bounds now live solely in package.json.
- Tests: drop the resolveVersionTimeout cases; runCodevVersion suite keeps the
explicit-timeout (positive) + default-when-omitted (negative) + spawn-error +
5000 regression-anchor coverage.
Trade-off: no runtime clamp of a hand-edited out-of-range settings.json value
(VSCode UI validation covers the realistic path).
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.
Summary
The startup CLI preflight capped the
codev --versionprobe at 400ms. On slow environments that probe legitimately exceeds 400ms, so preflight falsely decided the CLI wasmissing— opening theGet started with Codevwalkthrough on every startup and no-op'ing every guarded command, even thoughcodev --versionsucceeds from a terminal in the same window.Fixes #1024
Root Cause
VERSION_TIMEOUT_MS = 400inpackages/vscode/src/preflight/preflight.tswas too tight against real latency sources: remote SSH / VSCode Server, WSL / VM-mediated shells,nvm/fnm/voltashim re-resolution on every spawn, AV / index scanning, and network filesystems. A timed-out probe returns "no version" →decidePreflightreturnsmissing. The intent (kill a hung binary so it can't stall activation) was sound; the value left too little headroom over the ~80-100ms cold local baseline.Fix
DEFAULT_VERSION_TIMEOUT_MS). Comfortable headroom against the realistic slow-env upper bound while staying well under "feels hung" — and the optimistic-pendingpath means commands fire normally during the probe window anyway.codev.cliVersionTimeoutMssetting (number, default 5000, min 100, max 60000) so users on extra-slow infra can override without hand-patchingdist/extension.js. The override survives extension upgrades.[Preflight]line to the OutputChannel on timeout, naming the timeout value and the recovery action, so a slow-env false-negative is diagnosable instead of silently reported as a missing CLI.runCodevVersionnow returns atimedOutflag to distinguish the timeout case from a genuinely absent / broken binary.runCodevVersionprobe + a pureresolveVersionTimeouthelper intopreflight-core.tsso they are unit-testable under vitest without mocking the vscode module. The probe logic itself is unchanged (spawn path not rewritten), just moved.Test Plan
preflight-version-timeout.test.ts):runCodevVersionhonours an explicittimeoutMs(kills a hung probe, reportstimedOut) and the OK/spawn-error paths;resolveVersionTimeoutfalls back to 5000 when the setting is unset and clamps out-of-range values.npm run build).npm test; vscode package vitest: new + existing preflight suites green).Notes
okwell within the bumped budget, and the hung-binary protection still fires — now at the 5000ms ceiling instead of 400ms.pendingwindow, and the Tower-version probe (vscode + tower: detect installed-vs-running Tower version divergence; preflight needs an in-memory version probe, not justcodev --version#983) timeouts are untouched.