fix(init): harden /dev/tty teardown and adopt Symbol.dispose#824
Merged
Conversation
Follow-up hardening on top of #802. Promotes the explicit try/finally for `closeFreshTtyForwarding()` to a `using` declaration so teardown is guaranteed by the compiler on every exit path (success, early return, throw, unknown). Correctness fixes: - Capture the pre-install `process.stdin.isTTY` value and restore it verbatim instead of hardcoding `undefined` — future- proofs against concurrent writers. - Restore termios (`fresh.setRawMode(false)`) before destroying the fresh stream so a mid-prompt crash doesn't leave the user's shell in raw mode with no echo. - Keep the original `error` listener attached across `destroy()` — Bun can asynchronously emit EBADF after the fd closes, which would otherwise crash the process. API refactor: - `forwardFreshTtyToStdin` now accepts an optional `TtyDeps` object (`{ openTty, isTty }`) for dependency injection in tests. Production callers pass no args. - Returns a `Disposable` (`TtyForwardingHandle`) unconditionally so `using tty = forwardFreshTtyToStdin()` works without null checks. No-install cases return a disposable that routes through `closeFreshTtyForwarding` (a documented no-op in that state) which preserves test observability of the lifecycle call. - Secondary callers (already-installed) get a pure no-op disposable so they don't accidentally tear down the primary. - `runWizardInner` merges back into `runWizard`; the outer try/finally disappears in favor of `using _tty`. Tests: - 11 new tests covering install/teardown round trip, isTTY backfill branches, re-install after teardown, secondary-caller no-op semantics, termios restoration, `using`-scope lifetime, and `using`-on-throw teardown. Uses `/dev/ptmx` as a pseudo- TTY fixture so the install path runs in piped-stdin `bun test` environments. Skips gracefully on platforms without `/dev/ptmx`. - New wizard-runner test covers `WizardError` rethrow spinner- stop side effect. - Existing early-return tests (preflight null, git-safety fail) now assert `closeFreshTtyForwardingSpy` fires once via `using` teardown.
Contributor
|
4 tasks
On CI and some Node/Bun runtimes, `process.stdin.isTTY` is defined as a non-writable property, so bare `stdin.isTTY = …` throws `TypeError: Attempted to assign to readonly property`. `Object.defineProperty` with `writable: true, configurable: true` overrides the existing descriptor in-place, working on every runtime. Affects both: - Production code — the `isTTY` backfill when forwarding is installed, and the restore step in teardown. - Tests — the `setIsTTY` helper used by the backfill-branch and already-set tests. Lives on `main` pre-#802 because the previous code only ran when `isTTY === undefined` AND `isatty(0) === true`, which doesn't hit on piped-stdin CI. The new test suite forces `isTty: () => true` to exercise the backfill path, surfacing the latent bug.
Contributor
Codecov Results 📊✅ 138 passed | Total: 138 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 1937 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 95.27% 95.29% +0.02%
==========================================
Files 284 284 —
Lines 41071 41146 +75
Branches 0 0 —
==========================================
+ Hits 39131 39209 +78
- Misses 1940 1937 -3
- Partials 0 0 —Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 65a5b28. Configure here.
Cursor Bugbot caught that the `errorListener` field on
`InstalledState` was stored at install time but never read
back in teardown — the old code called
`fresh.off('error', errorListener)` which justified storing it,
but the post-#802 teardown intentionally keeps the listener
attached (see the install-site comment) to absorb Bun's
asynchronous EBADF emission after `destroy()`.
Inline the listener as an anonymous callback at the install site,
drop the field from the type, and update the teardown comment
to reference the install-site rationale instead of repeating it.
BYK
added a commit
that referenced
this pull request
Apr 23, 2026
## Summary Create an `AbortController` alongside the `MastraClient` in `runWizard`, pass its signal via `abortSignal` in `ClientOptions`, and abort it from a `using` disposable so any in-flight fetches are canceled on every exit path (success, error, cancellation). Companion PR to #824 — both are independent follow-ups to #802 and can land in either order. ## Why `MastraClient` has no `close()`/`dispose()` API (verified in `node_modules/@mastra/client-js/dist/client.d.ts`). Without an explicit abort, keep-alive sockets in Bun's fetch dispatcher can hold the event loop alive past the wizard's natural exit, causing the shell to appear stuck. The original `process.exit(0)` workaround (removed in #802) papered over this symptom by forcing exit; this PR addresses the root cause so `sentry init` releases cleanly under any runtime. Explicit cancellation also removes the implicit dependency on Bun's fetch dispatcher auto-unref'ing idle sockets, making the fix robust across future Bun versions and alternative runtimes. ## Implementation - `AbortController` created per-`runWizard` call. Scope matches the `MastraClient`. - `using _mastraCleanup` disposable calls `abortController.abort()` on every exit path. Idempotent — guarded by `signal.aborted` to avoid double-abort diagnostics. - Custom `fetch` wrapper preserves `init.signal` via the existing object spread — MastraClient's per-request signals still reach the underlying `fetch` call. - **No `run.cancel()`.** The server observes the dropped fetch connection and cancels the run server-side. Avoids an extra HTTP round-trip during teardown, which could be slow if the server is why we're erroring. ## Tests - Capture `ClientOptions` from each `MastraClient` instance via a prototype `getWorkflow` hook that reads `this.options` (exposed via `BaseResource`). - Assert `abortSignal` is aborted after success, tool-error, and `WizardCancelledError` paths. - Assert the signal is forwarded live to `MastraClient` at construction (not pre-aborted) so in-flight fetches during the run are actually gated on it. ## Test plan - [x] `bun test test/lib/init/wizard-runner.test.ts` — 19 pass, 0 fail - [x] `bun test test/lib/init/ test/commands/init.test.ts` — green - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing markdown.ts warning) ## Notes - Uses `using` (TS 5 / Bun native) in exactly one place — matches the pattern adopted in #824 for `/dev/tty` teardown. `tsconfig.json` already has `target: "ESNext"` so no build-config change needed. - If #824 lands first, this PR's `using` declaration is the second one in `wizard-runner.ts` — idiomatically consistent. If this lands first, it's the first one.
6 tasks
BYK
added a commit
that referenced
this pull request
Apr 23, 2026
…831) ## Summary At the end of a `sentry init` flow, the process hangs until the user presses a key. Follow-up to #802, #824, #825 — addresses the third and final contributor to the post-wizard hang. ## Root cause Our no-op patch of `process.stdin.pause` silently swallowed clack's `rl.close() → input.pause()` call. Stdin stayed in flowing/ref'd mode from `readline.createInterface()`'s internal `input.resume()`, keeping the libuv event loop alive until any keypress delivered a `data` event. ### Why the patch was needed `stdin-reopen.ts` replaces `process.stdin.pause`/`resume` with no-ops at install time to dodge Bun's fd-0 `EINVAL` on pause/resume transitions (see the comment at the install site). That fix is correct and must stay. ### Why the bug was invisible `rl.close()` is the only place clack ever pauses stdin — it relies entirely on Node's standard readline cleanup discipline. Our no-op patch swallowed every call without any visible error, so there was no log line, no warning, no failed assertion. ### Why PRs #802/#824/#825 didn't catch it - #802 fixed the `/dev/tty` ReadStream contribution (explicit `.destroy()`). - #824 adopted `using`/`Symbol.dispose` for guaranteed teardown + termios restore. - #825 aborted the MastraClient signal to release keep-alive sockets. Post-teardown state after all three fixes: - `/dev/tty` ReadStream: destroyed ✓ - MastraClient AbortController: aborted ✓ - **`process.stdin`: still ref'd and flowing ✗** ← the remaining anchor PR #782's original `process.exit(0)` workaround masked this by killing the process unconditionally. Each subsequent PR peeled off one contributor; stdin was the last one standing. ## Fix One call to the just-restored `original.pause` at the end of `closeFreshTtyForwarding()`: ```ts // Release the libuv handle on fd 0. Clack's prompt lifecycle relies on // `rl.close() → rl.pause() → this.input.pause()` to pause stdin, but we // replaced `process.stdin.pause` with a no-op at install time... Now that // the original `.pause()` is restored, invoke it directly so stock // Node/Bun cleanup can finish. Idempotent: safe when stdin was already // paused. try { original.pause.call(process.stdin); } catch { // Defensive: swallow errors from runtimes that throw if stdin is // already destroyed. } ``` Rationale for `.pause()` over alternatives: - Exactly what Node's `rl.close()` would have called — matches clack's implicit contract. - Idempotent on already-paused streams. - Doesn't destroy the stream (unlike `.destroy()`); any future code that wanted to read stdin (none does in `init`; it's a terminal command) could still resume. ## Regression tests Two new tests in `test/lib/init/stdin-reopen.test.ts`: ### Unit: teardown invokes restored pause exactly once Replaces the beforeEach stub with a counting spy BEFORE install (so install captures the spy as `original.pause`). Verifies: - During install, `process.stdin.pause` is the patched no-op (not the spy). - Calls made mid-wizard hit the no-op (spy count stays 0). - After teardown, `process.stdin.pause` is restored to the spy AND the spy was invoked exactly once. ### Integration: stdin is not flowing after teardown Puts `process.stdin` into flowing mode via real `Readable.prototype.resume` (simulating what clack does via `readline.createInterface`). Runs install + teardown. Asserts `process.stdin.readableFlowing !== true` after disposal — without the fix, this assertion would fail because the no-op pause never actually pauses. ## Test plan - [x] `bun test test/lib/init/stdin-reopen.test.ts` — 15 pass (13 existing + 2 new) - [x] `bun test test/lib/init/ test/commands/init.test.ts` — 193 pass - [x] `bun test --timeout 15000 test/lib test/commands test/types` — 5777 pass, 0 fail - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing markdown.ts warning) - [ ] Manual: `curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash` — shell prompt should return immediately after "Setup complete" without a keypress. ## Risk Very low. Single call to a restored function (known state) guarded by try/catch. No API changes, no new dependencies, no test fixture churn. Two new tests exercise the exact regression. ## Out of scope - Revisiting whether the no-op `pause`/`resume` patch is still necessary with current Bun (Bun's fd-0 EINVAL may be fixed — worth investigating later as a simplification, but not as part of this hot-fix). - E2E spawn test asserting process-exits-without-keypress (requires pty fixture infrastructure we don't currently have).
6 tasks
BYK
added a commit
that referenced
this pull request
Apr 23, 2026
## Summary After "Sentry SDK installed successfully!", `sentry init` still hangs until a keypress despite #802/#824/#825/#831. Root cause is a Bun 1.3.11 libuv refcount bug that userland cannot fix. Restores PR #782's `process.exit` workaround, but properly wrapped in `setTimeout(..., 100).unref()` so it's transparent in the happy path and terminal only when the Bun bug bites. ## Root cause (verified) Opening our fresh `/dev/tty` ReadStream (the `curl | bash` TTY-delivery workaround in `stdin-reopen.ts`) combined with clack's internal `readline.createInterface(process.stdin)` leaks a libuv handle that NO userland cleanup releases. Verified by systematic matrix test against real `/dev/tty` under a pty: | Scenario | Result | |---------------------------------------|-----------| | `fresh` alone | FAST ✓ | | `readline.createInterface` alone | FAST ✓ | | `readline` + our pause/resume patch | FAST ✓ | | `fresh + readline` | HANG 7s | | `fresh + readline + fresh.destroy()` | HANG 7s | | `fresh + readline + rl.close()` | HANG 7s | | `fresh + readline + process.stdin.destroy()` | HANG 7s | | `fresh + readline + removeAllListeners` | HANG 7s | | `fresh + readline + setTimeout(exit, 100).unref()` | FAST (via forced exit) | `process.stdin.unref()` is `undefined` on Bun 1.3.11, so Node's canonical "let the process exit" escape hatch isn't available. ## Why PRs #802/#824/#825/#831 didn't fix it Each peeled off a **legitimate contributing cause** — all should stay: - #802: `/dev/tty` ReadStream being ref'd (explicit `fresh.destroy()`) - #824: hardened teardown via `using`/`Symbol.dispose` + termios restore - #825: MastraClient keep-alive sockets (AbortController) - #831: `process.stdin` flowing state (restored `pause()` call) But the libuv refcount bug is a Bun-internal issue, not a stream-state or socket issue. No amount of userland cleanup fixes it. ## Fix Restore a force-exit safety net in `src/commands/init.ts`, wrapped in `setTimeout(..., 100).unref()`: ```ts if (process.env.NODE_ENV !== "test") { setTimeout(() => { process.exit(process.exitCode ?? 0); }, 100).unref(); } ``` Properties: - **Transparent in the happy path** — when the loop drains naturally (future Bun versions that fix the refcount bug, non-TTY flows, `--yes` with no prompts), the `.unref()` timer doesn't hold the loop. Process exits before the timer fires. - **Terminal when needed** — when the Bun bug bites, the timer fires after a 100ms grace period. Imperceptible to the user. - **100ms grace period** — enough for Sentry telemetry flush and stdio buffer flush to complete first. Matches best practices for terminal commands. ## Test gate `NODE_ENV !== "test"` guard: `bun test` sets `NODE_ENV=test` automatically. Without this guard, each call to `initCommand.func` in tests would schedule an unref'd 100ms timer; accumulated timers fire across test files and terminate the test runner mid-suite. The guard avoids this while leaving the safety net active in all real-world invocations (including `bun run dev`, compiled binary, npm bundle). ## Test plan - [x] `bun test test/commands/init.test.ts test/lib/init/` — 193 pass - [x] `bun test --timeout 15000 test/lib test/commands test/types` — 5777 pass, 0 fail - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing markdown.ts warning) - [x] Manual repro: the production scenario (real `/dev/tty` under a pty) hangs for 7s without this fix, exits in 286ms with it. - [ ] User validation via `curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash` after merge. ## Follow-ups - Exploration task: find an alternative to the fresh `/dev/tty` ReadStream approach for the `curl | bash` TTY-delivery workaround (the original bug #767 was fixing). If we can make that work without a second ReadStream on stdin, the Bun refcount bug is sidestepped entirely and the safety net becomes redundant. - File a Bun upstream issue with the systematic matrix repro. ## Risk Low. Single-file change. `.unref()` ensures the timer is transparent in healthy flows. Guarded against test-runner interference. All prior fixes remain in place because each addresses a legit cause.
5 tasks
BYK
added a commit
that referenced
this pull request
Apr 23, 2026
…ety net (#835) ## Summary Delete `src/lib/init/stdin-reopen.ts` entirely and the `setTimeout().unref()` safety net from #833. Net **−838 / +1 lines**. The `forwardFreshTtyToStdin` workaround was created to fix a Bun single-file-binary bug where TTY fds inherited via `curl | bash` → `exec sentry init </dev/tty` (in install.sh) accepted `setRawMode(true)` but never delivered keypress events. Research shows that bug is fixed on Bun 1.3.11 — and the workaround is actively causing the newer hang patched by #833. ## Empirical findings Reproduction harness: Python `pty.fork()` mirroring install.sh's exact `exec bin </dev/tty` flow against `bun build --compile --target=bun-linux-x64` binaries on Bun 1.3.11. ### The original bug is gone | Observable | Original bug | Bun 1.3.11 | |-----------------------------------------|--------------|------------| | `process.stdin.isTTY` after `</dev/tty` | `undefined` | `true` | | `setRawMode(true)` | no effect | works | | `data` events on keystroke | **never** | delivered | | Clack `text/confirm/select` prompts | hung forever | completes | Verified with three binaries running sequential clack prompts through the exact `exec bin </dev/tty` invocation. All exit cleanly on Enter without any workaround. ### The workaround IS the cause of the current hang | Scenario (real `/dev/tty` under pty) | Result | |--------------------------------------------|--------------------| | Clack prompts + fetch, **no workaround** | exits clean, 4.26s | | Clack prompts (no fetch) + workaround | exits clean, 4.19s | | Clack prompts + fetch + **workaround** | **HANG 30s** | Upstream: [oven-sh/bun#29126](oven-sh/bun#29126) — Bun's `tty.ReadStream` extends `fs.ReadStream` with default highWaterMark; any `new ReadStream(tty_fd)` holds the libuv loop open and `destroy()` doesn't release the handle. Our workaround opened a second `tty.ReadStream` on `/dev/tty` alongside clack's `readline.createInterface(process.stdin)`, leaking that handle. ## Changes **Deleted:** - `src/lib/init/stdin-reopen.ts` (320 lines) - `test/lib/init/stdin-reopen.test.ts` (452 lines) - `using _tty = forwardFreshTtyToStdin()` + namespace import in `wizard-runner.ts` - 6 × `expect(closeFreshTtyForwardingSpy).toHaveBeenCalledTimes(1)` assertions + spy setup in `wizard-runner.test.ts` - The `setTimeout(process.exit, 100).unref()` safety net in `init.ts` (from #833 — no longer needed once the root cause is removed) **Kept (orthogonal & legitimate):** - PR #824's `using`/`Symbol.dispose` pattern for the MastraClient `AbortController` - PR #825's MastraClient `AbortController` cleanup ## Validation plan This cleanup deletes the workaround based on PTY-harness testing. The real-world `curl | bash` flow has subtle differences (different terminal types, macOS vs Linux glibc vs Alpine, bash vs zsh, etc.), so a phased rollout is recommended: 1. **Merge to main.** Triggers nightly GHCR publish. 2. **Nightly smoke test** — install from cli.sentry.dev/install with `SENTRY_VERSION=nightly SENTRY_INIT=1` on: - macOS (system Terminal.app) - Linux glibc (Ubuntu) - Linux musl (Alpine) - WSL 3. **Monitor Sentry telemetry** for `channel=nightly` users for a few days for any keystroke-delivery regressions. 4. **Promote to stable** after the nightly window confirms clean. If any platform regresses the original keystroke bug, the revert is a single commit away and we'll scope the workaround narrowly (e.g. `process.platform === "darwin"` only) instead of always-on. ## Test plan - [x] `bun test test/lib/init/ test/commands/init.test.ts` — 178 pass (15 deleted stdin-reopen tests accounted for) - [x] `bun test --timeout 15000 test/lib test/commands test/types` — 5762 pass, 0 fail - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing markdown.ts warning) - [ ] Manual nightly verification: `curl -fsSL https://cli.sentry.dev/install | SENTRY_VERSION=nightly SENTRY_INIT=1 bash` on each target platform. ## Follow-ups - File a Bun upstream issue specifically about `tty.ReadStream + process.stdin` handle leak (distinct from but related to #29126). - Once nightly telemetry confirms no regressions, propagate the pattern deletion to any other commands that might have adopted similar stdin workarounds (none currently; `init` was the only one).
6 tasks
BYK
added a commit
that referenced
this pull request
Apr 23, 2026
## Summary PR #835 deleted `forwardFreshTtyToStdin` on the theory that Bun 1.3.11 fixes the inherited-TTY keystroke-delivery bug that the workaround was written for. That's true on **Linux** (verified via PTY harness). It's **NOT** true on **macOS**: `curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash` hangs at the first clack prompt — user can't type. Restore the workaround, gated on `process.platform === "darwin"`. ## User-reported reproduction ``` curl -fsSL https://cli.sentry.dev/install | SENTRY_INIT=1 bash -s -- --version nightly ``` After install, wizard shows: ``` sentry init EXPERIMENTAL: This feature is experimental and may modify your code. Continue? ● Yes / ○ No ``` Keystrokes (Y/N/Enter) don't reach clack. Prompt hangs indefinitely. Exactly what `forwardFreshTtyToStdin` was fixing originally. ## Fix ### `src/lib/init/wizard-runner.ts` Gate the `using _tty = forwardFreshTtyToStdin()` install on `process.platform === "darwin"`: ```ts using _tty = process.platform === "darwin" ? forwardFreshTtyToStdin() : { [Symbol.dispose]: (): void => { /* no-op on non-Darwin */ } }; ``` - **Linux / WSL / Windows**: no workaround installed, loop drains naturally. Keeps the performance/simplicity win from #835. - **macOS**: workaround installs, keystrokes deliver — original bug fixed again. ### `src/commands/init.ts` Restore the `setTimeout(process.exit, 100).unref()` safety net from #833, also gated to darwin: ```ts if (process.platform === "darwin" && process.env.NODE_ENV !== "test") { setTimeout(() => { process.exit(process.exitCode ?? 0); }, 100).unref(); } ``` The safety net is needed on macOS because the workaround opens a second `tty.ReadStream` which leaks a libuv handle (upstream [oven-sh/bun#29126](oven-sh/bun#29126)). On Linux that leak doesn't occur (no workaround installed). The `NODE_ENV=test` gate ensures `bun test` doesn't accumulate unref'd timers mid-suite. ### Restored (unchanged from pre-#835 state) - `src/lib/init/stdin-reopen.ts` (320 lines) — includes all hardening from #824 (`using`/Symbol.dispose, termios restore), #831 (`original.pause.call` in teardown), and #833-era CI fixes (`Object.defineProperty` for `isTTY`). - `test/lib/init/stdin-reopen.test.ts` (452 lines) — 15 tests covering install/teardown lifecycle via `/dev/ptmx` fixture. ## Test plan - [x] `bun test test/lib/init/ test/commands/init.test.ts` — 193 pass - [x] `bun test --timeout 15000 test/lib test/commands test/types` — 5777 pass, 0 fail - [x] `bun run typecheck` — clean - [x] `bun run lint` — clean (only pre-existing markdown.ts warning) - [ ] **Required**: Manual verification on macOS via nightly: `curl -fsSL https://cli.sentry.dev/install | SENTRY_VERSION=nightly SENTRY_INIT=1 bash`. Expect: first clack prompt accepts keystrokes, wizard completes, shell returns promptly after "Setup complete". - [ ] Also verify on Linux (Ubuntu glibc + Alpine musl) and WSL that the workaround is NOT installed and exit is still clean. ## Risk Low. Platform-gated to darwin, so any bugs in the workaround only affect macOS users. Non-Darwin path unchanged from #835 (proven clean on Linux). If macOS has further issues, the revert is a single commit away. ## Follow-ups - File an upstream Bun bug specifically about the macOS compiled-binary keystroke-delivery issue (distinct from oven-sh/bun#29126 which is about `process.stdin`'s readable-listener polling). A minimal repro running the stock `readline` module on a redirected `/dev/tty` fd should be straightforward. - Revisit when Bun 1.3.12+ is available — if the macOS bug is fixed upstream, we can delete the workaround for good.
5 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
1 participant
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.
Summary
Follow-up hardening on top of #802. The previous PR replaced
process.exit(0)with an explicittry/finally { closeFreshTtyForwarding() }. A retrospective review surfaced five non-blocking concerns and several test gaps — this PR addresses all of them.Fixes #798 (follow-up).
Motivation
#802was a good direction but left several loose ends:isTTYwas reset to a hardcodedundefined— stomps on any concurrent writer.EBADFon theReadStreamafterdestroy(), crashing the process if the error listener was already removed.using/Symbol.disposepattern gives us compiler-enforced teardown on every exit path — strictly better than a hand-rolledtry/finally.stdin-reopen.test.tsonly covered null-state paths; none of the install→teardown state transitions were directly asserted.Changes
Correctness fixes
previousIsTtyinstead of a boolean flag. Teardown restores the exact pre-install value, future-proofing against concurrent writers that might setisTTYduring the wizard.try { fresh.setRawMode(false); } catch {}ahead offresh.destroy()ensures a mid-prompt crash doesn't leave the user's shell in raw mode.errorlistener attached acrossdestroy(). Bun can asynchronously emit EBADF after the fd closes; removing the listener pre-destroy would crash the process. Verified by a local repro showingrs error: EBADFfires afterdestroy()completes.API refactor
forwardFreshTtyToStdinnow accepts an optionalTtyDepsobject ({ openTty, isTty }) for dependency injection. Production callers pass no args; tests pass/dev/ptmx+isTty: () => true.Disposable(TtyForwardingHandle) —using tty = forwardFreshTtyToStdin()works without null-checks.runWizardInnermerges back intorunWizard; the outertry/finallydisappears in favor ofusing _tty = forwardFreshTtyToStdin().Test coverage
isTTYbackfill branches (set / unset / already-set), re-install after teardown, secondary-caller no-op semantics, termios-restore tolerance,using-scope lifetime,using-on-throw teardown. Uses/dev/ptmxas a pseudo-TTY fixture so the install path runs in piped-stdinbun testenvironments. Skips gracefully on platforms without/dev/ptmx.WizardErrorrethrow path, which the catch-block reorder in fix(init): release /dev/tty handle on all exit paths #802 made newly responsible for stopping the spinner.closeFreshTtyForwardingSpyfires exactly once viausingteardown.Test plan
bun test test/lib/init/stdin-reopen.test.ts test/lib/init/wizard-runner.test.ts test/commands/init.test.ts— 65 pass, 0 failbun test test/lib/init/ test/commands/init.test.ts— 191 pass, 0 failbun run typecheck— cleanbun run lint— clean (only pre-existing markdown.ts warning)Notes
tsconfig.jsonalready hastarget: "ESNext", sousing/Symbol.disposework without any build-config change.AbortControllerinto theMastraClientlifecycle to release in-flight fetch keep-alive sockets on teardown.