fix(tui): recover Windows shell UI from mid-session console blanking#192
Merged
Conversation
The Windows shell UI could go blank mid-session (transcript and input box gone until terminal restart). Two mechanisms, both fixed: - Child processes attached to the interactive console: Shell-tool, background-task, and shell-escape spawns now pass CREATE_NO_WINDOW so a child touching the console via the Win32 console API (cls, Clear-Host, SetConsoleMode) cannot corrupt the TUI. Kill semantics are unchanged (TerminateProcess; no CTRL_BREAK usage anywhere). - prompt_toolkit's differential renderer diffing against a stale frame after the real screen diverged (ConPTY resize rewrap, terminal replay, half-completed teardown erase): the prompt renderer state is now reset after terminal geometry changes and failed scrollback handoffs so the next redraw is absolute. Also corrects the sync_output docstring: Windows10_Output delegates _buffer to its vt100 output, so DEC-2026 marks ARE installed on VT-capable Windows consoles (safe: WT expires after 100ms, xterm.js 5s).
- Add an explicit Ctrl+L binding (pins prompt_toolkit's default clear-screen) with a contained-failure handler, list it in /help shortcuts and docs/en/reference/keyboard.md. - Add spawn-flag tests for the background worker child and the `!` foreground shell command (CREATE_NO_WINDOW on Windows), plus hard-repaint tests. - Reviewed e2e cancellation failure: test_shell_cancel_running_command_kills_ process_and_recovers passes outside the sandboxed test runner and on CI; the local failure was sandbox interference with PTY signal delivery.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR updates Windows subprocess spawning to detach child processes from the interactive console and adds prompt repaint recovery for Ctrl+L, resize changes, and scrollback handoff failures. ChangesWindows subprocess console detachment
Prompt renderer hard repaint recovery
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CustomPromptSession
participant Renderer
User->>CustomPromptSession: press Ctrl+L
CustomPromptSession->>Renderer: clear()
CustomPromptSession->>CustomPromptSession: log debug on clear() failure
sequenceDiagram
participant Shell as Shell._run_shell_command
participant PromptLiveView as _PromptLiveView
participant Renderer
Shell->>Shell: set creationflags on Windows
PromptLiveView->>Renderer: reset() on resize
PromptLiveView->>Renderer: reset() on scrollback handoff failure
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/pythinker-host/src/pythinker_host/local.py`:
- Around line 199-210: The Windows creationflags logic is duplicated across
multiple subprocess launch paths and should be centralized. Extract the repeated
`getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", ...) | getattr(subprocess,
"CREATE_NO_WINDOW", ...)` into a shared helper (for example, in
`pythinker_host`) and have `local.py`, `background/manager.py`
(`_launch_worker`), `background/worker.py`, and the UI shell code call that
helper instead. Keep the fallback constant consistent in one place so the
behavior cannot drift between `process_options` setup sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b9ad1333-8816-427e-bae6-a6f1d123b0b9
⛔ Files ignored due to path filters (1)
docs/en/reference/keyboard.mdis excluded by!docs/**
📒 Files selected for processing (15)
CHANGELOG.mdpackages/pythinker-host/src/pythinker_host/local.pypackages/pythinker-host/tests/test_local_host.pysrc/pythinker_code/background/manager.pysrc/pythinker_code/background/worker.pysrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/ui/shell/sync_output.pysrc/pythinker_code/ui/shell/visualize/_interactive.pytests/background/test_manager.pytests/background/test_worker.pytests/ui_and_conv/test_prompt_hard_repaint.pytests/ui_and_conv/test_shell_bang_spawn.pytests/ui_and_conv/test_visualize_running_prompt.py
elkaix
changed the title
The CREATE_NEW_PROCESS_GROUP | CREATE_NO_WINDOW computation was duplicated across four subprocess spawn sites, and had already drifted: local.py used a 0x00000200 fallback for CREATE_NEW_PROCESS_GROUP while manager.py/worker.py used 0. Extract a shared windows_console_detach_flags() helper so the fallback constants and console-detachment logic can't diverge again.
Contributor
Author
|
All findings addressed in f22bc0c:
Codecov's 1-line patch-coverage note is left as-is — the |
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.
Related Issue
No linked issue.
Description
Fixes a Windows-specific bug where the shell UI's transcript and input box
could go blank mid-session and stay blank until the terminal was restarted.
pythinker_host.LocalHost.exec, the backgroundtask manager/worker, and the shell
!command runner no longer attach tothe interactive console on Windows (
CREATE_NO_WINDOWcombined withCREATE_NEW_PROCESS_GROUP), so a child touching the Win32 console API canno longer bypass redirected stdio and blank the parent TUI.
creationflagscomputation is centralized in a sharedpythinker_host.windows.windows_console_detach_flags()helper instead ofbeing duplicated (and drifting) across the four spawn sites.
Ctrl+L, afterterminal resizes, and after failed scrollback handoffs, instead of diffing
against a possibly stale frame.
Checklist
Summary by CodeRabbit
Bug Fixes
New Features
Documentation