From 51d9a9ea91b93bba2c69d3fbf5c4c8b8513f6d40 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 1 Aug 2026 00:53:00 -0400 Subject: [PATCH] feat: extend the no-terminal guard to every command that prompts seamless init stopped rendering prompts to a pipe in #158; every other command still hung there until its job timed out. Every prompt is now fronted by requireInteractive, so a run without a TTY on stdin fails naming the flag that answers the question: login, profile add, users delete, users prepare-device-replacement, sessions revoke, org members remove, config apply, and config oauth-providers remove. A guard with nothing to point at would only convert a hang into a dead end, so the destructive confirmations now take --force, matching what init means by it. They share confirmDestructive, which answers itself when forced and otherwise asks. hasForceFlag also accepts --yes and -y, because config oauth-providers remove --yes shipped before the convention existed. --force does not override --dry-run, and cancelling still reads as declining rather than as an error. The one-time code at login has no flag form (it does not exist until the code is sent), so that step reports that it genuinely needs a terminal. Tests get a setup file that marks stdin a TTY, since the guard would otherwise fail every test that exercises a prompt; the no-terminal tests opt back out. Closes #159 --- .changeset/require-tty-everywhere.md | 23 ++++++ AGENTS.md | 16 ++-- README.md | 22 ++++++ src/commands/config.test.ts | 54 +++++++++++++ src/commands/config.ts | 34 +++++---- src/commands/helpTopics.ts | 40 +++++++--- src/commands/init.test.ts | 7 -- src/commands/login.test.ts | 10 +++ src/commands/org.test.ts | 23 ++++++ src/commands/org.ts | 12 ++- src/commands/profile.test.ts | 18 +++++ src/commands/profile.ts | 9 +++ src/commands/sessions.test.ts | 28 +++++++ src/commands/sessions.ts | 22 ++++-- src/commands/users.test.ts | 32 ++++++++ src/commands/users.ts | 19 +++-- src/core/confirmAction.test.ts | 109 +++++++++++++++++++++++++++ src/core/confirmAction.ts | 47 ++++++++++++ src/core/interactiveLogin.ts | 11 +++ src/prompts/appSelect.test.ts | 15 +--- src/prompts/projectSetup.test.ts | 7 -- src/testSetup.ts | 9 +++ vitest.config.ts | 3 +- 23 files changed, 493 insertions(+), 77 deletions(-) create mode 100644 .changeset/require-tty-everywhere.md create mode 100644 src/core/confirmAction.test.ts create mode 100644 src/core/confirmAction.ts create mode 100644 src/testSetup.ts diff --git a/.changeset/require-tty-everywhere.md b/.changeset/require-tty-everywhere.md new file mode 100644 index 0000000..b0b523d --- /dev/null +++ b/.changeset/require-tty-everywhere.md @@ -0,0 +1,23 @@ +--- +"seamless-cli": minor +--- + +No command renders a prompt when stdin is not a terminal. `seamless init` got this in 0.11.0; it now +covers every command that prompts (`login`, `profile add`, `users delete`, +`users prepare-device-replacement`, `sessions revoke`, `org members remove`, `config apply`, and +`config oauth-providers remove`). Each one stops naming the flag that answers the question, so a CI +step or a scripted run fails immediately instead of hanging until its job times out. + +The confirmations that guard a destructive action now take `--force`, matching what `init` already +means by it: + +```bash +seamless users delete --force +seamless sessions revoke --all --force +seamless org members remove --force +seamless config apply config.json --force +``` + +`--yes` and `-y` are accepted aliases everywhere, so `seamless config oauth-providers remove --yes` +keeps working. `--force` does not override `--dry-run`: `config apply --dry-run --force` still +changes nothing, and cancelling a confirmation still reads as declining rather than as an error. diff --git a/AGENTS.md b/AGENTS.md index 8c899e1..576449c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,11 +84,10 @@ The entry point is [src/index.ts](src/index.ts), which dispatches to a command m managed application and a local stack requires `--app` or `--local`. Flag parsing lives in `parseInitArgs` ([src/index.ts](src/index.ts)); everything it produces is validated in `runCLI` before a directory is created. - - Every prompt in the init flow is fronted by `requireInteractive` - ([src/core/tty.ts](src/core/tty.ts)), so a run without a TTY on stdin fails naming the flag that - answers the question instead of rendering a prompt nobody can answer. When adding a prompt to - this flow, guard it the same way. The other commands that prompt (`login`, `profile`, - `config apply`, `users delete`, `sessions revoke`) are not guarded yet, see #153. + - Every prompt is fronted by `requireInteractive` ([src/core/tty.ts](src/core/tty.ts)), so a run + without a TTY on stdin fails naming the flag that answers the question instead of rendering a + prompt nobody can answer. This holds across every command, not just `init`. When adding a + prompt anywhere, guard it the same way. - **templates** ([src/commands/templates.ts](src/commands/templates.ts)) lists the registry (`seamless templates list [--json]`) so those ids and flags are discoverable without a checkout. It reads the same source `init` does and needs no login. @@ -97,6 +96,13 @@ The entry point is [src/index.ts](src/index.ts), which dispatches to a command m [src/core/oauthProviders.ts](src/core/oauthProviders.ts)). The chosen providers are wired into the auth server env (`OAUTH_PROVIDERS`, per-provider `*_CLIENT_SECRET`, the `oauth` login method) by `buildAuthEnv` in [src/generators/docker/docker.ts](src/generators/docker/docker.ts). +- **destructive confirmations** go through `confirmDestructive` + ([src/core/confirmAction.ts](src/core/confirmAction.ts)), which answers itself when `--force` is + set and otherwise asks. `--force` is the standing spelling for "do it without asking"; + `hasForceFlag` also accepts `--yes` and `-y`, because `config oauth-providers remove --yes` + shipped before the convention existed. `--yes` means something narrower on `init` (answer the + ordinary questions, never the destructive ones), so do not add `--yes` alone to a destructive + step. Cancelling a confirmation reads as declining, not as an error. - **check** health-checks a running stack (local or managed). - **verify** ([src/commands/verify.ts](src/commands/verify.ts)) runs the conformance harness (below). - **instance management** — `profile` (targets, plus `profile login`), diff --git a/README.md b/README.md index 02531e4..a030e5e 100644 --- a/README.md +++ b/README.md @@ -456,6 +456,28 @@ seamless whoami Because `/refresh` rotates the refresh token on every call, this path is best for a single invocation; the rotated token is held only in memory for that process. +No command will render a prompt when stdin is not a terminal. Each one stops instead, naming the +flag that answers the question, so a CI step fails immediately rather than hanging until its job +times out. + +The commands that ask before destroying something take `--force` (with `--yes` and `-y` accepted as +aliases), which is both how you skip the confirmation and how you run them unattended: + +```bash +seamless users delete --force +seamless sessions revoke --all --force +seamless org members remove --force +seamless config apply config.json --force +seamless config oauth-providers remove google --force +``` + +`--force` never overrides `--dry-run`: `config apply --dry-run --force` still changes nothing. + +The prompts that gather input rather than confirm an action are answered by the flags those +commands already take, for example `seamless login --identifier you@example.com` and +`seamless profile add prod --instance-url https://auth.example.com`. The one-time code at login is +the exception: it only exists after the code is sent, so that step genuinely needs a terminal. + --- ## What is configured for you diff --git a/src/commands/config.test.ts b/src/commands/config.test.ts index e86c4d9..7ed78fb 100644 --- a/src/commands/config.test.ts +++ b/src/commands/config.test.ts @@ -674,3 +674,57 @@ describe("runConfig oauth-providers", () => { expect(errOutput()).toContain("Could not read file: missing.json"); }); }); + +describe("config --force", () => { + beforeEach(() => { + vi.mocked(fs.readFileSync).mockReturnValue( + JSON.stringify({ app_name: "New" }), + ); + }); + + it.each(["--force", "--yes", "-y"])( + "applies without confirming when given %s", + async (flag) => { + const { client } = fakeClient(() => + response(200, { app_name: "Old", updatedKeys: ["app_name"] }), + ); + vi.mocked(createAuthClient).mockResolvedValue(client); + + await runConfig(["apply", "local.json", flag]); + + expect(vi.mocked(confirm)).not.toHaveBeenCalled(); + expect(output()).toContain("Applied."); + }, + ); + + // --dry-run means "show me, change nothing", which --force does not override. + it("still applies nothing with --dry-run --force", async () => { + const { client } = fakeClient(() => response(200, { app_name: "Old" })); + vi.mocked(createAuthClient).mockResolvedValue(client); + + await runConfig(["apply", "local.json", "--dry-run", "--force"]); + + expect(output()).toContain("Dry run: no changes applied."); + expect(vi.mocked(confirm)).not.toHaveBeenCalled(); + }); + + it("refuses to ask without a terminal, naming --force", async () => { + process.stdin.isTTY = false; + const { client } = fakeClient(() => response(200, { app_name: "Old" })); + vi.mocked(createAuthClient).mockResolvedValue(client); + + await expect(runConfig(["apply", "local.json"])).rejects.toThrow( + /needs an interactive terminal[\s\S]*--force/, + ); + }); + + it("removes an OAuth provider without confirming when forced", async () => { + const { client } = fakeClient(() => response(200, { message: "ok" })); + vi.mocked(createAuthClient).mockResolvedValue(client); + + await runConfig(["oauth-providers", "remove", "google", "--force"]); + + expect(vi.mocked(confirm)).not.toHaveBeenCalled(); + expect(output()).toContain("Removed OAuth provider: google"); + }); +}); diff --git a/src/commands/config.ts b/src/commands/config.ts index 5e85bc7..a558dc9 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -1,5 +1,4 @@ import fs from "fs"; -import { confirm, isCancel } from "@clack/prompts"; import kleur from "kleur"; import { extractFlag } from "../core/args.js"; import { createAuthClient, ReauthRequiredError, type AuthClient } from "../core/authClient.js"; @@ -20,6 +19,10 @@ import { type OAuthProvider, type SystemConfig, } from "../core/systemConfig.js"; +import { + confirmDestructive, + hasForceFlag, +} from "../core/confirmAction.js"; export async function runConfig(args: string[]): Promise { const sub = args[0]; @@ -180,13 +183,14 @@ async function configApply(client: AuthClient, rest: string[]): Promise { return; } - const proceed = await confirm({ + const proceed = await confirmDestructive({ message: `Apply ${changes.length} change${ changes.length === 1 ? "" : "s" } to ${client.profile.instanceUrl}?`, - initialValue: false, + force: hasForceFlag(rest), + remedy: "Pass --force to apply without confirming.", }); - if (isCancel(proceed) || !proceed) { + if (!proceed) { console.log("Cancelled."); return; } @@ -304,24 +308,24 @@ async function oauthProvidersRemove( client: AuthClient, rest: string[], ): Promise { - const skipConfirm = rest.includes("--yes") || rest.includes("-y"); const id = rest.find((arg) => !arg.startsWith("-")); if (!id) { console.error( - kleur.red("Usage: seamless config oauth-providers remove [--yes]"), + kleur.red("Usage: seamless config oauth-providers remove [--force]"), ); process.exit(1); } - if (!skipConfirm) { - const proceed = await confirm({ - message: `Remove OAuth provider "${id}" from ${client.profile.instanceUrl}?`, - initialValue: false, - }); - if (isCancel(proceed) || !proceed) { - console.log("Cancelled."); - return; - } + const proceed = await confirmDestructive({ + message: `Remove OAuth provider "${id}" from ${client.profile.instanceUrl}?`, + // This subcommand shipped with --yes before --force was the convention, so + // hasForceFlag keeps accepting it. + force: hasForceFlag(rest), + remedy: "Pass --force to remove it without confirming.", + }); + if (!proceed) { + console.log("Cancelled."); + return; } await deleteOAuthProvider(client, id); diff --git a/src/commands/helpTopics.ts b/src/commands/helpTopics.ts index 617dba9..4a55503 100644 --- a/src/commands/helpTopics.ts +++ b/src/commands/helpTopics.ts @@ -267,7 +267,10 @@ instead. Fails cleanly if not logged in.`, }, { name: "sessions", - usage: ["seamless sessions [list]", "seamless sessions revoke "], + usage: [ + "seamless sessions [list]", + "seamless sessions revoke [--force]", + ], sections: [ { heading: "sessions [list]", @@ -275,9 +278,13 @@ instead. Fails cleanly if not logged in.`, marked. Shows the session id, device or user agent, IP, and last-used time.`, }, { - heading: "sessions revoke ", + heading: "sessions revoke [--force]", body: `Revoke one session by id, or every session with --all. Revoking the current -session (or --all) prompts for confirmation and then clears local tokens.`, +session (or --all) prompts for confirmation and then clears local tokens. + +--force + • Skip that confirmation (--yes and -y are accepted aliases), which is also + what lets this run without a terminal attached`, }, ], }, @@ -303,15 +310,21 @@ config roles [--json] config diff • Show how a local JSON config file differs from the instance -config apply [--dry-run] +config apply [--dry-run] [--force] • Apply a local JSON config file after a confirmation prompt + • --force skips the confirmation; --dry-run still changes nothing config oauth-providers • Manage OAuth providers one at a time. Client secrets stay server-side, referenced by clientSecretEnv; the secret value is never sent. (for example: config oauth-providers add --file google.json, config oauth-providers update google '{"enabled":false}', - config oauth-providers remove google)`, + config oauth-providers remove google --force) + +--force + • Skips the confirmation on apply and oauth-providers remove (--yes and -y + are accepted aliases), which is also what lets them run without a + terminal attached`, }, ], }, @@ -327,12 +340,17 @@ config oauth-providers users list [--limit ] [--offset ] [--json] • List users -users delete +users delete [--force] • Delete a user (asks for confirmation) users credentials [--json] • Show a user's registered credentials -users prepare-device-replacement [--keep-sessions] [--keep-passkeys] [--keep-totp] - • Admin-assisted account recovery (needs an elevated session)`, +users prepare-device-replacement [--force] [--keep-sessions] [--keep-passkeys] [--keep-totp] + • Admin-assisted account recovery (needs an elevated session) + +--force + • Skips the confirmation on delete and prepare-device-replacement (--yes and + -y are accepted aliases), which is also what lets them run without a + terminal attached`, }, ], }, @@ -355,7 +373,11 @@ org update [--name ] [--slug ] org members list [--json] org members add (--user | --email ) [--roles a,b] [--scopes a,b] org members update [--roles a,b] [--scopes a,b] -org members remove `, +org members remove [--force] + +--force + • Skips the confirmation on members remove (--yes and -y are accepted + aliases), which is also what lets it run without a terminal attached`, }, ], }, diff --git a/src/commands/init.test.ts b/src/commands/init.test.ts index dcf934d..260706d 100644 --- a/src/commands/init.test.ts +++ b/src/commands/init.test.ts @@ -195,16 +195,10 @@ function app(over: Record = {}) { }; } -// Captured before any test flips it, so a suite run leaves the process as it -// found it. -const ORIGINAL_TTY = process.stdin.isTTY; - let logs: string[]; beforeEach(() => { vi.clearAllMocks(); - // The prompt paths refuse to run without a terminal, and vitest has none. - process.stdin.isTTY = true; logs = []; vi.spyOn(console, "log").mockImplementation((msg?: unknown) => { logs.push(String(msg ?? "")); @@ -222,7 +216,6 @@ beforeEach(() => { }); afterEach(() => { - process.stdin.isTTY = ORIGINAL_TTY; vi.restoreAllMocks(); }); diff --git a/src/commands/login.test.ts b/src/commands/login.test.ts index 94e118f..1c67a4c 100644 --- a/src/commands/login.test.ts +++ b/src/commands/login.test.ts @@ -538,3 +538,13 @@ describe("runLogin: failure handling", () => { expect(exitSpy).not.toHaveBeenCalled(); }); }); + +describe("runLogin without a terminal", () => { + it("refuses the identifier prompt, naming --identifier", async () => { + process.stdin.isTTY = false; + + await expect(runLogin([])).rejects.toThrow( + /"Email or phone" needs an interactive terminal[\s\S]*--identifier/, + ); + }); +}); diff --git a/src/commands/org.test.ts b/src/commands/org.test.ts index fab726b..cb3b4d5 100644 --- a/src/commands/org.test.ts +++ b/src/commands/org.test.ts @@ -419,3 +419,26 @@ describe("runOrg members remove", () => { expect(logs()).toContain("Removed user u1 from o1."); }); }); + +describe("org members remove --force", () => { + it.each(["--force", "--yes", "-y"])( + "removes without confirming when given %s", + async (flag) => { + vi.mocked(removeMember).mockResolvedValue(undefined as never); + + await runOrg(["members", "remove", "o1", "u1", flag]); + + expect(vi.mocked(confirm)).not.toHaveBeenCalled(); + expect(logs()).toContain("Removed user u1 from o1."); + }, + ); + + it("refuses to ask without a terminal, naming --force", async () => { + process.stdin.isTTY = false; + + await expect(runOrg(["members", "remove", "o1", "u1"])).rejects.toThrow( + /needs an interactive terminal[\s\S]*--force/, + ); + expect(vi.mocked(removeMember)).not.toHaveBeenCalled(); + }); +}); diff --git a/src/commands/org.ts b/src/commands/org.ts index a886406..d98a73b 100644 --- a/src/commands/org.ts +++ b/src/commands/org.ts @@ -1,4 +1,3 @@ -import { confirm, isCancel } from "@clack/prompts"; import kleur from "kleur"; import { extractFlag } from "../core/args.js"; import { createAuthClient, type AuthClient } from "../core/authClient.js"; @@ -14,6 +13,10 @@ import { type Json, } from "../core/admin.js"; import { parseList, reportAdminError } from "./adminShared.js"; +import { + confirmDestructive, + hasForceFlag, +} from "../core/confirmAction.js"; export async function runOrg(args: string[]): Promise { if (args[0] === "members") { @@ -246,11 +249,12 @@ async function membersRemove(client: AuthClient, rest: string[]): Promise process.exit(1); } - const proceed = await confirm({ + const proceed = await confirmDestructive({ message: `Remove user ${userId} from organization ${orgId}?`, - initialValue: false, + force: hasForceFlag(rest), + remedy: "Pass --force to remove the member without confirming.", }); - if (isCancel(proceed) || !proceed) { + if (!proceed) { console.log("Cancelled."); return; } diff --git a/src/commands/profile.test.ts b/src/commands/profile.test.ts index f47e0a0..f00d333 100644 --- a/src/commands/profile.test.ts +++ b/src/commands/profile.test.ts @@ -361,3 +361,21 @@ describe("profile login", () => { expect(vi.mocked(text)).toHaveBeenCalledTimes(1); }); }); + +describe("profile add without a terminal", () => { + it("refuses the name prompt, pointing at the positional form", async () => { + process.stdin.isTTY = false; + + await expect(runProfile(["add"])).rejects.toThrow( + /"Profile name" needs an interactive terminal[\s\S]*seamless profile add /, + ); + }); + + it("refuses the instance URL prompt, naming --instance-url", async () => { + process.stdin.isTTY = false; + + await expect(runProfile(["add", "prod"])).rejects.toThrow( + /"Instance URL" needs an interactive terminal[\s\S]*--instance-url/, + ); + }); +}); diff --git a/src/commands/profile.ts b/src/commands/profile.ts index bb81627..67735e9 100644 --- a/src/commands/profile.ts +++ b/src/commands/profile.ts @@ -14,6 +14,7 @@ import { } from "../core/config.js"; import { deleteTokens, KeychainUnavailableError } from "../core/keychain.js"; import { loginToInstance } from "./instanceLogin.js"; +import { requireInteractive } from "../core/tty.js"; export async function runProfile(args: string[]): Promise { const sub = args[0]; @@ -82,6 +83,10 @@ async function profileAdd(rest: string[]): Promise { intro("Add a Seamless profile"); if (!name) { + requireInteractive( + "Profile name", + "Pass the name positionally: seamless profile add --instance-url .", + ); const answer = await text({ message: "Profile name", placeholder: DEFAULT_PROFILE_NAME, @@ -102,6 +107,10 @@ async function profileAdd(rest: string[]): Promise { } if (!instanceUrl) { + requireInteractive( + "Instance URL", + "Pass --instance-url .", + ); const answer = await text({ message: "Instance URL", placeholder: "https://auth.example.com", diff --git a/src/commands/sessions.test.ts b/src/commands/sessions.test.ts index 77fc78b..9ddada2 100644 --- a/src/commands/sessions.test.ts +++ b/src/commands/sessions.test.ts @@ -327,3 +327,31 @@ describe("runSessions revoke ", () => { expect(output()).toContain("Run seamless login to sign in again."); }); }); + +describe("sessions --force", () => { + it.each(["--force", "--yes", "-y"])( + "revokes every session without confirming when given %s", + async (flag) => { + const client = fakeClient(() => response(200, { message: "ok" })); + vi.mocked(createAuthClient).mockResolvedValue(client); + + await runSessions(["revoke", "--all", flag]); + + expect(confirm).not.toHaveBeenCalled(); + expect(clearLocalSession).toHaveBeenCalledWith(client.profile); + expect(output()).toContain("Revoked all sessions."); + }, + ); + + it("refuses to ask without a terminal, naming --force", async () => { + process.stdin.isTTY = false; + const client = fakeClient(() => response(200, { message: "ok" })); + vi.mocked(createAuthClient).mockResolvedValue(client); + + await expect(runSessions(["revoke", "--all"])).rejects.toThrow( + "process.exit(1)", + ); + expect(errOutput()).toMatch(/needs an interactive terminal[\s\S]*--force/); + expect(clearLocalSession).not.toHaveBeenCalled(); + }); +}); diff --git a/src/commands/sessions.ts b/src/commands/sessions.ts index d7afd95..8bdb3eb 100644 --- a/src/commands/sessions.ts +++ b/src/commands/sessions.ts @@ -1,4 +1,3 @@ -import { confirm, isCancel } from "@clack/prompts"; import kleur from "kleur"; import { extractFlag } from "../core/args.js"; import { createAuthClient, ReauthRequiredError, type AuthClient } from "../core/authClient.js"; @@ -9,6 +8,10 @@ import { revokeSessionById, type SessionInfo, } from "../core/sessions.js"; +import { + confirmDestructive, + hasForceFlag, +} from "../core/confirmAction.js"; export async function runSessions(args: string[]): Promise { const sub = args[0] === "list" || args[0] === "revoke" ? args[0] : undefined; @@ -49,12 +52,13 @@ async function revoke(client: AuthClient, positional: string[]): Promise { const id = positional.find((arg) => !arg.startsWith("--")); if (all) { - const proceed = await confirm({ + const proceed = await confirmDestructive({ message: "Revoke every session, including this one? You will be signed out here.", - initialValue: false, + force: hasForceFlag(positional), + remedy: "Pass --force to revoke every session without confirming.", }); - if (isCancel(proceed) || !proceed) { + if (!proceed) { console.log("Cancelled."); return; } @@ -83,11 +87,13 @@ async function revoke(client: AuthClient, positional: string[]): Promise { const isCurrent = sessions.find((session) => session.id === id)?.current ?? false; if (isCurrent) { - const proceed = await confirm({ - message: "This is your current session. Revoking it signs you out here. Continue?", - initialValue: false, + const proceed = await confirmDestructive({ + message: + "This is your current session. Revoking it signs you out here. Continue?", + force: hasForceFlag(positional), + remedy: "Pass --force to revoke it without confirming.", }); - if (isCancel(proceed) || !proceed) { + if (!proceed) { console.log("Cancelled."); return; } diff --git a/src/commands/users.test.ts b/src/commands/users.test.ts index b0eb566..3b57fc0 100644 --- a/src/commands/users.test.ts +++ b/src/commands/users.test.ts @@ -322,3 +322,35 @@ describe("runUsers prepare-device-replacement", () => { expect(logs()).toContain("Revoked sessions: 0, removed credentials: 0, disabled TOTP: 0"); }); }); + +describe("users --force", () => { + it.each(["--force", "--yes", "-y"])( + "deletes without confirming when given %s", + async (flag) => { + vi.mocked(deleteUser).mockResolvedValue(undefined); + + await runUsers(["delete", "u1", flag]); + + expect(vi.mocked(confirm)).not.toHaveBeenCalled(); + expect(vi.mocked(deleteUser)).toHaveBeenCalledWith(expect.anything(), "u1"); + }, + ); + + it("prepares a device replacement without confirming when forced", async () => { + vi.mocked(prepareDeviceReplacement).mockResolvedValue({} as never); + + await runUsers(["prepare-device-replacement", "u1", "--force"]); + + expect(vi.mocked(confirm)).not.toHaveBeenCalled(); + expect(vi.mocked(prepareDeviceReplacement)).toHaveBeenCalled(); + }); + + it("refuses to ask without a terminal, naming --force", async () => { + process.stdin.isTTY = false; + + await expect(runUsers(["delete", "u1"])).rejects.toThrow( + /needs an interactive terminal.*--force/s, + ); + expect(vi.mocked(deleteUser)).not.toHaveBeenCalled(); + }); +}); diff --git a/src/commands/users.ts b/src/commands/users.ts index d8c3da6..c0c2330 100644 --- a/src/commands/users.ts +++ b/src/commands/users.ts @@ -1,4 +1,3 @@ -import { confirm, isCancel } from "@clack/prompts"; import kleur from "kleur"; import { extractFlag } from "../core/args.js"; import { createAuthClient, type AuthClient } from "../core/authClient.js"; @@ -10,6 +9,10 @@ import { type Json, } from "../core/admin.js"; import { reportAdminError } from "./adminShared.js"; +import { + confirmDestructive, + hasForceFlag, +} from "../core/confirmAction.js"; export async function runUsers(args: string[]): Promise { const sub = args[0]; @@ -81,11 +84,12 @@ async function usersDelete(client: AuthClient, rest: string[]): Promise { process.exit(1); } - const proceed = await confirm({ + const proceed = await confirmDestructive({ message: `Permanently delete user ${id}? This cannot be undone.`, - initialValue: false, + force: hasForceFlag(rest), + remedy: "Pass --force to delete without confirming.", }); - if (isCancel(proceed) || !proceed) { + if (!proceed) { console.log("Cancelled."); return; } @@ -158,11 +162,12 @@ async function usersPrepareDeviceReplacement( opts.disableTotp ? "disable TOTP" : null, ].filter(Boolean); - const proceed = await confirm({ + const proceed = await confirmDestructive({ message: `Prepare device replacement for ${id}? This will ${actions.join(", ")}.`, - initialValue: false, + force: hasForceFlag(rest), + remedy: "Pass --force to prepare the replacement without confirming.", }); - if (isCancel(proceed) || !proceed) { + if (!proceed) { console.log("Cancelled."); return; } diff --git a/src/core/confirmAction.test.ts b/src/core/confirmAction.test.ts new file mode 100644 index 0000000..f3725ad --- /dev/null +++ b/src/core/confirmAction.test.ts @@ -0,0 +1,109 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { confirm, isCancel } from "@clack/prompts"; +import { confirmDestructive, hasForceFlag } from "./confirmAction.js"; + +vi.mock("@clack/prompts", () => { + const CANCEL = Symbol("cancel"); + return { + CANCEL, + confirm: vi.fn(), + isCancel: (value: unknown) => value === CANCEL, + }; +}); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("hasForceFlag", () => { + it.each(["--force", "--yes", "-y"])("accepts %s", (flag) => { + expect(hasForceFlag([flag])).toBe(true); + }); + + it("is false without one", () => { + expect(hasForceFlag(["--json", "some-id"])).toBe(false); + }); + + // --yes predates the --force convention (config oauth-providers remove + // shipped with it), so dropping it would break a documented command. + it("keeps accepting the flag that shipped first", () => { + expect(hasForceFlag(["provider-id", "--yes"])).toBe(true); + }); +}); + +describe("confirmDestructive", () => { + it("does not ask when --force answered it", async () => { + await expect( + confirmDestructive({ message: "Delete it?", force: true }), + ).resolves.toBe(true); + expect(confirm).not.toHaveBeenCalled(); + }); + + it("asks, and returns the answer", async () => { + vi.mocked(confirm).mockResolvedValue(true as never); + + await expect( + confirmDestructive({ message: "Delete it?", force: false }), + ).resolves.toBe(true); + expect(confirm).toHaveBeenCalledWith({ + message: "Delete it?", + initialValue: false, + }); + }); + + it("declines by default", async () => { + vi.mocked(confirm).mockResolvedValue(false as never); + + await expect( + confirmDestructive({ message: "Delete it?", force: false }), + ).resolves.toBe(false); + }); + + // Ctrl-C at a destructive confirmation means "do not do it", which is the + // same outcome as answering No. + it("treats a cancel as a decline", async () => { + const { CANCEL } = (await import("@clack/prompts")) as unknown as { + CANCEL: symbol; + }; + vi.mocked(confirm).mockResolvedValue(CANCEL as never); + expect(isCancel(CANCEL)).toBe(true); + + await expect( + confirmDestructive({ message: "Delete it?", force: false }), + ).resolves.toBe(false); + }); + + it("refuses to ask without a terminal, naming --force", async () => { + process.stdin.isTTY = false; + + await expect( + confirmDestructive({ message: "Delete it?", force: false }), + ).rejects.toThrow(/"Delete it\?" needs an interactive terminal.*--force/s); + expect(confirm).not.toHaveBeenCalled(); + }); + + it("uses a caller's remedy in the no-terminal error", async () => { + process.stdin.isTTY = false; + + await expect( + confirmDestructive({ + message: "Delete it?", + force: false, + remedy: "Pass --force to delete without confirming.", + }), + ).rejects.toThrow(/Pass --force to delete without confirming\./); + }); + + it("runs to completion without a terminal when --force answered it", async () => { + process.stdin.isTTY = false; + + await expect( + confirmDestructive({ message: "Delete it?", force: true }), + ).resolves.toBe(true); + }); +}); diff --git a/src/core/confirmAction.ts b/src/core/confirmAction.ts new file mode 100644 index 0000000..08142d6 --- /dev/null +++ b/src/core/confirmAction.ts @@ -0,0 +1,47 @@ +import { confirm, isCancel } from "@clack/prompts"; + +import { requireInteractive } from "./tty.js"; + +// `--force` is the standing spelling for "take the destructive step without +// asking", matching what `seamless init` means by it: `--yes` answers ordinary +// questions, `--force` answers the ones that destroy something. `--yes` and `-y` +// stay accepted here because `config oauth-providers remove --yes` shipped with +// them, and because it is the flag most people reach for first. +export function hasForceFlag(args: string[]): boolean { + return ( + args.includes("--force") || args.includes("--yes") || args.includes("-y") + ); +} + +export const FORCE_FLAGS = ["--force", "--yes", "-y"]; + +export interface ConfirmDestructiveOptions { + message: string; + force: boolean; + // What to tell someone running without a terminal. Defaults to naming --force, + // which is the answer for every destructive confirmation. + remedy?: string; +} + +// Asks a destructive question unless --force already answered it. Without a +// terminal it fails naming the flag rather than rendering a prompt nobody can +// answer, which used to hang the command until its job timed out. +// +// Cancelling reads as declining rather than throwing, because every caller here +// treats a Ctrl-C at the confirmation as "do not do it" and reports it the same +// way as a No. +export async function confirmDestructive({ + message, + force, + remedy, +}: ConfirmDestructiveOptions): Promise { + if (force) return true; + + requireInteractive( + message, + remedy ?? "Pass --force to take this action without confirming.", + ); + + const answer = await confirm({ message, initialValue: false }); + return !isCancel(answer) && answer === true; +} diff --git a/src/core/interactiveLogin.ts b/src/core/interactiveLogin.ts index d9d295f..a5a39f3 100644 --- a/src/core/interactiveLogin.ts +++ b/src/core/interactiveLogin.ts @@ -2,6 +2,7 @@ import { text, isCancel } from "@clack/prompts"; import kleur from "kleur"; import { completeLogin, type LoginResult } from "./loginFlow.js"; +import { requireInteractive } from "./tty.js"; export interface InteractiveLoginOptions { instanceUrl: string; @@ -26,6 +27,10 @@ export async function promptLogin( let identifier = opts.identifier?.trim(); if (!identifier) { + requireInteractive( + "Email or phone", + "Pass the identifier positionally, or with --identifier .", + ); const answer = await text({ message: "Email or phone", placeholder: opts.knownEmail ?? "you@example.com", @@ -45,6 +50,12 @@ export async function promptLogin( localDelivery: opts.localDelivery ?? false, getCode: async ({ resent, channel }) => { const email = channel === "email"; + // No flag can answer this one: the code only exists after the request is + // sent. Failing here is still better than waiting forever on a pipe. + requireInteractive( + "Enter the code we sent you", + "A one-time code cannot be supplied ahead of time, so this step needs a terminal.", + ); const answer = await text({ message: resent ? "Enter the new code" : "Enter the code we sent you", placeholder: email ? "ABCDEF" : "123456", diff --git a/src/prompts/appSelect.test.ts b/src/prompts/appSelect.test.ts index 55ff2f2..82b2de6 100644 --- a/src/prompts/appSelect.test.ts +++ b/src/prompts/appSelect.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { select, isCancel, cancel } from "@clack/prompts"; import { CancelledError } from "../core/cancel.js"; @@ -21,19 +21,6 @@ function app(over: Partial = {}): PortalApp { }; } -// Captured before any test flips it, so a suite run leaves the process as it -// found it. -const ORIGINAL_TTY = process.stdin.isTTY; - -beforeEach(() => { - // The prompt path refuses to run without a terminal, and vitest has none. - process.stdin.isTTY = true; -}); - -afterEach(() => { - process.stdin.isTTY = ORIGINAL_TTY; -}); - describe("selectApplication", () => { it("throws when there are no applications", async () => { await expect(selectApplication([])).rejects.toBeInstanceOf( diff --git a/src/prompts/projectSetup.test.ts b/src/prompts/projectSetup.test.ts index bbce863..811cb3b 100644 --- a/src/prompts/projectSetup.test.ts +++ b/src/prompts/projectSetup.test.ts @@ -69,15 +69,9 @@ function fullRegistry(): RegistryEntry[] { ]; } -// Captured before any test flips it, so a suite run leaves the process as it -// found it. -const ORIGINAL_TTY = process.stdin.isTTY; - let logs: string[]; beforeEach(() => { - // The prompt paths refuse to run without a terminal, and vitest has none. - process.stdin.isTTY = true; // Every full run answers the owner-email prompt; tests that care override it. vi.mocked(text).mockResolvedValue("dev@example.com" as never); logs = []; @@ -87,7 +81,6 @@ beforeEach(() => { }); afterEach(() => { - process.stdin.isTTY = ORIGINAL_TTY; vi.restoreAllMocks(); }); diff --git a/src/testSetup.ts b/src/testSetup.ts new file mode 100644 index 0000000..f886712 --- /dev/null +++ b/src/testSetup.ts @@ -0,0 +1,9 @@ +import { beforeEach } from "vitest"; + +// Prompts refuse to run without a TTY on stdin (see core/tty.ts), and vitest has +// none, so every test that exercises a prompt would otherwise fail on the guard +// rather than on what it is testing. Tests that want the no-terminal behavior +// set isTTY back to false in their own beforeEach, which runs after this one. +beforeEach(() => { + process.stdin.isTTY = true; +}); diff --git a/vitest.config.ts b/vitest.config.ts index 903da1a..43f166a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -3,12 +3,13 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { include: ["src/**/*.test.ts"], + setupFiles: ["src/testSetup.ts"], coverage: { provider: "v8", reporter: ["text", "html", "json-summary"], reportsDirectory: "./coverage", include: ["src/**/*.ts"], - exclude: ["src/**/*.d.ts", "src/**/*.test.ts"], + exclude: ["src/**/*.d.ts", "src/**/*.test.ts", "src/testSetup.ts"], // Regression floor, set just below the current numbers (lines/statements // ~99.8%, functions ~99.6%, branches ~96.9%). The remaining gap is a // small set of unreachable branches; keep new code at or above these.