From c3958bb665767a06832fdd36cf4e1ad00286166e Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:07:26 -0400 Subject: [PATCH 1/8] fix(daemon): repair virtual plist paths, sweep orphans, launchd-exclusive start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of 'daemon starts then exits' on compiled-binary installs: the launchd plist captured process.argv[1] from inside the compiled Bun binary — the virtual /$bunfs/root/hack path, which passes pathExists inside that process but is unexecutable by launchd (exit status 78, flapping forever). Reproduced and fixed live on this machine. The bin-path resolver now rejects virtual paths and prefers stable locations; daemon start self-repairs invalid plists; daemons that outlived their pid file (invisible to status/clear/start) are detected and swept; an incompatible running daemon is replaced instead of reported as success; and when launchd manages the service, start goes through launchd exclusively — never a bare spawn beside it. Co-Authored-By: Claude Fable 5 --- src/commands/daemon.ts | 126 ++++++++++++++++++++++++++++++++++-- src/daemon/launchd.ts | 75 ++++++++++++++++++++- src/daemon/process.ts | 80 +++++++++++++++++++++++ tests/daemon-orphan.test.ts | 64 ++++++++++++++++++ 4 files changed, 335 insertions(+), 10 deletions(-) create mode 100644 tests/daemon-orphan.test.ts diff --git a/src/commands/daemon.ts b/src/commands/daemon.ts index 7b37e7b8..095b3d65 100644 --- a/src/commands/daemon.ts +++ b/src/commands/daemon.ts @@ -14,10 +14,16 @@ import { installLaunchdService, kickstartLaunchdService, type LaunchdServiceStatus, + repairLaunchdProgramIfInvalid, uninstallLaunchdService, } from "../daemon/launchd.ts"; import { type DaemonPaths, resolveDaemonPaths } from "../daemon/paths.ts"; -import { removeFileIfExists, waitForProcessExit } from "../daemon/process.ts"; +import { + findOrphanDaemonProcesses, + removeFileIfExists, + terminateOrphanDaemonProcesses, + waitForProcessExit, +} from "../daemon/process.ts"; import { runDaemon } from "../daemon/server.ts"; import { buildDaemonRepairMessage, @@ -220,11 +226,38 @@ async function handleDaemonStart({ const paths = resolveDaemonPaths({}); const status = await readDaemonStatus({ paths }); - if (status.running) { - logger.success({ - message: `hackd already running (pid ${status.pid ?? "unknown"})`, + if (status.running && status.pid !== null) { + const api = await checkDaemonApi({ + socketExists: status.socketExists, + paths, }); - return 0; + if (api.reachable && !api.compatible) { + // An incompatible daemon (usually a pre-upgrade binary) must be + // replaced, not reported as success — leaving it running is how + // machines end up with doctor/status contradictions. + logger.warn({ + message: `Replacing incompatible hackd (pid ${status.pid})`, + }); + await stopDaemonProcess({ pid: status.pid, paths }); + } else { + logger.success({ + message: `hackd already running (pid ${status.pid})`, + }); + return 0; + } + } + + // Daemons that outlived their pid file (e.g. a launchd-managed instance + // surviving a manual clear/restart) hold the API socket invisibly and + // make every freshly spawned daemon exit cleanly. Sweep them first. + const orphans = await findOrphanDaemonProcesses({ + trackedPid: status.pid, + }); + if (orphans.length > 0) { + logger.warn({ + message: `Stopping orphaned hackd process(es) not tracked by the pid file: ${orphans.join(", ")}`, + }); + await terminateOrphanDaemonProcesses({ pids: orphans }); } await removeFileIfExists({ path: paths.socketPath }); @@ -235,6 +268,43 @@ async function handleDaemonStart({ return 0; } + // When launchd manages the daemon, start through launchd — spawning a + // bare process next to a loaded agent means two process managers fight + // over one socket and pid file. + const launchdStatus = await resolveLaunchdStatus({ paths }); + if (launchdStatus?.loaded) { + const repair = await repairLaunchdProgramIfInvalid({ paths }); + if (repair === "repaired") { + logger.warn({ + message: + "Repaired launchd service: its program path was invalid (stale or compiled-binary virtual path)", + }); + } + // launchd owns the daemon: never spawn a bare process next to it — + // two process managers for one socket is how machines end up with + // start-then-exit flapping and stale-pid contradictions. + const kick = await kickstartLaunchdService(); + if (!kick.ok) { + logger.error({ + message: `launchd kickstart failed: ${kick.error ?? "unknown error"} | Check: hack daemon logs --tail 200`, + }); + return 1; + } + const startedViaLaunchd = await waitForDaemonStart({ + paths, + timeoutMs: 8000, + }); + if (startedViaLaunchd) { + logger.success({ message: "hackd started (launchd)" }); + return 0; + } + logger.warn({ + message: + "launchd kickstarted hackd but it did not report ready yet | Check: hack daemon logs --tail 200", + }); + return 1; + } + const invocation = await resolveHackInvocation(); const cmd = [...invocation.args, "daemon", "start", "--foreground"]; const proc = Bun.spawn([invocation.bin, ...cmd], { @@ -256,6 +326,31 @@ async function handleDaemonStart({ return 0; } +async function stopDaemonProcess(opts: { + readonly pid: number; + readonly paths: DaemonPaths; +}): Promise { + try { + process.kill(opts.pid, "SIGTERM"); + } catch { + // Already gone. + } + const exited = await waitForProcessExit({ + pid: opts.pid, + timeoutMs: 2000, + pollMs: 200, + }); + if (!exited) { + try { + process.kill(opts.pid, "SIGKILL"); + } catch { + // Already gone. + } + } + await removeFileIfExists({ path: opts.paths.pidPath }); + await removeFileIfExists({ path: opts.paths.socketPath }); +} + async function handleDaemonStop({ args: _args, }: { @@ -506,11 +601,26 @@ async function handleDaemonClear({ return 1; } + const orphans = await findOrphanDaemonProcesses({ + trackedPid: status.pid, + }); + if (orphans.length > 0) { + logger.warn({ + message: `Stopping orphaned hackd process(es): ${orphans.join(", ")}`, + }); + await terminateOrphanDaemonProcesses({ pids: orphans }); + } + const pidExists = await pathExists(paths.pidPath); const socketExists = await pathExists(paths.socketPath); if (!(pidExists || socketExists)) { - logger.info({ message: "No stale hackd state found" }); + logger.info({ + message: + orphans.length > 0 + ? "Cleared orphaned hackd process(es); no stale files found" + : "No stale hackd state found", + }); return 0; } @@ -549,10 +659,12 @@ async function handleDaemonRestart({ async function waitForDaemonStart({ paths, + timeoutMs, }: { readonly paths: ReturnType; + readonly timeoutMs?: number; }): Promise { - const deadline = Date.now() + 2000; + const deadline = Date.now() + (timeoutMs ?? 2000); while (Date.now() < deadline) { const status = await readDaemonStatus({ paths }); if (status.running && status.socketExists) { diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index baa6c399..82bea61c 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -167,12 +167,16 @@ async function resolveLaunchdHackBinPath(opts: { readonly args: readonly string[]; }; }): Promise { + // Stable locations first: versioned paths (homebrew Cellar) rot on + // upgrade, and argv paths inside a compiled Bun binary report the + // virtual /$bunfs mount, which only exists inside that process — + // launchd exec'ing it fails forever with status 78. const candidates = [ - process.argv[1] ?? null, + resolve(resolveGlobalHackDir(), "bin", "hack"), + await findExecutableInPath("hack"), opts.invocation.args[0] ?? null, opts.invocation.bin, - await findExecutableInPath("hack"), - resolve(resolveGlobalHackDir(), "bin", "hack"), + process.argv[1] ?? null, ]; for (const raw of candidates) { @@ -192,6 +196,9 @@ function normalizeHackExecutablePath(raw: string | null): string | null { if (!trimmed) { return null; } + if (isVirtualExecutablePath(trimmed)) { + return null; + } const base = basename(trimmed).toLowerCase(); if (!(base === "hack" || base.startsWith("hack-"))) { return null; @@ -199,6 +206,68 @@ function normalizeHackExecutablePath(raw: string | null): string | null { return trimmed; } +/** + * Paths inside a compiled Bun binary's virtual filesystem. They pass + * pathExists from INSIDE the same process but no other process (launchd + * included) can exec them. + */ +export function isVirtualExecutablePath(path: string): boolean { + return path.startsWith("/$bunfs/") || path.includes("~BUN"); +} + +/** + * Extracts ProgramArguments[0] from rendered launchd plist text. + */ +export function extractLaunchdProgramPath(opts: { + readonly plistText: string; +}): string | null { + const anchor = opts.plistText.indexOf("ProgramArguments"); + if (anchor < 0) { + return null; + } + const match = opts.plistText.slice(anchor).match(/([^<]+)<\/string>/); + return match?.[1] ?? null; +} + +export type LaunchdProgramRepairResult = + | "ok" + | "repaired" + | "not-installed" + | "failed"; + +/** + * Repairs an installed launchd plist whose program path is invalid — a + * compiled-binary virtual path (/$bunfs) or a binary that no longer exists + * (e.g. an upgraded homebrew Cellar path). Broken plists leave launchd + * flapping with exit status 78 and the daemon unable to stay up. + */ +export async function repairLaunchdProgramIfInvalid(opts: { + readonly paths: DaemonPaths; +}): Promise { + const plistText = await readTextFile(opts.paths.launchdPlistPath); + if (plistText === null) { + return "not-installed"; + } + const program = extractLaunchdProgramPath({ plistText }); + const valid = + program !== null && + !isVirtualExecutablePath(program) && + (await pathExists(program)); + if (valid) { + return "ok"; + } + + const result = await installLaunchdService({ + paths: opts.paths, + config: { + installed: true, + runAtLoad: plistText.includes("RunAtLoad\n "), + guiSessionOnly: plistText.includes("LimitLoadToSessionType"), + }, + }); + return result.ok ? "repaired" : "failed"; +} + export interface LaunchdUninstallResult { readonly ok: boolean; readonly error?: string; diff --git a/src/daemon/process.ts b/src/daemon/process.ts index 8de46dcd..b6183f24 100644 --- a/src/daemon/process.ts +++ b/src/daemon/process.ts @@ -66,3 +66,83 @@ export async function waitForProcessExit({ function sleep({ ms }: { readonly ms: number }): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } + +const DAEMON_COMMAND_MARKER = "daemon start --foreground"; + +/** + * Finds hackd processes that the pid file does not track ("orphans"). + * + * Orphans appear when a daemon keeps running after its pid/socket files were + * replaced or cleared (e.g. a launchd-managed instance surviving a manual + * restart). They are invisible to status/start/clear, which only consult the + * pid file — the root cause of "daemon starts then exits" contradictions: + * the orphan holds the API while every newly spawned daemon exits. + * + * @param opts.trackedPid - pid currently recorded in the pid file, if any. + * @param opts.psLines - injectable `ps -axo pid=,command=` lines for tests. + */ +export async function findOrphanDaemonProcesses(opts: { + readonly trackedPid: number | null; + readonly psLines?: readonly string[]; +}): Promise { + const lines = opts.psLines ?? (await listProcessTable()); + const orphans: number[] = []; + for (const line of lines) { + const trimmed = line.trim(); + const spaceIdx = trimmed.indexOf(" "); + if (spaceIdx <= 0) { + continue; + } + const pid = Number.parseInt(trimmed.slice(0, spaceIdx), 10); + const command = trimmed.slice(spaceIdx + 1); + if (!Number.isFinite(pid) || pid <= 0) { + continue; + } + if (!command.includes(DAEMON_COMMAND_MARKER)) { + continue; + } + if (pid === opts.trackedPid || pid === process.pid) { + continue; + } + orphans.push(pid); + } + return orphans; +} + +async function listProcessTable(): Promise { + const proc = Bun.spawn(["ps", "-axo", "pid=,command="], { + stdout: "pipe", + stderr: "ignore", + stdin: "ignore", + }); + const text = await new Response(proc.stdout).text(); + await proc.exited; + return text.split("\n"); +} + +/** + * Terminates orphan daemon processes (SIGTERM, bounded wait). Returns the + * pids that were actually terminated. + */ +export async function terminateOrphanDaemonProcesses(opts: { + readonly pids: readonly number[]; + readonly timeoutMs?: number; +}): Promise { + const terminated: number[] = []; + for (const pid of opts.pids) { + try { + process.kill(pid, "SIGTERM"); + } catch { + continue; + } + const exited = await waitForProcessExit({ + pid, + timeoutMs: opts.timeoutMs ?? 3000, + pollMs: 100, + }); + if (exited) { + terminated.push(pid); + } + } + return terminated; +} diff --git a/tests/daemon-orphan.test.ts b/tests/daemon-orphan.test.ts new file mode 100644 index 00000000..cb09813b --- /dev/null +++ b/tests/daemon-orphan.test.ts @@ -0,0 +1,64 @@ +import { expect, test } from "bun:test"; + +import { findOrphanDaemonProcesses } from "../src/daemon/process.ts"; + +const PS_LINES = [ + " 123 /Users/x/.hack/bin/hack daemon start --foreground", + " 456 /opt/homebrew/bin/hack daemon start --foreground", + " 789 vim src/commands/daemon.ts", + " 999 hack daemon status", +]; + +test("finds daemon processes not tracked by the pid file", async () => { + const orphans = await findOrphanDaemonProcesses({ + trackedPid: 123, + psLines: PS_LINES, + }); + expect(orphans).toEqual([456]); +}); + +test("all daemon processes are orphans when no pid is tracked", async () => { + const orphans = await findOrphanDaemonProcesses({ + trackedPid: null, + psLines: PS_LINES, + }); + expect(orphans).toEqual([123, 456]); +}); + +test("ignores its own pid and requires the exact daemon marker", async () => { + const orphans = await findOrphanDaemonProcesses({ + trackedPid: null, + psLines: [ + ` ${process.pid} hack daemon start --foreground`, + " 789 tail -f daemon-start-foreground.log", + " 790 node something daemon start --foreground", + ], + }); + expect(orphans).toEqual([790]); +}); + +import { + extractLaunchdProgramPath, + isVirtualExecutablePath, +} from "../src/daemon/launchd.ts"; + +test("virtual bunfs executable paths are rejected", () => { + expect(isVirtualExecutablePath("/$bunfs/root/hack")).toBe(true); + expect(isVirtualExecutablePath("/Users/x/.hack/bin/hack")).toBe(false); +}); + +test("extracts the launchd program path from plist text", () => { + const plist = [ + "Label", + "dance.hack.hackd", + "ProgramArguments", + "", + " /$bunfs/root/hack", + " daemon", + "", + ].join("\n"); + expect(extractLaunchdProgramPath({ plistText: plist })).toBe( + "/$bunfs/root/hack" + ); + expect(extractLaunchdProgramPath({ plistText: "" })).toBeNull(); +}); From 78e068b38539e805d34039423d5de3d948c88292 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:20:09 -0400 Subject: [PATCH 2/8] fix: support non-interactive hack global trust and hack doctor --fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/commands/doctor.ts | 112 +++++++++--- src/commands/global.ts | 42 ++++- tests/doctor-fix-noninteractive.test.ts | 233 ++++++++++++++++++++++++ tests/global-command.macos.test.ts | 223 ++++++++++++++++++++++- 4 files changed, 570 insertions(+), 40 deletions(-) create mode 100644 tests/doctor-fix-noninteractive.test.ts diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 815c278b..74c378d7 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1,6 +1,6 @@ import { lookup } from "node:dns/promises"; import { dirname, resolve } from "node:path"; -import { confirm, isCancel, note, spinner } from "@clack/prompts"; +import { note, spinner } from "@clack/prompts"; import { YAML } from "bun"; import type { CommandHandlerFor } from "../cli/command.ts"; import { defineCommand, defineOption, withHandler } from "../cli/command.ts"; @@ -60,6 +60,10 @@ import { import { getString, isRecord } from "../lib/guards.ts"; import { resolveHackInvocation } from "../lib/hack-cli.ts"; import { inspectHackEnvOverlayWarnings } from "../lib/hack-env.ts"; +import { + confirmSafe, + isExplicitlyNonInteractive, +} from "../lib/interactivity.ts"; import { ensureBundledMutagenInstalled, getManagedMutagenAgentBundlePath, @@ -604,7 +608,7 @@ const handleDoctor: CommandHandlerFor = async ({ if (args.options.fix || args.options.migrateEnvConfig) { await runDoctorFix({ startDir, - migrateEnvConfig: args.options.fix || args.options.migrateEnvConfig, + migrateEnvConfig: args.options.migrateEnvConfig === true, }); note("Re-run: hack doctor", "doctor"); } @@ -2359,8 +2363,9 @@ async function runDoctorFix(opts: { migrateEnvConfig: opts.migrateEnvConfig, }); note(remediationPlan.join("\n"), "doctor plan"); + skippedDoctorFixSteps.length = 0; - const ok = await confirmOrThrow({ + const ok = await doctorConfirm({ message: opts.migrateEnvConfig ? "Start guided remediation steps now? (includes env migration when applicable)" : "Start guided remediation steps now?", @@ -2408,6 +2413,27 @@ async function runDoctorFix(opts: { if (opts.migrateEnvConfig) { await maybeMigrateProjectEnvConfig({ startDir: opts.startDir }); } + + renderSkippedDoctorFixSteps(); +} + +/** + * Prints a single summary of destructive/system-level `--fix` steps that + * were skipped because the run was non-interactive (no TTY, `--no-interactive`, + * or `HACK_NO_INTERACTIVE`), with the command to run them interactively. + */ +function renderSkippedDoctorFixSteps(): void { + if (skippedDoctorFixSteps.length === 0) { + return; + } + note( + [ + "Skipped (non-interactive; involve sudo/system-level changes):", + ...skippedDoctorFixSteps.map((what) => `- ${what}`), + "Run interactively (without --no-interactive/HACK_NO_INTERACTIVE) to apply them: hack doctor --fix", + ].join("\n"), + "doctor fix" + ); } async function maybeMigrateProjectEnvConfig(opts: { @@ -2457,7 +2483,7 @@ async function maybeMigrateProjectEnvConfig(opts: { ].join("\n"), "env migration" ); - const shouldMigrate = await confirmOrThrow({ + const shouldMigrate = await doctorConfirm({ message: "Migrate project env from .hack/hack.env.json/.hack/.env to hack.env.*.yaml now?", initialValue: true, @@ -2493,7 +2519,7 @@ async function maybeMigrateProjectEnvConfig(opts: { }), "compose env repair" ); - const shouldRepairCompose = await confirmOrThrow({ + const shouldRepairCompose = await doctorConfirm({ message: "Remove legacy .hack/.env* env_file references from compose now?", initialValue: true, @@ -2544,7 +2570,7 @@ async function maybeMigrateProjectEnvConfig(opts: { ].join("\n"), "env cleanup" ); - const shouldCleanup = await confirmOrThrow({ + const shouldCleanup = await doctorConfirm({ message: "Remove legacy env files and obsolete project env config entries now?", initialValue: true, @@ -2592,7 +2618,7 @@ async function maybeRepairModernProjectComposeEnvReferences(opts: { }), "compose env repair" ); - const shouldRepairCompose = await confirmOrThrow({ + const shouldRepairCompose = await doctorConfirm({ message: "Remove legacy .hack/.env* env_file references from compose now?", initialValue: true, }); @@ -2794,7 +2820,7 @@ async function maybeRepairProjectTicketsGitHealth(opts: { reasons.push("non-ticket files present"); } - const okRepair = await confirmOrThrow({ + const okRepair = await doctorConfirm({ message: `Repair tickets git storage now? (${reasons.join("; ")})`, initialValue: true, }); @@ -2804,7 +2830,7 @@ async function maybeRepairProjectTicketsGitHealth(opts: { const pruneLegacyRef = health.hasLegacyRef && - (await confirmOrThrow({ + (await doctorConfirm({ message: `Prune legacy tickets ref ${health.legacyRef ?? "refs/heads/hack/tickets"} after repair?`, initialValue: true, })); @@ -2850,7 +2876,7 @@ async function maybeInstallMutagenForDoctorFix(): Promise { return; } - const okMutagen = await confirmOrThrow({ + const okMutagen = await doctorConfirm({ message: "Install managed mutagen at ~/.hack/bin/mutagen?", initialValue: true, }); @@ -2873,20 +2899,50 @@ async function maybeInstallMutagenForDoctorFix(): Promise { note(`Mutagen install failed (${installed.reason}${detail})`, "doctor"); } -async function confirmOrThrow(opts: { +/** + * `hack doctor --fix` confirmation helper. + * + * Wraps {@link confirmSafe} with the doctor-specific non-interactive + * defaults: safe remediations (network/CoreDNS/CA/daemon restarts, generated + * file writes, tickets git repair) proceed automatically + * (`nonInteractive: "accept-default"`); destructive/system-level steps + * (anything invoking `sudo`, touching the macOS keychain, or writing outside + * the project) decline automatically and print a note via + * {@link noteSkippedDoctorFixStep} so the run stays non-blocking and + * auditable. + */ +async function doctorConfirm(opts: { readonly message: string; readonly initialValue: boolean; + readonly destructive?: boolean; }): Promise { - const ok = await confirm({ + const ok = await confirmSafe({ message: opts.message, initialValue: opts.initialValue, + nonInteractive: opts.destructive ? "decline" : "accept-default", }); - if (isCancel(ok)) { - throw new Error("Canceled"); + if (!ok && opts.destructive && isExplicitlyNonInteractive()) { + noteSkippedDoctorFixStep({ what: firstLine(opts.message) }); } return ok; } +function firstLine(text: string): string { + return text.split("\n")[0] ?? text; +} + +const skippedDoctorFixSteps: string[] = []; + +/** + * Records a destructive/system-level `--fix` step skipped under + * non-interactive mode so {@link runDoctorFix} can print a single summary + * note (with the interactive command to run them) instead of scattering the + * detail across the remediation log. + */ +function noteSkippedDoctorFixStep(opts: { readonly what: string }): void { + skippedDoctorFixSteps.push(opts.what); +} + async function dockerInfoOk(): Promise { const dockerOk = await exec(["docker", "info"], { stdin: "ignore" }); return dockerOk.exitCode === 0; @@ -2899,7 +2955,7 @@ async function maybeRepairHackd(): Promise { } if (report.status === "incompatible") { - const okRestart = await confirmOrThrow({ + const okRestart = await doctorConfirm({ message: "Restart incompatible hackd now?", initialValue: true, }); @@ -2910,7 +2966,7 @@ async function maybeRepairHackd(): Promise { } if (report.status === "stale") { - const okClear = await confirmOrThrow({ + const okClear = await doctorConfirm({ message: "Clear stale hackd pid/socket files?", initialValue: true, }); @@ -2919,7 +2975,7 @@ async function maybeRepairHackd(): Promise { } } - const okStart = await confirmOrThrow({ + const okStart = await doctorConfirm({ message: "Start hackd now?", initialValue: true, }); @@ -2969,7 +3025,7 @@ async function ensureIngressNetwork(): Promise<{ } const action = ingress.exists ? "Recreate" : "Create"; - const okNetwork = await confirmOrThrow({ + const okNetwork = await doctorConfirm({ message: `${action} ${DEFAULT_INGRESS_NETWORK} with subnet ${DEFAULT_INGRESS_SUBNET}?`, initialValue: true, }); @@ -3041,7 +3097,7 @@ async function maybeExportCaddyCaCert(opts: { return; } - const okCa = await confirmOrThrow({ + const okCa = await doctorConfirm({ message: "Export Caddy Local CA cert for container trust?", initialValue: true, }); @@ -3063,10 +3119,11 @@ async function maybeRepairMacHostTlsTrust(): Promise { } note(hostTlsTrust.message, "doctor"); - const okRepair = await confirmOrThrow({ + const okRepair = await doctorConfirm({ message: - "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack)", + "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack; may prompt for sudo to update the System keychain)", initialValue: true, + destructive: true, }); if (!okRepair) { return; @@ -3131,13 +3188,11 @@ async function migrateDnsmasqToContainerIpIfNeeded(): Promise< return "not-needed"; } - const okMigrate = await confirm({ - message: `Update dnsmasq to use ${targetIp} for host routing?`, + const okMigrate = await doctorConfirm({ + message: `Update dnsmasq to use ${targetIp} for host routing? (requires sudo to restart dnsmasq and flush the DNS cache)`, initialValue: true, + destructive: true, }); - if (isCancel(okMigrate)) { - throw new Error("Canceled"); - } if (!okMigrate) { return "skipped"; } @@ -3330,13 +3385,10 @@ async function writeWithPromptIfDifferent( } if (existing !== null) { - const ok = await confirm({ + const ok = await doctorConfirm({ message: `Overwrite existing file?\n${absolutePath}`, initialValue: true, }); - if (isCancel(ok)) { - throw new Error("Canceled"); - } if (!ok) { return; } diff --git a/src/commands/global.ts b/src/commands/global.ts index b7fa6f3b..4fec59d2 100644 --- a/src/commands/global.ts +++ b/src/commands/global.ts @@ -63,7 +63,10 @@ import { } from "../lib/fs.ts"; import { getString, isRecord } from "../lib/guards.ts"; import { resolveHackInvocation } from "../lib/hack-cli.ts"; -import { confirmSafe } from "../lib/interactivity.ts"; +import { + confirmSafe, + isExplicitlyNonInteractive, +} from "../lib/interactivity.ts"; import { parseJsonLines } from "../lib/json-lines.ts"; import { buildHackHostTrustEnvironment, @@ -2724,10 +2727,19 @@ async function globalTrust(): Promise { const trustReady = await ensureMacTrustCaddyLocalCa({ certPath, }); - if (trustReady) { - await configureMacHostTlsTrust({ - certPath, - }); + // Host trust env (Bun/Node/curl/git) is independent of the macOS System + // keychain step, which is what makes the browser trust *.hack. Prepare + // it regardless so non-interactive runs still get CLI-tool trust even + // when the keychain step was skipped (declined, or sudo would have + // prompted for a password with no TTY to answer it). + await configureMacHostTlsTrust({ + certPath, + }); + if (!trustReady) { + note( + "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.", + "TLS" + ); } return 0; } @@ -3330,6 +3342,16 @@ async function ensureMacMkcert(): Promise { } } +/** + * Whether `sudo` can run without prompting for a password right now + * (`sudo -n true`). Used to avoid launching a `sudo` command that would hang + * waiting on a password prompt when there is no TTY to answer it. + */ +async function canSudoWithoutPassword(): Promise { + const check = await exec(["sudo", "-n", "true"], { stdin: "ignore" }); + return check.exitCode === 0; +} + async function ensureMacTrustCaddyLocalCa(input: { readonly certPath: string; }): Promise { @@ -3337,7 +3359,7 @@ async function ensureMacTrustCaddyLocalCa(input: { message: "Trust Caddy Local CA in macOS System keychain? (enables trusted https://*.hack; requires sudo)", initialValue: true, - nonInteractive: "decline", + nonInteractive: "accept-default", }); if (!ok) { logger.info({ @@ -3365,6 +3387,14 @@ async function ensureMacTrustCaddyLocalCa(input: { return true; } + if (isExplicitlyNonInteractive() && !(await canSudoWithoutPassword())) { + logger.info({ + message: + "Skipped macOS System keychain trust (non-interactive; sudo would prompt for a password). Run `hack global trust` interactively to finish browser trust.", + }); + return false; + } + logger.step({ message: "Installing Caddy Local CA to System keychain (requires sudo)…", }); diff --git a/tests/doctor-fix-noninteractive.test.ts b/tests/doctor-fix-noninteractive.test.ts new file mode 100644 index 00000000..bfb1c231 --- /dev/null +++ b/tests/doctor-fix-noninteractive.test.ts @@ -0,0 +1,233 @@ +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + expect, + test, +} from "bun:test"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { + GLOBAL_DAEMON_DIR_NAME, + GLOBAL_DAEMON_PID_FILENAME, + GLOBAL_HACK_DIR_NAME, +} from "../src/constants.ts"; +import { resetNoInteractiveFlagForTests } from "../src/lib/interactivity.ts"; +import { registerScopedModuleMock } from "./helpers/scoped-module-mock.ts"; + +/** + * Verifies `hack doctor --fix` under `HACK_NO_INTERACTIVE=1`: + * - safe remediations (stale daemon pid/socket cleanup) apply automatically + * without prompting, + * - destructive/system-level remediations (macOS System keychain repair) + * are skipped with a printed note instead of hanging on a prompt. + * + * Follows the module-mock pattern from tests/global-command.macos.test.ts. + */ + +const runCalls: string[][] = []; +const execCalls: string[][] = []; +const noteCalls: string[] = []; + +const clackMock = await registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "@clack/prompts", + overrides: { + confirm: async () => { + throw new Error( + "confirm() must not be called under HACK_NO_INTERACTIVE=1" + ); + }, + isCancel: () => false, + note: (message: string) => { + noteCalls.push(message); + }, + spinner: () => ({ + start: () => {}, + stop: () => {}, + }), + }, +}); + +const shellMock = await registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "../src/lib/shell.ts", + overrides: { + exec: async (cmd: readonly string[]) => { + execCalls.push([...cmd]); + if (cmd[0] === "docker" && cmd[1] === "info") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (cmd[0] === "docker" && cmd[1] === "network" && cmd[2] === "inspect") { + return { exitCode: 0, stdout: "[]", stderr: "" }; + } + return { exitCode: 1, stdout: "", stderr: "" }; + }, + execOrThrow: async () => ({ exitCode: 0, stdout: "", stderr: "" }), + run: async (cmd: readonly string[]) => { + runCalls.push([...cmd]); + return 0; + }, + findExecutableInPath: (name?: string) => { + if (name === "hack" || name === "bun" || name === "docker") { + return `/usr/local/bin/${name}`; + } + return null; + }, + CommandError: class CommandError extends Error {}, + }, +}); + +const osMock = await registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "../src/lib/os.ts", + overrides: { + isMac: () => true, + isLinux: () => false, + openUrl: async () => 0, + }, +}); + +let tempDir: string | null = null; +let originalHome: string | undefined; +let originalNoInteractive: string | undefined; +let originalMutagenPath: string | undefined; +let originalLogger: string | undefined; + +beforeAll(() => { + clackMock.activate(); + shellMock.activate(); + osMock.activate(); +}); + +beforeEach(async () => { + originalHome = process.env.HOME; + originalNoInteractive = process.env.HACK_NO_INTERACTIVE; + originalMutagenPath = process.env.HACK_MUTAGEN_PATH; + originalLogger = process.env.HACK_LOGGER; + tempDir = await mkdtemp(join(tmpdir(), "hack-doctor-fix-")); + process.env.HOME = tempDir; + process.env.HACK_NO_INTERACTIVE = "1"; + // Short-circuit mutagen path resolution so the fix flow never attempts a + // real network install. + process.env.HACK_MUTAGEN_PATH = "/usr/local/bin/mutagen"; + process.env.HACK_LOGGER = "console"; + runCalls.length = 0; + execCalls.length = 0; + noteCalls.length = 0; +}); + +afterEach(async () => { + resetNoInteractiveFlagForTests(); + if (tempDir) { + await rm(tempDir, { recursive: true, force: true }); + tempDir = null; + } + process.env.HOME = originalHome; + if (originalNoInteractive === undefined) { + Reflect.deleteProperty(process.env, "HACK_NO_INTERACTIVE"); + } else { + process.env.HACK_NO_INTERACTIVE = originalNoInteractive; + } + if (originalMutagenPath === undefined) { + Reflect.deleteProperty(process.env, "HACK_MUTAGEN_PATH"); + } else { + process.env.HACK_MUTAGEN_PATH = originalMutagenPath; + } + process.env.HACK_LOGGER = originalLogger; +}); + +afterAll(() => { + clackMock.deactivate(); + shellMock.deactivate(); + osMock.deactivate(); +}); + +async function createProjectWithStaleDaemonPid(): Promise { + if (!tempDir) { + throw new Error("tempDir not set"); + } + const projectRoot = resolve(tempDir, "project"); + await mkdir(resolve(projectRoot, ".hack"), { recursive: true }); + await writeFile( + resolve(projectRoot, ".hack", "docker-compose.yml"), + "services:\n api:\n image: alpine:3.19\n" + ); + + // Stale daemon pid: a pid that cannot be running (very large, unlikely to + // exist) with no socket file, so buildDaemonStatusReport resolves "stale". + const daemonDir = resolve( + tempDir, + GLOBAL_HACK_DIR_NAME, + GLOBAL_DAEMON_DIR_NAME + ); + await mkdir(daemonDir, { recursive: true }); + await writeFile(resolve(daemonDir, GLOBAL_DAEMON_PID_FILENAME), "999999\n"); + + return projectRoot; +} + +test("doctor --fix under HACK_NO_INTERACTIVE clears a stale daemon pid without prompting", async () => { + const projectRoot = await createProjectWithStaleDaemonPid(); + + const { runCli } = await import("../src/cli/run.ts"); + const code = await runCli([ + "doctor", + "--fix", + "--no-interactive", + "--path", + projectRoot, + ]); + + expect(code).toBe(0); + // Safe remediation applied automatically: `hack daemon clear` was invoked + // to clean up the stale pid/socket state. + expect(runCalls).toEqual( + expect.arrayContaining([expect.arrayContaining(["daemon", "clear"])]) + ); +}); + +test("doctor --fix under HACK_NO_INTERACTIVE skips macOS keychain repair and reports it", async () => { + const projectRoot = await createProjectWithStaleDaemonPid(); + + const { runCli } = await import("../src/cli/run.ts"); + const code = await runCli([ + "doctor", + "--fix", + "--no-interactive", + "--path", + projectRoot, + ]); + + expect(code).toBe(0); + // Destructive/system-level step (macOS host TLS trust repair, which chains + // into a System keychain sudo prompt) is declined automatically and + // surfaced in the skipped-steps summary instead of hanging on a prompt. + const skippedNote = noteCalls.find((message) => + message.includes("Skipped (non-interactive") + ); + expect(skippedNote).toBeDefined(); + expect(skippedNote).toContain("Repair macOS host TLS trust"); + expect(skippedNote).toContain("hack doctor --fix"); +}); + +test("doctor --fix never calls the raw clack confirm prompt under HACK_NO_INTERACTIVE", async () => { + const projectRoot = await createProjectWithStaleDaemonPid(); + + const { runCli } = await import("../src/cli/run.ts"); + const code = await runCli([ + "doctor", + "--fix", + "--no-interactive", + "--path", + projectRoot, + ]); + + // The clack `confirm` mock throws if invoked; reaching a normal exit code + // proves every prompt site in the --fix path routed through confirmSafe's + // non-interactive branch instead of the raw clack prompt. + expect(code).toBe(0); +}); diff --git a/tests/global-command.macos.test.ts b/tests/global-command.macos.test.ts index 5e1c07fa..1366c170 100644 --- a/tests/global-command.macos.test.ts +++ b/tests/global-command.macos.test.ts @@ -19,6 +19,7 @@ import { GLOBAL_LOGGING_COMPOSE_FILENAME, GLOBAL_LOGGING_DIR_NAME, } from "../src/constants.ts"; +import { resetNoInteractiveFlagForTests } from "../src/lib/interactivity.ts"; import { resetGumPathCacheForTests } from "../src/ui/gum.ts"; import { registerScopedModuleMock } from "./helpers/scoped-module-mock.ts"; @@ -35,6 +36,7 @@ let tempDir: string | null = null; let originalHome: string | undefined; let originalLogger: string | undefined; let originalUser: string | undefined; +let originalNoInteractive: string | undefined; let reachabilityByHost: Record = {}; let idUser = "mock-user"; let pathExistsOverrides = new Map(); @@ -221,6 +223,7 @@ beforeEach(async () => { originalHome = process.env.HOME; originalLogger = process.env.HACK_LOGGER; originalUser = process.env.USER; + originalNoInteractive = process.env.HACK_NO_INTERACTIVE; tempDir = await mkdtemp(join(tmpdir(), "hack-global-macos-")); process.env.HOME = tempDir; process.env.USER = "env-user"; @@ -246,6 +249,7 @@ beforeEach(async () => { }); afterEach(async () => { + resetNoInteractiveFlagForTests(); if (tempDir) { await rm(tempDir, { recursive: true, force: true }); tempDir = null; @@ -253,6 +257,11 @@ afterEach(async () => { process.env.HOME = originalHome; process.env.USER = originalUser; process.env.HACK_LOGGER = originalLogger; + if (originalNoInteractive === undefined) { + Reflect.deleteProperty(process.env, "HACK_NO_INTERACTIVE"); + } else { + process.env.HACK_NO_INTERACTIVE = originalNoInteractive; + } resetGumPathCacheForTests(); }); @@ -1043,7 +1052,7 @@ test("global trust prepares host runtime trust env for future shells", async () ); }); -test("global trust leaves host TLS env unchanged when keychain trust is declined", async () => { +test("global trust still prepares host TLS env when keychain trust is declined", async () => { const caddyCompose = join( tempDir!, GLOBAL_HACK_DIR_NAME, @@ -1075,6 +1084,19 @@ test("global trust leaves host TLS env unchanged when keychain trust is declined ) { return { exitCode: 0, stdout: "caddy-123\n", stderr: "" }; } + if ( + cmd[0] === "security" && + cmd[1] === "find-certificate" && + cmd[2] === "-a" && + cmd[3] === "-p" + ) { + return { + exitCode: 0, + stdout: + "-----BEGIN CERTIFICATE-----\nSYSTEM\n-----END CERTIFICATE-----\n", + stderr: "", + }; + } return null; }; @@ -1097,8 +1119,10 @@ test("global trust leaves host TLS env unchanged when keychain trust is declined ); expect(code).toBe(0); - expect(await fileExists(bundlePath)).toBe(false); - expect(await fileExists(envScriptPath)).toBe(false); + // Declining the System keychain step (browser trust) no longer blocks + // Bun/Node/curl/git trust env, which is independent of the keychain. + expect(await Bun.file(bundlePath).text()).toContain("LOCAL"); + expect(await fileExists(envScriptPath)).toBe(true); expect(runCalls).toEqual( expect.arrayContaining([ [ @@ -1107,11 +1131,12 @@ test("global trust leaves host TLS env unchanged when keychain trust is declined "caddy-123:/data/caddy/pki/authorities/local/root.crt", localCaPath, ], + ["launchctl", "setenv", "NODE_EXTRA_CA_CERTS", localCaPath], ]) ); expect(runCalls).not.toEqual( expect.arrayContaining([ - ["launchctl", "setenv", "NODE_EXTRA_CA_CERTS", localCaPath], + expect.arrayContaining(["sudo", "security", "add-trusted-cert"]), ]) ); }); @@ -1197,3 +1222,193 @@ test("global trust falls back to an existing exported CA when Caddy is unavailab expect.arrayContaining([expect.arrayContaining(["docker", "cp"])]) ); }); + +test("global trust proceeds under --no-interactive when sudo is passwordless", async () => { + const caddyCompose = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + GLOBAL_CADDY_COMPOSE_FILENAME + ); + await writeComposeFile(caddyCompose); + + const localCaPath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-local-authority.crt" + ); + await mkdir(dirname(localCaPath), { recursive: true }); + await writeFile( + localCaPath, + "-----BEGIN CERTIFICATE-----\nLOCAL\n-----END CERTIFICATE-----\n" + ); + + execMockResponder = (cmd) => { + if ( + cmd[0] === "docker" && + cmd[1] === "compose" && + cmd[2] === "-f" && + cmd[4] === "ps" + ) { + return { exitCode: 0, stdout: "caddy-123\n", stderr: "" }; + } + if (cmd[0] === "sudo" && cmd[1] === "-n" && cmd[2] === "true") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if ( + cmd[0] === "security" && + cmd[1] === "find-certificate" && + cmd[2] === "-c" + ) { + // Not yet trusted: fast-path lookup misses so the confirmed flow + // proceeds to the `sudo security add-trusted-cert` install. + return { exitCode: 1, stdout: "", stderr: "" }; + } + if ( + cmd[0] === "security" && + cmd[1] === "find-certificate" && + cmd[2] === "-a" && + cmd[3] === "-p" + ) { + return { + exitCode: 0, + stdout: + "-----BEGIN CERTIFICATE-----\nSYSTEM\n-----END CERTIFICATE-----\n", + stderr: "", + }; + } + return null; + }; + + const { runCli } = await import("../src/cli/run.ts"); + const code = await runCli(["global", "trust", "--no-interactive"]); + + const bundlePath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-host-trust-bundle.pem" + ); + + expect(code).toBe(0); + // Non-interactive proceeds with the documented default (trust): the sudo + // keychain install runs without ever calling clack's confirm prompt. + expect(runCalls).toEqual( + expect.arrayContaining([ + [ + "sudo", + "security", + "add-trusted-cert", + "-d", + "-r", + "trustRoot", + "-k", + "/Library/Keychains/System.keychain", + localCaPath, + ], + ]) + ); + expect(await Bun.file(bundlePath).text()).toContain("LOCAL"); +}); + +test("global trust skips the keychain step under --no-interactive when sudo needs a password, but still writes host trust env", async () => { + const caddyCompose = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + GLOBAL_CADDY_COMPOSE_FILENAME + ); + await writeComposeFile(caddyCompose); + + const localCaPath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-local-authority.crt" + ); + await mkdir(dirname(localCaPath), { recursive: true }); + await writeFile( + localCaPath, + "-----BEGIN CERTIFICATE-----\nLOCAL\n-----END CERTIFICATE-----\n" + ); + + execMockResponder = (cmd) => { + if ( + cmd[0] === "docker" && + cmd[1] === "compose" && + cmd[2] === "-f" && + cmd[4] === "ps" + ) { + return { exitCode: 0, stdout: "caddy-123\n", stderr: "" }; + } + if (cmd[0] === "sudo" && cmd[1] === "-n" && cmd[2] === "true") { + // sudo would prompt for a password; there is no TTY to answer it. + return { + exitCode: 1, + stdout: "", + stderr: "sudo: a password is required", + }; + } + if ( + cmd[0] === "security" && + cmd[1] === "find-certificate" && + cmd[2] === "-c" + ) { + return { exitCode: 1, stdout: "", stderr: "" }; + } + if ( + cmd[0] === "security" && + cmd[1] === "find-certificate" && + cmd[2] === "-a" && + cmd[3] === "-p" + ) { + return { + exitCode: 0, + stdout: + "-----BEGIN CERTIFICATE-----\nSYSTEM\n-----END CERTIFICATE-----\n", + stderr: "", + }; + } + return null; + }; + + const { runCli } = await import("../src/cli/run.ts"); + const code = await runCli(["global", "trust", "--no-interactive"]); + + const bundlePath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-host-trust-bundle.pem" + ); + const envScriptPath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-host-trust-env.sh" + ); + + expect(code).toBe(0); + // The sudo-gated keychain install never runs (would have blocked on a + // password prompt), but host trust env (Bun/Node/curl/git) is still + // prepared so non-interactive runs get CLI-tool trust. + expect(runCalls).not.toEqual( + expect.arrayContaining([ + expect.arrayContaining(["sudo", "security", "add-trusted-cert"]), + ]) + ); + expect(await Bun.file(bundlePath).text()).toContain("LOCAL"); + expect(await Bun.file(bundlePath).text()).toContain("SYSTEM"); + expect(await Bun.file(envScriptPath).text()).toContain("NODE_EXTRA_CA_CERTS"); + expect(runCalls).toEqual( + expect.arrayContaining([ + ["launchctl", "setenv", "NODE_EXTRA_CA_CERTS", localCaPath], + ]) + ); +}); From fc56b74649de91e1078a9f4435135e0e27d11690 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:23:06 -0400 Subject: [PATCH 3/8] fix: gate tickets git cache on extension enablement, cover it in ignores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field report: hack setup sync --all-scopes left an untracked .hack/tickets/ local bare-git cache in a repo. setup sync itself was already correctly gated (checkTicketsSkill/installTicketsSkill only run when resolveTicketsIntegrationEnablement().project is true), so the actual culprit is `hack doctor` (and `--fix`), which every project runs as routine maintenance per this repo's own instructions ("run hack doctor, then hack doctor --fix"). checkProjectTicketsGitHealth and maybeRepairProjectTicketsGitHealth (src/commands/doctor.ts) only checked controlPlane.config.tickets.git.enabled — a config key that defaults to `true` and is unrelated to whether the tickets extension is actually enabled for the project (controlPlane.extensions["dance.hack.tickets"].enabled, default `false`). So every `hack doctor` run called createGitTicketsChannel(...).inspect(), whose ensureCheckedOut() creates .hack/tickets/git/{bare.git,worktree} as a side effect of merely inspecting health — regardless of enablement. Both call sites now also check resolveTicketsIntegrationEnablement() (the same gate setup.ts already uses for the tickets skill) before touching the store. Ignore coverage: .hack/tickets/ is genuinely machine-local (confirmed against docs/guides/tickets.md: local working state that syncs via the hidden refs/hack/tickets ref; no project is documented to intentionally commit it). Added `tickets/` to HACK_DIR_GITIGNORE_ENTRIES (src/templates.ts) so the committed .hack/.gitignore covers it, and `/tickets` to the doctor generated-files pathspecs (src/lib/doctor-generated-files.ts) so a tracked copy is flagged and `hack doctor --fix` can untrack it. Tests: new tests/doctor-tickets-dir-gating.test.ts covers hack doctor and hack doctor --fix creating no .hack/tickets/ when the extension is disabled, and creating it (gitignored, clean git status) when enabled. Extended tests/hack-gitignore.test.ts and tests/doctor-generated-files.test.ts for the new tickets/ pattern. Co-Authored-By: Claude Fable 5 --- src/commands/doctor.ts | 19 ++ src/lib/doctor-generated-files.ts | 7 + src/templates.ts | 1 + tests/doctor-generated-files.test.ts | 49 +++++ tests/doctor-tickets-dir-gating.test.ts | 256 ++++++++++++++++++++++++ tests/hack-gitignore.test.ts | 2 + 6 files changed, 334 insertions(+) create mode 100644 tests/doctor-tickets-dir-gating.test.ts diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 74c378d7..b4d8e6be 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -26,6 +26,7 @@ import { } from "../constants.ts"; import { resolveGatewayConfig } from "../control-plane/extensions/gateway/config.ts"; import { listGatewayTokens } from "../control-plane/extensions/gateway/tokens.ts"; +import { resolveTicketsIntegrationEnablement } from "../control-plane/extensions/tickets/enablement.ts"; import { createGitTicketsChannel } from "../control-plane/extensions/tickets/tickets-git-channel.ts"; import { readControlPlaneConfig } from "../control-plane/sdk/config.ts"; import { probeDaemonApi } from "../daemon/client.ts"; @@ -1695,6 +1696,17 @@ async function checkProjectTicketsGitHealth({ }; } + const ticketsEnablement = await resolveTicketsIntegrationEnablement({ + projectRoot: project.projectRoot, + }); + if (!ticketsEnablement.project) { + return { + name: "tickets git", + status: "ok", + message: "Disabled", + }; + } + const controlPlane = await readControlPlaneConfig({ projectDir: project.projectDir, }); @@ -2777,6 +2789,13 @@ async function maybeRepairProjectTicketsGitHealth(opts: { return; } + const ticketsEnablement = await resolveTicketsIntegrationEnablement({ + projectRoot: project.projectRoot, + }); + if (!ticketsEnablement.project) { + return; + } + const controlPlane = await readControlPlaneConfig({ projectDir: project.projectDir, }); diff --git a/src/lib/doctor-generated-files.ts b/src/lib/doctor-generated-files.ts index 12a2bc25..c76610e2 100644 --- a/src/lib/doctor-generated-files.ts +++ b/src/lib/doctor-generated-files.ts @@ -16,6 +16,12 @@ export type TrackedGeneratedFilesInspection = { * Intentionally excluded: `/hack.env.local.yaml` — older repos may track * it on purpose as the shared `--env local` overlay (legacy compatibility in * `project-env-config.ts`), so a tracked copy is not treated as a leak. + * + * `/tickets` is included because the tickets extension's local git + * cache (`/tickets/git/bare.git`, `.../worktree`) is machine-local + * working state that syncs via the hidden `refs/hack/tickets` ref — see + * `docs/guides/tickets.md`. No project is documented to intentionally commit + * it, so a tracked copy is always treated as a leak. */ export function buildGeneratedFilePathspecs(opts: { readonly projectDirName: string; @@ -27,6 +33,7 @@ export function buildGeneratedFilePathspecs(opts: { `${dir}/.env`, `${dir}/.env.state.json`, `${dir}/hack.env.*.local.yaml`, + `${dir}/tickets`, PROJECT_ENV_KEY_FILENAME, ]; } diff --git a/src/templates.ts b/src/templates.ts index 232b9461..affec8f9 100644 --- a/src/templates.ts +++ b/src/templates.ts @@ -318,6 +318,7 @@ export const HACK_DIR_GITIGNORE_ENTRIES = [ ".env.state.json", "hack.env.local.yaml", "hack.env.*.local.yaml", + "tickets/", ] as const; /** diff --git a/tests/doctor-generated-files.test.ts b/tests/doctor-generated-files.test.ts index 05ea66f4..96f17796 100644 --- a/tests/doctor-generated-files.test.ts +++ b/tests/doctor-generated-files.test.ts @@ -125,6 +125,55 @@ test("tracked .hack/hack.env.local.yaml is not flagged (legacy shared overlay)", expect(inspection?.trackedPaths).toEqual([]); }); +test("buildGeneratedFilePathspecs covers the tickets extension's local git cache", () => { + expect(buildGeneratedFilePathspecs({ projectDirName: ".hack" })).toContain( + ".hack/tickets" + ); +}); + +test("tracked .hack/tickets/ (leaked tickets git cache) is flagged and can be untracked", async () => { + const dir = await mkdtemp(join(tmpdir(), "hack-doctor-generated-tickets-")); + tempDirs.add(dir); + const repoRoot = resolve(dir, "repo"); + await mkdir(resolve(repoRoot, ".hack", "tickets", "git"), { + recursive: true, + }); + await runGit(["init", "-b", "main"], repoRoot); + await runGit(["config", "user.name", "Hack Test"], repoRoot); + await runGit(["config", "user.email", "hack@example.com"], repoRoot); + await writeFile( + resolve(repoRoot, ".hack", "docker-compose.yml"), + "services:\n api: {}\n" + ); + await writeFile( + resolve(repoRoot, ".hack", "tickets", "git", "marker.txt"), + "leaked tickets cache\n" + ); + await runGit(["add", "-f", "."], repoRoot); + await runGit(["commit", "-m", "leak tickets cache"], repoRoot); + + const inspection = await inspectTrackedGeneratedFiles({ + projectRoot: repoRoot, + projectDirName: ".hack", + }); + expect(inspection?.trackedPaths).toEqual([".hack/tickets/git/marker.txt"]); + + const untracked = await untrackGeneratedFiles({ + projectRoot: repoRoot, + paths: inspection?.trackedPaths ?? [], + }); + expect(untracked).toEqual({ ok: true, error: null }); + + const second = await inspectTrackedGeneratedFiles({ + projectRoot: repoRoot, + projectDirName: ".hack", + }); + expect(second?.trackedPaths).toEqual([]); + expect( + await pathExists(resolve(repoRoot, ".hack", "tickets", "git", "marker.txt")) + ).toBe(true); +}); + test("untrackGeneratedFiles removes offenders from the index, keeps files on disk, and a rerun is clean", async () => { const repoRoot = await createLeakedRepo(); diff --git a/tests/doctor-tickets-dir-gating.test.ts b/tests/doctor-tickets-dir-gating.test.ts new file mode 100644 index 00000000..49965301 --- /dev/null +++ b/tests/doctor-tickets-dir-gating.test.ts @@ -0,0 +1,256 @@ +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + expect, + test, +} from "bun:test"; +import { mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { ensureHackDirGitignore } from "../src/lib/project-env-config.ts"; +import { registerScopedModuleMock } from "./helpers/scoped-module-mock.ts"; + +/** + * Regression coverage for the field report: `hack doctor` (and `--fix`) must + * not create the tickets extension's local git cache (`.hack/tickets/`) + * unless the extension is actually enabled for the project — it previously + * checked the unrelated `tickets.git.enabled` flag (default `true`) instead + * of `controlPlane.extensions["dance.hack.tickets"].enabled` (default + * `false`), so every `hack doctor` run silently created the directory. + * + * Also covers the companion fix: once the committed `.hack/.gitignore` is + * generated, `tickets/` is covered so an enabled project's cache never shows + * up as untracked. + * + * Docker/shell/OS are mocked (as in tests/doctor-fix-noninteractive.test.ts) + * so `--fix` never touches this machine's real Docker/global infra state. + */ + +async function dirExists(path: string): Promise { + try { + const info = await stat(path); + return info.isDirectory(); + } catch { + return false; + } +} + +const clackMock = await registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "@clack/prompts", + overrides: { + confirm: async () => { + throw new Error( + "confirm() must not be called under HACK_NO_INTERACTIVE=1" + ); + }, + isCancel: () => false, + note: () => {}, + spinner: () => ({ + start: () => {}, + stop: () => {}, + }), + }, +}); + +const shellMock = await registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "../src/lib/shell.ts", + overrides: { + exec: async (cmd: readonly string[]) => { + if (cmd[0] === "docker" && cmd[1] === "info") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if (cmd[0] === "docker" && cmd[1] === "network" && cmd[2] === "inspect") { + return { exitCode: 0, stdout: "[]", stderr: "" }; + } + return { exitCode: 1, stdout: "", stderr: "" }; + }, + execOrThrow: async () => ({ exitCode: 0, stdout: "", stderr: "" }), + run: async () => 0, + findExecutableInPath: (name?: string) => { + if (name === "hack" || name === "bun" || name === "docker") { + return `/usr/local/bin/${name}`; + } + return null; + }, + CommandError: class CommandError extends Error {}, + }, +}); + +const osMock = await registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "../src/lib/os.ts", + overrides: { + isMac: () => true, + isLinux: () => false, + openUrl: async () => 0, + }, +}); + +let tempHome: string | null = null; +let originalHome: string | undefined; +let originalMutagenPath: string | undefined; + +beforeAll(() => { + clackMock.activate(); + shellMock.activate(); + osMock.activate(); +}); + +beforeEach(async () => { + originalHome = process.env.HOME; + originalMutagenPath = process.env.HACK_MUTAGEN_PATH; + tempHome = await mkdtemp(join(tmpdir(), "hack-doctor-tickets-home-")); + process.env.HOME = tempHome; + // Short-circuit mutagen path resolution so --fix never attempts a real + // network install. + process.env.HACK_MUTAGEN_PATH = "/usr/local/bin/mutagen"; +}); + +afterEach(async () => { + if (tempHome) { + await rm(tempHome, { recursive: true, force: true }); + tempHome = null; + } + process.env.HOME = originalHome; + if (originalMutagenPath === undefined) { + Reflect.deleteProperty(process.env, "HACK_MUTAGEN_PATH"); + } else { + process.env.HACK_MUTAGEN_PATH = originalMutagenPath; + } +}); + +afterAll(() => { + clackMock.deactivate(); + shellMock.deactivate(); + osMock.deactivate(); +}); + +const tempDirs = new Set(); + +afterEach(async () => { + for (const dir of tempDirs) { + await rm(dir, { recursive: true, force: true }); + } + tempDirs.clear(); +}); + +async function createTempDir(): Promise { + const dir = await mkdtemp(join(tmpdir(), "hack-doctor-tickets-repo-")); + tempDirs.add(dir); + return dir; +} + +async function runGit(args: readonly string[], cwd: string): Promise { + const proc = Bun.spawn({ + cmd: ["git", ...args], + cwd, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + if (exitCode !== 0) { + throw new Error(stderr || stdout || `git ${args.join(" ")} failed`); + } + return stdout.trim(); +} + +async function gitCheckIgnore(opts: { + readonly repoRoot: string; + readonly path: string; +}): Promise { + const proc = Bun.spawn({ + cmd: ["git", "check-ignore", "-q", "--", opts.path], + cwd: opts.repoRoot, + stdout: "ignore", + stderr: "ignore", + }); + return (await proc.exited) === 0; +} + +async function createFixtureRepo(opts: { + readonly ticketsEnabled: boolean; +}): Promise { + const dir = await createTempDir(); + const repoRoot = resolve(dir, "repo"); + await mkdir(resolve(repoRoot, ".hack"), { recursive: true }); + await runGit(["init", "-b", "main"], repoRoot); + await runGit(["config", "user.name", "Hack Test"], repoRoot); + await runGit(["config", "user.email", "hack@example.com"], repoRoot); + + await writeFile( + resolve(repoRoot, ".hack", "docker-compose.yml"), + "services:\n api:\n image: alpine:3.19\n" + ); + await writeFile( + resolve(repoRoot, ".hack", "hack.config.json"), + `${JSON.stringify( + { + name: "fixture", + controlPlane: { + extensions: { + "dance.hack.tickets": { enabled: opts.ticketsEnabled }, + }, + }, + }, + null, + 2 + )}\n` + ); + await ensureHackDirGitignore({ projectDir: resolve(repoRoot, ".hack") }); + await runGit(["add", "."], repoRoot); + await runGit(["commit", "-m", "init"], repoRoot); + return repoRoot; +} + +async function runDoctor(opts: { + readonly repoRoot: string; + readonly extraArgs?: readonly string[]; +}): Promise { + const { runCli } = await import("../src/cli/run.ts"); + return await runCli([ + "doctor", + "--path", + opts.repoRoot, + "--no-interactive", + ...(opts.extraArgs ?? []), + ]); +} + +test("hack doctor does not create .hack/tickets/ when the extension is disabled", async () => { + const repoRoot = await createFixtureRepo({ ticketsEnabled: false }); + + await runDoctor({ repoRoot }); + + expect(await dirExists(resolve(repoRoot, ".hack", "tickets"))).toBe(false); +}); + +test("hack doctor --fix does not create .hack/tickets/ when the extension is disabled", async () => { + const repoRoot = await createFixtureRepo({ ticketsEnabled: false }); + + await runDoctor({ repoRoot, extraArgs: ["--fix"] }); + + expect(await dirExists(resolve(repoRoot, ".hack", "tickets"))).toBe(false); +}); + +test("hack doctor creates .hack/tickets/ when the extension is enabled, and it is gitignored", async () => { + const repoRoot = await createFixtureRepo({ ticketsEnabled: true }); + + await runDoctor({ repoRoot }); + + expect(await dirExists(resolve(repoRoot, ".hack", "tickets"))).toBe(true); + expect( + await gitCheckIgnore({ repoRoot, path: ".hack/tickets/git/bare.git" }) + ).toBe(true); + + const status = await runGit(["status", "--porcelain"], repoRoot); + expect(status).toBe(""); +}); diff --git a/tests/hack-gitignore.test.ts b/tests/hack-gitignore.test.ts index 1857c59d..f2f9463d 100644 --- a/tests/hack-gitignore.test.ts +++ b/tests/hack-gitignore.test.ts @@ -185,6 +185,7 @@ test("nested .hack/.gitignore makes git ignore generated files, including in lin ".hack/.env.state.json", ".hack/hack.env.local.yaml", ".hack/hack.env.qa.local.yaml", + ".hack/tickets/git/bare.git", ]) { expect( await gitCheckIgnore({ repoRoot: sourceRoot, path }), @@ -200,6 +201,7 @@ test("nested .hack/.gitignore makes git ignore generated files, including in lin ".hack/.branch/compose.x.override.yml", ".hack/.env.state.json", ".hack/hack.env.qa.local.yaml", + ".hack/tickets/git/bare.git", ]) { expect( await gitCheckIgnore({ repoRoot: linkedRoot, path }), From a0bcde462cdb132d525f80e2c9a6028c82136777 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:40:12 -0400 Subject: [PATCH 4/8] fix(global): install prepares host trust env independent of keychain step Mirrors the globalTrust restructure into bootstrapMacGlobalInstall: configureMacHostTlsTrust runs once a cert path exists, with a note when the keychain step didn't complete. Also neutralizes fixture paths that tripped the privacy check in CI. Co-Authored-By: Claude Fable 5 --- src/commands/global.ts | 11 ++++- tests/daemon-orphan.test.ts | 4 +- tests/global-command.macos.test.ts | 71 ++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/commands/global.ts b/src/commands/global.ts index 4fec59d2..9fdacbef 100644 --- a/src/commands/global.ts +++ b/src/commands/global.ts @@ -723,8 +723,15 @@ async function bootstrapMacGlobalInstall(): Promise { const certPath = await exportCaddyLocalCaCert(); if (certPath) { const trustReady = await ensureMacTrustCaddyLocalCa({ certPath }); - if (trustReady) { - await configureMacHostTlsTrust({ certPath }); + // Mirror globalTrust: the host trust env (Bun/Node/curl/git) is + // independent of the macOS System keychain step — prepare it regardless + // so non-interactive installs still get CLI-tool trust. + await configureMacHostTlsTrust({ certPath }); + if (!trustReady) { + note( + "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.", + "TLS" + ); } } await maybeOfferMacRecoverySetup(); diff --git a/tests/daemon-orphan.test.ts b/tests/daemon-orphan.test.ts index cb09813b..707ebee0 100644 --- a/tests/daemon-orphan.test.ts +++ b/tests/daemon-orphan.test.ts @@ -3,7 +3,7 @@ import { expect, test } from "bun:test"; import { findOrphanDaemonProcesses } from "../src/daemon/process.ts"; const PS_LINES = [ - " 123 /Users/x/.hack/bin/hack daemon start --foreground", + " 123 /tmp/hack-home/.hack/bin/hack daemon start --foreground", " 456 /opt/homebrew/bin/hack daemon start --foreground", " 789 vim src/commands/daemon.ts", " 999 hack daemon status", @@ -44,7 +44,7 @@ import { test("virtual bunfs executable paths are rejected", () => { expect(isVirtualExecutablePath("/$bunfs/root/hack")).toBe(true); - expect(isVirtualExecutablePath("/Users/x/.hack/bin/hack")).toBe(false); + expect(isVirtualExecutablePath("/tmp/hack-home/.hack/bin/hack")).toBe(false); }); test("extracts the launchd program path from plist text", () => { diff --git a/tests/global-command.macos.test.ts b/tests/global-command.macos.test.ts index 1366c170..d3902a4f 100644 --- a/tests/global-command.macos.test.ts +++ b/tests/global-command.macos.test.ts @@ -1412,3 +1412,74 @@ test("global trust skips the keychain step under --no-interactive when sudo need ]) ); }); + +test("global install still prepares host TLS env when keychain trust is declined", async () => { + await prepareManagedTools(tempDir!); + reachabilityByHost = { + [DEFAULT_CADDY_IP]: true, + [DEFAULT_HOST_DNS_IP]: true, + }; + confirmResponder = () => false; + + const localCaPath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-local-authority.crt" + ); + await mkdir(dirname(localCaPath), { recursive: true }); + await writeFile( + localCaPath, + "-----BEGIN CERTIFICATE-----\nLOCAL\n-----END CERTIFICATE-----\n" + ); + + execMockResponder = (cmd) => { + if ( + cmd[0] === "docker" && + cmd[1] === "compose" && + cmd[2] === "-f" && + cmd[4] === "ps" + ) { + return { exitCode: 0, stdout: "caddy-123\n", stderr: "" }; + } + if ( + cmd[0] === "security" && + cmd[1] === "find-certificate" && + cmd[2] === "-a" && + cmd[3] === "-p" + ) { + return { + exitCode: 0, + stdout: + "-----BEGIN CERTIFICATE-----\nSYSTEM\n-----END CERTIFICATE-----\n", + stderr: "", + }; + } + return null; + }; + + const { runCli } = await import("../src/cli/run.ts"); + const code = await runCli(["global", "install"]); + + const bundlePath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-host-trust-bundle.pem" + ); + const envScriptPath = join( + tempDir!, + GLOBAL_HACK_DIR_NAME, + GLOBAL_CADDY_DIR_NAME, + "pki", + "caddy-host-trust-env.sh" + ); + + expect(code).toBe(0); + // Same contract as `global trust`: declining the System keychain step + // (browser trust) must not block the Bun/Node/curl/git host trust env. + expect(await Bun.file(bundlePath).text()).toContain("LOCAL"); + expect(await fileExists(envScriptPath)).toBe(true); +}); From 61ac7f64092809d1c7ec796d7553f26b9dda7384 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:46:12 -0400 Subject: [PATCH 5/8] =?UTF-8?q?fix(daemon):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20strict=20orphan=20match,=20stale-path=20repair,=20d?= =?UTF-8?q?ocs=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Orphan detection now requires a hack executable (basename hack/hack-* or a bun dev-tree invocation) — a bare substring match could have terminated unrelated processes. Plist repair also normalizes programs that still exist but differ from the current stable resolution (surviving homebrew Cellar paths would relaunch the old binary in an incompatible-daemon loop). Instruction source lists the tickets/ ignore entry; surfaces resynced. Co-Authored-By: Claude Fable 5 --- .codex/skills/hack-cli/SKILL.md | 2 +- .cursor/rules/hack.mdc | 2 +- AGENTS.md | 2 +- CLAUDE.md | 2 +- src/agents/instruction-source.ts | 2 +- src/daemon/launchd.ts | 12 +++++++++++- src/daemon/process.ts | 23 +++++++++++++++++++++++ tests/agent-instruction-source.test.ts | 3 +++ tests/daemon-orphan.test.ts | 6 ++++-- 9 files changed, 46 insertions(+), 8 deletions(-) diff --git a/.codex/skills/hack-cli/SKILL.md b/.codex/skills/hack-cli/SKILL.md index 718859e1..abe2e5dd 100644 --- a/.codex/skills/hack-cli/SKILL.md +++ b/.codex/skills/hack-cli/SKILL.md @@ -65,7 +65,7 @@ Use `hack` as the primary interface for local-first development. - Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose..override.yml`. - Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands). - Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`. -- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). +- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). ## Linked git worktrees diff --git a/.cursor/rules/hack.mdc b/.cursor/rules/hack.mdc index 60dc8140..acf4b028 100644 --- a/.cursor/rules/hack.mdc +++ b/.cursor/rules/hack.mdc @@ -29,7 +29,7 @@ Prefer `hack` when shell access is available. Use MCP only when shell access is - Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose..override.yml`. - Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands). - Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`. -- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). +- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). ## Linked git worktrees diff --git a/AGENTS.md b/AGENTS.md index 16fef654..dc531c4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -257,7 +257,7 @@ Project files (managed vs generated): - Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose..override.yml`. - Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands). - Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`. -- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). +- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). Linked git worktrees: - Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments. diff --git a/CLAUDE.md b/CLAUDE.md index 0cea99fa..7a17fb43 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,7 +127,7 @@ Project files (managed vs generated): - Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose..override.yml`. - Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands). - Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`. -- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). +- Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk). Linked git worktrees: - Secret key inherits from the primary checkout automatically through the shared git common dir; set `HACK_ENV_SECRET_KEY` for CI or detached environments. diff --git a/src/agents/instruction-source.ts b/src/agents/instruction-source.ts index 394fae2f..f59b899c 100644 --- a/src/agents/instruction-source.ts +++ b/src/agents/instruction-source.ts @@ -113,7 +113,7 @@ export const INSTRUCTION_SECTIONS: readonly InstructionSection[] = [ "Generated (do not hand-edit): `.hack/.internal/compose.override.yml`, `.hack/.internal/compose.env.override.yml`, `.hack/.branch/compose..override.yml`.", "Managed via CLI: `.hack/.internal/extra-hosts.json` (use `hack internal extra-hosts ...` commands).", "Lifecycle runtime files: `.hack/.internal/lifecycle/state.json`, `.hack/.internal/lifecycle/*.log`.", - "Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).", + "Ignore rules: hack owns a committed `.hack/.gitignore` (self-healing on init/up) covering machine-local generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`); keep it committed, and if generated files leaked into git, `hack doctor --fix` untracks them (files stay on disk).", ], }, { diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index 82bea61c..b3a32000 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -249,13 +249,23 @@ export async function repairLaunchdProgramIfInvalid(opts: { return "not-installed"; } const program = extractLaunchdProgramPath({ plistText }); - const valid = + const invocation = await resolveHackInvocation(); + const resolved = await resolveLaunchdHackBinPath({ invocation }); + const exists = program !== null && !isVirtualExecutablePath(program) && (await pathExists(program)); + // A program that still exists can still be stale: a versioned homebrew + // Cellar path survives until brew cleanup, so kickstart would relaunch + // the OLD binary forever (incompatible-daemon loop). Normalize to the + // current stable resolution whenever it differs. + const valid = exists && (resolved === null || program === resolved); if (valid) { return "ok"; } + if (!exists && resolved === null) { + return "failed"; + } const result = await installLaunchdService({ paths: opts.paths, diff --git a/src/daemon/process.ts b/src/daemon/process.ts index b6183f24..ac81d6fe 100644 --- a/src/daemon/process.ts +++ b/src/daemon/process.ts @@ -101,6 +101,9 @@ export async function findOrphanDaemonProcesses(opts: { if (!command.includes(DAEMON_COMMAND_MARKER)) { continue; } + if (!isHackDaemonCommand({ command })) { + continue; + } if (pid === opts.trackedPid || pid === process.pid) { continue; } @@ -109,6 +112,26 @@ export async function findOrphanDaemonProcesses(opts: { return orphans; } +/** + * True when the process command line is actually a hack daemon: the + * executable's basename must be `hack`/`hack-*` (or a `bun .../hack` + * dev-tree invocation). A bare substring match on the daemon arguments + * would also kill unrelated processes that merely mention them. + */ +function isHackDaemonCommand(opts: { readonly command: string }): boolean { + const firstToken = opts.command.trim().split(/\s+/)[0] ?? ""; + const base = firstToken.split("/").pop()?.toLowerCase() ?? ""; + if (base === "hack" || base.startsWith("hack-")) { + return true; + } + if (base === "bun") { + const secondToken = opts.command.trim().split(/\s+/)[1] ?? ""; + const secondBase = secondToken.split("/").pop()?.toLowerCase() ?? ""; + return secondBase === "index.ts" || secondBase.startsWith("hack"); + } + return false; +} + async function listProcessTable(): Promise { const proc = Bun.spawn(["ps", "-axo", "pid=,command="], { stdout: "pipe", diff --git a/tests/agent-instruction-source.test.ts b/tests/agent-instruction-source.test.ts index 57fb7d1b..bc9b7ffc 100644 --- a/tests/agent-instruction-source.test.ts +++ b/tests/agent-instruction-source.test.ts @@ -103,6 +103,9 @@ test("tickets appear at most once, only as an optional extension", () => { for (const [surface, rendered] of Object.entries(RENDERED_SURFACES)) { const ticketLines = rendered .split("\n") + // The gitignore pattern literal `tickets/` (managed-files section) is + // a filename, not tickets promotion — the thing this test guards. + .map((line) => line.replaceAll("`tickets/`", "")) .filter((line) => /ticket/i.test(line)); expect( ticketLines.length, diff --git a/tests/daemon-orphan.test.ts b/tests/daemon-orphan.test.ts index 707ebee0..fed98e6a 100644 --- a/tests/daemon-orphan.test.ts +++ b/tests/daemon-orphan.test.ts @@ -25,16 +25,18 @@ test("all daemon processes are orphans when no pid is tracked", async () => { expect(orphans).toEqual([123, 456]); }); -test("ignores its own pid and requires the exact daemon marker", async () => { +test("ignores its own pid, non-hack executables, and near-miss commands", async () => { const orphans = await findOrphanDaemonProcesses({ trackedPid: null, psLines: [ ` ${process.pid} hack daemon start --foreground`, " 789 tail -f daemon-start-foreground.log", " 790 node something daemon start --foreground", + " 791 /tmp/hack-repo/bin/hack-dev daemon start --foreground", + " 792 bun /tmp/hack-repo/index.ts daemon start --foreground", ], }); - expect(orphans).toEqual([790]); + expect(orphans).toEqual([791, 792]); }); import { From 90d1514f42ca7a5b2b9312f67ed93950fcc07f78 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:49:15 -0400 Subject: [PATCH 6/8] style(daemon): hoist regexes to module scope for the repo lint profile Co-Authored-By: Claude Fable 5 --- src/daemon/launchd.ts | 4 +++- src/daemon/process.ts | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index b3a32000..878e7c2c 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -218,6 +218,8 @@ export function isVirtualExecutablePath(path: string): boolean { /** * Extracts ProgramArguments[0] from rendered launchd plist text. */ +const PLIST_STRING_PATTERN = /([^<]+)<\/string>/; + export function extractLaunchdProgramPath(opts: { readonly plistText: string; }): string | null { @@ -225,7 +227,7 @@ export function extractLaunchdProgramPath(opts: { if (anchor < 0) { return null; } - const match = opts.plistText.slice(anchor).match(/([^<]+)<\/string>/); + const match = opts.plistText.slice(anchor).match(PLIST_STRING_PATTERN); return match?.[1] ?? null; } diff --git a/src/daemon/process.ts b/src/daemon/process.ts index ac81d6fe..4050009d 100644 --- a/src/daemon/process.ts +++ b/src/daemon/process.ts @@ -68,6 +68,7 @@ function sleep({ ms }: { readonly ms: number }): Promise { } const DAEMON_COMMAND_MARKER = "daemon start --foreground"; +const WHITESPACE_PATTERN = /\s+/; /** * Finds hackd processes that the pid file does not track ("orphans"). @@ -119,13 +120,13 @@ export async function findOrphanDaemonProcesses(opts: { * would also kill unrelated processes that merely mention them. */ function isHackDaemonCommand(opts: { readonly command: string }): boolean { - const firstToken = opts.command.trim().split(/\s+/)[0] ?? ""; + const firstToken = opts.command.trim().split(WHITESPACE_PATTERN)[0] ?? ""; const base = firstToken.split("/").pop()?.toLowerCase() ?? ""; if (base === "hack" || base.startsWith("hack-")) { return true; } if (base === "bun") { - const secondToken = opts.command.trim().split(/\s+/)[1] ?? ""; + const secondToken = opts.command.trim().split(WHITESPACE_PATTERN)[1] ?? ""; const secondBase = secondToken.split("/").pop()?.toLowerCase() ?? ""; return secondBase === "index.ts" || secondBase.startsWith("hack"); } From 71c3601395d8da957fad2c64ca635bf6d3742870 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:55:44 -0400 Subject: [PATCH 7/8] fix(doctor): host TLS repair runs under --no-interactive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The step delegates to hack global trust, which is itself non-interactive-safe (sudo preflight; keychain-only skip; host trust env always written) — classifying the whole repair as destructive denied non-interactive runs the safe env/bundle writes. From PR review. Co-Authored-By: Claude Fable 5 --- src/commands/doctor.ts | 6 ++++-- tests/doctor-fix-noninteractive.test.ts | 13 +++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index b4d8e6be..6196e2a4 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -3138,11 +3138,13 @@ async function maybeRepairMacHostTlsTrust(): Promise { } note(hostTlsTrust.message, "doctor"); + // Not classified destructive: `hack global trust` is itself + // non-interactive-safe — it preflights sudo and skips only the System + // keychain step, while the safe host trust env/bundle writes always run. const okRepair = await doctorConfirm({ message: - "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack; may prompt for sudo to update the System keychain)", + "Repair macOS host TLS trust now? (Bun/Node/curl/git trust for https://*.hack; the sudo System-keychain step is skipped automatically when it would prompt)", initialValue: true, - destructive: true, }); if (!okRepair) { return; diff --git a/tests/doctor-fix-noninteractive.test.ts b/tests/doctor-fix-noninteractive.test.ts index bfb1c231..f6518d8e 100644 --- a/tests/doctor-fix-noninteractive.test.ts +++ b/tests/doctor-fix-noninteractive.test.ts @@ -203,15 +203,16 @@ test("doctor --fix under HACK_NO_INTERACTIVE skips macOS keychain repair and rep ]); expect(code).toBe(0); - // Destructive/system-level step (macOS host TLS trust repair, which chains - // into a System keychain sudo prompt) is declined automatically and - // surfaced in the skipped-steps summary instead of hanging on a prompt. + // Host TLS repair is safe to run non-interactively: it delegates to + // `hack global trust`, which preflights sudo and skips only the System + // keychain step while still writing the host trust env. It must NOT be + // in the skipped-steps summary. const skippedNote = noteCalls.find((message) => message.includes("Skipped (non-interactive") ); - expect(skippedNote).toBeDefined(); - expect(skippedNote).toContain("Repair macOS host TLS trust"); - expect(skippedNote).toContain("hack doctor --fix"); + if (skippedNote !== undefined) { + expect(skippedNote).not.toContain("Repair macOS host TLS trust"); + } }); test("doctor --fix never calls the raw clack confirm prompt under HACK_NO_INTERACTIVE", async () => { From 103575a7beb2c4fb894feb25366af5f54c8fa607 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 7 Jul 2026 22:57:20 -0400 Subject: [PATCH 8/8] docs: tickets/ in every durable ignore-block listing Co-Authored-By: Claude Fable 5 --- docs/architecture.md | 2 +- docs/cli.md | 3 ++- docs/env.md | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 32969ca3..3a88a05f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -292,7 +292,7 @@ same repo. - `hack.config.json` - `hack.branches.json` (optional) - `.gitignore` (committed, self-healing on `init`/`up`; covers machine-local generated files — - `.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`. If generated files + `.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, `tickets/`. If generated files leaked into git, `hack doctor --fix` untracks them without deleting them from disk.) - `hack.env.default.yaml` plus optional `hack.env..yaml` (committed env) - `hack.env.local.yaml` / `hack.env..local.yaml` (worktree-local overrides) diff --git a/docs/cli.md b/docs/cli.md index 1f2261cc..b910caa6 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -128,7 +128,8 @@ materialized `.hack/.env` or `.hack/.env.state.json` is stale and should be rege ## Project files Hack owns a committed `.hack/.gitignore` (self-healing on `init`/`up`) that ignores machine-local -generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`). Keep +generated files (`.internal/`, `.branch/`, `.env`, `.env.state.json`, `hack.env*.local.yaml`, +`tickets/`). Keep it committed. If generated files ever leak into git, `hack doctor --fix` untracks them (the files stay on disk). See [Architecture](architecture.md) for the full file map. diff --git a/docs/env.md b/docs/env.md index 2cc61d81..5dc4fad7 100644 --- a/docs/env.md +++ b/docs/env.md @@ -51,6 +51,7 @@ worktrees inherit the rules with zero setup. It covers (patterns relative to - `.env.state.json` - `hack.env.local.yaml` - `hack.env.*.local.yaml` +- `tickets/` (local tickets-extension cache) How it is maintained: