fix(middleware): refund rate-limit counter on idempotency cache hit (#167)

* fix(middleware): refund rate-limit counter on idempotency cache hit

# Finding

CLAUDE.md FINDING API-1 — `POST /queue/new` (and every provision endpoint)
with the same `Idempotency-Key` retried after a transient 5xx burns the
per-fingerprint daily rate-limit counter on every retry. The published
Stripe-shape replay contract says replays should return the cached body
verbatim — the actual middleware honours that on the body side but the
upstream rate-limit slot is gone, so agents hit 429 inside their retry
budget and silently abandon.

# Root cause

Ordering, not a missing cache lookup.

- `internal/router/router.go:201` — `app.Use(middleware.RateLimit(...))`
  at global scope, runs BEFORE every per-route middleware chain. The
  per-fingerprint daily INCR fires here on every request, including
  replays.
- `internal/middleware/idempotency.go:283-301` — the explicit-key cache
  HIT branch (and 392-406 fingerprint-fallback HIT branch) correctly
  serves the cached body verbatim, but by then RateLimit has already
  burned the slot. The original design comment at idempotency.go:46-59
  documented the conflict as deliberate "anti-abuse"; the OpenAPI
  contract at openapi.go:204 explicitly told callers replays didn't
  cost rate-limit budget. Those two contradicted; the contract loses.

# Fix (Option C, per CEO memo 2026-05-29)

Refund the rate-limit counter on cache HIT — single Redis `DECR` from
the idempotency cache-hit branches via a new `RefundRateLimitCounter`
helper in `rate_limit.go`. The first call still pays the INCR cost
(an attacker reusing one key cannot get free attempts, only
amortized-cheaper). The per-fingerprint provision-dedup cap
(CLAUDE.md rule 6, handler-internal `prov:<fp>:<date>` counter) is
NOT touched — that's where the abuse signal actually lives.

Self-contained: 1 helper + 2 call sites + 1 metric + 1 OpenAPI string.
No API contract change. No router rewiring. No new test fixtures.

# Files

- `internal/middleware/rate_limit.go` — add `RefundRateLimitCounter`,
  stash the computed Redis key + configured limit into Fiber Locals so
  the refund helper can DECR the exact key without re-deriving prefix.
- `internal/middleware/idempotency.go` — call refund from both cache-HIT
  branches (explicit-key + body-fingerprint). 409 conflict path does
  NOT refund (genuine error, agent pays for the mistake).
- `internal/metrics/metrics.go` — new `IdempotencyReplayRefunded`
  CounterVec labelled by `route` so on-call sees which provision
  endpoints absorb the most retry-storm traffic.
- `internal/handlers/openapi.go` — update Idempotency-Key parameter
  description to reflect the new contract (replays NO LONGER consume
  rate-limit budget; FIRST call still pays).
- `internal/middleware/idempotency_test.go` — 4 new regression tests:
  - `TestIdempotencyCacheHitRefundsRateLimit` — headline boundary:
    counter stays at 1 across 2 same-key calls, X-RateLimit-Remaining
    reflects post-refund budget.
  - `TestIdempotencyDifferentKeyDoesNotRefund` — negative: distinct
    keys must burn 2 slots.
  - `TestIdempotencyConflictDoesNotRefund` — 409 is a genuine error,
    must NOT refund.
  - `TestRefundRateLimitCounterSafeNoOps` — nil rdb + no LocalKey =
    safe no-op (not panic / not corruption).

# Coverage block (per CLAUDE.md rule 17)

```
Symptom:        POST /queue/new (and every provision endpoint) with
                retried Idempotency-Key burns rate-limit slot,
                eventually 429s the agent inside its retry budget.
Enumeration:    grep -n 'c.Status(entry.StatusCode).Send(entry.Body)'
                internal/middleware/idempotency.go
Sites found:    2  (explicit-key HIT @ idempotency.go:283-301,
                    fingerprint HIT @ idempotency.go:392-406)
Sites touched:  2  (both branches call RefundRateLimitCounter; conflict
                    path explicitly does NOT refund — see code comment)
Coverage test:  TestIdempotencyCacheHitRefundsRateLimit (explicit),
                TestIdempotencyDifferentKeyDoesNotRefund (negative).
                Helper safety: TestRefundRateLimitCounterSafeNoOps.
Live verified:  pending — fix-api-idem branch, awaiting CI green +
                deploy + /healthz commit-sha grep.
```

# Surface checklist (per CLAUDE.md rule 22)

- [x] api/internal/middleware/* — fix + tests
- [x] api/internal/metrics/metrics.go — new counter
- [x] api/internal/handlers/openapi.go — contract description updated
- [x] infra companion PR — alert + dashboard tile + METRICS-CATALOG
      row (rule 25): InstaNode-dev/infra branch
      `fix/idempotency-replay-refund-observability`
- [n/a] content/llms.txt — current llms.txt doesn't mention the old
        rate-limit-budget contract, no change needed.
- [n/a] dashboard upgradeCopy.ts — no pricing/upsell impact.
- [n/a] CHANGELOG — repo has no CHANGELOG.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(middleware): cover refund DECR-error + clamp branches

CI's diff-cover gate flagged rate_limit.go:190-197, 203-204, 213-214 as
uncovered patch lines (80% — fails the 100% per-PR floor). Add three
sub-tests under TestRefundRateLimitCounterSafeNoOps:

- redis_decr_error_fails_open — closes a Redis client mid-test, asserts
  the WARN-log + RedisErrors metric path is hit without propagating the
  error to the response.
- decr_below_zero_clamps — covers BOTH the newCount<0 clamp (DECR on a
  non-existent key returns -1) AND the remaining<0 clamp (pre-seed a
  counter > configured limit so DECR still leaves it over-cap; the
  X-RateLimit-Remaining header must floor at 0, never go negative).

Brings the patch coverage on this PR to 100% per project memory rule
`feedback_coverage_95_floor_100_patch`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: mastermanas805

internal/handlers/openapi.go

@@ -201,7 +201,7 @@ const openAPISpec = `{
       "post": {
         "summary": "Provision a Postgres database",
         "description": "Returns a real postgres:// connection string with pgvector pre-installed. Anonymous tier: 10MB, 2 connections, 24h TTL.\n\nSupports Stripe/AWS-style idempotency via the optional Idempotency-Key request header — see the parameter description below.",
-        "parameters": [{ "name": "Idempotency-Key", "in": "header", "required": false, "schema": { "type": "string", "maxLength": 255 }, "description": "Opaque client-supplied key (1-255 ASCII printable chars) that makes this POST safe to retry. The first response is cached for 24h; subsequent calls carrying the same key return the cached response verbatim with X-Idempotent-Replay: true. Reusing a key with a different body returns 409. Replays still consume rate-limit budget (anti-abuse) but do NOT consume quota budget (the original call already did)." }],
+        "parameters": [{ "name": "Idempotency-Key", "in": "header", "required": false, "schema": { "type": "string", "maxLength": 255 }, "description": "Opaque client-supplied key (1-255 ASCII printable chars) that makes this POST safe to retry. The first response is cached for 24h; subsequent calls carrying the same key return the cached response verbatim with X-Idempotent-Replay: true. Reusing a key with a different body returns 409. Replays do NOT consume rate-limit budget — the per-fingerprint daily counter is refunded on every cache hit so an agent retrying transient 5xx with the same key gets the documented replay (FINDING API-1, 2026-05-29). The FIRST call still pays the rate-limit cost; replays are refunded. The per-fingerprint provision-dedup cap (5 fresh resources/day, anti-abuse) is unchanged." }],
         "requestBody": { "content": { "application/json": { "schema": { "$ref": "#/components/schemas/ProvisionRequest" } } } },
         "responses": {
           "201": { "description": "Database provisioned", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/DBProvisionResponse" } } } },

internal/metrics/metrics.go

@@ -57,6 +57,27 @@ var (
 		Help: "Requests blocked by fingerprint rate limiting",
 	})
 
+	// IdempotencyReplayRefunded counts the rate-limit counter refunds the
+	// Idempotency middleware issues on a cache HIT — one increment per
+	// replayed response that successfully DECR'd the per-fingerprint
+	// daily counter (CLAUDE.md FINDING API-1, fix Option C).
+	//
+	// Labelled by route_path so on-call can see which endpoints absorb the
+	// most retry-storm traffic. A steady non-zero rate is healthy (agents
+	// are retrying transient 5xx and we're honoring the published Stripe-
+	// shape replay contract). A sudden spike on one route correlates with
+	// upstream brownouts; flip to NR and check the corresponding 5xx rate
+	// for the same route.
+	//
+	// Companion alert (infra repo): "idempotency replay refund spike (1h)"
+	// fires when rate(idempotency_replay_refunded_total[1h]) > 5×7d
+	// baseline — points the operator at a brownout in the underlying
+	// provisioner before agents start abandoning.
+	IdempotencyReplayRefunded = promauto.NewCounterVec(prometheus.CounterOpts{
+		Name: "instant_idempotency_replay_refunded_total",
+		Help: "Rate-limit counter refunds issued by Idempotency middleware on cache hit",
+	}, []string{"route"})
+
 	// RecycleGateBlocked counts anonymous provision attempts blocked by the
 	// free-tier recycle gate (Option B from FREE-TIER-RECYCLE-2026-05-12).
 	// Labelled by resource_type so we can see which services see the most

internal/middleware/idempotency.go

@@ -46,16 +46,22 @@ import (
 // Middleware ordering (see internal/router/router.go for the per-route
 // wiring): RateLimit runs at app.Use scope (global, before OptionalAuth),
 // so by the time this middleware runs the per-fingerprint daily counter
-// has already incremented. THIS IS DELIBERATE: a malicious agent must NOT
-// be able to bypass rate limiting via Idempotency-Key reuse, so replays
-// still consume rate budget. The original-call cost is borne by the
-// counter on the FIRST request; replays add an extra increment, which is
-// the conservative choice — the customer paid for the first call (in
-// quota terms) but a key-reuse attacker doesn't get free attempts.
-// Quota-walls inside handlers (CheckAndIncrementToken) similarly continue
-// to fire on replay paths, but the replay short-circuits BEFORE the
-// handler so the quota counter is unaffected — the cached response simply
-// goes out the wire. Net effect: rate-limit budget = abuse-protected;
+// has already incremented. To honor the published Stripe-shape replay
+// contract — same Idempotency-Key replays the cached response without
+// burning a fresh rate-limit slot — every cache HIT path below calls
+// RefundRateLimitCounter (single Redis DECR) BEFORE sending the cached
+// response. The FIRST call still pays the cost (the original INCR), so
+// an attacker reusing one key 100× gets amortised-cheaper attempts, NOT
+// free attempts (FINDING API-1, CEO Option C, 2026-05-29). The handler-
+// internal per-fingerprint provision-dedup cap (5/day, CLAUDE.md rule 6)
+// is NOT touched by the refund — that abuse signal lives in handler
+// code and is independent of the request-rate-limit counter.
+//
+// Quota-walls inside handlers (CheckAndIncrementToken) continue to fire
+// on replay paths, but the replay short-circuits BEFORE the handler so
+// the quota counter is unaffected — the cached response simply goes out
+// the wire. Net effect: rate-limit budget = refunded on replay (Stripe
+// contract); fingerprint provision-dedup = abuse-protected (unchanged);
 // quota budget = customer-friendly (no double-charge for retries).
 //
 // Cache key shape: idem:<scope>:<endpoint>:<sha256(key)> where <scope> is
@@ -287,12 +293,20 @@ func idempotencyExplicit(c *fiber.Ctx, rdb *redis.Client, endpoint, scope, rawKe
 				"error", jerr, "endpoint", endpoint)
 		} else {
 			if entry.BodyHash != reqBodyHash {
+				// 409 is a genuine error response, not a replay — DO NOT
+				// refund the rate-limit counter here. The agent did the
+				// wrong thing (reused a key for a different body) and
+				// should still pay the cost of that mistake.
 				return c.Status(fiber.StatusConflict).JSON(fiber.Map{
 					"ok":      false,
 					"error":   "idempotency_key_conflict",
 					"message": "Idempotency-Key already used with a different body",
 				})
 			}
+			// Cache HIT — refund the rate-limit slot RateLimit burned on
+			// the way in (FINDING API-1, Option C). Fail-open: a refund
+			// error logs WARN but never blocks the cached response.
+			RefundRateLimitCounter(c, rdb)
 			c.Set(idempotencyReplayHeader, "true")
 			if entry.ContentType != "" {
 				c.Set(fiber.HeaderContentType, entry.ContentType)
@@ -397,6 +411,12 @@ func idempotencyFingerprint(c *fiber.Ctx, rdb *redis.Client, endpoint, scope str
 				"error", jerr, "endpoint", endpoint)
 			// Corrupt — fall through to handler and overwrite below.
 		} else {
+			// Cache HIT on the body-fingerprint fallback path. Same
+			// refund semantics as the explicit-key branch: the
+			// rate-limit slot RateLimit burned on the way in is
+			// returned because we're serving a cached response, not
+			// re-running the handler (FINDING API-1, Option C).
+			RefundRateLimitCounter(c, rdb)
 			c.Set(idempotencySourceHeader, idempotencySourceFingerprint)
 			c.Set(idempotencyReplayHeader, "true")
 			if entry.ContentType != "" {