fix: guard _read_line_nonblocking against sys.stdin is None (#1170)#1172
Open
fix: guard _read_line_nonblocking against sys.stdin is None (#1170)#1172
_read_line_nonblocking against sys.stdin is None (#1170)#1172Conversation
Add a None check at the top of _read_line_nonblocking that raises ValueError (already caught by _interactive_loop) before select.select can raise an uncaught TypeError. - Update docstring to mention sys.stdin is None as a trigger - Update implementation.md fallback documentation - Add unit test for the ValueError raise - Add integration test for _interactive_loop fallback path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the interactive CLI input path so _interactive_loop() no longer crashes when sys.stdin is None, which can happen in closed-stdin or CI/test environments. It fits the CLI’s existing fallback design by converting that condition into the same exception path already used for non-selectable stdin.
Changes:
- Added a guard in
_read_line_nonblocking()to raiseValueErrorwhensys.stdin is None. - Added unit and interactive-loop tests covering the new stdin-none path.
- Updated implementation docs to mention
sys.stdin is Noneas a fallback trigger.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/copilot_usage/cli.py |
Adds the stdin-none guard in the nonblocking reader docstring and implementation. |
src/copilot_usage/docs/implementation.md |
Updates the interactive-loop design docs for the new fallback trigger. |
tests/copilot_usage/test_cli.py |
Adds regression tests for _read_line_nonblocking() and interactive fallback behavior. |
| ### Fallback to threaded `_start_input_reader_thread()` | ||
|
|
||
| If `select()` raises `ValueError` or `OSError` (e.g. stdin is not selectable, notably on Windows, or stdin is detached during testing), the loop starts a daemon thread via `_start_input_reader_thread()` (in `cli.py`) that feeds lines into a `queue.SimpleQueue`: | ||
| If `select()` raises `ValueError` or `OSError` (e.g. stdin is not selectable, notably on Windows, stdin is detached during testing, or `sys.stdin is None`), the loop starts a daemon thread via `_start_input_reader_thread()` (in `cli.py`) that feeds lines into a `queue.SimpleQueue`: |
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
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.
Closes #1170
Problem
When
sys.stdin is None(e.g. process started with closed stdin, or certain test/CI environments),select.select([None], [], [], timeout)raises aTypeErrorthat isn't caught by theexcept (ValueError, OSError)handler in_interactive_loop. This crashes the interactive session instead of falling back to the threaded reader.Fix
Added a
sys.stdin is Noneguard at the top of_read_line_nonblockingthat raisesValueError("stdin is None"), which the existingexcept (ValueError, OSError)handler in_interactive_loopalready catches and handles via the threaded fallback.Changes
src/copilot_usage/cli.py— Addedif sys.stdin is None: raise ValueError("stdin is None")guard and updated docstring to documentsys.stdin is Noneas a trigger conditionsrc/copilot_usage/docs/implementation.md— Updated fallback documentation to includesys.stdin is Noneas a third triggering casetests/copilot_usage/test_cli.py— Added:test_none_stdin_raises_value_errorinTestReadLineNonblockingclass (unit test)test_interactive_loop_none_stdin_falls_back_to_inputin Gap 3 section (integration test)Warning
The following domains were blocked by the firewall during workflow execution:
astral.shpypi.orgreleaseassets.githubusercontent.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.