fix(api_server): serialize concurrent agent turns per session

[meiqinsi]

What does this PR do?

Serialize concurrent agent turns on the same session_id inside APIServerAdapter, so overlapping /v1/chat/completions (including wake self-posts) and /v1/runs cannot run two conversation loops against one SessionDB transcript.
Without this, co-turns load stale history snapshots and can re-execute the same work / fight over persistence (duplicate side effects, lock_contended / session_persistence_failed class failures). A per-session asyncio lock queues the second turn behind the first. Empty session_id still skips the lock; different sessions stay parallel. /v1/runs still returns 202 immediately — the background task waits on the lock after admit.
This is an in-process api_server serialize fix only. It does not reload history after acquiring the lock, and does not add a cross-process / DB-level turn queue.

Related Issue

Related to #84235 (partial: in-process serialize for api_server chat + runs; does not close post-wait history reload or cross-process queue).
Complementary to #77800 (wake idempotency / coalesce); this PR does not change wake timeout retry policy.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• gateway/platforms/api_server.py: add _session_turn_locks / _hold_session_turn_lock; acquire around _run_agent and inside /v1/runs _run_and_close (after 202).
• tests/gateway/test_wake_delivery.py: same-session serialize, different-session parallel, wiring into _run_agent + _handle_runs.
• tests/gateway/test_api_server_runs.py: while the shared lock is held, /v1/runs stays queued and does not _create_agent; completes after release.

How to Test

1. scripts/run_tests.sh tests/gateway/test_wake_delivery.py
2. scripts/run_tests.sh tests/gateway/test_api_server_runs.py -k session_turn_lock
3. Manual / e2e (optional): same-session /v1/runs that spawns async delegation + wake self-post → parent session shows one continuous turn (no interleaved short turns; no lock_contended / session_persistence_failed). Expect wake retries to queue behind an in-flight parent turn rather than starting parallel loops.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run scripts/run_tests.sh tests/gateway/test_wake_delivery.py tests/gateway/test_api_server_runs.py -k session_turn_lock (project CI-parity wrapper; not bare pytest) and targeted tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS (darwin 24.6.0)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

• Targeted tests via scripts/run_tests.sh: 8 passed (wake_delivery + session_turn_lock).

• E2E after this change (same session, async delegation + wake): wake timeout retries queue behind one long parent turn; four /v1/chat/completions complete together when the lock is released; no lock_contended / session_persistence_failed. Parent wall-clock looks longer because wakes are serialized — expected and desirable vs concurrent stale-snapshot turns.

[spfcraze]

This was generated by AI during triage.

Summary:
The new per-session lock map is written but never pruned, so a long-lived APIServerAdapter keeps one asyncio.Lock per distinct session_id it ever saw.

Problems:

• _get_session_turn_lock (api_server.py:6183) only inserts into _session_turn_locks; nothing in the head removes an entry, so the map grows without bound.
• Every other per-key dict this adapter owns is pruned in a finally block (_active_run_tasks.pop, _active_run_agents.pop, _run_approval_sessions.pop, _stopping_run_ids.discard at api_server.py:7003), so the lock map breaks the cleanup discipline the runs lifecycle otherwise follows.
• The added comment notes "Ephemeral fingerprint sessions still serialize when they share the derived id" — ephemeral sessions are exactly the high-churn case, and their lock entries are never released.

Solution:
Remove a session's lock (under _session_turn_locks_guard) once its turn completes and no waiter remains, e.g. pop it in _hold_session_turn_lock after lock.release(), so the map tracks only live sessions.

Evidence

no deterministic fact backs this claim — model belief, not executed or read evidence

---

Checked against 5969da0 — the tip of fix/api-server-session-turn-lock when this was written — and 8c8d55b, main at the same moment.

[meiqinsi]

@spfcraze Thanks — agreed the map must not grow unbounded. Popping immediately after lock.release() is unsafe when a waiter has already checked out that same Lock object: the next turn would insert a new lock and run in parallel with the waiter (the original bug this PR is fixing).

Addressed in 5b98fd3 with a checkout refcount (holder + waiters). The map entry is removed only when the last ref drops, including if acquire() is cancelled.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(api_server): serialize concurrent agent turns per session

1. test_session_turn_lock_wired_into_run_agent_and_handle_runs reads method source via inspect.getsource and asserts string containment. This is exactly the "never read source code in tests" pattern the repo bans in AGENTS.md — it fails on innocuous refactors and can pass while the wiring is subtly wrong. Replace with a behavioral test (hold the lock, drive _run_agent/_handle_runs through a mock client, assert serialization) or drop it; the other new tests already cover the lock semantics well.

2. The per-session lock is non-reentrant. A turn that holds the lock and synchronously awaits another turn on the same session id (any future inline wake/delegation path that calls back into the adapter within the same task rather than through a fresh HTTP request) would self-deadlock. A doc note on _hold_session_turn_lock stating "re-entry must arrive as a new request" would guard a future refactor.

3. _release_session_turn_lock uses self._session_turn_lock_refs.get(session_id, 1) - 1: the default-1 silently masks an accounting bug (a missing ref entry is treated as 1 and pops the entry). The guard lock makes this unreachable today, but an explicit pop/KeyError would surface misuse instead of hiding it.

4. Minor: the ~260-line re-indent of _run_and_close (everything moved under the lock) buries the actual change and inflates the merge-conflict surface. Extracting the inner body into a helper would make the diff reviewable.

[meiqinsi]

@Enough1122 Thanks for the review — addressed in latest push:

• Removed test_session_turn_lock_wired_into_run_agent_and_handle_runs (inspect.getsource); lock wiring is covered by behavioral tests (test_start_waits_for_session_turn_lock_shared_with_run_agent, etc.).
• Documented non-reentrant constraint on _hold_session_turn_lock.
• Left points 3–4 as-is for minimal diff unless you prefer otherwise.

Also rebased onto latest main and resolved _run_and_close conflict (session turn lock + browser_control binding + _ProviderAuthResolutionError handler).

[Enough1122]

Thanks @meiqinsi — dropping the inspect.getsource test in favor of behavioral coverage is the right call; noted at 3de643184.