fix(session-server): restore lock across Phase 1+2+3, preempt DELETE via cancellation channel

[rmfan]

Summary

Split out of #33 per offline review request: this PR contains ONLY the lock-restoration + DELETE-preemption cancellation channel + SessionStateConflictError 409 surface. #33 keeps multi-backend orchestration and logging/observability.

Why

PR #31 split session.lock around the SGLang proxy call to keep DELETE non-blocking while a chat was in flight. That split-lock window has a documented race:

1. Two same-session writers can both reach Phase 3 (the post-proxy commit).
2. The Phase-3 stale-state guard at sessions.py:241 on prod does:

   if session.num_assistant != expected_num_assistant:
       logger.warning(... "skipping state update")
       return backend.build_proxy_response(result)

   i.e. silently drops the second commit and still returns the 200 SGLang response to its caller.
3. The caller treats the 200 as success and appends the assistant message to its trajectory.
4. The next chat from that caller arrives with a trajectory the server hasn't seen → cursor mismatch.

Multi-backend (#33) doesn't fix this — it inherits the race per-backend. Restoring the lock around Phase 2 closes the race; the cancellation-channel preserves the non-blocking DELETE behaviour that the split-lock design was originally trying to achieve.

What changed

sessions.py — delete_session

After setting session.closing = True, cancel session.current_proxy_task if non-None. The in-flight chat coroutine catches CancelledError and returns 410 Gone, then releases the lock so DELETE's await session.lock.acquire() proceeds.

sessions.py — chat_completions

Phase 1+2+3 all run inside one async with session.lock: block. Phase 2 wraps the proxy call (and the rollback-retry path) in asyncio.create_task(...) stored on session.current_proxy_task. try/except asyncio.CancelledError → JSONResponse(410) / finally: session.current_proxy_task = None.

sessions.py — Phase-3 state-changed guard

Now raise SessionStateConflictError(...) (→ 409) instead of logger.warning + silent return of build_proxy_response(result). Unreachable under the lock-restored flow; defense-in-depth against a future split-lock regression.

linear_trajectory.py

Adds current_proxy_task: asyncio.Task | None = None to LinearTrajectory.

session_errors.py

Adds SessionStateConflictError(SessionError) with status_code = 409.

Test updates

Two race tests encoded the old "DELETE waits for chat to finish" contract:

• test_double_delete_second_returns_404 — chat now returns 410 (was 200) when DELETE preempts it
• test_multiple_chats_queued_then_delete — in-flight chat at DELETE time now returns 410; queued chats still 404. Status-code set widened to {200, 404, 410}; at-least-one-in-flight assertion widened to accept 410.

Test plan

• Race tests baseline preserved: 5 pass / 4 fail (same as prod). The 4 failures are the documented get_or_create_session auto-create-after-delete behaviour, unrelated to this PR.
  • Before this PR (prod): 4 fail / 5 pass — test_delete_can_proceed_while_chat_is_mid_proxy, test_chat_during_delete_returns_404, test_chat_after_delete_returns_404, test_rapid_create_chat_delete_cycles.
  • After this PR (with test updates): same 4 fail / 5 pass.
• CI green
• Manual: drive a DELETE while a chat is in flight; confirm chat returns 410 and DELETE returns 204 within milliseconds.

Observable behavior changes

• A chat in flight when its session is DELETEd now returns 410 Gone (was: 200 with the upstream SGLang response). Distinct from 404 ("never existed") and 409 ("retryable conflict"). Harbor / litellm should treat 410 as definite session-closed.
• A future split-lock regression now surfaces 409 with SessionStateConflictError instead of silently corrupting trajectory state.

Relationship to #33

#33 keeps a copy of these lock-restoration changes today; once this PR lands on prod, #33 will rebase and shed the duplicated changes. The two PRs are not blocked on each other — #33 can land first too, in which case this PR's rebase is a no-op.

🤖 Generated with Claude Code

[odp]

Raising SessionStateConflictError here makes one think that this can still happen, which is impossible because of the new locking protocol. IMO this can be removed safely to avoid confusion, and only the lock related change should stay.

[odp]

PR #31 split session.lock around the SGLang proxy call to keep DELETE non-blocking while a chat was in flight. That split-lock window has a documented race

have we observed this in one of our runs? If yes, please document.