fix(acp): cap the per-channel cancelled-batch side-map#3043
Open
shani-singh1 wants to merge 1 commit into
Open
fix(acp): cap the per-channel cancelled-batch side-map#3043shani-singh1 wants to merge 1 commit into
shani-singh1 wants to merge 1 commit into
Conversation
`requeue_as_cancelled` appended each cancelled turn's events to `cancelled_batches[channel]` with no bound, while the main queue right beside it is capped at `MAX_PENDING_PER_CHANNEL` in every other requeue/push path. That side-map is only cleared when a turn *completes* (or the channel drains). Under a sustained mid-turn interrupt/steer flood — every turn cancelled before it finishes — each cancel carries the accumulated cancelled events forward, so the map grows one turn's worth of events per cancel. `format_prompt` then renders every accumulated event (full content + tags) into the prompt on each subsequent turn, so both harness memory and the LLM prompt grow without bound: eventual OOM of the agent process plus per-turn token-cost amplification. With `respond_to = anyone` on a public channel this is triggerable by any participant; with `allowlist` it needs one misbehaving allowed key. Enforce the same cap the main queue uses, trimming oldest so the most-recent "what you were working on" framing survives. New regression test drives the cancel cycle past the cap and asserts it holds and keeps the newest event; it fails without the fix. Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3042.
Problem
The ACP harness's per-channel
cancelled_batchesside-map (crates/buzz-acp/src/queue.rs) is never size-capped, while the main event queue beside it is capped atMAX_PENDING_PER_CHANNEL(500) in every growth path (push,requeue,requeue_preserve_timestamps, …).requeue_as_cancelledjustextends. That map is only cleared when a turn completes (or the channel drains), so under a sustained mid-turn interrupt/steer flood — every turn cancelled before it finishes — each cancel carries the accumulated cancelled events forward and the map grows one turn's worth of events per cancel.format_promptthen renders every accumulated event (full content + tags) into the prompt on each subsequent turn, so both harness memory and the LLM prompt grow without bound: eventual OOM of the agent process plus per-turn token-cost amplification.Reachable by any channel participant under
respond_to = anyone, or by one misbehaving key underallowlist. Full analysis and repro in #3042.Fix
Enforce the same cap the main queue already uses, in
requeue_as_cancelled, trimming the oldest events so the most-recent "what you were working on" framing survives:Same constant, same drop-oldest-with-warn shape as the existing queue overflow handling — this is the consistency fix that closes the one growth path that was missed. A single
drainkeeps it O(n).Tests
New
test_sustained_cancellation_caps_cancelled_batchesdrives the real cancel cycle (push → flush_next → requeue_as_cancelled → mark_complete)MAX_PENDING_PER_CHANNEL + 50times and asserts the map stays<= MAX_PENDING_PER_CHANNELand still retains the newest event. It fails without the fix (550 > 500) — verified by reverting the cap and re-running.Validation
Built on Windows with the
x86_64-pc-windows-gnutoolchain.cargo test -p buzz-acp --lib— all 20 cancel-path tests pass, including the new one, and the existingtest_double_cancel_preserves_all_events/test_drain_channel_clears_cancelled_batchesstill pass (behaviour under the cap is unchanged).cargo fmt -p buzz-acp -- --check— clean.acp.rs(system-prompt / idle / keepalive / steering) fail on my machine, but they fail identically on cleanorigin/mainwith this branch stashed — they are pre-existing, timing-sensitive failures on this toolchain, in a different module, unaffected by this one-function change inqueue.rs.