fix(opencode): preflight a shell-free launch plan#3927
Conversation
There was a problem hiding this comment.
Pull request overview
This PR formalizes OpenCode’s two-stage launch plan (PowerShell host + inner opencode resolution on Windows) into the shared runtime-availability contract, then reuses that canonical probe across the chat route’s early plan, immediate pre-spawn recheck, and /api/harnesses availability reporting—while preserving stdin JSON argv transport and existing post-launch error classification.
Changes:
- Add
openCodeAvailabilityProbe()to encode the exact launch plan + spawn env into aRuntimeAvailabilityProbewithout consulting user prompt contents. - Reuse the canonical probe in
/api/harnessesand/api/chat/sendfor both the early “plan” gate and the just-before-spawn recheck. - Add unit + route-level integration coverage to prove metacharacter prompts remain data and started-CLI failures keep their existing classification.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/opencode-bin.ts | Adds openCodeAvailabilityProbe() to map OpenCode’s launch plan/env into the shared availability model. |
| src/lib/opencode-bin.test.ts | Adds focused unit coverage for missing/ready/unlaunchable/probe_failed classification and prompt-independence. |
| src/app/api/harnesses/route.ts | Switches OpenCode availability to the canonical probe so /api/harnesses matches the real launch plan/env. |
| src/app/api/chat/send/route.ts | Uses the canonical probe for early planning and the immediate pre-spawn recheck (preventing drift). |
| src/app/api/chat/send/route-opencode-preflight.integration.test.ts | New integration test exercising the real route: metacharacter prompt remains data; started CLI failure stays post-launch classified/persisted. |
| src/app/api/chat/send/harness-routing-opencode.test.ts | Updates route-shape assertions to pin canonical-probe usage and the pre-spawn recheck behavior. |
| scripts/run-tests.mjs | Wires the new integration test into the API suite and alias-loader set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d78c65a to
69e93f2
Compare
69e93f2 to
38f197f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/lib/coven-bin.ts:449
- windowsShimTargetFromFile() currently swallows all read/exists errors and returns null, which means permission/IO failures (e.g. EACCES when reading a PATH-winning .cmd shim) get misclassified as
unresolvedWindowsShiminstead ofresolutionFailed→runtime_probe_failed. That loses the intended “could not verify” remediation and incorrectly tells users to reinstall. Consider rethrowing non-missing errors from the shim parser so OpenCode’s resolver can fail closed withresolutionFailedand the shared availability contract can surface the probe_failed copy.
export function windowsShimLaunchCommandForBinary(
binary: string,
platform: NodeJS.Platform = process.platform,
): CovenLaunchCommand {
if (platform !== "win32" || !/\.(cmd|bat)$/i.test(binary)) {
c4ab3a1 to
d4fd287
Compare
Co-authored-by: Timothy Wayne Gregg <5861166+CompleteDotTech@users.noreply.github.com>
d4fd287 to
33d3e35
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/app/api/chat/send/chat-send-capabilities.ts:621
- openCodeRunSupportsModel() checks isOpenCodeLaunchSpawnable() but doesn’t gate on launch.requiredFiles. On Windows, a resolved
node <script>launch can be “spawnable” yet still broken if the fixed script target has been removed/unreadable, and this code would still spawn (potentially returning false due to an unrelated failure rather than lack of --model support). Consider preflighting with evaluateRuntimeAvailability(openCodeAvailabilityProbe(launch, env)) and returning false when it is not ready, to keep capability probes passive/fail-closed when the launch plan is incomplete.
export function openCodeRunSupportsModel(): Promise<boolean> {
const env = openCodeSpawnEnv();
const launch = openCodeLaunch(["run", "--help"], process.platform, env);
if (!isOpenCodeLaunchSpawnable(launch)) return Promise.resolve(false);
return (openCodeModelFlagProbe ??= probeHelp(
launch.command,
launch.args,
(help) => /(^|\s)--model(?![\w-])/m.test(help),
env,
));
Summary
/api/harnessesContext
PR #3907 established the initial OpenCode preflight on
main. Its Windows matrix then proved that the PowerShell transport was lossy at its final native-process hop: PowerShell reserialized the complete argv when invoking OpenCode. Keeping the prompt in argv also exposed valid large or multiline input to platform command-line limits and quoting rules.This follow-up reads a standard npm shim only to prove its target, calls Node
spawn()on that native target with an option-only argv array, and sends the prompt through an observed UTF-8 stdin pipe. No prompt is evaluated as command syntax, noshell: truepath is introduced, and an unproven launch plan fails closed.Verification
Exact signed head:
09f51f3a86d1151b70b72f15b472b1fb76798a30Built directly on
origin/mainatda2cf1046714c445a3bc8e8d4e06820ae5f660f0; final feature diff is 18 files.pnpm test:app— 921/921 test files passedpnpm test:api— 278/278 test files passedpnpm test:conformance— 3/3 local files passed; 2 explicit Windows-host scenarios run in CIpnpm typecheckpnpm lintpnpm check:tests-wired— 1,269/1,269 files (1 allowlisted)git diff --checkCredit
Builds on the OpenCode preflight contributed by @CompleteDotTech in #3907. The signed head and final squash preserve Timothy Wayne Gregg as a co-author.