fix(file-ops): shell-layer non-regular-file guard for all read paths (salvage #82075) - #83008
Merged
Merged
Conversation
The size probe every read path starts with — `wc -c < path` — opens the path. On a FIFO with no writer, a socket, or a character device that never reaches EOF, that read never returns, and read_file/read_file_raw/ read_file_bytes all pass no timeout to _exec. The turn wedges until the process is killed. The device blocklist in tools/file_tools.py cannot close this: it matches literal /dev/* names, so it can only ever cover paths someone thought to enumerate. A FIFO is a file type and can sit at any path. Gate the probe behind `[ -f ]`, which stats instead of opening, and report a path that exists but is not a regular file as such. A missing path keeps its existing not-found handling.
The combined [ -f ]/wc -c probe changes the first shell command each read issues; update the stale mocks that only answered bare 'wc -c'. The fifo tool-layer test now accepts the merged stat-guard's success=False note (a fact, not an error) with the shell sentinel behind it.
Contributor
૮ >ﻌ< ა ci reviewrunning on 746d409 — test: adapt edge-case pagination mock to the sentinel probe waiting for jobs to start… |
Same stale-mock class as the previous commit — the sweep missed test_file_operations_edge_cases.py. Verified no bare wc -c mocks remain anywhere under tests/.
alt-glitch
added
type/bug
This was referenced Aug 10, 2026
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
3 participants
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.
Summary
Salvage of #82075 by @Drexuxux: the shell-layer complement to the merged stat-based guard (0e63ed1 / #82792).
read_file,read_file_raw, andread_file_bytesinShellFileOperationsall open the path viawc -cwith no deadline — on a FIFO/socket/device that never reaches EOF, remote backends (docker/ssh/modal) still wedge, because the merged guard is host-only (_file_ops_uses_host_pathsgate) and doesn't cover the raw/bytes paths.Drexuxux's mechanism folds a
[ -f ]check into the existing size probe (sentinel echo in the same shell invocation), so all three read paths on every backend refuse non-regular files with zero added round-trips. This beats the earlier #29428 approach (separate preflight exec = +1 round-trip per read); @haileymarshall was the earliest submitter on this bug class (May 20) — credit to both.Eval check (evals/readtool fifo battery, 3 reps, this branch vs main): no per-read regression on regular-file tasks; fifo task unchanged vs merged guard (tool layer still intercepts first on host; the sentinel is the remote/raw safety net behind it).
Changes
tools/file_operations.py(authored by @Drexuxux, cherry-picked): combined[ -f ]/wc -cprobe +NOT_REGULAR_SENTINELhandling inread_file,read_file_raw,read_file_bytes;_not_regular_error()helper.tests/tools/test_file_operations.py,tests/tools/test_file_read_guards.py: contributor's regression tests + follow-up commit adapting 5 stalewc -cmocks and reconciling the fifo tool-layer test with the merged note-based guard (tool layer notes, shell layer errors — either surface is a pass).Validation
read_file0.03s,read_file_raw0.01s,read_file_bytes0.01s — all refused instantlyrun_tests.sh)Closes #82075 (salvaged, authorship preserved). Supersedes #29428 (same class, earlier submission, costlier mechanism — close with credit).
Infographic