fix: address R4 review feedback

- Handle spawn 'error' events (ENOENT, EACCES) in runCliCommand so missing
  or unexecutable binaries reject the promise instead of hanging the
  progress dialog (DEREM-23).
- Thread the shell-escape choice through to getHeaderArgs so the value is
  only quoted in shell:true contexts. Multi-word coder.headerCommand values
  now reach the CLI verbatim in execFile/no-shell paths (DEREM-24).
- Report the terminating signal in the error message when the process is
  killed (DEREM-25).
- Expose stdinEnd from the spawn test harness and assert it in the
  happy-path test so the DEREM-18 EOF fix is locked in (DEREM-26).

Also added regression tests for spawn 'error' and signal-kill paths, and
updated cliExec/cliConfig tests for the corrected no-shell header value.

Author: EhabY

src/api/workspace.ts

@@ -80,14 +80,19 @@ function runCliCommand(ctx: CliContext, args: string[]): Promise<void> {
 			capturedStderr += text;
 		});
 
-		proc.on("close", (code: number) => {
+		// Settle on ENOENT/EACCES; later `close` rejects are then no-ops.
+		proc.on("error", reject);
+
+		proc.on("close", (code: number | null, signal: NodeJS.Signals | null) => {
 			if (code === 0) {
 				resolve();
-			} else {
-				let msg = `"${fullArgs.join(" ")}" exited with code ${code}`;
-				if (capturedStderr) msg += `: ${capturedStderr}`;
-				reject(new Error(msg));
+				return;
 			}
+			const exit =
+				code !== null ? `code ${code}` : `signal ${signal ?? "unknown"}`;
+			let msg = `"${fullArgs.join(" ")}" exited with ${exit}`;
+			if (capturedStderr) msg += `: ${capturedStderr}`;
+			reject(new Error(msg));
 		});
 	});
 }

src/settings/cli.ts

@@ -1,5 +1,5 @@
 import { isKeyringSupported } from "../core/cliCredentialManager";
-import { escapeCommandArg } from "../util";
+import { escapeCommandArg, escapeShellArg } from "../util";
 
 import { getHeaderArgs } from "./headers";
 
@@ -29,41 +29,38 @@ export function getUserGlobalFlags(
 		);
 }
 
-/**
- * Returns global configuration flags for Coder CLI commands with auth values
- * escaped for shell use (e.g., `terminal.sendText`, `spawn({ shell: true })`).
- */
+/** Flags for shell contexts (`terminal.sendText`, `spawn({ shell: true })`). */
 export function getGlobalShellFlags(
 	configs: Pick<WorkspaceConfiguration, "get">,
 	auth: CliAuth,
 ): string[] {
-	return buildGlobalFlags(configs, auth, escapeCommandArg);
+	return buildGlobalFlags(configs, auth, escapeCommandArg, escapeShellArg);
 }
 
-/**
- * Returns global configuration flags for Coder CLI commands with raw auth
- * values suitable for `execFile` (no shell escaping).
- */
+/** Raw flags for `execFile` or `spawn` without a shell. */
 export function getGlobalFlags(
 	configs: Pick<WorkspaceConfiguration, "get">,
 	auth: CliAuth,
 ): string[] {
-	return buildGlobalFlags(configs, auth, (s) => s);
+	return buildGlobalFlags(configs, auth, identity, identity);
 }
 
