Skip to content

Ensure blueprint creator is app owner; add tests #163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
sellakumaran wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Ensure blueprint creator is app owner; add tests #163

sellakumaran wants to merge 4 commits into from

Conversation

@sellakumaran
Copy link
Contributor

@sellakumaran sellakumaran commented Jan 16, 2026

Adds logic to make the current user an owner of the Agent Blueprint application during configuration, using a new AddApplicationOwnerAsync method in GraphApiService. This method checks for existing ownership, adds the user if needed via the Graph beta API, and handles errors with clear logging and remediation steps. Includes a comprehensive test suite covering all major scenarios. This improves usability by ensuring the creator can manage the app in Azure and the Developer Portal. No breaking changes.

@sellakumaran
Ensure blueprint creator is app owner; add tests
04474cb 
Adds logic to make the current user an owner of the Agent Blueprint application during configuration, using a new AddApplicationOwnerAsync method in GraphApiService. This method checks for existing ownership, adds the user if needed via the Graph beta API, and handles errors with clear logging and remediation steps. Includes a comprehensive test suite covering all major scenarios. This improves usability by ensuring the creator can manage the app in Azure and the Developer Portal. No breaking changes.
Copilot AI review requested due to automatic review settings January 16, 2026 22:12
@sellakumaran sellakumaran requested review from a team as code owners January 16, 2026 22:12
Copilot AI reviewed Jan 16, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds functionality to ensure the current user becomes an owner of the Agent Blueprint application during configuration. This enables blueprint creators to manage the app in Azure Portal and configure callback URLs/bot IDs via the Developer Portal.

Changes:

  • Adds AddApplicationOwnerAsync method to GraphApiService with idempotency checks
  • Integrates owner assignment into the blueprint configuration workflow
  • Includes comprehensive test suite covering success, failure, and edge case scenarios
  • Removes outdated documentation file

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 20 comments.

File Description
src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs Adds AddApplicationOwnerAsync method with owner checking and Graph beta API integration; includes whitespace cleanup
src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs Integrates owner assignment into blueprint configuration with fallback instructions
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceAddApplicationOwnerTests.cs Comprehensive test suite covering idempotency, error handling, payload validation, and edge cases
docs/guides/custom-client-app-registration.md Removes entire documentation file
Comments suppressed due to low confidence (1)

docs/guides/custom-client-app-registration.md:1

  • This entire 336-line documentation file is being deleted. Ensure that this content is either no longer needed or has been moved/consolidated elsewhere. If this documentation is still relevant, consider whether it should be relocated rather than deleted entirely.

@sellakumaran
Ensure proper disposal in tests; update Graph API payload
a3d7caa 
- Update testing standards to require disposing IDisposable objects (e.g., HttpResponseMessage) in tests.
- Refactor GraphApiService to use LINQ for owner checks and use JsonObject with "@odata.id" for payloads.
- Use using statements for StringContent and test handlers to ensure disposal.
- Update tests to check for "@odata.id" and dispose handlers properly.
- Add Dispose overrides to test handlers to clean up HttpResponseMessage instances.
@sellakumaran sellakumaran marked this pull request as draft January 17, 2026 00:52
@sellakumaran
Improve app owner assignment via Graph API, add constants
696594e 
Refactored owner assignment to use new GraphApiConstants for URLs, versions, and scopes. Now explicitly requests Application.ReadWrite.All permissions and uses the beta endpoint with a fully qualified @odata.id payload. Enhanced error logging with actionable guidance. Updated tests for new payload format. Improves standards compliance and user experience.
@sellakumaran sellakumaran marked this pull request as ready for review January 17, 2026 01:29
Copilot AI review requested due to automatic review settings January 17, 2026 01:29
@sellakumaran sellakumaran enabled auto-merge (squash) January 17, 2026 01:29
Copilot AI reviewed Jan 17, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

@sellakumaran
Improve disposal of HTTP and JSON resources in GraphApiService
d9e1b95 
Ensure HttpResponseMessage and JsonDocument are disposed using using statements to prevent memory leaks. Suppress CA2000 in tests with a pragma and explanatory remark, as test handlers properly dispose queued responses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sellakumaran