Skip to content

Fix Redis state typings for url and client options#303

Open
haydenbleasel wants to merge 1 commit intovercel:mainfrom
haydenbleasel:codex/fix-redis-adapter-props
Open

Fix Redis state typings for url and client options#303
haydenbleasel wants to merge 1 commit intovercel:mainfrom
haydenbleasel:codex/fix-redis-adapter-props

Conversation

@haydenbleasel
Copy link
Contributor

@haydenbleasel haydenbleasel commented Mar 26, 2026

Summary

  • fix createRedisState typings so url appears in IntelliSense for createRedisState({ url: "..." })
  • add an explicit factory options type and keep REDIS_URL env fallback
  • support passing an existing Redis client and avoid closing caller-owned clients
  • add tests for explicit url usage and existing-client usage
  • include a patch changeset for @chat-adapter/state-redis

Test plan

  • Ran pnpm build and pnpm test, both successful

@vercel
Copy link
Contributor

vercel bot commented Mar 26, 2026

@haydenbleasel is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@bensabic bensabic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good overall, but I think the injected redis client path in connect() needs one follow-up. Right now it treats client.isOpen as enough to mark the adapter connected, but in node-redis v5 isOpen only means the socket is open, not that the client is ready to accept commands. The docs distinguish this from client.isReady, and isOpen can still be true while the client is connecting or reconnecting. That means await adapter.connect() can resolve too early for createRedisState({ client }), and the first Redis command can still race startup/reconnect.

Could we gate on client.isReady or wait for the ready event in the already-open case, and add a test that covers an injected client that is open but not yet ready?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants