Skip to content
Merged
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
7 changes: 5 additions & 2 deletions apps/server/src/orchestration-v2/Adapters/PiRpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
import { HostProcessPlatform } from "@t3tools/shared/hostProcess";
import { resolveSpawnCommand } from "@t3tools/shared/shell";

import { signalProcessGroup } from "../../process/processGroup.ts";

export class PiRpcError extends Schema.TaggedError<PiRpcError>()("PiRpcError", {
operation: Schema.String,
detail: Schema.optional(Schema.String),
Expand Down Expand Up @@ -238,7 +240,7 @@ export const makePiRpcConnection = Effect.fnUntraced(function* (options: PiRpcSp
if (platform === "win32") {
process.kill(Number(child.pid), signal);
} else {
process.kill(-Number(child.pid), signal);
signalProcessGroup(Number(child.pid), signal);
}
return true;
} catch {
Expand All @@ -250,7 +252,8 @@ export const makePiRpcConnection = Effect.fnUntraced(function* (options: PiRpcSp
const hasExited = (): boolean => {
if (childExited) return true;
try {
process.kill(platform === "win32" ? Number(child.pid) : -Number(child.pid), 0);
if (platform === "win32") process.kill(Number(child.pid), 0);
else signalProcessGroup(Number(child.pid), 0);
return false;
} catch {
return true;
Expand Down
45 changes: 45 additions & 0 deletions apps/server/src/process/processGroup.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// @effect-diagnostics nodeBuiltinImport:off
import * as NodeChildProcess from "node:child_process";

import { describe, expect, it } from "@effect/vitest";
import { HostProcessPlatform } from "@t3tools/shared/hostProcess";

import { signalProcessGroup } from "./processGroup.ts";

const errorCode = (run: () => void) => {
try {
run();
return undefined;
} catch (cause) {
return (cause as NodeJS.ErrnoException).code;
}
};

describe.skipIf(HostProcessPlatform.defaultValue() === "win32")("signalProcessGroup", () => {
// Signal 0 only probes, so this is safe to run unguarded: `kill(-1, 0)` and
// `kill(0, 0)` succeed, which is how a fake spawner's pid 1 became
// `kill(-1, SIGKILL)` and took down every process the user owned.
it("never reaches this server's group or every process the user owns", () => {
for (const pid of [1, 0, -1, Number.NaN, 1.5]) {
expect(
errorCode(() => signalProcessGroup(pid, 0)),
`pid ${pid}`,
).toBe("ESRCH");
}
});

it("signals a spawned process group", async () => {
const child = NodeChildProcess.spawn("/bin/sh", ["-c", "sleep 600 & wait"], {
detached: true,
stdio: "ignore",
});
const exited = new Promise<NodeJS.Signals | null>((resolve) =>
child.once("exit", (_code, signal) => resolve(signal)),
);
const pid = child.pid!;

expect(errorCode(() => signalProcessGroup(pid, 0))).toBeUndefined();
signalProcessGroup(pid, "SIGKILL");
expect(await exited).toBe("SIGKILL");
});
});
16 changes: 16 additions & 0 deletions apps/server/src/process/processGroup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* Signals a POSIX process group this server spawned, as `process.kill(-pid)`.
*
* `kill(0)` signals this server's own group and `kill(-1)` every process the
* user owns, and a fake spawner in tests reports pid 1. So a pid of 0 or 1, or
* one that is not an integer, fails with ESRCH like a group that has already
* exited, and callers keep their existing handling.
*/
export function signalProcessGroup(pid: number, signal: NodeJS.Signals | 0): void {
if (!Number.isSafeInteger(pid) || pid <= 1) {
throw Object.assign(new Error(`kill ESRCH: not a spawned process group (${pid})`), {
code: "ESRCH",
});
}
process.kill(-pid, signal);
}
5 changes: 3 additions & 2 deletions apps/server/src/provider/OpenCodeServerLedger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as Schema from "effect/Schema";
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";

import * as ServerConfig from "../config.ts";
import { signalProcessGroup } from "../process/processGroup.ts";

const ProcessIdentity = Schema.Struct({ pid: Schema.Int, startTime: Schema.String });
type ProcessIdentity = typeof ProcessIdentity.Type;
Expand Down Expand Up @@ -91,15 +92,15 @@ const parseDarwinPs = (output: string): ReadonlyArray<ObservedProcess> =>

const signalGroup = (pgid: number, signal: NodeJS.Signals) => {
try {
process.kill(-pgid, signal);
signalProcessGroup(pgid, signal);
} catch {
// The group may already be gone.
}
};

const groupExists = (pgid: number) => {
try {
process.kill(-pgid, 0);
signalProcessGroup(pgid, 0);
return true;
} catch (cause) {
return (cause as NodeJS.ErrnoException | undefined)?.code !== "ESRCH";
Expand Down
3 changes: 2 additions & 1 deletion apps/server/src/provider/acp/AcpSessionRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import type * as EffectAcpProtocol from "effect-acp/protocol";
import { resolveSpawnCommand } from "@t3tools/shared/shell";
import { HostProcessPlatform } from "@t3tools/shared/hostProcess";

import { signalProcessGroup } from "../../process/processGroup.ts";
import { appendAcpStderrTail, sanitizeAcpStderrExcerpt } from "./AcpStderr.ts";
import {
collectSessionConfigOptionValues,
Expand Down Expand Up @@ -1647,7 +1648,7 @@ export const make = (
const signalOwnedProcessGroup = (signal: NodeJS.Signals) =>
Effect.try({
try: () => {
process.kill(-Number(child.pid), signal);
signalProcessGroup(Number(child.pid), signal);
return true;
},
catch: (cause) =>
Expand Down
5 changes: 3 additions & 2 deletions apps/server/src/provider/opencodeRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import * as Schema from "effect/Schema";
import * as Stream from "effect/Stream";
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";

import { signalProcessGroup } from "../process/processGroup.ts";
import { isWindowsCommandNotFound } from "../processRunner.ts";
import * as OpenCodeServerLedger from "./OpenCodeServerLedger.ts";
import { collectStreamAsString } from "./providerSnapshot.ts";
Expand Down Expand Up @@ -609,7 +610,7 @@ const makeOpenCodeRuntime = Effect.gen(function* () {
? child.kill({ killSignal: "SIGKILL" }).pipe(Effect.asVoid)
: Effect.sync(() => {
try {
process.kill(-Number(child.pid), "SIGKILL");
signalProcessGroup(Number(child.pid), "SIGKILL");
} catch {
// The command and its process group may already have exited.
}
Expand Down Expand Up @@ -731,7 +732,7 @@ const makeOpenCodeRuntime = Effect.gen(function* () {
? child.kill({ killSignal: signal, forceKillAfter: "1 second" }).pipe(Effect.asVoid)
: Effect.sync(() => {
try {
process.kill(-Number(child.pid), signal);
signalProcessGroup(Number(child.pid), signal);
} catch {
// The direct child may already have exited after starting the
// server; the process group kill is best-effort cleanup for
Expand Down
Loading