Skip to content

feat(api): durable idempotency for POST /v1/runs - #88408

Open
pesho-vsn wants to merge 1 commit into
NousResearch:mainfrom
pesho-vsn:fix/88400-runs-idempotency
Open

pesho-vsn wants to merge 1 commit into
NousResearch:mainfrom
pesho-vsn:fix/88400-runs-idempotency

Conversation

@pesho-vsn

Copy link
Copy Markdown

Summary

  • add atomic SQLite-backed Idempotency-Key reservations for POST /v1/runs
  • replay the original run for identical retries across races, terminal states, and gateway restarts
  • reject key reuse with changed payloads using 409 idempotency_key_conflict
  • scope records and run access to the authenticated profile/credential, retain records for 24 hours, and keep keyless behavior unchanged
  • document the API contract and retention policy

Tests

  • uv run --extra dev --extra messaging pytest -q tests/gateway/test_api_server_runs.py --disable-warnings --maxfail=1 (29 passed)
  • uv run --extra dev ruff check gateway/platforms/api_server.py tests/gateway/test_api_server_runs.py
  • git diff --check

Closes #88400

Compatibility

The header is optional. Existing clients without Idempotency-Key continue to create a fresh run per request. Durable state is stored in owner-only runs_idempotency.db; only fingerprints, opaque auth/profile scopes, run IDs, and public statuses are persisted (not payloads or credentials).

Related Relay fix

The branch is based on current main, which already contains both 9a9b670e29f592fdf50bcca1e3e777150522b6b5 and merge commit 42708f8bb39c9c2fc19146956699699bc3ea2da5; they were not duplicated.

@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades area/sessions Session lifecycle, resume, persistence, history labels Aug 17, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Solid durable-idempotency design: (scope, key) reservations inserted under BEGIN IMMEDIATE make multi-worker admission actually exclusive, fingerprints compared with hmac.compare_digest, retention is enforced lazily, WAL is applied with a labeled fallback, file permissions are tightened to 0600 including -wal/-shm, and request bodies/secrets are deliberately excluded from storage. Tests cover both lifecycle branches. Two growth concerns:

  1. gateway/platforms/api_server.py:~1551 — self._run_idempotency_ids and _run_owners grow once per keyed run and are never pruned, even though the store's 24h retention deletes rows underneath them. On a long-lived gateway this is a slow memory leak plus stale ownership claims for runs whose rows aged out (owns_run then disagrees with the set). Mirror the store's retention: prune these structures on the same cadence or derive membership from a store query.
  2. api_server.py:_set_run_status (~6697) — every status transition (including per-tool-event updates arriving through the SSE callback) now issues an UPDATE+commit against the idempotency DB. High-frequency tool events turn this into per-event fsyncs; consider persisting only terminal/relevant transitions and keeping intermediate states in memory, since replay consumers only need the final row after completion anyway.
  3. The :memory: fallback silently converts durable idempotency into best-effort with no operator signal — one WARNING naming the reason (unwritable path) would explain otherwise-puzzling duplicate-run reports after a restart. (nit)
  4. Fingerprint mismatch returning "conflict" (presumably 409) rather than replaying the other body's result is exactly right. (positive)
  5. nit: RunIdempotencyStore.close() exists but I don't see it wired into adapter shutdown — long-lived WAL connections should be closed on teardown. (nit)

No blocking issues found beyond items 1–2's long-uptime profile.

@dokterdok

Copy link
Copy Markdown
Contributor

@pesho-vsn I am using this commit as a prerequisite in draft #95965 for an end-to-end RoomLink stack. Your commit and authorship are preserved unchanged; this is not intended to replace your PR. If this lands first, I will drop the patch-identical commit 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

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runs API: durable Idempotency-Key support with payload conflict semantics

4 participants