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/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: 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/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/commands/doctor.ts b/src/commands/doctor.ts index 815c278b..6196e2a4 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"; @@ -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"; @@ -60,6 +61,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 +609,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"); } @@ -1691,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, }); @@ -2359,8 +2375,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 +2425,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 +2495,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 +2531,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 +2582,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 +2630,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, }); @@ -2751,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, }); @@ -2794,7 +2839,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 +2849,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 +2895,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 +2918,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 +2974,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 +2985,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 +2994,7 @@ async function maybeRepairHackd(): Promise { } } - const okStart = await confirmOrThrow({ + const okStart = await doctorConfirm({ message: "Start hackd now?", initialValue: true, }); @@ -2969,7 +3044,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 +3116,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,9 +3138,12 @@ async function maybeRepairMacHostTlsTrust(): Promise { } note(hostTlsTrust.message, "doctor"); - const okRepair = await confirmOrThrow({ + // 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)", + "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, }); if (!okRepair) { @@ -3131,13 +3209,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 +3406,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..9fdacbef 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, @@ -720,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(); @@ -2724,10 +2734,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 +3349,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 +3366,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 +3394,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/src/daemon/launchd.ts b/src/daemon/launchd.ts index baa6c399..878e7c2c 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,80 @@ 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. + */ +const PLIST_STRING_PATTERN = /([^<]+)<\/string>/; + +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(PLIST_STRING_PATTERN); + 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 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, + 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..4050009d 100644 --- a/src/daemon/process.ts +++ b/src/daemon/process.ts @@ -66,3 +66,107 @@ 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"; +const WHITESPACE_PATTERN = /\s+/; + +/** + * 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 (!isHackDaemonCommand({ command })) { + continue; + } + if (pid === opts.trackedPid || pid === process.pid) { + continue; + } + orphans.push(pid); + } + 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(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(WHITESPACE_PATTERN)[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", + 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/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/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 new file mode 100644 index 00000000..fed98e6a --- /dev/null +++ b/tests/daemon-orphan.test.ts @@ -0,0 +1,66 @@ +import { expect, test } from "bun:test"; + +import { findOrphanDaemonProcesses } from "../src/daemon/process.ts"; + +const PS_LINES = [ + " 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", +]; + +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, 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([791, 792]); +}); + +import { + extractLaunchdProgramPath, + isVirtualExecutablePath, +} from "../src/daemon/launchd.ts"; + +test("virtual bunfs executable paths are rejected", () => { + expect(isVirtualExecutablePath("/$bunfs/root/hack")).toBe(true); + expect(isVirtualExecutablePath("/tmp/hack-home/.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(); +}); diff --git a/tests/doctor-fix-noninteractive.test.ts b/tests/doctor-fix-noninteractive.test.ts new file mode 100644 index 00000000..f6518d8e --- /dev/null +++ b/tests/doctor-fix-noninteractive.test.ts @@ -0,0 +1,234 @@ +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); + // 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") + ); + 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 () => { + 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/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/global-command.macos.test.ts b/tests/global-command.macos.test.ts index 5e1c07fa..d3902a4f 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,264 @@ 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], + ]) + ); +}); + +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); +}); 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 }),