Skip to content

flowey: don't leave the vhd download prompt open forever, timeout after 30s#3134

Open
chris-oo wants to merge 1 commit intomicrosoft:mainfrom
chris-oo:vhd-prompt-timeout
Open

flowey: don't leave the vhd download prompt open forever, timeout after 30s#3134
chris-oo wants to merge 1 commit intomicrosoft:mainfrom
chris-oo:vhd-prompt-timeout

Conversation

@chris-oo
Copy link
Copy Markdown
Member

This behavior is really annoying when you start a terminal, or run it in an agent. Fail the download step after 30s if the user didn't accept, or if the user didn't ask to skip the prompt in the first place.

@chris-oo chris-oo requested a review from a team as a code owner March 25, 2026 23:52
Copilot AI review requested due to automatic review settings March 25, 2026 23:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Flowey node responsible for downloading OpenVMM VMM test artifacts to avoid blocking indefinitely on the local “press enter to confirm download” prompt by introducing a 30s timeout.

Changes:

  • Add a 30-second timeout to the local interactive download confirmation prompt.
  • Return an error if the prompt times out (instead of waiting forever).

Comment on lines +231 to +235
if !skip_prompt {
let _ = std::io::stdin().read_line(&mut String::new());
// Only display the prompt for 30s before timing
// out.
let (tx, rx) = std::sync::mpsc::channel();

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

In non-interactive environments (stdin not a TTY), the prompt logic can still proceed immediately (read_line returns EOF) and start downloading without explicit user acceptance. Consider checking stdin().is_terminal() (or equivalent) here and failing with a message that the user must pass LocalOnlySkipDownloadPrompt/run in an interactive terminal.

Copilot uses AI. Check for mistakes.
Comment on lines +242 to +244
let _ = rx
.recv_timeout(std::time::Duration::from_secs(30))
.map_err(|_| anyhow!("timed out waiting for input after 30s"))?;
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

The value received from recv_timeout is an io::Result<usize> from read_line, but it’s currently ignored. If read_line returns Ok(0) (EOF) or Err(_), this should be treated as “no acceptance” and fail the step (rather than implicitly accepting and continuing).

Suggested change
let _ = rx
.recv_timeout(std::time::Duration::from_secs(30))
.map_err(|_| anyhow!("timed out waiting for input after 30s"))?;
let read_result = rx
.recv_timeout(std::time::Duration::from_secs(30))
.map_err(|_| anyhow!("timed out waiting for input after 30s"))?;
match read_result {
Ok(n) if n > 0 => {
// User pressed enter; proceed.
}
Ok(0) => {
// EOF before confirmation: treat as no acceptance.
return Err(anyhow!(
"stdin closed (EOF) before confirmation; aborting download"
));
}
Err(e) => {
// Read error: treat as no acceptance.
return Err(anyhow!(
"failed to read confirmation from stdin: {e}"
));
}
}

Copilot uses AI. Check for mistakes.
@chris-oo chris-oo force-pushed the vhd-prompt-timeout branch from a78eaf3 to d0f6545 Compare March 26, 2026 00:10
Copilot AI review requested due to automatic review settings March 26, 2026 00:26
@chris-oo chris-oo force-pushed the vhd-prompt-timeout branch from d0f6545 to 7316980 Compare March 26, 2026 00:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

@chris-oo chris-oo force-pushed the vhd-prompt-timeout branch from 7316980 to f70363d Compare March 27, 2026 22:39
Copilot AI review requested due to automatic review settings March 27, 2026 22:57
@chris-oo chris-oo force-pushed the vhd-prompt-timeout branch from f70363d to 9314ed3 Compare March 27, 2026 22:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

@chris-oo chris-oo force-pushed the vhd-prompt-timeout branch from 9314ed3 to a2ea837 Compare March 31, 2026 18:16
Copilot AI review requested due to automatic review settings March 31, 2026 19:37
@chris-oo chris-oo force-pushed the vhd-prompt-timeout branch from a2ea837 to e6d14c9 Compare March 31, 2026 19:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

@github-actions
Copy link
Copy Markdown

…er 30s

feedback: use crossterm to read terminal input and poll

just download if not interactive
@benhillis benhillis force-pushed the vhd-prompt-timeout branch from e6d14c9 to f6625f8 Compare March 31, 2026 21:56
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.

3 participants