Skip to content

Python: validate non-optional union settings - #8926

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
1aifanatic:contrib/8907-settings-unions
Oct 1, 2026
Merged

Eduard van Valkenburg (eavanvalkenburg) merged 1 commit into
microsoft:mainfrom
1aifanatic:contrib/8907-settings-unions

Conversation

@1aifanatic

Copy link
Copy Markdown
Contributor

Motivation & Context

Settings declared as int | float currently retain raw environment or dotenv strings, including invalid values. This can defer configuration failures until agent execution instead of reporting the invalid setting at startup.

Description & Review Guide

  • What are the major changes? Apply the existing per-arm coercion logic to every union type, using the same union-origin check already used by settings validation. Add six regression cases covering integer/floating-point conversion and rejection from both environment and dotenv sources.
  • What is the impact of these changes? Valid numeric strings become the declared numeric type; invalid strings raise the existing field/source error without exposing the supplied value. No new API or dependency.
  • What do you want reviewers to focus on? Reuse of existing optional-union handling and preservation of its fallback/error behavior. All six regressions fail before the fix; the 124 settings tests and full core unit suite pass (8,074 passed, 19 skipped, 2 expected failures). Source Pyright, all five test-typing checkers, Ruff lint/format, and diff checks pass.

Related Issue

Fixes #8907. No competing open PR was found.

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.

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.

Copilot review overview

🟢 Approval recommended

The implementation correctly reuses existing union coercion behavior and includes focused regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes settings coercion for non-optional unions so invalid values fail during startup.

Changes:

  • Applies per-arm coercion to all union types.
  • Adds environment and dotenv regression coverage.
File Description
python/​packages/​core/​agent_framework/​_settings.py Extends union coercion beyond optional unions.
python/​packages/​core/​tests/​core/​test_settings.py Tests numeric coercion, errors, source reporting, and value masking.

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

@moonbox3
Evan Mattson (moonbox3) added this pull request to the merge queue Oct 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 1, 2026
Merged via the queue into microsoft:main with commit 4ce5ae3 Oct 1, 2026
52 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth — 9b1c232a Deployed Oct 1, 2026 by 1aifanatic via team_check #5721
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]: load_settings silently accepts invalid values for non-Optional union fields

4 participants