re-enable create instance#347
Conversation
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 |
There was a problem hiding this comment.
Pull request overview
Re-enables the a365 create-instance workflow and updates it to align with current Graph API requirements (notably sponsor-required agent identity creation) and updated licensing, while modernizing command wiring to use dependency-injected services.
Changes:
- Restores
create-instancesubcommands (identity,licenses) and removes deprecation behavior. - Updates
A365CreateInstanceRunnerto require a sponsor and uses theMicrosoft_Agent_365_Tier_3SKU. - Updates tests and changelog to reflect the command reactivation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs | Re-enables command + subcommands and switches runner to DI-provided GraphApiService. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs | Enforces sponsor requirement and updates license assignment SKU/log output. |
| src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/CreateInstanceCommandTests.cs | Updates expectations to match command no longer being deprecated. |
| CHANGELOG.md | Documents command re-enable and sponsor-handling fix. |
Comments suppressed due to low confidence (1)
src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs:539
- The failure path reads
identityResponse.Contentmore than once (ReadAsStringAsyncearlier, then again here).HttpContentisn’t guaranteed to be readable multiple times; read the error body once and reuse it for all checks/logs.
if (!identityResponse.IsSuccessStatusCode)
{
var errorContent = await identityResponse.Content.ReadAsStringAsync(ct);
_logger.LogError("Failed to create agent identity: {Status} - {Error}", identityResponse.StatusCode, errorContent);
return (false, null);
|
Can you add the manual end to end test matrix that was verified to the PR please? |
|
Do we need admin consent step? Can you verify please? |
@sellakumaran Admin consent is not required. Consent inheritance is enabled by default. If consent inheritance is not enabled for some reason, the command attempts to apply the required scopes directly to the agent identity (which requires admin privileges). |
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs:61
- The dry-run output claims a Step 3 "Configure Bot Service", but this handler doesn’t appear to perform any bot configuration (and
IBotConfiguratorisn’t used). Either update the user-facing step text to reflect the actual actions performed, or implement the missing bot configuration step so the output is accurate.
logger.LogInformation("DRY RUN: Agent 365 Instance Creation - All Steps");
logger.LogInformation("This would execute the following operations:");
logger.LogInformation(" 1. Create Agent Identity and Agent User");
logger.LogInformation(" 2. Add licenses to Agent User");
logger.LogInformation(" 3. Configure Bot Service");
logger.LogInformation("No actual changes will be made.");
src/Microsoft.Agents.A365.DevTools.Cli/Services/A365CreateInstanceRunner.cs:539
identityResponse.Content.ReadAsStringAsync()is invoked more than once in this error-handling path. Consider reading the content once into a local variable and reusing it to avoid redundant reads and keep the flow easier to follow.
if (!identityResponse.IsSuccessStatusCode)
{
var errorContent = await identityResponse.Content.ReadAsStringAsync(ct);
_logger.LogError("Failed to create agent identity: {Status} - {Error}", identityResponse.StatusCode, errorContent);
return (false, null);
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/Microsoft.Agents.A365.DevTools.Cli/Commands/CreateInstanceCommand.cs:412
--verboseis accepted by the licenses subcommand handler but never used, so it currently has no effect. Consider using it to control the minimum log level of thecleanLoggerFactory(and any debug logging), or remove the option to avoid advertising unsupported behavior.
command.AddOption(configOption);
command.AddOption(verboseOption);
command.AddOption(dryRunOption);
command.SetHandler(async (config, verbose, dryRun) =>
{
if (dryRun)
{
logger.LogInformation("DRY RUN: Adding licenses to Agent User");
logger.LogInformation("This would assign M365 and Power Platform licenses to the agent user");
return;
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (2)
src/Microsoft.Agents.A365.DevTools.Cli/Services/AuthenticationService.cs:201
- This branch checks for the WorkIQ Tools production app ID, but the user-facing log still says "Authenticating to Agent 365 Tools". Update the message (and any other related wording) so it matches the Work IQ Tools naming to avoid confusing users during interactive auth.
if (resourceUrl == McpConstants.WorkIQToolsProdAppId)
{
scope = $"{resourceUrl}/.default";
_logger.LogInformation("Authenticating to Agent 365 Tools");
}
src/Microsoft.Agents.A365.DevTools.Cli/Constants/McpConstants.cs:18
- This constant was renamed to WorkIQToolsProdAppId, but the identifier URI XML comment still says "Agent 365 Tools". Update the comment (and potentially the identifier constant name) so terminology is consistent for maintainers.
// WorkIQ Tools App ID
public const string WorkIQToolsProdAppId = "ea9ffc3e-8a23-4a7d-836d-234d7c7565c1";
/// <summary>
/// Agent 365 Tools identifier URI (used for admin consent URL construction).
/// </summary>
public const string Agent365ToolsIdentifierUri = "https://agent365.svc.cloud.microsoft";
This pull request re-enables the
a365 create-instancecommand, updating it to align with current Microsoft Graph API requirements and product licensing. The command and its subcommands are no longer deprecated and now require a sponsor for agent identity creation. The implementation has been modernized to use dependency-injected services and the correct license SKU. Error handling and user guidance have also been improved. This resolves #274Command Reactivation and Modernization
a365 create-instancecommand, updating its description and restoring its subcommands (identity,licenses) for standard use instead of deprecation. [1] [2] [3]GraphApiServicethroughout the command and its subcommands, removing legacy authentication and service instantiation logic. [1] [2] [3] [4] [5] [6] [7]Sponsor Handling and Error Reporting
BadRequesterrors; added clear error messages and recommended actions if the sponsor cannot be determined. [1] [2] [3] [4]Licensing Updates
Microsoft_Agent_365_Tier_3SKU instead of the previous Teams and E5 SKUs, and updated related log messages. [1] [2] [3]Deprecation Removal and Documentation
Changelog and User Guidance
CHANGELOG.mdto reflect the re-enabled command, sponsor requirement, and improved error handling. [1] [2]Test Matrix