fix(core): abort prompts on stdin EOF instead of spinning at 100% CPU - #173
fix(core): abort prompts on stdin EOF instead of spinning at 100% CPU#173jamie-at-bunny wants to merge 3 commits into
Conversation
|
@codex review |
🦋 Changeset detectedLatest commit: 8e291f6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Refuse to prompt up front when stdin is not a TTY instead of racing EOF at read time, matching the standard CLI convention for non-interactive runs; the EOF race stays as a backstop for a terminal that hangs up mid-prompt. Piped prompt answers are no longer supported; flags and --force are the automation contract. prompts.inject() in tests is exempt since injected answers never touch stdin.
ac744fc to
b0f717b
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0f717b247
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const result = await promptOrEof(() => promptsLib(questions, options)); | ||
| return result ?? ({} as promptsLib.Answers<T>); |
There was a problem hiding this comment.
When stdin is non-interactive, returning {} lets callers that treat an unanswered prompt as an optional/default response complete successfully rather than fail fast. For example, bunny registries update <id> < /dev/null receives undefined for both prompts in commands/registries/update.ts, retains the existing display name, skips credential rotation, sends an unchanged PUT, and reports success. The wrapper needs to distinguish required prompts from optional offers, or affected callers must explicitly reject the EOF result.
Useful? React with 👍 / 👎.
| function hasInjectedAnswers(): boolean { | ||
| const injected = (promptsLib as unknown as { _injected?: unknown[] }) | ||
| ._injected; | ||
| return (injected?.length ?? 0) > 0; |
There was a problem hiding this comment.
The prompts package keeps answers supplied through prompts.inject() in an internal closure and does not expose an _injected property, so this check always reports no injected answers. Under Bun's non-TTY test runner, the wrapper therefore aborts before the library can consume injections used by sites/provision.test.ts and core/hostnames/bunny-dns.test.ts, causing those prompt-driven tests to receive empty/cancelled responses instead of their injected values. Track injections through an explicit wrapper or another supported mechanism rather than probing this nonexistent property.
AGENTS.md reference: AGENTS.md:L209-L209
Useful? React with 👍 / 👎.
An unattended registries update with no flags kept every existing value, sent an unchanged PUT, and reported success. It now exits non-zero up front naming the flags, matching the other update commands. Also pin the prompts.inject() escape hatch with a test that runs against the non-TTY test runner.
3 participants
Fixes #171