[Bug] Duplicate inbox delivery: concurrent deliver_pending picks the same PENDING message twice (pick→mark window unprotected)

[chaogebaba]

Symptom

The same PENDING inbox message is occasionally delivered twice to a terminal, a few milliseconds apart — the worker receives the identical prompt twice (double send_keys), which can double-run a task.

Evidence

Server log cao_2026-07-09_17-45-39.log (local run): identical send_keys pairs 4 ms apart at 17:51:09, 17:52:44, and 17:59:45 — three duplicate deliveries to the same terminal in one session.

Root cause pointer

src/cli_agent_orchestrator/services/inbox_service.py — deliver_pending's get-pending → type-into-terminal → mark-DELIVERED sequence is not atomic and has no per-terminal mutual exclusion. Two concurrent deliver_pending invocations (e.g. one triggered by a status-monitor idle transition and one by a message-send hook) both read the same PENDING row before either marks it DELIVERED.

Suggested direction

Either a per-terminal delivery lock (async lock keyed by terminal id held across pick→send→mark), or DB-level claim-before-send (single UPDATE … WHERE status='PENDING' … RETURNING as the pick). We run the per-terminal-lock variant in our fork and it has held under burst tests (message floods while the terminal flaps idle/busy); happy to upstream it as a PR if maintainers want.

[haofeif]

@call-me-ram can you please help to review this if you get a chance ?

[call-me-ram]

Confirmed against current main. deliver_pending's mark-DELIVERED-before-send closes the re-entrancy window from #164, but there's no mutual exclusion across the concurrent entry points — the status-event path (asyncio.to_thread), the immediate-delivery path, and the PENDING retry sweep can each read the same PENDING row before any of them marks it. That matches the ~4ms-apart duplicate send_keys pairs in your logs.

A PR would be very welcome. Mild preference for the DB-level claim (a single UPDATE … WHERE status='PENDING' … RETURNING as the atomic pick) since it's also multi-process-safe, but the per-terminal async lock you've burst-tested in your fork is a fine first step if that's what's proven out. Ideally include a regression test that races two deliver_pending calls and asserts one delivery.