fix(acp): preserve usage and terminal evidence on timeout

[kywch]

Summary

Fixes #933.

ACP wall/idle deadlines previously cancelled the local ACPClient.prompt() task without invoking the existing ACPClient.cancel() method. That prompt task owns the sole ACP response reader, so BenchFlow discarded available cancellation-time evidence:

• final PromptResponse.usage;
• terminal session/update tool events;
• final cancellation stop reason.

Timed-out native ACP runs could therefore report total_tokens=0, usage_source=unavailable, and leave tool calls pending.

Fix

• Send one ACP session/cancel attempt before hard local cancellation.
• Keep the original prompt task as the sole transport reader.
• Use one 5-second total cleanup budget:
  • up to 4.75 seconds for peer updates and the original prompt response;
  • remaining time, capped at 0.25 seconds, to concurrently cancel and boundedly drain the prompt and cancellation tasks.
• Apply the same behavior to wall-clock and idle deadlines.
• Collect native ACP usage before disconnect() clears the live session.

The deadline remains a timeout even when the peer cooperates. BenchFlow preserves genuine peer evidence; it does not convert cancellation into success or synthesize terminal state or token counts.

Prior work

This change is independently implemented from current main.

• fix(acp): preserve streamed usage on timeout #934 adds parsing for hypothetical/provider-style, Usage-shaped session extensions, but does not send session/cancel and remains unmerged. Standard ACP usage_update reports context occupancy rather than cumulative consumed-token totals.
• Preserve native ACP artifacts at prompt timeout #1051 demonstrated cancel-first handling, but mixed it with sandbox, verifier, and artifact-policy changes. Review found a 60-second noncooperative grace period plus unrelated idle, sandbox_user=None, and verifier regressions; it was closed unmerged.

This PR keeps only the narrow ACP runtime and usage-lifecycle fix. It does not change verifier behavior, sandbox hardening, publication policy, diagnostics schemas, provider routing, dependencies, or streamed-usage parsing.

Regression coverage

Tests use protocol-shaped JSON-RPC through the real ACPClient reader and cover:

• cooperative wall and idle cancellation;
• final native usage and peer-authored terminal tool updates;
• timeout verdict preservation;
• prompt completion while the cancel sender stalls;
• simultaneous pre-delivery cancel stall and noncooperative prompt;
• bounded return when tasks resist cancellation—the test explicitly releases and gathers the surviving prompt task;
• final usage promotion before session teardown.

Validation

uv run python -m pytest -q \
  tests/test_acp.py \
  tests/test_native_acp_usage.py \
  tests/test_trajectory_streaming.py \
  tests/test_session_factory_runtime.py
136 passed

uv run ruff check \
  src/benchflow/acp/runtime.py \
  src/benchflow/rollout/__init__.py \
  tests/test_acp.py \
  tests/test_native_acp_usage.py
All checks passed

uv run ty check \
  src/benchflow/acp/runtime.py \
  src/benchflow/rollout/__init__.py
All checks passed

git diff --check
clean

No live provider or sandbox rollout was run.

Limitation

For cooperative agents returning PromptResponse.usage, this timeout accounting gap is fully fixed.

Agents that omit PromptResponse.usage may still report total_tokens=0 with usage_source=unavailable. This is intentional and honest: BenchFlow has no reliable native token total to preserve and does not invent one.

Provider-routed runs may still obtain usage through separate LiteLLM telemetry. Native subscription agents that omit final usage remain affected.

This is not a merge blocker. A follow-up should add another evidence source only if real traces show important supported agents routinely omit PromptResponse.usage after cancellation.

---

[kywch]

Thanks @Galius5136 — your A/B review on #1051 directly shaped this narrower implementation.

This PR keeps the useful cancel-first ACP behavior while addressing the regressions you identified:

• preserves existing sandbox_user=None verifier/reward/trajectory behavior;
• replaces the 60-second grace period with one 5-second total cleanup budget;
• applies the same cancellation/evidence path to wall and idle deadlines;
• excludes the global sandbox-process hard gate and synthetic terminalization policy.

The result is limited to preserving genuine ACP cancellation evidence and usage without changing verifier or sandbox behavior.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[bingran-you]

Thermo-nuclear review at exact head 89f1d43d3b2cae1a946ee4372ed7d491639b24f5: requesting changes.

The cooperative timeout path works in the normal case: I ran a real Docker claude-agent-acp timeout and confirmed that native ACP usage plus the honest partial trajectory were preserved. The full suite also passes (5896 passed, 58 skipped, 7 deselected). However, there is still a terminal-evidence loss path in _cancel_prompt_after_timeout().

The function waits for FIRST_COMPLETED across the original prompt task and a supervisor that both sends cancel() and waits for the prompt. If the cancel notification raises immediately (for example, the write side closes) while the peer can still return terminal usage inside the grace period, the supervisor completes first. Control then leaves the cooperative wait and hard-cancels the original prompt, losing the evidence this PR is meant to preserve.

A direct probe reproduced this with a 0.5 s cleanup budget: cancel() raised immediately, the prompt would have returned after 0.2 s, but the prompt task was locally cancelled before the peer completed.

There is a simpler ownership model here:

1. Send cancellation in an independent best-effort task.
2. Let only the original prompt task determine whether terminal evidence arrived during the grace period.
3. Log/suppress a cancel-send failure without shortening that grace period.
4. At the shared deadline, hard-cancel/drain whichever tasks remain.

Please add a regression where cancel() raises but the prompt returns usage/terminal evidence within the grace period. This also avoids adding another coupled supervisor state to an already busy runtime. As currently queued with #1076, runtime.py reaches 1,001 lines, so I would use this fix as the point to extract timeout cleanup into a focused module rather than growing the orchestration file further.

[kywch]

Addressed in ee209502.

cancel() failures no longer shorten prompt grace. Cancel runs independently; failures are logged; prompt completion alone controls evidence collection; both tasks drain at shared deadline.

Regression covers immediate cancel failure + delayed terminal usage. Old code fails test.

Validation: affected suite 137 passed; Ruff/type/diff checks pass. Full suite: 5896 passed, 2 baseline credential-isolation failures.

[kywch]

Thank you, @bingran-you! Addressed your concerns, but deferred runtime.py extraction.

[bingran-you]

Thermo-nuclear exact-head follow-up at 416b3ea22ea5038c74ef5ca4600f809a5b528737: the timeout contract is ready; independent human review is the remaining merge gate.

The cancellation race is closed and the lifecycle is now decomposed into acp/timeout_cleanup.py: cancel sending is independent, only the original prompt controls whether terminal evidence arrived, both tasks share a bounded cleanup deadline, and remaining work is hard-cancelled and drained without inventing terminal tool evidence. Dedicated regressions cover immediate cancel failure plus delayed terminal usage.

A fresh real native-Claude OAuth Docker run at this exact head executed sleep 120 and hit the intended 60-second wall timeout. It preserved one live tool call, the pending tool ID, native ACP usage (3 input, 182 output, 13,340 cache-read, 6,404 cache-creation tokens), and an honest terminal_trajectory_complete=false; the timeout event was recorded and the verifier completed normally with reward 0 and no verifier error. Validation also passed: full suite 5908 passed, 86 skipped, 7 deselected, Ruff, ty, diff checks, and all current CI checks.

I wrote the extraction/final repair during this review, so this still requires an independent maintainer review of the exact SHA before squash merge under the repository policy.