feat: expose idempotency key to HTTP, MCP, and CLI callers#251
Merged
windoliver merged 9 commits intomainfrom Apr 14, 2026
Merged
feat: expose idempotency key to HTTP, MCP, and CLI callers#251windoliver merged 9 commits intomainfrom
windoliver merged 9 commits intomainfrom
Conversation
The idempotency engine (fingerprint-based, single-flight, STATE_CONFLICT on mismatch) already existed at the operation layer but was unreachable from external callers. This wires it through all three surfaces so agents and scripts can submit retry-safe contributions. - HTTP: Idempotency-Key header on POST /api/contributions (RFC 8284) - MCP: idempotencyKey param on all 5 contribution tools (shared Zod fragment) - CLI: --idempotency-key flag on grove contribute - Core: idempotencyKey added to ReviewInput, ReproduceInput, DiscussInput, AdoptInput - Docs: docs/idempotency.md with full contract reference - Tests: 15 new integration tests across all three surfaces Closes #209
The in-memory idempotency cache was invisible across CLI invocations because each `grove contribute` spawns a new process with an empty Map. This made `--idempotency-key` silently useless for CLI users. - Add `idempotency_keys` table to SQLite schema (DDL in SCHEMA_DDL) - Add `SqliteIdempotencyStore` class with lookup/store/clear - Add `IdempotencyStore` interface to `OperationDeps` - `contributeOperation` checks SQLite on in-memory miss, writes to both layers after commit - Wire `SqliteIdempotencyStore` through CLI `executeContribute` - In-memory Map retained for single-flight (pending Promise coalescence) Verified: 5/5 cross-process E2E tests pass (retry, conflict, agent scoping, no-key baseline). 1143 unit tests pass, 0 regressions.
serve-http.ts only created a TopologyRouter when nexusClient was present, leaving local-mode MCP servers without topology routing or handoffs. This mirrored the serve.ts pattern: if topology exists, use NexusEventBus when available, otherwise fall back to LocalEventBus. Also wires runtime.handoffStore as fallback when nexusHandoffStore is unavailable, and passes eventBus into McpDeps. Verified: 7/7 E2E tests pass — coder submits work via MCP, handoff created to reviewer, idempotent retry returns cached CID with no duplicate handoff, STATE_CONFLICT on key reuse, reviewer review auto-transitions handoff to replied.
Two findings from adversarial review: 1. Durable idempotency row is now written inside the same SQLite transaction as the contribution (via onCommit callback in writeContributionWithHandoffs). Closes the crash window where a contribution could be committed without its idempotency record. 2. Pending in-memory cache entries are no longer TTL-expired or LRU-evicted. A slow write (>5min) no longer lets a retry bypass single-flight and start a duplicate write.
Adversarial review round 2 findings: 1. Cross-process race: two processes could both miss the SQLite lookup and both commit. Now uses INSERT OR IGNORE reservation before the write — only one process wins. Losers detect the conflict via a follow-up lookup and return the winner's CID or STATE_CONFLICT. 2. HTTP session leakage: idempotency keys were not scoped by sessionId, so a cached CID from session A could bypass session B's policy. Now appends sessionId to the key when present.
…urfaces
Adversarial review round 3 findings:
1. Durable reservation now uses pending/committed states. Pending rows
are never returned as successful results — callers get a retryable
STATE_CONFLICT instead. Eliminates the {} placeholder parse bug and
prevents cross-process duplicate writes.
2. IdempotencyStore now wired through all surfaces:
- Server (serve.ts) via ServerDeps + toOperationDeps
- MCP stdio (serve.ts) via McpDeps
- MCP HTTP (serve-http.ts) via McpDeps
- CLI (contribute.ts) — already wired
All surfaces now use durable SQLite reservation.
Adversarial review round 4 findings: 1. Pending reservations now rolled back on pre-commit failures via rollback() — no more stuck pending rows for the full TTL. 2. CLI path (no handoffs) now uses putWithCowrite when onCommit is set, making contribution + idempotency row truly atomic even without handoff records. 3. reserve() purges expired rows before INSERT OR IGNORE, so keys are reusable after TTL with different fingerprints.
…identity warning Adversarial review round 5 (final) findings: 1. reserve() now only purges expired committed rows, never pending — prevents stealing an in-flight reservation from a slow process. 2. Idempotency cache key now includes GROVE_SESSION_ID when set, preventing cross-session namespace collisions in MCP HTTP. 3. CLI warns when --idempotency-key is used without --role or --agent-id (hostname-pid is process-scoped, breaks cross-process retries). Docs updated with guidance.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #209.
The idempotency engine (fingerprint-based dedup, single-flight concurrency,
STATE_CONFLICTon key reuse with different payload) already existed at the operation layer but no external caller could use it — HTTP, MCP, and CLI surfaces didn't expose theidempotencyKeyparameter.This PR wires it through all three surfaces:
Idempotency-Keyheader onPOST /api/contributions(RFC 8284 / Stripe convention)idempotencyKeyparam on all 5 contribution tools via shared Zod fragment--idempotency-keyflag ongrove contributeidempotencyKeyfield added toReviewInput,ReproduceInput,DiscussInput,AdoptInputand threaded through sugar operationsdocs/idempotency.md— full contract reference (TTL, scoping, fingerprint coverage, conflict behavior)What this enables
Agents running in retry loops (network timeouts, process restarts, iterative workflows) can now submit contributions with a stable key. If the same key + same payload arrives again within 5 minutes, Grove returns the cached result instead of creating a duplicate. If the same key arrives with a different payload, Grove rejects it with
STATE_CONFLICT— surfacing the client bug instead of silently returning stale data.This is especially important for:
Test plan
grove_submit_work, flow-through for review/discuss/reproduce/adopttsc --noEmit)