fix(rate-limit): SQL interval parameterization — Phase 3 hotfix#316
Merged
Conversation
…lue, not inside a quoted literal Phase 3 (#315) introduced a per-IP rate limit that was a silent no-op in production. Symptom: 12 streaming requests in a row all returned 200; rate_limit_events table had 0 rows. Root cause: the SQL used `interval '${WINDOW_SECONDS} seconds'` inside a tagged-template literal. The @neondatabase/serverless driver substitutes `${...}` placeholders as $N parameters, but parameters cannot appear inside a Postgres string literal. The driver emitted `interval '$2 seconds'` and the planner rejected it with `invalid input syntax for type interval`. The proxy's fail-open catch then allowed the request through. Fix: build `WINDOW_INTERVAL = '60 seconds'` at module load and splice it as a parameterized value cast to ::interval: `ts < now() - ${WINDOW_INTERVAL}::interval` That emits `ts < now() - $2::interval`, which Postgres evaluates correctly. Also added `AND ts > now() - ${WINDOW_INTERVAL}::interval` to the SELECT — the DELETE+SELECT now use the same window boundary so the count can't accidentally include rows that the DELETE didn't yet prune. Smoke-tested against the live Neon DB: Request 1: count=1, allowed=true ... Request 10: count=10, allowed=true Request 11: count=11, allowed=false ← rate limited Request 12: count=12, allowed=false Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
blove
added a commit
that referenced
this pull request
May 14, 2026
The Phase 3 hotfix in #316 fixed a SQL bug in scripts/rate-limit.ts but never reached production: the CI Check if demo changed step's regex only watched assemble-demo, demo-middleware, and langgraph-proxy. A rate-limit.ts change alone evaluated demo_changed=false, the deploy step was skipped, and demo.cacheplane.ai kept serving the original broken bundle. Two changes: 1. Extend the watched regex to include scripts/rate-limit.ts. 2. Touch scripts/demo-middleware.ts (a documentation comment) so THIS PR matches the existing watched paths and triggers the redeploy that picks up #316's SQL fix. Also documents the gating contract in demo-middleware.ts itself so a future engineer splitting rate-limit into multiple files knows to update the regex. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
blove
added a commit
that referenced
this pull request
May 14, 2026
The Phase 3 hotfix in #316 fixed a SQL bug in scripts/rate-limit.ts but never reached production: the CI Check if demo changed step's regex only watched assemble-demo, demo-middleware, and langgraph-proxy. A rate-limit.ts change alone evaluated demo_changed=false, the deploy step was skipped, and demo.cacheplane.ai kept serving the original broken bundle. Two changes: 1. Extend the watched regex to include scripts/rate-limit.ts. 2. Touch scripts/demo-middleware.ts (a documentation comment) so THIS PR matches the existing watched paths and triggers the redeploy that picks up #316's SQL fix. Also documents the gating contract in demo-middleware.ts itself so a future engineer splitting rate-limit into multiple files knows to update the regex. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Hotfix for #315. The Phase 3 rate limit was a silent no-op in
production. Symptom: 12 streaming requests in a row from one IP all
returned 200; the `rate_limit_events` table stayed empty.
Root cause
The SQL used `interval '${WINDOW_SECONDS} seconds'` inside a
`@neondatabase/serverless` tagged template. The driver substitutes
`${...}` placeholders as `$N` parameters, but parameters cannot
appear inside a Postgres string literal. The driver emitted
`interval '$2 seconds'`, Postgres rejected it with
`invalid input syntax for type interval`, and the proxy's
fail-open catch allowed the request through silently.
Fix
Build `WINDOW_INTERVAL = '60 seconds'` at module load and splice it
as a parameterized value cast to `::interval`:
```ts
ts < now() - ${WINDOW_INTERVAL}::interval
```
That emits `ts < now() - $2::interval`, which Postgres evaluates
correctly. Also added the same window filter to the SELECT so
DELETE+SELECT share the same boundary.
Verification
Smoke-tested against the live Neon DB before pushing:
```
Request 1: count=1, allowed=true
...
Request 10: count=10, allowed=true
Request 11: count=11, allowed=false ← rate limited
Request 12: count=12, allowed=false
```
Test plan
🤖 Generated with Claude Code