Repository navigation
fix(prompts): end an interrupted spinner with the signal status - #3432
Conversation
clack's block() calls process.exit(0) on Ctrl+C and Escape, so an interrupted command reported success. Replace it with a local input block that stops the spinner and sends SIGINT to the foreground process group, as the terminal does outside raw mode. Windows has no signals, so the process exits with 130 there. Escape and other keys are ignored, as the Rust progress row does. A key pressed just before the spinner stops no longer leaves a listener on stdin. Refs voidzero-dev#3429
An active spinner listened for SIGINT and SIGTERM, stopped itself, and returned, so the command kept running after a termination request. Raise the signal again after stopping, but only when no other listener owns it: none was registered before the spinner started and none was added while it spun. An emit without the signal name only stops the spinner. Windows exits with the matching status instead. Closes voidzero-dev#3429
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 19eee2b1a4
ℹ️ 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".
The Ctrl+C and Escape tests expected process.kill, which only the POSIX branch calls. On Windows the spinner exits with 130 instead, so the tests failed there. Pin a POSIX platform unless a test asks for Windows.
Each active spinner blocked input, so two spinners both received a single Ctrl+C and each sent SIGINT to the process group. A shutdown handler then saw two interrupts for one key press. Let only one spinner block input at a time, as the Rust progress row does. The others receive the forwarded signal through their signal listeners.
The Ctrl+C test waited for the worker's PID to disappear. Where nothing reaps orphans, as in some Linux containers, the worker stays a zombie after the group is interrupted, so the test timed out although the worker had ended. Read the process state on Linux and accept a zombie.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
A spinner remembered that another listener had been registered for a signal and never forgot it. After that listener was removed, a signal only stopped the spinner and the command kept running. Check the registered listeners when the signal arrives instead. Node removes a one-shot listener just before calling it, so a removal during the current delivery still counts as another owner, and its cleanup is not cut short.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
jong-kyung
added
test: e2e
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ec452874e
ℹ️ 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".
The key listener was registered for one key and again only after the echoed key had been erased. A Ctrl+C that followed another key in the same chunk, as happens after typing while the event loop is blocked, found no listener and was dropped. clack's block() has the same pattern. Keep the listener registered while the spinner blocks input. Every key of a chunk is now erased as well.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
The tsdown extension option repeated the default. The wait in the key test covered the deferred listener registration that no longer exists.
The helper replaced process.platform by hand and a nested hook put it back, so a call from another block would have leaked the value. A spy is restored by the existing vi.restoreAllMocks().
signal-exit ends the process only when its listener is the last one left, and a spinner leaves the shutdown to any other listener. With signal-exit registered first, each waited for the other, so the command kept running after Ctrl+C with two spinners or after a signal. Register the spinner listener first. It has stopped and removed itself by the time other listeners run, and a real shutdown handler still receives the signal once. signal-exit joins the dev dependencies for the regression tests.
The loops over the process signals reused the name of the spinner's AbortSignal option, which the next lines then read. Rename the process signal bindings, and drop two comments that restate the code.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
fengmk2
added
the
preview-build
Preview binary sizes (
|
| Binary | Latest release | Preview | Change |
|---|---|---|---|
vp (macOS ARM64) |
5.46 MiB | 5.46 MiB | +32 B (+0.00%) |
| NAPI (macOS ARM64) | 43.39 MiB | 43.40 MiB | +16.14 KiB (+0.04%) |
vp (macOS x64) |
6.92 MiB | 6.92 MiB | +4.03 KiB (+0.06%) |
| NAPI (macOS x64) | 48.76 MiB | 48.76 MiB | +8 B (+0.00%) |
vp (Linux ARM64 GNU) |
6.90 MiB | 6.91 MiB | +4.05 KiB (+0.06%) |
| NAPI (Linux ARM64 GNU) | 32.68 MiB | 32.66 MiB | -16.00 KiB (-0.05%) |
vp (Linux ARM64 musl) |
6.97 MiB | 6.98 MiB | +7.58 KiB (+0.11%) |
| NAPI (Linux ARM64 musl) | 31.89 MiB | 31.89 MiB | -2.75 KiB (-0.01%) |
vp (Linux x64 GNU) |
8.33 MiB | 8.35 MiB | +16.05 KiB (+0.19%) |
| NAPI (Linux x64 GNU) | 36.69 MiB | 36.70 MiB | +4.00 KiB (+0.01%) |
vp (Linux x64 musl) |
8.41 MiB | 8.42 MiB | +4.70 KiB (+0.05%) |
| NAPI (Linux x64 musl) | 35.90 MiB | 35.90 MiB | +1.13 KiB (+0.00%) |
vp (Windows x64) |
6.19 MiB | 6.20 MiB | +5.00 KiB (+0.08%) |
| NAPI (Windows x64) | 32.45 MiB | 32.45 MiB | +1.50 KiB (+0.00%) |
| Trampoline (Windows x64) | 13.00 KiB | 13.00 KiB | 0 B (0.00%) |
| Installer (Windows x64) | 4.69 MiB | 4.69 MiB | +4.00 KiB (+0.08%) |
vp (Windows ARM64) |
5.42 MiB | 5.42 MiB | +5.00 KiB (+0.09%) |
| NAPI (Windows ARM64) | 28.66 MiB | 28.66 MiB | +1.50 KiB (+0.01%) |
| Trampoline (Windows ARM64) | 13.00 KiB | 13.00 KiB | 0 B (0.00%) |
| Installer (Windows ARM64) | 3.86 MiB | 3.86 MiB | +4.00 KiB (+0.10%) |
gzip -9 sizes
| Binary | Latest release | Preview | Change |
|---|---|---|---|
vp (macOS ARM64) |
3.25 MiB | 3.25 MiB | +2.53 KiB (+0.08%) |
| NAPI (macOS ARM64) | 18.85 MiB | 18.85 MiB | +1.73 KiB (+0.01%) |
vp (macOS x64) |
3.43 MiB | 3.43 MiB | +4.31 KiB (+0.12%) |
| NAPI (macOS x64) | 20.12 MiB | 20.12 MiB | +1.75 KiB (+0.01%) |
vp (Linux ARM64 GNU) |
3.74 MiB | 3.75 MiB | +3.86 KiB (+0.10%) |
| NAPI (Linux ARM64 GNU) | 13.93 MiB | 13.93 MiB | -763 B (-0.01%) |
vp (Linux ARM64 musl) |
3.77 MiB | 3.77 MiB | +1.48 KiB (+0.04%) |
| NAPI (Linux ARM64 musl) | 13.64 MiB | 13.63 MiB | -5.89 KiB (-0.04%) |
vp (Linux x64 GNU) |
3.81 MiB | 3.82 MiB | +3.43 KiB (+0.09%) |
| NAPI (Linux x64 GNU) | 14.65 MiB | 14.65 MiB | +2.18 KiB (+0.01%) |
vp (Linux x64 musl) |
3.84 MiB | 3.84 MiB | +3.47 KiB (+0.09%) |
| NAPI (Linux x64 musl) | 14.33 MiB | 14.33 MiB | -3.14 KiB (-0.02%) |
vp (Windows x64) |
2.86 MiB | 2.87 MiB | +3.88 KiB (+0.13%) |
| NAPI (Windows x64) | 13.31 MiB | 13.31 MiB | -1.23 KiB (-0.01%) |
| Trampoline (Windows x64) | 7.14 KiB | 7.14 KiB | -1 B (-0.01%) |
| Installer (Windows x64) | 2.25 MiB | 2.25 MiB | +1.91 KiB (+0.08%) |
vp (Windows ARM64) |
2.73 MiB | 2.74 MiB | +1.99 KiB (+0.07%) |
| NAPI (Windows ARM64) | 12.62 MiB | 12.62 MiB | -1.18 KiB (-0.01%) |
| Trampoline (Windows ARM64) | 6.93 KiB | 6.92 KiB | -2 B (-0.03%) |
| Installer (Windows ARM64) | 2.14 MiB | 2.14 MiB | +2.48 KiB (+0.11%) |
Raw sizes are uncompressed file sizes. gzip sizes recompress each binary at level 9; they are not package or download archive sizes.
The baseline uses published release files, including NAPI packages from npm at the same version. Build versions and toolchains can affect the difference.
Registry bridge build (
|
| Package | Version |
|---|---|
vite-plus |
0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5 |
@voidzero-dev/vite-plus-core |
0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5 |
create-vite-plus |
0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5 |
Install the Vite+ CLI built from this commit, then migrate a project:
# macOS / Linux
curl -fsSL https://raw.githubusercontent.com/jong-kyung/vite-plus/098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5/packages/cli/install.sh | VP_PR_VERSION=3432 VP_LEGACY_INSTALLER_URL=https://raw.githubusercontent.com/jong-kyung/vite-plus/098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5/packages/cli/install-legacy.sh bash# Windows (PowerShell)
$env:VP_PR_VERSION="3432"; $env:VP_LEGACY_INSTALLER_URL="https://raw.githubusercontent.com/jong-kyung/vite-plus/098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5/packages/cli/install-legacy.ps1"; irm https://raw.githubusercontent.com/jong-kyung/vite-plus/098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5/packages/cli/install.ps1 | iexOr install with Homebrew (macOS / glibc Linux):
These commands use the formula and binary from this published commit. Replace brew install with brew reinstall if the tap package is already installed.
If Homebrew Core’s vite-plus is installed, run brew uninstall vite-plus first. This keeps your user data; do not run vp implode to switch installations.
(
set -eu
export HOMEBREW_NO_AUTO_UPDATE=1
export HOMEBREW_VP_PR_VERSION=098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5
brew tap voidzero-dev/vite-plus https://github.com/voidzero-dev/vite-plus
preview_tap="$(brew --repository voidzero-dev/vite-plus)"
git -C "$preview_tap" fetch --depth=1 https://github.com/jong-kyung/vite-plus.git "$HOMEBREW_VP_PR_VERSION"
git -C "$preview_tap" switch --detach "$HOMEBREW_VP_PR_VERSION"
brew install voidzero-dev/vite-plus/vp
brew test voidzero-dev/vite-plus/vp
"$(brew --prefix)/bin/vp"
)The tap stays at the tested commit. Return it to main after testing:
git -C "$(brew --repository voidzero-dev/vite-plus)" switch mainOr download the standalone Windows installer built from this commit:
| Architecture | Installer |
|---|---|
| x64 | vp-setup-x86_64-pc-windows-msvc.exe |
| Arm64 | vp-setup-aarch64-pc-windows-msvc.exe |
GitHub requires you to sign in and downloads each installer as a ZIP artifact. Extract vp-setup.exe, then run it against this preview build:
.\vp-setup.exe --version "0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5" --registry "https://registry-bridge.viteplus.dev/"After installing, upgrade the current project's vite-plus to this test build with:
vp migrateOr create a new project from this commit without installing the CLI:
export npm_config_registry=https://registry-bridge.viteplus.dev/ pnpm_config_registry=https://registry-bridge.viteplus.dev/
pnpm create vite-plus@0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5 # or: npm create vite-plus@0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5$env:npm_config_registry="https://registry-bridge.viteplus.dev/"; $env:pnpm_config_registry="https://registry-bridge.viteplus.dev/"
pnpm create vite-plus@0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5 # or: npm create vite-plus@0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5pnpm 11+ reads pnpm_config_registry, npm and older pnpm read npm_config_registry. The new project pins this build, so keep the registry set for later installs, or configure it as below.
Or point your package manager at the bridge registry https://registry-bridge.viteplus.dev/:
| Package manager | Registry config |
|---|---|
| npm / pnpm / Bun | .npmrc: registry=https://registry-bridge.viteplus.dev/ |
| Yarn (v2+) | .yarnrc.yml: npmRegistryServer: "https://registry-bridge.viteplus.dev/" |
Then pin the build (vite aliases to vite-plus-core; pnpm can use a catalog, npm an overrides entry):
{
"devDependencies": {
"vite-plus": "0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5",
"vite": "npm:@voidzero-dev/vite-plus-core@0.0.0-commit.098d02eab8fa1363177ec0aadfbfd4e7f9d79fa5"
}
}
fengmk2
removed
the
preview-build
fengmk2
added
the
preview-build
ChatGPT Codex Connector
Closes #3429. Supersedes #3345.
Summary
The TypeScript spinner reported an interrupt as success. This PR makes it end by the signal, as the Rust progress row does, so the shell reports 130 or 143.
Changes
block(), which callsprocess.exit(0). It stops the spinnerand sends
SIGINTto the foreground process group.SIGINTandSIGTERM: the spinner raises the signal again after it stops, but only when no other listenerowns the signal.