Stage 4: integrate reserved toolbar handoffs, job control, and resize recovery - #1761
Open
tleonhardt wants to merge 29 commits into
Open
tleonhardt wants to merge 29 commits into
tleonhardt wants to merge 29 commits into
Conversation
Keep nested prompts and selection on the shared reservation while transferring input ownership. Release physical rows before job-control suspension and before measuring the first pager frame, then reacquire using current terminal geometry. Restore bindings and terminal state after interrupted prompts, failed pager teardown, and reacquisition errors. Add transition regressions for completion, secrets, typeahead, resize, fallback, and signals. Validation: 2647 passed, 6 skipped; make check and make docs-test pass. Acceptance and dynamic harness gates pass at 12, 24, and 40 rows. Real-terminal and Windows qualification remain open.
Defer clearing the main-screen band until the first prepared pager frame is ready to commit, while exposing full terminal geometry during layout. Clear retained cells before managed output, external handoffs, recovery, or shutdown so they cannot leak into history. Add regressions for startup visibility and deferred cleanup. Validation: 2652 passed, 6 skipped; make check and make docs-test pass.
Resolve SIGTSTP conditionally before scheduling job-control suspension, preserving POSIX behavior without referencing a signal absent from Windows. Verified with mypy --platform win32 and the full test suite.
…hat raises an exception
Let terminal-attached POSIX pipelines share cmd2's process group so Ctrl-Z and fg transfer input ownership together. Avoid forwarding SIGINT back to that group, while retaining isolated groups for nonterminal pipelines and Windows. Add real PTY coverage for repeated suspend/resume, shell input, resizing, and command recovery, plus process-group and SIGINT regression tests.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## reserved_row_toolbar #1761 +/- ##
========================================================
+ Coverage 99.75% 99.81% +0.05%
========================================================
Files 35 35
Lines 8153 8511 +358
========================================================
+ Hits 8133 8495 +362
+ Misses 20 16 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Bash 5.1 and later turn bracketed paste off with "\x1b[?2004l\r" before running a command, so the reply to "stty size" follows a bare carriage return rather than "\r\n". The test's predicate required the latter and timed out on Linux CI while passing on macOS, whose bash 3.2 has no bracketed paste. Match the reply at any line boundary instead.
The helper pager announced readiness and resumption with print. Its SIGCONT handler could therefore run while the main thread was still inside the PAGER_READY print, since the test stops the job the instant that marker appears, and Python raised "reentrant call inside <_io.BufferedWriter>". The pager died with a traceback whose source line contained PAGER_RESUMED, which satisfied the first wait by accident, and the second timed out. Write the markers with os.write, which is safe from a signal handler, and match them as whole lines so a traceback can never satisfy the wait.
…paths Three paths had no test: declining to suspend where the platform or the stop signal is missing, refusing a nested session whose layout has no toolbar window, and invalidating a nested prompt's bridge when the terminal is handed to a guest program. Each new test fails if its line is removed.
…control test The shell takes the terminal back as soon as its direct child stops, but the helper pager, a grandchild, may not have processed its stop signal yet. It sits in a one-byte terminal read, and the tty read loop hands a woken reader any byte already queued before it checks for the pending stop, so the pager consumed the first keystroke of the probe command and the shell ran "rintf". Free-threaded builds under load widened the window enough to hit on CI. Wait until both cmd2 and the pager report the stopped state before typing.
Member
Author
|
@kmvanbrunt @bambu The flicker-free bottom bar is now in a more mature state where it is more completely integrated with I'll keep reviewing and testing. But if others have time for manual testing on other platforms use: uv run examples/getting_started.pyThe bottom bar should now be fully persistent unless you invoke a shell application which is a TUI and takes over the whole screen and it should remain flicker-free. Though if for some reason you shrink the terminal vertical height to a pathological level of only 2 rows, it will revert to a default Please report any bugs you find along with the following:
NOTE: This is not the "done" state. I'm anticipating a good deal of cleanup along with adding support for multiple reserved rows. |
…ad pipes A shell command piped to an interactive consumer, such as `shell git log | less`, hung: the child inherited the raw pipe descriptor, bypassing the writer that lends the terminal per write, so the consumer stopped on its first terminal access while the producer blocked on the full pipe. do_shell now spawns its child in the pipeline's process group and lends the terminal for the child's lifetime, so the consumer keeps the terminal and Ctrl-C and Ctrl-Z reach both processes as in a shell pipeline. If the pipeline exits before the child can join its group, the child runs in our own group as before. Because such a producer can outlive the consumer that led the group, ProcReader.send_sigint() falls back to the leader's pid as the group id once the leader has been reaped, rather than doing nothing. A pipe started off the main thread could not install job-control signal handlers and failed after Popen, leaving the child unreaped. The terminal pipeline path is now taken only on the main thread; elsewhere the pipeline keeps running in its own session.
Exercise ProcReader.terminal_group for readers without a terminal and for a finished pipeline, the shell command's retry in its own group when the pipeline's group is gone before it can join, and a PermissionError from Popen that has nothing to do with the join, which must still propagate.
A signal that interrupts a blocking pipe write, such as the job-control stop ProcReader relays for Ctrl-Z, returned a partial count, so PipelineWriter ended its lend and the terminal went back to cmd2's group for the instant before the next write re-lent it. A consumer that had just resumed a terminal read could hit that window and be stopped again with SIGTTIN, racing with the next Ctrl-Z in the job-control test on a loaded runner. Finish the buffer under a single lend instead. When a terminal condition times out, the job-control tests now also report the pty's foreground group and the state and wait channel of every process under the outer shell, so a silent transcript still shows which process was waiting for whom.
The watcher thread stopped cmd2's job by sending SIGTSTP to the whole process with os.kill(). A process-directed signal may be taken by any thread that does not block it, and only the main thread runs Python signal handlers, so when it went elsewhere while the main thread slept in a system call, the handler never ran: the pipeline stayed stopped, the terminal was already back with cmd2's group, and the outer shell never saw the job stop. CI caught this with a shell producer sleeping in waitpid, and the process listing showed the main thread still there with the pipeline stopped. Use pthread_kill() on the main thread, which queues the signal on that thread and interrupts its system call.
Only the main thread runs Python signal handlers, and the job-control stop the pipeline watcher relays can wake another thread, however it is addressed: CI on macOS showed the pipeline stopped, the terminal already returned, and cmd2's main thread still asleep in the shell command's waitpid, and the earlier Linux failures had it asleep in a pipe write. A thread that never returns from its system call never runs the handler, so the job is never suspended. Wait in short polls at both points instead, as wait_for_exit already does: PipelineWriter writes through a non-blocking pipe and polls for room, and the shell command waits in slices while its child belongs to a terminal pipeline. The job-control test now delivers the relay to the watcher thread itself in the two cases that failed, which reproduced the hang deterministically. The reserved-terminal pager test waits for the bar repainted after a handoff, as the command display renders on its own thread and can land it a frame later than the resume, which Windows CI hit.
Making the pipe non-blocking regressed shell commands piped to a terminal consumer: do_shell() hands the same open file description to the child, which writes to it directly and failed with EAGAIN once the pipe was full. Leave the descriptor blocking. PipelineWriter still returns to Python regularly, as the job-control relay requires, by polling for room and writing at most PIPE_BUF bytes at a time, which cannot block once the pipe reports it is writable. The job-control test's timeout diagnostics now cope with a sandbox that cannot run ps.
Ctrl-Z in the built-in pager, or at the command toolbar, sends SIGTSTP from the display thread. A process-directed signal lands on the main thread, and on Linux the display thread carries on for a few milliseconds: it restores raw mode, reattaches its input and redraws before the job has stopped. When the shell resumes the job it puts back the terminal modes it saved beforehand, and nothing is left to undo that. The terminal stays cooked, keys are held until Enter, and with a refreshing toolbar every cursor-position query is echoed as ^[[row;colR, as reported after fg with the embedded pager on Debian. After sending the signal, wait for the stop to take the sending thread as well. A stop shows as time passing while asleep; a signal that was ignored, or discarded for an orphaned process group, shows as nothing and the wait ends on its own. A thread-directed duplicate would be simpler, but macOS does not discard it when the job is continued, and the job stops again after fg. The command toolbar installs this as the application's suspend_to_background() unless the reserved toolbar's job-control hook, which releases its rows first and then stops the process the same way, is already there. The regression test drives the pager under an interactive bash on a pty, suspends and resumes it, and requires that it still answers q.
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
Complete Stage 4 of the reserved-row toolbar work: transfer terminal and input ownership correctly across nested prompts, paging, external applications, suspend/resume, and resize. This PR targets
reserved_row_toolbar, which already contains Stage 3; it retains the current one-row reservation and opt-in mode defaults.What changed
read_input(),read_secret(), andselect()borrow the existing reservation after the command reader stops. Temporary applications receive their own bridge, retain the current toolbar content/styles, and restore original bindings when they finish, are interrupted, or fall back. This avoids a second input reader and preserves completion, secret masking, and typeahead.SIGTSTPconditionally so Windows type checking and unsupported-platform behavior remain valid.help -v | less, Ctrl-Z, andfgcould leavelessrunning while cmd2 was stopped, allowing it to steal shell input. Avoid forwarding SIGINT back into cmd2's own group; retain isolated groups for nonterminal pipelines and the existing Windows process-group behavior.pyin the getting-started example and add an exception-raising script for manual testing. Add nested-input, pager, cleanup, polling, partial-output, and real-PTY job-control regressions.Review focus
The implementation retains the existing alternating main/command application lifecycle and serialized-output architecture. Review ownership restoration and failure ordering, especially nested suspension, alternate-screen exit, native Windows geometry, and partial output. The
ChoiceInput._create_application()integration and polling tests depend on prompt-toolkit internals; qualification remains at exactly 3.0.53. Broader dependency/backend qualification follows in Stage 8.Roadmap
The additional investigations requested after Stage 4 move the former Stage 5 release qualification to Stage 8.
reserved_row_toolbarvia #1758cmd2/toolbar/package; align focused tests with production modules; keep cross-module integration suites; separate mechanical moves from justified behavior-preserving refactorsValidation
At head
37645b59on Mac:make test: 2,667 passed, 6 skipped.Maintainer-reported manual results: nested prompts on Mac and Windows; embedded paging and external tool/interpreter recovery; Mac job-control edge cases; clean shell return; and the exact pipeline sequence in Mac iTerm2. The Windows 2 → 3-row issue was subsequently reported fixed and pulled in
37645b59; reviewers should repeat the exact boundary check below and attach the native result. Mac review of that commit found no blocking issues. PR CI is additional evidence, not a substitute for native terminal verification.Manual reviewer checklist
Run from the repository root with
uv run python examples/stage4_manual.py.stage4_manual.pyis a super-set of thegetting_started.pyapplication which adds commands for testing nested prompts viaread_input,read_secret, andselect. The example uses AUTO with a live timestamp. Record the commit, OS, terminal/version, shell, Python and prompt-toolkit versions, actual selected backend/mode, and measured viewport dimensions. AUTO can fall back: a visible native toolbar alone does not prove reserved rendering.scripts/toolbar_inventory.pycan help identify the environment; inspect its usage first because it includes interactive probes.Perform applicable checks on Mac/Linux and Windows Terminal. POSIX Ctrl-Z/fg is not expected in PowerShell. External TUIs intentionally hide the reserved toolbar while they own the terminal; it must return when they exit. Exact physical row counts matter, especially at the minimum height.
echo helloandwork 5; edit input and use completion/historynested; use Tab and Up, resize, then accept; repeat and press Ctrl-C or EOFsecretwith dummy text only; runchooseand use arrows/Enter; repeat with cancellation and resizenested, quickly enteralphafollowed by a newline andecho afterfollowed by a newlinecat cmd2/cmd2.py; use Space/PgDn, b/PgUp,/search,n, resize andqcat examples/scripts/raise_exception.pyin a normal-height terminalhelp -v | less; resize inside it, navigate and quit withqeditwith a disposable file and a configured editor; resize and exitpy/ipy, print text and exit normally; run a normalrun_pyscriptandrun_pyscript examples/scripts/raise_exception.pyhandoff, thenhandoff fail; resize at the guest input and complete itwork 10, press Ctrl-Z; run a shell command, resize,fg; repeatfginside the embedded pager; navigate and quit afterwardhelp -v | less, Ctrl-Z; type a complete shell command such asprintf 'shell owns input\n';fg;q; thenecho afterin cmd2quiet, thenpartial; resize repeatedly through 2/3/4 rows during the pausePARTIALENDremains intact and output never lands in the toolbar bandOptional commands for managed-input and handoff checks
Run
uv run python examples/stage4_manual.py. These commands execute inside the normal command lifecycle. Callingread_input()from the externalpyshell would instead exercise an already-suspended handoff.Fault-injection cases such as failing pager erasure, failed reacquisition and reservation abandonment are covered by automated tests; reviewers should inspect those outcomes rather than make production callbacks fail ad hoc. Record PASS/FAIL/not available per manual row, with terminal details and screenshots/diagnostic traces for failures. The broader release terminal matrix and configurable multiple reserved rows remain later-stage work.