Improve agent setup: clarify S2S and consent messaging#404
Merged
sellakumaran merged 5 commits intomainfrom May 5, 2026
Merged
Improve agent setup: clarify S2S and consent messaging#404sellakumaran merged 5 commits intomainfrom
sellakumaran merged 5 commits intomainfrom
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
Improves a365 setup permissions bot operator guidance by tightening success/warning messaging around S2S app-role assignment outcomes and clarifying the “admin consent required” wording, with corresponding release notes in CHANGELOG.md.
Changes:
- Update MSAL consent-required warning to state that a Global Administrator must grant tenant-wide consent.
- Adjust
setup permissions botcompletion messaging and return value to account for S2S app-role assignment failure. - Document the behavior change in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Microsoft.Agents.A365.DevTools.Cli/Services/MsalBrowserCredential.cs | Refines consent-required warning text to better match the actual failure condition. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/PermissionsSubcommand.cs | Attempts to gate the “configured successfully” message and boolean return on S2S assignment success. |
| CHANGELOG.md | Adds release notes describing the updated messaging behavior. |
sellakumaran
added a commit
that referenced
this pull request
May 5, 2026
- Only log "Bot API permissions configured successfully" if S2S app role assignment succeeds; otherwise, show a warning with retry instructions. - Return value now reflects both consent and S2S assignment success to avoid false positives. - Update consent-required message to clarify that a Global Administrator must grant tenant-wide consent, improving accuracy and user guidance.
caf441c to
b7460a4
Compare
…dministrator Application Administrator and Agent ID Administrator can also grant tenant-wide admin consent — the previous wording incorrectly implied only Global Administrators could do so. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l safety Reorders the logging conditional so the non-admin path (consentGranted=false) is checked before the S2S state, preventing a null S2SAppRoleGranted from routing to the S2S-failure warning instead of the consent-required message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- CHANGELOG: broaden S2S entry from Observability-specific to any S2S failure
- CHANGELOG: correct consent message to match actual code ("An administrator" not "A Global Administrator")
- Rename WhenConsentPending tests to WhenGraphAuthFails — null GraphGetAsync drives the phase1Result=null (auth-failed) path, not the non-admin consent-pending path
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gwharris7
approved these changes
May 5, 2026
ajmfehr
approved these changes
May 5, 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 #402