Skip to content

fix: error on non-interactive terminals without --quiet #173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Skn0tt wants to merge 2 commits into
base: main
Choose a base branch
from
+8 −0
Open

fix: error on non-interactive terminals without --quiet #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88b15eb
fix: error on non-interactive terminals without --quiet
Skn0tt Apr 2, 2026
73d1fbf
Merge branch 'main' into agents-non-tty-guard
Skn0tt Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ export class Generator {
installPlaywrightBrowsers: !this.options.noBrowsers,
};
}
if (!process.stdin.isTTY) {
console.error('Non-interactive terminal detected. See --help for options, then run again with --quiet.');
process.exit(1);
}
Comment on lines +98 to +101
Copy link
Copy Markdown
Contributor

@karlhorky karlhorky Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Skn0tt hmm this seems like an antipattern 🤔

  1. after fix: ensure non-TTY agents dont hang #172, --quiet is only one way to pass in all options. if options are passed in, the questions could be skipped below
  2. if I remember correctly, --quiet overrides all options passed in (which makes it less useful)

actually with all of the flags, --quiet isn't really needed, unless you want to silence the stdout (that's probably what a --quiet flag should do, not modify any options or read anything related to TTY)

if the enquirer questions are skipped, the --quiet flag is no longer needed. it's a simpler mental model - if you pass all answers via flags, it skips all questions

Copy link
Copy Markdown
Member Author

@Skn0tt Skn0tt Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It respects all options passed in: https://github.com/Skn0tt/create-playwright/blob/88b15eb00ff1d63618791b4cfe54da6b40bfdb3d/src/generator.ts#L90-L95

Can you elaborate on what you think will be broken by this?

Copy link
Copy Markdown
Contributor

@karlhorky karlhorky Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would be broken: passing in all answers via flags without the --quiet flag

it's less obvious what this does:

pnpm create playwright --browser=chromium --test-dir=playwright --quiet

it would be easier to understand if you can see the option in the command (either of these would break after your PR):

pnpm create playwright --browser=chromium --test-dir=playwright --no-gha

pnpm create playwright --browser=chromium --test-dir=playwright --gha

that's why I was asking for a --no-gha flag, to be able to just exhaustively pass in all options

once you can exhaustively pass all options (and they skip the questions), then it works non-interactively already, and the --quiet flag can be dropped / deprecated

Copy link
Copy Markdown
Contributor

@karlhorky karlhorky Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Skn0tt alternative PR of what I mean with my proposal:

Copy link
Copy Markdown
Member Author

@Skn0tt Skn0tt Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for elaborating. With the approach you're outlining, some agent implementations will start hanging if they forget both --no-gha and --gha. My approach where non-interactive terminals need to use --quiet is a little more cumbersome for humans with non-interactive terminals, but one less footgun for agents. Since humans use interactive terminals, this seems to be the right approach.

Copy link
Copy Markdown
Contributor

@karlhorky karlhorky Apr 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, but the --quiet flag is worse UX for humans

some agent implementations will start hanging if they forget both --no-gha and --gha

Good point - so my PR #174 could check for non-interactive (same as your PR) and then short-circuit like --quiet currently does

Better behavior than erroring out for both AI and humans

Edit: added over in my PR #174


const isDefinitelyTS = fs.existsSync(path.join(this.rootDir, 'tsconfig.json'));

Expand Down
4 changes: 4 additions & 0 deletions tests/integration.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ test('should install with "npm i" in GHA when using npm with package-lock disabl
expect(workflowContent).not.toContain('run: npm ci');
});

test('should require --quiet for non-interactive terminals', async ({ exec }) => {
await expect(exec('node', [path.join(__dirname, '..')])).rejects.toThrow('Non-interactive terminal detected');
});

test('should not prompt and skip existing files in --quiet mode', async ({ run, dir }) => {
// First run: generate the project normally
await run([], { installGitHubActions: false, testDir: 'tests', language: 'TypeScript', installPlaywrightDependencies: false, installPlaywrightBrowsers: false });
Expand Down
Loading