Skip to content

fix(runner): read child output by chunk to prevent hanging on interactive prompts#13

Merged
insign merged 2 commits into
mainfrom
fix/buffered-output-runner-312348395045082456
Apr 28, 2026
Merged

fix(runner): read child output by chunk to prevent hanging on interactive prompts#13
insign merged 2 commits into
mainfrom
fix/buffered-output-runner-312348395045082456

Conversation

@insign
Copy link
Copy Markdown
Contributor

@insign insign commented Apr 23, 2026

This PR changes the CommandExecutor streaming logic to read stdout and stderr by byte chunks rather than line-by-line. This fixes a major UX bug where commands that pause for user input without a trailing newline (like sudo password prompts or apt-get confirmations) would appear completely hung because the output stream was waiting for a newline to yield the prompt to the user. This guarantees real-time output synchronization.


PR created automatically by Jules for task 312348395045082456 started by @insign

…tive prompts

Previously, the command executor read child process stdout/stderr line-by-line using `BufReader::lines().next_line()`. This blocked output buffering if the process prompted for user input without emitting a newline (e.g., `[Y/n] ` or `[sudo] password:`). The user would see a hung process.
This changes the reading logic to use chunked byte-reading `tokio::io::AsyncReadExt::read()`, which streams partial lines immediately to the user's terminal, greatly improving reliability and UX for interactive commands.

Co-authored-by: insign <1113045+insign@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6923ec208

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/executor/runner.rs
match res {
Ok(0) => stdout_done = true,
Ok(n) => {
print!("{}", String::from_utf8_lossy(&stdout_buf[..n]));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve UTF-8 boundaries when streaming chunked output

Decoding each read() chunk independently with String::from_utf8_lossy can corrupt valid UTF-8 output whenever a multibyte character is split across chunk boundaries. In that case, this path emits replacement characters () and loses the original text, so commands that print non-ASCII output (localized package managers, Unicode filenames, etc.) can display garbled prompts/logs. The same pattern is used for both stdout and stderr, so both streams are affected.

Useful? React with 👍 / 👎.

@insign
Copy link
Copy Markdown
Contributor Author

insign commented Apr 28, 2026

Este PR ainda não pode ser mesclado porque o job Security Audit está vermelho. O cargo audit encontrou vulnerabilidades em rustls-webpki 0.103.10: RUSTSEC-2026-0104, RUSTSEC-2026-0098 e RUSTSEC-2026-0099, via reqwest/rustls. Atualize o lock/dependências para resolver para rustls-webpki >= 0.103.13 (ou outra versão compatível sem advisory) e mantenha lint/testes Ubuntu/Windows verdes.

@google-labs-jules
Copy link
Copy Markdown
Contributor

Este PR ainda não pode ser mesclado porque o job Security Audit está vermelho. O cargo audit encontrou vulnerabilidades em rustls-webpki 0.103.10: RUSTSEC-2026-0104, RUSTSEC-2026-0098 e RUSTSEC-2026-0099, via reqwest/rustls. Atualize o lock/dependências para resolver para rustls-webpki >= 0.103.13 (ou outra versão compatível sem advisory) e mantenha lint/testes Ubuntu/Windows verdes.

Updated rustls-webpki to version 0.103.13 via cargo update to resolve the security advisories (RUSTSEC-2026-0104, RUSTSEC-2026-0098, RUSTSEC-2026-0099). The tests and linters remain green.

Previously, the command executor read child process stdout/stderr line-by-line using `BufReader::lines().next_line()`. This blocked output buffering if the process prompted for user input without emitting a newline (e.g., `[Y/n] ` or `[sudo] password:`). The user would see a hung process.
This changes the reading logic to use chunked byte-reading `tokio::io::AsyncReadExt::read()`, which streams partial lines immediately to the user's terminal, greatly improving reliability and UX for interactive commands.
This commit also updates `rustls-webpki` to version `0.103.13` to resolve security vulnerabilities (RUSTSEC-2026-0104, RUSTSEC-2026-0098, RUSTSEC-2026-0099).

Co-authored-by: insign <1113045+insign@users.noreply.github.com>
@insign insign merged commit baa54da into main Apr 28, 2026
9 checks passed
@insign insign deleted the fix/buffered-output-runner-312348395045082456 branch April 28, 2026 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant