fix(redis): wait out the cold-start connect before failing closed by polylane[bot] · Pull Request #60 · gilsmt/cache-app

polylane · 2026-09-19T07:39:59Z

Fixes: cache-app: Redis is an unmonitored single dependency and its fail-open paths silently remove rate limiting

When Redis is configured but its connection is still being established, the app treated that connecting state as an outage and rejected MCP tool calls and link-reachability probes, so the first call handled by every fresh instance failed. The change waits briefly for the connection to settle before deciding Redis is unavailable, so a cold start succeeds while a genuine outage still fails closed. Deployments without REDIS_URL are unchanged.

flowchart TD
  A["First call on a fresh instance"] --> B["getRedisClient: socket still connecting, isReady false"]
  B --> C["returns null"]
  C --> D["fail-closed caller reads null as an outage"]
  D --> E["MCP tool call or link probe refused"]
  B -.-> F["fix: getReadyRedisClient waits up to 1s for the connect"]
  F --> G["connect settles: ready client returned"]
  F -.-> H["still not ready: unavailable, fail closed"]

What caused this

Affected: int_0b563423d00164u048hh0syk

Why this fix

The review is correct, and the read is confirmed in the client library. getRedisClient returns null whenever the client's socket is not ready, and node-redis isReady reflects socket readiness while connect() settles only once the socket is ready. On a fresh instance the first call therefore always arrived before readiness, and the new fail-closed branches read that null as an outage: the MCP tool call was rejected and the first link-probe batch was refused. The transient connects in milliseconds; a genuine outage does not.

This revision separates the two states. Callers that bound abuse now go through a readiness-aware accessor that waits, bounded to one second, for an in-flight connect to settle before deciding Redis is unavailable. A cold start resolves to a usable client; a Redis that is genuinely down or reconnecting still resolves to unavailable and still fails closed, so the throttle on a stolen MCP token and the outbound-probe budget are not weakened. The bound keeps a down dependency from stalling the request.

Two supporting details changed with it. The degraded-state warning no longer fires during the initial connect, so a normal cold start is not logged as an outage; only the loss of an established connection warns. The cache path is untouched and still degrades silently, which is correct for a cache.

flowchart TD
  A["First call on a fresh instance"] --> B["getRedisClient: socket still connecting, isReady false"]
  B --> C["returns null"]
  C --> D["fail-closed caller reads null as an outage"]
  D --> E["MCP tool call or link probe refused"]
  B -.-> F["fix: getReadyRedisClient waits up to 1s for the connect"]
  F --> G["connect settles: ready client returned"]
  F -.-> H["still not ready: unavailable, fail closed"]

Remaining and outside this diff: the Redis endpoint still has no health check or alert, and the warning this adds is only usable if the runtime log stream is collected, which this account cannot read.

Out of scope

Redis endpoint monitoring and alerting: no code change here can create a monitor; the connected account exposes no queryable metrics and no log drain.
Whether REDIS_URL points at a single instance with no replica or failover: the environment value is not readable from this workspace.

7 files changed (+396/-72)

app/mcp/route.ts: modified, +26/-11
lib/collections/link-reachability.test.ts: added, +47/-0
lib/collections/link-reachability.ts: modified, +25/-6
lib/common/redis.test.ts: added, +62/-0
lib/common/redis.ts: modified, +116/-10
lib/integrations/mcp/rate-limit.test.ts: added, +75/-0
lib/integrations/mcp/rate-limit.ts: modified, +45/-45

Generated by Polylane. You can ask follow-ups by mentioning @polylane in a comment.

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>

coderabbitai · 2026-09-19T07:40:03Z

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 7f0b6828-ee37-4786-9b3b-9708bc9633f4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

vercel · 2026-09-19T07:40:04Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
cache-app	Ready	Preview	Sep 19, 2026 12:08pm UTC

polylane · 2026-09-19T07:44:13Z

Note

Superseded by a newer Polylane verdict for 29f4d16.

Caution

Hold this merge. Moderate impact.
getRedisClient() returns null while the client it just created is still connecting, and the new fail-closed branch reads that as a Redis outage whenever REDIS_URL is set — so the first MCP tool call handled by every fresh serverless instance now errors, and the first check-links invocation refuses its probes. Await the connect (or expose a connection state) before failing closed.

