Skip to content

fix(acp): retain live subprocess stderr - #980

Merged
xdotli merged 2 commits into
mainfrom
fix/976-acp-stderr
Aug 13, 2026
Merged

fix(acp): retain live subprocess stderr#980
xdotli merged 2 commits into
mainfrom
fix/976-acp-stderr

Conversation

@kywch

@kywch kywch commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Captures bounded stderr from subprocess-backed ACP agents throughout their lifetime and persists a redacted tail when transport closes.

Root cause

BenchFlow read stderr only after ACP stdout reached EOF. A protocol-valid empty end_turn can leave stdout open, so diagnostics written to stderr were discarded at normal teardown.

Safety

  • 64 KiB stderr tail cap
  • redact before rollout artifact and typed transport diagnostic persistence
  • close waits at most two seconds for an inherited stderr FD, then cancels the drain

Validation

  • uv run python -m pytest tests/test_acp.py tests/test_process.py
  • uv run ruff format --check src tests
  • uv run ruff check src tests
  • git diff --check

Fixes #976.

@kywch
kywch deployed to pypi-internal-preview August 12, 2026 18:22 — with GitHub Actions Active
@kywch
kywch force-pushed the fix/976-acp-stderr branch from 25bb72c to 05242f7 Compare August 12, 2026 18:22
@kywch
kywch deployed to pypi-internal-preview August 12, 2026 18:22 — with GitHub Actions Active
@kywch
kywch deployed to pypi-internal-preview August 12, 2026 18:26 — with GitHub Actions Active
@kywch
kywch marked this pull request as ready for review August 12, 2026 18:31
@kywch
kywch deployed to pypi-internal-preview August 12, 2026 18:31 — with GitHub Actions Active
@Galius5136

Galius5136 commented Aug 12, 2026

Copy link
Copy Markdown

I reproduced the stderr-loss path from #976 locally on WSL using the deterministic fake-ACP regression described in the issue, and compared the merge-base (29c64e7) with the PR head (d769f34). I drove ContainerTransport directly (both with a local subprocess and the real DockerProcess) rather than running a full bench eval run.

On the base, the ACP response completes normally with end_turn, but stderr is not retained and the agent log is not created.

With #980, the ACP response stays unchanged, while the stderr diagnostic is preserved in the agent log. I also verified the same behavior through the real DockerProcess path.

I checked the bounded stderr tail as well: it stays at 65536 bytes, drops the head, and retains the tail. Secret redaction also worked as expected: the diagnostic sentinel was present, REDACTED was present, and the original AIzaSy... token prefix was absent.

Everything behaved as expected on my setup, and I didn't observe any protocol regression or shutdown hang.

@xdotli
xdotli merged commit d30527b into main Aug 13, 2026
15 checks passed
Galius5136 added a commit to Galius5136/benchflow that referenced this pull request Aug 14, 2026
Picks up d30527b (fix(acp): retain live subprocess stderr, benchflow-ai#980). No conflicts:
that commit touches acp/container_transport.py, sandbox/process/* and their
tests, none of which Slice A or the trajectories export path touches.
@bingran-you
bingran-you deleted the fix/976-acp-stderr branch August 15, 2026 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACP subprocess stderr is lost when an agent returns an empty end_turn

3 participants