mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
fix(adapter-utils): handle closed child stdin safely (#14057)
Handle closed child stdin without losing the child result. Keep each child stdin error handler scoped to that child. Expected EPIPE and stream-destroyed failures no longer crash the host. The child close event remains the source of its exit result. Verification: 2,024 tests passed and 12 live/platform cases skipped across 87 complete adapter-utils suites. The final 140-test server-utils suite passed. The regression reproduces the EPIPE crash on unchanged master. Final qualification: all applicable CI passed and exact-head Greptile Apex scored 5/5 with no unresolved findings. Co-Authored-By: Grzegorz Golaś <193330749+greegorij@users.noreply.github.com> Co-Authored-By: Devin Foley <devinfoley@users.noreply.github.com> Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
2 files changed
+109
-5
No files matched your search
@@ -1,8 +1,10 @@
|
||||
import { spawn } from "node:child_process";
|
||||
import { execFile as execFileCallback, spawn } from "node:child_process";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { promisify } from "node:util";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { CONNECTION_INTENT_AGENT_GUIDANCE } from "@paperclipai/shared";
|
||||
import {
|
||||
@@ -44,6 +46,8 @@ import {
|
||||
WATCHDOG_DEFAULT_MANDATE,
|
||||
} from "./server-utils.js";
|
||||
|
||||
const execFile = promisify(execFileCallback);
|
||||
|
||||
it("keeps Pi draft models empty and preserves saved native Pi settings", () => {
|
||||
expect(normalizePaperclipRunnerAdapterConfig("paperclip_runner", { provider: "acpx", acpxAgent: "pi", model: "" })).toMatchObject({ provider: "acpx", acpxAgent: "pi", model: "" });
|
||||
const saved = { provider: "acpx", acpxAgent: "pi", model: "openrouter/provider/model", piThinkingLevel: "max", acpxPermissionMode: "approve-reads", lifecycleMode: "warm" };
|
||||
@@ -653,6 +657,86 @@ describe("adapter skill snapshots", () => {
|
||||
});
|
||||
|
||||
describe("runChildProcess", () => {
|
||||
it("does not crash when the child closes stdin before a delayed write", async () => {
|
||||
const tempDir = await fs.mkdtemp(
|
||||
path.join(os.tmpdir(), "paperclip-stdin-epipe-"),
|
||||
);
|
||||
const harnessPath = path.join(tempDir, "stdin-epipe-harness.ts");
|
||||
const stdinClosedMarkerPath = path.join(tempDir, "stdin-closed");
|
||||
const releaseChildMarkerPath = path.join(tempDir, "release-child");
|
||||
const serverUtilsUrl = pathToFileURL(
|
||||
path.join(import.meta.dirname, "server-utils.ts"),
|
||||
).href;
|
||||
const tsxCli = path.resolve(
|
||||
import.meta.dirname,
|
||||
"../../../cli/node_modules/tsx/dist/cli.mjs",
|
||||
);
|
||||
|
||||
try {
|
||||
await fs.writeFile(
|
||||
harnessPath,
|
||||
[
|
||||
"import fs from 'node:fs/promises';",
|
||||
`import { runChildProcess } from ${JSON.stringify(serverUtilsUrl)};`,
|
||||
`const stdinClosedMarkerPath = ${JSON.stringify(stdinClosedMarkerPath)};`,
|
||||
`const releaseChildMarkerPath = ${JSON.stringify(releaseChildMarkerPath)};`,
|
||||
"void (async () => {",
|
||||
"const result = await runChildProcess(",
|
||||
" 'stdin-epipe-regression',",
|
||||
" process.execPath,",
|
||||
" [",
|
||||
" '-e',",
|
||||
" \"const fs = require('node:fs'); fs.closeSync(0); fs.writeFileSync(process.argv[1], 'closed'); const deadline = Date.now() + 3000; const timer = setInterval(() => { if (fs.existsSync(process.argv[2])) { clearInterval(timer); process.exit(23); } if (Date.now() >= deadline) { clearInterval(timer); process.exit(24); } }, 10);\",",
|
||||
" stdinClosedMarkerPath,",
|
||||
" releaseChildMarkerPath,",
|
||||
" ],",
|
||||
" {",
|
||||
" cwd: process.cwd(),",
|
||||
" env: {},",
|
||||
" stdin: 'payload after child closed stdin',",
|
||||
" timeoutSec: 5,",
|
||||
" graceSec: 1,",
|
||||
" onLog: async () => {},",
|
||||
" onSpawn: async () => {",
|
||||
" const deadline = Date.now() + 2_000;",
|
||||
" while (Date.now() < deadline) {",
|
||||
" try {",
|
||||
" await fs.access(stdinClosedMarkerPath);",
|
||||
" setTimeout(() => {",
|
||||
" void fs.writeFile(releaseChildMarkerPath, 'release');",
|
||||
" }, 100);",
|
||||
" return;",
|
||||
" } catch {",
|
||||
" await new Promise((resolve) => setTimeout(resolve, 10));",
|
||||
" }",
|
||||
" }",
|
||||
" throw new Error('child did not confirm closing stdin');",
|
||||
" },",
|
||||
" },",
|
||||
");",
|
||||
"if (result.exitCode !== 23) {",
|
||||
" throw new Error(`unexpected exit code: ${result.exitCode}`);",
|
||||
"}",
|
||||
"process.stdout.write(JSON.stringify({ exitCode: result.exitCode, timedOut: result.timedOut }));",
|
||||
"})().catch((error) => {",
|
||||
" console.error(error);",
|
||||
" process.exitCode = 1;",
|
||||
"});",
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const completed = await execFile(process.execPath, [tsxCli, harnessPath], {
|
||||
cwd: process.cwd(),
|
||||
timeout: 10_000,
|
||||
});
|
||||
expect(JSON.parse(completed.stdout)).toEqual({ exitCode: 23, timedOut: false });
|
||||
expect(completed.stderr).not.toMatch(/Unhandled 'error' event|write EPIPE/);
|
||||
} finally {
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not arm a timeout when timeoutSec is 0", async () => {
|
||||
const result = await runChildProcess(
|
||||
randomUUID(),
|
||||
|
||||
@@ -4933,10 +4933,30 @@ export async function runChildProcess(
|
||||
|
||||
const stdin = child.stdin;
|
||||
if (opts.stdin != null && stdin) {
|
||||
void spawnPersistPromise.finally(() => {
|
||||
if (child.killed || stdin.destroyed) return;
|
||||
stdin.write(opts.stdin as string);
|
||||
stdin.end();
|
||||
let stdinFailureHandled = false;
|
||||
const handleStdinFailure = (err: unknown) => {
|
||||
if (!err || stdinFailureHandled) return;
|
||||
stdinFailureHandled = true;
|
||||
const code = (err as NodeJS.ErrnoException).code;
|
||||
if (code === "EPIPE" || code === "ERR_STREAM_DESTROYED") return;
|
||||
onLogError(err, runId, "failed to write child process stdin");
|
||||
};
|
||||
|
||||
stdin.on("error", handleStdinFailure);
|
||||
void spawnPersistPromise.then(() => {
|
||||
if (
|
||||
child.exitCode !== null ||
|
||||
child.signalCode !== null ||
|
||||
child.killed ||
|
||||
stdin.destroyed
|
||||
)
|
||||
return;
|
||||
try {
|
||||
stdin.write(opts.stdin as string, handleStdinFailure);
|
||||
stdin.end();
|
||||
} catch (err) {
|
||||
handleStdinFailure(err);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user