Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,24 @@

## Unreleased

### New public APIs

- **`IoRedisStateAdapter`**: `RedisStateAdapter` subclass defaulting to the
`ioredis_` lock-token prefix used by upstream Vercel Chat's `ioredis`-backed
state. Enables cross-runtime Redis sharing between TS and Python chat-sdk
deployments during migrations. Closes #71.
Note: the token *shape* after the prefix diverges intentionally — Python
emits `ioredis_{ms}_{hex32}` (`secrets.token_hex(16)`, CSPRNG) whereas
upstream emits `ioredis_{ms}_{base36<=13}` (`Math.random().toString(36)`,
not CSPRNG). Lock-release still works across runtimes because each
runtime generates its own token on acquire and `release_lock` / `extend`
compare the full token string — the divergence is observability-only
(log lines, bytes-in-Redis), not a functional incompatibility. We will
not regress to `Math.random()` for cosmetic byte-for-byte parity.
- **`RedisStateAdapter(token_prefix=...)`**: new `token_prefix` kwarg
(default `"redis"`). Parameterizes the lock-token prefix for observability
and interop.

### Test hygiene

- Sweep remaining `time.sleep` → `await asyncio.sleep` in async tests
Expand Down
1 change: 1 addition & 0 deletions docs/UPSTREAM_SYNC.md
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,7 @@ stay explicit instead of being rediscovered in code review.
| Teams divider rendering | `card_to_adaptive_card` hoists `separator: True` onto the next sibling (or emits a non-empty Container for a trailing divider) | `convertDividerToElement` emits an empty `Container` with `separator: True` | Upstream shares the same bug: Microsoft Teams renders an empty Container at zero height, so the separator line is effectively invisible. Python port fixes locally (issue #45) rather than blocking on upstream. |
| `SlackAdapter.current_token` / `current_client` | Public `@property` accessors that return the request-context-bound token and a preconfigured `AsyncWebClient` | Not exposed (`getToken()` is private on the TS `SlackAdapter`) | Python-only addition (issue #47). Downstream code that calls Slack Web APIs from inside a handler — email resolution, user profile fetches, reaction bookkeeping — otherwise depends on underscore-prefixed helpers. |
| `ConcurrencyConfig.max_concurrent` | Enforced via `asyncio.Semaphore` in the `"concurrent"` strategy path; rejects non-integer or `<= 0` values, and rejects any non-`None` `max_concurrent` paired with a non-`"concurrent"` strategy | Accepted into the config type with docstring "Default: Infinity" but never read (3 writes, 0 reads) | Silent correctness bug upstream — consumers setting `max_concurrent=N` with `strategy="concurrent"` reasonably expect an N-way bound on in-flight handlers. We honor the documented contract via a semaphore and fail-fast on misconfiguration so it's never silent. `max_concurrent=None` stays compatible with every strategy (unbounded default). |
| Redis lock token format | `{token_prefix}_{ms}_{secrets.token_hex(16)}` — always 32 hex chars, CSPRNG-sourced | `ioredis_${Date.now()}_${Math.random().toString(36).substring(2, 15)}` — base36, ≤13 chars, **not** CSPRNG | Interop via `IoRedisStateAdapter(token_prefix="ioredis")` still works for lock-release (release/extend compare by full-string equality, and each runtime only releases what it issued), but the token byte-shape diverges. Intentional — CSPRNG should not be regressed to `Math.random()` for cosmetic byte-for-byte compatibility. |

### Platform-specific gaps

Expand Down
7 changes: 6 additions & 1 deletion src/chat_sdk/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@

from chat_sdk.state.memory import MemoryStateAdapter, create_memory_state
from chat_sdk.state.postgres import PostgresStateAdapter, create_postgres_state
from chat_sdk.state.redis import RedisStateAdapter, create_redis_state
from chat_sdk.state.redis import (
IoRedisStateAdapter,
RedisStateAdapter,
create_redis_state,
)

__all__ = [
"IoRedisStateAdapter",
"MemoryStateAdapter",
"PostgresStateAdapter",
"RedisStateAdapter",
Expand Down
63 changes: 59 additions & 4 deletions src/chat_sdk/state/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
# ---------------------------------------------------------------------------


def _generate_token() -> str:
return f"redis_{int(time.time() * 1000)}_{secrets.token_hex(16)}"
def _generate_token(prefix: str = "redis") -> str:
return f"{prefix}_{int(time.time() * 1000)}_{secrets.token_hex(16)}"


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -88,6 +88,14 @@ class RedisStateAdapter:
across multiple server instances.

Implements the full :class:`~chat_sdk.types.StateAdapter` protocol.

Lock tokens are generated with the shape ``{token_prefix}_{ms}_{hex16}``.
The prefix defaults to ``"redis"``, matching this port's historical
behavior. Set ``token_prefix="ioredis"`` (or use :class:`IoRedisStateAdapter`)
when sharing a Redis with a TypeScript Vercel Chat deployment whose
``ioredis``-backed state emits ``ioredis_`` tokens. Release and extend
semantics compare the full token string, so the prefix is an observability
and migration aid, not a security boundary.
"""

def __init__(
Expand All @@ -96,8 +104,10 @@ def __init__(
url: str | None = None,
client: Any | None = None,
key_prefix: str = "chat-sdk",
token_prefix: str = "redis",
) -> None:
self._key_prefix = key_prefix
self._token_prefix = token_prefix
self._connected = False
self._connect_lock = asyncio.Lock()

Expand Down Expand Up @@ -162,7 +172,7 @@ async def is_subscribed(self, thread_id: str) -> bool:
async def acquire_lock(self, thread_id: str, ttl_ms: int) -> Lock | None:
self._ensure_connected()

token = _generate_token()
token = _generate_token(self._token_prefix)
lock_key = self._key("lock", thread_id)

# SET NX PX for atomic lock acquisition
Expand Down Expand Up @@ -341,6 +351,41 @@ def _ensure_connected(self) -> None:
raise StateNotConnectedError("RedisStateAdapter")


# ---------------------------------------------------------------------------
# ioredis-compatible subclass
# ---------------------------------------------------------------------------


class IoRedisStateAdapter(RedisStateAdapter):
""":class:`RedisStateAdapter` variant that emits ``ioredis_``-prefixed lock tokens.

Use this when sharing a Redis with a TypeScript Vercel Chat deployment whose
``ioredis``-backed state adapter writes ``ioredis_``-prefixed lock tokens.
Behavior is otherwise identical to :class:`RedisStateAdapter` — same Lua
scripts, same key layout, same ``SET NX PX`` acquisition, same ``PEXPIRE``
extend. Tokens compare by full string equality, so the prefix is an
observability and migration aid for cross-runtime Redis sharing, not a
security boundary.

Note: the token suffix shape diverges intentionally. Python emits
``ioredis_{ms}_{secrets.token_hex(16)}`` (32 hex chars, CSPRNG), whereas
upstream emits ``ioredis_${Date.now()}_${Math.random().toString(36).substring(2, 15)}``
(base36, ≤13 chars, **not** CSPRNG). Interop for lock-release is
unaffected — each runtime issues and releases its own tokens and the
comparison is by full string equality — but the bytes in Redis and in
log lines differ. We do not regress to ``Math.random()`` for cosmetic
byte-for-byte parity; see ``docs/UPSTREAM_SYNC.md`` Known Non-Parity.

Enables migrations like "drain TS, then flip Python on" with tokens
identifiable by runtime-of-origin at a glance — e.g. reading a lock's
value via ``GET chat-sdk:lock:<thread_id>``, or in any logs/trace IDs
where the token is surfaced on release/extend failures.
"""

def __init__(self, *, token_prefix: str = "ioredis", **kwargs: Any) -> None:
super().__init__(token_prefix=token_prefix, **kwargs)
Comment on lines +385 to +386
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The create_redis_state factory function (located at the end of this file) should be updated to accept and forward the new token_prefix parameter to the RedisStateAdapter constructor. This ensures consistency between the class and its factory method, allowing users of the factory to leverage the new parameterization.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 43f4f40create_redis_state() now accepts and forwards token_prefix (and tests cover default + override). Thanks.



# ---------------------------------------------------------------------------
# Factory
# ---------------------------------------------------------------------------
Expand All @@ -351,10 +396,20 @@ def create_redis_state(
url: str | None = None,
client: Any | None = None,
key_prefix: str = "chat-sdk",
token_prefix: str = "redis",
) -> RedisStateAdapter:
"""Create a new Redis state adapter.

Either provide a ``url`` or an existing ``client``. If neither is given,
the ``REDIS_URL`` environment variable is used.

``token_prefix`` defaults to ``"redis"`` to match
:class:`RedisStateAdapter`'s default; pass ``"ioredis"`` (or instantiate
:class:`IoRedisStateAdapter`) for TS interop.
"""
return RedisStateAdapter(url=url, client=client, key_prefix=key_prefix)
return RedisStateAdapter(
url=url,
client=client,
key_prefix=key_prefix,
token_prefix=token_prefix,
)
Loading
Loading