Skip to content

feat(peer): async run/status with idempotent /v1/runs admission - #94336

Open
giaiant wants to merge 2 commits into
NousResearch:mainfrom
giaiant:feat/peer-async-run-status
Open

giaiant wants to merge 2 commits into
NousResearch:mainfrom
giaiant:feat/peer-async-run-status

Conversation

@giaiant

@giaiant giaiant commented Aug 25, 2026

Copy link
Copy Markdown

What

Adds peer async dispatch (Hermes peer run/status) with idempotent admission to the API server's /v1/runs endpoint.

  • gateway/platforms/api_server.py: idempotent admission for /v1/runs — a lock serializes concurrent POSTs so the same idempotency key cannot create duplicate runs. The idempotency key is registered before the history read, closing the race where two same-key requests both pass the check and both create a run.
  • hermes_cli/subcommands/peer.py: peer run / peer status subcommands.
  • Tests: tests/gateway/test_api_server_runs.py + new concurrent same-key race test; tests/hermes_cli/test_peer_cmd.py.
  • Docs: peer run/status + Idempotency-Key reference.

Why

Previously, the peer async work lived as an uncommitted diff. This promotes it to a reviewable change, and fixes a real race (idempotency key registered after the await, so concurrent same-key POSTs could double-create runs).

Test

  • tests/gateway/test_api_server_runs.py + tests/hermes_cli/test_peer_cmd.py: 39 passed (incl. new race test)
  • Regression tests/gateway/test_api_server.py + test_api_server_active_work_drain.py: 129 passed

Deepcool added 2 commits August 25, 2026 10:08
- hermes peer run starts a canonical Bot Chat turn through POST /v1/runs and returns a run_id immediately; hermes peer status polls it without holding the HTTP connection.

- POST /v1/runs admits an Idempotency-Key: a replay resolves before the concurrency cap so a retry after a lost 202 returns the original run instead of a 429, and the session transcript is reloaded after admission.

- Serialize run admission under a per-adapter asyncio.Lock so two concurrent same-key requests resolve to one run instead of two (fixes the check/register TOCTOU race across the session-history reload).

- Tests: peer run/status against a fake peer; runs endpoint replay, conflict, session-history load, and the concurrent same-key path.
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 25, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

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

Solid design overall: resolving a replay before the concurrency gate (gateway/platforms/api_server.py:6771-6774) is exactly right for lost-202 recovery, key validation bounds abuse, the idempotency map is swept in lockstep with status retention (api_server.py:7468-7472), and the lock makes check-and-register atomic against the history-reload await. Points:

  1. The global admission lock is held across per-request session I/Ogateway/platforms/api_server.py:6695-6699 serializes all /v1/runs admissions, and the critical section now includes await self._conversation_history_for_session(...) (:6820-6823), which is disk/store I/O of unbounded duration per request. Every concurrent admission — including ones with no idempotency key at all — queues behind each other's history reloads, so p99 admission latency becomes the sum of in-flight history reads. Consider splitting into two short sections: fingerprint + replay-check under the lock, release for the history reload, then re-acquire only to re-validate the key and register the run. That keeps the atomicity that matters (key→run_id) without making unrelated admissions wait on one caller's store.

  2. Replay responses are shaped differently from originals — originals return {"status": "started", "replayed": false} plus X-Hermes-Session-Key and other response headers (:7166-7170); replays return {"status": <live>, "replayed": true} with no headers (:6815-6821). A client that recovered from a lost 202 gets a subtly poorer response than the one it missed — most importantly the session-key header. Worth either attaching the stored run's headers or documenting the asymmetry in the endpoint docs you added.

  3. Fingerprint coverage is an explicit allowlist — only input, session_id, instructions, conversation_history, previous_response_id, model, provider, model_options participate (:6790-6810). Two requests sharing a key but differing in any other semantic field (tool selection, metadata the run honors) silently replay the original instead of getting a 409. Defensible (stream-flag exclusion is actually correct), but the doc should list which fields are identity-bearing so callers don't assume whole-body semantics.

Tests covering the contended-wait race via _slow_history are exactly the right kind.

@dokterdok

Copy link
Copy Markdown
Contributor

@giaiant I am using the async run/status commits as prerequisites in draft #95965 for an end-to-end RoomLink stack. Your commits and authorship are preserved unchanged; this is not intended to replace your PR. If this lands first, I will drop the patch-identical commits from the composition.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants