Add confirmation prompts for app registration changes#325
Merged
sellakumaran merged 6 commits intomainfrom Mar 28, 2026
Merged
Conversation
- Prompt user before mutating app registrations; add --yes flag to skip prompts for CI/non-interactive use - Show mutation summary before applying changes (permissions, redirect URIs, public client flows) - Improve error and warning logging for requirement checks - Propagate OperationCanceledException for clean Ctrl+C exits - Enhance ClientAppValidator with read-only mutation checks and consent helpers - Expand documentation and update tests for new behaviors
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds explicit user confirmation before the CLI mutates Entra app registrations (permissions, redirect URIs, and public client flows), while providing a --yes escape hatch for CI/non-interactive usage. It also refines requirement-check logging/output and ensures OperationCanceledException propagates for clean cancellation behavior.
Changes:
- Add confirmation prompting + mutation summaries for client app registration “self-healing”, with
--yes/-yto skip prompts. - Improve requirement-check logging (warnings/failures) and propagate
OperationCanceledExceptionthrough validation and requirement checks. - Update constants/docs and expand tests for the new behaviors.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/FrontierPreviewRequirementCheckTests.cs | Updates warning-message assertion to match new requirement warning format. |
| src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/Requirements/ClientAppRequirementCheckTests.cs | Adds cancellation propagation test; adjusts mock calls to use named ct: argument. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementChecks/ClientAppRequirementCheck.cs | Uses richer validation error details; rethrows OperationCanceledException. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs | Changes baseline requirement check warning/failure logging output formatting. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/IClientAppValidator.cs | Extends validator API with skipConfirmation parameter. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/ConsoleConfirmationProvider.cs | Treats closed stdin as cancellation during confirmation prompts. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigurationWizardService.cs | Updates validator call site to use named ct: argument. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs | Implements pre-flight mutation summary + confirmation gating; adds consent helper methods; improves public-client-flow messaging. |
| src/Microsoft.Agents.A365.DevTools.Cli/Constants/AuthenticationConstants.cs | Expands documentation for required redirect URIs and why they’re needed. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs | Avoids swallowing CleanExitException from requirement checks. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs | Avoids swallowing CleanExitException from requirement checks. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AdminSubcommand.cs | Avoids swallowing CleanExitException from requirement checks. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/ConfigCommand.cs | Adds --yes/-y and wires it to client app validation to skip confirmations. |
src/Microsoft.Agents.A365.DevTools.Cli/Services/ConsoleConfirmationProvider.cs
Show resolved
Hide resolved
src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs
Show resolved
Hide resolved
Improve error handling for AAD app config checks Switch to fail-closed logic in client app validation methods to ensure users are prompted on errors, preventing silent misconfiguration. Use GraphApiService with correct permissions for service principal propagation checks to avoid 403s and unnecessary retries. Update log messages to clarify intentional fail-closed behavior for safer, more robust error handling.
Update warning log prefix to [WARN] in checks and tests Changed warning log prefix from "WARNING:" to "[WARN]" in RequirementCheck. Updated related unit test to expect the new prefix, ensuring consistency between implementation and test expectations.
src/Microsoft.Agents.A365.DevTools.Cli/Services/Requirements/RequirementCheck.cs
Show resolved
Hide resolved
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs
Outdated
Show resolved
Hide resolved
ajmfehr
previously approved these changes
Mar 27, 2026
Improve client app validation prompts and test coverage Enhance ClientAppValidator to throw detailed exceptions when the user declines to apply required fixes (permissions, redirect URIs, public client flows), listing all pending issues for manual configuration. Update BlueprintSubcommand to clarify delegated scope usage. Add "[FAIL]" prefix to failed requirement logs. Introduce comprehensive tests for confirmation prompt logic, ensuring correct behavior for both acceptance and decline scenarios, and improve test setup utilities. These changes increase robustness, user feedback, and test coverage for the validation and self-healing process.
The MSAL.NET PR deleted AzCliHelper.InvalidateAzCliTokenCache() from ClientAppValidator, but our branch had modified the same line to use the fully-qualified name. Git's ort strategy silently kept our version instead of applying the deletion, leaving a call to a now-removed method. Remove the stale call and update GraphGetWithResponseAsync mock signatures in tests to match the new forceRefresh parameter added in the same PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
src/Microsoft.Agents.A365.DevTools.Cli/Services/ClientAppValidator.cs
Outdated
Show resolved
Hide resolved
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ClientAppValidatorTests.cs
Show resolved
Hide resolved
gwharris7
approved these changes
Mar 27, 2026
ajmfehr
approved these changes
Mar 28, 2026
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 #322