fix: update bin/codex.js so it listens for exit on the child process (#1590)
When Codex CLI is installed via `npm`, we use a `.js` wrapper script to launch the Rust binary. - Previously, we were not listening for signals to ensure that killing the Node.js process would also kill the underlying Rust process. - We also did not have a proper `exit` handler in place on the child process to ensure we exited from the Node.js process. This PR fixes these things and hopefully addresses https://github.com/openai/codex/issues/1570. This also adds logic so that Windows falls back to the TypeScript CLI again, which should address https://github.com/openai/codex/issues/1573.
This commit is contained in:
@@ -15,7 +15,6 @@
|
|||||||
* current platform / architecture, an error is thrown.
|
* current platform / architecture, an error is thrown.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { spawnSync } from "child_process";
|
|
||||||
import fs from "fs";
|
import fs from "fs";
|
||||||
import path from "path";
|
import path from "path";
|
||||||
import { fileURLToPath, pathToFileURL } from "url";
|
import { fileURLToPath, pathToFileURL } from "url";
|
||||||
@@ -35,7 +34,7 @@ const wantsNative = fs.existsSync(path.join(__dirname, "use-native")) ||
|
|||||||
: false);
|
: false);
|
||||||
|
|
||||||
// Try native binary if requested.
|
// Try native binary if requested.
|
||||||
if (wantsNative) {
|
if (wantsNative && process.platform !== 'win32') {
|
||||||
const { platform, arch } = process;
|
const { platform, arch } = process;
|
||||||
|
|
||||||
let targetTriple = null;
|
let targetTriple = null;
|
||||||
@@ -74,22 +73,76 @@ if (wantsNative) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const binaryPath = path.join(__dirname, "..", "bin", `codex-${targetTriple}`);
|
const binaryPath = path.join(__dirname, "..", "bin", `codex-${targetTriple}`);
|
||||||
const result = spawnSync(binaryPath, process.argv.slice(2), {
|
|
||||||
|
// Use an asynchronous spawn instead of spawnSync so that Node is able to
|
||||||
|
// respond to signals (e.g. Ctrl-C / SIGINT) while the native binary is
|
||||||
|
// executing. This allows us to forward those signals to the child process
|
||||||
|
// and guarantees that when either the child terminates or the parent
|
||||||
|
// receives a fatal signal, both processes exit in a predictable manner.
|
||||||
|
const { spawn } = await import("child_process");
|
||||||
|
|
||||||
|
const child = spawn(binaryPath, process.argv.slice(2), {
|
||||||
stdio: "inherit",
|
stdio: "inherit",
|
||||||
});
|
});
|
||||||
|
|
||||||
const exitCode = typeof result.status === "number" ? result.status : 1;
|
child.on("error", (err) => {
|
||||||
process.exit(exitCode);
|
// Typically triggered when the binary is missing or not executable.
|
||||||
}
|
// Re-throwing here will terminate the parent with a non-zero exit code
|
||||||
|
// while still printing a helpful stack trace.
|
||||||
|
// eslint-disable-next-line no-console
|
||||||
|
console.error(err);
|
||||||
|
process.exit(1);
|
||||||
|
});
|
||||||
|
|
||||||
// Fallback: execute the original JavaScript CLI.
|
// Forward common termination signals to the child so that it shuts down
|
||||||
|
// gracefully. In the handler we temporarily disable the default behavior of
|
||||||
|
// exiting immediately; once the child has been signaled we simply wait for
|
||||||
|
// its exit event which will in turn terminate the parent (see below).
|
||||||
|
const forwardSignal = (signal) => {
|
||||||
|
if (child.killed) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
child.kill(signal);
|
||||||
|
} catch {
|
||||||
|
/* ignore */
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
// Resolve the path to the compiled CLI bundle
|
["SIGINT", "SIGTERM", "SIGHUP"].forEach((sig) => {
|
||||||
const cliPath = path.resolve(__dirname, "../dist/cli.js");
|
process.on(sig, () => forwardSignal(sig));
|
||||||
const cliUrl = pathToFileURL(cliPath).href;
|
});
|
||||||
|
|
||||||
// Load and execute the CLI
|
// When the child exits, mirror its termination reason in the parent so that
|
||||||
(async () => {
|
// shell scripts and other tooling observe the correct exit status.
|
||||||
|
// Wrap the lifetime of the child process in a Promise so that we can await
|
||||||
|
// its termination in a structured way. The Promise resolves with an object
|
||||||
|
// describing how the child exited: either via exit code or due to a signal.
|
||||||
|
const childResult = await new Promise((resolve) => {
|
||||||
|
child.on("exit", (code, signal) => {
|
||||||
|
if (signal) {
|
||||||
|
resolve({ type: "signal", signal });
|
||||||
|
} else {
|
||||||
|
resolve({ type: "code", exitCode: code ?? 1 });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
if (childResult.type === "signal") {
|
||||||
|
// Re-emit the same signal so that the parent terminates with the expected
|
||||||
|
// semantics (this also sets the correct exit code of 128 + n).
|
||||||
|
process.kill(process.pid, childResult.signal);
|
||||||
|
} else {
|
||||||
|
process.exit(childResult.exitCode);
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Fallback: execute the original JavaScript CLI.
|
||||||
|
|
||||||
|
// Resolve the path to the compiled CLI bundle
|
||||||
|
const cliPath = path.resolve(__dirname, "../dist/cli.js");
|
||||||
|
const cliUrl = pathToFileURL(cliPath).href;
|
||||||
|
|
||||||
|
// Load and execute the CLI
|
||||||
try {
|
try {
|
||||||
await import(cliUrl);
|
await import(cliUrl);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
@@ -97,4 +150,4 @@ const cliUrl = pathToFileURL(cliPath).href;
|
|||||||
console.error(err);
|
console.error(err);
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
})();
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user