Use noncanonical PTY mode for Daytona ACP

[bingran-you]

Summary

• put Daytona ACP PTY sessions into non-canonical/no-echo mode before handing off to the agent
• prevents long ACP JSON-RPC messages from hitting the terminal line-discipline limit and corrupting prompts around 4096 bytes
• adds a regression assertion for the PTY bootstrap command

Evidence

• Local py_compile: python -m py_compile src/benchflow/sandbox/process.py tests/test_process.py
• Local in-venv regression: instantiated DaytonaPtyProcess with a fake PTY and verified the startup command includes stty -echo -icanon min 1 time 0 without leaking env secrets
• Real SkillsBench/OpenHands/Daytona repair: previously zero-tool rows ada-bathroom-plan-repair, bike-rebalance, and manufacturing-equipment-maintenance all reached real tool/model trajectories after this patch; no OpenHands JSONDecodeError: Invalid control character at line 1 column 4096 remained

Notes

• Full pytest was not run because this clean CLI venv lacks the test-only acp dependency. The repair run is the spendful end-to-end proof for the affected path.

[bingran-you]

Users Simulation automation review (2026-07-06): blocked because the PR is still draft, though the touched code path validated cleanly.

Evidence:

• Isolated worktree checks passed: py_compile, pytest tests/test_process.py (25 passed), ruff, ty check src/benchflow/sandbox/process.py, bench --help, bench agent list, known-good tasks check, and git diff --check.
• CI integration-light rollout artifact was downloaded and revalidated locally: 1/1 rollout healthy, ACP + LLM trajectories present, 19 LLM exchanges with provider usage, results.jsonl training-ready, reward 1, 291,031 tokens, 18 tool calls, timing total 245.2s.
• Code-quality review found no structural regression in the 2-file PTY patch.

Caveat: the CI GLM user-endpoint artifact has agent_result.cost_usd: null / price_source: null; token usage and timing are healthy, but this does not satisfy the stricter Users Simulation request for non-null USD cost metadata. Keep status:blocked until the PR is marked ready for review and the remaining cost-metadata expectation is addressed or explicitly waived for user-endpoint providers.

[bingran-you]

Users Simulation automation update (2026-07-06): pushed 075adc686 with the PR-convention cleanup for the Daytona PTY regression test.\n\nWhat changed:\n- The PTY env-transport regression test docstring now explicitly names PR #896.\n- The cleanup assertion now matches the current raw-mode implementation: the uploaded /tmp/benchflow_pty_exec_* script is removed, and no provider secret is leaked.\n\nValidation:\n- uv run pytest tests/test_process.py -q -> 25 passed\n- uv run ruff check tests/test_process.py -> passed\n- uv run ruff format --check tests/test_process.py -> passed\n- git diff --check -> passed\n\nKeeping status:blocked because the PR is still draft and the remaining cost-metadata expectation for user-endpoint providers is a policy/review decision outside this PTY patch.

[bingran-you]

Users Simulation automation follow-up (2026-07-06): the branch was force-updated after my earlier 075adc686 push, so I reapplied the same test cleanup on top of the live head and pushed a717acdd8.\n\nValidation on the refreshed head:\n- uv run pytest tests/test_process.py -q -> 25 passed\n- uv run ruff check tests/test_process.py -> passed\n- uv run ruff format --check tests/test_process.py -> passed\n- git diff --check -> passed\n\nKeeping status:blocked because the PR is still draft and the cost-metadata expectation remains a policy/review decision outside this PTY patch.

[bingran-you]

Users Simulation automation note (2026-07-06): the branch was force-updated again after my a717acdd8 push. Current head is 8a082a84, and it no longer contains the test cleanup from a717acdd8.\n\nCurrent-head finding:\n- tests/test_process.py again has the old docstring that does not name PR #896.\n- The same test still expects cleanup of /tmp/benchflow_env_*, while the current PTY implementation removes /tmp/benchflow_pty_exec_*; this was the local failure I fixed before the force-update.\n\nKeeping status:blocked. Because this is an actively force-updated draft branch, I am not repeatedly re-pushing over the author/automation. Reapply the small test cleanup on the stable head before marking ready.

[bingran-you]

Updated the branch after the first canary exposed two more host-shell constraints: no python3 on the host bootstrap path, and no bash for the short PTY handoff. The current patch now uploads the long command via POSIX heredoc and sends only \ through the PTY.\n\nFresh spendful canary evidence: \ through BenchFlow + OpenHands + Daytona + SGLang identity LoRA produced a clean scored row: selected=1, scored=1, issue_row_count=0, n_tool_calls=134. This is the compose-task path that previously failed with PTY command echo / empty ACP trajectory.

[bingran-you]

Correcting the previous comment, which had shell-expanded backticks.

Updated the branch after the first canary exposed two host-shell constraints: no python3 on the host bootstrap path, and no bash for the short PTY handoff. The current patch now uploads the long command via POSIX heredoc and sends only exec sh /tmp/benchflow_pty_exec_*.sh through the PTY.

Fresh spendful canary evidence: drone-planning-control through BenchFlow + OpenHands + Daytona + SGLang identity LoRA produced a clean scored row: selected=1, scored=1, issue_row_count=0, n_tool_calls=134. This is the compose-task path that previously failed with PTY command echo / empty ACP trajectory.

[bingran-you]

Users Simulation blocker follow-up (2026-07-07): pushed f7d5c6733 to address the code-side blocker from the earlier force-update note.

What changed:

• Reapplied the regression-test convention cleanup on the current PR Use noncanonical PTY mode for Daytona ACP #896 head: test_provider_env_value_not_sent_through_pty_input now explicitly names PR Use noncanonical PTY mode for Daytona ACP #896 in its docstring.
• No production PTY behavior changed; this is a one-line test/docstring fix.

Validation:

• uv run pytest tests/test_process.py -q -> 25 passed
• uv run ruff check tests/test_process.py -> passed
• uv run ruff format --check tests/test_process.py -> passed
• uv run ty check src/benchflow/sandbox/process.py -> passed
• git diff --check -> passed

Labels unchanged: keeping status:blocked because the PR is still draft and the remaining Users Simulation blocker is the non-null USD cost-metadata expectation for user-endpoint providers, which is a review/policy decision outside this PTY code/config patch.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.