View the full analysis →

Why. getRedisClient() returns null while the client it just created is still connecting, and the new fail-closed branch in checkMcpRateLimit (lib/integrations/mcp/rate-limit.ts:62-67) reads that null as an outage whenever REDIS_URL is set. The MCP limiter is only reached from authorizeToolCall for tools/call, so the call that creates the Redis client is itself a tool call: the first MCP tool call handled by every fresh serverless instance now returns an error where main served it, and lib/collections/link-reachability.ts:111-118 refuses probes on the same state.

flowchart LR
  A["first tools/call on a fresh instance"] --> B["getRedisClient()"]
  B --> C{"client exists?"}
  C -->|"no"| D["createClient + connect(), not awaited"]
  D --> E["isReady false -> null"]
  C -->|"yes, not ready"| E
  E --> F{"REDIS_URL set?"}
  F -->|"yes"| G["unavailable -> tool call errors"]
  F -->|"no"| H["allowed"]

Reaches cache-app, plus 3 downstream resources (~28% of observed traffic).

Before merging: Fail closed only for an established client that has disconnected: await connect() in getRedisClient() (or expose a connection state so "connecting" is not "unreachable"), keeping a real outage fail-closed.

Evidence · 5 steps

Trigger: the first MCP tools/call (or first check-links invocation) handled by a freshly created serverless instance while REDIS_URL is set — the call that creates the Redis client always observes isReady=false (Deterministic, 1 rejection per fresh instance (lib/common/redis.ts:107-117 returns null after starting connect()), not a measured rate: Vercel's metrics API returned 'the team does not have Observability Plus' and runtime function logs are not exposed, so no production numerator/denominator exists; the deployment event stream is the only readable source.)

getRedisClient() creates the client, starts connect() without awaiting it, and returns null because isReady is still false — so the creating call never receives a client. _{lib/common/redis.ts:107-117 (resulting code at head 011e7c8): connect() fired without await, then if (globalRedisClient.isReady) ... return null.}
In the /mcp function the only Redis caller is the rate limiter, reached from authorizeToolCall for tools/call; initialize and tools/list never touch Redis, so nothing warms the client before the first tool call. _{app/mcp/route.ts:120-128 calls checkMcpRateLimit; rg over the repo shows getRedisClient callers only at lib/integrations/mcp/rate-limit.ts:62, lib/collections/link-reachability.ts:111 and the lazy-loaded app/api/preview/route.ts.}
null plus REDIS_URL set is mapped to status 'unavailable'. _{lib/integrations/mcp/rate-limit.ts:62-67 (!redis -> isRedisConfigured() ? {status:"unavailable"} : {status:"allowed"}).}
The route returns isError:true, so the tool call fails where main let it through. _{app/mcp/route.ts:131-132 and 153-155; the diff removes main's fail-open incrementMcpRateCounter/isOverLimit pair where a null decision returned false.}
The same not-ready state refuses the link-probe budget instead of falling back to the in-process Map. _{lib/collections/link-reachability.ts:111-119 (null + isRedisConfigured() -> {allowed:false, retryAfterMs:PROBE_BUDGET_WINDOW_MS}) and 142-148.}

Traffic: Observability gap: Vercel metrics unavailable for this workspace (returned reason: no Observability Plus) and runtime function log lines are not exposed; readable evidence is the current production deployment manifest of dpl_5XnF2219xJ3VhHRP4DumSDnHT5sR, which lists /mcp and /mcp/prompt as dynamic functions, and the project's env keys, which include REDIS_URL. The likelihood rests on the deterministic per-instance code path rather than measured request rates.

Blast radius · 3 resources

Resource	Via	Hops	Volume
`cache-app`	contains	1	3.12 digest:log_events_per_hour
`automationRunWorkflow`	contains	1	0.08 digest:log_events_per_hour
`automationRunWorkflow`	contains	1	0 digest:log_events_per_hour

Also considered · 2 refuted

Refuted · Fail-closed leaves MCP calls unserved for the whole of a real Redis outage · This is the change's declared purpose, not an unintended regression: the module documents fail-closed for a configured-but-unreachable Redis (lib/integrations/mcp/rate-limit.ts:15-21) and the linked issue asks for exactly that.
Refuted · Redis-less deployments begin denying MCP calls and probes · isRedisConfigured() reads process.env.REDIS_URL (lib/common/redis.ts:28-30) and the null branch returns 'allowed' when it is absent (rate-limit.ts:63-66); link-reachability keeps tryConsumeLocalProbeBudget at link-reachability.ts:119, and the added tests assert both branches for the Redis-less case.

Vercel build route manifest — /mcp deployed as a dynamic function · dpl_5XnF2219xJ3VhHRP4DumSDnHT5sR · production build log

├ ƒ /mcp

Full log (3 of 100 lines)

├ ƒ /mcp
├ ƒ /mcp/prompt
ƒ (Dynamic)            server-rendered on demand

Analysed against 1 Project and 1 repository

Project: cache-app
Repository: gilsmt/cache-app

_{Polylane analysed 011e7c8 for production impact. You can ask follow-ups by mentioning @polylane in a comment.}

Did this help? React 👍 or 👎 so the next review is sharper.

polylane

Suggested observability improvements for code this pull request changes. They are advisory and never affect the production-impact verdict.

polylane · 2026-09-19T07:44:15Z

+        };
+    }
+    if (rateLimit.status === "unavailable") {
+        return { result: rateLimitUnavailableResult() };


The new rejection path logs nothing identifying which user's call was refused, so an operator cannot tell whose traffic is being turned away during the outage.

Suggested change

return { result: rateLimitUnavailableResult() };

return { result: rateLimitUnavailableResult(auth.userId) };

polylane · 2026-09-19T07:44:16Z

+function rateLimitUnavailableResult(): CallToolResult {
+    log.warn("rate limit unavailable; rejecting request");


Adds the user identifier to the rate-limit-unavailable event so rejected calls can be attributed to a user after the fact.

Suggested change

function rateLimitUnavailableResult(): CallToolResult {

log.warn("rate limit unavailable; rejecting request");

function rateLimitUnavailableResult(userId: string): CallToolResult {

log.warn("rate limit unavailable; rejecting request", { userId });

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>

polylane · 2026-09-19T07:55:27Z

Verdict: the review is correct, and the pull request is updated

The finding holds against the code. getRedisClient() creates the client, starts connect() without awaiting it, and then returns null because isReady is still false — node-redis 6.2.1 reports isReady from socket readiness and connect() settles only once that socket is ready. So the first call handled by every fresh serverless instance arrived before readiness, and the new fail-closed branches read that null as an outage: the first MCP tool call was rejected with "Rate limiting is unavailable right now" and the first link-probe batch was refused. The transient connects in milliseconds; a real outage does not.

What changed

lib/common/redis.ts now exposes a readiness-aware accessor for callers that must fail closed. It waits, bounded to one second, for the in-flight connect to settle before deciding Redis is unavailable. A cold start resolves to a usable client; a Redis that is genuinely down or reconnecting still resolves to unavailable and still fails closed, so neither the throttle on a stolen MCP token nor the outbound-probe budget is weakened. The bound keeps a down dependency from stalling the request.
The degraded-state warning no longer fires during the initial connect, so a normal cold start is not logged as an outage; only the loss of an established connection warns.
lib/integrations/mcp/rate-limit.ts and lib/collections/link-reachability.ts use the readiness-aware accessor. The cache path in the preview route is untouched and still degrades silently, which is correct for a cache, and a deployment with no REDIS_URL short-circuits before any wait and keeps its prior behavior.

Validation

bun run lint (ultracite check): clean, 357 files.
bunx next typegen && bun run type-check (tsc --noEmit): clean.
bun test: 138 pass, 0 fail across 16 files. The new lib/common/redis.test.ts asserts the call does not settle while the client is connecting and then resolves to the ready client once the socket becomes ready; the consumer tests assert fail-closed (unavailable) when Redis is configured but unreachable and fail-open when it was never configured.

Remaining work, unchanged by this diff: the Redis endpoint still has no health check or alert, and the warning now emitted is only usable if the runtime log stream is collected, which this account cannot read (no queryable metrics, no log drain permission). Whether REDIS_URL points at a single instance with no replica or failover remains unverified because the environment value is not readable from here. Reviewers merge the pull request; I do not.

Pushed in 29f4d16.

polylane · 2026-09-19T07:57:23Z

Tip

Expected to resolve the linked issue.

Linked issue: iss_0b88eaf19001i1vfn6j66obs — cache-app: Redis is an unmonitored single dependency and its fail-open paths silently remove rate limiting

The concern raised on an earlier push appears addressed as of 29f4d16.

Delta over 29f4d16 adds only userId/bucket/amount fields to existing warn logs — no request-path change. Vercel's metrics API returns nothing for this account; the head build completed clean and cache-app's log digest shows 0 error events/hour (~72h).

View the full analysis →

Also considered · 1 plausible, 4 refuted

Plausible · Cold-start connect may settle before isReady, so a fresh instance's first abuse-bounded call still fails closed · possible, low
Refuted · Fail-closed abuse bounds reject MCP calls and refuse link probes during a Redis outage · The refusal is the declared purpose of the issue this PR fixes (iss_0b88eaf19001i1vfn6j66obs: fail-open paths silently remove rate limiting) — it swaps a silent loss of throttling for a retryable per-call refusal, and the chain from 'merge' to 'degraded production' therefore needs an outage the readable baseline does not show: cache-app's log digest reports 0 error events/hour and 0.06 warn events/hour across the window, with no Redis-outage signature, and each refusal is bounded and retryable with no data loss.
Refuted · Removed rate-limit exports and the renamed probe-budget helper break a consumer during the rolling deploy · No consumer exists outside the changed files (rg returns only the changed modules plus the unchanged preview route that uses getRedisClient), the workspace has exactly one code repository, and the head commit built clean with 0 errors (dpl_3r4hNAN88PrbZ89ekDwMkK37UsXY).
Refuted · The new warn-log fields (userId, bucket, amount) alter behaviour or leak sensitive data · A log payload cannot alter control flow, the returned CallToolResult text is unchanged, and no credential, token, or request body is added — the new fields are an internal user id and a bucket name already logged by the same module.
Refuted · The bounded ready-wait adds latency or stacks requests against a down Redis · The wait is bounded at 1 s and paid at most once per fresh instance while a connect is pending; in steady state the settled connect promise makes it return immediately, so an outage costs one refusal per call rather than accumulating latency, and no extra command reaches Redis.

cache-app head deployment build events · dpl_3r4hNAN88PrbZ89ekDwMkK37UsXY · branch polylane/autofix/hzqaqasosy02 @ 1ee1cfc

Build Completed in /vercel/output [36s]
Deployment completed

Full log (9 of 192 lines)

├ ƒ /mcp
├ ƒ /mcp/prompt
○  (Static)             prerendered as static content
ƒ  (Dynamic)            server-rendered on demand
Build Completed in /vercel/output [36s]
Deploying outputs...
Deployment completed
Creating build cache...
Created build cache: 1m

Analysed against 1 Project and 1 repository

Project: cache-app
Repository: gilsmt/cache-app

_{Polylane analysed 1ee1cfc for production impact. You can ask follow-ups by mentioning @polylane in a comment.}

Rate this review: 👍 helpful · 👎 not helpful · 😕 confusing

Tip

Improvements: #63. Merge it into your branch before merging this pull request.

Previous verdicts (2)

Head	Verdict	Analysis
`29f4d16`	Expected to resolve the linked issue	analysis
`011e7c8`	Hold this merge	analysis

polylane

Suggested observability improvements for code this pull request changes. They are advisory and never affect the production-impact verdict.

polylane · 2026-09-19T07:57:27Z

+            : { status: "allowed" };
+    } catch (error) {
+        log.warn("MCP rate limit counter failed; failing closed", {
+            bucket: bucket.name,


The fail-closed counter log records the bucket and error but not which token's user hit it, so an outage's blast radius on the MCP surface cannot be attributed from the event.

Suggested change

bucket: bucket.name,

bucket: bucket.name,

userId,

polylane · 2026-09-19T08:07:41Z

Polylane opened #62 with observability improvements for the code this pull request changes.

It targets this pull request's branch, so you can merge it into your branch to fold the improvements in before this pull request lands. Polylane keeps it updated as this pull request evolves. If the changes are not wanted, close it and Polylane will not open another one for this pull request.

…probe count (#62) Co-authored-by: Gilberto <37253958+gilsmt@users.noreply.github.com> Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>

polylane

Suggested observability improvements for code this pull request changes. They are advisory and never affect the production-impact verdict.

polylane · 2026-09-19T12:11:53Z

+    bucket: { name: string },
+    retryAfterSeconds: number
+): CallToolResult {
+    log.warn(`rate limit hit (${bucket.name})`, { retryAfterSeconds });


The bucket name is interpolated into the message today, so read and write limit hits cluster as two separate templates; passing it as a field makes rate-limit hits countable and filterable by bucket.

Suggested change

log.warn(`rate limit hit (${bucket.name})`, { retryAfterSeconds });

log.warn("rate limit hit", { bucket: bucket.name, retryAfterSeconds });

polylane · 2026-09-19T12:22:06Z

Polylane opened #63 with observability improvements for the code this pull request changes.

It targets this pull request's branch, so you can merge it into your branch to fold the improvements in before this pull request lands. Polylane keeps it updated as this pull request evolves. If the changes are not wanted, close it and Polylane will not open another one for this pull request.

fix(redis): fail closed on abuse bounds during a Redis outage

011e7c8

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>

polylane Bot added polylane severity:medium Polylane autofix severity: medium labels Sep 19, 2026

polylane Bot requested a review from gilsmt September 19, 2026 07:40

vercel Bot deployed to Preview September 19, 2026 07:41 View deployment

polylane Bot commented Sep 19, 2026

View reviewed changes

fix(redis): wait for the cold-start connect before failing closed

29f4d16

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>

polylane Bot changed the title ~~fix(redis): fail closed on abuse bounds during a Redis outage~~ fix(redis): wait out the cold-start connect before failing closed Sep 19, 2026

vercel Bot deployed to Preview September 19, 2026 07:56 View deployment

polylane Bot commented Sep 19, 2026

View reviewed changes

polylane Bot mentioned this pull request Sep 19, 2026

fix(observability): attribute outage refusals to caller, bucket, and probe count #62

Merged

fix(observability): attribute outage refusals to caller, bucket, and …

1ee1cfc

…probe count (#62) Co-authored-by: Gilberto <37253958+gilsmt@users.noreply.github.com> Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>

vercel Bot deployed to Preview September 19, 2026 12:08 View deployment

polylane Bot commented Sep 19, 2026

View reviewed changes

polylane Bot mentioned this pull request Sep 19, 2026

fix(redis): log the ready-wait outcome when a caller is refused #63

Open

	return { result: rateLimitUnavailableResult() };
	return { result: rateLimitUnavailableResult(auth.userId) };

		function rateLimitUnavailableResult(): CallToolResult {
		log.warn("rate limit unavailable; rejecting request");

	log.warn(`rate limit hit (${bucket.name})`, { retryAfterSeconds });
	log.warn("rate limit hit", { bucket: bucket.name, retryAfterSeconds });

Conversation

polylane Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What caused this

Why this fix

Uh oh!

coderabbitai Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

vercel Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

polylane Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

polylane Bot left a comment

Choose a reason for hiding this comment

Uh oh!

polylane Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

polylane Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

polylane Bot commented Sep 19, 2026

Verdict: the review is correct, and the pull request is updated

Uh oh!

polylane Bot commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

polylane Bot left a comment

Choose a reason for hiding this comment

Uh oh!

polylane Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

polylane Bot commented Sep 19, 2026

Uh oh!

polylane Bot left a comment

Choose a reason for hiding this comment

Uh oh!

polylane Bot Sep 19, 2026

Choose a reason for hiding this comment

Uh oh!

polylane Bot commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

polylane Bot commented Sep 19, 2026 •

edited

Loading

coderabbitai Bot commented Sep 19, 2026 •

edited

Loading

vercel Bot commented Sep 19, 2026 •

edited

Loading

polylane Bot commented Sep 19, 2026 •

edited

Loading

polylane Bot commented Sep 19, 2026 •

edited

Loading