Skip to content

Commit 9d55545

Browse files
committed
fix(subprocess_adapter): set the stderr-truncation flag on the crossing chunk (#955)
The GLM precision review caught a boundary hole in 86d059f, the commit that added the marker. Reproduced before fixing. `retained` counts the whole chunk while only `chunk[:room]` is appended, so the chunk that crosses the cap takes the retaining branch and leaves the flag alone -- it was only ever set by a *later* read. When the crossing chunk is the last one before EOF (total stderr between the cap and one 64 KiB read above it), the loop exits with the flag still false and the dropped tail reads as a complete error message. That is precisely the case the marker was added for, so the fix was defeated in exactly its own boundary window. The existing test writes 200 KB against a 1000-char cap, so several non-empty chunks always followed the crossing one and the path never ran. The new test writes 1500 chars, which arrive in a single read. Refs #955
1 parent 7f1af60 commit 9d55545

2 files changed

Lines changed: 35 additions & 4 deletions

File tree

codeframe/core/adapters/subprocess_adapter.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,19 @@ def _drain_stderr() -> None:
222222
chunk = process.stderr.read(65536)
223223
if not chunk:
224224
break
225+
# Say so, like stdout does. Silently capped stderr reads as a
226+
# complete error message, so whoever is debugging a failed
227+
# run trusts a truncated one. The flag has to be set by the
228+
# chunk that *crosses* the cap, not only by a later one:
229+
# when the crossing chunk is the last before EOF the loop
230+
# ends and there is no later one. (#955 review)
225231
if retained < MAX_RETAINED_STDERR_CHARS:
226-
stderr_chunks.append(chunk[: MAX_RETAINED_STDERR_CHARS - retained])
232+
room = MAX_RETAINED_STDERR_CHARS - retained
233+
stderr_chunks.append(chunk[:room])
227234
retained += len(chunk)
235+
if len(chunk) > room:
236+
stderr_truncated = True
228237
else:
229-
# Say so, like stdout does. Silently capped stderr reads
230-
# as a complete error message, so whoever is debugging a
231-
# failed run trusts a truncated one. (#955 review)
232238
stderr_truncated = True
233239

234240
stderr_thread = threading.Thread(target=_drain_stderr, daemon=True)

tests/core/adapters/test_subprocess_bounds_955.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,28 @@ def test_uncapped_stderr_carries_no_marker(self, tmp_path, monkeypatch):
173173

174174
assert "truncated" not in (result.error or "")
175175
assert "short failure" in (result.error or "")
176+
177+
def test_the_marker_survives_a_cap_crossed_on_the_final_chunk(
178+
self, tmp_path, monkeypatch
179+
):
180+
"""The disclosure must not depend on more output arriving after the cap.
181+
182+
`retained` counts the *whole* chunk while only the part that fits is
183+
appended, so the chunk that crosses the cap takes the retaining branch
184+
and never sets the flag — only a *later* read does. When the crossing
185+
chunk is the last one (total stderr between the cap and one 64 KiB read
186+
above it), the loop exits with the flag still false and the dropped tail
187+
reads as a complete error. Exactly the case the marker exists for.
188+
(#955 review)
189+
"""
190+
monkeypatch.setattr(sa, "MAX_RETAINED_STDERR_CHARS", 1000)
191+
# 1500 chars arrive in a single read, so nothing follows the crossing.
192+
script = (
193+
"import sys\n"
194+
"sys.stderr.write('E' * 1500); sys.stderr.flush()\n"
195+
"sys.exit(1)\n"
196+
)
197+
198+
result = _py_adapter(script, timeout_s=30).run("t", "go", tmp_path)
199+
200+
assert "[stderr truncated]" in (result.error or "")

0 commit comments

Comments
 (0)