fix: stop test scripts from inheriting the terminal

[bananaplain]

Any command in a test script can change the settings of the terminal the TUI runs on, because the script inherits it. flutter is one example: when it exits, it turns line mode and echo back on. Usually that would be an example of a command "playing nice", but in this case it breaks our TUI.

After a flutter command runs, the TUI is broken:

• mouse movement prints as raw escape codes (^[[<35;2;16M…)

• shortcuts stop working

The render itself carries on as normal and the TUI keeps being redrawn.

Fix

Start test scripts with an empty stdin (/dev/null). stdout and stderr were already redirected into a pipe, so stdin was the only one still connected to the terminal:

subprocess.Popen(
    cmd,
    stdin=subprocess.DEVNULL,   # new: empty input instead of the terminal
    stdout=subprocess.PIPE,     # already there: codeplain collects the output
    stderr=subprocess.STDOUT,   # already there: errors go into the same pipe
)

NOTE ⚠️

If a test script happens to run docker run -t / docker exec -it it will now fail with "the input device is not a TTY". This can be fixed by dropping the -t. A quick sweep of codeplain repos didn't find such a script, but it would already have failed whenever a codeplain render was started without a terminal on stdin, e.g. in headless mode.

Summary by CodeRabbit

• Bug Fixes
  • Scripts launched by the app now run without access to the app’s standard input. This prevents them from unexpectedly reading input intended for the surrounding process and helps them exit reliably when they reach the end of their input.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9d12fbd9-236c-4298-a365-6617c215e48f

📥 Commits

Reviewing files that changed from the base of the PR and between 16bcf5d and e49b73e.

📒 Files selected for processing: 2

• render_machine/render_utils.py
• tests/test_execute_script.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

execute_script now launches child scripts with standard input disconnected. A non-Windows test checks that a script reads EOF even when file descriptor 0 contains input.

Changes: Script standard input isolation

Layer / File(s)	Summary
Redirect child input and verify EOF render_machine/render_utils.py, tests/test_execute_script.py	execute_script sets child standard input to subprocess.DEVNULL. The test supplies input on file descriptor 0 and checks that the child script exits successfully after reading EOF.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: nejcs

Merge Risk: ⚪ Minimal · up to e49b7

Test scripts no longer inherit the terminal's input, which avoids the terminal corruption described in the PR. Scripts that need a TTY, such as docker run -t, may fail, but the PR already documents that and suggests dropping -t. Merge risk is minimal.

Security architecture risk: ⚪ Minimal · up to e49b7

Launched scripts lose access to the caller’s standard input without gaining new privileges or execution paths. Scripts requiring interactive input may need adjustment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The input-boundary change applies to scripts launched through execute_script. The same DEVNULL setting is used after either the Windows PowerShell or non-Windows command-selection branch; the added regression test covers only non-Windows execution.

Trust Boundaries and Controls

• inferred — A launched script can no longer consume caller input through inherited stdin because its fd 0 is bound to DEVNULL. This narrows the parent-to-child data path without changing executable reachability, arguments or session-creation policy.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preventing test scripts from inheriting the terminal.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}