fix(buzz-acp): bound busy-owner hold to prevent cross-channel starvation

[wpfleger96]

Problem

#6732 added a busy-owner hold to the ACP harness: when a scope's recorded session owner (session_owners) is checked out on any turn, dispatch_pending holds the scope's batch instead of dispatching it. The hold was added to keep one provider session per thread — but it is unconditional: it applies to Conversation scopes too, and it has no time bound.

Under the default session_policy=channel, every channel collapses to a single Conversation scope, so once two channels' sessions land on the same worker (pass 2 of try_claim picks the first idle worker by index, so this happens quickly after any restart), channel A's mention starves behind channel B's in-flight turn — for up to the full max_turn_duration (7200s by default) — while other workers sit idle. The only signal is a DEBUG-level log, and the 👀 seen-reaction is added at queue admission before the hold decision, so the user sees the agent acknowledge the mention and then nothing.

Observed in production on the first day of the v0.5.22 rollout: three separate incidents where a mention got 👀 but no turn started until an unrelated channel's turn ended on the shared worker (in the worst case the blocking turn sat in a single tool call for 6+ minutes).

Fix

One new seam, AgentPool::hold_decision, replaces the raw should_hold_for_busy_owner check in dispatch_pending (the predicate itself is unchanged and remains the inner check):

• Conversation scopes never hold. Channel-policy channels and all DMs dispatch immediately; a busy owner means forking onto an idle worker, exactly the pre-feat(buzz-acp): give each channel thread its own agent session #6732 behavior. This removes the cross-channel head-of-line blocking entirely for the default policy.
• Thread scopes hold for a bounded window. HOLD_BUSY_OWNER_TIMEOUT (10s) is measured from the first time the batch is held (held_since stamp); once elapsed, the batch stops holding and forks a fresh session on an idle worker, rebuilding thread context from the relay. This preserves feat(buzz-acp): give each channel thread its own agent session #6732's session-continuity intent for the momentary-busy case while capping the worst-case wait. No new timer is needed: held batches are requeued with preserved timestamps and re-evaluated on every dispatch trigger (turn end, relay event, 30s maintenance tick), so the effective worst-case re-check gap on a fully silent system is one maintenance tick.
• Holds are observable. Holding logs at INFO and a hold expiry logs at WARN (previously DEBUG-only), and both emit observer-feed events (busy_owner_hold, busy_owner_hold_forked) with the scope, owner index, and held duration.

held_since is derived state and is cleared on every removal path: dispatch/fork (inside hold_decision), invalidate_channel_sessions, invalidate_scope_session, and switch_idle_agent_model.

Accepted trade-offs

• A fork after an expired hold leaves the old owner's now-orphaned thread session in its session map until natural rotation/invalidation — benign, and identical to pre-feat(buzz-acp): give each channel thread its own agent session #6732 fork semantics (loadSession: false; sessions are worker-pinned, so migration is not an option).
• Under sustained pool exhaustion the hold stamp is cleared on the fork attempt and re-stamped next cycle, so the bound is effectively "timeout after a worker frees up," not absolute wall clock.

Tests

• New table test hold_decision_covers_variant_session_busy_and_timeout over the full input space (scope variant × idle-session presence × owner busyness × elapsed vs. window). The Conversation + busy-owner row is the cross-channel regression guard; the past-window row guards the bound. Both were mutation-checked: removing the variant gate or the timeout branch fails the suite.
• busy_session_owner_holds_batch_instead_of_forking_session extended with the Hold → ForkAfterHold transition, the Conversation dispatch guard, and held_since pruning on channel invalidation.
• Scope-invalidation and idle-model-switch tests extended to cover held_since cleanup alongside the existing session_owners assertions.

[github-actions]

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

• Exact PR diff: d595806fc3b9c9758992e39b9b51cbb5f55791b0...65f73c7bd4ea9e38dc78331df717d9a66fc5e872
• Model: gpt-5.6-sol

💡 Click "edited" above to see earlier reviews for this PR.

---

Review Summary

Overall Risk: MEDIUM

The new bounded busy-owner hold is not actually bounded to 10 seconds, so queued thread work can still starve despite idle capacity.

Findings

[MEDIUM] Busy-owner timeout is not scheduled and can last for a full turn

• Category: Reliability
• Location: crates/buzz-acp/src/lib.rs:4397 (source)
• Description: hold_decision is evaluated only when dispatch_pending happens. Recording held_since does not install a wakeup for the 10-second deadline. On a quiet relay, the next dispatch may wait for the 30-second maintenance check; when typing, presence, heartbeat, and inactivity timers are disabled, it can wait until the owning turn completes, whose default hard limit is two hours. Additionally, ForkAfterHold clears held_since before try_claim succeeds, so if every worker is busy at expiry, the next worker completion starts a fresh hold window.
• Impact: A long-running request can continue starving another thread even when a non-owning worker later becomes available, defeating the availability fix this pull request intends.
• Recommendation: Add a main-loop timer for the earliest hold deadline and trigger dispatch when it expires. Preserve the expired state until a worker is successfully claimed or the original owner becomes reusable, rather than clearing it before try_claim.

Notes

• No additional limitations were reported.

---

_{Generated by Codex Security Review |
Requested by: @wpfleger96 |
Workflow run}

[salman1993]

🤖

1. P2 — The timeout has no independent wakeup. The “30s maintenance tick” is an elapsed-time check at the top of the loop, not a timer. With typing, presence, heartbeat and reaper timers disabled, a held thread can still wait for the unrelated busy turn to finish despite an idle worker. Add a scheduled deadline and a quiet-loop regression. Sources: pool.rs:873–897; lib.rs:2880–2883, 3018–3061, 3122–3710 under crates/buzz-acp/src/.

2. P2 — A fork can later resume the old session. Worker 0 owns A, runs B; A forks to worker 1; both return. The next A claim picks worker 0’s stale session by slot order, despite the directory naming worker 1. The old copy is not actually orphaned. Fence stale session state and test fork → both return → next claim. Sources: lib.rs:4425–4450, 4528–4530; pool.rs:909–955. Independently found by Maldini and checked against the source.

[salman1993]

🤖 These are P2 correctness issues, not critical stop-ship failures. My “request changes” was too broad for an urgent restoration of widely used channel sessions.

• Missing timeout wakeup: affects the new thread-scope timeout, not the channel-scope bypass. It should not block a channel-only hotfix.
• Stale session after a fork: more consequential because a later turn can resume an older provider context. But for channel scopes, this restores the pre-#6732 behavior rather than introducing a wholly new risk. Extending that behavior to thread scopes is the additional risk here.

My recommendation: ship the channel-scope bypass now, keep the existing thread hold, and fix bounded thread dispatch separately. That restores the broken default without coupling it to the unfinished thread-timeout behavior. Require a focused live-local check of the affected channel workflow before shipping.