fix(cli): guard ssh/node prompts without a TTY, add secrets delete --yes, and test every prompt guard by roodboi · Pull Request #466 · hack-dance/hack

roodboi · 2026-10-24T03:18:32Z

Follow-ups to #446 (HACK-1385) from its verifier.

Root cause

@clack/prompts opens a terminal readline over stdin. On a piped stdin it never settles and spins at full CPU. #446 put canPrompt() guards in front of the known prompt sites, but three gaps were left:

The per-site guards had no regression test. Replacing the mcp guard with a no-op passed every existing test, while echo | hack mcp install spun.
hack ssh and hack node pair walkthrough called clack with no guard. hack ssh --direct and hack ssh --host <h> were confirmed to spin on a piped stdin (killed at the bound with the prompt rendered). The node walkthrough gated its prompts on isTty(), which checks stdout only, so a terminal stdout with a piped stdin still reached text/confirm. Its "Execute remote approval now?" confirm had no gate at all.
hack secrets delete <name> could not be scripted. Its fail policy refused every run that could not prompt, and no flag confirmed the deletion.

Fix

src/commands/ssh.ts, each site behind canPrompt():
- Connection method: refuses with E_INTERACTIVE_REQUIRED, naming --tailscale and --direct --host <host>.
- Direct host: refuses, naming --host.
- Follow-up "What would you like to do?": answers "Done", because the connection info is already printed and the workspace can be passed as the positional argument.
- "Turn on Tailscale?": declines with a note. It never runs tailscale up without an answer.
src/commands/node.ts:
- The walkthrough input prompts gate on canPrompt() instead of isTty().
- The execute-now confirm is skipped without a TTY. The session is left pending, using the existing "complete manually" path.
- Other isTty() uses (passing interactive to the ssh bootstrap) are not clack prompts and are unchanged.
src/commands/secrets.ts: hack secrets delete <name> --yes confirms the deletion explicitly. Without --yes, a run that cannot prompt still refuses closed with E_INTERACTIVE_REQUIRED before the keychain is touched, and the hint now names --yes. A TTY run without --yes still asks, with default no.
src/commands/session.ts: promptAttachedWorkspaceAction is exported, with a doc comment, so its guard can be tested directly. In a non-TTY spawn the picker refuses first, so this guard is unreachable through the CLI.
Docs: docs/cli.md lists secrets delete --yes and ssh --host among the scriptable forms. docs/reference/cli.md is regenerated (bun run docs:cli-reference) for the new option and description. The consumer agent guidance does not mention these commands, so the plugin bundle is unchanged.

Tests

tests/no-tty-prompt-sites-cli.test.ts (new) has 14 cases. Each spawns a real child with privateCliEnv and a stdin that is piped and closed at once, and sets no HACK_NO_INTERACTIVE. The child also gets a private HACK_HOME, HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1 and HACK_SESSIONS_MUX=none, plus a SIGKILL bound of 10 s and a 30 s test timeout. Each case asserts no signal, wall time under the bound, no clack ◆ prompt rendered, and the policy outcome:

Site	Command	Expected
mcp `resolveTargets`	`mcp print`	`E_INTERACTIVE_REQUIRED` naming `--all`/`--cursor`/`--claude`
session picker	`session` (one registered project)	refusal naming `hack session list` / `attach <name>`
session attached workspace	worker `tests/fixtures/session-attached-prompt-worker.ts`	refusal naming `--new`
update `confirmInstall`	`update --json` with a fetch-stub preload (release v999) and a stdout-TTY preload	`{"ok":false,"error":"Non-interactive shell; re-run with --yes …"}`; the stand-in binary is untouched
secrets name	`secrets get`	refusal; the `Bun.secrets` stub log is empty
secrets value	`secrets set <name>`	refusal; stub log empty
secrets delete, no `--yes`	`secrets delete <name>`	refusal naming `--yes`; stub log empty
secrets delete `--yes`	`secrets delete <name> --yes` (plus `HACK_NO_INTERACTIVE=1`)	exit 0, `Deleted secret`, stub log `delete hack-cli <name>`
agent client picker	`agent init --path <repo>`	exit 0, prompt printed, no picker
ssh method	`ssh`	refusal naming `--tailscale`/`--direct`/`--host`
ssh host	`ssh --direct`	refusal naming `--host`
ssh follow-up	`ssh --host <h>`	exit 0, "Copy the SSH command above"
node walkthrough input	`node pair walkthrough` with a stdout-TTY preload	exit 1, `Missing --source`
node execute-now confirm	`node pair walkthrough --source … --endpoint <local Bun.serve>`	exit 0, "Session left pending"

