Skip to content

Commit 78e068b

Browse files
roodboiclaude
roodboi
and
claude
committed
fix: support non-interactive hack global trust and hack doctor --fix
Field report: `hack global trust` presented a clack select/confirm an agent couldn't drive without a TTY, and `hack doctor --fix` opened a guided remediation flow with no non-interactive path at all — it never imported the interactivity convention (src/lib/interactivity.ts) and routed every prompt through a local confirmOrThrow() wrapper around raw clack confirm()/isCancel(), which would hang or misbehave under HACK_NO_INTERACTIVE. hack global trust (src/commands/global.ts): - ensureMacTrustCaddyLocalCa's confirmSafe flips from "decline" to "accept-default" so non-interactive runs proceed with the documented default (trust), per the command's whole purpose. - Before running `sudo security add-trusted-cert`, check `sudo -n true` first; if sudo would prompt for a password, skip the keychain step with a note instead of hanging. - globalTrust() now prepares the host TLS trust env (bundle + launchctl setenv for Bun/Node/curl/git) regardless of whether the System keychain step ran, since browser trust and CLI-tool trust are independent — previously a declined/skipped keychain step silently skipped the trust-env write too. hack doctor --fix (src/commands/doctor.ts): - Replaced confirmOrThrow (raw clack confirm/isCancel, no non-interactive handling) and two more raw confirm()/isCancel() call sites with doctorConfirm(), built on confirmSafe. - Safe remediations (stale daemon pid/socket cleanup, docker network create, mutagen install, tickets git repair, regenerating managed Caddy/CoreDNS files, legacy env file cleanup) use "accept-default" so they apply automatically under --no-interactive. - Destructive/system-level steps (macOS System keychain repair via `hack global trust`, dnsmasq sudo restart + DNS cache flush) use "decline" and print a single summary note listing what was skipped and the interactive command to run them. - Fixed a latent bug: plain `--fix` (without --migrate-env-config) was passing `migrateEnvConfig: true` to runDoctorFix whenever `args.options.fix` was set, unconditionally running env-migration prompts; it now only migrates when --migrate-env-config is passed. Tests: tests/global-command.macos.test.ts covers global trust under --no-interactive with passwordless and password-required sudo, plus an existing decline-path test updated for the trust-env/keychain decoupling. New tests/doctor-fix-noninteractive.test.ts covers a stale-daemon-pid fixture reaching the safe repair without prompting and the mac-keychain destructive step being skipped and reported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c3958bb commit 78e068b

4 files changed

Lines changed: 570 additions & 40 deletions

File tree

‎src/commands/doctor.ts‎

