feat: extend the no-terminal guard to every command that prompts - #160
Merged
Conversation
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
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #159. Follow-up to #153 / #158, which shipped the helper and the
initcoverage.seamless initstopped rendering prompts to a pipe in #158. Every other command still hung there until its job timed out. This closes that gap and settles the flag convention you picked:--force, with--yesand-yas accepted aliases.Guarded
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 login--identifier, or positionallogin,profile loginprofile addprofile add--instance-urlusers delete--forceusers prepare-device-replacement--forcesessions revoke --all/ current--forceorg members remove--forceconfig apply--forceconfig oauth-providers remove--forceThe one-time code is the honest exception: it does not exist until the code is sent, so no flag can supply it ahead of time. That step reports that it genuinely needs a terminal rather than pretending otherwise.
--forceA guard with nothing to point at only converts a hang into a dead end, so the destructive confirmations get a way to be answered. They share
confirmDestructive(src/core/confirmAction.ts), which answers itself when forced and otherwise asks.hasForceFlagalso accepts--yesand-y, soconfig oauth-providers remove --yeskeeps working. That subcommand previously wrapped its confirm inif (!skipConfirm); the wrapper is gone and the flag flows through the helper instead.Two behaviors deliberately preserved:
--forcedoes not override--dry-run(config apply --dry-run --forcestill changes nothing), and cancelling a confirmation reads as declining rather than raising, which is what all six call sites already did.Tests
The guard would otherwise fail every existing test that exercises a prompt, since vitest has no TTY. Rather than patching each file,
src/testSetup.tsmarks stdin a TTY before each test, and the no-terminal tests opt back out. That also let me drop the per-file setup #158 added to three test files.Checks
npm run buildandnpm testpass (815 passed, 4 skipped).confirmAction.tsandtty.tsare at 100% coverage, as areusers.ts,sessions.ts, andorg.ts. Verified against the built CLI thatlogin,profile add, and a fully flaggedprofile addbehave correctly on a pipe with the right exit codes, and that the new flags show up in--help.