From 02780eb9bea661659edbcde1accbfbf4b2659673 Mon Sep 17 00:00:00 2001
From: Jarred Sumner <jarred@jarredsumner.com>
Date: Thu, 3 Jul 2025 01:06:22 -0700
Subject: [PATCH] Fixes #20753 (#20789)

Co-authored-by: Jarred-Sumner <709451+Jarred-Sumner@users.noreply.github.com>
---
 src/js/node/child_process.ts        |   5 +-
 test/regression/issue/20753.test.js | 116 ++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+), 2 deletions(-)
 create mode 100644 test/regression/issue/20753.test.js

diff --git a/src/js/node/child_process.ts b/src/js/node/child_process.ts
index 458e3dae39..c144e6281a 100644
--- a/src/js/node/child_process.ts
+++ b/src/js/node/child_process.ts
@@ -272,12 +272,13 @@ function execFile(file, args, options, callback) {
     // merge chunks
     let stdout;
     let stderr;
-    if (child.stdout?.readableEncoding) {
+    if (encoding || child.stdout?.readableEncoding) {
       stdout = ArrayPrototypeJoin.$call(_stdout, "");
     } else {
       stdout = BufferConcat(_stdout);
     }
-    if (child.stderr?.readableEncoding) {
+
+    if (encoding || child.stderr?.readableEncoding) {
       stderr = ArrayPrototypeJoin.$call(_stderr, "");
     } else {
       stderr = BufferConcat(_stderr);
diff --git a/test/regression/issue/20753.test.js b/test/regression/issue/20753.test.js
new file mode 100644
index 0000000000..df84b3e267
--- /dev/null
+++ b/test/regression/issue/20753.test.js
@@ -0,0 +1,116 @@
+import { describe, expect, test } from "bun:test";
+import { isWindows } from "harness";
+import { execFile } from "node:child_process";
+import { promisify } from "node:util";
+
+const execFileAsync = promisify(execFile);
+
+describe.skipIf(isWindows /* accessing posix-specific paths */)("stdout should always be a string", () => {
+  test("execFile returns string stdout/stderr even when process fails to spawn", done => {
+    // Test case that would cause the issue: non-existent command
+    execFile("/does/not/exist", [], (err, stdout, stderr) => {
+      expect(err).toBeTruthy();
+      expect(err.code).toBe("ENOENT");
+
+      // These should never be undefined - they should be strings by default
+      expect(stdout).toBeDefined();
+      expect(stderr).toBeDefined();
+      expect(typeof stdout).toBe("string");
+      expect(typeof stderr).toBe("string");
+      expect(stdout).toBe("");
+      expect(stderr).toBe("");
+
+      // This is what claude-code was trying to do that failed
+      expect(() => stdout.trim()).not.toThrow();
+      expect(() => stderr.trim()).not.toThrow();
+
+      done();
+    });
+  });
+
+  test("execFile returns string stdout/stderr for permission denied errors", done => {
+    // Another edge case: file exists but not executable
+    execFile("/etc/passwd", [], (err, stdout, stderr) => {
+      expect(err).toBeTruthy();
+      expect(err.code).toBe("EACCES");
+
+      expect(stdout).toBeDefined();
+      expect(stderr).toBeDefined();
+      expect(typeof stdout).toBe("string");
+      expect(typeof stderr).toBe("string");
+      expect(stdout).toBe("");
+      expect(stderr).toBe("");
+
+      done();
+    });
+  });
+
+  test("execFile returns Buffer stdout/stderr when encoding is 'buffer'", done => {
+    execFile("/does/not/exist", [], { encoding: "buffer" }, (err, stdout, stderr) => {
+      expect(err).toBeTruthy();
+      expect(err.code).toBe("ENOENT");
+
+      expect(stdout).toBeDefined();
+      expect(stderr).toBeDefined();
+      expect(Buffer.isBuffer(stdout)).toBe(true);
+      expect(Buffer.isBuffer(stderr)).toBe(true);
+      expect(stdout.length).toBe(0);
+      expect(stderr.length).toBe(0);
+
+      done();
+    });
+  });
+
+  test("execFile promisified version includes stdout/stderr in error object", async () => {
+    try {
+      await execFileAsync("/does/not/exist", []);
+      expect.unreachable("Should have thrown");
+    } catch (err) {
+      expect(err.code).toBe("ENOENT");
+
+      // Promisified version attaches stdout/stderr to the error object
+      expect(err.stdout).toBeDefined();
+      expect(err.stderr).toBeDefined();
+      expect(typeof err.stdout).toBe("string");
+      expect(typeof err.stderr).toBe("string");
+      expect(err.stdout).toBe("");
+      expect(err.stderr).toBe("");
+    }
+  });
+
+  test("execFile returns stdout/stderr for process that exits with error code", done => {
+    execFile(
+      process.execPath,
+      ["-e", "console.log('output'); console.error('error'); process.exit(1)"],
+      (err, stdout, stderr) => {
+        expect(err).toBeTruthy();
+        expect(err.code).toBe(1);
+
+        expect(stdout).toBeDefined();
+        expect(stderr).toBeDefined();
+        expect(typeof stdout).toBe("string");
+        expect(typeof stderr).toBe("string");
+        expect(stdout).toBe("output\n");
+        expect(stderr).toBe("error\n");
+
+        done();
+      },
+    );
+  });
+
+  test("execFile handles fast-exiting processes correctly", done => {
+    // Process that exits immediately
+    execFile("true", [], (err, stdout, stderr) => {
+      expect(err).toBeNull();
+
+      expect(stdout).toBeDefined();
+      expect(stderr).toBeDefined();
+      expect(typeof stdout).toBe("string");
+      expect(typeof stderr).toBe("string");
+      expect(stdout).toBe("");
+      expect(stderr).toBe("");
+
+      done();
+    });
+  });
+});