diff --git a/CHANGELOG.md b/CHANGELOG.md index 94ccb70e..9367e48d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,11 @@ 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): +**Option A — Entra portal** (existing blueprints that already have the Observability inheritable-permission entry): 1. [Entra portal](https://entra.microsoft.com) > **App registrations** > select your **Blueprint** app > **API permissions** 2. **Add a permission** > **APIs my organization uses** > search for the Observability app ID for your cloud: @@ -24,7 +24,9 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g 4. Repeat step 2 > **Application permissions** > select `Agent365.Observability.OtelWrite` > **Add permissions** 5. **Grant admin consent for \** > confirm -**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. +**Option B — CLI**: `a365 setup admin` has been removed in this release. For blueprint agents created after this release, run `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` to stamp the inheritable permission and request consent. For AI Teammates, the `a365 setup all` summary also prints the PowerShell steps for the application permission. + +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 - `a365 develop-mcp grant-agents-access --agent-blueprint-id --mcp-server-name ` reports which agent instances of a blueprint are missing the permission to call a BYO MCP server, and prompts you to select which ones to grant it to (#500). @@ -84,7 +86,8 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g - Cloud-specific Graph, authority, and Agent 365 Tools endpoint overrides now apply consistently across setup, consent, authentication, query, and create-instance flows for sovereign and custom clouds. (#478) - `a365 query-entra instance-scopes` now reports consent status correctly and fails visibly when permission grants cannot be read (#478). - `a365 publish` no longer crashes when `manifest.json` has a non-string `name.short` value (#478). -- `setup all --agent-registration-only` now exits non-zero and reports errors when the requested agent registration step fails, while full setup continues to treat registration as best-effort (#478). +- `setup all --agent-registration-only` now exits non-zero and reports errors when the requested agent registration step fails (#478). +- `a365 setup all` now exits with code 1 when blueprint-agent registration fails or cannot be verified, including `--agent-registration-only` runs with unverifiable existing registrations (#501). - `a365 develop-mcp grant-agents-access --device-code` no longer prompts repeatedly within a single command, and no longer fails in embedded or remote terminals where the sign-in prompt could not be displayed (#500). - 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). @@ -131,6 +134,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g ### Changed +- `a365 setup all` no longer requests Observability API permissions for blueprint agents; registered agents that export telemetry through the app-only S2S endpoint need no 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..f509abf4 100644 --- a/docs/agent365-guided-setup/a365-observability-instructions.md +++ b/docs/agent365-guided-setup/a365-observability-instructions.md @@ -10,6 +10,10 @@ Add Agent 365 observability to your agent at any point after `a365 setup all` ha > > **Prerequisite:** `a365 setup all` must have completed successfully so `AgentId` and `TenantId` values are present in `appsettings.json` (written by setup) or `a365.generated.config.json`. If neither source has these values, run `a365 setup all` first. +> **Source of truth:** For non-AI-Teammate blueprint agents, use the `plugins/agent365/skills/instrument-observability` skill from [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills). The skill contains the current, verified code for the app-only S2S exporter in .NET, Node.js, and Python. + +> **Blueprint telemetry model:** Every non-AI-Teammate blueprint agent exports telemetry through the S2S route (`/observabilityService/...`) with an app-only token for the exporting agent identity. Set the SDK route flag (`UseS2SEndpoint`, `useS2SEndpoint`, or `a365_use_s2s_endpoint`), wire an app-only token resolver, and do **not** use delegated Observability token caches or per-turn delegated OBS token refresh. Registered blueprint agents do not need `Agent365.Observability.OtelWrite`; `a365 setup all` no longer requests it for blueprint agents. + --- ## Overview @@ -71,11 +75,11 @@ If `agentType` and `authMode` are already present in the detection cache (from a Store `agentType` (`ai-teammate` = AI Teammate, or `system-agent` = Agent (Non AI Teammate)) and `authMode`: - **AI Teammate:** `user-delegated` (OBO as signed-in user) or `agentic-identity` (OBO as agent's own M365 identity) -- **Agent (Non AI Teammate):** `agentic-identity` (Assistive OBO) or `S2S` (Autonomous / Service Principal) +- **Agent (Non AI Teammate):** workload auth may be `obo`, `s2s`, or `both`, but telemetry wiring is always the app-only S2S exporter described above. **Update `.a365-workspace-detection.json`** — merge `agentType` and `authMode` into the existing cache file, preserving all other fields (`agentStack`, `programmingLanguage`, `usesTeamsOrCopilot`, `detectedAt`). Use the **Write** tool to write the merged object back. -The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry point wiring (Phase 3), message handler pattern (Phase 4), and token resolver (Phase 5). **Phases 2, 6, 7, and 8 are identical regardless of `authMode`.** +For non-AI-Teammate agents, `authMode` controls workload tokens and permission grants only; it must not switch telemetry back to a delegated-route exporter. Phases 3–5 must select the S2S route and app-only token resolver in every auth mode. **TaskUpdate** — Mark complete: "Determine agent type and authentication mode" @@ -112,13 +116,13 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin 1. **Bash** — Run package installation (path-dependent): - **OBO path** (`user-delegated` or `agentic-identity`): + **AI Teammate / legacy delegated path only**: ```bash dotnet add package Microsoft.Agents.A365.Observability.Runtime dotnet add package Microsoft.Agents.A365.Observability.Hosting ``` - **S2S / autonomous agents — use the unified distro** (preferred; do NOT also add Runtime/Hosting): + **Non-AI-Teammate blueprint agents — use the app-only S2S distro in every workload auth mode** (preferred; do NOT also add Runtime/Hosting): ```bash dotnet add package Microsoft.OpenTelemetry --version 1.0.0-beta.1 dotnet add package Azure.Identity @@ -173,6 +177,8 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin npm install @microsoft/agents-a365-observability npm install @microsoft/agents-a365-runtime npm install @microsoft/agents-a365-observability-hosting + # Non-AI-Teammate blueprint telemetry uses app-only S2S in every workload auth mode. + npm install @azure/msal-node @azure/identity ``` 2. **Optional auto-instrumentation extensions** — ask the user which AI framework they use. @@ -200,10 +206,10 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin 1. **Bash** — Run package installation (unified distro + S2S deps): ```bash pip3 install microsoft-opentelemetry 2>/dev/null || pip install microsoft-opentelemetry - # S2S path also requires: + # Non-AI-Teammate blueprint telemetry uses app-only S2S in every workload auth mode. pip3 install msal azure-identity httpx 2>/dev/null || pip install msal azure-identity httpx ``` - > **OBO path:** only `microsoft-opentelemetry` is required. The `msal`, `azure-identity`, and `httpx` packages are only needed for the S2S token service. + > **AI Teammate / legacy delegated path:** follow the AI Teammate sample's delegated hosting package guidance instead of the blueprint app-only resolver. 2. **Optional auto-instrumentation extensions** — ask the user which AI framework they use and install accordingly: ```bash @@ -239,9 +245,10 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin 2. **Edit** — Add observability wiring following the reference pattern in `dotnet-observability.md`: - Add using directives for the observability namespaces - - **OBO path** (`user-delegated` or `agentic-identity`): call `builder.Services.AddAgenticTracingExporter();` then `builder.AddA365Tracing();` - - **S2S path**: First **Write** the two scaffold files from the reference doc — `Observability/ObservabilityServiceExtensions.cs` (DI extension with `AddAgent365Observability()` using `ServiceTokenCache` and conditional `ObservabilityTokenService`) and `Observability/ObservabilityTokenService.cs` (background service that acquires the Observability API token via the MSAL FMI 3-hop chain with `.WithFmiPath()` targeting scope `api://9b975845-388f-4429-889e-eab1ef63949c/.default`, supports MSI with client-secret fallback). Then call `builder.Services.AddAgent365Observability();` and `builder.UseMicrosoftOpenTelemetry(...)` with token resolver reading from the `ServiceTokenCache`. **Critical:** Set `o.Agent365.Exporter.UseS2SEndpoint = true` in the options callback — without this, the exporter posts to the wrong path (`/observability/` instead of `/observabilityService/`) and gets HTTP 401. See "Known Issues" section. - - Optionally register `adapter.Use(new BaggageTurnMiddleware())` (OBO path only) to auto-populate baggage on every request + - **Non-AI-Teammate blueprint agents:** follow the .NET reference in [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills) (`plugins/agent365/skills/instrument-observability/references/dotnet-observability.md`). The required shape is `UseMicrosoftOpenTelemetry(...)` with `o.Agent365.UseS2SEndpoint = true` (or `o.Agent365.Exporter.UseS2SEndpoint = true` on older distros) plus an app-only token resolver for the exporting agent identity. + - **Do not** call `AddAgenticTracingExporter()` for non-AI-Teammate blueprint telemetry; it wires the delegated route and delegated token cache. + - **AI Teammate only:** keep the existing hosting-package pattern if that is what the AI Teammate sample uses. + - Optionally register baggage middleware only when it does not introduce a delegated Observability token dependency. - Mark all new lines with: `// A365 Observability — best-effort instrumentation (verify against official sample)` 3. **Preserve** all existing code — only add new lines, never remove. @@ -251,10 +258,10 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin 1. **Read** the current entry point (`index.ts`, `app.ts`, or detected file). 2. **Edit** — Add observability initialization following the reference pattern in `nodejs-observability.md`: - - Add imports for `ObservabilityManager` from `@microsoft/agents-a365-observability` - - **OBO path**: Call `useMicrosoftOpenTelemetry({ a365: { enabled: true, tokenResolver } })` from `@microsoft/opentelemetry` **before** any LLM/framework imports. The `tokenResolver` reads from `AgenticTokenCacheInstance`. - - **S2S path**: First **Write** `observability/token-cache.ts` (in-memory token cache with `cacheToken`/`getCachedToken`/`tokenResolver`) and `observability/observability-token-service.ts` using the scaffold pattern from `nodejs-observability.md` (S2S section). This module acquires the Observability API token via MSAL FMI 3-hop chain (`@azure/msal-node` with `fmiPath` parameter, targeting scope `api://9b975845-388f-4429-889e-eab1ef63949c/.default`, supports MSI with client-secret fallback) and refreshes it every 50 min. Then call `useMicrosoftOpenTelemetry()` with the S2S workaround pattern from `nodejs-observability.md` (custom `Agent365Exporter` + `A365SpanProcessor` via `spanProcessors` when `AGENT365_USE_S2S_ENDPOINT=true`). Set `ENABLE_A365_OBSERVABILITY_EXPORTER=false` in `.env`. Also run `npm install @microsoft/opentelemetry @azure/msal-node @azure/identity @opentelemetry/sdk-trace-base`. - - Optionally register `adapter.use(new BaggageMiddleware())` (OBO path) to auto-populate baggage on every request + - Initialize `useMicrosoftOpenTelemetry({ a365: { enabled: true, useS2SEndpoint: true, tokenResolver } })` **before** any LLM/framework imports. + - Implement `tokenResolver` as an app-only resolver for the exporting agent identity, using the current Node reference in [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills). + - **Do not** read from `AgenticTokenCacheInstance` for non-AI-Teammate blueprint telemetry and do not call `RefreshObservabilityToken` to obtain exporter tokens; those produce delegated tokens for the wrong route. + - Optionally register `BaggageMiddleware` only for baggage propagation; it must not be used as the exporter token source. - Mark all new lines with: `// A365 Observability — best-effort instrumentation (verify against official sample)` 3. **Preserve** all existing code — only add new lines, never remove. @@ -265,9 +272,9 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin 2. **Edit** — Add observability configuration following the reference pattern in `python-observability.md`: - Add `from microsoft.opentelemetry import use_microsoft_opentelemetry` and call `use_microsoft_opentelemetry(enable_a365=True, a365_token_resolver=...)` with `service_name` and `service_namespace` - - **OBO path**: Wire `a365_token_resolver` to return the cached agentic token from `token_cache.py`. - - **S2S path**: First **Write** `observability/token_cache.py` (in-memory token cache with `cache_token`/`get_cached_token`) and `observability/observability_token_service.py` using the scaffold pattern from `python-observability.md` (S2S section). This module acquires the Observability API token via MSAL FMI 3-hop chain (`msal.ConfidentialClientApplication` with `fmi_path` parameter, targeting scope `api://9b975845-388f-4429-889e-eab1ef63949c/.default`, supports MSI with client-secret fallback) and refreshes it every 50 min via an `asyncio` background task. Then call `use_microsoft_opentelemetry(enable_a365=True, a365_token_resolver=...)` from `microsoft.opentelemetry` and schedule `run_token_service()` as an asyncio task. Also install `msal` and `azure-identity` if not already present. - - Optionally register `BaggageMiddleware` or use `ObservabilityHostingManager` on the adapter (OBO path) to auto-populate baggage on every request + - Pass `a365_use_s2s_endpoint=True` and an app-only `a365_token_resolver` for the exporting agent identity, using the current Python reference in [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills). + - **Do not** wire `a365_token_resolver` to a delegated `AgenticTokenCache` token for non-AI-Teammate blueprint telemetry. + - Optionally register baggage helpers only for baggage propagation; they must not perform a delegated Observability token exchange. - Mark all new lines with: `# A365 Observability — best-effort instrumentation (verify against official sample)` 3. **Preserve** all existing code — only add new lines, never remove. @@ -284,40 +291,18 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin > baggage propagation automatically for every request. > **Auth mode note:** All three `authMode` values use `authHandlerName: "AGENTIC"` in the -> code — the token exchange call is identical. The identity in traces is determined by Azure AD -> provisioning and the incoming token. Add an inline comment indicating which mode was chosen. +> workload-auth code, but telemetry export for non-AI-Teammate blueprint agents is always app-only S2S. +> Do not add delegated Observability token refresh to a message handler. ### For .NET AgentFramework 1. **Read** the detected message handler file. -2. **Edit** — Follow the reference pattern in `dotnet-observability.md` based on `authMode`: - - **OBO path** (`user-delegated` or `agentic-identity`): - - Inject `IExporterTokenCache` in the constructor - - Use `new BaggageBuilder().FromTurnContext(turnContext).Build()` — requires `using Microsoft.Agents.A365.Observability.Hosting.Extensions;`; `Build()` returns `IDisposable`, use `using var` - - Call `RegisterObservability` with all four arguments per turn (wrap in try/catch — non-fatal): - ```csharp - _agentTokenCache.RegisterObservability( - turnContext.Activity.Recipient.AgenticAppId, - turnContext.Activity.Recipient.TenantId, - new AgenticTokenStruct( - userAuthorization: UserAuthorization, - turnContext: turnContext, - authHandlerName: "AGENTIC"), - EnvironmentUtils.GetObservabilityAuthenticationScope() - ); - ``` - - `user-delegated`: token exchange resolves to the **signed-in user's** identity → traces attributed to the user - - `agentic-identity`: token exchange resolves to the **agentic user** provisioned in Azure AD → traces attributed to the agent - - Add inline comment: `// A365 auth mode: {authMode} — see: https://learn.microsoft.com/en-us/entra/agent-id/agent-on-behalf-of-oauth-flow` - - **S2S path**: - - Inject `Agent365ObservabilityContext` (singleton registered by `AddAgent365Observability()`) in the constructor — **not** `IExporterTokenCache` - - **Baggage:** Use `new BaggageBuilder().FromTurnContext(turnContext).Build()` as a separate `using var baggageScope` — `FromTurnContext()` is an extension on `BaggageBuilder` **only**; it does not exist on `InvokeAgentScope` or any scope type - - **Scope:** Use `InvokeAgentScope.Start(new Request(...), new InvokeAgentScopeDetails(endpoint: new Uri("...")), _obs.AgentDetails, callerDetails)` as a separate `using var scope` — `InvokeAgentScopeDetails` has **no parameterless constructor**; always pass at least `endpoint`. `CallerDetails` with the blueprint sponsor's identity is **required** for S2S traces to appear in the portal - - **No** per-turn `RegisterObservability()` call; **no** `.FromTurnContext()` chaining on the scope - - Add inline comment: `// A365 auth mode: S2S — FMI 3-hop chain via ObservabilityTokenService (scope: api://9b975845-388f-4429-889e-eab1ef63949c/.default)` +2. **Edit** — Follow the current .NET pattern from the `instrument-observability` skill: + - Use `new BaggageBuilder().FromTurnContext(turnContext).Build()` as baggage context. + - Use the app-only `AgentDetails` / `CallerDetails` pattern from the reference when creating manual scopes. + - **Do not** inject `IExporterTokenCache` or call `RegisterObservability` for non-AI-Teammate blueprint telemetry; the exporter token is supplied by the app-only resolver configured in Phase 3. + - Add inline comment: `// A365 telemetry: app-only S2S route; workload auth mode: {authMode}` Mark all new lines with: `// A365 Observability — best-effort instrumentation (verify against official sample)` @@ -329,15 +314,11 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin 2. **Edit** — Add BaggageBuilder context following the reference pattern in `nodejs-observability.md`: - Import `BaggageBuilder` from `@microsoft/agents-a365-observability` - - Import `AgenticTokenCacheInstance`, `BaggageBuilderUtils` from `@microsoft/agents-a365-observability-hosting` - - Import `getObservabilityAuthenticationScope` from `@microsoft/agents-a365-runtime` - - **OBO paths only** (`user-delegated` / `agentic-identity`): Call `AgenticTokenCacheInstance.RefreshObservabilityToken(agentId, tenantId, context, authorization, scopes)` at the start of each turn (non-fatal, wrap in try/catch): - - `user-delegated`: `authorization` is the **user's** delegated token → traces attributed to the user - - `agentic-identity`: `authorization` resolves to the **agentic user** provisioned in Azure AD → traces attributed to the agent - - **S2S path**: Do **NOT** call `AgenticTokenCacheInstance.RefreshObservabilityToken` — there is no user authorization token. The `tokenResolver` passed to `useMicrosoftOpenTelemetry()` (set up in Phase 3) handles authentication via the FMI 3-hop chain token service. + - Import `BaggageBuilderUtils` from `@microsoft/agents-a365-observability-hosting` + - **Do not** call `AgenticTokenCacheInstance.RefreshObservabilityToken` for non-AI-Teammate blueprint telemetry. The `tokenResolver` passed to `useMicrosoftOpenTelemetry()` supplies an app-only token for the S2S route. - Use `BaggageBuilderUtils.fromTurnContext(new BaggageBuilder(), context).build()` to build baggage automatically from TurnContext - Wrap the handler body in `await baggageScope.run(async () => { ... })` - - Add inline comment: `// A365 auth mode: {authMode} — see: https://learn.microsoft.com/en-us/entra/agent-id/agent-on-behalf-of-oauth-flow` + - Add inline comment: `// A365 telemetry: app-only S2S route; workload auth mode: {authMode}` - Mark all new lines with: `// A365 Observability — best-effort instrumentation (verify against official sample)` 3. **Preserve** all existing handler logic. @@ -349,15 +330,10 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin 2. **Edit** — Add BaggageBuilder context following the reference pattern in `python-observability.md`: - Import `BaggageBuilder` from `microsoft.opentelemetry.a365.core` - Import `populate` from `microsoft.opentelemetry.a365.hosting.scope_helpers.populate_baggage` - - Import `AgenticTokenCache`, `AgenticTokenStruct` from `microsoft.opentelemetry.a365.hosting.token_cache_helpers` - - Import `get_observability_authentication_scope` from `microsoft.opentelemetry.a365.runtime` - - Call `token_cache.register_observability(agent_id=..., tenant_id=..., token_generator=AgenticTokenStruct(authorization=AGENT_APP.auth, turn_context=context), observability_scopes=get_observability_authentication_scope())`: - - `user-delegated`: the OBO exchange resolves to the **signed-in user's** identity - - `agentic-identity`: the OBO exchange resolves to the **agentic user** provisioned in Azure AD - - `S2S`: agent authenticates as itself — no user context available + - **Do not** call `cache_agentic_token`, `register_observability`, or `exchange_token(...get_observability_authentication_scope...)` for non-AI-Teammate blueprint telemetry. The `a365_token_resolver` configured in Phase 3 supplies an app-only token for the S2S route. - Use `populate(builder, turn_context)` to auto-populate baggage, then `with builder.build():` - Wrap existing agent logic inside the baggage scope - - Add inline comment: `# A365 auth mode: {authMode} — see: https://learn.microsoft.com/en-us/entra/agent-id/agent-on-behalf-of-oauth-flow` + - Add inline comment: `# A365 telemetry: app-only S2S route; workload auth mode: {authMode}` - Mark all new lines with: `# A365 Observability — best-effort instrumentation (verify against official sample)` 3. **Preserve** all existing handler logic. @@ -370,41 +346,33 @@ The `authMode` value drives Phases 3–5: OBO and S2S paths differ in entry poin **TaskCreate** — "Implement agentic token resolver with caching" -For AI Teammate agents using the hosting packages, the built-in token cache (`AddAgenticTracingExporter` for .NET, `AgenticTokenCacheInstance` for Node.js, `AgenticTokenCache` for Python) handles caching automatically — no custom resolver needed. Skip to step 3 for these agents. +For non-AI-Teammate blueprint agents, Phase 5 means "ensure the app-only S2S token resolver from the `instrument-observability` skill is present." Do not create or use delegated Observability token caches. For AI Teammate agents using the hosting packages, the built-in delegated token cache may still be the sample's expected pattern. -### For .NET AgentFramework (hosting path) +### For .NET AgentFramework (AI Teammate / legacy delegated path only) 1. `AddAgenticTracingExporter()` (registered in Phase 3) provides the `IExporterTokenCache` DI instance — no additional token resolver class needed. 2. In the agent class, inject `IExporterTokenCache` in the constructor and call `RegisterObservability(...)` per turn (already done in Phase 4). -### For .NET AgentFramework (S2S path) - -The `ObservabilityTokenService` background service (created in Phase 3 via the scaffold) acquires and refreshes the Observability API token automatically via the FMI 3-hop chain (Blueprint → Agent Identity → Power Platform PFAT token) — no manual `TokenResolver` delegate needed. - -1. **Check** if `Observability/ObservabilityServiceExtensions.cs` and `Observability/ObservabilityTokenService.cs` exist. If yes, **skip** — they were already created in Phase 3. +### For .NET AgentFramework (blueprint agents) -2. **If absent** (Phase 3 was skipped or re-running the skill on a partial state), create them now following the S2S scaffold patterns in `dotnet-observability.md`. These files provide `AddAgent365Observability()` (DI extension registering `AddServiceTracingExporter`, `ObservabilityTokenService`, and `Agent365ObservabilityContext`) and `ObservabilityTokenService` (background service that acquires the Observability API token via the FMI 3-hop chain and refreshes it every 50 minutes). +Use the app-only token resolver pattern from [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills), set the S2S route flag, and keep delegated OBS token registration out of the handler. -### For Node.js (OBO path) +### For Node.js (AI Teammate / legacy delegated path only) -`AgenticTokenCacheInstance` from `@microsoft/agents-a365-observability-hosting` handles caching automatically. The `useMicrosoftOpenTelemetry()` call in Phase 3 wires it as the `tokenResolver`. No additional token resolver module is needed unless `Use_Custom_Resolver=true` is required (see reference doc for custom resolver pattern). +`AgenticTokenCacheInstance` from `@microsoft/agents-a365-observability-hosting` handles delegated token caching automatically. Do not use it for non-AI-Teammate blueprint telemetry. -### For Node.js (S2S path) +### For Node.js (blueprint agents) -**Check** if `observability/observability-token-service.ts` exists. If yes, **skip** — it was created in Phase 3. +Create or reuse the app-only resolver from the `instrument-observability` skill and pass it to `useMicrosoftOpenTelemetry({ a365: { useS2SEndpoint: true, tokenResolver } })`. -**If absent** (Phase 3 was skipped or re-running), create `observability/token-cache.ts` and `observability/observability-token-service.ts` now using the scaffold from `nodejs-observability.md` (S2S section). The token service uses MSAL (`@azure/msal-node`) with `fmiPath` to acquire tokens via the FMI 3-hop chain targeting scope `api://9b975845-388f-4429-889e-eab1ef63949c/.default`. Call `startTokenService(config)` at app startup and pass `tokenResolver` from the cache module to `useMicrosoftOpenTelemetry()`. +### For Python (AI Teammate / legacy delegated path only) -### For Python (OBO path) +`AgenticTokenCache` from `microsoft.opentelemetry.a365.hosting.token_cache_helpers` handles delegated token caching automatically. Do not use it for non-AI-Teammate blueprint telemetry. -`AgenticTokenCache` from `microsoft.opentelemetry.a365.hosting.token_cache_helpers` handles caching automatically. It was wired as the `token_resolver` in the `configure()` call in Phase 3. No additional module is needed. +### For Python (blueprint agents) -### For Python (S2S path) - -**Check** if `observability/observability_token_service.py` exists. If yes, **skip** — it was created in Phase 3. - -**If absent**, create `observability/token_cache.py` and `observability/observability_token_service.py` now using the scaffold from `python-observability.md` (S2S section). The token service uses MSAL (`msal.ConfidentialClientApplication`) with `fmi_path` to acquire tokens via the FMI 3-hop chain targeting scope `api://9b975845-388f-4429-889e-eab1ef63949c/.default`. Call `acquire_initial_token()` for pre-warm, schedule `run_token_service()` as `asyncio.create_task()`, and pass `token_cache.get_cached_token` as the `a365_token_resolver` in `use_microsoft_opentelemetry()`. +Create or reuse the app-only resolver from the `instrument-observability` skill and pass it to `use_microsoft_opentelemetry(..., a365_use_s2s_endpoint=True, a365_token_resolver=...)`. **TaskUpdate** — Mark complete. @@ -775,26 +743,35 @@ 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 the `Agent365.Observability.OtelWrite` application role to the **blueprint service principal**, 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): +For a **blueprint agent still exporting through the delegated (OBO) route**, prefer the CLI opt-back-in path (requires Global Administrator): + +```bash +a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite +``` + +This stamps the Observability inheritable-permission entry on the blueprint and requests admin consent. The Entra portal path below works for existing blueprints only when that inheritable-permission entry already exists. + +If the AI Teammate script was not captured, grant the permission manually via Entra portal (requires Global Administrator): 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` +2. Add a permission > APIs my organization uses > search the Observability app ID for your cloud (commercial: `9b975845-388f-4429-889e-eab1ef63949c`; see `src/Microsoft.Agents.A365.DevTools.Cli/design.md#environment-variable-overrides` for other clouds) 3. Add both **Delegated** and **Application** `Agent365.Observability.OtelWrite` > Grant admin consent -Alternatively, read the `agentIdentityClientId` from `a365.generated.config.json` and use the Graph API: +Alternatively for **AI Teammates**, look up the Blueprint and Observability enterprise applications and create the application-role assignment the S2S route accepts: -```bash -# Create a temp JSON body file (required on Windows due to az rest escaping) -echo '{"principalId":"","resourceId":"2a275186-1775-4439-8551-5438df22cdfc","appRoleId":"8f71190c-00c8-461d-a63b-f74abde9ba52"}' > body.json -az rest --method POST --url "https://graph.microsoft.com/v1.0/servicePrincipals//appRoleAssignments" --body @body.json -rm body.json +```powershell +$blueprintSp = Get-MgServicePrincipal -Filter "appId eq ''" +$obsSp = Get-MgServicePrincipal -Filter "appId eq ''" +$roleId = ($obsSp.AppRoles | Where-Object { $_.Value -eq 'Agent365.Observability.OtelWrite' }).Id +New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $blueprintSp.Id -PrincipalId $blueprintSp.Id -ResourceId $obsSp.Id -AppRoleId $roleId ``` -- `resourceId` `2a275186-...` is the Observability API SP object ID -- `appRoleId` `8f71190c-...` is the OtelWrite role ID +- `` is the resource application ID, not the service-principal object ID. - For agents provisioned before CLI 1.1, this manual step is still required ### Node.js and .NET SDK `/otlp/` URL Path Bug diff --git a/docs/agent365-guided-setup/a365-setup-instructions.md b/docs/agent365-guided-setup/a365-setup-instructions.md index de61eaa1..863a66ba 100644 --- a/docs/agent365-guided-setup/a365-setup-instructions.md +++ b/docs/agent365-guided-setup/a365-setup-instructions.md @@ -429,7 +429,7 @@ After `a365 setup all` completes, show the user exactly this — nothing more, n 4. After showing the CLI output sections above, output exactly one of these closing lines — choose based on what the CLI reported: - **If the CLI printed an admin consent action item** (i.e., you showed a PowerShell script in step 2 above): > "Your agent is provisioned. Have a Global Admin run the PowerShell script above to complete admin consent." - - **If Permission Grants row in the Summary shows `granted`** (no action item was printed): + - **If Permission Grants row in the Summary shows `granted` or `not required`** (no action item was printed): > "Your agent is provisioned." ### Step 4 completion diff --git a/docs/agent365-guided-setup/references/dotnet-observability.md b/docs/agent365-guided-setup/references/dotnet-observability.md index 3b046a8c..b92dd52e 100644 --- a/docs/agent365-guided-setup/references/dotnet-observability.md +++ b/docs/agent365-guided-setup/references/dotnet-observability.md @@ -4,6 +4,8 @@ Authoritative package versions and code patterns for instrumenting A365 observab into a .NET AgentFramework agent. All samples mirror the official Microsoft Learn docs (updated 2026-04-30). +> **Blueprint agents:** Prefer the current `instrument-observability` skill in [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills). Non-AI-Teammate blueprint agents must export on the S2S route with an app-only token resolver; do not wire `AddAgenticTracingExporter()` or delegated OBS token refresh for those agents. + --- ## NuGet Packages @@ -188,7 +190,7 @@ public static class ObservabilityServiceExtensions > - `true` (production) — MSI → Blueprint FIC → Agent Identity → API > - `false` (local dev) — Client Secret → Blueprint FIC → Agent Identity → API > -> **Note:** As of CLI 1.1, `a365 setup all` automatically grants `Agent365.Observability.OtelWrite` to the Agent Identity SP (both delegated and application). No manual role assignment is needed for newly provisioned agents. +> **Note:** `a365 setup all` no longer requests `Agent365.Observability.OtelWrite` for blueprint agents. Registered blueprint agents that use the app-only S2S endpoint need no Observability admin consent. Agents that still export through the delegated route must opt back in with `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` (commercial: `9b975845-388f-4429-889e-eab1ef63949c`; other clouds are listed under [Environment Variable Overrides](../../../src/Microsoft.Agents.A365.DevTools.Cli/design.md#environment-variable-overrides)). ```csharp using Azure.Core; @@ -932,7 +934,7 @@ The `a365 setup` command (as of April 2026) automatically writes the following t | No logs in Defender | Missing `Logging.LogLevel` config | Add `Microsoft.Agents.A365.Observability: Debug` to appsettings.json | | `AgenticAppId` is null | Missing `AGENTIC_APP_ID` env var | Set it in `.env` or App Service config | | Token resolver returns null | `AddAgenticTracingExporter()` not called | Add to `Program.cs` DI | -| 401 from A365 exporter | OAuth consent not granted | Run `a365 setup permissions observability`; also check if upgrading past `0.3-beta` (requires new `Agent365.Observability.OtelWrite` permission) | +| 401 from A365 exporter | Delegated route or delegated-token wiring is still in use | For blueprint agents, set `UseS2SEndpoint` and use an app-only token resolver. If you intentionally use the delegated route, grant OtelWrite with `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` (commercial example: `9b975845-388f-4429-889e-eab1ef63949c`; see the per-cloud table in the CLI design) | | Build error on `BaggageBuilder` | Wrong namespace | Use `Microsoft.Agents.A365.Observability.Runtime.Common` | | Build error on `AgenticTokenStruct` | Object initializer syntax used | Use constructor: `new AgenticTokenStruct(userAuthorization: ..., turnContext: ..., authHandlerName: "AGENTIC")` | | Build error on `IExporterTokenCache` | Wrong namespace | Use `Microsoft.Agents.A365.Observability.Hosting.Caching` | @@ -940,9 +942,9 @@ The `a365 setup` command (as of April 2026) automatically writes the following t | Build error on `AddA365Tracing` | Wrong namespace | Use `Microsoft.Agents.A365.Observability.Runtime` | | Spans dropped silently | Missing tenant/agent ID in baggage | Ensure `BaggageBuilder` is set up before creating spans, or register `BaggageTurnMiddleware` | | S2S: token service skipped at startup | Placeholder or missing `Agent365Observability` credentials | Run `a365 setup all` or populate `TenantId`, `AgentId`, `ClientId`, and `ClientSecret` (when `UseManagedIdentity` is `false`) | -| S2S: 401 on export | Token acquired for wrong scope or app | Verify FMI Hop 3 scope is `api://9b975845-388f-4429-889e-eab1ef63949c/.default`. For agents provisioned before CLI 1.1, verify Agent Identity SP has `Agent365.Observability.OtelWrite` app role via Entra portal | +| S2S: 401 on export | Token acquired for wrong scope/app or the exporter is not on the S2S route | Verify FMI Hop 3 scope is `api://9b975845-388f-4429-889e-eab1ef63949c/.default`, set the S2S route flag, and ensure the token is app-only for the exporting agent identity | | S2S: FMI Hop 1+2 fails | Blueprint credentials wrong or `.WithFmiPath(agentId)` target incorrect | Check `ClientId` (Blueprint app ID) and `ClientSecret` in appsettings; verify `AgentId` matches the Agent Identity app ID | -| S2S: FMI Hop 3 → 401 on export | Wrong scope or missing role | FMI Hop 3 scope is `api://9b975845-388f-4429-889e-eab1ef63949c/.default`; Agent Identity SP needs `OtelWrite` role assigned via Graph API | +| S2S: 403 `insufficient_scope` on export | Agent instance is not registered, or AI Teammate OtelWrite app-role grant is incomplete | Blueprint agents: run `a365 setup all --agent-registration-only` and retry. AI Teammates: complete the OtelWrite application-role PowerShell step printed by `a365 setup all --aiteammate` | | S2S: MSI fails locally | No Managed Identity available in dev | Set `UseManagedIdentity: false` in appsettings.Development.json, ensure `ClientSecret` is populated | | S2S: `UseMicrosoftOpenTelemetry` not found | Unified distro not installed | Run `dotnet add package Microsoft.OpenTelemetry --version 1.0.0-beta.1` | | S2S: Runtime `FileNotFoundException` for `Microsoft.Extensions.Logging v10.0.0` | `Microsoft.OpenTelemetry` v1.0.0-beta.1 depends on v10 logging | (1) Upgrade TFM to `net9.0`. (2) Run `dotnet add package Microsoft.Extensions.Logging --version "10.0.4"` — use the **stable** version, not a preview; specifying a preview causes NU1605 downgrade errors because `Microsoft.Agents.A365.Observability.Hosting` requires `>= 10.0.4`. | diff --git a/docs/agent365-guided-setup/references/nodejs-observability.md b/docs/agent365-guided-setup/references/nodejs-observability.md index 31ad9085..683f0ad8 100644 --- a/docs/agent365-guided-setup/references/nodejs-observability.md +++ b/docs/agent365-guided-setup/references/nodejs-observability.md @@ -3,6 +3,8 @@ Authoritative package versions and code patterns for instrumenting A365 observability into a Node.js agent. All samples mirror the official Microsoft Learn docs (updated 2026-04-30). +> **Blueprint agents:** Prefer the current `instrument-observability` skill in [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills). Non-AI-Teammate blueprint agents must export on the S2S route with an app-only token resolver; do not wire `AgenticTokenCacheInstance` or delegated OBS token refresh for those agents. + --- ## npm Packages @@ -109,7 +111,7 @@ No OBO user token is required. > standard client-credential request. This workaround will be removed once MSAL ships native > `fmiPath` support for the client-secret credential path. -> **Note:** `a365 setup all` attempts to grant `Agent365.Observability.OtelWrite` to the Agent Identity SP, but this requires **Global Administrator** privileges. If the assignment fails (403), a Global Admin must manually grant the role via Entra portal — otherwise trace exports will return HTTP 403. +> **Note:** `a365 setup all` no longer requests `Agent365.Observability.OtelWrite` for blueprint agents. Registered blueprint agents that use the app-only S2S endpoint need no Observability admin consent. Agents that still export through the delegated route must opt back in with `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` (commercial: `9b975845-388f-4429-889e-eab1ef63949c`; other clouds are listed under [Environment Variable Overrides](../../../src/Microsoft.Agents.A365.DevTools.Cli/design.md#environment-variable-overrides)). > **IMPORTANT — SDK `useS2SEndpoint` bug (v0.1.0-beta.1):** The `@microsoft/opentelemetry` > distro does **not** pass `useS2SEndpoint` to `Agent365Exporter`. The exporter defaults @@ -1045,13 +1047,13 @@ setLogger({ | `Cannot find module '@microsoft/agents-a365-observability'` | Package not installed | Run `npm install @microsoft/agents-a365-observability` | | `Cannot find module '@microsoft/agents-a365-observability-hosting'` | Package not installed | Run `npm install @microsoft/agents-a365-observability-hosting` | | Traces not in Admin Center | Exporter env var not set | Set `ENABLE_A365_OBSERVABILITY_EXPORTER=true` in production | -| 401 on export | Missing permission | Check if upgrading past `0.2.0-preview.1` (requires new `Agent365.Observability.OtelWrite` permission) | +| 401 on export | Delegated route or delegated-token wiring is still in use | For blueprint agents, set `useS2SEndpoint: true` and use an app-only token resolver. If you intentionally use the delegated route, grant OtelWrite with `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` (commercial example: `9b975845-388f-4429-889e-eab1ef63949c`; see the per-cloud table in the CLI design) | | Spans dropped silently | Missing tenant/agent ID | Ensure `BaggageBuilder` (or `BaggageMiddleware`) populates tenant/agent ID before creating spans | | TypeScript error on `agentAuid` in `AgentDetails` | Interface field is `agentAUID` (uppercase UID), not `agentAuid` | Change to `agentAUID: '...'` | | `extensions-openai` install fails / peer dep error | Missing `@openai/agents` peer dep | Run `npm install @openai/agents@^0.7.0` first; this is the OpenAI Agents SDK, not the `openai` package | | S2S: AADSTS82001 or AADSTS1002012 | Direct MSAL client credentials not supported | Use the 3-hop FMI chain: Blueprint → FMI path → Agent Identity → Observability API token. | -| S2S: 401 on export | Token scope mismatch | Ensure Hop 3 scope is `api://9b975845-388f-4429-889e-eab1ef63949c/.default`. Also ensure Agent Identity SP has OtelWrite role assigned | -| S2S: 403 on `observabilityService/` endpoint | Missing app role | Assign `Agent365.Observability.OtelWrite` to the **Agent Identity** SP (not just the Blueprint) via Graph API | +| S2S: 401 on export | Token scope mismatch, delegated token, or wrong route | Ensure Hop 3 scope is `api://9b975845-388f-4429-889e-eab1ef63949c/.default`, set `useS2SEndpoint: true`, and use an app-only token for the exporting agent identity | +| S2S: 403 `insufficient_scope` on `observabilityService/` endpoint | Agent instance is not registered, or AI Teammate OtelWrite app-role grant is incomplete | Blueprint agents: run `a365 setup all --agent-registration-only` and retry. AI Teammates: complete the OtelWrite application-role PowerShell step printed by `a365 setup all --aiteammate` | | S2S: MSI fails locally | No Managed Identity in dev | Set `AGENT365_USE_MANAGED_IDENTITY=false` and provide `AGENT365_CLIENT_SECRET` | | S2S: token resolver never called | `RefreshObservabilityToken` called for S2S | Remove `AgenticTokenCacheInstance.RefreshObservabilityToken` — not used in S2S; token comes from `a365.tokenResolver` in `useMicrosoftOpenTelemetry(...)` | | `fromTurnContext` not found on `BaggageBuilder` | Static method is on `BaggageBuilderUtils`, not `BaggageBuilder` | Use `BaggageBuilderUtils.fromTurnContext(new BaggageBuilder(), context)` | diff --git a/docs/agent365-guided-setup/references/python-observability.md b/docs/agent365-guided-setup/references/python-observability.md index 3f7e4f58..184a55ac 100644 --- a/docs/agent365-guided-setup/references/python-observability.md +++ b/docs/agent365-guided-setup/references/python-observability.md @@ -3,6 +3,8 @@ Authoritative package versions and code patterns for instrumenting A365 observability into a Python agent. All samples mirror the official Microsoft Learn docs (updated 2026-04-30). +> **Blueprint agents:** Prefer the current `instrument-observability` skill in [microsoft/agent365-skills](https://github.com/microsoft/agent365-skills). Non-AI-Teammate blueprint agents must export on the S2S route with an app-only token resolver; do not wire delegated `AgenticTokenCache` / `exchange_token(...observability...)` flows for those agents. + --- ## pip Packages @@ -62,7 +64,7 @@ No OBO user token is required. > **⚠️ Known Issue (msal v1.34.0):** Python MSAL does NOT properly support `fmi_path` as a parameter to `acquire_token_for_client()`. Passing it causes `TypeError: Session.request() got an unexpected keyword argument 'fmi_path'`. Use **direct HTTP POST** to the token endpoint with `fmi_path` as a form parameter for Hop 1+2 (same workaround as Node.js). MSAL is fine for Hop 3 (no `fmi_path` needed). -> **Note:** As of CLI 1.1, `a365 setup all` automatically grants `Agent365.Observability.OtelWrite` to the Agent Identity SP (both delegated and application). No manual role assignment is needed for newly provisioned agents. +> **Note:** `a365 setup all` no longer requests `Agent365.Observability.OtelWrite` for blueprint agents. Registered blueprint agents that use the app-only S2S endpoint need no Observability admin consent. Agents that still export through the delegated route must opt back in with `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` (commercial: `9b975845-388f-4429-889e-eab1ef63949c`; other clouds are listed under [Environment Variable Overrides](../../../src/Microsoft.Agents.A365.DevTools.Cli/design.md#environment-variable-overrides)). #### Step 1 — Create `observability/token_cache.py` @@ -858,11 +860,11 @@ python -c "from microsoft.opentelemetry import use_microsoft_opentelemetry; from | Token resolver returns `None` | Per-turn OBO token cache was never refreshed | Call `exchange_token()` and `cache_agentic_token()` at the start of each message handler turn | | `ModuleNotFoundError` | Package not installed | Run `pip install microsoft-opentelemetry` and install `msal azure-identity httpx` when needed | | Traces not in Admin Center | Exporter env var not set | Set `ENABLE_A365_OBSERVABILITY_EXPORTER=true` in production | -| 401 on export | Missing permission | Check if upgrading past `0.3.0` (requires new `Agent365.Observability.OtelWrite` permission) | +| 401 on export | Delegated route or delegated-token wiring is still in use | For blueprint agents, set `a365_use_s2s_endpoint=True` and use an app-only token resolver. If you intentionally use the delegated route, grant OtelWrite with `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` (commercial example: `9b975845-388f-4429-889e-eab1ef63949c`; see the per-cloud table in the CLI design) | | Spans dropped silently | Missing tenant/agent ID | Ensure `BaggageBuilder` or `populate()` adds tenant/agent identity before creating spans | | S2S: OBO token-refresh code still runs in the handler | S2S does not use per-turn OBO token exchange | Remove the OBO handler refresh path; token comes from the background token service via `a365_token_resolver` | | S2S 401: wrong Hop 3 scope | FMI Hop 3 used `https://api.powerplatform.com/.default` from older samples | Change Hop 3 scope to `api://9b975845-388f-4429-889e-eab1ef63949c/.default` | -| S2S 401 even with correct scope | `OtelWrite` role not on Agent Identity SP | For agents provisioned before CLI 1.1, manually assign `Agent365.Observability.OtelWrite` to the Agent Identity SP via Entra portal (App registrations > Blueprint app > API permissions) | +| S2S 401 even with correct scope | Exporter is not on the S2S route or received a delegated token | Set `a365_use_s2s_endpoint=True`, use the app-only resolver, and verify the token has no `scp` claim | | S2S: Spans appear to run but nothing is exported | `ENABLE_A365_OBSERVABILITY=true` not set | Python SDK has **two** env vars: `ENABLE_A365_OBSERVABILITY_EXPORTER` (exporter creation) AND `ENABLE_A365_OBSERVABILITY` (span creation). Both must be `true`. Without the second, `InvokeAgentScope.start()` creates a no-op scope. | | S2S: `_is_telemetry_enabled()` returns `False` | `ENABLE_A365_OBSERVABILITY` env var missing | Set `ENABLE_A365_OBSERVABILITY=true` in `.env` — this is separate from `ENABLE_A365_OBSERVABILITY_EXPORTER` | | S2S: MSI fails locally | No Managed Identity in dev | Set `AGENT365_USE_MANAGED_IDENTITY=false` and provide `AGENT365_CLIENT_SECRET` | @@ -870,6 +872,6 @@ python -c "from microsoft.opentelemetry import use_microsoft_opentelemetry; from | S2S: `TypeError: Session.request() got an unexpected keyword argument 'fmi_path'` | MSAL Python v1.34.0 bug | Use direct HTTP POST to `https://login.microsoftonline.com/{tenantId}/oauth2/v2.0/token` with `fmi_path` as form data instead of MSAL `acquire_token_for_client(fmi_path=...)`. MSAL is still used for Hop 3 (no `fmi_path` needed). | | S2S: `InferenceCallDetails.__init__() got an unexpected keyword argument 'operation_name'` | Python SDK uses camelCase kwargs | Use `operationName=`, `providerName=`, `inputTokens=`, `outputTokens=`, `finishReasons=` (camelCase, NOT snake_case) | | S2S: HTTP 400 TenantIdInvalid from exporter | Token not yet acquired when exporter first fires | Ensure `acquire_initial_token()` runs in lifespan BEFORE monitor starts. The `a365_token_resolver` returns `""` when no cached token exists, causing 400. | -| S2S: HTTP 403 `insufficient_scope: Required app role: Agent365.Observability.OtelWrite` | OtelWrite role not assigned to Agent Identity SP | Run PowerShell: `Connect-MgGraph; $sp = Get-MgServicePrincipal -Filter "appId eq ''"` then `New-MgServicePrincipalAppRoleAssignment` with OtelWrite role from observability API SP (`9b975845-388f-4429-889e-eab1ef63949c`) | +| S2S: HTTP 403 `insufficient_scope: Required app role: Agent365.Observability.OtelWrite` | Agent instance is not registered, or AI Teammate OtelWrite app-role grant is incomplete | Blueprint agents: run `a365 setup all --agent-registration-only` and retry. AI Teammates: complete the OtelWrite application-role PowerShell step printed by `a365 setup all --aiteammate` | | S2S: FMI Hop 3 returns `AADSTS700024` | Agent Identity has no FMI credential | Verify `a365 setup all` completed successfully — it creates the federated credential on the Agent Identity | | S2S: HTTP 200 but `rejectedSpans > 0` | Missing baggage context (tenant_id/agent_id) | Ensure `BaggageBuilder().tenant_id(...).agent_id(...).build()` wraps all scope code — without it, spans lack identity and are rejected | 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 6c48f45f..49ace519 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs @@ -143,8 +143,8 @@ public static Command CreateCommand( "--authmode", description: "Authentication pattern for the agent identity (blueprint agents only).\n" + " obo — on-behalf-of (default); principal-scoped delegated grants; no admin consent needed.\n" + - " s2s — service-to-service; app permissions on agent identity; Global Admin needed or PowerShell fallback.\n" + - " both — delegated grants (OBO) and app permissions (S2S).\n" + + " s2s — service-to-service; grants the app roles in requested specs (blueprint agents no longer request OtelWrite).\n" + + " both — delegated grants (OBO) plus those S2S app-role grants.\n" + "Not supported with --aiteammate true."); var skipSpProvisioningOption = new Option( @@ -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; } @@ -441,7 +447,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; @@ -1012,7 +1019,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/BatchPermissionsOrchestrator.cs b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs index ecf2bfff..c4a23c67 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BatchPermissionsOrchestrator.cs @@ -227,7 +227,10 @@ internal static class BatchPermissionsOrchestrator { logger.LogInformation("Skipping S2S app role assignment per operator response. The setup summary lists the manual steps."); if (setupResults is not null) + { setupResults.BlueprintS2SOutcome = Models.GrantOutcome.Failed; + SetPendingBlueprintAppRoleSpecs(setupResults, specs); + } } else { @@ -250,6 +253,7 @@ internal static class BatchPermissionsOrchestrator { logger.LogInformation("Application permissions granted."); setupResults.BlueprintS2SOutcome = Models.GrantOutcome.Granted; + setupResults.PendingBlueprintAppRoleSpecs.Clear(); } else if (attempted) logger.LogWarning("Some app role assignments did not complete - see output above. Manual steps in summary."); @@ -268,7 +272,10 @@ internal static class BatchPermissionsOrchestrator { var hasS2SSpecs = specs.Any(s => s.AppRoleScopes is { Length: > 0 }); if (hasS2SSpecs) + { setupResults.BlueprintS2SOutcome = Models.GrantOutcome.Failed; + SetPendingBlueprintAppRoleSpecs(setupResults, specs); + } } // --- Admin consent --- @@ -515,6 +522,7 @@ private static async Task PerformS2SGrantsAsync( // true so the early-return-protected loop (zero specs cannot reach here) stays true // only when EVERY spec returned AllAlreadyAssigned=true. var allAlreadyAssigned = true; + var failedSpecs = new List(); foreach (var spec in s2sSpecs) { logger.LogDebug( @@ -546,6 +554,7 @@ private static async Task PerformS2SGrantsAsync( // adding actionable detail — keep the actionable Action Required item, drop the // redundant warning to reduce summary noise. allS2SOk = false; + failedSpecs.Add(spec); } // Any spec that newly created at least one assignment, or failed, breaks the "all already assigned" claim. @@ -559,9 +568,22 @@ private static async Task PerformS2SGrantsAsync( // Only meaningful when the grant succeeded: distinguishes "everything was already there" // from "we POSTed at least one new assignment" for the summary's "already granted" wording. setupResults.BlueprintS2SAlreadyAssigned = allS2SOk && allAlreadyAssigned; + // The summary's PowerShell block lists exactly these roles. + setupResults.PendingBlueprintAppRoleSpecs.Clear(); + setupResults.PendingBlueprintAppRoleSpecs.AddRange(failedSpecs); } } + /// + /// Records every app-role spec as pending on the blueprint when the grant was not attempted + /// (non-admin caller or declined prompt), replacing any earlier entries. + /// + private static void SetPendingBlueprintAppRoleSpecs(SetupResults setupResults, IEnumerable specs) + { + setupResults.PendingBlueprintAppRoleSpecs.Clear(); + setupResults.PendingBlueprintAppRoleSpecs.AddRange(specs.Where(s => s.AppRoleScopes is { Length: > 0 })); + } + /// /// Phase 3: Checks for existing consent (skips browser if found), then either opens the /// browser for admins or returns a consolidated consent URL for non-admins. 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 017cbea7..c249e315 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,9 +33,18 @@ 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); + var observabilityPermissionsEffectivelySkipped = + skipObservabilityPermissions && !SetupHelpers.CustomPermissionsRequestObservability(config); + // Dry-run S2S work comes only from fixed specs today; MCP and custom specs carry delegated scopes. + var fixedSpecsHaveAppRoles = SetupHelpers.GetFixedApiPermissionSpecs( + setInheritable: true, + isM365, + config.Environment, + includeObservability: !skipObservabilityPermissions) + .Any(s => s.AppRoleScopes is { Length: > 0 }); // --messaging-endpoint flag (if supplied) wins over the init-only config value for the plan. var plannedEndpoint = !string.IsNullOrWhiteSpace(messagingEndpointOverride) ? messagingEndpointOverride @@ -117,14 +126,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 (observabilityPermissionsEffectivelySkipped) + 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; @@ -133,9 +145,13 @@ public static void PrintDryRunPlan(Agent365Config config, ILogger logger, bool i if (effectiveMode is "obo") logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + "delegated grants — attempted programmatically for the signed-in principal (403 may indicate additional delegated consent or permissions are required)"); else if (effectiveMode is "s2s") - logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + "S2S app roles — attempted programmatically ({Roles} required if 403)", AuthenticationConstants.S2SGrantRequiredRoles); + logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + (!fixedSpecsHaveAppRoles + ? "not required (no S2S app roles to grant)" + : $"S2S app roles — attempted programmatically ({AuthenticationConstants.S2SGrantRequiredRoles} required if 403)")); else if (effectiveMode is "both") - logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + "delegated grants for the signed-in principal + S2S app roles — attempted programmatically; {Roles} required for S2S if 403", AuthenticationConstants.S2SGrantRequiredRoles); + logger.LogInformation(SetupHelpers.DryRunRow(4, "Blueprint Permission Grants") + (!fixedSpecsHaveAppRoles + ? "delegated grants for the signed-in principal; no S2S app roles to grant" + : $"delegated grants for the signed-in principal + S2S app roles — attempted programmatically; {AuthenticationConstants.S2SGrantRequiredRoles} required for S2S if 403")); // 5. Agent identity (created after blueprint-side grants so all blueprint rows are grouped) var agentIdentityDisplayName = config.AgentIdentityDisplayName ?? "Agent"; @@ -266,6 +282,7 @@ await ctx.ClientAppValidator.GrantConsentForPermissionsAsync( public static async Task ExecuteAsync(SetupContext ctx) { ctx.Results.IsNonDwBlueprintFlow = true; + ctx.Results.ObservabilityPermissionsSkipped = ctx.ObservabilityPermissionsEffectivelySkipped; ctx.Results.TenantId = ctx.Config.TenantId; // Bootstrap already printed the "Running..." banner before auth steps; skip here to avoid duplication. if (!ctx.IsBootstrap) @@ -312,7 +329,9 @@ public static async Task ExecuteAsync(SetupContext ctx) { ctx.Logger.LogError("Agent identity ID is not set in config. Run 'a365 setup all' first to create the agent identity, then retry with --agent-registration-only."); ctx.Results.AgentIdentityFailed = true; + ctx.Results.AgentIdentityFailureIsError = true; ctx.Results.AgentRegistrationFailed = true; + ctx.Results.AgentRegistrationFailureIsError = true; ctx.Results.Errors.Add("Agent identity ID not found in config. Run 'a365 setup all' (without --agent-registration-only) to create it first."); } else @@ -362,8 +381,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.ObservabilityPermissionsEffectivelySkipped) + 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 +456,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) @@ -444,6 +465,12 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( // Skipped when --agent-registration-only: identity result flags are pre-set by the caller. if (!skipIdentityAndPermissions) { + // Record the auth mode and whether any S2S app role is requested before identity creation, + // so the summary stays accurate when the identity step fails. + ctx.Results.EffectiveAuthMode = ctx.IsBothMode ? Models.AuthMode.Both : ctx.IsS2sMode ? Models.AuthMode.S2s : Models.AuthMode.Obo; + if (ctx.IsS2sMode || ctx.IsBothMode) + ctx.Results.NoS2SAppRolesToGrant = !specs.Any(s => s.AppRoleScopes is { Length: > 0 }); + ctx.Logger.LogInformation(""); if (!string.IsNullOrWhiteSpace(ctx.Config.AgenticAppId)) @@ -484,8 +511,15 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( ctx.Logger.LogInformation("Creating agent identity..."); if (string.IsNullOrWhiteSpace(ctx.Config.AgentBlueprintClientSecret)) { + var message = + "Agent registration failed: blueprint client secret is not available, so the agent identity could not be created. " + + "Re-run 'a365 setup blueprint' to create the secret, then re-run 'a365 setup all'."; ctx.Results.AgentIdentityFailed = true; - ctx.Logger.LogError("Blueprint client secret is not available. Re-run 'a365 setup blueprint' to create it."); + ctx.Results.AgentIdentityFailureIsError = true; + ctx.Results.AgentRegistrationFailed = true; + ctx.Results.AgentRegistrationFailureIsError = true; + ctx.Results.Errors.Add(message); + ctx.Logger.LogError(message); return; } @@ -514,6 +548,7 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( else if (!ctx.Results.AgentIdentityFailed) { ctx.Results.AgentIdentityFailed = true; + ctx.Results.AgentIdentityFailureIsError = false; ctx.Results.Warnings.Add("Agent identity creation failed. Ensure blueprint setup completed and the client secret is available."); ctx.Logger.LogWarning("Agent identity creation failed. Ensure blueprint setup completed and the client secret is available."); } @@ -523,8 +558,6 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( // Step 5a: Grant permissions to the agent identity, gated by authMode. if (!string.IsNullOrWhiteSpace(ctx.Config.AgenticAppId)) { - ctx.Results.EffectiveAuthMode = ctx.IsBothMode ? Models.AuthMode.Both : ctx.IsS2sMode ? Models.AuthMode.S2s : Models.AuthMode.Obo; - // OBO and Both: delegated permissions for the agent identity are inherited from the // blueprint via the inheritable permissions configured in Phase 1 plus the tenant-wide // admin consent granted via the /v2.0/adminconsent URL in Phase 2. No per-identity @@ -549,15 +582,23 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( ctx.Logger.LogInformation(""); ctx.Logger.LogInformation("Registering agent..."); + // Registration failure fails setup when it is the run's purpose (--agent-registration-only) or the agent's only Observability authorization. + var registrationRequired = skipIdentityAndPermissions || ctx.ObservabilityPermissionsEffectivelySkipped; + void RecordRegistrationFailure(string message) + { + ctx.Results.AgentRegistrationFailed = true; + ctx.Results.AgentRegistrationFailureIsError = registrationRequired; + (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 +606,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 +633,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 +654,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,20 +683,11 @@ private static async Task ExecuteAgentIdentityAndRegistrationAsync( ctx.Logger.LogInformation(""); } } - else + else if (!verificationFailed) { - ctx.Results.AgentRegistrationFailed = true; - const string registrationFailedMessage = "Agent registration failed via Graph copilot/agentRegistrations API."; - if (skipIdentityAndPermissions) - { - ctx.Results.Errors.Add(registrationFailedMessage); - ctx.Logger.LogError(registrationFailedMessage); - } - else - { - ctx.Results.Warnings.Add(registrationFailedMessage); - ctx.Logger.LogWarning(registrationFailedMessage); - } + RecordRegistrationFailure( + "Agent registration failed via Graph copilot/agentRegistrations API. " + + $"Ensure the CLI client app has admin consent for Microsoft Graph {AuthenticationConstants.AgentRegistrationReadWriteAllScope}."); } } // end else (AgenticAppId present) @@ -679,6 +722,9 @@ internal static async Task GrantAgentIdentityS2SPermissionsAsync( List specs) { var hasS2sSpecs = specs.Any(s => s.AppRoleScopes is { Length: > 0 }); + // Blueprint agents no longer request OtelWrite, the only app role setup requested, so this + // step usually has nothing to grant. Record that so the summary does not report a delegated grant. + ctx.Results.NoS2SAppRolesToGrant = !hasS2sSpecs; if (hasS2sSpecs && AgentIdentityInheritsBlueprintAppRoles(ctx.Results)) { ctx.Logger.LogDebug("Agent identity inherits S2S app roles from the blueprint; skipping redundant direct grant."); @@ -718,6 +764,7 @@ internal static async Task GrantOrInstructAgentIdentityAppPermissionsAsync( { ctx.Logger.LogWarning("Agent identity SP object ID is missing. App role assignments must be granted manually."); ctx.Results.AgentIdentityS2SOutcome = Models.GrantOutcome.Failed; + ctx.Results.PendingAgentIdentityAppRoleSpecs.AddRange(s2sSpecs); return; } @@ -766,6 +813,7 @@ internal static async Task GrantOrInstructAgentIdentityAppPermissionsAsync( // Non-admin fallback: print PowerShell instructions for only the failed resources. ctx.Results.AgentIdentityS2SOutcome = Models.GrantOutcome.Failed; + ctx.Results.PendingAgentIdentityAppRoleSpecs.AddRange(failedSpecs); ctx.Logger.LogInformation(""); ctx.Logger.LogInformation("S2S app role assignments require {Roles}. Run the following PowerShell:", AuthenticationConstants.S2SGrantRequiredRoles); ctx.Logger.LogInformation(""); 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..2c4aaebc 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,14 @@ 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). OtelWrite was the only app role setup requested. Unless another permission adds an app role, `--authmode s2s` and `both` have nothing to assign for blueprint agents, and the setup summary reports the S2S grant as not required. + +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. + +To opt a blueprint agent back into delegated-route Observability, add the Observability API to `customBlueprintPermissions` or run `a365 setup permissions custom --resource-app-id --scopes Agent365.Observability.OtelWrite` (commercial: `9b975845-388f-4429-889e-eab1ef63949c`; other clouds are listed under [Environment Variable Overrides](../../design.md#environment-variable-overrides)); the custom path stamps inheritable permissions and requires admin-run consent (custom permissions are not included in the non-admin combined consent URL). + --- ### 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..57632ed7 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,16 @@ 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; } + + /// True only when neither the default specs nor custom permissions request Observability. + public bool ObservabilityPermissionsEffectivelySkipped => + SkipObservabilityPermissions && !SetupHelpers.CustomPermissionsRequestObservability(Config); + /// /// 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 +164,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 +183,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 742a5d45..741c3f6a 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupHelpers.cs @@ -49,7 +49,9 @@ 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, using the app ID for + /// 's cloud, 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. /// @@ -57,7 +59,8 @@ internal static void PrintDryRunBlueprintReuseRows(ILogger logger, string bluepr internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( bool setInheritable, bool isM365, - string? environment = null) + string? environment = null, + bool includeObservability = true) { var specs = new List(); if (isM365) @@ -76,12 +79,15 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( new[] { ConfigConstants.MessagingBotApiAdminConsentScope }, setInheritable)); } - specs.Add(new ResourcePermissionSpec( - ConfigConstants.GetObservabilityApiAppId(environment), - "Observability API", - new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, - setInheritable, - AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); + if (includeObservability) + { + specs.Add(new ResourcePermissionSpec( + ConfigConstants.GetObservabilityApiAppId(environment), + "Observability API", + new[] { ConfigConstants.ObservabilityApiOtelWriteScope }, + setInheritable, + AppRoleScopes: new[] { ConfigConstants.ObservabilityApiOtelWriteScope })); + } specs.Add(new ResourcePermissionSpec( PowerPlatformConstants.PowerPlatformApiResourceAppId, "Power Platform API", @@ -90,13 +96,38 @@ internal static ResourcePermissionSpec[] GetFixedApiPermissionSpecs( return specs.ToArray(); } + /// + /// Mirrors custom-permission inclusion to detect explicit OtelWrite opt-back-in for the configured cloud. + /// Scope names are matched case-insensitively, consistent with custom-permission validation. + /// + internal static bool CustomPermissionsRequestObservability(Agent365Config config) + { + string? expectedAppId = null; + foreach (var customPerm in config.CustomBlueprintPermissions ?? new List()) + { + var (isValid, _) = customPerm.Validate(); + if (!isValid || !ConfigConstants.IsObservabilityApiAppId(customPerm.ResourceAppId)) + continue; + + expectedAppId ??= ConfigConstants.GetObservabilityApiAppId(config.Environment); + if (string.Equals(customPerm.ResourceAppId, expectedAppId, StringComparison.OrdinalIgnoreCase) + && customPerm.Scopes.Contains(ConfigConstants.ObservabilityApiOtelWriteScope, StringComparer.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } + /// /// Builds the full resource permission spec list from config. Used by both the DW (AI Teammate) /// and non-DW (blueprint-only) setup flows. /// /// 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. /// /// @@ -110,7 +141,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) @@ -149,7 +181,7 @@ internal static async Task> BuildConfiguredPermissi : "Agent 365 Tools", kvp.Value, SetInheritable: setInheritable))); - specs.AddRange(GetFixedApiPermissionSpecs(setInheritable, isM365, config.Environment)); + specs.AddRange(GetFixedApiPermissionSpecs(setInheritable, isM365, config.Environment, includeObservability)); foreach (var customPerm in config.CustomBlueprintPermissions ?? new List()) { @@ -626,6 +658,10 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str // (e.g. admin already granted tenant consent but the per-principal call still failed). var pendingDelegatedAction = agentIdDelegatedFailed && !pendingAdminAction; var pendingS2SAction = permissionGrantsPending && isS2SFlow; + // Blueprint agents no longer request OtelWrite, the only app role setup requested, so an + // s2s/both run usually has no S2S grant at all. Say so explicitly: otherwise the row falls + // through to the delegated wording, which for s2s-only shows a PENDING with no action item. + var noS2SAppRolesToGrant = isNonDw && (isS2sOnlyMode || isBothMode) && results.NoS2SAppRolesToGrant && !isS2SFlow; if (results.PermissionGrantsSkipped && isNonDw) { @@ -714,6 +750,8 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str logger.LogInformation(DryRunRow(permGrantStep, "Blueprint Permission Grants") + notRun); else if (results.PermissionGrantsSkipped) logger.LogInformation(DryRunRow(permGrantStep, "Blueprint Permission Grants") + "skipped (--agent-registration-only)"); + else if (noS2SAppRolesToGrant && isS2sOnlyMode) + logger.LogInformation(DryRunRow(permGrantStep, "Blueprint Permission Grants") + "not required (no S2S app roles to grant)"); else if (isS2SFlow && s2sOk) { if (isBothMode && !delegatedOk) @@ -759,6 +797,9 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str : "granted tenant-wide delegated") : tenantConsentUnverified ? "unverified — run 'a365 query-entra inheritance' to confirm" : "PENDING"; + // Both mode with no app roles: this row reports only the delegated half, so name the S2S half too. + if (noS2SAppRolesToGrant) + delegatedLabel += "; no S2S app roles to grant"; logger.LogInformation(DryRunRow(permGrantStep, "Blueprint Permission Grants") + delegatedLabel); } @@ -774,7 +815,14 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str results.AgentIdentityDisplayName ?? "unknown", results.AgentIdentityId ?? "unknown"); } else if (results.AgentIdentityFailed) - logger.LogWarning(DryRunRow(5, "Agent identity") + "failed — see warnings"); + { + var identityMessage = DryRunRow(5, "Agent identity") + + (results.AgentIdentityFailureIsError ? "failed — see errors" : "failed — see warnings"); + if (results.AgentIdentityFailureIsError) + logger.LogError(identityMessage); + else + logger.LogWarning(identityMessage); + } } // Non-DW only: Agent Registration — step 6 @@ -790,6 +838,8 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str logger.LogInformation(DryRunRow(6, "Agent Registration") + registrationVerb + " '{Name}' (ID: {Id})", results.AgentRegistrationDisplayName ?? "unknown", results.AgentInstanceId ?? "unknown"); } + else if (results.AgentRegistrationFailed && results.AgentRegistrationFailureIsError) + logger.LogError(DryRunRow(6, "Agent Registration") + "failed — see errors"); else if (results.AgentRegistrationFailed) logger.LogWarning(DryRunRow(6, "Agent Registration") + "failed — see warnings"); } @@ -900,10 +950,13 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str if (isNonDw && string.IsNullOrWhiteSpace(consentUrl)) { logger.LogInformation(" {N}. Permission Grants — must be granted by {Roles} in the Entra portal:", actionCount, AuthenticationConstants.DelegatedGrantRequiredRoles); + var consentSpecs = BuildNonDwAdminConsentSpecs(observabilityResourceAppId); + if (results.ObservabilityPermissionsSkipped) + consentSpecs = consentSpecs.Where(s => !ConfigConstants.IsObservabilityApiAppId(s.ResourceAppId)).ToList(); LogNonDwAdminConsentInstructions( logger, adminCmdBlueprintId, - specs: BuildNonDwAdminConsentSpecs(observabilityResourceAppId), + specs: consentSpecs, tenantId: results.TenantId); } else @@ -929,42 +982,77 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str } if (pendingS2SAction) { - actionCount++; - logger.LogInformation(""); - logger.LogInformation(" {N}. Observability API S2S app role (PowerShell):", actionCount); - logger.LogInformation(" Required role: {Roles}", AuthenticationConstants.S2SGrantRequiredRoles); - if (!string.IsNullOrWhiteSpace(results.TenantId)) - logger.LogInformation(" Connect-MgGraph -TenantId '{TenantId}' -Scopes 'AppRoleAssignment.ReadWrite.All','Application.Read.All'", results.TenantId); - else - logger.LogInformation(" Connect-MgGraph -Scopes 'AppRoleAssignment.ReadWrite.All','Application.Read.All'"); - // Switch on which side actually failed rather than on DW vs non-DW: non-DW now - // stamps the blueprint too, so blueprintS2sFailed is reachable in the non-DW flow. - if (agentIdS2sFailed) + // One hand-off per failed target. Non-DW can fail on both in one run (blueprint stamping, + // then the agent identity grant), and each hand-off lists only the app roles that target + // did not receive; never assume a particular role. + var s2sTargets = new List<(bool IsAgentIdentity, List Specs)>(); + if (isNonDw && agentIdS2sFailed) + s2sTargets.Add((true, results.PendingAgentIdentityAppRoleSpecs.Where(spec => spec.AppRoleScopes is { Length: > 0 }).Distinct().ToList())); + if (blueprintS2sFailed) + s2sTargets.Add((false, results.PendingBlueprintAppRoleSpecs.Where(spec => spec.AppRoleScopes is { Length: > 0 }).Distinct().ToList())); + + foreach (var (isAgentIdentityTarget, pendingAppRoleSpecs) in s2sTargets) { - // Grant targets the agent identity SP directly (SP object ID, not an app ID). - var agentSpId = results.AgentIdentityId ?? ""; - logger.LogInformation(" $agentSpId = '{AgentSpId}'", agentSpId); - logger.LogInformation(" $obs = Get-MgServicePrincipal -Filter \"appId eq '{ObsApiAppId}'\"", observabilityResourceAppId); - logger.LogInformation(" $rid = ($obs.AppRoles | Where-Object {{ $_.Value -eq '{ObsScope}' }}).Id", ConfigConstants.ObservabilityApiOtelWriteScope); - logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $agentSpId -PrincipalId $agentSpId -ResourceId $obs.Id -AppRoleId $rid"); + actionCount++; + var pendingResourceNames = pendingAppRoleSpecs + .Select(spec => spec.ResourceName) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); + var s2sHeading = pendingResourceNames.Count == 1 ? $"{pendingResourceNames[0]} S2S app role" : "S2S app roles"; + var targetSuffix = s2sTargets.Count > 1 ? (isAgentIdentityTarget ? " on the agent identity" : " on the blueprint") : ""; logger.LogInformation(""); + logger.LogInformation(" {N}. {Heading}{Target} (PowerShell):", actionCount, s2sHeading, targetSuffix); + logger.LogInformation(" Required role: {Roles}", AuthenticationConstants.S2SGrantRequiredRoles); + if (pendingAppRoleSpecs.Count == 0) + { + // Defensive: every writer of a failed S2S outcome records the specs it could not assign. + logger.LogInformation(" Re-run 'a365 setup all' as {Roles} to assign the app roles reported in the setup output above.", AuthenticationConstants.S2SGrantRequiredRoles); + continue; + } + if (!string.IsNullOrWhiteSpace(results.TenantId)) - logger.LogInformation(" Tenant : {TenantId}", results.TenantId); - logger.LogInformation(" Agent Identity: {AgentSpId}", agentSpId); - } - else - { - // DW: grant targets the blueprint SP (looked up by app ID). - logger.LogInformation(" $bp = Get-MgServicePrincipal -Filter \"appId eq '{BlueprintAppId}'\"", blueprintAppId); - logger.LogInformation(" $obs = Get-MgServicePrincipal -Filter \"appId eq '{ObsApiAppId}'\"", observabilityResourceAppId); - logger.LogInformation(" $rid = ($obs.AppRoles | Where-Object {{ $_.Value -eq '{ObsScope}' }}).Id", ConfigConstants.ObservabilityApiOtelWriteScope); - logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $bp.Id -PrincipalId $bp.Id -ResourceId $obs.Id -AppRoleId $rid"); + logger.LogInformation(" Connect-MgGraph -TenantId '{TenantId}' -Scopes 'AppRoleAssignment.ReadWrite.All','Application.Read.All'", results.TenantId); + else + logger.LogInformation(" Connect-MgGraph -Scopes 'AppRoleAssignment.ReadWrite.All','Application.Read.All'"); + + // The agent identity grant targets its SP object ID directly; the blueprint grant + // looks the blueprint SP up by app ID. + var agentSpId = results.AgentIdentityId ?? ""; + if (isAgentIdentityTarget) + logger.LogInformation(" $agentSpId = '{AgentSpId}'", agentSpId); + else + logger.LogInformation(" $bp = Get-MgServicePrincipal -Filter \"appId eq '{BlueprintAppId}'\"", blueprintAppId); + + foreach (var spec in pendingAppRoleSpecs) + { + if (pendingResourceNames.Count > 1) + logger.LogInformation(" # {ResourceName}", spec.ResourceName); + logger.LogInformation(" $res = Get-MgServicePrincipal -Filter \"appId eq '{ResourceAppId}'\"", spec.ResourceAppId); + foreach (var role in spec.AppRoleScopes!) + { + logger.LogInformation(" $rid = ($res.AppRoles | Where-Object {{ $_.Value -eq '{Role}' }}).Id", role); + if (isAgentIdentityTarget) + logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $agentSpId -PrincipalId $agentSpId -ResourceId $res.Id -AppRoleId $rid"); + else + logger.LogInformation(" New-MgServicePrincipalAppRoleAssignment -ServicePrincipalId $bp.Id -PrincipalId $bp.Id -ResourceId $res.Id -AppRoleId $rid"); + } + } logger.LogInformation(""); - logger.LogInformation(" To share with your {Roles}:", AuthenticationConstants.S2SGrantRequiredRoles); - logger.LogInformation(" Blueprint : {BlueprintAppId}", blueprintAppId); - if (!string.IsNullOrWhiteSpace(results.TenantId)) - logger.LogInformation(" Tenant : {TenantId}", results.TenantId); - logger.LogInformation(" Run the PowerShell commands listed above."); + + if (isAgentIdentityTarget) + { + if (!string.IsNullOrWhiteSpace(results.TenantId)) + logger.LogInformation(" Tenant : {TenantId}", results.TenantId); + logger.LogInformation(" Agent Identity: {AgentSpId}", agentSpId); + } + else + { + logger.LogInformation(" To share with your {Roles}:", AuthenticationConstants.S2SGrantRequiredRoles); + logger.LogInformation(" Blueprint : {BlueprintAppId}", blueprintAppId); + if (!string.IsNullOrWhiteSpace(results.TenantId)) + logger.LogInformation(" Tenant : {TenantId}", results.TenantId); + logger.LogInformation(" Run the PowerShell commands listed above."); + } } } if (pendingDelegatedAction) @@ -978,11 +1066,14 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str logger.LogInformation(""); logger.LogInformation(" $agentSpId = '{AgentSpId}'", results.AgentIdentityId ?? ""); logger.LogInformation(""); - logger.LogInformation(" # Observability API"); - logger.LogInformation(" $obsSp = Get-MgServicePrincipal -Filter \"appId eq '{ObsAppId}'\"", observabilityResourceAppId); - logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $obsSp.Id; scope = '{ObsScope}' }} | ConvertTo-Json", ConfigConstants.ObservabilityApiOtelWriteScope); - logger.LogInformation(" Invoke-MgGraphRequest -Method POST -Uri '{GraphBaseUrl}/v1.0/oauth2PermissionGrants' -Body $body -ContentType 'application/json'", resolvedGraphBaseUrl); - logger.LogInformation(""); + if (!results.ObservabilityPermissionsSkipped) + { + logger.LogInformation(" # Observability API"); + logger.LogInformation(" $obsSp = Get-MgServicePrincipal -Filter \"appId eq '{ObsAppId}'\"", observabilityResourceAppId); + logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $obsSp.Id; scope = '{ObsScope}' }} | ConvertTo-Json", ConfigConstants.ObservabilityApiOtelWriteScope); + logger.LogInformation(" Invoke-MgGraphRequest -Method POST -Uri '{GraphBaseUrl}/v1.0/oauth2PermissionGrants' -Body $body -ContentType 'application/json'", resolvedGraphBaseUrl); + logger.LogInformation(""); + } logger.LogInformation(" # Power Platform API"); logger.LogInformation(" $ppSp = Get-MgServicePrincipal -Filter \"appId eq '{PpAppId}'\"", PowerPlatformConstants.PowerPlatformApiResourceAppId); logger.LogInformation(" $body = @{{ clientId = $agentSpId; consentType = 'AllPrincipals'; resourceId = $ppSp.Id; scope = '{PpScope}' }} | ConvertTo-Json", PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead); @@ -1147,10 +1238,10 @@ public static void DisplaySetupSummary(SetupResults results, ILogger logger, str /// 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. @@ -1160,7 +1251,8 @@ internal static List PopulateAdminConsentUrls( IEnumerable mcpScopes, bool isM365 = true, IReadOnlyDictionary? mcpScopesByAudience = null, - IReadOnlyDictionary>? mcpAudienceDisplayNames = null) + IReadOnlyDictionary>? mcpAudienceDisplayNames = null, + bool includeObservability = true) { var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(config.Environment, config.GraphBaseUrl); var graphResourceUri = graphBaseUrl; @@ -1170,7 +1262,11 @@ internal static List PopulateAdminConsentUrls( var urls = BuildAdminConsentUrls( config.TenantId, config.AgentBlueprintId!, config.AgentApplicationScopes, mcpScopes, isM365, mcpScopesByAudience, mcpAudienceDisplayNames, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId); + mcpResourceAppId, observabilityResourceAppId, includeObservability); + + // Clear an Observability consent URL saved by an earlier run so the admin is not asked for permissions this run skipped. + if (!includeObservability) + ClearSkippedObservabilityConsentUrl(config); // 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 @@ -1348,8 +1444,8 @@ internal static string BuildFullyQualifiedScope( /// 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 @@ -1368,7 +1464,8 @@ internal static string BuildFullyQualifiedScope( string graphResourceUri = AuthenticationConstants.MicrosoftGraphResourceUri, string? authorityHost = null, string? sharedMcpResourceAppId = null, - string? observabilityResourceAppId = null) + string? observabilityResourceAppId = null, + bool includeObservability = true) { var urls = new List<(string, string)>(); @@ -1431,9 +1528,12 @@ string Build(string tenant, string client, string resourceUri, IEnumerable /// 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 @@ -1460,7 +1561,8 @@ internal static string BuildCombinedConsentUrl( string graphResourceUri = AuthenticationConstants.MicrosoftGraphResourceUri, string? authorityHost = null, string? sharedMcpResourceAppId = null, - string? observabilityResourceAppId = null) + string? observabilityResourceAppId = null, + bool includeObservability = true) { var allScopes = new List(); foreach (var s in graphScopes) @@ -1493,8 +1595,9 @@ internal static string BuildCombinedConsentUrl( if (isM365) allScopes.Add($"{ConfigConstants.MessagingBotApiIdentifierUri}/{ConfigConstants.MessagingBotApiAdminConsentScope}"); - allScopes.Add( - $"{ConfigConstants.BuildObservabilityApiIdentifierUri(observabilityResourceAppId ?? ConfigConstants.ObservabilityApiAppId)}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); + if (includeObservability) + allScopes.Add( + $"{ConfigConstants.BuildObservabilityApiIdentifierUri(observabilityResourceAppId ?? ConfigConstants.ObservabilityApiAppId)}/{ConfigConstants.ObservabilityApiOtelWriteScope}"); allScopes.Add($"{PowerPlatformConstants.PowerPlatformApiIdentifierUri}/{PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead}"); return BuildAdminConsentUrl(tenantId, blueprintClientId, allScopes, authorityHost); } @@ -1504,10 +1607,12 @@ 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 - /// can complete the hand-off with a single URL. No-op if admin consent was already granted or - /// the blueprint ID is absent. + /// 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. When Observability permissions are skipped, a + /// consent URL saved for them by an earlier run is cleared on every run, admin runs included. + /// Otherwise this is a no-op if admin consent was already granted or the blueprint ID is absent. /// /// internal static void ApplyConsentUrlsIfNeeded( @@ -1519,10 +1624,15 @@ internal static void ApplyConsentUrlsIfNeeded( IReadOnlyDictionary? mcpScopesByAudience = null, IReadOnlyDictionary>? mcpAudienceDisplayNames = null) { + // Before the early return, so an admin run also clears a URL saved by an earlier non-admin run. + if (ctx.ObservabilityPermissionsEffectivelySkipped) + ClearSkippedObservabilityConsentUrl(ctx.Config); + 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); var graphBaseUrl = ConfigConstants.GetGraphBaseUrl(ctx.Config.Environment, ctx.Config.GraphBaseUrl); @@ -1532,7 +1642,18 @@ internal static void ApplyConsentUrlsIfNeeded( ctx.Results.CombinedConsentUrl = BuildCombinedConsentUrl( ctx.Config.TenantId!, ctx.Config.AgentBlueprintId!, graphScopes, mcpScopes, isM365, mcpScopesByAudience, graphResourceUri, authorityHost, - mcpResourceAppId, observabilityResourceAppId); + mcpResourceAppId, observabilityResourceAppId, includeObservability); + } + + /// + /// Clears the saved Observability API consent URL, for any cloud, so an admin is not asked for permissions + /// that setup no longer requests. The entry itself stays, including + /// and the inheritable-permission state: re-running setup does not revoke what an earlier run granted. + /// + internal static void ClearSkippedObservabilityConsentUrl(Agent365Config config) + { + foreach (var consent in config.ResourceConsents.Where(rc => ConfigConstants.IsObservabilityApiAppId(rc.ResourceAppId))) + consent.ConsentUrl = null; } /// 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 a842e664..088761c2 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs +++ b/src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/SetupResults.cs @@ -275,9 +275,15 @@ public class SetupResults /// Whether step 6 (Agent identity creation) was attempted but failed. public bool AgentIdentityFailed { get; set; } + /// Whether an agent identity failure is fatal and should point to Errors. + public bool AgentIdentityFailureIsError { get; set; } + /// Whether step 7 (Agent registration) was attempted but failed. public bool AgentRegistrationFailed { get; set; } + /// Whether an agent registration failure is fatal and should point to Errors. + public bool AgentRegistrationFailureIsError { get; set; } + /// Whether step 8 (Project settings) was written to appsettings.json. public bool ProjectSettingsWritten { get; set; } @@ -287,6 +293,32 @@ 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; } + + /// + /// True when the s2s/both grant step ran but no permission spec carries an app role (blueprint + /// agents no longer request the Observability API OtelWrite role). The setup summary then reports + /// that no S2S app roles were needed instead of falling back to the delegated rows. + /// + public bool NoS2SAppRolesToGrant { get; set; } + + /// + /// App-role specs not assigned on the blueprint service principal because the grant failed or + /// was not attempted (non-admin caller or declined prompt). The setup summary's S2S PowerShell + /// block lists exactly these roles. + /// + internal List PendingBlueprintAppRoleSpecs { get; } = new(); + + /// + /// App-role specs whose assignment on the agent identity service principal failed. The setup + /// summary's S2S PowerShell block lists exactly these roles. + /// + internal List PendingAgentIdentityAppRoleSpecs { get; } = new(); + public List Errors { get; } = new(); public List Warnings { get; } = new(); diff --git a/src/Microsoft.Agents.A365.DevTools.Cli/design.md b/src/Microsoft.Agents.A365.DevTools.Cli/design.md index 0011dcce..28141334 100644 --- a/src/Microsoft.Agents.A365.DevTools.Cli/design.md +++ b/src/Microsoft.Agents.A365.DevTools.Cli/design.md @@ -496,7 +496,7 @@ The non-DW spec list is a strict subset of the DW list: | Microsoft Graph (delegated) | ✓ | — | | Agent 365 Tools (delegated) | ✓ | — | | Messaging Bot API | ✓ | — | -| Observability API | ✓ | ✓ | +| Observability API | ✓ | — | | Power Platform API | ✓ | ✓ | --- 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..432272d6 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,243 @@ 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, List? customPermissions = null) + { + 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, + CustomBlueprintPermissions = customPermissions, + }, + 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"); + } + + [Fact] + public void ApplyConsentUrlsIfNeeded_AdminRun_ClearsObservabilityConsentUrlSavedByAnEarlierRun() + { + var ctx = BuildPermissionsContext(skipObservabilityPermissions: true); + ctx.Results.TenantWideConsentOutcome = GrantOutcome.Granted; + ctx.Config.ResourceConsents.Add(new ResourceConsent + { + ResourceName = "Observability API", + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + ConsentUrl = "https://login.microsoftonline.com/earlier-non-admin-run", + InheritablePermissionsConfigured = true, + }); + + SetupHelpers.ApplyConsentUrlsIfNeeded( + ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, new[] { "McpServers.Mail.All" }, isM365: false); + + var observability = ctx.Config.ResourceConsents.Should().ContainSingle( + rc => rc.ResourceAppId == ConfigConstants.ObservabilityApiAppId, + because: "re-running setup does not revoke, so the earlier record is kept").Which; + observability.ConsentUrl.Should().BeNull( + because: "an admin run must also stop handing out a consent URL that an earlier non-admin run saved for permissions setup no longer requests"); + observability.InheritablePermissionsConfigured.Should().Be(true, + because: "only the URL is cleared; the rest of the earlier record stays"); + ctx.Results.CombinedConsentUrl.Should().BeNull( + because: "consent was granted in this run, so there is nothing to hand off"); + } + + [Fact] + public void ApplyConsentUrlsIfNeeded_AdminRun_CustomObservabilityPermission_KeepsSavedObservabilityConsentUrl() + { + var ctx = BuildPermissionsContext( + skipObservabilityPermissions: true, + customPermissions: + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + ResourceName = "Observability API", + Scopes = [ConfigConstants.ObservabilityApiOtelWriteScope], + } + ]); + ctx.Results.TenantWideConsentOutcome = GrantOutcome.Granted; + ctx.Config.ResourceConsents.Add(new ResourceConsent + { + ResourceName = "Observability API", + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + ConsentUrl = "https://login.microsoftonline.com/custom-observability", + InheritablePermissionsConfigured = true, + }); + + SetupHelpers.ApplyConsentUrlsIfNeeded( + ctx, McpConstants.WorkIQToolsProdAppId, ctx.Config.AgentApplicationScopes, new[] { "McpServers.Mail.All" }, isM365: false); + + ctx.Config.ResourceConsents.Should().ContainSingle(rc => + rc.ResourceAppId == ConfigConstants.ObservabilityApiAppId && + rc.ConsentUrl == "https://login.microsoftonline.com/custom-observability", + because: "custom Observability permissions opt back into the resource, so setup must not clear the saved hand-off URL"); + } + + public static TheoryData CustomObservabilityCases() => new() + { + { + new Agent365Config + { + Environment = "gcc", + CustomBlueprintPermissions = + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + Scopes = [ConfigConstants.ObservabilityApiOtelWriteScope], + } + ], + }, + false, + "a commercial Observability app ID is not an opt-back-in for a GCC blueprint" + }, + { + new Agent365Config + { + CustomBlueprintPermissions = + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + Scopes = ["Agent365.Observability.Read"], + } + ], + }, + false, + "a custom Observability entry without OtelWrite does not authorize telemetry export" + }, + { + new Agent365Config + { + CustomBlueprintPermissions = + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + Scopes = [ConfigConstants.ObservabilityApiOtelWriteScope.ToUpperInvariant()], + } + ], + }, + true, + "scope names are compared case-insensitively throughout permission handling" + }, + { + new Agent365Config + { + Environment = "gcc", + CustomBlueprintPermissions = + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.GccObservabilityApiAppId, + Scopes = [ConfigConstants.ObservabilityApiOtelWriteScope], + } + ], + }, + true, + "the configured GCC Observability app ID with OtelWrite is an explicit opt-back-in" + }, + }; + + [Theory] + [MemberData(nameof(CustomObservabilityCases))] + public void CustomPermissionsRequestObservability_RequiresConfiguredCloudAndOtelWrite( + Agent365Config config, + bool expected, + string because) + { + SetupHelpers.CustomPermissionsRequestObservability(config).Should().Be(expected, because); + } + + [Fact] + public void CustomPermissionsRequestObservability_NonObservabilityCustomPermission_DoesNotResolveAmbiguousGovernmentCloud() + { + var config = new Agent365Config + { + Environment = "AzureUSGovernment", + CustomBlueprintPermissions = + [ + new CustomResourcePermission + { + ResourceAppId = AuthenticationConstants.MicrosoftGraphResourceAppId, + Scopes = ["User.Read"], + } + ], + }; + + var act = () => SetupHelpers.CustomPermissionsRequestObservability(config); + + act.Should().NotThrow( + because: "GetObservabilityApiAppId throws for ambiguous AzureUSGovernment, so it must be resolved only after an Observability custom entry is detected"); + act().Should().BeFalse( + because: "non-Observability custom permissions must not opt back into Observability handling"); + } } diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs index 7cfdeb20..fa985611 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BatchPermissionsOrchestratorTests.cs @@ -349,6 +349,8 @@ await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( // Assert setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Granted, because: "when the az rest POST /appRoleAssignments succeeds the Action Required block must be suppressed"); + setupResults.PendingBlueprintAppRoleSpecs.Should().BeEmpty( + because: "a completed fallback leaves no app role for the summary's hand-off"); } /// @@ -378,6 +380,8 @@ await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( // Assert setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Failed, because: "a non-zero az rest exit code means the assignment was not created — Action Required must remain visible"); + setupResults.PendingBlueprintAppRoleSpecs.Should().ContainSingle(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId, + because: "the summary's hand-off must list the app role the fallback could not assign"); } /// @@ -410,6 +414,8 @@ await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( // Assert — outcome is Failed (Action Required surfaces manual steps). setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Failed, because: "operator declined the confirmation, so no S2S grants were attempted; Action Required must surface the manual steps"); + setupResults.PendingBlueprintAppRoleSpecs.Should().ContainSingle(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId, + because: "a declined grant leaves every requested app role for the summary's hand-off"); // Primary path: no Graph API S2S call should have been made. await _blueprintService.DidNotReceive().GrantAppRoleAssignmentAsync( @@ -533,6 +539,8 @@ await BatchPermissionsOrchestrator.ConfigureAllPermissionsAsync( // Assert setupResults.BlueprintS2SOutcome.Should().Be(GrantOutcome.Failed, because: "a non-admin user cannot complete S2S app role assignment directly — the outcome must be marked Failed so DisplaySetupSummary surfaces the hand-off block"); + setupResults.PendingBlueprintAppRoleSpecs.Should().ContainSingle(s => s.ResourceAppId == ConfigConstants.ObservabilityApiAppId, + because: "a non-admin run leaves every requested app role for the summary's hand-off"); } // ────────────────────────────────────────────────────────────────────────────────────── diff --git a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs index a2b0dc82..0f9765ad 100644 --- a/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs +++ b/src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NonDwBlueprintSetupOrchestratorDryRunTests.cs @@ -3,6 +3,7 @@ using FluentAssertions; using Microsoft.Agents.A365.DevTools.Cli.Commands.SetupSubcommands; +using Microsoft.Agents.A365.DevTools.Cli.Constants; using Microsoft.Agents.A365.DevTools.Cli.Models; using Microsoft.Extensions.Logging; using NSubstitute; @@ -22,7 +23,8 @@ public class NonDwBlueprintSetupOrchestratorDryRunTests private static Agent365Config BuildConfig( string displayName = "My Agent", string tenantId = "tenant-id", - string? blueprintId = null) => + string? blueprintId = null, + List? customPermissions = null) => new() { AgentIdentityDisplayName = displayName, @@ -31,7 +33,8 @@ private static Agent365Config BuildConfig( UseBlueprint = true, ClientAppId = "client-app-id", DeploymentProjectPath = "./app", - AgentBlueprintId = blueprintId + AgentBlueprintId = blueprintId, + CustomBlueprintPermissions = customPermissions, }; private bool AnyLogContains(string value) => @@ -168,6 +171,41 @@ public void PrintDryRunPlan_AuthModeS2s_ShowsAppPermsOnAgentIdentity_NoDelegated AnyLogContains("delegated").Should().BeFalse(because: "S2S mode must not show delegated grants on the agent identity SP — no user context"); } + /// + /// Blueprint agents skip OtelWrite by default, which leaves the S2S half with no app roles to assign. + /// + [Fact] + public void PrintDryRunPlan_AuthModeS2s_WhenObservabilitySkipped_ShowsNoAppRolesToGrant() + { + NonDwBlueprintSetupOrchestrator.PrintDryRunPlan(BuildConfig(), _logger, authMode: "s2s", skipObservabilityPermissions: true); + + AnyLogContains("not required (no S2S app roles to grant)").Should().BeTrue( + because: "the dry-run plan must match the real summary when the spec list has no app roles"); + AnyLogContains("Global Administrator required if 403").Should().BeFalse( + because: "there is no S2S grant to perform when blueprint agents skip OtelWrite"); + } + + [Fact] + public void PrintDryRunPlan_CustomObservabilityPermission_DoesNotClaimObservabilityNotRequested() + { + var config = BuildConfig(customPermissions: + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + ResourceName = "Observability API", + Scopes = [ConfigConstants.ObservabilityApiOtelWriteScope], + } + ]); + + NonDwBlueprintSetupOrchestrator.PrintDryRunPlan(config, _logger, authMode: "s2s", skipObservabilityPermissions: true); + + AnyLogContains("Observability API not requested").Should().BeFalse( + because: "a custom Observability permission explicitly opts back into requesting Observability permissions"); + AnyLogContains("not required (no S2S app roles to grant)").Should().BeTrue( + because: "custom permissions carry delegated scopes only, so there is still no S2S app role to grant"); + } + /// /// Both mode must surface both delegated grants and application permissions on the /// agent identity SP. @@ -181,6 +219,20 @@ public void PrintDryRunPlan_AuthModeBoth_ShowsBothGrantRowsOnAgentIdentity() AnyLogContains("S2S app roles").Should().BeTrue(because: "Both mode includes S2S app role assignments on the agent identity SP"); } + /// + /// Both mode still has delegated consent work, but no S2S grant when no spec carries app roles. + /// + [Fact] + public void PrintDryRunPlan_AuthModeBoth_WhenObservabilitySkipped_ShowsDelegatedOnly() + { + NonDwBlueprintSetupOrchestrator.PrintDryRunPlan(BuildConfig(), _logger, authMode: "both", skipObservabilityPermissions: true); + + AnyLogContains("delegated grants for the signed-in principal; no S2S app roles to grant").Should().BeTrue( + because: "both mode must preserve the delegated half while saying the S2S half has no role to assign"); + AnyLogContains("Global Administrator required for S2S if 403").Should().BeFalse( + because: "there is no S2S grant fallback to describe when blueprint agents skip OtelWrite"); + } + /// /// Null authMode must default to OBO behaviour — the agent-identity step shows /// delegated grants. 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 31c5d0ba..657e7938 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 @@ -60,7 +60,7 @@ private static CommandExecutor BuildMockExecutor() return executor; } - private static SetupContext BuildContext(Agent365Config? config = null, bool skipRequirements = true) + private static SetupContext BuildContext(Agent365Config? config = null, bool skipRequirements = true, bool skipObservabilityPermissions = false) { var cfg = config ?? new Agent365Config { @@ -136,7 +136,8 @@ private static SetupContext BuildContext(Agent365Config? config = null, bool ski blueprintLookupService: blueprintLookupService, federatedCredentialService: federatedCredentialService, clientAppValidator: Substitute.For(), - loginHintResolver: () => Task.FromResult(null)); + loginHintResolver: () => Task.FromResult(null), + skipObservabilityPermissions: skipObservabilityPermissions); } /// @@ -153,6 +154,34 @@ public async Task ExecuteAsync_ReturnsExitCode1_WhenBlueprintFails() exitCode.Should().Be(1); } + [Fact] + public async Task ExecuteAsync_CustomObservabilityPermission_DoesNotMarkObservabilitySkipped() + { + var ctx = BuildContext( + new Agent365Config + { + AiTeammate = false, + TenantId = "tenant-id", + AgentIdentityDisplayName = "Test Agent", + ClientAppId = "client-app-id", + CustomBlueprintPermissions = + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + ResourceName = "Observability API", + Scopes = [ConfigConstants.ObservabilityApiOtelWriteScope], + } + ], + }, + skipObservabilityPermissions: true); + + await NonDwBlueprintSetupOrchestrator.ExecuteAsync(ctx); + + ctx.Results.ObservabilityPermissionsSkipped.Should().BeFalse( + because: "a valid custom Observability permission is an explicit opt-back-in even though setup omits the default fixed OtelWrite spec"); + } + /// /// When blueprint creation fails, errors must be added to SetupResults /// so the summary display can show what went wrong. @@ -256,12 +285,17 @@ 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, ILogger? logger = null) + BuildIdempotencyTestContext( + Agent365Config? config = null, + ILogger? logger = null, + bool agentInstanceOnly = true, + bool skipObservabilityPermissions = false, + string? authMode = null) { var graph = Substitute.ForPartsOf(); @@ -312,8 +346,10 @@ 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, + authMode: authMode); return (ctx, graph, blueprintService); } @@ -378,8 +414,12 @@ public async Task Step5_FailsWithError_WhenIdentityNotFoundByApiLookup() because: "missing agent identity is a fatal error for --agent-registration-only"); ctx.Results.AgentIdentityFailed.Should().BeTrue( because: "identity not found via API lookup must surface as an identity failure"); + ctx.Results.AgentIdentityFailureIsError.Should().BeTrue( + because: "registration-only cannot continue without an identity, so the identity row must point to Errors"); ctx.Results.AgentRegistrationFailed.Should().BeTrue( because: "registration cannot proceed without an agent identity"); + ctx.Results.AgentRegistrationFailureIsError.Should().BeTrue( + because: "registration-only cannot complete without an identity, so the registration row must point to Errors"); await graph.DidNotReceive().CreateAgentIdentityDelegatedAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } @@ -412,8 +452,10 @@ public async Task Step6_RegistrationOnly_ReturnsExitCode1AndAvoidsSuccessfulSumm exitCode.Should().Be(1, because: "registration-only mode requested only agent registration, so that failure must be fatal"); - ctx.Results.Errors.Should().ContainSingle(error => error == "Agent registration failed via Graph copilot/agentRegistrations API."); - ctx.Results.Warnings.Should().NotContain("Agent registration failed via Graph copilot/agentRegistrations API."); + ctx.Results.Errors.Should().ContainSingle(error => error.StartsWith("Agent registration failed via Graph copilot/agentRegistrations API."), + because: "registration-only failures are fatal and must be listed under Errors for the summary and exit-code path"); + ctx.Results.Warnings.Should().NotContain(warning => warning.StartsWith("Agent registration failed"), + because: "registration-only failures must not be downgraded to warnings"); logger.AllOutput.Should().Contain("Setup completed with errors", because: "the summary must not present a registration-only failure as successful"); logger.AllOutput.Should().NotContain("Setup completed successfully", @@ -671,37 +713,335 @@ 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: agentInstanceOnly, skipObservabilityPermissions: 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 5: when the blueprint secret is missing, setup cannot create the identity or register the agent. + /// + [Fact] + public async Task Step5_MissingBlueprintClientSecret_RecordsErrorForExitCode1() + { + var projectDir = Path.Combine(Path.GetTempPath(), "NonDwMissingSecretTests_" + 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", + DeploymentProjectPath = projectDir, + AgentBlueprintClientSecret = null, + }; + var (ctx, _, blueprintService) = BuildIdempotencyTestContext( + config, agentInstanceOnly: false, skipObservabilityPermissions: true); + blueprintService.FindExistingAgentIdentityAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns((string?)null); + + await NonDwBlueprintSetupOrchestrator.ExecuteAgentIdentityAndRegistrationAsync(ctx, specs: []); + + ctx.Results.AgentIdentityFailed.Should().BeTrue( + because: "without the blueprint secret setup cannot create the agent identity"); + ctx.Results.AgentIdentityFailureIsError.Should().BeTrue( + because: "the missing secret path records an error, so the identity summary row must say see errors"); + ctx.Results.AgentRegistrationFailed.Should().BeTrue( + because: "registration is mandatory when Observability permissions were skipped"); + ctx.Results.AgentRegistrationFailureIsError.Should().BeTrue( + because: "the missing secret path records an error before registration can run"); + ctx.Results.Errors.Should().ContainSingle(e => e.Contains("blueprint client secret is not available"), + because: "ExecuteAsync returns exit code 1 whenever Results.HasErrors is true"); + ctx.Results.Errors.Single().Should().Contain("re-run 'a365 setup all'") + .And.NotContain("--agent-registration-only", + because: "--agent-registration-only skips identity creation, so it cannot recover a run that never created the identity"); + ctx.Results.HasErrors.Should().BeTrue( + because: "the full setup command maps recorded errors to exit code 1"); + } + finally + { + Directory.Delete(projectDir, recursive: true); + } + } + + /// + /// Step 5: an s2s run whose identity step fails still records its auth mode and that no S2S app role + /// is requested, so the summary does not fall back to delegated-consent wording. + /// + [Fact] + public async Task Step5_IdentityStepFails_S2sMode_SummaryStillReportsNoS2SAppRolesToGrant() + { + var projectDir = Path.Combine(Path.GetTempPath(), "NonDwS2sIdentityFailureTests_" + 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", + DeploymentProjectPath = projectDir, + AgentBlueprintClientSecret = null, + }; + var (ctx, _, blueprintService) = BuildIdempotencyTestContext( + config, agentInstanceOnly: false, skipObservabilityPermissions: true, authMode: "s2s"); + blueprintService.FindExistingAgentIdentityAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns((string?)null); + // A non-admin run: the blueprint grants completed but tenant-wide consent was not granted. + ctx.Results.IsNonDwBlueprintFlow = true; + ctx.Results.BatchPermissionsPhase1Completed = true; + ctx.Results.BatchPermissionsPhase2Completed = true; + ctx.Results.TenantWideConsentOutcome = GrantOutcome.Failed; + + await NonDwBlueprintSetupOrchestrator.ExecuteAgentIdentityAndRegistrationAsync(ctx, specs: []); + + ctx.Results.AgentIdentityFailed.Should().BeTrue(because: "precondition: the missing secret stops identity creation"); + ctx.Results.EffectiveAuthMode.Should().Be(AuthMode.S2s, + because: "the auth mode is known before identity creation, and the summary derives delegated-consent applicability from it"); + ctx.Results.NoS2SAppRolesToGrant.Should().BeTrue( + because: "no requested spec carries an app role, whether or not the identity step succeeds"); + + var logger = new CapturingLogger(); + SetupHelpers.DisplaySetupSummary(ctx.Results, logger); + logger.AllOutput.Split('\n').Should().ContainSingle(l => l.Contains("Blueprint Permission Grants")) + .Which.Should().Contain("not required (no S2S app roles to grant)", + because: "an s2s run with no app roles to grant needs no blueprint grant, even when the identity step failed"); + } + finally + { + Directory.Delete(projectDir, recursive: true); + } + } + + [Fact] + public async Task Step5_IdentityCreationNull_RecordsIdentityWarningSeverity() + { + var projectDir = Path.Combine(Path.GetTempPath(), "NonDwIdentityNullTests_" + 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", + DeploymentProjectPath = projectDir, + AgentBlueprintClientSecret = "secret", + }; + var (ctx, graph, blueprintService) = BuildIdempotencyTestContext( + config, agentInstanceOnly: false, skipObservabilityPermissions: false); + blueprintService.FindExistingAgentIdentityAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns((string?)null); + graph.CreateAgentIdentityAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns((string?)null); + + await NonDwBlueprintSetupOrchestrator.ExecuteAgentIdentityAndRegistrationAsync(ctx, specs: []); + + ctx.Results.AgentIdentityFailed.Should().BeTrue( + because: "a null identity creation result is still surfaced to the summary"); + ctx.Results.AgentIdentityFailureIsError.Should().BeFalse( + because: "identity creation returning null is recorded as a warning path, not an Errors path"); + } + finally + { + Directory.Delete(projectDir, recursive: true); + } + } + + /// + /// 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)"); + if (skipObservabilityPermissions) + ctx.Results.Errors.Should().Contain(e => e.Contains(AuthenticationConstants.AgentRegistrationReadWriteAllScope), + because: "the failure guidance should name the Graph permission the registration API requires"); + 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); + } + } + + [Fact] + public async Task Step6_RegistrationFailureWithCustomObservabilityPermission_RemainsWarning() + { + 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", + DeploymentProjectPath = projectDir, + CustomBlueprintPermissions = + [ + new CustomResourcePermission + { + ResourceAppId = ConfigConstants.ObservabilityApiAppId, + ResourceName = "Observability API", + Scopes = [ConfigConstants.ObservabilityApiOtelWriteScope], + } + ], + }; + var (ctx, graph, _) = BuildIdempotencyTestContext( + config, agentInstanceOnly: false, skipObservabilityPermissions: true); + StubRegistrationFailure(graph); + + await NonDwBlueprintSetupOrchestrator.ExecuteAgentIdentityAndRegistrationAsync(ctx, specs: []); + + ctx.Results.Errors.Should().NotContain(e => e.Contains("Agent registration failed"), + because: "custom Observability permissions mean registration is not the agent's only Observability authorization"); + ctx.Results.Warnings.Should().Contain(w => w.Contains("Agent registration failed"), + because: "registration failure remains non-fatal when Observability was explicitly requested through custom permissions"); + } + finally + { + Directory.Delete(projectDir, recursive: true); + } } // ------------------------------------------------------------------------- @@ -806,6 +1146,8 @@ public async Task GrantOrInstructAgentIdentityAppPermissions_AgentSpIdMissing_Se ctx.Results.AgentIdentityS2SOutcome.Should().Be(Cli.Models.GrantOutcome.Failed, because: "when AgenticAppId (the SP object ID) is absent, grants cannot proceed and the outcome must be Failed"); + ctx.Results.PendingAgentIdentityAppRoleSpecs.Should().ContainSingle(s => s.ResourceName == "Test Resource", + because: "the summary's hand-off must list the app role that could not be assigned"); await blueprintService.DidNotReceive().GrantAppRoleAssignmentAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any>(), Arg.Any?>(), Arg.Any()); @@ -880,6 +1222,8 @@ await ctx.Executor.Received().ExecuteAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); ctx.Results.AgentIdentityS2SOutcome.Should().Be(Cli.Models.GrantOutcome.Failed, because: "when both the Graph grant and the az rest fallback fail, AgentIdentityS2SOutcome must be Failed"); + ctx.Results.PendingAgentIdentityAppRoleSpecs.Should().ContainSingle(s => s.ResourceAppId == resourceAppId, + because: "the summary's hand-off must list exactly the app role that failed"); ctx.Results.HasWarnings.Should().BeTrue( because: "a failed S2S grant must add a warning so the setup summary shows Action Required"); ctx.Results.Warnings.Should().ContainSingle() @@ -943,6 +1287,8 @@ await ctx.Executor.Received(1).ExecuteAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); ctx.Results.AgentIdentityS2SOutcome.Should().Be(Cli.Models.GrantOutcome.Granted, because: "when the az rest fallback assigns the app role, the agent identity S2S grant succeeded"); + ctx.Results.PendingAgentIdentityAppRoleSpecs.Should().BeEmpty( + because: "a completed fallback leaves no app role for the summary's hand-off"); ctx.Results.HasWarnings.Should().BeFalse( because: "a successful az rest fallback must not surface a PowerShell hand-off warning"); } @@ -990,6 +1336,8 @@ await blueprintService.Received(1).GrantAppRoleAssignmentAsync( Arg.Any>(), Arg.Any?>(), Arg.Any()); ctx.Results.AgentIdentityS2SOutcome.Should().Be(Cli.Models.GrantOutcome.Granted, because: "without inheritance the direct grant runs and, when it succeeds, the outcome is Granted"); + ctx.Results.NoS2SAppRolesToGrant.Should().BeFalse( + because: "an app role was requested, so the summary must report the S2S grant"); } /// @@ -1013,6 +1361,8 @@ await blueprintService.DidNotReceive().GrantAppRoleAssignmentAsync( Arg.Any>(), Arg.Any?>(), Arg.Any()); ctx.Results.AgentIdentityS2SOutcome.Should().Be(Cli.Models.GrantOutcome.NotApplicable, because: "with no S2S specs there is nothing to grant or inherit, so the outcome must remain NotApplicable"); + ctx.Results.NoS2SAppRolesToGrant.Should().BeTrue( + because: "the summary needs to know the s2s/both grant step had no app role to grant"); } // ------------------------------------------------------------------------- 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..2af4e503 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 @@ -588,6 +588,22 @@ public async Task SetupAll_AuthMode_InvalidValue_ExitsWithCode1() Arg.Any>()); } + /// + /// Help text must not promise default S2S app-role grants for blueprint agents. + /// + [Fact] + public void SetupAll_AuthMode_HelpText_StatesBlueprintAgentsDoNotRequestOtelWrite() + { + var setup = BuildSetupCommand(); + var all = setup.Children.OfType().Single(c => c.Name == "all"); + var authMode = all.Options.Single(o => o.Name == "authmode"); + + authMode.Description.Should().Contain("blueprint agents no longer request OtelWrite", + because: "the CLI no longer requests the Observability app role, but future default specs may carry other app roles"); + authMode.Description.Should().NotContain("blueprint agents grant none by default", + because: "that wording would become stale if another default spec adds app roles"); + } + /// /// --authmode obo with --aiteammate is redundant (obo is the AI Teammate default) but not /// conflicting. It must emit a warning and continue — not exit with an error before setup runs. @@ -684,4 +700,128 @@ 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>()); + _mockLogger.Received().Log( + LogLevel.Information, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains("Blueprint Permission Grants") && o.ToString()!.Contains("no S2S app roles to grant")), + 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 ca1fae04..63b64cd5 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 b4711797..cfa16533 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 @@ -331,6 +331,45 @@ public void PopulateAdminConsentUrls_NonM365_ResourceConsentsExcludeMessagingBot because: "no Messaging Bot consent URL is generated for non-M365 agents, so no resourceConsents entry should be persisted"); } + [Theory] + [InlineData("prod", ConfigConstants.ObservabilityApiAppId)] + [InlineData("gcc", ConfigConstants.GccObservabilityApiAppId)] + public void PopulateAdminConsentUrls_WithoutObservability_ClearsObservabilityConsentUrlFromEarlierRun(string environment, string observabilityAppId) + { + var config = new Agent365Config + { + TenantId = TenantId, + AgentBlueprintId = BlueprintClientId, + Environment = environment, + }; + config.ResourceConsents.Add(new ResourceConsent + { + ResourceName = "Observability API", + ResourceAppId = observabilityAppId, + ConsentUrl = "https://login.microsoftonline.com/old-observability-consent", + ConsentGranted = true, + InheritablePermissionsConfigured = true, + }); + + var names = SetupHelpers.PopulateAdminConsentUrls( + config, McpConstants.WorkIQToolsProdAppId, new[] { "McpServers.Mail.All" }, + isM365: false, includeObservability: false); + + var observability = config.ResourceConsents.Should().ContainSingle( + rc => rc.ResourceAppId == observabilityAppId, + because: "re-running setup does not revoke, so the record of the earlier grant is kept").Which; + observability.ConsentUrl.Should().BeNull( + because: "an Observability consent URL saved by an earlier run must not keep asking the admin for permissions this run no longer requests, in any cloud"); + observability.ConsentGranted.Should().BeTrue( + because: "clearing the URL must not erase that an earlier run granted consent"); + observability.InheritablePermissionsConfigured.Should().Be(true, + because: "clearing the URL must not erase the earlier inheritable-permission state"); + names.Should().NotContain("Observability API"); + config.ResourceConsents.Should().Contain( + rc => rc.ResourceAppId == PowerPlatformConstants.PowerPlatformApiResourceAppId, + because: "clearing the stale Observability URL 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 1a0843a6..b4f3f316 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 @@ -20,6 +20,18 @@ public class SetupHelpersDisplaySetupSummaryTests private const string AgentSpId = "agent-sp-id-123"; private const string TenantId = "tenant-id-456"; private const string BlueprintId = "blueprint-app-id-789"; + private const string CustomAppRoleResourceAppId = "contoso-api-app-id"; + private const string NoAppRolesConsentUrl = "https://login.microsoftonline.com/tenant/v2.0/adminconsent?client_id=bp-no-app-roles"; + + private static readonly ResourcePermissionSpec CustomAppRoleSpec = + new(CustomAppRoleResourceAppId, "Contoso API", [], false, AppRoleScopes: ["Contoso.Write"]); + + private static readonly ResourcePermissionSpec BlueprintOnlyAppRoleSpec = + new("fabrikam-api-app-id", "Fabrikam API", [], false, AppRoleScopes: ["Fabrikam.Read"]); + + private static readonly ResourcePermissionSpec ObservabilityAppRoleSpec = + new(ConfigConstants.ObservabilityApiAppId, "Observability API", [ConfigConstants.ObservabilityApiOtelWriteScope], false, + AppRoleScopes: [ConfigConstants.ObservabilityApiOtelWriteScope]); private sealed class CapturingLogger : ILogger { @@ -99,6 +111,21 @@ public void DisplaySetupSummary_PendingDelegatedAction_EmitsRequiredRoles() because: "the required role must be surfaced so the admin knows which Entra role is needed"); } + [Fact] + public void DisplaySetupSummary_PendingDelegatedAction_WhenObservabilitySkipped_OmitsObservabilityGrant() + { + var logger = new CapturingLogger(); + var results = BuildDelegatedPendingResults(); + results.ObservabilityPermissionsSkipped = true; + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().NotContain(ConfigConstants.ObservabilityApiOtelWriteScope, + because: "blueprint agents that skipped Observability permissions must not be told to grant OtelWrite later"); + logger.AllOutput.Should().Contain(PowerPlatformConstants.PermissionNames.ConnectivityConnectionsRead, + because: "gating the Observability scope must keep the remaining delegated remediation intact"); + } + // ── pendingS2SAction (non-DW path) ───────────────────────────────────────── [Fact] @@ -165,6 +192,162 @@ public void DisplaySetupSummary_PendingS2SAction_NonDw_EmbedsTenantId() because: "the Connect-MgGraph call must include -TenantId so the admin targets the correct tenant"); } + // ── S2S app roles: hand-off lists what failed; none to grant for blueprint agents ─ + + [Fact] + public void DisplaySetupSummary_PendingS2SAction_ListsOnlyTheAppRolesThatFailed() + { + var logger = new CapturingLogger(); + var results = BuildS2SPendingResults(); + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain("Contoso API S2S app role (PowerShell)", + because: "the hand-off heading must name the resource whose app role was not assigned"); + logger.AllOutput.Should().Contain($"appId eq '{CustomAppRoleResourceAppId}'", + because: "the PowerShell must look up the resource whose grant actually failed"); + logger.AllOutput.Should().Contain("$_.Value -eq 'Contoso.Write'", + because: "the PowerShell must assign the role whose grant actually failed"); + logger.AllOutput.Should().NotContain(ConfigConstants.ObservabilityApiOtelWriteScope, + because: "setup no longer requests OtelWrite for blueprint agents, so the hand-off must not grant it"); + } + + [Fact] + public void DisplaySetupSummary_PendingBlueprintS2S_AiTeammate_ListsTheObservabilityRole() + { + var logger = new CapturingLogger(); + var results = new SetupResults + { + IsNonDwBlueprintFlow = false, + BlueprintCreated = true, + BlueprintId = BlueprintId, + TenantId = TenantId, + TenantWideConsentOutcome = Cli.Models.GrantOutcome.Granted, + BlueprintS2SOutcome = Cli.Models.GrantOutcome.Failed, + PendingBlueprintAppRoleSpecs = { ObservabilityAppRoleSpec }, + BatchPermissionsPhase1Completed = true, + BatchPermissionsPhase2Completed = true, + }; + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain("Observability API S2S app role (PowerShell)", + because: "AI Teammates still request OtelWrite, so a declined or failed grant keeps its hand-off"); + logger.AllOutput.Should().Contain($"$_.Value -eq '{ConfigConstants.ObservabilityApiOtelWriteScope}'", + because: "the recorded pending role is OtelWrite"); + logger.AllOutput.Should().Contain($"appId eq '{BlueprintId}'", + because: "the AI Teammate grant targets the blueprint service principal"); + } + + [Fact] + public void DisplaySetupSummary_PendingS2SAction_WithoutRecordedSpecs_DoesNotAssumeARole() + { + var logger = new CapturingLogger(); + var results = BuildS2SPendingResults(); + results.PendingAgentIdentityAppRoleSpecs.Clear(); + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain("Re-run 'a365 setup all'", + because: "without a recorded spec the hand-off can only point back to setup"); + logger.AllOutput.Should().NotContain("New-MgServicePrincipalAppRoleAssignment", + because: "without a recorded spec there is no role to assign"); + logger.AllOutput.Should().NotContain(ConfigConstants.ObservabilityApiOtelWriteScope, + because: "the hand-off must never fall back to a hardcoded role"); + } + + [Fact] + public void DisplaySetupSummary_PendingS2SAction_RepeatedSpec_IsListedOnce() + { + var logger = new CapturingLogger(); + var results = BuildS2SPendingResults(); + results.PendingAgentIdentityAppRoleSpecs.Add(CustomAppRoleSpec); + + SetupHelpers.DisplaySetupSummary(results, logger); + + System.Text.RegularExpressions.Regex.Matches(logger.AllOutput, "Value -eq 'Contoso.Write'").Count.Should().Be(1, + because: "a role recorded twice for the same target needs only one assignment command"); + } + + [Fact] + public void DisplaySetupSummary_PendingS2SAction_BothTargetsFailed_ListsEachTargetsRoles() + { + var logger = new CapturingLogger(); + var results = BuildS2SPendingResults(); + results.BlueprintS2SOutcome = Cli.Models.GrantOutcome.Failed; + results.PendingBlueprintAppRoleSpecs.Add(BlueprintOnlyAppRoleSpec); + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain("Contoso API S2S app role on the agent identity (PowerShell)", + because: "the agent identity's failed role needs its own hand-off"); + logger.AllOutput.Should().Contain("Fabrikam API S2S app role on the blueprint (PowerShell)", + because: "a blueprint role that also failed in the same run must not be dropped"); + logger.AllOutput.Should().Contain("$_.Value -eq 'Contoso.Write'") + .And.Contain("$_.Value -eq 'Fabrikam.Read'"); + logger.AllOutput.Should().Contain($"$agentSpId = '{AgentSpId}'") + .And.Contain($"appId eq '{BlueprintId}'", because: "each hand-off targets its own service principal"); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void DisplaySetupSummary_S2sMode_NoAppRolesToGrant_ReportsNotRequired(bool isAdmin) + { + var logger = new CapturingLogger(); + var results = BuildNoAppRolesToGrantResults(Cli.Models.AuthMode.S2s, isAdmin); + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain("not required (no S2S app roles to grant)", + because: "blueprint agents request no app role, so the s2s grant step has nothing to do"); + logger.AllOutput.Should().NotContain("tenant-wide delegated", + because: "an s2s run must not be reported as a delegated grant"); + logger.AllOutput.Should().NotContain("PENDING", + because: "nothing is pending when no app role needs to be granted"); + logger.AllOutput.Should().NotContain("Action Required", + because: "an s2s run with no app roles leaves nothing for an administrator to do"); + logger.AllOutput.Should().Contain("Setup completed successfully", + because: "no step failed and nothing is pending"); + } + + [Theory] + [InlineData(true, "granted tenant-wide delegated; no S2S app roles to grant")] + [InlineData(false, "PENDING; no S2S app roles to grant")] + public void DisplaySetupSummary_BothMode_NoAppRolesToGrant_ReportsTheDelegatedHalf(bool isAdmin, string expectedRow) + { + var logger = new CapturingLogger(); + var results = BuildNoAppRolesToGrantResults(Cli.Models.AuthMode.Both, isAdmin); + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Should().Contain(expectedRow, + because: "both mode still needs delegated consent, and the row must say the S2S half had nothing to grant"); + logger.AllOutput.Should().NotContain("New-MgServicePrincipalAppRoleAssignment", + because: "no app role was requested, so there is no S2S hand-off"); + if (!isAdmin) + logger.AllOutput.Should().Contain(NoAppRolesConsentUrl, + because: "a non-admin both-mode run still hands the delegated consent URL to an administrator"); + } + + private static SetupResults BuildNoAppRolesToGrantResults(Cli.Models.AuthMode authMode, bool isAdmin) => new() + { + IsNonDwBlueprintFlow = true, + BlueprintCreated = true, + BlueprintId = BlueprintId, + BlueprintServicePrincipalCreated = true, + AgentIdentityCreated = true, + AgentIdentityId = AgentSpId, + TenantId = TenantId, + EffectiveAuthMode = authMode, + ObservabilityPermissionsSkipped = true, + NoS2SAppRolesToGrant = true, + BatchPermissionsPhase1Completed = true, + BatchPermissionsPhase2Completed = true, + TenantWideConsentOutcome = isAdmin ? Cli.Models.GrantOutcome.Granted : Cli.Models.GrantOutcome.Failed, + CombinedConsentUrl = isAdmin ? null : NoAppRolesConsentUrl, + }; + // ── pendingAdminAction (DW path) ────────────────────────────────────────── [Fact] @@ -273,6 +456,84 @@ 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"); } + [Theory] + [InlineData(null)] + [InlineData(ConfigConstants.GccObservabilityApiAppId)] + public void DisplaySetupSummary_ObservabilitySkipped_PortalWalkthroughOmitsObservability(string? observabilityResourceAppId) + { + 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, + ObservabilityResourceAppId = observabilityResourceAppId, + }; + + 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, in any cloud"); + 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_IdentityFailed_RowPointsToTheListHoldingTheFailure(bool failureIsError, string expectedStatus) + { + var logger = new CapturingLogger(); + var results = new SetupResults + { + IsNonDwBlueprintFlow = true, + BlueprintCreated = true, + BlueprintId = BlueprintId, + AgentIdentityFailed = true, + AgentIdentityFailureIsError = failureIsError, + }; + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Split('\n').Should().ContainSingle(l => l.Contains("Agent identity")) + .Which.Should().Contain(expectedStatus, + because: "the identity row must point to Errors only when the writer recorded an error-severity identity failure"); + } + + [Theory] + [InlineData(true, "failed — see errors")] + [InlineData(false, "failed — see warnings")] + public void DisplaySetupSummary_RegistrationFailed_RowPointsToTheListHoldingTheFailure(bool failureIsError, string expectedStatus) + { + var logger = new CapturingLogger(); + var results = new SetupResults + { + IsNonDwBlueprintFlow = true, + BlueprintCreated = true, + BlueprintId = BlueprintId, + AgentIdentityCreated = true, + AgentIdentityId = AgentSpId, + AgentRegistrationFailed = true, + AgentRegistrationFailureIsError = failureIsError, + }; + + SetupHelpers.DisplaySetupSummary(results, logger); + + logger.AllOutput.Split('\n').Should().ContainSingle(l => l.Contains("Agent Registration")) + .Which.Should().Contain(expectedStatus, + because: "the registration row must point to Errors only when the writer recorded an error-severity registration failure"); + } + [Fact] public void DisplaySetupSummary_NonDwGccAdminConsentPending_UsesGccObservabilityResource() { @@ -718,6 +979,7 @@ private static SetupResults BuildFullSetupRegistrationWarningResults() TenantId = TenantId, EffectiveAuthMode = Cli.Models.AuthMode.S2s, AgentIdentityS2SOutcome = Cli.Models.GrantOutcome.Failed, + PendingAgentIdentityAppRoleSpecs = { CustomAppRoleSpec }, BatchPermissionsPhase1Completed = true, BatchPermissionsPhase2Completed = true, };