Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 98 additions & 1 deletion src/lib/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<ConsoleReport>;
if (typeof report.guardEffective !== "boolean") return undefined;
return report as ConsoleReport;
}

const systemCaCheck: Check = {
key: "system-ca",
label: "System certificate store",
Expand Down Expand Up @@ -1168,6 +1264,7 @@ export const PREFLIGHT_CHECKS: Check[] = [nodeVersionCheck, proxyEnvCheck];
export const ENVIRONMENT_CHECKS: Check[] = [
nodeVersionCheck,
terminalCheck,
consoleCtrlCCheck,
npmAvailableCheck,
npmPrefixCheck,
npmRegistryCheck,
Expand Down
62 changes: 55 additions & 7 deletions src/lib/npm.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -89,7 +89,14 @@ export function recordCommands(): void {
commandLog.entries = [];
}

export function execAsync(file: string, args: string[]): Promise<ExecResult> {
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<ExecResult> {
// 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
Expand Down Expand Up @@ -151,18 +158,59 @@ export function execAsync(file: string, args: string[]): Promise<ExecResult> {
// 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),
);
});
}

Expand Down
23 changes: 15 additions & 8 deletions src/lib/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,34 @@ 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(";")
: [""];
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<number> {
Expand Down
78 changes: 78 additions & 0 deletions tests/lib/doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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
Expand Down
Loading