Skip to content

Fix affected stdin terminal detection#2555

Open
DominicVonk wants to merge 1 commit into
moonrepo:masterfrom
DominicVonk:fix-changed-files-stdin-terminal
Open

Fix affected stdin terminal detection#2555
DominicVonk wants to merge 1 commit into
moonrepo:masterfrom
DominicVonk:fix-changed-files-stdin-terminal

Conversation

@DominicVonk

@DominicVonk DominicVonk commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • Only auto-read changed files from stdin when stdin is not an interactive terminal.
  • Adds a comment explaining that interactive terminals would otherwise hang waiting for EOF.
  • Adds coverage for moon query affected consuming changed files from stdin.

Validation

  • git diff --check
  • Could not run Rust formatting/tests locally because cargo and rustfmt are not installed in this container.

@DominicVonk DominicVonk force-pushed the fix-changed-files-stdin-terminal branch 2 times, most recently from 9f420b1 to ac20881 Compare June 4, 2026 22:38
@DominicVonk DominicVonk changed the title [codex] Fix changed files stdin detection [codex] Make affected query stdin explicit Jun 4, 2026
@milesj

milesj commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

@DominicVonk Unfortunately this is a breaking change.

@dominicvonk-milence

dominicvonk-milence commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Sorry, currently still working on it

@DominicVonk DominicVonk force-pushed the fix-changed-files-stdin-terminal branch from ac20881 to 3f4874b Compare June 4, 2026 22:47
@DominicVonk DominicVonk changed the title [codex] Make affected query stdin explicit [codex] Improve affected stdin auto detection Jun 4, 2026
@DominicVonk DominicVonk force-pushed the fix-changed-files-stdin-terminal branch 3 times, most recently from 12cab8f to e0ed010 Compare June 5, 2026 06:53
@DominicVonk DominicVonk changed the title [codex] Improve affected stdin auto detection [codex] Fix affected stdin terminal detection Jun 5, 2026
@DominicVonk DominicVonk force-pushed the fix-changed-files-stdin-terminal branch from e0ed010 to 94df1ba Compare June 5, 2026 06:56
@DominicVonk DominicVonk changed the title [codex] Fix affected stdin terminal detection Fix affected stdin terminal detection Jun 5, 2026
@DominicVonk DominicVonk marked this pull request as ready for review June 5, 2026 06:58
Copilot AI review requested due to automatic review settings June 5, 2026 06:58
Copilot AI reviewed Jun 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR prevents query changed-files from auto-reading stdin when running in an interactive terminal (avoiding hangs waiting for EOF), and adds a CLI integration test to ensure affected projects can be selected via stdin when input is piped.

Changes:

  • Gate stdin auto-read on whether stdin is a terminal (!stdin().is_terminal()).
  • Add an integration test covering query affected reading file paths from stdin.

Reviewed changes

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

File Description
crates/cli/tests/query_affected_test.rs Adds coverage for piping a file path via stdin to query affected.
crates/app/src/queries/changed_files.rs Avoids blocking for stdin in interactive terminals by only reading stdin when non-terminal.

Comment thread crates/app/src/queries/changed_files.rs
Comment on lines +228 to +229
// Only auto-read stdin for piped input, otherwise interactive terminals hang waiting for EOF.
stdin: !stdin().is_terminal(),
Comment thread crates/app/src/queries/changed_files.rs
local: !ci,
stdin: true,
// Only auto-read stdin for piped input, otherwise interactive terminals hang waiting for EOF.
stdin: !stdin().is_terminal(),
@codspeed-hq

codspeed-hq Bot commented Jun 5, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 44.22%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

❌ 1 regressed benchmark
✅ 22 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ WallTime hash_files[1000] 9.8 ms 17.6 ms -44.22%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

Comparing DominicVonk:fix-changed-files-stdin-terminal (94df1ba) with master (2f3491e)

Open in CodSpeed

@codspeed-hq

codspeed-hq Bot commented Jun 5, 2026

Copy link
Copy Markdown

Unable to generate the flame graphs

The performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists.

@milesj

milesj commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

@DominicVonk We already have checks around stdin here: https://github.com/moonrepo/moon/blob/master/crates/app/src/queries/changed_files.rs#L189

What issues are you running into exactly?

@harlequin

harlequin commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

I have also the same problem, our environment is azure devops pipelines.
My workaround so far is

set -xe     
faketty () {
  script -qefc "$(printf "%q " "$@")" /dev/null
}     
faketty moon query tasks --affected --upstream deep --downstream deep

When running without this faketty it will wait in an endless loop.

@DominicVonk

DominicVonk commented Jun 10, 2026

Copy link
Copy Markdown
Author

This exact problem our current workaround is to pipe /dev/null as stdin

@milesj

milesj commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

stdin().is_terminal() is not a good enough check then, as that is already in place.

@DominicVonk

DominicVonk commented Jun 10, 2026

Copy link
Copy Markdown
Author

Yes, indeed. I will debug further and update the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@DominicVonk @milesj @dominicvonk-milence @harlequin