Skip to content

fix(acp): reset the pending grace on in-progress tool-call updates; truthful expiry diagnostics - #1

Merged
Benjamin-eecs merged 1 commit into
Benjamin-eecs:fix-idle-watchdog-pending-gracefrom
JeremyJC67:fix/idle-grace-progress
Aug 30, 2026
Merged

fix(acp): reset the pending grace on in-progress tool-call updates; truthful expiry diagnostics#1
Benjamin-eecs merged 1 commit into
Benjamin-eecs:fix-idle-watchdog-pending-gracefrom
JeremyJC67:fix/idle-grace-progress

Conversation

@JeremyJC67

Copy link
Copy Markdown

Hey Benjamin — following up on the review notes on benchflow-ai#1066: this keeps your bounded-grace design and the 3x constant exactly as-is, and only makes the grace clock restart when a pending call streams in-progress tool_call_updates (those mutate ToolCallRecord in place, so they were invisible to both the pending-set snapshot and _activity_count, and a single long call that kept reporting progress past the grace was killed as idle). The watchdog now snapshots a new monotonic ACPSession.tool_call_update_count alongside your pending-set tuple — a call that keeps talking defers as long as it talks, while one that stays silent for the full grace still trips, so your lost-completion test passes unmodified. On grace expiry the message and IdleTimeoutDiagnostic now say what actually happened (which calls exceeded the grace, when the pending set last changed, when the last update arrived, how many updates were seen) via additive fields, keeping the diagnostic shape and classify_error behavior backward compatible.

Red→green: the two new tests fail on your branch as-is (the streaming-call false kill, and the untruthful expiry message) and pass with this patch; your own tests are untouched and green (tests/test_acp.py 99 passed). ruff / format / ty clean.

Merging this into your branch makes it part of benchflow-ai#1066 — and feel free to close it and implement your own version instead; the review stands either way.

…ruthful expiry diagnostics

A single long tool call that streamed in-progress tool_call_update
notifications past the grace boundary was killed as idle: those updates
mutate ToolCallRecord in place, invisible to both the pending-set snapshot
and _activity_count. The watchdog now observes a monotonic
ACPSession.tool_call_update_count each poll, and any change restarts the
pending grace clock alongside pending-set changes — a call that keeps
talking defers as long as it talks, while one silent for the full grace
still trips. On grace expiry, the raised message and IdleTimeoutDiagnostic
now report the truth via additive fields: which pending calls exceeded the
grace, when the pending set last changed, when the last update was
observed, and how many updates were seen.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZggU3vNYzr3hfDZJUE7jX
@Benjamin-eecs
Benjamin-eecs merged commit fc49b78 into Benjamin-eecs:fix-idle-watchdog-pending-grace Aug 30, 2026
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.

2 participants