fix(tui): keep parent stdin paused after exit by fuller-stack-dev · Pull Request #93159 · openclaw/openclaw

fuller-stack-dev · 2026-06-15T04:20:29Z

Summary

Keep the parent CLI stdin stream paused after an inherited-stdio TUI child exits so /exit does not leave the parent process/container alive.
Stop onboarding finalize cleanup from resuming stdin around the post-setup TUI handoff.
Add focused regression coverage for the parent-stdin lifecycle.

Verification

Pre-fix clean-head PTY reproduction: parent script called launchTuiCli, child exited 0, output printed launch returned, then timed out waiting for parent exit with status 124.
Azure Crabbox real PTY reproduction (run_aa5f98a89a2c, lease cbx_5c205da5a519): same parent/child PTY proof printed launch returned and exited 0 while the PTY master remained open.
node scripts/run-vitest.mjs src/tui/tui-launch.test.ts src/wizard/setup.finalize.test.ts -> 23 tests passed.
node scripts/run-vitest.mjs run --config test/vitest/vitest.tui-pty.config.ts -> 13 tests passed.
env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 corepack pnpm check:changed -> passed typecheck, lint, guards, and import-cycle checks.
git diff --check -> passed.
Trusted autoreview -> autoreview clean: no accepted/actionable findings reported.

Real behavior proof

Behavior or issue addressed: Bare/inherited-stdio TUI /exit can return from the child while the parent process remains alive because parent stdin was resumed.
Real environment tested: Azure Crabbox Linux PTY using @lydell/node-pty, preserving the PTY master open after the child TUI process exits.
Exact steps or command run after this patch: Ran a PTY reproduction that imports src/tui/tui-launch.ts, marks parent stdin flowing with process.stdin.resume(), launches an inherited-stdio child entry that exits 0 for tui, and waits up to 4 seconds for the parent to exit.
Evidence after fix (copied live console output):

PTY output:
launch returned
PTY result: {"exitCode":0,"signal":0,"timedOut":false}

Observed result after fix: Parent process exits after the TUI child exits, so the container/PTY is not kept alive by a resumed stdin stream.
What was not tested: Full Docker interactive package lane was not run; the focused PTY proof covers the inherited-stdio lifecycle that caused the observed Docker hang.

clawsweeper · 2026-06-15T04:22:26Z

Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 1:45 AM ET / 05:45 UTC.

Summary
The PR removes stdin resumption from the inherited-stdio TUI relaunch path, stops setup finalize from resuming stdin around the TUI hatch, and updates focused regression tests.

PR surface: Source -6, Tests +15. Total +9 across 4 files.

Reproducibility: yes. via the PR's PTY parent/child reproduction and source inspection. Current main resumes stdin after the child exits, while terminal-core documents that resuming stdin can keep stdin_open container parents alive.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] Full Docker interactive package replay was not run; the supplied Azure PTY proof and TUI PTY lane cover the inherited-stdio lifecycle directly.

Maintainer options:

Decide the mitigation before merge
Land the focused TUI/setup stdin lifecycle fix after required checks and maintainer review, keeping cleanup-then-exit paths from resuming stdin by default.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P2] No repair lane: the open PR already contains the focused patch, accepted proof, and regression coverage; remaining work is ordinary maintainer and required-check review.

Security
Cleared: The diff only changes TUI/setup stdin lifecycle code and colocated tests; it does not add dependencies, scripts, secret handling, or supply-chain surfaces.

Review details

Best possible solution:

Land the focused TUI/setup stdin lifecycle fix after required checks and maintainer review, keeping cleanup-then-exit paths from resuming stdin by default.

Do we have a high-confidence way to reproduce the issue?

Yes, via the PR's PTY parent/child reproduction and source inspection. Current main resumes stdin after the child exits, while terminal-core documents that resuming stdin can keep stdin_open container parents alive.

Is this the best way to solve the issue?

Yes. launchTuiCli is only used by the setup TUI hatch, and leaving stdin paused matches the existing terminal-core cleanup contract better than restoring the previous flowing state.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b3f315461ba7.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body now includes structured after-fix real PTY proof with copied live console output showing the parent process exits without timing out.
add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body now includes structured after-fix real PTY proof with copied live console output showing the parent process exits without timing out.
remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: The PR fixes a bounded TUI/setup process-lifecycle hang with limited blast radius.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body now includes structured after-fix real PTY proof with copied live console output showing the parent process exits without timing out.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body now includes structured after-fix real PTY proof with copied live console output showing the parent process exits without timing out.

Evidence reviewed

PR surface:

Source -6, Tests +15. Total +9 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	4	10	-6
Tests	2	20	5	+15
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	24	15	+9

What I checked:

Current main restores flowing stdin after TUI exit: Current main records whether stdin was paused, pauses stdin while the inherited-stdio child runs, then resumes stdin in a finally block when it was previously flowing. (src/tui/tui-launch.ts:99, b3f315461ba7)
PR head keeps stdin paused after child exit: The PR head removes the previous-state restore path and leaves stdin paused after the TUI child exits so setup/container parents can terminate. (src/tui/tui-launch.ts:99, 6e59ac6406ae)
Setup TUI handoff matches cleanup-then-exit contract: The PR head changes both pre-setup and post-setup TUI restore calls to avoid resuming paused stdin around the TUI hatch handoff. (src/wizard/setup.finalize.ts:532, 6e59ac6406ae)
Terminal restore contract supports the fix shape: Terminal core documents that resuming stdin can keep stdin_open Docker parents alive and defaults resumeStdinIfPaused to false for cleanup-then-exit paths. (packages/terminal-core/src/restore.ts:38, b3f315461ba7)
Regression coverage added for the affected lifecycle: The updated tests assert launchTuiCli pauses but does not resume stdin after the child exits, and setup finalize now expects resumeStdinIfPaused: false around the TUI hatch. (src/tui/tui-launch.test.ts:140, 6e59ac6406ae)
Updated proof and checks: The PR body now includes copied Azure Crabbox PTY output showing launch returned with exitCode 0 and timedOut false, and live PR checks show check-lint, check-test-types, check-guards, and later Real behavior proof runs succeeded. (6e59ac6406ae)

