From 7662d1240a5039fc037d0eb29c901184eea9fcff Mon Sep 17 00:00:00 2001 From: minhn4 Date: Thu, 6 Aug 2026 10:40:07 +0700 Subject: [PATCH] feat(doctor): add a console ctrl+c check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quitting CoDev Code hangs for some Windows users and not others, and the variable we could not observe is how ctrl+c is delivered. CoDev Code clears ENABLE_PROCESSED_INPUT on the shared console while it runs so ctrl+c arrives as a keypress; when that guard does not take, ctrl+c stays a console break delivered to every process attached to the console, including the cmd.exe shims in our launch chain, which stop at "Terminate batch job (Y/N)?" instead of exiting. The agent reports its own verdict (`codev debug console --json`): 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 sticks. This check relays that and names which precondition failed. Two supporting changes: - run.ts splits resolveAgentPath out of agentOnPath. The check probes the resolved path, never the bare name — the bare name would re-enter our own PATH shim and relaunch the agent through the hub, with the upload daemon and gateway key refresh in tow. - npm.ts gains execAsync({ inheritStdin }). The probe reads the console input mode through its stdin handle, and a piped stdin is not a console. execFile has no stdio option, so that path uses spawn and collects the streams itself. Its shell branch now also quotes a file carrying spaces, since callers can pass a resolved absolute path. Skips rather than fails on a build predating `codev debug console`, so the two repos can land in either order. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/doctor.ts | 99 +++++++++++++++++++++++++++++++++++++++- src/lib/npm.ts | 62 ++++++++++++++++++++++--- src/lib/run.ts | 23 ++++++---- tests/lib/doctor.test.ts | 78 +++++++++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 16 deletions(-) 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