Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions apps/server/src/provider/Layers/OpenCodeProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const runtimeMock = {
runVersionError: null as Error | null,
versionStdout: DEFAULT_VERSION_STDOUT,
inventoryError: null as Error | null,
closeError: null as Error | null,
closeCalls: 0,
inventory: {
providerList: { connected: [] as string[], all: [] as unknown[], default: {} },
Expand All @@ -44,6 +45,7 @@ const runtimeMock = {
this.state.runVersionError = null;
this.state.versionStdout = DEFAULT_VERSION_STDOUT;
this.state.inventoryError = null;
this.state.closeError = null;
this.state.closeCalls = 0;
this.state.inventory = {
providerList: { connected: [], all: [] as unknown[], default: {} },
Expand All @@ -64,6 +66,9 @@ const OpenCodeRuntimeTestDouble: OpenCodeRuntimeShape = {
yield* Effect.addFinalizer(() =>
Effect.sync(() => {
runtimeMock.state.closeCalls += 1;
if (runtimeMock.state.closeError) {
throw runtimeMock.state.closeError;
}
}),
);
}
Expand Down Expand Up @@ -201,6 +206,41 @@ it.layer(testLayer)("checkOpenCodeProviderStatus", (it) => {
assert.equal(runtimeMock.state.closeCalls, 1);
}),
);

it.effect("preserves a successful provider refresh when probe teardown throws", () =>
Effect.gen(function* () {
runtimeMock.state.inventory = {
providerList: {
connected: ["openai"],
all: [
{
id: "openai",
name: "OpenAI",
models: {
"gpt-5": {
id: "gpt-5",
name: "GPT-5",
variants: {},
},
},
},
],
default: {},
},
agents: [],
};
runtimeMock.state.closeError = new Error("close failed");

const snapshot = yield* checkOpenCodeProviderStatus(makeOpenCodeSettings(), process.cwd());

assert.equal(runtimeMock.state.closeCalls, 1);
assert.equal(snapshot.status, "ready");
assert.equal(
snapshot.models.some((model) => model.slug === "openai/gpt-5"),
true,
);
}),
);
});

it.layer(testLayer)("checkOpenCodeProviderStatus with configured server URL", (it) => {
Expand Down
61 changes: 29 additions & 32 deletions apps/server/src/provider/Layers/OpenCodeProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
openCodeRuntimeErrorDetail,
type OpenCodeInventory,
} from "../opencodeRuntime.ts";
import { scopedSafeTeardown } from "./scopedSafeTeardown.ts";
import type { Agent, ProviderListResponse } from "@opencode-ai/sdk/v2";

const PROVIDER = ProviderDriverKind.make("opencode");
Expand Down Expand Up @@ -406,38 +407,34 @@ export const checkOpenCodeProviderStatus = Effect.fn("checkOpenCodeProviderStatu
}

const inventoryExit = yield* Effect.exit(
Effect.scoped(
Effect.gen(function* () {
const server = yield* openCodeRuntime
.connectToOpenCodeServer({
binaryPath: openCodeSettings.binaryPath,
serverUrl: openCodeSettings.serverUrl,
environment,
})
.pipe(
Effect.mapError(
(cause) =>
new OpenCodeProbeError({ cause, detail: openCodeRuntimeErrorDetail(cause) }),
),
);
return yield* openCodeRuntime
.loadOpenCodeInventory(
openCodeRuntime.createOpenCodeSdkClient({
baseUrl: server.url,
directory: cwd,
...(isExternalServer && openCodeSettings.serverPassword
? { serverPassword: openCodeSettings.serverPassword }
: {}),
}),
)
.pipe(
Effect.mapError(
(cause) =>
new OpenCodeProbeError({ cause, detail: openCodeRuntimeErrorDetail(cause) }),
),
);
}),
),
Effect.gen(function* () {
const server = yield* openCodeRuntime
.connectToOpenCodeServer({
binaryPath: openCodeSettings.binaryPath,
serverUrl: openCodeSettings.serverUrl,
environment,
})
.pipe(
Effect.mapError(
(cause) => new OpenCodeProbeError({ cause, detail: openCodeRuntimeErrorDetail(cause) }),
),
);
return yield* openCodeRuntime
.loadOpenCodeInventory(
openCodeRuntime.createOpenCodeSdkClient({
baseUrl: server.url,
directory: cwd,
...(isExternalServer && openCodeSettings.serverPassword
? { serverPassword: openCodeSettings.serverPassword }
: {}),
}),
)
.pipe(
Effect.mapError(
(cause) => new OpenCodeProbeError({ cause, detail: openCodeRuntimeErrorDetail(cause) }),
),
);
}).pipe(scopedSafeTeardown("opencode-provider-probe")),
);
if (inventoryExit._tag === "Failure") {
return fallback(Cause.squash(inventoryExit.cause), version);
Expand Down
119 changes: 117 additions & 2 deletions apps/server/src/provider/opencodeRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,100 @@ function ensureRuntimeError(
: new OpenCodeRuntimeError({ operation, detail, cause });
}

function commandBasename(commandPath: string): string {
return commandPath.split(/[\\/]/).at(-1) ?? commandPath;
}

function isMatchingOpenCodeServeCommand(input: {
readonly command: string;
readonly binaryPath: string;
readonly hostname: string;
readonly port: number;
}): boolean {
const executable = commandBasename(input.binaryPath);
const command = input.command;
return (
command.includes(executable) &&
/\bserve\b/.test(command) &&
command.includes(`--hostname=${input.hostname}`) &&
command.includes(`--port=${input.port}`)
);
}

const makeOpenCodeRuntime = Effect.gen(function* () {
const spawner = yield* ChildProcessSpawner.ChildProcessSpawner;
const netService = yield* NetService.NetService;

const listProcessCommands: Effect.Effect<
ReadonlyArray<{ readonly pid: number; readonly command: string }>
> = Effect.gen(function* () {
const child = yield* spawner.spawn(ChildProcess.make("ps", ["-axo", "pid=,command="]));
const [stdout, code] = yield* Effect.all(
[collectStreamAsString(child.stdout), child.exitCode],
{
concurrency: "unbounded",
},
);
if (Number(code) !== 0) {
return [];
}
return stdout
.split("\n")
.flatMap((line) => {
const match = line.trim().match(/^(\d+)\s+(.+)$/);
const command = match?.[2];
if (!match || command === undefined) {
return [];
}
return [{ pid: Number(match[1]), command }];
})
.filter((entry) => Number.isInteger(entry.pid) && entry.pid > 0);
}).pipe(
Effect.scoped,
Effect.catch(() => Effect.succeed([])),
);

const terminateMatchingOpenCodeServeProcesses = (input: {
readonly binaryPath: string;
readonly hostname: string;
readonly port: number;
readonly signal: NodeJS.Signals;
}): Effect.Effect<void> => {
if (process.platform === "win32") {
return Effect.void;
}

return listProcessCommands.pipe(
Effect.flatMap((processes) =>
Effect.forEach(
processes,
(entry) => {
if (
entry.pid === process.pid ||
!isMatchingOpenCodeServeCommand({
command: entry.command,
binaryPath: input.binaryPath,
hostname: input.hostname,
port: input.port,
})
) {
return Effect.void;
}
return Effect.sync(() => {
try {
process.kill(entry.pid, input.signal);
} catch {
// The process may have exited between `ps` and `kill`.
}
});
},
{ discard: true },
),
),
Effect.ignore,
);
};

const runOpenCodeCommand: OpenCodeRuntimeShape["runOpenCodeCommand"] = (input) =>
Effect.gen(function* () {
const child = yield* spawner.spawn(
Expand Down Expand Up @@ -370,9 +460,34 @@ const makeOpenCodeRuntime = Effect.gen(function* () {
// any serve process left in that group.
}
});
const terminateChild = killOpenCodeProcessGroup("SIGTERM").pipe(
const killDirectChild = (signal: NodeJS.Signals) =>
child.kill({ killSignal: signal, forceKillAfter: "1 second" }).pipe(Effect.asVoid);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failable killDirectChild short-circuits SIGKILL cleanup phase

High Severity

killDirectChild uses Effect.asVoid (which only discards the success value) instead of Effect.ignore (which also suppresses errors). Since child.kill from @effect/platform is a failable effect, it can produce a PlatformError (e.g., when the child already exited). When it fails inside Effect.all, the entire SIGTERM phase fails, which short-circuits Effect.andThen so the SIGKILL phase never runs. The trailing Effect.ignore on the pipeline only suppresses the overall error — it doesn't undo the short-circuit. This means killOpenCodeProcessGroup("SIGKILL") and killMatchingServeProcesses("SIGKILL") are skipped, potentially leaving orphaned serve processes alive — the exact problem this PR aims to fix.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b538723. Configure here.

const killMatchingServeProcesses = (signal: NodeJS.Signals) =>
terminateMatchingOpenCodeServeProcesses({
binaryPath: input.binaryPath,
hostname,
port,
signal,
});
const terminateChild = Effect.all(
[
killOpenCodeProcessGroup("SIGTERM"),
killDirectChild("SIGTERM"),
killMatchingServeProcesses("SIGTERM"),
],
{ concurrency: "unbounded", discard: true },
).pipe(
Effect.andThen(Effect.sleep("1 second")),
Effect.andThen(killOpenCodeProcessGroup("SIGKILL")),
Effect.andThen(
Effect.all(
[
killOpenCodeProcessGroup("SIGKILL"),
killDirectChild("SIGKILL"),
killMatchingServeProcesses("SIGKILL"),
],
{ concurrency: "unbounded", discard: true },
),
),
Effect.ignore,
);
yield* Scope.addFinalizer(runtimeScope, terminateChild);
Expand Down
Loading