Lines changed: 82 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { lookup } from "node:dns/promises";
22
import { dirname, resolve } from "node:path";
3-
import { confirm, isCancel, note, spinner } from "@clack/prompts";
3+
import { note, spinner } from "@clack/prompts";
44
import { YAML } from "bun";
55
import type { CommandHandlerFor } from "../cli/command.ts";
66
import { defineCommand, defineOption, withHandler } from "../cli/command.ts";
@@ -60,6 +60,10 @@ import {
6060
import { getString, isRecord } from "../lib/guards.ts";
6161
import { resolveHackInvocation } from "../lib/hack-cli.ts";
6262
import { inspectHackEnvOverlayWarnings } from "../lib/hack-env.ts";
63+
import {
64+
confirmSafe,
65+
isExplicitlyNonInteractive,
66+
} from "../lib/interactivity.ts";
6367
import {
6468
ensureBundledMutagenInstalled,
6569
getManagedMutagenAgentBundlePath,
@@ -604,7 +608,7 @@ const handleDoctor: CommandHandlerFor<typeof doctorSpec> = async ({
604608
if (args.options.fix || args.options.migrateEnvConfig) {
605609
await runDoctorFix({
606610
startDir,
607-
migrateEnvConfig: args.options.fix || args.options.migrateEnvConfig,
611+
migrateEnvConfig: args.options.migrateEnvConfig === true,
608612
});
609613
note("Re-run: hack doctor", "doctor");
610614
}
@@ -2359,8 +2363,9 @@ async function runDoctorFix(opts: {
23592363
migrateEnvConfig: opts.migrateEnvConfig,
23602364
});
23612365
note(remediationPlan.join("\n"), "doctor plan");
2366+
skippedDoctorFixSteps.length = 0;
23622367

2363-
const ok = await confirmOrThrow({
2368+
const ok = await doctorConfirm({
23642369
message: opts.migrateEnvConfig
23652370
? "Start guided remediation steps now? (includes env migration when applicable)"
23662371
: "Start guided remediation steps now?",
@@ -2408,6 +2413,27 @@ async function runDoctorFix(opts: {
24082413
if (opts.migrateEnvConfig) {
24092414
await maybeMigrateProjectEnvConfig({ startDir: opts.startDir });
24102415
}
2416+
2417+
renderSkippedDoctorFixSteps();
2418+
}
2419+
2420+
/**
2421+
* Prints a single summary of destructive/system-level `--fix` steps that
2422+
* were skipped because the run was non-interactive (no TTY, `--no-interactive`,
2423+
* or `HACK_NO_INTERACTIVE`), with the command to run them interactively.
2424+
*/
2425+
function renderSkippedDoctorFixSteps(): void {
2426+
if (skippedDoctorFixSteps.length === 0) {
2427+
return;
2428+
}
2429+
note(
2430+
[
2431+
"Skipped (non-interactive; involve sudo/system-level changes):",
2432+
...skippedDoctorFixSteps.map((what) => `- ${what}`),
2433+
"Run interactively (without --no-interactive/HACK_NO_INTERACTIVE) to apply them: hack doctor --fix",
2434+
].join("\n"),
2435+
"doctor fix"
2436+
);
24112437
}
24122438

24132439
async function maybeMigrateProjectEnvConfig(opts: {
@@ -2457,7 +2483,7 @@ async function maybeMigrateProjectEnvConfig(opts: {
24572483
].join("\n"),
24582484
"env migration"
24592485
);
2460-
const shouldMigrate = await confirmOrThrow({
2486+
const shouldMigrate = await doctorConfirm({
24612487
message:
24622488
"Migrate project env from .hack/hack.env.json/.hack/.env to hack.env.*.yaml now?",
24632489
initialValue: true,
@@ -2493,7 +2519,7 @@ async function maybeMigrateProjectEnvConfig(opts: {
24932519
}),
24942520
"compose env repair"
24952521
);
2496-
const shouldRepairCompose = await confirmOrThrow({
2522+
const shouldRepairCompose = await doctorConfirm({
24972523
message:
24982524
"Remove legacy .hack/.env* env_file references from compose now?",
24992525
initialValue: true,
@@ -2544,7 +2570,7 @@ async function maybeMigrateProjectEnvConfig(opts: {
25442570
].join("\n"),
25452571
"env cleanup"
25462572
);
2547-
const shouldCleanup = await confirmOrThrow({
2573+
const shouldCleanup = await doctorConfirm({
25482574
message:
25492575
"Remove legacy env files and obsolete project env config entries now?",
25502576
initialValue: true,
@@ -2592,7 +2618,7 @@ async function maybeRepairModernProjectComposeEnvReferences(opts: {
25922618
}),
25932619
"compose env repair"
25942620
);
2595-
const shouldRepairCompose = await confirmOrThrow({
2621+
const shouldRepairCompose = await doctorConfirm({
25962622
message: "Remove legacy .hack/.env* env_file references from compose now?",
25972623
initialValue: true,
25982624
});
@@ -2794,7 +2820,7 @@ async function maybeRepairProjectTicketsGitHealth(opts: {
27942820
reasons.push("non-ticket files present");
27952821
}
27962822

2797-
const okRepair = await confirmOrThrow({
2823+
const okRepair = await doctorConfirm({
27982824
message: `Repair tickets git storage now? (${reasons.join("; ")})`,
27992825
initialValue: true,
28002826
});
@@ -2804,7 +2830,7 @@ async function maybeRepairProjectTicketsGitHealth(opts: {
28042830

28052831
const pruneLegacyRef =
28062832
health.hasLegacyRef &&
2807-
(await confirmOrThrow({
2833+
(await doctorConfirm({
28082834
message: `Prune legacy tickets ref ${health.legacyRef ?? "refs/heads/hack/tickets"} after repair?`,
28092835
initialValue: true,
28102836
}));
@@ -2850,7 +2876,7 @@ async function maybeInstallMutagenForDoctorFix(): Promise<void> {
28502876
return;
28512877
}
28522878

2853-
const okMutagen = await confirmOrThrow({
2879+
const okMutagen = await doctorConfirm({
28542880
message: "Install managed mutagen at ~/.hack/bin/mutagen?",
28552881
initialValue: true,
28562882
});
@@ -2873,20 +2899,50 @@ async function maybeInstallMutagenForDoctorFix(): Promise<void> {
28732899
note(`Mutagen install failed (${installed.reason}${detail})`, "doctor");
28742900
}
28752901

2876-
async function confirmOrThrow(opts: {
2902+
/**
2903+
* `hack doctor --fix` confirmation helper.
2904+
*
2905+
* Wraps {@link confirmSafe} with the doctor-specific non-interactive
2906+
* defaults: safe remediations (network/CoreDNS/CA/daemon restarts, generated
2907+
* file writes, tickets git repair) proceed automatically
2908+
* (`nonInteractive: "accept-default"`); destructive/system-level steps
2909+
* (anything invoking `sudo`, touching the macOS keychain, or writing outside
2910+
* the project) decline automatically and print a note via
2911+
* {@link noteSkippedDoctorFixStep} so the run stays non-blocking and
2912+
* auditable.
2913+
*/
2914+
async function doctorConfirm(opts: {
28772915
readonly message: string;
28782916
readonly initialValue: boolean;
2917+
readonly destructive?: boolean;
28792918
}): Promise<boolean> {
2880-
const ok = await confirm({
2919+
const ok = await confirmSafe({
28812920
message: opts.message,
28822921
initialValue: opts.initialValue,
2922+
nonInteractive: opts.destructive ? "decline" : "accept-default",
28832923
});
2884-
if (isCancel(ok)) {
2885-
throw new Error("Canceled");
2924+
if (!ok && opts.destructive && isExplicitlyNonInteractive()) {
2925+
noteSkippedDoctorFixStep({ what: firstLine(opts.message) });
28862926
}
28872927
return ok;
28882928
}
28892929

2930+
function firstLine(text: string): string {
2931+
return text.split("\n")[0] ?? text;
2932+
}
2933+
2934+
const skippedDoctorFixSteps: string[] = [];
2935+
2936+
/**
2937+
* Records a destructive/system-level `--fix` step skipped under
2938+
* non-interactive mode so {@link runDoctorFix} can print a single summary
2939+
* note (with the interactive command to run them) instead of scattering the
2940+
* detail across the remediation log.
2941+
*/
2942+
function noteSkippedDoctorFixStep(opts: { readonly what: string }): void {
2943+
skippedDoctorFixSteps.push(opts.what);
2944+
}
2945+
28902946
async function dockerInfoOk(): Promise<boolean> {
28912947
const dockerOk = await exec(["docker", "info"], { stdin: "ignore" });
28922948
return dockerOk.exitCode === 0;
@@ -2899,7 +2955,7 @@ async function maybeRepairHackd(): Promise<void> {
28992955
}
29002956

29012957
if (report.status === "incompatible") {
2902-
const okRestart = await confirmOrThrow({
2958+
const okRestart = await doctorConfirm({
29032959
message: "Restart incompatible hackd now?",
29042960
initialValue: true,
29052961
});
@@ -2910,7 +2966,7 @@ async function maybeRepairHackd(): Promise<void> {
29102966
}
29112967

29122968
if (report.status === "stale") {
2913-
const okClear = await confirmOrThrow({
2969+
const okClear = await doctorConfirm({
29142970
message: "Clear stale hackd pid/socket files?",
29152971
initialValue: true,
29162972
});
@@ -2919,7 +2975,7 @@ async function maybeRepairHackd(): Promise<void> {
29192975
}
29202976
}
29212977

2922-
const okStart = await confirmOrThrow({
2978+
const okStart = await doctorConfirm({
29232979
message: "Start hackd now?",
29242980
initialValue: true,
29252981
});
@@ -2969,7 +3025,7 @@ async function ensureIngressNetwork(): Promise<{
29693025
}
29703026

29713027
const action = ingress.exists ? "Recreate" : "Create";
2972-
const okNetwork = await confirmOrThrow({
3028+
const okNetwork = await doctorConfirm({
29733029
message: `${action} ${DEFAULT_INGRESS_NETWORK} with subnet ${DEFAULT_INGRESS_SUBNET}?`,
29743030
initialValue: true,
29753031
});
@@ -3041,7 +3097,7 @@ async function maybeExportCaddyCaCert(opts: {
30413097
return;
30423098
}
30433099

3044-
const okCa = await confirmOrThrow({
3100+
const okCa = await doctorConfirm({
30453101
message: "Export Caddy Local CA cert for container trust?",
30463102
initialValue: true,
30473103
});
@@ -3063,10 +3119,11 @@ async function maybeRepairMacHostTlsTrust(): Promise<void> {
30633119
}
30643120

30653121
note(hostTlsTrust.message, "doctor");
3066-
const okRepair = await confirmOrThrow({
3122+
const okRepair = await doctorConfirm({
30673123
message:
3068-
"Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack)",
3124+
"Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack; may prompt for sudo to update the System keychain)",
30693125
initialValue: true,
3126+
destructive: true,
30703127
});
30713128
if (!okRepair) {
30723129
return;
@@ -3131,13 +3188,11 @@ async function migrateDnsmasqToContainerIpIfNeeded(): Promise<
31313188
return "not-needed";
31323189
}
31333190

3134-
const okMigrate = await confirm({
3135-
message: `Update dnsmasq to use ${targetIp} for host routing?`,
3191+
const okMigrate = await doctorConfirm({
3192+
message: `Update dnsmasq to use ${targetIp} for host routing? (requires sudo to restart dnsmasq and flush the DNS cache)`,
31363193
initialValue: true,
3194+
destructive: true,
31373195
});
3138-
if (isCancel(okMigrate)) {
3139-
throw new Error("Canceled");
3140-
}
31413196
if (!okMigrate) {
31423197
return "skipped";
31433198
}
@@ -3330,13 +3385,10 @@ async function writeWithPromptIfDifferent(
33303385
}
33313386

33323387
if (existing !== null) {
3333-
const ok = await confirm({
3388+
const ok = await doctorConfirm({
33343389
message: `Overwrite existing file?\n${absolutePath}`,
33353390
initialValue: true,
33363391
});
3337-
if (isCancel(ok)) {
3338-
throw new Error("Canceled");
3339-
}
33403392
if (!ok) {
33413393
return;
33423394
}

‎src/commands/global.ts‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ import {
6363
} from "../lib/fs.ts";
6464
import { getString, isRecord } from "../lib/guards.ts";
6565
import { resolveHackInvocation } from "../lib/hack-cli.ts";
66-
import { confirmSafe } from "../lib/interactivity.ts";
66+
import {
67+
confirmSafe,
68+
isExplicitlyNonInteractive,
69+
} from "../lib/interactivity.ts";
6770
import { parseJsonLines } from "../lib/json-lines.ts";
6871
import {
6972
buildHackHostTrustEnvironment,
@@ -2724,10 +2727,19 @@ async function globalTrust(): Promise<number> {
27242727
const trustReady = await ensureMacTrustCaddyLocalCa({
27252728
certPath,
27262729
});
2727-
if (trustReady) {
2728-
await configureMacHostTlsTrust({
2729-
certPath,
2730-
});
2730+
// Host trust env (Bun/Node/curl/git) is independent of the macOS System
2731+
// keychain step, which is what makes the browser trust *.hack. Prepare
2732+
// it regardless so non-interactive runs still get CLI-tool trust even
2733+
// when the keychain step was skipped (declined, or sudo would have
2734+
// prompted for a password with no TTY to answer it).
2735+
await configureMacHostTlsTrust({
2736+
certPath,
2737+
});
2738+
if (!trustReady) {
2739+
note(
2740+
"Host trust env is prepared, but the browser will still show warnings for https://*.hack until the System keychain step runs. Run `hack global trust` interactively to finish it.",
2741+
"TLS"
2742+
);
27312743
}
27322744
return 0;
27332745
}
@@ -3330,14 +3342,24 @@ async function ensureMacMkcert(): Promise<void> {
33303342
}
33313343
}
33323344

3345+
/**
3346+
* Whether `sudo` can run without prompting for a password right now
3347+
* (`sudo -n true`). Used to avoid launching a `sudo` command that would hang
3348+
* waiting on a password prompt when there is no TTY to answer it.
3349+
*/
3350+
async function canSudoWithoutPassword(): Promise<boolean> {
3351+
const check = await exec(["sudo", "-n", "true"], { stdin: "ignore" });
3352+
return check.exitCode === 0;
3353+
}
3354+
33333355
async function ensureMacTrustCaddyLocalCa(input: {
33343356
readonly certPath: string;
33353357
}): Promise<boolean> {
33363358
const ok = await confirmSafe({
33373359
message:
33383360
"Trust Caddy Local CA in macOS System keychain? (enables trusted https://*.hack; requires sudo)",
33393361
initialValue: true,
3340-
nonInteractive: "decline",
3362+
nonInteractive: "accept-default",
33413363
});
33423364
if (!ok) {
33433365
logger.info({
@@ -3365,6 +3387,14 @@ async function ensureMacTrustCaddyLocalCa(input: {
33653387
return true;
33663388
}
33673389

3390+
if (isExplicitlyNonInteractive() && !(await canSudoWithoutPassword())) {
3391+
logger.info({
3392+
message:
3393+
"Skipped macOS System keychain trust (non-interactive; sudo would prompt for a password). Run `hack global trust` interactively to finish browser trust.",
3394+
});
3395+
return false;
3396+
}
3397+
33683398
logger.step({
33693399
message: "Installing Caddy Local CA to System keychain (requires sudo)…",
33703400
});

0 commit comments

Comments
 (0)