New fixtures:

tests/fixtures/secrets-store-stub-preload.ts: a Bun.secrets stub, so no case reaches the OS keychain, even under mutation.
tests/fixtures/release-fetch-stub-preload.ts: answers only the GitHub latest-release URL. Any other fetch throws, so nothing is downloaded.
tests/fixtures/stdout-tty-preload.ts: a terminal stdout with a piped stdin. It proves the stdin half of canPrompt().

tests/global-command.macos.test.ts now counts calls that reach the mocked clack confirm. Two TTY-less cases (stdin and stdout isTTY=false, no opt-out) cover the two policies global commands use:

accept-default: the global trust keychain case is now test.each over --no-interactive and no-TTY. Both assert the sudo keychain install runs and confirm is never called.
decline: global authorize without a TTY skips the sudoers install, although the prompt's default is yes, and never calls confirm.
No global command uses the fail policy. fail without a TTY is covered by the secrets cases above and by the existing tests/interactivity.test.ts and tests/env-unset-no-tty-cli.test.ts.

Mutation proof

Each guard was mutated alone: the guard was made a no-op, or the original gate was restored where one existed. Then only its test was run. All 17 mutations were caught:

Mutation	Result
mcp, session picker, session attached, secrets name/value, agent, ssh method/host/follow-up guards → `if (false)`	each test fails: child SIGKILLed at 10 s
update `isTty: canPrompt()` → `true`	fails: SIGKILL
update → `process.stdout.isTTY === true` (the pre-#446 gate)	fails: SIGKILL
node input `canPrompt()` → `isTty()` (the old gate)	fails: SIGKILL
node execute-now confirm guard removed	fails: SIGKILL
secrets delete `--yes` ignored	`--yes` test fails (exit 1, expected 0)
secrets delete `--yes` always on	refusal test fails (exit 0, expected 1)
`confirmSafe` gate → `!isExplicitlyNonInteractive()` (pre-#446)	both new global-command cases fail; the other 31 still pass

Evidence

Bun 1.4.2 (mise x bun@1.4.2), bun install --frozen-lockfile, no bun.lock change.
bun run --cwd packages/cli typecheck: 0 errors (covers tests/).
Root bun run check, which includes privacy:check: 2/2 tasks successful, privacy-check: ok.
ultracite check on every changed file: clean.
Focused run of 20 files: 181 pass, 0 fail (183 tests). The files are the new test, cli-spawn-env-isolation, global-command.macos, env-unset-no-tty-cli, interactivity, ssh-command, the session-* files, node-pair-command, node-workspace-command, node-attach-instructions, cli-per-command-options, cli-reference-docs, update-command, mcp-install, mcp, agent-onboard-command and doctor-fix-noninteractive.
gitleaks 8.30.1 over the exported origin/next..HEAD range: no leaks.
Before the fix, echo | hack ssh --direct and echo | hack ssh --host example.test were killed at a 6 s / 30 s bound with the clack prompt rendered. After the fix, both settle in about 1–3 s (CLI cold start) with the policy outcome above.
Full suite: hosted CI test job at the exact PR head 3a3efd24 passed: 8109 pass, 0 fail (8266 tests across 575 files, including tests/no-tty-prompt-sites-cli.test.ts; run 40057772445). The docker-e2e shards 1–4, the ubuntu Candidate core and Config compiler jobs, runtime models, linux-process-lifetime, runtime-images and secret-scan also passed. The macOS Candidate core and Config compiler jobs were still queued for a runner when this was written.

Gaps

The session attached-workspace guard is tested through a worker that imports the exported function. In a spawned non-TTY CLI the picker guard always fires first, so this guard is defense in depth.
node.ts still passes interactive: isTty() to the ssh key bootstrap (ensureNodePairSshReady). That path runs ssh with an inherited stdin, not a clack prompt, so it is out of scope here. ssh/node are unsupported surfaces (AGENTS.md); only their clack prompt sites were guarded.
Each CLI spawn takes about 2.5–3 s here (cold module graph), so the new file takes about 40 s. The 10 s kill bound leaves headroom for slower hosted runners.
No fail-policy confirmSafe site exists in global commands, so global-command.macos.test.ts covers decline and accept-default only.

Refs HACK-1410

🤖 Generated with Claude Code

…yes, and test every prompt guard Follow-ups to #446 (HACK-1385): - Add one bounded non-TTY spawn test per canPrompt() guard (mcp target selection, session picker and attached-workspace choice, update install confirmation, secrets name/value prompts, agent client picker), each proven by mutation. - Guard the clack prompts in `hack ssh` (connection method, host, follow-up choice, Tailscale start) and `hack node pair walkthrough` (input prompts now gate on canPrompt() instead of stdout alone; the execute-now confirm leaves the session pending without a TTY). - `hack secrets delete <name> --yes` confirms the deletion explicitly; without it a TTY-less run still refuses closed with E_INTERACTIVE_REQUIRED, and the hint now names --yes. - tests/global-command.macos.test.ts keeps a TTY-less case for the decline and accept-default policies and asserts the prompt is never entered. Refs HACK-1410 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

linear-code · 2026-10-24T03:18:37Z

HACK-1410

@t

…urate gitignore entry, no-TTY Tailscale test) (#479) The six small CLI follow-ups in HACK-1426, from the verifiers of #466, #441, #453, #460 and #462. ## Root cause 1. **Tailscale guard had no test.** The `canPrompt()` guard in front of "Turn on Tailscale?" in `setupTailscale` (`src/commands/ssh.ts`) had no case in `tests/no-tty-prompt-sites-cli.test.ts`. If you changed it to `if (false)`, every ssh case still passed. 2. **Cold cache in the first case.** Each run of that file uses a fresh private HOME. Bun keeps its runtime transpiler cache under HOME (`~/Library/Caches/bun/@t@` on macOS), so the first spawn started cold inside its 10 s kill bound. On a loaded host that first case took 6.3 s and was SIGKILLed. 3. **`hack up` dropped the error code.** For a missing project, `runUpCommand` caught `MissingProjectContextError` and, without `--json`, logged only its message. `down`, `restart`, `ps`, `logs` and `open` let the error reach the CLI's closed-error renderer, which prints `E_PROJECT_NOT_FOUND: ...`. 4. **Indented `.gitignore` entry counted as present.** `ensureGitignoreEntry` (`src/lib/fs.ts`) compared trimmed lines. Git keeps leading whitespace as part of the pattern, so ` .hack.secret.key` does not ignore the key. Hack counted that line as the entry, wrote nothing, then found the key not ignored. It then refused with the "a later rule re-includes the key" message, which names the wrong cause. 5. **ACL test could leave mode 0444.** In `tests/git-info-exclude.test.ts`, the macOS ACL test ran `chmod 0444`, the ACL grant and `access(W_OK)` before the `try` whose `finally` restores mode 0644. If any of those steps failed, the file kept mode 0444. 6. **Paired fixture lost a case.** #462 gave the paired `unsearchable` fixture a `.hack/.internal` directory. After that, nothing covered a `.hack` at mode 000 with no `.hack/.internal`, which is the most common real layout. ## Fix 1. New paired no-TTY case: `ssh --tailscale` runs against a logging `tailscale` stand-in placed first on PATH. The stand-in reports `{"BackendState":"Stopped"}`. The case expects exit 1, the `Run \`tailscale up\`, then re-run \`hack ssh\`.` hint, no prompt, and no `tailscale up` call. 2. `beforeAll` now runs one warm-up child (`index.ts --help`) under the file's private HOME. It has its own named budget: `WARM_UP_KILL_AFTER_MS` = 90 s, and the hook bound is that plus 10 s. A kill fails the file with a clear message. The cases keep the 10 s bound. 3. `runUpCommand` now handles `MissingProjectContextError` itself only under `--json`. Without `--json` it rethrows, so `hack up` prints `ERROR: E_PROJECT_NOT_FOUND: No .hack/ (or legacy .dev/) found. Run: hack init` (console logger), the same as the other lifecycle commands. `docs/cli.md` already promised this. 4. `ensureGitignoreEntry` now reads each line the way git does: it drops a trailing CR and trailing spaces, and keeps leading whitespace. An indented line no longer counts as the entry, so Hack appends the exact entry, git ignores the key, and the secret write goes ahead. The "re- included" refusal now appears only when the exact entry is present and git still does not ignore the key, which is what its message says. Tabs are not trimmed, which matches git's `trim_trailing_spaces`. 5. The ACL test's mode change, ACL grant and `access` probe now run inside the `try`. 6. New `unsearchable-no-internal` fixture (never adopted, no `.hack/.internal`). Its `.hack` at mode 000 is the first case in the paired CLI/MCP loop. The proof-matrix row in `docs/reference/ax-target-result-contract.md` now names this case. ## Tests and mutation evidence For each mutation I applied it, ran the focused tests, and restored the file. - **`if (!canPrompt())` changed to `if (false)` in `setupTailscale`:** the new case fails. The child was SIGKILLed at the 10 s bound with the prompt open. The other 3 ssh cases pass. The stand-in recorded no `up` call. - **`up`'s non-JSON branch reverted:** 2 tests fail, `up -p <empty dir> without --json …` and `up from a working directory without a project …`. The same new loop over `down`, `restart`, `ps`, `logs` and `open` still passes. - **`line.trim() === entryLine` restored in `ensureGitignoreEntry`:** 2 tests fail, the CLI case "an indented entry … is completed by an exact entry instead of being refused as re-included" and the unit case "ensureGitignoreEntry reads each line as git does …". - **Exact comparison, `line === entryLine`:** the unit case fails on the trailing-space and CRLF inputs. - **EACCES/EPERM no longer mapped to `permission` in the adoption marker's `classifyStateRootFailure`:** the paired loop fails on its first case, `unsearchable-no- internal` with `.hack` at 000. It gets `adoption_state_unsafe` / `E_CONFIG_INVALID` where `input_permission_denied` / `E_PERMISSION` is expected. - **Warm-up removed (timing, idle host):** the first case took 1.10 s without the warm-up and 0.65 s with it. The later cases are unchanged. ## Validation - Focused tests: 6 files (`no-tty-prompt-sites-cli`, `missing-project-error-code`, `project-env- key-ignore`, `project-target-inspect`, `git-info-exclude`, `doctor-generated-files`), 121 pass and 0 fail. `project-up-command` also passes. Runs used a private HOME, HACK_HOME and DOCKER_CONFIG, and `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`, as a non-root user on macOS. - `bun run --cwd packages/cli typecheck`: 0 errors. - Root `bun run check`: `privacy-check: ok`. `@hack/cli:check` shows the 34 warnings that were already there. Biome finds no issues in the changed files. - `bun run docs:cli-reference`: no diff. Only an error line changed, not the CLI surface. No consumer agent guidance changed. - gitleaks 8.30.1 (`gitleaks dir` on the exported `git log -p origin/next..HEAD`): no leaks found. - Full suite: the hosted CI `test` job (all shards) at this PR head. ## Gaps - I did not reproduce item 2 under load. The evidence is that the warm-up moves the cold start out of the first case: on an idle host, 1.10 s became 0.65 s. The reported failure was 6.3 s at load about 60. - Item 5 is a structural move with unchanged assertions. No mutation tells the old and new placement apart without failing the ACL setup itself. - As in #462, no mutation tells the no-`.internal` case apart from the `.hack` 000 case with `.internal`. Both get EACCES from the same state-root `lstat`. The new case is live (it fails first under the classifier mutation) but not independent. - Locally, `tests/native-compose-adoption-search-depth.test.ts` "a pending adoption more than 64 directories above …" fails in its setup with `E_LEGACY_ADOPTION_DEADLINE`. It fails the same way with `origin/next`'s `src/`, so this change did not cause it. Hosted CI decides. - Not run as root. The mode-dependent cases are skipped as root, as before. Refs HACK-1426 🤖 Generated with [Claude Code](https://claude.com/claude-code)

roodboi mentioned this pull request Oct 24, 2026

train b52 integration gate (do not merge) #472

Closed

roodboi marked this pull request as ready for review October 24, 2026 05:48

roodboi merged commit f678e8f into next Oct 24, 2026
17 checks passed

roodboi deleted the wf/g76-prompt-site-guards branch October 24, 2026 05:48

roodboi mentioned this pull request Oct 24, 2026

fix(cli): close HACK-1426 follow-ups (up E_PROJECT_NOT_FOUND, git-accurate gitignore entry, no-TTY Tailscale test) #479

Merged

Navigation Menu

Repository navigation

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

fix(cli): guard ssh/node prompts without a TTY, add secrets delete --yes, and test every prompt guard#466
roodboi merged 1 commit into
nextfrom
wf/g76-prompt-site-guards

roodboi commented Oct 24, 2026 •

edited

Loading

Uh oh!

linear-code Bot commented Oct 24, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

roodboi commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause

Fix

Tests

Mutation proof

Evidence

Gaps

Uh oh!

linear-code Bot commented Oct 24, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

roodboi commented Oct 24, 2026 •

edited

Loading