fix: restore terminal state after remote→local mode switch#570
fix: restore terminal state after remote→local mode switch#570tomstetson wants to merge 2 commits intoslopus:mainfrom
Conversation
Fix terminal input corruption (dropped characters, phantom mode switches,
unresponsive Claude) when switching from remote mode back to local mode
via double-space.
Root causes addressed:
- Wrong cleanup order: raw mode was disabled BEFORE Ink unmounted, but Ink
expects raw mode during teardown. Now Ink unmounts first.
- setEncoding("utf8") was never reset: poisons stdin for subsequent
Buffer-mode consumers. Now reset via _readableState.
- process.exit() in signal handlers bypasses finally blocks: terminal
left in raw mode. Now restoreStdin() runs before exit.
- No signal forwarding to binary children: orphan Claude processes
compete for stdin. Now SIGINT/SIGTERM/SIGHUP forwarded.
- stdin left in flowing mode: no pause() after remote cleanup. Now
restoreStdin() pauses stdin.
Fixes slopus#301, slopus#422, slopus#423, slopus#424, slopus#430, slopus#443, slopus#527
Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)
Co-Authored-By: Claude <[email protected]>
Co-Authored-By: Happy <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b9a99171b
ℹ️ 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".
| if (!child.killed) { | ||
| child.kill(signal); |
There was a problem hiding this comment.
Forward all repeated termination signals to child
child.killed becomes true as soon as the first kill() is sent, not when the child has actually exited, so this guard drops every later signal even if the child is still running. In binary-install flows, if Claude traps the first signal (for graceful interrupt) and keeps running, a subsequent SIGTERM from a mode switch or shutdown will never be forwarded, which can leave the launcher waiting indefinitely for exit while the child continues owning stdin.
Useful? React with 👍 / 👎.
…rwarding child.killed becomes true immediately after kill() is called, not when the child actually exits. This caused subsequent signals to be silently dropped if the child trapped the initial signal and kept running, potentially leaving the launcher hanging while the child holds stdin. Replace the child.killed guard with an explicit childExited flag that is only set on the child's 'exit' event, and add a try/catch around kill() to handle the race between our check and the child exiting. Addresses review feedback from codex bot on PR slopus#570. https://claude.ai/code/session_01XFfhV1BPjmPfpEximDf6ev
child.killed becomes true after the first kill() call, not when the child actually exits. This means if the child traps the first signal (e.g., SIGINT for graceful shutdown), subsequent signals from mode switches would be silently dropped, leaving the launcher waiting indefinitely. Track exit state with a boolean instead, and wrap kill() in try-catch for the race where the child exits between our check and the kill call. Addresses review feedback from Codex on PR slopus#570. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <[email protected]> Co-Authored-By: Happy <[email protected]>
Adds restoreStdin utility, forwards SIGHUP alongside SIGINT/SIGTERM, and properly restores terminal raw mode after Claude binary exits. Prevents orphan processes competing for stdin. Cherry-picked from slopus/happy PR slopus#570 (commit 1/2). Co-authored-by: blechschmidt <[email protected]>
child.killed becomes true after the first kill() call, not when the child actually exits. This means if the child traps the first signal (e.g., SIGINT for graceful shutdown), subsequent signals from mode switches would be silently dropped, leaving the launcher waiting indefinitely. Track exit state with a boolean instead, and wrap kill() in try-catch for the race where the child exits between our check and the kill call. Addresses review feedback from Codex on PR slopus#570. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <[email protected]> Co-Authored-By: Happy <[email protected]>
Cherry-picked from upstream PRs: - slopus#566: Path normalization alignment with Claude Code (Adam Murphy) - slopus#561: YOLO mode race condition fix (Adam Murphy) - slopus#570: Terminal state restoration after remote→local switch (blechschmidt/tomstetson) - slopus#539: tmux detach stalling prevention (claudio) All 4 PRs applied successfully. slopus#561 required conflict resolution (merged effectiveMode race fix with AskUserQuestion guard).
Ports: slopus/happy#570 (partial) Refs: slopus/happy#570
child.killed becomes true after the first kill() call, not when the child actually exits. This means if the child traps the first signal (e.g., SIGINT for graceful shutdown), subsequent signals from mode switches would be silently dropped, leaving the launcher waiting indefinitely. Track exit state with a boolean instead, and wrap kill() in try-catch for the race where the child exits between our check and the kill call. Addresses review feedback from Codex on PR slopus#570. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <[email protected]> Co-Authored-By: Happy <[email protected]>
This comment was marked as abuse.
This comment was marked as abuse.
child.killed becomes true after the first kill() call, not when the child actually exits. This means if the child traps the first signal (e.g., SIGINT for graceful shutdown), subsequent signals from mode switches would be silently dropped, leaving the launcher waiting indefinitely. Track exit state with a boolean instead, and wrap kill() in try-catch for the race where the child exits between our check and the kill call. Addresses review feedback from Codex on PR slopus#570. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <[email protected]> Co-Authored-By: Happy <[email protected]>
|
Hit this twice on macOS (M1). Had to kill the terminal both times to recover. Would love to see this land — thanks for the fix @tomstetson. |
Cherry-picked from upstream PR slopus#647 (by Haknt, rebased from PR slopus#570 by tomstetson). Fixes stdin corruption when switching from remote back to local mode by addressing five root causes: cleanup sequencing, encoding persistence, signal handler bypass, missing signal forwarding, and flowing mode not being paused. Fixes: slopus#301, slopus#422, slopus#423, slopus#424, slopus#430, slopus#443, slopus#527, slopus#613 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Cherry-picked from upstream PR slopus#647 (by Haknt, rebased from PR slopus#570 by tomstetson). Fixes stdin corruption when switching from remote back to local mode by addressing five root causes: cleanup sequencing, encoding persistence, signal handler bypass, missing signal forwarding, and flowing mode not being paused. Fixes: slopus#301, slopus#422, slopus#423, slopus#424, slopus#430, slopus#443, slopus#527, slopus#613 Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary
Fixes the most-reported bug in Happy CLI: terminal input corruption after switching from remote mode (mobile) back to local mode via double-space. Users experience dropped characters, phantom mode switches, and an unresponsive Claude session.
Root cause analysis
Five independent bugs compound to corrupt stdin during the remote→local transition:
setEncoding("utf8")never resetprocess.exit()bypasses finally blocksprocess.exit()which skips the finally block that would restore the terminal — leaves it in raw modepause()after remote cleanup, so stdin data events keep firing into the voidApproach: why this fix over alternatives
Compared to PR #458 (pkill-based process cleanup)
PR #458 addresses the orphan process symptom by adding
HAPPY_SESSION_IDenvironment tracking andpkill-based cleanup with escalating SIGTERM→SIGKILL. While it correctly identifies the orphan problem, it:pkill -f "HAPPY_SESSION_ID=..."with hardcoded timeouts (100ms → 500ms) which is platform-dependent and fragileRemoteModeDisplay.tsxwithsetTimeout(100ms)that papers over the real ordering bugCompared to PR #553 (node-pty proxy)
PR #553 takes a nuclear approach: replace
stdio: 'inherit'with a full PTY proxy vianode-pty, giving Ink a separate/dev/ttyfd. This correctly isolates stdin but:node-pty) requiring compilation on every platform — the comment thread already found a PTY fd leak bug in node-pty v1.1.0 (fix: /dev/ptmx leak on macOS microsoft/node-pty#882)claudeLocal.tsincluding IPC socket plumbing to replace fd 3This PR's approach
Minimal, targeted fixes at each root cause — 94 lines added, 8 removed, zero new dependencies:
restoreStdin()utility (new file) — idempotent function that properly restores stdin: disable raw mode, pause, reset encoding via_readableState, remove orphaned listeners. Every operation wrapped in try-catch.Fix cleanup order in
claudeRemoteLauncher.ts— unmount Ink first (while raw mode is still active), then callrestoreStdin(). This is the core fix. Also removed dead code (process.stdin.off('data', abort)whereabortwas never a data listener).Defensive guard in
claudeLocal.ts— callrestoreStdin()before spawning child with inherited stdio. Catches edge cases where remote cleanup was incomplete.Signal handler protection in
runClaude.ts— callrestoreStdin()at the top of the cleanup function, before any async work orprocess.exit(). Prevents terminal being left in raw mode when signals bypass finally blocks.Signal forwarding in
claude_version_utils.cjs— forward SIGINT/SIGTERM/SIGHUP to binary child processes. Clean up handlers on child exit. Prevents orphan Claude processes.Files changed
packages/happy-cli/src/utils/restoreStdin.tspackages/happy-cli/src/claude/claudeRemoteLauncher.tsrestoreStdin()packages/happy-cli/src/claude/claudeLocal.tsrestoreStdin()before spawnpackages/happy-cli/src/claude/runClaude.tsrestoreStdin()in signal handlerpackages/happy-cli/scripts/claude_version_utils.cjsTest plan
tsc --noEmit)yarn build)vitest run)ps aux | grep claudeafter mode switches — verify no orphan processesGenerated with Claude Code
via Happy