Fix a365.config.json missing user principal name#13
Merged
sellakumaran merged 4 commits intomainfrom Nov 15, 2025
Merged
Conversation
Enhanced logging with detailed summaries, actionable error messages, and refined log levels. Improved resilience by wrapping critical operations in try-catch blocks and adding fallback instructions. Enhanced validation for Service Principals and OAuth2 permissions with clearer error messages. Added comprehensive test coverage for dry-run execution, error handling, and setup summaries. Refactored repetitive logging patterns and improved code readability. Updated test framework to use FluentAssertions and better mock dependencies.
Enhanced error handling for installation issues, including fixes for PATH configuration. Updated `ConfigCommand` to enforce strict separation of static and dynamic properties in `a365.config.json`. Improved `SetupCommand` with better logging, inheritable permissions checks, and clearer user guidance. Refactored `SetupResults` to include new properties for tracking setup status. Adjusted logging levels in `A365SetupRunner` and `InteractiveGraphAuthService` for better clarity. Added regression tests to ensure static/dynamic property separation and validated `GetStaticConfig()` and `GetGeneratedConfig()` methods. Improved inline documentation and logging for maintainability.
- Made `AgentUserPrincipalName` immutable in `Agent365Config.cs`. - Updated `CleanupCommand` to accept `graphApiService` in `Program.cs`. - Refactored `CreateFederatedIdentityCredentialAsync` in `A365SetupRunner.cs`: - Made `graphToken` a required parameter. - Added fallback for multiple Graph API endpoints. - Introduced exponential backoff for retries. - Improved error handling and logging. - Added `ConsistencyLevel` header for eventual consistency. - Replaced `LocalAppData` with a global config directory in `ConfigService.cs`. - Adjusted logging level for transient errors in `DelegatedConsentService.cs`.
Josina20
previously approved these changes
Nov 15, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical bug where AgentUserPrincipalName and other dynamic properties were being incorrectly saved to a365.config.json. The fix enforces strict separation between static configuration (saved to a365.config.json) and dynamic/generated properties (saved to a365.generated.config.json).
Key Changes:
- Changed
AgentUserPrincipalNamefrom mutable (set) to init-only (init) property to ensure it's treated as static configuration - Modified
ConfigCommandto useGetStaticConfig()when saving configuration, preventing dynamic properties from leaking into static config file - Added comprehensive test coverage to prevent regression of this bug
- Improved logging levels (changed warnings to info/debug where appropriate)
- Enhanced error handling and setup summary reporting in
SetupCommand
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| Agent365Config.cs | Changed AgentUserPrincipalName from mutable to init-only property |
| ConfigCommand.cs | Added filtering to only save static properties to a365.config.json |
| ConfigCommandStaticDynamicSeparationTests.cs | New comprehensive test suite to verify static/dynamic property separation |
| SetupCommand.cs | Enhanced error handling, setup tracking, and summary reporting |
| SetupCommandTests.cs | Added tests for error handling and logging behavior |
| A365SetupRunner.cs | Improved logging levels and FIC creation with multiple endpoint fallbacks |
| ConfigService.cs | Refactored config path resolution to use consistent global directory helper |
| InteractiveGraphAuthService.cs | Changed permission messages from Warning to Information level |
| DelegatedConsentService.cs | Changed transient grant error from Warning to Debug level |
| Program.cs | Added missing graphApiService parameter to CleanupCommand |
| DEVELOPER.md | Added troubleshooting guide for PATH configuration |
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupCommand.cs
Outdated
Show resolved
Hide resolved
...crosoft.Agents.A365.DevTools.Cli.Tests/Commands/ConfigCommandStaticDynamicSeparationTests.cs
Show resolved
Hide resolved
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Agents.A365.DevTools.Cli/Services/A365SetupRunner.cs
Outdated
Show resolved
Hide resolved
Refactored `ConfigCommand` to always include the `init` subcommand, simplifying logic and enhancing error handling. Updated `SetupCommand` to support test invokers, improve logging for inheritable permissions, and streamline configuration steps. Simplified `SetupResults` class structure for consistency. Adjusted `Program` to remove unused `graphApiService` dependency in `CleanupCommand`. Improved retry logic in `A365SetupRunner` for better readability and maintained exponential backoff. Enhanced `SetupCommandTests` by replacing static flags with dynamic logging assertions, improving test coverage and readability. General cleanup included formatting, comment adjustments, and removal of redundant code.
mengyimicro
approved these changes
Nov 15, 2025
mengyimicro
approved these changes
Nov 15, 2025
rahuldevikar761
approved these changes
Nov 15, 2025
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.
No description provided.