diff --git a/src/lib/doctor.ts b/src/lib/doctor.ts index d8243d5..6cfcaee 100644 --- a/src/lib/doctor.ts +++ b/src/lib/doctor.ts @@ -69,12 +69,13 @@ import { stripNoProxyFor, } from "@/lib/proxy.js"; import { spawner } from "@/lib/reexec.js"; -import { agentOnPath } from "@/lib/run.js"; +import { agentOnPath, resolveAgentPath } from "@/lib/run.js"; import { detectInstalledShims } from "@/lib/shims.js"; import { isCertError, tlsApi } from "@/lib/tls.js"; import { describeStdin, rawModeSupported, + stdinApi, stdinKind, terminalCause, terminalEvidence, @@ -1110,6 +1111,101 @@ const terminalCheck: Check = { }, }; +// Windows only. CoDev Code clears ENABLE_PROCESSED_INPUT on the shared console +// while it runs, so ctrl+c arrives as a keypress its own double-ctrl+c exit can +// handle. When that guard doesn't take, ctrl+c stays a console break delivered +// to every process attached to the console — including the cmd.exe shims in the +// launch chain, which stop at "Terminate batch job (Y/N)?" instead of exiting, +// and the terminal looks frozen. +// +// The agent reports its own verdict (`codev debug console`): it runs under Bun +// and owns the FFI that reads the console mode, and it installs the real guard +// to check that clearing the bit actually sticks. This check only relays that. +const consoleCtrlCCheck: Check = { + key: "console-ctrl-c", + label: "Console ctrl+c handling", + group: "environment", + run: async () => { + if (process.platform !== "win32") + return { status: "skip", detail: "Windows-only check." }; + if (!stdinApi.isTty()) + return { + status: "skip", + detail: + "stdin is not a console, so the console mode can't be read. Re-run from an interactive terminal.", + }; + const agent = resolveAgentPath(CLI["codev-code"]); + if (!agent) + return { + status: "skip", + detail: "CoDev Code is not installed. Run `codevhub install`.", + }; + + // Resolved path, not the bare name: the bare name would re-enter our own + // PATH shim and relaunch the agent through the hub. + const probe = await execAsync(agent, ["debug", "console", "--json"], { + inheritStdin: true, + }); + const report = parseConsoleReport(probe.stdout); + if (!report) + return { + status: "skip", + detail: + "This CoDev Code build has no `debug console` command. Run `codevhub update`.", + }; + if (report.guardEffective) + return { + status: "pass", + detail: `ctrl+c reaches the agent as a keypress (${report.terminal}).`, + }; + + const cause = !report.ffi + ? "CoDev Code could not read the console mode at all (kernel32 dlopen or GetConsoleMode failed)." + : !report.guardInstalled + ? "CoDev Code could not install its ctrl+c guard on this console." + : "The guard installed but ENABLE_PROCESSED_INPUT stayed set, so something is re-applying it."; + return { + status: "fail", + detail: `ctrl+c will reach this console as a break event (${report.terminal}).`, + fix: "Quitting may hang. Until this is fixed, quit with `/exit` instead of ctrl+c, and report this doctor output.", + diagnosis: { + what: "ctrl+c is not being delivered to CoDev Code as a keypress.", + cause, + fix: "Report this output — the console mode line is the part that matters.", + context: [ + `${process.platform} ${process.arch} · terminal ${report.terminal}`, + `console mode ${report.mode.before ?? "?"} -> ${report.mode.guarded ?? "?"} (guarded)`, + ], + raw: probe.stdout.trim().split("\n"), + }, + }; + }, +}; + +interface ConsoleReport { + terminal: string; + ffi: boolean; + guardInstalled: boolean; + guardEffective: boolean; + mode: { before: string | null; guarded: string | null }; +} + +// Returns undefined for anything that isn't the report — a build predating the +// subcommand answers with yargs' usage text on stderr and an empty stdout. +function parseConsoleReport(stdout: string): ConsoleReport | undefined { + const parsed: unknown = (() => { + try { + return JSON.parse(stdout.trim() || "null"); + } catch { + return null; + } + })(); + if (!parsed || typeof parsed !== "object") return undefined; + const report = parsed as Partial; + if (typeof report.guardEffective !== "boolean") return undefined; + return report as ConsoleReport; +} + const systemCaCheck: Check = { key: "system-ca", label: "System certificate store", @@ -1168,6 +1264,7 @@ export const PREFLIGHT_CHECKS: Check[] = [nodeVersionCheck, proxyEnvCheck]; export const ENVIRONMENT_CHECKS: Check[] = [ nodeVersionCheck, terminalCheck, + consoleCtrlCCheck, npmAvailableCheck, npmPrefixCheck, npmRegistryCheck, diff --git a/src/lib/npm.ts b/src/lib/npm.ts index d87814a..2efcf3e 100644 --- a/src/lib/npm.ts +++ b/src/lib/npm.ts @@ -1,4 +1,4 @@ -import { execFile } from "node:child_process"; +import { execFile, spawn } from "node:child_process"; import { existsSync, statSync } from "node:fs"; import { join } from "node:path"; import type { Tool } from "@/lib/configure.js"; @@ -89,7 +89,14 @@ export function recordCommands(): void { commandLog.entries = []; } -export function execAsync(file: string, args: string[]): Promise { +export function execAsync( + file: string, + args: string[], + // `inheritStdin` hands the child our real stdin instead of a pipe. Only the + // console-mode probe needs it: it reads the Windows console input mode + // through its stdin handle, and a piped stdin is not a console. + options: { inheritStdin?: boolean } = {}, +): Promise { // Every child process codev shells out to funnels through here (npm, the // agent --version probes, `code --install-extension`, JetBrains CLIs, // codegraph), so this one seam gives the diagnostic log full child-process @@ -151,18 +158,59 @@ export function execAsync(file: string, args: string[]): Promise { // pass it as the only positional argument. Our args are simple npm // flags + package names with no whitespace, so naive concatenation // matches what Node was already doing — same semantics, no warning. + // + // Quote the file when it carries spaces: callers may pass a resolved + // absolute path (`C:\Program Files\...\codev.cmd`), which cmd.exe would + // otherwise split at the space. Args stay unquoted — see above. + const shellCommand = `${file.includes(" ") ? `"${file}"` : file} ${args.join(" ")}`; + + if (options.inheritStdin) { + // execFile has no stdio option (neither its typings nor its docs), so + // a child that needs the caller's real stdin goes through spawn and + // collects the streams itself. + const child = USE_SHELL + ? spawn(shellCommand, { + stdio: ["inherit", "pipe", "pipe"], + shell: true, + }) + : spawn(file, args, { stdio: ["inherit", "pipe", "pipe"] }); + const out: Buffer[] = []; + const err: Buffer[] = []; + child.stdout?.on("data", (chunk: Buffer) => out.push(chunk)); + child.stderr?.on("data", (chunk: Buffer) => err.push(chunk)); + child.once("error", (error) => + done( + error as NodeJS.ErrnoException, + Buffer.concat(out).toString(), + Buffer.concat(err).toString(), + ), + ); + child.once("close", (code) => + done( + code === 0 + ? null + : Object.assign(new Error(`${file} exited with code ${code}`), { + code: String(code), + }), + Buffer.concat(out).toString(), + Buffer.concat(err).toString(), + ), + ); + return; + } + if (USE_SHELL) { execFile( - `${file} ${args.join(" ")}`, + shellCommand, { shell: true, encoding: "utf-8" }, (err, stdout, stderr) => done(err as NodeJS.ErrnoException | null, stdout, stderr), ); - } else { - execFile(file, args, { encoding: "utf-8" }, (err, stdout, stderr) => - done(err as NodeJS.ErrnoException | null, stdout, stderr), - ); + return; } + execFile(file, args, { encoding: "utf-8" }, (err, stdout, stderr) => + done(err as NodeJS.ErrnoException | null, stdout, stderr), + ); }); } diff --git a/src/lib/run.ts b/src/lib/run.ts index 9b43b54..84cd858 100644 --- a/src/lib/run.ts +++ b/src/lib/run.ts @@ -20,11 +20,11 @@ export const spawner = { spawn: nodeSpawn, }; -// Cheap PATH probe (no child process) used by the bare-`codevhub` dispatch to -// decide between opening CoDev Code and falling back to the hub help. Skips -// the shim dir, mirroring the spawn PATH below. Windows spawns go through the -// shell (PATHEXT resolution), so probe the standard executable extensions. -export function agentOnPath(cmd: string): boolean { +// Cheap PATH lookup (no child process). Skips the shim dir, mirroring the spawn +// PATH below, so callers get the real agent rather than our own shim — spawning +// the shim would re-enter the hub. Windows spawns go through the shell (PATHEXT +// resolution), so probe the standard executable extensions. +export function resolveAgentPath(cmd: string): string | undefined { const exts = process.platform === "win32" ? (process.env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD").split(";") @@ -32,15 +32,22 @@ export function agentOnPath(cmd: string): boolean { for (const dir of stripShimDirFromPath(process.env.PATH).split(delimiter)) { if (!dir) continue; for (const ext of exts) { + const candidate = join(dir, cmd + ext); try { - accessSync(join(dir, cmd + ext), fsConstants.X_OK); - return true; + accessSync(candidate, fsConstants.X_OK); + return candidate; } catch { // Not here — keep scanning. } } } - return false; + return undefined; +} + +// Used by the bare-`codevhub` dispatch to decide between opening CoDev Code and +// falling back to the hub help. +export function agentOnPath(cmd: string): boolean { + return resolveAgentPath(cmd) !== undefined; } export function runAgent(cmd: string, args: string[]): Promise { diff --git a/tests/lib/doctor.test.ts b/tests/lib/doctor.test.ts index a864be8..7be5bb4 100644 --- a/tests/lib/doctor.test.ts +++ b/tests/lib/doctor.test.ts @@ -39,6 +39,7 @@ import * as npm from "@/lib/npm.js"; import * as proxy from "@/lib/proxy.js"; import { envVarKeys, PROXY_APPLIED_ENV } from "@/lib/proxy.js"; import * as reexec from "@/lib/reexec.js"; +import * as run from "@/lib/run.js"; import * as tls from "@/lib/tls.js"; import * as tty from "@/lib/tty.js"; @@ -685,6 +686,83 @@ describe("environment checks", () => { test("terminal is not part of the install pre-flight", () => { expect(PREFLIGHT_CHECKS.some((c) => c.key === "terminal")).toBe(false); }); + + // The console check shells out to the agent and grades what it reports. + // `windows()` sets up the platform + TTY + installed-agent preconditions so + // each test only has to vary the agent's answer. + function windows(stdout: string): void { + Object.defineProperty(process, "platform", { + value: "win32", + configurable: true, + }); + vi.spyOn(tty.stdinApi, "isTty").mockReturnValue(true); + vi.spyOn(run, "resolveAgentPath").mockReturnValue("C:\\bin\\codev.cmd"); + vi.spyOn(npm, "execAsync").mockResolvedValue({ + stdout, + stderr: "", + error: null, + }); + } + + const REPORT = { + terminal: "Windows Terminal", + ffi: true, + guardInstalled: true, + guardEffective: true, + mode: { before: "0x01f7", guarded: "0x01f6" }, + }; + + test("console-ctrl-c is skipped off Windows", async () => { + Object.defineProperty(process, "platform", { + value: "darwin", + configurable: true, + }); + expect((await runOne("console-ctrl-c")).status).toBe("skip"); + }); + + test("console-ctrl-c passes when the guard clears processed input", async () => { + windows(JSON.stringify(REPORT)); + const o = await runOne("console-ctrl-c"); + expect(o.status).toBe("pass"); + expect(o.detail).toContain("Windows Terminal"); + }); + + test("console-ctrl-c fails and names the cause when the guard is ineffective", async () => { + windows(JSON.stringify({ ...REPORT, ffi: false, guardEffective: false })); + const o = await runOne("console-ctrl-c"); + expect(o.status).toBe("fail"); + expect(o.diagnosis?.cause).toContain("kernel32"); + // The agent's own report is what a support ticket is read from. + expect(o.diagnosis?.raw.join(" ")).toContain("guardEffective"); + }); + + test("console-ctrl-c distinguishes a guard that installed but did not stick", async () => { + windows(JSON.stringify({ ...REPORT, guardEffective: false })); + expect((await runOne("console-ctrl-c")).diagnosis?.cause).toContain( + "re-applying", + ); + }); + + // An agent predating `debug console` answers with yargs usage on stderr and + // nothing parseable on stdout. That is a stale install, not a broken console. + test("console-ctrl-c is skipped on an agent without the subcommand", async () => { + windows(""); + const o = await runOne("console-ctrl-c"); + expect(o.status).toBe("skip"); + expect(o.detail).toContain("codevhub update"); + }); + + test("console-ctrl-c probes the resolved path, never the bare name", async () => { + windows(JSON.stringify(REPORT)); + await runOne("console-ctrl-c"); + // The bare name would re-enter our own PATH shim and relaunch the agent + // through the hub, with the upload daemon and key refresh in tow. + expect(npm.execAsync).toHaveBeenCalledWith( + "C:\\bin\\codev.cmd", + ["debug", "console", "--json"], + { inheritStdin: true }, + ); + }); }); // Regression: the probe used to GET the API base path and let `fetch` follow