Python: fix(redis): scope RedisHistoryProvider keys by source_id - #7494
Open
he-yufeng wants to merge 2 commits into
Open
Python: fix(redis): scope RedisHistoryProvider keys by source_id#7494he-yufeng wants to merge 2 commits into
he-yufeng wants to merge 2 commits into
Conversation
Two providers with different source_ids but the same key_prefix shared one Redis list per session, so a write-only audit sink contaminated the primary provider's loaded history, and clear() on one deleted the other's conversation. The key now includes source_id, matching the Cosmos provider's scoping. Existing keys written under the old layout are left in place; deleting them would risk removing a sibling provider's data, and they simply become unreadable by the new code.
he-yufeng
temporarily deployed
to
github-app-auth
August 3, 2026 21:39 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
he-yufeng
temporarily deployed
to
github-app-auth
August 3, 2026 21:39 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
he-yufeng
temporarily deployed
to
github-app-auth
August 3, 2026 21:39 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
he-yufeng
temporarily deployed
to
github-app-auth
August 3, 2026 21:39 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
github-actions
Bot
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a Redis key-collision bug in the Python RedisHistoryProvider by scoping stored message lists by source_id, preventing multiple providers with the same key_prefix from contaminating or deleting each other’s session history (as described in #7471).
Changes:
- Updated
RedisHistoryProviderRedis key layout to includesource_id({key_prefix}:{source_id}:{session_id|default}). - Updated and extended Redis provider tests to reflect the new key format and to verify per-
source_idisolation and safeclear()behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/redis/agent_framework_redis/_history_provider.py | Includes source_id in the Redis storage key to isolate histories across provider instances. |
| python/packages/redis/tests/test_providers.py | Updates key-format assertions and adds coverage proving source_id isolation and non-destructive clear(). |
Comment on lines
+109
to
+111
| def _redis_key(self, session_id: str | None) -> str: | ||
| """Get the Redis key for a given session's messages.""" | ||
| return f"{self.key_prefix}:{session_id or 'default'}" | ||
| return f"{self.key_prefix}:{self.source_id}:{session_id or 'default'}" |
Contributor
Author
There was a problem hiding this comment.
Good catch. Colon-joined segments are ambiguous whenever one of them carries a colon, so the separator is now the ASCII unit separator in d309e32. Same migration story as before: keys written with the colon layout stay in place and become unreadable by the new code.
| with patch("agent_framework_redis._history_provider.redis.from_url") as mock_from_url: | ||
| mock_from_url.return_value = mock_redis_client | ||
| audit = RedisHistoryProvider("audit", redis_url="redis://localhost:6379") | ||
| primary = RedisHistoryProvider("primary", redis_url="redis://localhost:6379") |
Contributor
Author
There was a problem hiding this comment.
Fixed in d309e32 by making the variable load-bearing: the test now also asserts the delete was NOT issued for the other provider's key, so primary is part of the expectation.
…t in tests Colon-joined keys were ambiguous for source ids or session ids containing a colon (a:b + c vs a + b:c). Join with the ASCII unit separator instead. The clear-isolation test now asserts on the other provider's key too, so the unused variable lint is gone as well.
he-yufeng
temporarily deployed
to
github-app-auth
August 4, 2026 01:29 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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.
Closes #7471.
_redis_keynow readskey_prefix:source_id:session_id, so two providers sharing a key_prefix stop sharing a Redis list: the audit sink's copies no longer load back into the primary provider's context, andclear()on one can no longer wipe the other's session. This matches howCosmosHistoryProviderscopes everything by source_id.On compatibility: keys written under the old layout stay in Redis but become unreadable by the new code. I deliberately did not make
clear()delete the old shared key, since that key can hold a sibling provider's history and deleting it would reproduce the exact cross-provider destruction this fixes. A leftover key per session is harmless beyond the storage; admins can expire it manually.Tests: the key-format and trim/clear assertions moved to the new layout, plus two new cases proving keys differ per source_id and that clearing one provider leaves the other provider's list untouched. 57/57 in the redis package suite pass locally.