fix(cli): guard ssh/node prompts without a TTY, add secrets delete --yes, and test every prompt guard

docs/cli.md

@@ -159,7 +159,9 @@ Historical remote guides are excluded from the supported documentation.
   a safe default apply it (`hack env unset KEY` removes the key; `hack doctor --fix` applies safe
   remediations and skips destructive ones with a note); prompts that need a real answer fail
   closed with `E_INTERACTIVE_REQUIRED`, naming the flag or argument that makes the invocation
-  scriptable (for example `hack update --yes`, `hack mcp --all`, `hack session attach <name>`).
+  scriptable (for example `hack update --yes`, `hack mcp --all`, `hack session attach <name>`,
+  `hack secrets delete <name> --yes`, `hack ssh --host <host>`). Destructive confirmations without
+  such a flag decline or refuse; they never proceed on their own.
   Set `HACK_NO_INTERACTIVE=1` explicitly in scripts anyway so the intent is visible.
 - `NO_COLOR` (or `HACK_NO_COLOR`) disables colored/decorated output.
 

docs/reference/cli.md

@@ -2254,6 +2254,9 @@ Delete a stored secret
 hack secrets delete [name] [options]
 ```
 
+Asks for confirmation before deleting. Without a TTY (or with HACK_NO_INTERACTIVE / --no-interactive)
+the deletion is refused with E_INTERACTIVE_REQUIRED unless --yes is passed.
+
 ### Arguments
 
 | Arg | Description |
@@ -2265,6 +2268,7 @@ hack secrets delete [name] [options]
 | Option | Description |
 | --- | --- |
 | `--service <service>` | Override Bun.secrets service name (default: hack-cli) |
+| `--yes` | Confirm the deletion without prompting (required without a TTY or with HACK_NO_INTERACTIVE) |
 | `--no-interactive` | Never prompt: apply documented defaults or fail with E_INTERACTIVE_REQUIRED (also via HACK_NO_INTERACTIVE=1) |
 | `--help, -h` | Show help |
 | `--version, -v` | Show version |

src/commands/node.ts

@@ -21,6 +21,7 @@ import { readControlPlaneConfig } from "../control-plane/sdk/config.ts";
 import { createGatewayClient } from "../control-plane/sdk/gateway-client.ts";
 import { resolveDaemonPaths } from "../daemon/paths.ts";
 import { pathExists } from "../lib/fs.ts";
+import { canPrompt } from "../lib/interactivity.ts";
 import {
   cancelNodePairingSession,
   consumeNodePairingSession,
@@ -2046,10 +2047,15 @@ async function handleNodePairWalkthrough({
     ],
   });
 
-  const executeNow = await confirm({
-    message: "Execute remote approval and complete pairing now?",
-    initialValue: true,
-  });
+  // Without a TTY the session is left pending: the remote approval runs SSH
+  // commands and needs an explicit answer, and a clack prompt cannot settle on
+  // a piped stdin (HACK-1385).
+  const executeNow = canPrompt()
+    ? await confirm({
+        message: "Execute remote approval and complete pairing now?",
+        initialValue: true,
+      })
+    : false;
   if (isCancel(executeNow)) {
     await cancelNodePairingSession({ sessionId: created.session.id });
     logger.info({
@@ -2165,7 +2171,9 @@ type PairWalkthroughValueResult = PairWalkthroughValue | PairWalkthroughError;
 async function resolvePairWalkthroughInput(input: {
   readonly args: PairWalkthroughArgs;
 }): Promise<PairWalkthroughResult> {
-  const interactive = isTty();
+  // canPrompt(), not isTty(): a TTY stdout with a piped stdin must not reach
+  // the clack prompts below (HACK-1385).
+  const interactive = canPrompt();
   const sourceResult = await resolveRequiredWalkthroughValue({
     currentValue: input.args.options.source,
     interactive,

src/commands/secrets.ts

@@ -21,13 +21,26 @@ const optService = defineOption({
   defaultValue: DEFAULT_SECRETS_SERVICE,
 } as const);
 
+const optYes = defineOption({
+  name: "yes",
+  type: "boolean",
+  long: "--yes",
+  description:
+    "Confirm the deletion without prompting (required without a TTY or with HACK_NO_INTERACTIVE)",
+} as const);
+
 const secretsOptions = [optService] as const;
+const deleteOptions = [optService, optYes] as const;
 const secretsPositionals = [{ name: "name", required: false }] as const;
 
 type SecretsArgs = CommandArgs<
   typeof secretsOptions,
   typeof secretsPositionals
 >;
+type SecretsDeleteArgs = CommandArgs<
+  typeof deleteOptions,
+  typeof secretsPositionals
+>;
 
 const setSpec = defineCommand({
   name: "set",
@@ -51,7 +64,11 @@ const deleteSpec = defineCommand({
   name: "delete",
   summary: "Delete a stored secret",
   group: "Secrets",
-  options: secretsOptions,
+  description: [
+    "Asks for confirmation before deleting. Without a TTY (or with HACK_NO_INTERACTIVE / --no-interactive)",
+    "the deletion is refused with E_INTERACTIVE_REQUIRED unless --yes is passed.",
+  ].join("\n"),
+  options: deleteOptions,
   positionals: secretsPositionals,
   subcommands: [],
 } as const);
@@ -70,7 +87,7 @@ export const secretsCommand = defineCommand({
   ],
 } as const);
 
-function resolveService(args: SecretsArgs): string {
+function resolveService(args: SecretsArgs | SecretsDeleteArgs): string {
   return args.options.service ?? DEFAULT_SECRETS_SERVICE;
 }
 
@@ -158,23 +175,33 @@ async function handleSecretsGet({
   return 0;
 }
 
+/**
+ * `hack secrets delete <name>`.
+ *
+ * `--yes` is the explicit confirmation that makes the deletion scriptable; it
+ * skips the prompt in every mode. Without it, a TTY run asks (default no) and
+ * a run that cannot prompt refuses closed with `E_INTERACTIVE_REQUIRED`
+ * before the keychain is touched.
+ */
 async function handleSecretsDelete({
   args,
 }: {
   readonly ctx: CliContext;
-  readonly args: SecretsArgs;
+  readonly args: SecretsDeleteArgs;
 }): Promise<number> {
   const service = resolveService(args);
   const name = await resolveName(
     [args.positionals.name].filter((v) => typeof v === "string")
   );
 
-  const ok = await confirmSafe({
-    message: `Delete secret "${name}" (${service})?`,
-    initialValue: false,
-    nonInteractive: "fail",
-    hint: "Run `hack secrets delete <name>` in a terminal to confirm the deletion.",
-  });
+  const ok =
+    args.options.yes === true ||
+    (await confirmSafe({
+      message: `Delete secret "${name}" (${service})?`,
+      initialValue: false,
+      nonInteractive: "fail",
+      hint: "Pass --yes to confirm the deletion (`hack secrets delete <name> --yes`), or run it in a terminal.",
+    }));
   if (!ok) {
     return 0;
   }

src/commands/session.ts

@@ -344,7 +344,16 @@ function buildSessionPickerOptions(opts: {
   return options;
 }
 
-async function promptAttachedWorkspaceAction(opts: {
+/**
+ * Asks whether to attach to a workspace that another client holds, or to
+ * create an isolated one. Without a TTY it refuses with
+ * `E_INTERACTIVE_REQUIRED` instead of entering the clack prompt, which cannot
+ * settle on a piped stdin (HACK-1385).
+ *
+ * Exported for the non-TTY regression test: in a spawned non-TTY CLI the
+ * picker refuses first, so this guard is exercised directly.
+ */
+export async function promptAttachedWorkspaceAction(opts: {
   readonly attachedWorkspaceName: string;
   readonly nextWorkspaceName: string;
 }): Promise<"attach" | "new" | null> {

src/commands/ssh.ts

@@ -2,6 +2,7 @@ import * as p from "@clack/prompts";
 import qrcode from "qrcode-terminal";
 import type { CliContext, CommandArgs } from "../cli/command.ts";
 import { defineCommand, defineOption, withHandler } from "../cli/command.ts";
+import { canPrompt, requireInteractive } from "../lib/interactivity.ts";
 import { findProjectContext, type ProjectContext } from "../lib/project.ts";
 import { run } from "../lib/shell.ts";
 import {
@@ -254,6 +255,13 @@ async function resolveConnection(opts: {
 }
 
 async function selectConnectionMethod(): Promise<ConnectionMethod | null> {
+  // A clack prompt cannot settle on a piped stdin and spins (HACK-1385).
+  if (!canPrompt()) {
+    requireInteractive({
+      what: "hack ssh needs a connection method",
+      hint: "Pass --tailscale, or --direct with --host <host>.",
+    });
+  }
   const selected = await p.select({
     message: "Connection method",
     options: [
@@ -283,6 +291,13 @@ async function resolveDirectHost(opts: {
     return hostname;
   }
 
+  if (!canPrompt()) {
+    requireInteractive({
+      what: "hack ssh --direct needs a host",
+      hint: "Pass --host <hostname or IP>.",
+    });
+  }
+
   const hostInput = await p.text({
     message: "SSH host (hostname or IP)",
     placeholder: "example.com or 192.168.1.100",
@@ -344,6 +359,11 @@ async function resolveSessionNameToConnect(opts: {
   readonly backend: MuxBackendName;
   readonly sessionName: string;
 } | null> {
+  // Without a TTY, answer "Done": the connection info is already printed, and
+  // a workspace can be named as the positional argument instead.
+  if (!canPrompt()) {
+    return null;
+  }
   const action = await p.select({
     message: "What would you like to do?",
     options: [
@@ -450,6 +470,12 @@ async function setupTailscale(): Promise<
         : "Tailscale is not connected"
     );
 
+    // Never run `tailscale up` without an explicit answer.
+    if (!canPrompt()) {
+      p.log.info("Run `tailscale up`, then re-run `hack ssh`.");
+      return { ok: false };
+    }
+
     const turnOn = await p.confirm({
       message: "Turn on Tailscale?",
       initialValue: true,

tests/fixtures/release-fetch-stub-preload.ts

@@ -0,0 +1,42 @@
+/**
+ * Preload for a source CLI child (`bun --preload <this> index.ts update`) that
+ * answers the GitHub "latest release" lookup with a release far newer than
+ * any real one, so `hack update` reaches its install confirmation without the
+ * network. The release lists the tarball for this platform; any other fetch
+ * (including the tarball download) throws, so nothing is downloaded.
+ */
+const LATEST_URL =
+  "https://api.github.com/repos/hack-dance/hack/releases/latest";
+const VERSION = "999.0.0";
+
+function targetSuffix(): string {
+  const platform = process.platform === "darwin" ? "darwin" : "linux";
+  const arch = process.arch === "arm64" ? "arm64" : "x86_64";
+  return `${platform}-${arch}`;
+}
+
+function requestUrl(input: unknown): string {
+  if (input instanceof Request) {
+    return input.url;
+  }
+  return String(input);
+}
+
+const stubFetch = async (input: unknown): Promise<Response> => {
+  if (requestUrl(input) !== LATEST_URL) {
+    throw new Error("release-fetch-stub: unexpected fetch");
+  }
+  return Response.json({
+    tag_name: `v${VERSION}`,
+    assets: [
+      {
+        name: `hack-${VERSION}-${targetSuffix()}.tar.gz`,
+        browser_download_url: "https://release-fetch-stub.invalid/hack.tar.gz",
+      },
+    ],
+  });
+};
+
+globalThis.fetch = Object.assign(stubFetch, {
+  preconnect: () => undefined,
+}) as unknown as typeof globalThis.fetch;

tests/fixtures/secrets-store-stub-preload.ts

@@ -0,0 +1,37 @@
+import { spyOn } from "bun:test";
+import { appendFileSync } from "node:fs";
+import { secrets } from "bun";
+
+/**
+ * Preload for a source CLI child (`bun --preload <this> index.ts secrets ...`)
+ * that replaces `Bun.secrets` with a stub, so no test reaches the OS keychain.
+ *
+ * Each call appends one line `<method> <service> <name>` to the file named by
+ * `HACK_TEST_SECRETS_LOG` (values are never written). `get` answers null,
+ * `set` stores nothing, and `delete` answers true, as if the secret existed.
+ */
+const log = process.env.HACK_TEST_SECRETS_LOG;
+if (!log) {
+  throw new Error("Missing HACK_TEST_SECRETS_LOG");
+}
+const logPath: string = log;
+
+function record(method: string, opts: unknown): void {
+  const { service, name } = (opts ?? {}) as {
+    readonly service?: unknown;
+    readonly name?: unknown;
+  };
+  appendFileSync(logPath, `${method} ${String(service)} ${String(name)}\n`);
+}
+
+spyOn(secrets, "get").mockImplementation(async (opts) => {
+  record("get", opts);
+  return null;
+});
+spyOn(secrets, "set").mockImplementation(async (opts) => {
+  record("set", opts);
+});
+spyOn(secrets, "delete").mockImplementation(async (opts) => {
+  record("delete", opts);
+  return true;
+});

tests/fixtures/session-attached-prompt-worker.ts

@@ -0,0 +1,25 @@
+import { promptAttachedWorkspaceAction } from "../../src/commands/session.ts";
+import { HackCliError } from "../../src/lib/cli-result.ts";
+
+/**
+ * Worker for tests/no-tty-prompt-sites-cli.test.ts. It calls the session
+ * "workspace is attached elsewhere" prompt in its own process, so a regressed
+ * guard spins here and is killed by the test's timeout instead of hanging the
+ * test runner. Prints `action:<value>` on success, or one `<code>: <message>`
+ * line for a HackCliError, and exits 1 on any error.
+ */
+try {
+  const action = await promptAttachedWorkspaceAction({
+    attachedWorkspaceName: "demo",
+    nextWorkspaceName: "demo-2",
+  });
+  process.stdout.write(`action:${String(action)}\n`);
+  process.exit(0);
+} catch (error: unknown) {
+  process.stderr.write(
+    error instanceof HackCliError
+      ? `${error.code}: ${error.message}\n`
+      : "E_WORKER_FAILED: unexpected error\n"
+  );
+  process.exit(1);
+}

tests/fixtures/stdout-tty-preload.ts

@@ -0,0 +1,12 @@
+/**
+ * Preload for a source CLI child (`bun --preload <this> index.ts ...`) whose
+ * stdout is a pipe: it makes `process.stdout.isTTY` report true, the shape of
+ * an interactive terminal whose stdin is piped (`echo | hack ...`). A prompt
+ * site gated on stdout alone would then enter a clack prompt that cannot
+ * settle on the piped stdin; `canPrompt()` must still refuse. Nothing else is
+ * changed.
+ */
+Object.defineProperty(process.stdout, "isTTY", {
+  configurable: true,
+  value: true,
+});