bug: SessionPool lock scope crosses await and can stall prompt handling

[clsung]

Description

Summary

SessionPool currently keeps lock scope too broad across async work, which can stall prompt handling.

In practice, this showed up in the Discord flow as:

• one stuck request previously blocking other threads
• after switching to per-connection locking, cross-thread behavior improved
• but same-thread requests can still wait indefinitely if the previous stream never finishes cleanly

Root cause

Two patterns in SessionPool are problematic:

1. get_or_create() performs long async initialization while holding the pool lock

   • AcpConnection::spawn(...).await
   • conn.initialize().await
   • conn.session_new(...).await

2. with_connection() awaits the entire connection closure while holding the pool lock

That means a slow or stuck prompt/session lifecycle can block unrelated work.

Observed behavior

In the Discord handler:

• a request starts normally
• the upstream notification stream may stop producing events or never terminate cleanly
• later requests can stall before entering actual prompt handling
• after moving to per-connection locking, different threads no longer block each other
• however, requests on the same thread/session can still hang indefinitely behind the previous request

Suggested fix

Store connections as per-connection handles, for example:

RwLock<HashMap<String, Arc<tokio::sync::Mutex<AcpConnection>>>>

Then:

• use the pool lock only to look up / insert the handle
• drop the pool lock before any long-running await
• lock only the specific connection while running per-session work

Also consider adding:

• timeout handling for same-thread prompt contention
• fail-fast "busy" behavior for same-thread concurrent requests
• regression tests for stuck-session scenarios

Steps to Reproduce

A practical reproduction path:

1. Send a Discord message that starts a prompt stream
2. Cause the upstream notification stream to stop producing events or never terminate cleanly
3. Send another message
4. Observe later requests stalling or waiting indefinitely, depending on whether they reuse the same session/thread

This makes the bot appear unresponsive under partial failure conditions, especially when an upstream ACP stream stops yielding notifications or never completes cleanly.

Expected Behavior

• pool-level locking should only protect the connection map
• long-running prompt/session work should not hold the global pool lock
• different threads should remain independent
• same-thread contention should not appear as an indefinite hang

[CHC-Agent]

Issue at a Glance

Thread A (prompt streaming)          Thread B (new message)
       │                                    │
       ▼                                    ▼
  pool.with_connection()              pool.get_or_create()
       │                                    │
       ├── state.write().await              ├── state.write().await
       │   ╰─► LOCK ACQUIRED                │   ╰─► ⏳ BLOCKED (waiting for A)
       │                                    │
       ├── f(conn).await                     │
       │   (minutes...)                      │
       │                                    │
       └── drop lock                        └── finally proceeds
                                                 (user waited minutes)

Hi @clsung,

This is the same root cause as #58 — with_connection() holds the pool write lock across the entire prompt streaming lifecycle. Your analysis and proposed fix (per-connection Arc<Mutex>) aligns with what's already been discussed there.

Note: PR #310 (merged) refactored pool.rs for process group kill and session suspend/resume, but with_connection still holds the write lock across await — this issue remains unfixed.

Related PRs addressing this:

• Your PR fix: avoid blocking unrelated sessions during pool initialization (#256) #257 — directly targets this issue
• PR refactor(pool): per-connection Arc<Mutex> to unblock concurrent sessions #183 by @ruan330 — same per-connection locking approach

Both PRs take a similar approach. Maintainer review needed to decide which to move forward with.

[clsung]

Update on #256:

I rebased the session-pool fix onto current openabdev/openab main and force-pushed the updated branch behind PR #257.

Modification summary:

• preserved the merged #310 PoolState + suspend/resume design
• switched active sessions to per-connection Arc<tokio::sync::Mutex<AcpConnection>>
• removed long-running awaits from the global pool/state lock path in get_or_create()
• changed with_connection() so only the target session mutex is held during prompt/session work
• kept idle cleanup from blocking on active sessions
• added identity checks so cleanup/eviction do not remove a freshly replaced connection under the same thread_id

Validation:

• added unit tests covering the identity-checked removal logic
• cargo test passes: 42 passed; 0 failed

The updated implementation is on PR #257.