Skip to content

Python: Skip Claude client settings when a client is injected - #9984

Open
ZedingZhang (ZedingZhang) wants to merge 1 commit into
microsoft:mainfrom
ZedingZhang:fix/claude-injected-client-settings-9960
Open

ZedingZhang (ZedingZhang) wants to merge 1 commit into
microsoft:mainfrom
ZedingZhang:fix/claude-injected-client-settings-9960

Conversation

@ZedingZhang

@ZedingZhang ZedingZhang (ZedingZhang) commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Motivation & Context

RawClaudeAgent(client=...) still loads client-construction settings, so an unrelated value such as CLAUDE_AGENT_MAX_TURNS=not-an-int prevents use of a preconfigured SDK client. The injected-client path never uses those settings.

Description & Review Guide

  • What are the major changes? Resolve ClaudeAgentSettings only when the framework owns client creation. Add eight parameterized regression/control cases covering both RawClaudeAgent and ClaudeAgent, and clarify the constructor's injected-client contract.
  • What is the impact of these changes? Injected SDK clients bypass client settings and .env loading. Framework instructions and tool normalization still run, and the injected client remains caller-owned.
  • What do you want reviewers to focus on? Tests cover invalid numeric environment values, missing .env files, ignored construction options, client reuse and cleanup, and explicit argument > selected file > environment precedence for framework-created clients. Six cases fail on the original implementation and all eight pass with the fix.

Related Issue

Fixes #9960. The issue has the ready-for-implementation label; no competing open PR was found before submission.

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution.

AI assistance details: Codex assisted with investigation, implementation, tests, the constructor docstring, and this PR description. Codex executed the reported checks; I reviewed all submitted content and take responsibility for it.

Contribution Checklist

  • The code builds clean without any errors or warnings — Claude sdist and wheel built successfully.
  • All unit tests pass, and I have added new tests where possible — Claude unit suite: 100 passed. Ruff formatting/lint, strict source Pyright, and all five test type checkers pass. Live Claude service calls were not tested.
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • 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 AI left a comment

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.

🟢 Approval recommended

No active review issues are reported for the change to bypass client-construction settings when a preconfigured Claude client is injected.

No open issues 🎉

What changed in this PR

This PR fixes #9960 by skipping client-construction settings when a preconfigured Claude SDK client is injected, while preserving settings resolution for framework-created clients.

Changes:

  • Load ClaudeAgentSettings only when the framework owns client creation.
  • Add eight parameterized regression and control cases covering settings bypass, caller ownership, and settings precedence.
  • Clarify that injected clients remain caller-owned and bypass client settings and .env loading.
File Description
python/​packages/​claude/​tests/​test_claude_agent.py Adds injected-client regression tests and framework-created client controls for both agent classes.
python/​packages/​claude/​agent_framework_claude/​_agent.py Guards settings loading by client ownership and documents the injected-client contract.

Review effort: Balanced

This branch was previously deployed

1 inactive deployment
github-app-auth — 366ac2ca Deployed Nov 2, 2026 by ZedingZhang via add_label #26871
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Claude Agent reads environment configuration before using an injected client

2 participants