fix(workspace/builder): Wait for shell prompt before layout and commands#1018
fix(workspace/builder): Wait for shell prompt before layout and commands#1018
Conversation
…layout and commands why: WorkspaceBuilder sends SIGWINCH (via select_layout) and commands (via send_keys) to panes before their shell finishes initializing (#365, open since 2018). With zsh, this triggers the PROMPT_SP partial-line marker — an inverse+bold "%" — because: tmux: screen_reinit() sets cx=cy=0 on new panes (screen.c:107-108). select_layout → layout_fix_panes → window_pane_resize enqueues a resize, and window_pane_send_resize delivers SIGWINCH via ioctl(TIOCSWINSZ) on the pane's PTY (window.c:449). zsh: preprompt() (utils.c:1530) runs the PROMPT_SP heuristic (utils.c:1545-1566) before every prompt. It outputs the PROMPT_EOL_MARK (default: "%B%S%#%s%b") to detect partial lines, then pads with spaces and returns the cursor. When SIGWINCH arrives mid-init, the interrupted redraw leaves the marker visible. what: - Add _wait_for_pane_ready() that polls cursor position until it moves from origin (0,0), indicating the shell has drawn its prompt - Call readiness check before select_layout in iter_create_panes() so SIGWINCH only arrives after the shell can handle resize gracefully - Wait for all default-shell panes, not just those with commands (blank panes via "- pane" shorthand expand to shell_command: []) - Remove redundant select_layout call in build() that doubled SIGWINCH signals after each pane yield - Add tests for readiness detection, timeout, call count, and layout deduplication
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1018 +/- ##
==========================================
+ Coverage 80.11% 80.32% +0.21%
==========================================
Files 28 28
Lines 2409 2430 +21
Branches 457 462 +5
==========================================
+ Hits 1930 1952 +22
- Misses 356 357 +1
+ Partials 123 121 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if "layout" in window_config: | ||
| window.select_layout(window_config["layout"]) | ||
|
|
There was a problem hiding this comment.
I did some code archaeology on this because I had the same concern about reintroducing old layout/space bugs.
This specific select_layout() call looks like the redundant one, not the essential one. The builder still applies select_layout() per-pane in iter_create_panes(), which is the part that actually preserves
the incremental rebalance during pane creation. The call removed here was the second pass in build(), so it was effectively doubling the layout application after each yielded pane.
The historical “no space for new pane” regressions were more closely tied to session geometry than to this duplicate call. In 2022, #793 changed both layout timing and default-size, and the fallout in #800
was mainly from the server starting at 80x24 before a client attached. The later fix path was to size new_session() correctly (8a59203, then 0777c3b/63ac676d), not to keep a second select_layout()
here.
I also checked the current regressions: test_issue_800_default_size_many_windows still passes, and this branch adds test_select_layout_not_called_after_yield specifically to lock in the intended behavior:
call select_layout() once per pane, not twice. So I don’t think removing this duplicate call should reintroduce the lack-of-space issue.
tony
changed the title
…iness check why: The except block in _wait_for_pane_ready discarded the exception traceback, making it impossible to diagnose pane refresh failures. what: - Add exc_info=True to logger.debug call in the except block
…ness skip coverage
why: The window_config.get("window_shell") fallback path at builder.py:566
was untested. Only pane-level shell: had fixture coverage.
what:
- Add skips_all_panes_with_window_shell fixture to PANE_READINESS_FIXTURES
- Verifies expected_wait_count=0 when window_shell is set
…c scope why: The readiness skip for panes with shell/window_shell was undocumented, making it unclear to future maintainers why pane_shell is None is the check. what: - Add comment explaining that shell/window_shell runs a command launcher, not an interactive shell, so there is no prompt to wait for
…o 5s why: The 2s timeout was too tight for slow environments, causing flaky doctest failures when the shell hadn't drawn its prompt yet. what: - Bump _wait_for_pane_ready doctest timeout from 2.0 to 5.0 seconds
why: Tmux doctests are inherently timing-sensitive — shell startup races can cause spurious failures even with generous timeouts. what: - Add pytest_collection_modifyitems hook to apply flaky(reruns=2) marker to DoctestItems from DOCTEST_NEEDS_TMUX modules
1 participant
Summary
Fixes #365 (open since March 2018). WorkspaceBuilder sends
SIGWINCH(viaselect_layout) andcommands (via
send_keys) to panes before their shell finishes initializing. With zsh, thisproduces the
PROMPT_SPpartial-line marker — an inverse+bold%.Root cause traced through tmux and zsh source:
screen_reinit()initializes new pane cursors at(0, 0)(screen.c:107-108).select_layouttriggersioctl(TIOCSWINSZ)on the pane PTY (window.c:449), deliveringSIGWINCHto the child shell.preprompt()(utils.c:1530) runs thePROMPT_SPheuristic (utils.c:1545-1566)before every prompt, outputting the
PROMPT_EOL_MARK(default%B%S%#%s%b). WhenSIGWINCHarrives mid-init, the interrupted redraw leaves the
%marker visible.Fix: Poll pane cursor position until it moves from origin
(0, 0)— indicating the shellhas drawn its prompt — before calling
select_layout. This ensuresSIGWINCHonly arrivesafter the shell can handle resize gracefully.
Changes
_wait_for_pane_ready()that pollscursor_x/cursor_yviapane.refresh()(2s timeout, 50ms interval)select_layoutiniter_create_panes()for all default-shell panes- paneshorthand expands toshell_command: [])select_layoutcall inbuild()that doubled SIGWINCH signalsTest plan
uv run ruff check . --fix --show-fixes— cleanuv run ruff format .— unchangeduv run mypy— no issues in 120 filesuv run py.test -vvv— 1027 passed, 2 skippedtmuxp load projectswith 12-window × 4-pane zsh config — no%markers