+const identity = (s: string) => s;
+
 function buildGlobalFlags(
 	configs: Pick<WorkspaceConfiguration, "get">,
 	auth: CliAuth,
-	esc: (s: string) => string,
+	escAuth: (s: string) => string,
+	escHeader: (s: string) => string,
 ): string[] {
 	const authFlags =
 		auth.mode === "url"
-			? ["--url", esc(auth.url)]
-			: ["--global-config", esc(auth.configDir)];
+			? ["--url", escAuth(auth.url)]
+			: ["--global-config", escAuth(auth.configDir)];
 
 	const filtered = stripManagedFlags(getUserGlobalFlags(configs));
 
-	return [...filtered, ...authFlags, ...getHeaderArgs(configs)];
+	return [...filtered, ...authFlags, ...getHeaderArgs(configs, escHeader)];
 }
 
 function stripManagedFlags(flags: string[]): string[] {

src/settings/headers.ts

@@ -1,5 +1,3 @@
-import { escapeShellArg } from "../util";
-
 import type { WorkspaceConfiguration } from "vscode";
 
 /** Returns the header command from settings or the CODER_HEADER_COMMAND env var. */
@@ -13,13 +11,14 @@ export function getHeaderCommand(
 	return cmd || undefined;
 }
 
-/** Returns `--header-command` CLI args, escaped for the current platform. */
+/** Returns `--header-command` args; shell callers pass `escapeShellArg` as `esc`. */
 export function getHeaderArgs(
 	config: Pick<WorkspaceConfiguration, "get">,
+	esc: (s: string) => string = (s) => s,
 ): string[] {
 	const command = getHeaderCommand(config);
 	if (!command) {
 		return [];
 	}
-	return ["--header-command", escapeShellArg(command)];
+	return ["--header-command", esc(command)];
 }

test/unit/api/workspace.test.ts

@@ -119,12 +119,17 @@ function controlSpawn() {
 	});
 	return {
 		spawned,
+		stdinEnd: proc.stdin.end,
 		stderr(data: string) {
 			proc.stderr.emit("data", Buffer.from(data));
 		},
-		async close(exitCode: number) {
+		async close(exitCode: number | null, signal?: NodeJS.Signals) {
 			await spawned;
-			proc.emit("close", exitCode);
+			proc.emit("close", exitCode, signal ?? null);
+		},
+		async error(err: Error) {
+			await spawned;
+			proc.emit("error", err);
 		},
 	};
 }
@@ -207,6 +212,7 @@ describe("updateWorkspace", () => {
 			"update",
 			"testuser/test-workspace",
 		]);
+		expect(sp.stdinEnd).toHaveBeenCalled();
 		expect(restClient.getWorkspace).toHaveBeenCalledWith(ctx.workspace.id);
 	});
 
@@ -223,6 +229,27 @@ describe("updateWorkspace", () => {
 		expect(restClient.getWorkspace).not.toHaveBeenCalled();
 	});
 
+	it("rejects when spawn emits an error (e.g. missing binary)", async () => {
+		const { ctx, restClient } = createUpdateCtx();
+		const sp = controlSpawn();
+
+		const result = updateWorkspace(ctx);
+		await sp.error(new Error("spawn /usr/bin/coder ENOENT"));
+
+		await expect(result).rejects.toThrow(/ENOENT/);
+		expect(restClient.getWorkspace).not.toHaveBeenCalled();
+	});
+
+	it("reports the terminating signal when the process is killed", async () => {
+		const { ctx } = createUpdateCtx();
+		const sp = controlSpawn();
+
+		const result = updateWorkspace(ctx);
+		await sp.close(null, "SIGTERM");
+
+		await expect(result).rejects.toThrow(/signal SIGTERM/);
+	});
+
 	it("falls back to the API update path when coder update is unsupported", async () => {
 		const { ctx, restClient, finalWorkspace } = createUpdateCtx({
 			featureSet: { cliUpdate: false },

test/unit/cliConfig.test.ts

@@ -190,14 +190,14 @@ describe("cliConfig", () => {
 			]);
 		});
 
-		it("should still escape header-command flags", () => {
+		it("passes header-command value through verbatim (no shell)", () => {
 			const config = new MockConfigurationProvider();
 			config.set("coder.headerCommand", "echo test");
 			expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([
 				"--global-config",
 				"/config/dir",
 				"--header-command",
-				quoteCommand("echo test"),
+				"echo test",
 			]);
 		});
 

test/unit/core/cliExec.test.ts

@@ -6,7 +6,6 @@ import { beforeAll, describe, expect, it, vi } from "vitest";
 import { MockConfigurationProvider } from "../../mocks/testHelpers";
 import {
 	isWindows,
-	quoteCommand,
 	writeExecutable,
 	writeStderrJs,
 	writeStdoutJs,
@@ -154,7 +153,7 @@ describe("cliExec", () => {
 				"--url",
 				"http://localhost:3000",
 				"--header-command",
-				quoteCommand("my-header-cmd"),
+				"my-header-cmd",
 				"speedtest",
 				"owner/workspace",
 				"--output",
@@ -211,7 +210,7 @@ describe("cliExec", () => {
 				"--url",
 				"http://localhost:3000",
 				"--header-command",
-				quoteCommand("my-header-cmd"),
+				"my-header-cmd",
 				"support",
 				"bundle",
 				"owner/workspace",