Skip to content

fix(buzz-acp): treat settled turns as activity for the idle-pool clock - #6519

Open
sanjay3290 wants to merge 2 commits into
block:mainfrom
sanjay3290:spark5/idle-pool-completion-clock-v2
Open

fix(buzz-acp): treat settled turns as activity for the idle-pool clock#6519
sanjay3290 wants to merge 2 commits into
block:mainfrom
sanjay3290:spark5/idle-pool-completion-clock-v2

Conversation

@sanjay3290

@sanjay3290 sanjay3290 commented Aug 21, 2026

Copy link
Copy Markdown

Problem

buzz-acp's idle-pool reaper anchors on last_activity, which is only advanced when a prompt is dispatched. It is never advanced when a turn settles.

So a turn that runs longer than the idle bound is torn down on the first reaper tick after it completes — the pool did useful work for the entire window, and the clock still reads as if it had been idle the whole time. The longer a turn runs, the more certain the teardown.

Fixes #6378.

Change

Adds a note_turn_settled seam and calls it unconditionally from both settle paths in the event loop — PoolEvent::Result (lib.rs:3177) and PoolEvent::Panic (lib.rs:3220) — before their trailing dispatch_pending.

The existing dispatch_pending call already bumps last_activity, but only when it actually dispatches something. In the exact case this bug is about — a long turn finishing with an empty queue — it dispatches nothing and the clock stays stale. Hence a separate bump at settle time.

Two commits, test first:

  • 0956a58f5 adds the seam and pins the contract. The re-anchor test fails at this commit.
  • a09fd9fe5 supplies the bump.

Verification

All commands run on a clean tree at a09fd9fe5 (git status --short empty, confirmed before and after each run).

cargo fmt --all -- --check                              exit 0
cargo clippy --workspace --all-targets -- -D warnings   exit 0, 0 diagnostics
cargo test -p buzz-acp                                  lib 806 passed / 0 failed
                                                        pool_lifecycle_state 9 passed / 0 failed

buzz-acp is not one of the nine packages in scripts/run-tests.sh unit, so the package test above is run separately and is the gate that actually executes these tests.

Behavioural probe. Checked out the test-only commit 0956a58f5 in a separate detached worktree — the seam and both call sites are present there, only the bump is absent, so the crate still compiles and the wiring is unchanged:

failures:
    idle_pool_sleep_tests::completed_long_turn_reanchors_clock_not_sleep_due

test result: FAILED. 805 passed; 1 failed

Exactly the decision test, and nothing else. Removing the behaviour while keeping the wiring fails the test.

Known limitation

The test is decision-level: it calls note_turn_settled and then asserts idle_pool_sleep_due. It does not drive the real tokio::select! loop, so it cannot catch someone deleting one of the two arm call sites individually — the unit tests cannot reach inside those arms without a refactor well beyond this fix.

What it does cover: both arms call the same single-statement helper, so removing the shared bump disables both at once, which is what the probe above exercises. Stating the gap rather than claiming coverage the test does not have.

Test coverage limitation — stated deliberately

The two new tests call note_turn_settled directly and then assert idle_pool_sleep_due. They pin the
decision rule, not the wiring: tokio_main's event loop is not reachable from a unit test, so deleting
either call site leaves the suite green. The bump lives in one shared one-line function called
unconditionally by both settle arms, so the two arms cannot diverge, but reviewers should read the two
call sites rather than trust the tests to cover them. Flagging this rather than implying coverage the
tests do not provide.

Add a note_turn_settled seam called unconditionally from the
PoolEvent::Result and PoolEvent::Panic arms, and pin the contract in
tests: a turn that settles after 2x the idle bound must re-anchor the
clock so the pool is not immediately sleep-due, while an idle pool
with no settled turn is still torn down on schedule. The reanchor test
fails at this commit; the next commit supplies the bump.

Signed-off-by: Sanjay Ramadugu <sramadugu1@gmail.com>
note_turn_settled now advances last_activity, so the idle-pool reaper
anchors on turn completion and panic recovery instead of dispatch time
only. A long-running turn that finishes inside the idle bound is no
longer torn down on the first reaper tick after it completes.

Signed-off-by: Sanjay Ramadugu <sramadugu1@gmail.com>
@sanjay3290
sanjay3290 requested a review from a team as a code owner August 21, 2026 21: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.

buzz-acp: idle pool teardown kills an in-flight session waiting on background subagent notifications (queued wakeup lost, no stop bookkeeping)

1 participant