diff --git a/CHANGELOG.md b/CHANGELOG.md index 59aa369b..dae25104 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Upgrade Notes -#### Existing agents: grant Observability API permissions +#### Agents exporting through the delegated (OBO) route: grant Observability API permissions -Agents provisioned before this release need `Agent365.Observability.OtelWrite` granted as both a **delegated** and an **application** permission on the blueprint app. Requires Global Administrator. +Agents that export telemetry through the delegated (OBO) route need `Agent365.Observability.OtelWrite` granted as both a **delegated** and an **application** permission on the blueprint app. Requires Global Administrator. **Option A — Entra portal** (no config files required): @@ -22,6 +22,8 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g **Option B — CLI** (`a365 setup admin`) has been removed in this release. Use Option A above, or copy the PowerShell instructions printed in the `a365 setup all` summary output. +Blueprint agents that export telemetry through the app-only S2S endpoint don't need these permissions, and `a365 setup all` no longer requests them for blueprint agents (#501). + ### Added - Setup and bootstrap now use Microsoft's first-party Agent 365 CLI application when it is present in your tenant, validating it without changing Microsoft's app registration, and fall back to a tenant-owned "Agent 365 CLI" app when it is not (#489). - Log separator written at the start of each CLI invocation now redacts values for secret-bearing options (e.g. `--idp-client-secret`) so they are not written to the log file in plain text. @@ -59,6 +61,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g - `a365 develop get-token --device-code` — forces device code auth for Microsoft Graph scopes the Windows WAM broker rejects (e.g. Exchange `MailboxSettings.ReadWrite`, `ExchangeMessageTrace.Read.All`). ### Fixed +- `a365 setup all` now exits with code 1 when agent registration fails or cannot be verified for blueprint agents or with `--agent-registration-only` (#501). - Setup no longer fails to detect the Agent 365 CLI application in tenants where it is not yet provisioned, and reports lookup errors instead of silently switching your configured client app (#489). - The first-party Agent 365 CLI app now uses device code authentication when Windows Account Manager is unavailable, avoiding unsupported browser-response errors in WSL, macOS, and Linux (#489). - `setup all --authmode s2s` no longer prints spurious "Action Required" PowerShell steps when the agent identity already inherits its app roles from the blueprint, and now retries the grant automatically before falling back to manual steps (#460). @@ -104,6 +107,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g ### Changed +- `a365 setup all` no longer requests Observability API permissions for blueprint agents, so registered agents export telemetry through the app-only S2S endpoint without admin consent (#501). - Hardened token storage: the CLI no longer writes access tokens to a plaintext file — they live only in the OS-protected MSAL cache (DPAPI/Keychain/owner-only file). Any legacy plaintext cache is removed automatically; sign-in prompts are unchanged. - `develop-mcp register-external-mcp-server` now sets `exit code 1` on failure paths (validation errors, tenant detection failure, Graph unavailable, Entra app creation failure, MCP-Platform AddMcpServer failure). Previously these paths logged an error and exited `0`, which made the command's success/failure status undetectable from scripts and CI. Successful dry-run and user-initiated cancellation at the y/N prompt continue to exit `0`. - Admin consent canary path (when the caller lacks `DelegatedPermissionGrant.Read.All`) no longer prompts for Enter immediately. The CLI now polls every 5 seconds, prints a friendly progress message at 30 seconds, and responds promptly to Enter or Ctrl+C. The previous jargon-heavy message about `oauth2PermissionGrants` was rewritten in plain English; technical details are demoted to `Debug`. diff --git a/docs/agent365-guided-setup/a365-observability-instructions.md b/docs/agent365-guided-setup/a365-observability-instructions.md index 0946e1d8..eaf8a3f5 100644 --- a/docs/agent365-guided-setup/a365-observability-instructions.md +++ b/docs/agent365-guided-setup/a365-observability-instructions.md @@ -775,11 +775,13 @@ This skill is safe to rerun. On subsequent runs: ### OtelWrite App Role Assignment -`a365 setup all` **attempts** to grant `Agent365.Observability.OtelWrite` to the Agent Identity SP, but this requires **Global Administrator** privileges. If the logged-in user is not a Global Admin, the assignment silently fails with 403 and trace exports will return HTTP 403 from the observability service. +> **Blueprint agents:** `a365 setup all` does not request Observability API permissions for blueprint agents in any auth mode — the S2S endpoint authorizes registered agent instances without the `OtelWrite` role, so no admin consent is needed. Setup exits with code 1 if registration fails; retry with `a365 setup all --agent-registration-only`. Grant `OtelWrite` manually (steps below) only if the agent still exports through the delegated (OBO) route. Permissions granted by earlier runs are not revoked. + +For **AI Teammate** agents, `a365 setup all` still **attempts** to grant `Agent365.Observability.OtelWrite` to the Agent Identity SP, which requires **Global Administrator** privileges. If the logged-in user is not a Global Admin, the assignment fails with 403 and trace exports can return HTTP 403 from the observability service. **The CLI prints a PowerShell admin consent script** in its output when the assignment fails. When running `a365 setup all`, **always scan the output for this script block** and display it to the user in a fenced code block so they can copy it and hand it to a Global Admin. -If the script was not captured, grant the permission manually via Entra portal (requires Global Admin): +If the script was not captured — or for a blueprint agent whose SDK still exports through the delegated (OBO) route — grant the permission manually via Entra portal (requires Global Admin): 1. [Entra portal](https://entra.microsoft.com) > App registrations > select Blueprint app > API permissions 2. Add a permission > APIs my organization uses > search `9b975845-388f-4429-889e-eab1ef63949c` 3. Add both **Delegated** and **Application** `Agent365.Observability.OtelWrite` > Grant admin consent diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs index a55f1892..2a9699ef 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -397,13 +397,19 @@ effectiveAuthModeForValidation is not ("obo" or "s2s" or "both")) return; } + // Registered blueprint agents export telemetry app-only over S2S without OtelWrite in every auth + // mode, so blueprint setup never requests it; AI Teammate setup (including an AI Teammate config + // kept for a dry run) is unchanged. + var skipObservabilityPermissions = nonDwConfig is not null + && (aiTeammateFlag == false || nonDwConfig.IsBlueprintAgent); + if (nonDwConfig is not null) { if (dryRun) { var rawArgs = context.ParseResult.Tokens.Select(t => t.Value).ToArray(); var effectiveAuthMode = authMode ?? nonDwConfig.AuthMode; - NonDwBlueprintSetupOrchestrator.PrintDryRunPlan(nonDwConfig, logger, isBootstrap, rawArgs, skipRequirements, isM365, agentRegistrationOnly, effectiveAuthMode, messagingEndpointFlag); + NonDwBlueprintSetupOrchestrator.PrintDryRunPlan(nonDwConfig, logger, isBootstrap, rawArgs, skipRequirements, isM365, agentRegistrationOnly, effectiveAuthMode, messagingEndpointFlag, skipObservabilityPermissions); return; } @@ -442,7 +448,8 @@ effectiveAuthModeForValidation is not ("obo" or "s2s" or "both")) confirmationProvider: confirmationProvider, skipSpProvisioning: skipSpProvisioning, messagingEndpointOverride: messagingEndpointFlag, - nonInteractive: Console.IsInputRedirected); + nonInteractive: Console.IsInputRedirected, + skipObservabilityPermissions: skipObservabilityPermissions); context.ExitCode = await NonDwBlueprintSetupOrchestrator.ExecuteAsync(nonDwCtx); return; @@ -1018,7 +1025,8 @@ await PermissionsSubcommand.RemoveStaleCustomPermissionsAsync( // for both DW and non-DW agents; serverNamesByAudience drives the per-server display // names so V2 audiences read as e.g. "mcp_MailTools" rather than "Agent 365 Tools". var specs = await SetupHelpers.BuildConfiguredPermissionSpecsAsync( - ctx.Config, setInheritable: true, isM365: ctx.IsM365, scopesByAudience, serverNamesByAudience); + ctx.Config, setInheritable: true, isM365: ctx.IsM365, scopesByAudience, serverNamesByAudience, + includeObservability: !ctx.SkipObservabilityPermissions); // Return the full scopesByAudience map alongside the V1-compat mcpScopes so V2 // callers (ApplyConsentUrlsIfNeeded) can route per-server audiences to the bare diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs index 08be6550..a3912787 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/NonDwBlueprintSetupOrchestrator.cs @@ -20,8 +20,8 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; /// 1. Requirements validation /// 2. Blueprint creation (shared with DW) /// 3. Batch permissions on the blueprint (shared with DW pipeline; non-DW spec set: -/// Observability API, Power Platform API, custom). MAC reads from the blueprint, -/// so stamping here gives the same set visibility there. +/// Power Platform API and custom; Observability API is not requested). MAC reads +/// from the blueprint, so stamping here gives the same set visibility there. /// 4. Agent Identity creation via POST /beta/servicePrincipals/Microsoft.Graph.AgentIdentity /// 5. Agent Identity permission grants (same spec set as step 3) — OBO or S2S /// 6. Agent registration via Graph API (copilot/agentRegistrations) @@ -33,7 +33,7 @@ internal static class NonDwBlueprintSetupOrchestrator /// Prints a dry-run plan showing all resources that would be created or configured, /// using actual names and values from the loaded config. Makes no API calls. /// - public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool isBootstrap = false, string[]? rawArgs = null, bool skipRequirements = false, bool isM365 = false, bool agentRegistrationOnly = false, string? authMode = null, string? messagingEndpointOverride = null) + public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool isBootstrap = false, string[]? rawArgs = null, bool skipRequirements = false, bool isM365 = false, bool agentRegistrationOnly = false, string? authMode = null, string? messagingEndpointOverride = null, bool skipObservabilityPermissions = false) { var sub = new string(' ', SetupHelpers.DryRunValCol); // --messaging-endpoint flag (if supplied) wins over the init-only config value for the plan. @@ -117,14 +117,17 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i logger.LogInformation(sub + "create managed identity"); } - // 3. Inheritable Permissions — non-DW spec set (Observability API, Power Platform API, custom) - // stamped on the blueprint via SetInheritablePermissionsAsync so MAC and other dependent - // systems can see them. The same set is applied to the agent identity SP in step 5. + // 3. Inheritable Permissions — non-DW spec set (Power Platform API and custom; Observability API is + // not requested) stamped on the blueprint via SetInheritablePermissionsAsync so MAC and other + // dependent systems can see them. The same set is applied to the agent identity SP in step 5. var selectedAuthMode = authMode ?? config.AuthMode; var effectiveMode = string.IsNullOrWhiteSpace(selectedAuthMode) ? "obo" : selectedAuthMode.Trim().ToLowerInvariant(); - logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for Observability API, Power Platform API, and custom permissions (Global Administrator required; consent URL printed if absent)"); + logger.LogInformation(SetupHelpers.DryRunRow(3, "Inheritable Permissions") + "configure for {Resources} (Global Administrator required; consent URL printed if absent)", + skipObservabilityPermissions ? "Power Platform API and custom permissions" : "Observability API, Power Platform API, and custom permissions"); + if (skipObservabilityPermissions) + logger.LogInformation(sub + "Observability API not requested (registered agents export telemetry with an app-only token)"); // 4. Blueprint Permission Grants — per authMode. The consent URL targets the blueprint // app, and S2S app-role assignments are persisted as grants flowing from the blueprint; @@ -266,6 +269,7 @@ await ctx.ClientAppValidator.GrantConsentForPermissionsAsync( public static async Task ExecuteAsync(SetupContext ctx) { ctx.Results.IsNonDwBlueprintFlow = true; + ctx.Results.ObservabilityPermissionsSkipped = ctx.SkipObservabilityPermissions; ctx.Results.TenantId = ctx.Config.TenantId; // Bootstrap already printed the "Running..." banner before auth steps; skip here to avoid duplication. if (!ctx.IsBootstrap) @@ -362,8 +366,10 @@ public static async Task ExecuteAsync(SetupContext ctx) // Step 3: Blueprint creation (shared with DW) await AllSubcommand.ExecuteBlueprintStepAsync(ctx); - // Step 4: Build permission specs — stamps Graph, manifest MCP audiences, Observability, - // Power Platform, custom permissions, and Messaging Bot (only when isM365). Mirrors DW. + // Step 4: Build permission specs — stamps Graph, manifest MCP audiences, Power Platform, + // custom permissions, Messaging Bot (only when isM365), and Observability unless skipped. + if (ctx.SkipObservabilityPermissions) + ctx.Logger.LogInformation("Observability API permissions not requested: registered agents export telemetry with an app-only token."); var buildResult = await AllSubcommand.BuildPermissionSpecsAsync(ctx); specs = buildResult.specs; @@ -435,7 +441,7 @@ await AllSubcommand.ExecuteBatchPermissionsStepAsync( /// When is true (--agent-registration-only), /// identity creation and permission grants are skipped — only registration and project settings run. /// - private static async Task ExecuteAgentIdentityAndRegistrationAsync( + internal static async Task ExecuteAgentIdentityAndRegistrationAsync( SetupContext ctx, List specs, bool skipIdentityAndPermissions = false) @@ -549,15 +555,23 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( ctx.Logger.LogInformation(""); ctx.Logger.LogInformation("Registering agent..."); + // Registration is the sole purpose of --agent-registration-only and, with OtelWrite skipped, + // the agent's only Observability authorization, so its failure must fail setup. + var registrationRequired = skipIdentityAndPermissions || ctx.SkipObservabilityPermissions; + void RecordRegistrationFailure(string message) + { + ctx.Results.AgentRegistrationFailed = true; + (registrationRequired ? ctx.Results.Errors : ctx.Results.Warnings).Add(message); + ctx.Logger.Log(registrationRequired ? LogLevel.Error : LogLevel.Warning, message); + } + if (string.IsNullOrWhiteSpace(ctx.Config.AgenticAppId)) { var registrationSkippedMessage = "Agent registration failed: agent identity ID is not available. " + "Ensure the agent identity was created successfully, then retry with: a365 setup all --agent-registration-only"; - ctx.Results.Warnings.Add(registrationSkippedMessage); using (ctx.Logger.Indent()) - ctx.Logger.LogWarning(registrationSkippedMessage); - ctx.Results.AgentRegistrationFailed = true; + RecordRegistrationFailure(registrationSkippedMessage); } else { @@ -565,6 +579,7 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( // If a registration ID is already stored, verify it still exists before skipping creation. string? registrationId = null; bool registrationAlreadyExisted = false; + bool verificationFailed = false; if (!string.IsNullOrWhiteSpace(ctx.Config.AgentRegistrationId)) { @@ -591,6 +606,16 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( // stale value on disk that would cause the same stale-ID check to repeat. await ctx.ConfigService.SaveStateAsync(ctx.Config); } + else if (registrationRequired) + { + // An unverifiable registration cannot be the agent's only authorization: keep the stored + // ID (no duplicate registration) but fail so the operator retries. + using (ctx.Logger.Indent()) + RecordRegistrationFailure( + $"Could not verify agent registration {ctx.Config.AgentRegistrationId} (auth or transient error). " + + "Retry with: a365 setup all --agent-registration-only"); + verificationFailed = true; + } else { // Verification inconclusive (auth or transient error) — preserve the stored ID @@ -602,7 +627,7 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( } } - if (string.IsNullOrWhiteSpace(registrationId)) + if (!verificationFailed && string.IsNullOrWhiteSpace(registrationId)) { var (newId, fromConflict) = await ctx.GraphApiService.RegisterAgentInstanceAsyncV2( ctx.Config.TenantId!, @@ -631,11 +656,9 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( ctx.Logger.LogInformation(""); } } - else + else if (!verificationFailed) { - ctx.Results.AgentRegistrationFailed = true; - ctx.Results.Warnings.Add("Agent registration failed via Graph copilot/agentRegistrations API."); - ctx.Logger.LogWarning("Agent registration failed via Graph copilot/agentRegistrations API."); + RecordRegistrationFailure("Agent registration failed via Graph copilot/agentRegistrations API."); } } // end else (AgenticAppId present) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md index 6c211a3e..5b0d88ae 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/README.md @@ -92,6 +92,12 @@ a365 setup all --authmode s2s a365 setup all --authmode both ``` +### Observability permissions + +For blueprint agents, `setup all` does not request `Agent365.Observability.OtelWrite` in any auth mode. Registered agents export telemetry with an app-only token through the S2S endpoint, which authorizes them by their agent registration, so no Observability admin consent is needed. Registration is then the agent's only authorization, so a registration failure is reported as an error (exit code 1). + +Agents whose SDK still exports through the delegated (OBO) route need `OtelWrite`; grant it manually (see the CHANGELOG upgrade note). AI Teammate setup is unchanged. Re-running setup does not revoke permissions granted earlier. + --- ### Messaging endpoint (M365 agents) diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs index bd471924..0f45114b 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupContext.cs @@ -100,6 +100,12 @@ internal sealed class SetupContext /// public bool NonInteractive { get; } + /// + /// When true, Observability API permissions are omitted from every grant and consent URL (non-DW blueprint only), + /// making agent registration the agent's only Observability authorization. + /// + public bool SkipObservabilityPermissions { get; } + /// /// Overrides the az CLI login hint resolver used during blueprint creation. /// Null in production — injected as a no-op in tests to avoid spawning 'az account show'. @@ -154,7 +160,8 @@ public SetupContext( IConfirmationProvider? confirmationProvider = null, bool skipSpProvisioning = false, string? messagingEndpointOverride = null, - bool nonInteractive = false) + bool nonInteractive = false, + bool skipObservabilityPermissions = false) { Config = config; Results = results; @@ -172,6 +179,7 @@ public SetupContext( MessagingEndpointOverride = string.IsNullOrWhiteSpace(messagingEndpointOverride) ? null : messagingEndpointOverride.Trim(); SkipSpProvisioning = skipSpProvisioning; NonInteractive = nonInteractive; + SkipObservabilityPermissions = skipObservabilityPermissions; ConfigService = configService; Executor = executor; BackendConfigurator = backendConfigurator; diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs index df6818c9..3b31f2c9 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -49,12 +49,13 @@ internal static void PrintDryRunBlueprintReuseRows(ILogger logger, string bluepr /// Returns the fixed-scope ResourcePermissionSpecs for the platform APIs that every /// agent blueprint requires. /// - /// Observability API and Power Platform API are always included. Messaging Bot API is + /// Power Platform API is always included. Observability API is included unless + /// is false. Messaging Bot API is /// included only when is true — non-M365 (blueprint-only) agents /// have no messaging surface so Bot scopes serve no purpose. /// /// - internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInheritable, bool isM365) + internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInheritable, bool isM365, bool includeObservability = true) { var specs = new List(); if (isM365) @@ -73,12 +74,15 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInhe new[] { ConfigConstants.MessagingBotApiAdminConsentScope }, setInheritable)); } - specs.Add(new ResourcePermissionSpec( - ConfigConstants.ObservabilityApiAppId, - "Observability API", - new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, - setInheritable, - AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); + if (includeObservability) + { + specs.Add(new ResourcePermissionSpec( + ConfigConstants.ObservabilityApiAppId, + "Observability API", + new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, + setInheritable, + AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); + } specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -93,7 +97,8 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs(bool setInhe /// /// Always includes Microsoft Graph (with config.AgentApplicationScopes), /// manifest-derived Agent 365 Tools scopes (when ToolingManifest.json is present), - /// Observability API, Power Platform API, and any valid custom blueprint permissions. + /// Power Platform API, and any valid custom blueprint permissions. Observability API is + /// included unless is false. /// Messaging Bot API is included only when is true. /// /// @@ -107,7 +112,8 @@ internal static async Task> BuildConfiguredPermissi bool setInheritable, bool isM365 = true, Dictionary? scopesByAudience = null, - Dictionary>? serverNamesByAudience = null) + Dictionary>? serverNamesByAudience = null, + bool includeObservability = true) { // Manifest read at most once, and only when scopesByAudience is not pre-supplied. // Callers that already have the manifest loaded (e.g. AllSubcommand.BuildPermissionSpecsAsync) @@ -146,7 +152,7 @@ internal static async Task> BuildConfiguredPermissi : "Agent 365 Tools", kvp.Value, SetInheritable: setInheritable))); - specs.AddRange(GetFixedApiPermissionSpecs(setInheritable, isM365)); + specs.AddRange(GetFixedApiPermissionSpecs(setInheritable, isM365, includeObservability)); foreach (var customPerm in config.CustomBlueprintPermissions ?? new List()) { @@ -721,6 +727,8 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) logger.LogInformation(DryRunRow(6, "Agent Registration") + registrationVerb + " '{Name}' (ID: {Id})", results.AgentRegistrationDisplayName ?? "unknown", results.AgentInstanceId ?? "unknown"); } + else if (results.AgentRegistrationFailed && results.ObservabilityPermissionsSkipped) + logger.LogError(DryRunRow(6, "Agent Registration") + "failed — see errors"); else if (results.AgentRegistrationFailed) logger.LogWarning(DryRunRow(6, "Agent Registration") + "failed — see warnings"); } @@ -831,7 +839,10 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) if (isNonDw && string.IsNullOrWhiteSpace(consentUrl)) { logger.LogInformation(" {N}. Permission Grants — must be granted by {Roles} in the Entra portal:", actionCount, AuthenticationConstants.DelegatedGrantRequiredRoles); - LogNonDwAdminConsentInstructions(logger, adminCmdBlueprintId, tenantId: results.TenantId); + var consentSpecs = results.ObservabilityPermissionsSkipped + ? NonDwAdminConsentSpecs.Where(s => !string.Equals(s.ResourceAppId, ConfigConstants.ObservabilityApiAppId, StringComparison.OrdinalIgnoreCase)).ToList() + : null; + LogNonDwAdminConsentInstructions(logger, adminCmdBlueprintId, consentSpecs, tenantId: results.TenantId); } else { @@ -1074,10 +1085,10 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger) /// resources. Called when the current user lacks the Global Administrator role so that the URLs /// can be saved to a365.generated.config.json and shared with a tenant administrator. /// - /// Graph, Agent 365 Tools (MCP), Observability API, and Power Platform API URLs are always - /// generated. Messaging Bot API is included only when is true — - /// non-M365 tenants typically lack the Messaging Bot resource SP and the consent endpoint - /// returns AADSTS650053 otherwise. + /// Graph, Agent 365 Tools (MCP), and Power Platform API URLs are always generated; the Observability + /// API URL is generated unless is false. Messaging Bot API is included only + /// when is true — non-M365 tenants typically lack the Messaging Bot + /// resource SP and the consent endpoint returns AADSTS650053 otherwise. /// /// /// Display names of the resources for which URLs were saved. @@ -1087,9 +1098,14 @@ internal static List PopulateAdminConsentUrls( IEnumerable mcpScopes, bool isM365 = true, IReadOnlyDictionary? mcpScopesByAudience = null, - IReadOnlyDictionary>? mcpAudienceDisplayNames = null) + IReadOnlyDictionary>? mcpAudienceDisplayNames = null, + bool includeObservability = true) { - var urls = BuildAdminConsentUrls(config.TenantId, config.AgentBlueprintId!, config.AgentApplicationScopes, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames); + var urls = BuildAdminConsentUrls(config.TenantId, config.AgentBlueprintId!, config.AgentApplicationScopes, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, includeObservability); + + // Drop an Observability entry saved by an earlier run so the admin is not asked for permissions this run skipped. + if (!includeObservability) + config.ResourceConsents.RemoveAll(rc => rc.ResourceAppId.Equals(ConfigConstants.ObservabilityApiAppId, StringComparison.OrdinalIgnoreCase)); // Map resource names to App IDs for upsert into ResourceConsents. The fixed-name // entries cover Graph + Bot + Obs + PP + the WorkIQ shared MCP audience. V2 @@ -1264,8 +1280,8 @@ internal static string BuildFullyQualifiedScope(string resourceAppId, string sco /// Builds per-resource admin consent URLs covering every resource stamped on the blueprint /// (mirrors ): Microsoft Graph (when /// non-empty), Agent 365 Tools (when - /// non-empty), Messaging Bot API (when is true), Observability API, - /// and Power Platform API. + /// non-empty), Messaging Bot API (when is true), Observability API + /// (unless is false), and Power Platform API. /// /// Messaging Bot is gated on because non-M365 tenants typically /// lack the Messaging Bot resource SP, in which case the /v2.0/adminconsent endpoint returns @@ -1280,7 +1296,8 @@ internal static string BuildFullyQualifiedScope(string resourceAppId, string sco IEnumerable mcpScopes, bool isM365 = true, IReadOnlyDictionary? mcpScopesByAudience = null, - IReadOnlyDictionary>? mcpAudienceDisplayNames = null) + IReadOnlyDictionary>? mcpAudienceDisplayNames = null, + bool includeObservability = true) { var urls = new List<(string, string)>(); @@ -1342,7 +1359,8 @@ static string Build(string tenant, string client, string resourceUri, IEnumerabl if (isM365) urls.Add(("Messaging Bot API", Build(tenantId, blueprintClientId, ConfigConstants.MessagingBotApiIdentifierUri, new[] { ConfigConstants.MessagingBotApiAdminConsentScope }))); - urls.Add(("Observability API", Build(tenantId, blueprintClientId, ConfigConstants.ObservabilityApiIdentifierUri, new[] { ConfigConstants.ObservabilityApiOtelWriteScope }))); + if (includeObservability) + urls.Add(("Observability API", Build(tenantId, blueprintClientId, ConfigConstants.ObservabilityApiIdentifierUri, new[] { ConfigConstants.ObservabilityApiOtelWriteScope }))); urls.Add(("Power Platform API", Build(tenantId, blueprintClientId, PowerPlatformConstants.PowerPlatformApiIdentifierUri, new[] { PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead }))); return urls; @@ -1350,7 +1368,8 @@ static string Build(string tenant, string client, string resourceUri, IEnumerabl /// /// Builds a single combined /v2.0/adminconsent URL covering every resource stamped on the - /// blueprint: Graph, Agent 365 Tools (MCP), Observability API, Power Platform API, and + /// blueprint: Graph, Agent 365 Tools (MCP), Observability API (unless + /// is false), Power Platform API, and /// Messaging Bot API (only when is true). /// /// Messaging Bot is gated on because non-M365 tenants typically @@ -1365,7 +1384,8 @@ internal static string BuildCombinedConsentUrl( IEnumerable graphScopes, IEnumerable mcpScopes, bool isM365 = true, - IReadOnlyDictionary? mcpScopesByAudience = null) + IReadOnlyDictionary? mcpScopesByAudience = null, + bool includeObservability = true) { var allScopes = new List(); foreach (var s in graphScopes) @@ -1397,7 +1417,8 @@ internal static string BuildCombinedConsentUrl( if (isM365) allScopes.Add($"{ConfigConstants.MessagingBotApiIdentifierUri}/{ConfigConstants.MessagingBotApiAdminConsentScope}"); - allScopes.Add($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); + if (includeObservability) + allScopes.Add($"{ConfigConstants.ObservabilityApiIdentifierUri}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes); } @@ -1407,8 +1428,9 @@ internal static string BuildCombinedConsentUrl( /// when the running account is not a Global Administrator. Called by both DW and non-DW setup paths /// after the batch permissions step. /// - /// Messaging Bot API URLs are included only when is true; all other - /// resources (Graph, MCP, Observability, Power Platform) are always included so a tenant admin + /// Messaging Bot API URLs are included only when is true, and + /// Observability API URLs only when the context requests Observability permissions; the other + /// resources (Graph, MCP, Power Platform) are always included so a tenant admin /// can complete the hand-off with a single URL. No-op if admin consent was already granted or /// the blueprint ID is absent. /// @@ -1425,12 +1447,13 @@ internal static void ApplyConsentUrlsIfNeeded( if (ctx.Results.TenantWideConsentOutcome == Models.GrantOutcome.Granted || string.IsNullOrWhiteSpace(ctx.Config.AgentBlueprintId)) return; - var consentResourceNames = PopulateAdminConsentUrls(ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames); + var includeObservability = !ctx.SkipObservabilityPermissions; + var consentResourceNames = PopulateAdminConsentUrls(ctx.Config, mcpResourceAppId, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, includeObservability); ctx.Results.ConsentUrlsSavedToPath = ctx.GeneratedConfigPath; ctx.Results.ConsentResourceNames.AddRange(consentResourceNames); ctx.Results.CombinedConsentUrl = BuildCombinedConsentUrl( ctx.Config.TenantId!, ctx.Config.AgentBlueprintId!, - graphScopes, mcpScopes, isM365, mcpScopesByAudience); + graphScopes, mcpScopes, isM365, mcpScopesByAudience, includeObservability); } /// diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs index 2465f917..aacd10c7 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -286,6 +286,12 @@ public class SetupResults /// public bool PermissionGrantsSkipped { get; set; } + /// + /// True when Observability API permissions were not requested (blueprint agents). + /// Registration failure is then an error, and the admin consent walkthrough omits Observability API. + /// + public bool ObservabilityPermissionsSkipped { get; set; } + public List Errors { get; } = new(); public List Warnings { get; } = new(); diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs index 4b03d7e9..6574b210 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AllSubcommandTests.cs @@ -353,4 +353,83 @@ public async Task ExecuteMessagingEndpointStepAsync_WhenOverrideProvidedAndConfi ctx.Results.MessagingEndpoint.Should().Be(overrideUrl, because: "the registered endpoint reported in the summary must be the override URL"); } + + // ----------------------------------------------------------------------- + // Observability API permission wiring + // ----------------------------------------------------------------------- + + private SetupContext BuildPermissionsContext(bool skipObservabilityPermissions) + { + var executor = Substitute.For(Substitute.For>()); + var graph = Substitute.For(); + var blueprintService = Substitute.For(Substitute.For>(), graph); + // The blueprint has no inheritable permissions yet, so stale-permission cleanup has nothing to remove. + blueprintService.ListInheritablePermissionsAsync( + Arg.Any(), Arg.Any(), Arg.Any?>(), Arg.Any()) + .Returns(new List<(string ResourceAppId, bool ScopesAllAllowed, bool RolesAllAllowed)>()); + + return new SetupContext( + config: new Agent365Config + { + AiTeammate = false, + TenantId = "tenant-id", + AgentBlueprintId = "blueprint-id", + ClientAppId = "client-app-id", + DeploymentProjectPath = _tempDir, + }, + results: new SetupResults(), + logger: NullLogger.Instance, + configFile: new FileInfo(Path.Combine(_tempDir, "a365.config.json")), + generatedConfigPath: Path.Combine(_tempDir, "a365.generated.config.json"), + correlationId: "test-correlation-id", + skipInfrastructure: true, + skipRequirements: true, + cancellationToken: CancellationToken.None, + configService: Substitute.For(), + executor: executor, + backendConfigurator: Substitute.For(), + authValidator: Substitute.For(NullLogger.Instance, executor), + platformDetector: Substitute.ForPartsOf(Substitute.For>()), + graphApiService: graph, + blueprintService: blueprintService, + blueprintLookupService: Substitute.ForPartsOf( + Substitute.For>(), graph), + federatedCredentialService: Substitute.ForPartsOf( + Substitute.For>(), graph), + clientAppValidator: Substitute.For(), + skipObservabilityPermissions: skipObservabilityPermissions); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BuildPermissionSpecsAsync_StampsObservabilityApiUnlessSkipped(bool skipObservabilityPermissions) + { + var ctx = BuildPermissionsContext(skipObservabilityPermissions); + + var (specs, _, _, _, _) = await AllSubcommand.BuildPermissionSpecsAsync(ctx); + + specs.Any(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId).Should().Be(!skipObservabilityPermissions, + because: "the spec list drives inheritable permissions, app role grants, and admin consent, so skipping Observability permissions must remove Observability API from it"); + specs.Any(s => s.AppRoleScopes is { Length: > 0 }).Should().Be(!skipObservabilityPermissions, + because: "OtelWrite is the only app role setup requests, so skipping it must leave no app role grant that needs a Global Administrator"); + specs.Should().Contain(s => s.ResourceAppId == PowerPlatformConstants.PowerPlatformApiResourceAppId, + because: "skipping Observability API must not drop the other required resources"); + } + + [Fact] + public void ApplyConsentUrlsIfNeeded_WhenObservabilitySkipped_HandsOffOnlyTheRemainingResources() + { + var ctx = BuildPermissionsContext(skipObservabilityPermissions: true); + + SetupHelpers.ApplyConsentUrlsIfNeeded( + ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, new[] { "McpServers.Mail.All" }, isM365: false); + + ctx.Results.ConsentResourceNames.Should().BeEquivalentTo(new[] { "Microsoft Graph", "Agent 365 Tools", "Power Platform API" }, + because: "a non-admin run must hand every stamped resource to an administrator, and Observability API is no longer stamped"); + ctx.Config.ResourceConsents.Should().NotContain(rc => rc.ResourceAppId == ConfigConstants.ObservabilityApiAppId, + because: "no Observability API consent URL may be persisted when its permissions were skipped"); + ctx.Results.CombinedConsentUrl.Should().NotContain(ConfigConstants.ObservabilityApiAppId, + because: "the single hand-off URL must not request Observability API scopes that setup skipped"); + } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorExecuteTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorExecuteTests.cs index 7a15f25a..f76b4d08 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorExecuteTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorExecuteTests.cs @@ -242,12 +242,12 @@ public void SetupResults_CanSetAgentInstanceRegisteredAndId() /// /// Builds a SetupContext suited for testing the agent identity + registration steps - /// (Steps 5–6) via the AgentInstanceOnly path. + /// (Steps 5–6), by default via the AgentInstanceOnly path. /// Returns the context, graph service mock, and blueprint service mock so tests can /// configure stub return values. /// private static (SetupContext ctx, GraphApiService graph, AgentBlueprintService blueprintService) - BuildIdempotencyTestContext(Agent365Config? config = null) + BuildIdempotencyTestContext(Agent365Config? config = null, bool agentInstanceOnly = true, bool skipObservabilityPermissions = false) { var graph = Substitute.ForPartsOf(); @@ -298,8 +298,9 @@ private static (SetupContext ctx, GraphApiService graph, AgentBlueprintService b federatedCredentialService: Substitute.ForPartsOf( Substitute.For>(), graph), clientAppValidator: Substitute.For(), - agentInstanceOnly: true, - loginHintResolver: () => Task.FromResult(null)); + agentInstanceOnly: agentInstanceOnly, + loginHintResolver: () => Task.FromResult(null), + skipObservabilityPermissions: skipObservabilityPermissions); return (ctx, graph, blueprintService); } @@ -621,37 +622,164 @@ public async Task Step6_SetsAlreadyExistedFlag_When409ConflictReturnedByRegister } /// - /// Step 6: When AgentRegistrationExistsAsync returns null (auth or transient error), - /// the stored registration ID must be preserved and re-registration must not be attempted. + /// Step 6: When AgentRegistrationExistsAsync returns null (auth or transient error) and registration is + /// optional (Observability permissions requested, not --agent-registration-only), the stored registration + /// ID must be preserved and re-registration must not be attempted. /// [Fact] public async Task Step6_PreservesStoredRegistrationId_WhenVerificationIsInconclusive() { - var config = new Agent365Config + // Empty project directory: the project settings step finds no project and writes nothing. + var projectDir = Path.Combine(Path.GetTempPath(), "NonDwRegistrationTests_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(projectDir); + try { - AiTeammate = false, - TenantId = "tenant-id", - AgentBlueprintId = "blueprint-id", - AgentIdentityDisplayName = "sellakapri211 Identity", - ClientAppId = "client-app-id", - AgenticAppId = "agentic-app-id", - AgentRegistrationId = "stored-reg-id", - }; - var (ctx, graph, _) = BuildIdempotencyTestContext(config); - - graph.AgentRegistrationExistsAsync( - Arg.Any(), Arg.Any(), Arg.Any()) - .Returns((bool?)null); + var config = new Agent365Config + { + AiTeammate = false, + TenantId = "tenant-id", + AgentBlueprintId = "blueprint-id", + AgentIdentityDisplayName = "sellakapri211 Identity", + ClientAppId = "client-app-id", + AgenticAppId = "agentic-app-id", + AgentRegistrationId = "stored-reg-id", + DeploymentProjectPath = projectDir, + }; + var (ctx, graph, _) = BuildIdempotencyTestContext(config, agentInstanceOnly: false); + + graph.AgentRegistrationExistsAsync( + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns((bool?)null); + + await NonDwBlueprintSetupOrchestrator.ExecuteAgentIdentityAndRegistrationAsync(ctx, specs: []); + + ctx.Results.AgentInstanceId.Should().Be("stored-reg-id", + because: "when verification is inconclusive the stored ID must be preserved to avoid unintended re-registration"); + ctx.Results.AgentRegistrationAlreadyExisted.Should().BeTrue( + because: "an inconclusive verification is treated as 'assume still exists' to prevent data loss"); + await graph.DidNotReceive().RegisterAgentInstanceAsyncV2( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any()); + } + finally + { + Directory.Delete(projectDir, recursive: true); + } + } - await NonDwBlueprintSetupOrchestrator.ExecuteAsync(ctx); + /// + /// Step 6: when registration is required (--agent-registration-only, or Observability permissions not + /// requested so registration is the agent's only authorization), an inconclusive verification must fail + /// setup instead of passing — while still keeping the stored ID and not creating a duplicate registration. + /// + [Theory] + [InlineData(true, false)] + [InlineData(false, true)] + public async Task Step6_RegistrationRequired_FailsWithoutReRegistering_WhenVerificationIsInconclusive(bool agentInstanceOnly, bool skipObservabilityPermissions) + { + var projectDir = Path.Combine(Path.GetTempPath(), "NonDwRegistrationTests_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(projectDir); + try + { + var config = new Agent365Config + { + AiTeammate = false, + TenantId = "tenant-id", + AgentBlueprintId = "blueprint-id", + AgentIdentityDisplayName = "Test Agent Identity", + ClientAppId = "client-app-id", + AgenticAppId = "agentic-app-id", + AgentRegistrationId = "stored-reg-id", + DeploymentProjectPath = projectDir, + }; + var (ctx, graph, _) = BuildIdempotencyTestContext(config, agentInstanceOnly, skipObservabilityPermissions); + graph.AgentRegistrationExistsAsync( + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns((bool?)null); + + await NonDwBlueprintSetupOrchestrator.ExecuteAgentIdentityAndRegistrationAsync( + ctx, specs: [], skipIdentityAndPermissions: agentInstanceOnly); + + ctx.Results.Errors.Should().ContainSingle(e => e.Contains("Could not verify agent registration"), + because: "an unverifiable registration cannot be relied on as the agent's only authorization, so setup must exit 1"); + ctx.Results.AgentInstanceRegistered.Should().BeFalse( + because: "the summary must not report a registration that could not be confirmed"); + ctx.Config.AgentRegistrationId.Should().Be("stored-reg-id", + because: "an auth or transient failure is not proof the registration is gone, so the stored ID is kept for the retry"); + await graph.DidNotReceive().RegisterAgentInstanceAsyncV2( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any(), Arg.Any()); + } + finally + { + Directory.Delete(projectDir, recursive: true); + } + } - ctx.Results.AgentInstanceId.Should().Be("stored-reg-id", - because: "when verification is inconclusive the stored ID must be preserved to avoid unintended re-registration"); - ctx.Results.AgentRegistrationAlreadyExisted.Should().BeTrue( - because: "an inconclusive verification is treated as 'assume still exists' to prevent data loss"); - await graph.DidNotReceive().RegisterAgentInstanceAsyncV2( + private static Agent365Config RegistrationReadyConfig(string deploymentProjectPath = "") => new() + { + AiTeammate = false, + TenantId = "tenant-id", + AgentBlueprintId = "blueprint-id", + AgentIdentityDisplayName = "Test Agent Identity", + ClientAppId = "client-app-id", + AgenticAppId = "agentic-app-id", + DeploymentProjectPath = deploymentProjectPath, + }; + + private static void StubRegistrationFailure(GraphApiService graph) => + graph.RegisterAgentInstanceAsyncV2( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), - Arg.Any(), Arg.Any(), Arg.Any()); + Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(((string?)null, false)); + + /// + /// Step 6 (--agent-registration-only): registration is the command's only purpose, so its failure must exit 1. + /// + [Fact] + public async Task Step6_AgentRegistrationOnly_ReturnsExitCode1_WhenRegistrationFails() + { + var (ctx, graph, _) = BuildIdempotencyTestContext(RegistrationReadyConfig()); + StubRegistrationFailure(graph); + + var exitCode = await NonDwBlueprintSetupOrchestrator.ExecuteAsync(ctx); + + exitCode.Should().Be(1, + because: "a registration-only run that did not register the agent failed, and scripts rely on the exit code"); + ctx.Results.Errors.Should().Contain(e => e.Contains("Agent registration failed"), + because: "the registration-only summary row points to the errors list for the failure details"); + } + + /// + /// Step 6: when Observability permissions are not requested, registration is the agent's only Observability + /// authorization, so its failure is an error; when they are requested (AI Teammate) it stays a warning. + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Step6_RegistrationFailureIsError_OnlyWhenObservabilityPermissionsSkipped(bool skipObservabilityPermissions) + { + // Empty project directory: the project settings step finds no project and writes nothing. + var projectDir = Path.Combine(Path.GetTempPath(), "NonDwRegistrationTests_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(projectDir); + try + { + var (ctx, graph, _) = BuildIdempotencyTestContext( + RegistrationReadyConfig(projectDir), agentInstanceOnly: false, skipObservabilityPermissions: skipObservabilityPermissions); + StubRegistrationFailure(graph); + + await NonDwBlueprintSetupOrchestrator.ExecuteAgentIdentityAndRegistrationAsync(ctx, specs: []); + + ctx.Results.AgentRegistrationFailed.Should().BeTrue(because: "precondition: the stubbed registration API returned no ID"); + ctx.Results.Errors.Any(e => e.Contains("Agent registration failed")).Should().Be(skipObservabilityPermissions, + because: "without OtelWrite an unregistered agent cannot export telemetry, so setup must fail (exit 1)"); + ctx.Results.Warnings.Any(w => w.Contains("Agent registration failed")).Should().Be(!skipObservabilityPermissions, + because: "when Observability permissions are requested the agent keeps OtelWrite, and a failed registration remains a non-fatal warning"); + } + finally + { + Directory.Delete(projectDir, recursive: true); + } } // ------------------------------------------------------------------------- diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs index 16410deb..b04c81c8 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupCommandTests.cs @@ -684,4 +684,122 @@ public async Task SetupAll_NoAuthMode_DefaultsToOboBehaviour() Arg.Any(), Arg.Any>()); } + + // ── Observability API permissions ────────────────────────────────────────── + + /// + /// Registered blueprint agents export telemetry with an app-only token, so the default (OBO) plan must not + /// request Observability API permissions — the admin consent they need is what this default removes. + /// + [Fact] + public async Task SetupAll_BlueprintAgent_DefaultPlan_OmitsObservabilityApi() + { + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(BlueprintConfig())); + var parser = new CommandLineBuilder(BuildSetupCommand()).Build(); + + var result = await parser.InvokeAsync("all --aiteammate false --dry-run", new TestConsole()); + + result.Should().Be(0, because: "a default blueprint-agent dry run is valid"); + _mockLogger.DidNotReceive().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Inheritable Permissions") && o.ToString()!.Contains("Observability")), + Arg.Any(), + Arg.Any>()); + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Observability API not requested")), + Arg.Any(), + Arg.Any>()); + } + + /// + /// The S2S endpoint authorizes registered agents without OtelWrite whatever the auth mode (validated live), so + /// s2s and both — from the flag or from a365.config.json — must not request Observability API permissions either. + /// + [Theory] + [InlineData("--authmode s2s", null)] + [InlineData("--authmode both", null)] + [InlineData("", "both")] + public async Task SetupAll_BlueprintAgent_AppRoleAuthModes_OmitObservabilityApi(string args, string? configAuthMode) + { + var config = new Agent365Config + { + TenantId = "tenant", + AgentIdentityDisplayName = "agent", + AgentBlueprintDisplayName = "TestBlueprint", + DeploymentProjectPath = ".", + AiTeammate = false, + UseBlueprint = true, + AuthMode = configAuthMode, + }; + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(config)); + var parser = new CommandLineBuilder(BuildSetupCommand()).Build(); + + var result = await parser.InvokeAsync($"all --aiteammate false {args} --dry-run", new TestConsole()); + + result.Should().Be(0, because: "s2s and both are valid blueprint-agent auth modes"); + _mockLogger.DidNotReceive().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Inheritable Permissions") && o.ToString()!.Contains("Observability")), + Arg.Any(), + Arg.Any>()); + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Observability API not requested")), + Arg.Any(), + Arg.Any>()); + } + + /// + /// AI Teammate setup is unchanged: its plan still requests Observability API permissions. + /// + [Fact] + public async Task SetupAll_AiTeammate_Plan_KeepsObservabilityApi() + { + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(BlueprintConfig())); + var parser = new CommandLineBuilder(BuildSetupCommand()).Build(); + + var result = await parser.InvokeAsync("all --aiteammate true --dry-run", new TestConsole()); + + result.Should().Be(0, because: "an AI Teammate dry run is valid"); + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Inheritable Permissions") && o.ToString()!.Contains("Observability API")), + Arg.Any(), + Arg.Any>()); + } + + /// + /// A dry run keeps an AI Teammate config even without --aiteammate; the plan must still treat it as an + /// AI Teammate and not claim Observability API permissions are skipped. + /// + [Fact] + public async Task SetupAll_AiTeammateConfig_DryRunWithoutFlag_DoesNotSkipObservabilityApi() + { + var config = new Agent365Config + { + TenantId = "tenant", + AgentIdentityDisplayName = "agent", + AgentBlueprintDisplayName = "TestBlueprint", + DeploymentProjectPath = ".", + AiTeammate = true, + }; + _mockConfigService.LoadAsync(Arg.Any(), Arg.Any()).Returns(Task.FromResult(config)); + var parser = new CommandLineBuilder(BuildSetupCommand()).Build(); + + var result = await parser.InvokeAsync("all --dry-run", new TestConsole()); + + result.Should().Be(0, because: "a dry run with an AI Teammate config is valid"); + _mockLogger.DidNotReceive().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Observability API not requested")), + Arg.Any(), + Arg.Any>()); + } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs index 674d6a52..a0991807 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/SetupSubcommands/PermissionSpecsTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.Agents.A365.DevTools.Cli.Tests.Commands.SetupSubcommands; /// input-driven rule that applies to both DW and non-DW agents: /// /// -/// Observability API and Power Platform API are always included. +/// Power Platform API is always included; Observability API is included unless includeObservability is false. /// Microsoft Graph is always included with AgentApplicationScopes. /// Messaging Bot API is included when isM365 == true. /// Agent 365 Tools (MCP audiences from ToolingManifest.json) are included when a manifest is present. diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs index 9607bbb4..6defb650 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersConsentUrlTests.cs @@ -307,6 +307,34 @@ public void PopulateAdminConsentUrls_NonM365_ResourceConsentsExcludeMessagingBot because: "no Messaging Bot consent URL is generated for non-M365 agents, so no resourceConsents entry should be persisted"); } + [Fact] + public void PopulateAdminConsentUrls_WithoutObservability_RemovesObservabilityEntryFromEarlierRun() + { + var config = new Agent365Config + { + TenantId = TenantId, + AgentBlueprintId = BlueprintClientId, + }; + config.ResourceConsents.Add(new ResourceConsent + { + ResourceName = "Observability API", + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + ConsentUrl = "https://login.microsoftonline.com/old-observability-consent", + }); + + var names = SetupHelpers.PopulateAdminConsentUrls( + config, McpConstants.WorkIQToolsProdAppId, new[] { "McpServers.Mail.All" }, + isM365: false, includeObservability: false); + + config.ResourceConsents.Should().NotContain( + rc => rc.ResourceAppId == ConfigConstants.ObservabilityApiAppId, + because: "an Observability consent URL saved by an earlier run must not keep asking the admin for permissions this run no longer requests"); + names.Should().NotContain("Observability API"); + config.ResourceConsents.Should().Contain( + rc => rc.ResourceAppId == PowerPlatformConstants.PowerPlatformApiResourceAppId, + because: "removing the stale Observability entry must not affect the resources that are still requested"); + } + // ── V2 per-server audience routing (issue #429) ────────────────────────── // // V2 manifest entries declare a per-server audience (a unique Entra appId) and the diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs index 4760b3bc..2a1d2856 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Helpers/SetupHelpersDisplaySetupSummaryTests.cs @@ -273,6 +273,59 @@ public void DisplaySetupSummary_NonDwAdminConsentPending_NoConsentUrl_FallsBackT because: "when no consent URL is available the non-DW summary must fall back to the LogNonDwAdminConsentInstructions portal walkthrough so the user still has a recovery path"); } + [Fact] + public void DisplaySetupSummary_ObservabilitySkipped_PortalWalkthroughOmitsObservability() + { + var logger = new CapturingLogger(); + var results = new SetupResults + { + IsNonDwBlueprintFlow = true, + ObservabilityPermissionsSkipped = true, + BlueprintCreated = true, + BlueprintId = BlueprintId, + AgentIdentityCreated = true, + AgentIdentityId = AgentSpId, + TenantId = TenantId, + EffectiveAuthMode = Cli.Models.AuthMode.Obo, + TenantWideConsentOutcome = Cli.Models.GrantOutcome.Failed, + BatchPermissionsPhase1Completed = true, + BatchPermissionsPhase2Completed = true, + }; + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain("Option A — Entra portal", + because: "precondition: without a consent URL the summary renders the portal walkthrough"); + logger.AllOutput.Should().NotContain(ConfigConstants.ObservabilityApiOtelWriteScope, + because: "the administrator must not be asked to add Observability API permissions that setup skipped"); + logger.AllOutput.Should().Contain(PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead, + because: "Power Platform API is still required and must stay in the walkthrough"); + } + + [Theory] + [InlineData(true, "failed — see errors")] + [InlineData(false, "failed — see warnings")] + public void DisplaySetupSummary_RegistrationFailed_RowPointsToTheListHoldingTheFailure(bool observabilitySkipped, string expectedStatus) + { + var logger = new CapturingLogger(); + var results = new SetupResults + { + IsNonDwBlueprintFlow = true, + ObservabilityPermissionsSkipped = observabilitySkipped, + BlueprintCreated = true, + BlueprintId = BlueprintId, + AgentIdentityCreated = true, + AgentIdentityId = AgentSpId, + AgentRegistrationFailed = true, + }; + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Split('\n').Should().ContainSingle(l => l.Contains("Agent Registration")) + .Which.Should().Contain(expectedStatus, + because: "a registration failure is recorded as an error when Observability permissions were skipped, so the row must point to the list that holds it"); + } + /// /// B2 regression — non-admin AID developer running `setup all` as OBO must see the consent URL /// surfaced as an action item. Pre-refactor, the orchestrator wrote a misleading