[Eduard van Valkenburg (eavanvalkenburg)]

Motivation & Context

RedisHistoryProvider currently constructs and owns its Redis client, so applications cannot supply connection policies such as bounded timeouts and health checks, and cannot share an existing connection pool.

Description & Review Guide

• What are the major changes? Added a keyword-only caller-provided async Redis client option, validated its required decoded-string response mode, and preserved provider ownership only for internally constructed clients. Added focused lifecycle and validation tests plus Redis package documentation.
• What is the impact of these changes? Existing URL and credential-provider construction remains unchanged. Applications can now configure and own a Redis client while RedisHistoryProvider uses it without closing it. Retry behavior remains caller-controlled; the documentation notes that transcript appends are not idempotent across ambiguous write replays.
• What do you want reviewers to focus on? The connection-source validation, decode_responses=True invariant, and owned-versus-borrowed close behavior.

Related Issue

Fixes #8764

Contribution Checklist

• The code builds clean without any errors or warnings
• All unit tests pass, and I have added new tests where possible
• The PR follows the Contribution Guidelines
• This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
• This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

[Copilot]

Copilot review overview

🟡 Changes recommended

Retry-enabled clients can replay a committed transcript append and duplicate messages.

Review effort: Balanced
Findings: 1

Open (1)

• Transcript append is unsafe with retrying Redis clients · New

What changed in this PR

This PR lets RedisHistoryProvider borrow an application-owned Redis client, so applications can configure transcript connections without changing existing URL or credential-provider construction.

Changes:

• Add a keyword-only client option with connection-source and decoded-response validation.
• Close only provider-owned clients, and add validation and lifecycle tests.
• Document client configuration and ownership.

File	Description
python/​packages/​redis/​tests/​test_providers.py	Tests borrowed-client validation and close behavior.
python/​packages/​redis/​README.md	Shows how to configure and close a borrowed client.
python/​packages/​redis/​AGENTS.md	Documents the ownership contract.
python/​packages/​redis/​agent_framework_redis/​_history_provider.py	Adds client borrowing and conditional closing.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): 249d6ae14431
Model: gpt-5.6-sol

Overview

The PR cleanly separates owned and borrowed Redis clients, rejects ambiguous connection sources and incompatible response decoding, and adds focused ownership tests and documentation. One residual risk remains: accepting caller retry policies without making transcript writes idempotent can replay a committed append after an ambiguous timeout, duplicating conversation history.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/redis/agent_framework_redis/_history_provider.py

[Jose Alvarez (jpalvarezl)]

Reviewed the Redis client ownership and cleanup changes. The implementation, validation, documentation, and tests look good to me.