fix: validate configured OpenCode binary (#1120)
* Validate configured OpenCode binary * fix: keep WSL OpenCode startup failures retryable Avoids misclassifying transient WSL resolution failures as invalid binary config Adds regression coverage for WSL strict-mode handling Cleans up temporary test directories Fix for #1119 issue
This commit is contained in:
committed by
GitHub
parent
a02f0cd7c7
commit
b18886598c
@@ -875,6 +875,51 @@ export const createOpenCodeEnvRuntime = (deps) => {
|
||||
}
|
||||
};
|
||||
|
||||
const isMacOpenCodeAppBundlePath = (candidate) => {
|
||||
if (process.platform !== 'darwin' || typeof candidate !== 'string') {
|
||||
return false;
|
||||
}
|
||||
return /\/OpenCode\.app\/Contents\/MacOS\/(?:OpenCode|opencode-cli)$/i.test(candidate);
|
||||
};
|
||||
|
||||
const createConfiguredOpencodeBinaryError = (raw, normalized) => {
|
||||
const configured = typeof raw === 'string' ? raw.trim() : '';
|
||||
const candidate = typeof normalized === 'string' && normalized.trim().length > 0 ? normalized.trim() : configured;
|
||||
const messageSuffix = 'OpenChamber needs the standalone opencode CLI. Install it and set settings.opencodeBinary to the CLI path, for example ~/.opencode/bin/opencode, or leave the setting empty to use PATH lookup.';
|
||||
const error = (() => {
|
||||
if (isMacOpenCodeAppBundlePath(candidate) || isMacOpenCodeAppBundlePath(configured)) {
|
||||
return new Error(`Configured OpenCode binary points at the macOS desktop app bundle, not the CLI: ${candidate}. ${messageSuffix}`);
|
||||
}
|
||||
|
||||
try {
|
||||
const configuredStat = fs.statSync(configured);
|
||||
if (configuredStat.isDirectory()) {
|
||||
return new Error(`Configured OpenCode binary directory does not contain an executable ${process.platform === 'win32' ? 'opencode.exe' : 'opencode'}: ${configured}. ${messageSuffix}`);
|
||||
}
|
||||
} catch {
|
||||
}
|
||||
|
||||
try {
|
||||
const stat = fs.statSync(candidate);
|
||||
if (stat.isDirectory()) {
|
||||
return new Error(`Configured OpenCode binary directory does not contain an executable ${process.platform === 'win32' ? 'opencode.exe' : 'opencode'}: ${candidate}. ${messageSuffix}`);
|
||||
}
|
||||
if (!stat.isFile()) {
|
||||
return new Error(`Configured OpenCode binary is not a file: ${candidate}. ${messageSuffix}`);
|
||||
}
|
||||
return new Error(`Configured OpenCode binary is not executable: ${candidate}. ${messageSuffix}`);
|
||||
} catch {
|
||||
return new Error(`Configured OpenCode binary not found: ${candidate}. ${messageSuffix}`);
|
||||
}
|
||||
})();
|
||||
error.code = 'OPENCODE_BINARY_INVALID';
|
||||
return error;
|
||||
};
|
||||
|
||||
const createConfiguredWslOpencodeError = (raw) => new Error(
|
||||
`Configured settings.opencodeBinary uses WSL but OpenChamber could not resolve a WSL OpenCode command: ${raw}. Ensure WSL is available and opencode is installed in the configured distro.`
|
||||
);
|
||||
|
||||
const normalizeOpencodeBinarySetting = (raw) => {
|
||||
if (typeof raw !== 'string') {
|
||||
return null;
|
||||
@@ -896,7 +941,8 @@ export const createOpenCodeEnvRuntime = (deps) => {
|
||||
return trimmed;
|
||||
};
|
||||
|
||||
const applyOpencodeBinaryFromSettings = async () => {
|
||||
const applyOpencodeBinaryFromSettings = async (options = {}) => {
|
||||
const strict = options?.strict === true;
|
||||
try {
|
||||
const settings = await readSettingsFromDiskMigrated();
|
||||
if (!settings || typeof settings !== 'object') {
|
||||
@@ -932,6 +978,9 @@ export const createOpenCodeEnvRuntime = (deps) => {
|
||||
if (applied) {
|
||||
return applied;
|
||||
}
|
||||
if (strict) {
|
||||
throw createConfiguredWslOpencodeError(raw);
|
||||
}
|
||||
}
|
||||
|
||||
if (process.platform === 'win32' && (isWslExecutableValue(raw) || isWslExecutableValue(normalized || ''))) {
|
||||
@@ -945,9 +994,12 @@ export const createOpenCodeEnvRuntime = (deps) => {
|
||||
if (applied) {
|
||||
return applied;
|
||||
}
|
||||
if (strict) {
|
||||
throw createConfiguredWslOpencodeError(raw);
|
||||
}
|
||||
}
|
||||
|
||||
if (normalized && isExecutable(normalized)) {
|
||||
if (normalized && isExecutable(normalized) && !isMacOpenCodeAppBundlePath(normalized)) {
|
||||
clearWslOpencodeResolution();
|
||||
process.env.OPENCODE_BINARY = normalized;
|
||||
prependToPath(path.dirname(normalized));
|
||||
@@ -958,9 +1010,15 @@ export const createOpenCodeEnvRuntime = (deps) => {
|
||||
}
|
||||
|
||||
if (raw) {
|
||||
if (strict) {
|
||||
throw createConfiguredOpencodeBinaryError(raw, normalized);
|
||||
}
|
||||
console.warn(`Configured settings.opencodeBinary is not executable: ${raw}`);
|
||||
}
|
||||
} catch {
|
||||
} catch (error) {
|
||||
if (strict) {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
import { createOpenCodeEnvRuntime } from './env-runtime.js';
|
||||
|
||||
const originalOpencodeBinary = process.env.OPENCODE_BINARY;
|
||||
const originalPlatform = process.platform;
|
||||
const tempDirs = [];
|
||||
|
||||
const createTempDir = (prefix) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
|
||||
tempDirs.push(dir);
|
||||
return dir;
|
||||
};
|
||||
|
||||
const setPlatform = (platform) => {
|
||||
Object.defineProperty(process, 'platform', {
|
||||
value: platform,
|
||||
});
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(process, 'platform', {
|
||||
value: originalPlatform,
|
||||
});
|
||||
|
||||
for (const dir of tempDirs.splice(0)) {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
if (typeof originalOpencodeBinary === 'string') {
|
||||
process.env.OPENCODE_BINARY = originalOpencodeBinary;
|
||||
return;
|
||||
}
|
||||
delete process.env.OPENCODE_BINARY;
|
||||
});
|
||||
|
||||
const createRuntime = (settings) => {
|
||||
const state = {
|
||||
cachedLoginShellEnvSnapshot: null,
|
||||
resolvedOpencodeBinary: null,
|
||||
resolvedOpencodeBinarySource: null,
|
||||
useWslForOpencode: false,
|
||||
resolvedWslBinary: null,
|
||||
resolvedWslOpencodePath: null,
|
||||
resolvedWslDistro: null,
|
||||
resolvedNodeBinary: null,
|
||||
resolvedBunBinary: null,
|
||||
managedOpenCodeShellEnvSnapshot: null,
|
||||
};
|
||||
|
||||
const runtime = createOpenCodeEnvRuntime({
|
||||
state,
|
||||
normalizeDirectoryPath: (value) => value,
|
||||
readSettingsFromDiskMigrated: async () => settings,
|
||||
ENV_CONFIGURED_OPENCODE_WSL_DISTRO: null,
|
||||
});
|
||||
|
||||
return { runtime, state };
|
||||
};
|
||||
|
||||
describe('OpenCode env runtime', () => {
|
||||
it('throws a specific error for a missing configured OpenCode binary in strict mode', async () => {
|
||||
const { runtime } = createRuntime({ opencodeBinary: '/missing/opencode' });
|
||||
|
||||
await expect(runtime.applyOpencodeBinaryFromSettings({ strict: true })).rejects.toMatchObject({
|
||||
code: 'OPENCODE_BINARY_INVALID',
|
||||
message: expect.stringContaining('Configured OpenCode binary not found: /missing/opencode'),
|
||||
});
|
||||
});
|
||||
|
||||
it('throws a specific error for a configured directory without an executable CLI in strict mode', async () => {
|
||||
const dir = createTempDir('openchamber-opencode-dir-');
|
||||
const { runtime } = createRuntime({ opencodeBinary: dir });
|
||||
|
||||
await expect(runtime.applyOpencodeBinaryFromSettings({ strict: true })).rejects.toMatchObject({
|
||||
code: 'OPENCODE_BINARY_INVALID',
|
||||
message: expect.stringContaining('Configured OpenCode binary directory does not contain an executable'),
|
||||
});
|
||||
});
|
||||
|
||||
it('applies a valid configured executable OpenCode binary', async () => {
|
||||
const dir = createTempDir('openchamber-opencode-bin-');
|
||||
const binary = path.join(dir, 'opencode');
|
||||
fs.writeFileSync(binary, '#!/bin/sh\nexit 0\n');
|
||||
fs.chmodSync(binary, 0o755);
|
||||
const { runtime, state } = createRuntime({ opencodeBinary: binary });
|
||||
|
||||
await expect(runtime.applyOpencodeBinaryFromSettings({ strict: true })).resolves.toBe(binary);
|
||||
expect(process.env.OPENCODE_BINARY).toBe(binary);
|
||||
expect(state.resolvedOpencodeBinary).toBe(binary);
|
||||
expect(state.resolvedOpencodeBinarySource).toBe('settings');
|
||||
});
|
||||
|
||||
it.runIf(process.platform === 'darwin')('rejects known macOS OpenCode app bundle executable paths', async () => {
|
||||
const { runtime } = createRuntime({ opencodeBinary: '/Applications/OpenCode.app/Contents/MacOS/OpenCode' });
|
||||
|
||||
await expect(runtime.applyOpencodeBinaryFromSettings({ strict: true })).rejects.toMatchObject({
|
||||
code: 'OPENCODE_BINARY_INVALID',
|
||||
message: expect.stringContaining('macOS desktop app bundle'),
|
||||
});
|
||||
});
|
||||
|
||||
it('does not classify failed WSL resolution as an invalid configured binary in strict mode', async () => {
|
||||
setPlatform('win32');
|
||||
const { runtime } = createRuntime({ opencodeBinary: 'wsl:/usr/local/bin/opencode' });
|
||||
|
||||
await expect(runtime.applyOpencodeBinaryFromSettings({ strict: true })).rejects.toThrow('uses WSL');
|
||||
await runtime.applyOpencodeBinaryFromSettings({ strict: true }).catch((error) => {
|
||||
expect(error.code).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -289,7 +289,10 @@ export const createOpenCodeLifecycleRuntime = (deps) => {
|
||||
|
||||
const onExit = (code, signal) => {
|
||||
const reason = signal ? `signal ${signal}` : `code ${code}`;
|
||||
finish(reject, new Error(`OpenCode exited with ${reason}. ${formatCapturedOutput({ stdout, stderr })}`));
|
||||
const appBundleHint = process.platform === 'darwin' && /\/OpenCode\.app\/Contents\/MacOS\/(?:OpenCode|opencode-cli)$/i.test(binary)
|
||||
? ' The configured binary appears to point at the macOS desktop app bundle; OpenChamber needs the standalone opencode CLI.'
|
||||
: '';
|
||||
finish(reject, new Error(`OpenCode process exited before serving with ${reason}. Binary used: ${binary}.${appBundleHint} ${formatCapturedOutput({ stdout, stderr })}`));
|
||||
};
|
||||
|
||||
const onError = (error) => {
|
||||
@@ -425,7 +428,7 @@ export const createOpenCodeLifecycleRuntime = (deps) => {
|
||||
: `Starting OpenCode on allocated port ${spawnPort}...`
|
||||
);
|
||||
|
||||
await applyOpencodeBinaryFromSettings();
|
||||
await applyOpencodeBinaryFromSettings({ strict: true });
|
||||
ensureOpencodeCliEnv();
|
||||
const openCodePassword = await ensureLocalOpenCodeServerPassword({ rotateManaged: true });
|
||||
const envPath = typeof buildManagedOpenCodePath === 'function'
|
||||
@@ -493,6 +496,9 @@ export const createOpenCodeLifecycleRuntime = (deps) => {
|
||||
return await startOpenCodeOnce();
|
||||
} catch (error) {
|
||||
lastError = error;
|
||||
if (error?.code === 'OPENCODE_BINARY_INVALID') {
|
||||
break;
|
||||
}
|
||||
if (attempt >= START_OPEN_CODE_MAX_ATTEMPTS) {
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -170,7 +170,7 @@ describe('OpenCode lifecycle', () => {
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it('reports the exit signal when managed OpenCode exits before becoming ready', async () => {
|
||||
it('reports the binary when managed OpenCode exits before becoming ready', async () => {
|
||||
delete process.env.OPENCODE_BINARY;
|
||||
const firstChild = createMockChild();
|
||||
const secondChild = createMockChild();
|
||||
@@ -189,10 +189,26 @@ describe('OpenCode lifecycle', () => {
|
||||
|
||||
const runtime = createRuntime();
|
||||
|
||||
await expect(runtime.startOpenCode()).rejects.toThrow('OpenCode exited with signal SIGTERM. No stdout/stderr captured');
|
||||
await expect(runtime.startOpenCode()).rejects.toThrow('OpenCode process exited before serving with signal SIGTERM. Binary used: opencode. No stdout/stderr captured');
|
||||
expect(spawnMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('does not retry managed startup when the configured OpenCode binary is invalid', async () => {
|
||||
delete process.env.OPENCODE_BINARY;
|
||||
const error = new Error('Configured OpenCode binary not found: /missing/opencode');
|
||||
error.code = 'OPENCODE_BINARY_INVALID';
|
||||
const applyOpencodeBinaryFromSettings = vi.fn(async () => {
|
||||
throw error;
|
||||
});
|
||||
|
||||
const runtime = createRuntime({ applyOpencodeBinaryFromSettings });
|
||||
|
||||
await expect(runtime.startOpenCode()).rejects.toThrow('Configured OpenCode binary not found: /missing/opencode');
|
||||
expect(applyOpencodeBinaryFromSettings).toHaveBeenCalledTimes(1);
|
||||
expect(applyOpencodeBinaryFromSettings).toHaveBeenCalledWith({ strict: true });
|
||||
expect(spawnMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('retries managed OpenCode startup once after a pre-ready exit', async () => {
|
||||
delete process.env.OPENCODE_BINARY;
|
||||
const firstChild = createMockChild();
|
||||
|
||||
Reference in New Issue
Block a user