fix(e2e): disable rate limiting on the E2E server to stop 429 flakes (BUG-2089)#922
Merged
Conversation
…(BUG-2089) The E2E harness runs the real pad binary with the real rate limiter, and every Playwright test shares one loopback IP (127.0.0.1). The auth limiter (5 logins/min/IP, burst 5) trips as soon as a spec logs in a couple of browser clients — collab-persistence.spec.ts logs in two per test — so browserLogin fails with "in-page login failed with status 429". This was deterministic, not flaky: it failed on TASK-2058's own PR and its push to main, and on every downstream PR since. Add a test-only env knob PAD_DISABLE_RATE_LIMITS: when truthy, New() leaves Server.rateLimiters nil, which RateLimit() already treats as a pass-through (Stop() and the MCP path are already nil-safe). Wire it into the Playwright webServer.env; run-pad.mjs spawns the binary with inherited env so it reaches the pad process. Limiters stay fully active in prod/self-host — the knob is an explicit opt-in only the E2E server sets. Verified: collab-persistence.spec.ts passes locally with the fix; the existing limiter tests still pass (limiters on when the env is unset); new TestRateLimit_DisabledByEnv pins the bypass. Claude-Session: https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF
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
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.
Problem
The
E2E (Playwright)CI job fails deterministically inweb/e2e/collab-persistence.spec.ts:Server logs:
Not a flake — introduced by TASK-2058 (
348c8a3d); CI for that PR and its push tomainboth failed the same way, and every downstream PR since (e.g. #921).mainis currently red on E2E.Root cause
The E2E harness runs the real
padbinary with the real rate limiter, and every test shares one loopback IP (127.0.0.1). Theauthlimiter allows 5 logins/min/IP, burst 5 (internal/server/middleware_ratelimit.go:206).collab-persistence.spec.tscallsbrowserLoginfor two browser clients per test across several tests, blowing past the cap.Fix
Add a test-only env knob
PAD_DISABLE_RATE_LIMITS. When truthy,New()leavesServer.rateLimitersnil — andRateLimit()already treats nil as a pass-through (middleware_ratelimit.go:379), withStop()and the MCP path already nil-safe. Wired into the PlaywrightwebServer.env;run-pad.mjsspawns the binary with inherited env so it reaches the pad process.Limiters stay fully active in prod / self-host — the knob is an explicit opt-in only the E2E server sets.
Testing
collab-persistence.spec.tspasses locally with the fix (2/2, was failing on 429).TestRateLimit_DisabledByEnvpins the bypass; existing limiter tests still pass (limiters on when the env is unset).go build ./...green.Fixes BUG-2089.
https://claude.ai/code/session_015yuBJQYfDj95cgX3DaD8SF