Repository navigation
Replies: 1 comment
|
Both write sites are exactly where you say, and the failure is real. Confirmed on
Neither reads the return value, and nothing else in Two details that make the impact a bit worse than "the file is short":
Your four-byte arithmetic checks out, by the way. With On the fix shape, two things I would add to what you propose:
One framing note, since you were careful about it in your report and it is worth keeping: this is not an ordinary-write complaint. |
Uh oh!
There was an error while loading. Please reload this page.
I reproduced an output-integrity issue on current master at
687f282e8625c6b8d11ff58211a90524be2e4317(0.2.1-rc.2): a subprocess spill file can silently lose bytes after a successful partialfs.writeSync()call, while still being advertised as full output.Trigger and impact
When a subprocess stream exceeds its in-memory limit,
OutputCollector.spillAll()replays buffered chunks and appends the current chunk to a spill file. Both write sites ignore the byte count returned bywriteSync(). If that count is positive but smaller than the buffer length, the remaining bytes are never written. No exception reaches the existing failure handler, so the incomplete file keeps its advertised spill path.The bounded memory tail can remain correct while the file intended to recover full command output is incomplete. Users or agents consulting that file can therefore receive silently truncated content. This concerns exceptional storage conditions; it is not a claim that ordinary file writes routinely return short counts.
Reproduction
The regression uses the real collector and real temporary files. It intercepts only writes to the collector's own descriptor, makes selected calls actually write and return one byte, and delegates other I/O to Node. With a four-byte memory limit, pushing
AAAA,BBBB, andCCCCshould produceAAAABBBBCCCCon disk.ABBBBCCCCAAAABCCCCAAAABBBBCABCThe original implementation exposes these incomplete files without invoking its failure reporter. A final short write also returns without attempting the remainder, so a subsequent storage error is not observed during that append. Five of the initial seven regression cases fail against the original implementation.
The final regression file contains ten tests covering ordinary writes, UTF-8 bytes, all three write positions, repeated partial progress, and
ENOSPCafter partial progress. The injection intercepts nativefs.writeSync, including calls made inside Node'swriteFileSync, rather than mocking completion away.Minimal patch
Use
writeFileSync(fd, Buffer)at both append sites. Node completes partial writes on the existing descriptor without reopening, truncating, or closing it. If completion throws, the existing handler discards the incomplete spill, withdraws its path, reports once, and continues collecting the bounded memory tail.The existing exception-injection test is updated to intercept the new write operation. Public API, dependencies, configuration, output format, caps, and file lifetime are unchanged.
The branch also contains three independent validation fixes, each in its own commit: honor the CLI help concurrency override, avoid treating a uv artifact byte count as a commit reference, and use a directory junction for the Windows image-containment fixture. The core commit can be taken independently.
Validation
Node
24.19.0, pnpm11.28.5; Windows plus Ubuntu on a real WSL2 Linux kernel and ext4 filesystem.output.tshas 100% statements, branches, functions, and lines under the normal repository configuration.bash-spillrecorded-session replay passes on Linux through the shipped headless profile using built artifacts. No expected snapshots were changed.Commands used for the Linux owning tests and repository checks:
Related reports and limits
Before publishing, I searched public Discussions, issues/PRs, and commits for short writes,
writeSync, andOutputCollector; I did not locate an equivalent short-return-value report. #260 and #5175 concern thrown spill I/O failures, while #7854 concerns the spill size cap. This report addresses a separate remaining failure mode in the same subsystem.Fault injection establishes the handling defect and regression behavior. It does not measure real disk-full/quota frequency or prove crash/power-loss durability. Not every shell/backend combination has been exercised, and macOS was not tested dynamically. Node
22.19.0's built-in implementation was checked at source level; dynamic verification used Node24.19.0.AI-assisted audit, reproduction, implementation, and validation. The commits include a Codex co-author.
All reactions