Likely related people:

shakkernerd: Merged Fix setup TUI hatch terminal handoff #69524, which introduced the fresh-child setup TUI hatch and restore-before/after handoff touched by this PR. (role: introduced setup TUI handoff; confidence: high; commits: 4aeb81eeeb1e, 3669babf8f57, 459f1fc12ddb; files: src/tui/tui-launch.ts, src/wizard/setup.finalize.ts)
joshavant: Merged Wizard: bound hatch TUI timeout #76241, which recently changed the setup TUI hatch timeout path and added coverage across setup finalize and launchTuiCli tests. (role: recent adjacent contributor; confidence: medium; commits: a9cb4539a738; files: src/wizard/setup.finalize.ts, src/wizard/setup.finalize.test.ts, src/tui/tui-launch.test.ts)
fuller-stack-dev: Beyond this PR, authored merged fix(tui): bound standalone exit #83501 for adjacent standalone TUI /exit lifecycle behavior. (role: recent TUI exit contributor; confidence: medium; commits: 94ec14db252e, bd3920ea038c; files: src/tui/tui.ts, src/cli/tui-cli.ts, src/tui/tui-command-handlers.test.ts)
vincentkoc: Earlier Docker onboarding hang discussion traced stdin.resume with stdin_open as the relevant failure mode and recommended avoiding stdin resume for onboarding exit paths. (role: adjacent stdin/Docker lifecycle investigator; confidence: medium; commits: 2413f48fc89b; files: packages/terminal-core/src/restore.ts, src/commands/onboard-interactive.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

vincentkoc · 2026-06-15T09:37:34Z

Maintainer review complete at 6e59ac6406ae508fc1b0686ec12de6c1c7af8815.

Best-fix verdict: yes. launchTuiCli has one production caller, the setup TUI hatch, and terminal-core already defines cleanup-then-exit paths as leaving stdin paused. Resuming stdin recreates the observed container/PTY hang; forcing process exit would be a broader and more abrupt fix.

Fresh verification:

node scripts/run-vitest.mjs src/tui/tui-launch.test.ts src/wizard/setup.finalize.test.ts -> 23 passed
node scripts/run-vitest.mjs run --config test/vitest/vitest.tui-pty.config.ts -> 13 passed
git diff --check origin/main...HEAD -> passed
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --stream-engine-output -> clean, no accepted/actionable findings, patch correct at 0.94 confidence
Live PR state -> mergeable CLEAN; substantive CI, security, CodeQL, and tui-pty checks green; no required checks reported

Real behavior proof accepted from the PR body: pre-fix PTY parent timed out with status 124 after launch returned; Azure Crabbox run run_aa5f98a89a2c / lease cbx_5c205da5a519 exits 0 after the patch with timedOut:false while the PTY master remains open.

Known proof gap: full Docker interactive package replay was not rerun. A fresh remote changed-gate attempt was blocked by missing Blacksmith CLI and expired Azure auth; the PR's supplied changed gate and full GitHub CI are green.

@fuller-stack-dev

Keep the setup TUI parent stdin paused after its inherited-stdio child exits so Docker and PTY setup parents terminate cleanly. Align pre/post setup terminal cleanup with the cleanup-then-exit contract and add lifecycle regression coverage. Thanks @fuller-stack-dev. (cherry picked from commit 773ffd8)

@fuller-stack-dev

Keep the setup TUI parent stdin paused after its inherited-stdio child exits so Docker and PTY setup parents terminate cleanly. Align pre/post setup terminal cleanup with the cleanup-then-exit contract and add lifecycle regression coverage. Thanks @fuller-stack-dev.

@fuller-stack-dev

Keep the setup TUI parent stdin paused after its inherited-stdio child exits so Docker and PTY setup parents terminate cleanly. Align pre/post setup terminal cleanup with the cleanup-then-exit contract and add lifecycle regression coverage. Thanks @fuller-stack-dev.

@fuller-stack-dev

Keep the setup TUI parent stdin paused after its inherited-stdio child exits so Docker and PTY setup parents terminate cleanly. Align pre/post setup terminal cleanup with the cleanup-then-exit contract and add lifecycle regression coverage. Thanks @fuller-stack-dev.

fix(tui): keep parent stdin paused after exit

37dbcad

openclaw-barnacle Bot added size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 15, 2026

test(tui): bind stdin spies

6e59ac6

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 15, 2026

vincentkoc self-assigned this Jun 15, 2026

vincentkoc merged commit 773ffd8 into openclaw:main Jun 15, 2026
201 of 212 checks passed

github-actions Bot mentioned this pull request Jun 15, 2026

📡 Upstream Digest — 2026-06-15 09:53 UTC curtismercier/openclaw-mods#1082

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(tui): keep parent stdin paused after exit#93159

fix(tui): keep parent stdin paused after exit#93159
vincentkoc merged 2 commits into
openclaw:mainfrom
fuller-stack-dev:codex/tui-exit-stdin-lifecycle

fuller-stack-dev commented Jun 15, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 15, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

fuller-stack-dev commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Real behavior proof

Uh oh!

clawsweeper Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentkoc commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fuller-stack-dev commented Jun 15, 2026 •

edited

Loading

clawsweeper Bot commented Jun 15, 2026 •

edited

Loading