Address PR #499 review: gate proxy app, add options, harden cleanup

Publish still forwards the A365 proxy app credentials on every call (the
CLI can't classify custom vs first-party before the platform does), but
now reconciles after publish: when the response shows no connector was
created, the unused proxy app is deleted so no orphaned credential lingers.

- Post-publish cleanup of the unused proxy app for first-party/Dataverse
  servers, gated on the platform returning a connector id / redirect URI.
- Redirect-URI warning now fires only when a connector was actually
  created but no URI came back, not on every first-party publish.
- Proxy required-resource-access grant applied only when a connector
  exists; Public Clients grant unchanged.
- New --service-tree-id and --secret-lifetime-months options on publish,
  threaded to both created Entra apps, mirroring register.
- Orphaned proxy app is deleted if Public Clients creation throws after
  the proxy app was created.
- Dry-run output now describes proxy creation, permission/redirect config,
  and cleanup.
- CHANGELOG entry references (#499).

Tests: proxy grant on both apps only when a connector exists, unused-proxy
deletion, orphan-cleanup-on-throw, option flow-through, and updated publish
option/dry-run assertions with documented requirement changes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: deepaligargms

CHANGELOG.md

@@ -23,7 +23,7 @@ Agents provisioned before this release need `Agent365.Observability.OtelWrite` g
 **Option B — CLI** (`a365 setup admin`) has been removed in this release. Use Option A above, or copy the PowerShell instructions printed in the `a365 setup all` summary output.
 
 ### Added
-- `develop-mcp publish` now creates the A365 proxy Entra app and sends its credentials to the platform, so custom (non-Dataverse) MCP servers get a Power Platform connector created at publish time.
+- `develop-mcp publish` now creates the A365 proxy Entra app and sends its credentials to the platform, so custom (non-Dataverse) MCP servers get a Power Platform connector created at publish time. The proxy app is removed again when the server turns out not to need a connector, and `--service-tree-id` / `--secret-lifetime-months` options are honored for it (#499).
 - Setup and bootstrap now use Microsoft's first-party Agent 365 CLI application when it is present in your tenant, validating it without changing Microsoft's app registration, and fall back to a tenant-owned "Agent 365 CLI" app when it is not (#489).
 - Log separator written at the start of each CLI invocation now redacts values for secret-bearing options (e.g. `--idp-client-secret`) so they are not written to the log file in plain text.
 - Authentication context (tenant and user) is now logged at the `Information` level whenever the resolved sign-in identity changes, giving operators a clear audit trail in the log file of who the CLI is acting as, without exposing credentials.

src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs

@@ -386,6 +386,12 @@ private static Command CreatePublishSubcommand(
             description: "Publisher name for the MCP Server. Required for custom (user-created) MCP servers; ignored for 1p Microsoft-owned servers (e.g. msdyn_DataverseMCPServer) which always publish as 'Microsoft'.");
         command.AddOption(publisherNameOption);
 
+        var serviceTreeIdOption = new Option<string?>("--service-tree-id", description: "ServiceTree ID for Entra app registration (required in Microsoft corporate tenants)");
+        command.AddOption(serviceTreeIdOption);
+
+        var secretLifetimeMonthsOption = new Option<int?>(["--secret-lifetime-months", "-l"], description: "Lifetime in months (1-24) for the generated client secret on the A365 proxy Entra app. Default is 2 years. Set a value smaller than the appManagementPolicies cap in your tenant.");
+        command.AddOption(secretLifetimeMonthsOption);
+
         var yesOption = new Option<bool>(
             ["--yes", "-y"],
             description: "Skip the interactive 'Proceed with publish? (y/N)' confirmation.");
@@ -406,7 +412,9 @@ private static Command CreatePublishSubcommand(
                 DisplayName: context.ParseResult.GetValueForOption(displayNameOption),
                 PublisherName: context.ParseResult.GetValueForOption(publisherNameOption),
                 Yes: context.ParseResult.GetValueForOption(yesOption),
-                DryRun: context.ParseResult.GetValueForOption(dryRunOption));
+                DryRun: context.ParseResult.GetValueForOption(dryRunOption),
+                ServiceTreeId: context.ParseResult.GetValueForOption(serviceTreeIdOption),
+                SecretLifetimeMonths: context.ParseResult.GetValueForOption(secretLifetimeMonthsOption));
 
             var executor = new PublishCommandExecutor(logger, toolingService, graphApiService);
             var success = await executor.ExecuteAsync(args, context.GetCancellationToken());

src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs

@@ -19,7 +19,9 @@ internal record RawPublishArgs(
     string? DisplayName,
     string? PublisherName,
     bool Yes,
-    bool DryRun);
+    bool DryRun,
+    string? ServiceTreeId = null,
+    int? SecretLifetimeMonths = null);
 
 /// <summary>
 /// Orchestrates first-party MCP server publish in one CLI command. The shape mirrors
@@ -68,6 +70,14 @@ private sealed record ResolvedInput
         // When true, skip the interactive "Proceed with publish? (y/N)" confirmation. Set via
         // --yes / -y. Required for non-interactive contexts (CI scripts, automation).
         public required bool Yes { get; init; }
+
+        // ServiceTree ID stamped onto the Entra apps created here. Required in Microsoft corporate
+        // tenants; null elsewhere. Applied to both the A365 proxy and Public Clients apps.
+        public string? ServiceTreeId { get; init; }
+
+        // Optional client-secret lifetime (months) for the A365 proxy app's secret. Null uses Graph's
+        // default; a smaller value avoids the appManagementPolicies cap failing publish in strict tenants.
+        public int? SecretLifetimeMonths { get; init; }
     }
 
     internal sealed record EntraAppSet(
@@ -89,7 +99,8 @@ internal async Task<bool> ExecuteAsync(RawPublishArgs args, CancellationToken ct
         if (input.DryRun)
         {
             _logger.LogInformation("[DRY RUN] Would create Entra apps '{PublicClients}' and '{A365Proxy}' in tenant", $"{input.ServerName}-PublicClients", $"{input.ServerName}-A365Proxy");
-            _logger.LogInformation("[DRY RUN] Would call publish endpoint and back-fill PPMI scope on the created app");
+            _logger.LogInformation("[DRY RUN] Would call the publish endpoint and forward the A365 proxy app credentials so the platform can create the Power Platform connector for custom (non-Dataverse) servers");
+            _logger.LogInformation("[DRY RUN] Would back-fill the PPMI scope on the created apps, add the McpServer API permission and connector redirect URI to the A365 proxy app, and delete the proxy app when the publish response shows no connector was created");
             return true;
         }
 
@@ -282,6 +293,8 @@ internal async Task<bool> ExecuteAsync(RawPublishArgs args, CancellationToken ct
                 PublisherName = string.IsNullOrWhiteSpace(publisherName) ? null : publisherName,
                 Yes = args.Yes,
                 DryRun = args.DryRun,
+                ServiceTreeId = string.IsNullOrWhiteSpace(args.ServiceTreeId) ? null : args.ServiceTreeId,
+                SecretLifetimeMonths = args.SecretLifetimeMonths,
             };
         }
         catch (ArgumentException ex)
@@ -304,6 +317,10 @@ private void DisplayPublishSummary(ResolvedInput input)
         DevelopMcpCommand.WriteLabel("  Alias:          "); Console.WriteLine(input.Alias);
         DevelopMcpCommand.WriteLabel("  Display Name:   "); Console.WriteLine(input.DisplayName);
         DevelopMcpCommand.WriteLabel("  Publisher:      "); Console.WriteLine(input.PublisherName ?? "(none — platform will reject if this is a custom server)");
+        if (input.SecretLifetimeMonths is { } lifetime)
+        {
+            DevelopMcpCommand.WriteLabel("  Secret Lifetime: "); Console.WriteLine($"{lifetime} month(s)");
+        }
         Console.WriteLine();
     }
 
@@ -328,20 +345,36 @@ private void DisplayPublishSummary(ResolvedInput input)
         // platform creates the Power Platform connector only when its credentials are supplied.
         var a365ProxyApp = await provisioner.CreateProxyAppAsync(
             input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy",
-            serviceTreeId: null, ct: ct);
+            serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths, ct: ct);
         if (a365ProxyApp is null) return null;
 
-        var publicClients = await provisioner.CreatePublicClientsAppAsync(
-            input.ServerName, tenantId, serviceTreeId: null, warnings, ct);
-
-        return new EntraAppSet(
-            PublicClientsClientId: publicClients.ClientId,
-            PublicClientsObjectId: publicClients.ObjectId,
-            PublicClientsAppName: publicClients.AppName,
-            A365AppClientId: a365ProxyApp.ClientId,
-            A365AppSecret: a365ProxyApp.Secret,
-            A365AppObjectId: a365ProxyApp.ObjectId,
-            A365AppName: a365ProxyApp.AppName);
+        // If Public Clients creation throws after the proxy app exists, the proxy app (with its
+        // secret) is orphaned — RollbackEntraAppsAsync only runs once we have a full EntraAppSet and
+        // the platform call fails. Clean it up here so a Graph error / throttling / cancellation
+        // doesn't leak a credential. Use CancellationToken.None for the compensating delete so a
+        // caller Ctrl+C still removes the orphan.
+        try
+        {
+            var publicClients = await provisioner.CreatePublicClientsAppAsync(
+                input.ServerName, tenantId, serviceTreeId: input.ServiceTreeId, warnings, ct);
+
+            return new EntraAppSet(
+                PublicClientsClientId: publicClients.ClientId,
+                PublicClientsObjectId: publicClients.ObjectId,
+                PublicClientsAppName: publicClients.AppName,
+                A365AppClientId: a365ProxyApp.ClientId,
+                A365AppSecret: a365ProxyApp.Secret,
+                A365AppObjectId: a365ProxyApp.ObjectId,
+                A365AppName: a365ProxyApp.AppName);
+        }
+        catch (Exception ex)
+        {
+            _logger.LogError("Failed to create the Public Clients Entra app after the A365 proxy app was created; deleting the orphaned proxy app '{A365Proxy}'.", a365ProxyApp.AppName);
+            _logger.LogDebug("Exception details: {Exception}", ex.ToString());
+            await TryDeleteEntraAppAsync(tenantId, a365ProxyApp.ObjectId, a365ProxyApp.ClientId, a365ProxyApp.AppName, CancellationToken.None);
+            if (ex is OperationCanceledException && ct.IsCancellationRequested) throw;
+            return null;
+        }
     }
 
     // Best-effort compensating delete for the Entra apps created in CreateEntraAppsAsync, run when
@@ -358,40 +391,41 @@ internal async Task RollbackEntraAppsAsync(EntraAppSet apps, string tenantId, Ca
 
         _logger.LogInformation("Rolling back Entra app registrations created for failed publish...");
 
-        if (!string.IsNullOrWhiteSpace(apps.PublicClientsObjectId))
-        {
-            await DeleteOneAsync(apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct);
-        }
+        await TryDeleteEntraAppAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct);
+        await TryDeleteEntraAppAsync(tenantId, apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct);
+    }
 
-        if (!string.IsNullOrWhiteSpace(apps.A365AppObjectId))
+    // Best-effort compensating delete for a single Entra app. No-op when the object id is unknown or
+    // Graph is unavailable. Failures are logged with both clientId and objectId so the user can clean
+    // up manually; the delete never throws.
+    private async Task TryDeleteEntraAppAsync(string tenantId, string? objectId, string? clientId, string appName, CancellationToken ct)
+    {
+        if (_graphApiService is null || string.IsNullOrWhiteSpace(objectId))
         {
-            await DeleteOneAsync(apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct);
+            return;
         }
 
-        async Task DeleteOneAsync(string objectId, string? clientId, string appName, CancellationToken cancellationToken)
+        try
         {
-            try
+            var deleted = await _graphApiService.DeleteEntraAppAsync(tenantId, objectId, ct);
+            if (deleted)
             {
-                var deleted = await _graphApiService!.DeleteEntraAppAsync(tenantId, objectId, cancellationToken);
-                if (deleted)
-                {
-                    _logger.LogInformation("Rolled back Entra app '{AppName}' (objectId {ObjectId})", appName, objectId);
-                }
-                else
-                {
-                    _logger.LogError(
-                        "Failed to roll back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.",
-                        appName, clientId ?? "<unknown>", objectId);
-                }
+                _logger.LogInformation("Rolled back Entra app '{AppName}' (objectId {ObjectId})", appName, objectId);
             }
-            catch (Exception ex)
+            else
             {
                 _logger.LogError(
-                    ex,
-                    "Exception rolling back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.",
+                    "Failed to roll back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.",
                     appName, clientId ?? "<unknown>", objectId);
             }
         }
+        catch (Exception ex)
+        {
+            _logger.LogError(
+                ex,
+                "Exception rolling back Entra app '{AppName}' (clientId {ClientId}, objectId {ObjectId}). Delete it manually in the Azure portal.",
+                appName, clientId ?? "<unknown>", objectId);
+        }
     }
 
     private async Task ConfigureEntraAppsAsync(
@@ -405,6 +439,19 @@ private async Task ConfigureEntraAppsAsync(
         var tasks = new List<Task>();
         var concurrentWarnings = new System.Collections.Concurrent.ConcurrentBag<string>();
 
+        // The proxy app + secret are forwarded on every publish because the CLI can't classify the
+        // server (custom vs 1p/Dataverse) before the platform does. Only custom servers actually get
+        // a Power Platform connector; the platform signals that by returning a connector id and/or a
+        // redirect URI. When neither is present the proxy app is unused, so delete it here rather than
+        // leave an unused credential in the tenant (the Public Clients app and its PPMI grant remain).
+        var connectorCreated = !string.IsNullOrWhiteSpace(response.A365ProxyConnectorId)
+            || !string.IsNullOrWhiteSpace(response.A365ProxyRedirectUri);
+        if (!connectorCreated)
+        {
+            _logger.LogInformation("Publish returned no A365 proxy connector for '{ServerName}' (first-party / Dataverse server); removing the unused A365 proxy app '{A365Proxy}'.", input.ServerName, apps.A365AppName);
+            await TryDeleteEntraAppAsync(tenantId, apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct);
+        }
+
         // Grant required-resource-access on the just-created Public Clients Entra app.
         // The platform resolves the right resource per server type (Custom: managedidentityid; app-based
         // / Dataverse MCP: 1p mappings; fallback: platform's own app id) and returns both the resource
@@ -442,9 +489,13 @@ private async Task ConfigureEntraAppsAsync(
         {
             // The platform wires the A365 proxy connector with the proxy app as its OAuth client and
             // McpServerAppId as the resource, so the proxy app must hold this required-resource-access
-            // grant or Entra rejects the token request (AADSTS650057). Grant it on both the proxy app
-            // and the Public Clients app, mirroring register.
-            tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct));
+            // grant or Entra rejects the token request (AADSTS650057). Grant it on the proxy app only
+            // when a connector was created (otherwise the proxy app was just deleted above); always
+            // grant it on the Public Clients app, mirroring register.
+            if (connectorCreated)
+            {
+                tasks.Add(AddRequiredResourceAccessAsync(tenantId, apps.A365AppObjectId, apps.A365AppName, resourceAppId!, resourceScopeId.Value, concurrentWarnings, ct));
+            }
 
             if (apps.PublicClientsObjectId != null)
             {
@@ -460,16 +511,21 @@ private async Task ConfigureEntraAppsAsync(
 
         // Custom (non-Dataverse) servers get a Power Platform connector whose redirect URI the
         // platform returns here. Write it onto the A365 proxy app so the connector's OAuth flow works.
-        var a365RedirectUri = response.A365ProxyRedirectUri;
-        if (!string.IsNullOrWhiteSpace(a365RedirectUri))
-        {
-            tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings, ct));
-        }
-        else
+        // Only relevant when a connector was created; first-party servers get no connector (and the
+        // proxy app was already removed), so no redirect URI is expected and none is warned about.
+        if (connectorCreated)
         {
-            var msg = "A365 Proxy redirect URI was not returned by publish. Redirect URI configuration skipped.";
-            _logger.LogWarning(msg);
-            concurrentWarnings.Add(msg);
+            var a365RedirectUri = response.A365ProxyRedirectUri;
+            if (!string.IsNullOrWhiteSpace(a365RedirectUri))
+            {
+                tasks.Add(UpdateA365RedirectUrisAsync(tenantId, apps, a365RedirectUri, concurrentWarnings, ct));
+            }
+            else
+            {
+                var msg = "A365 Proxy connector was created but publish returned no redirect URI. Redirect URI configuration skipped.";
+                _logger.LogWarning(msg);
+                concurrentWarnings.Add(msg);
+            }
         }
 
         await Task.WhenAll(tasks);

src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs

@@ -124,8 +124,10 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases()
         var options = subcommand.Options.ToList();
 
         // Verify all expected options exist. Tenant ID is auto-detected from the current az login
-        // session, so publish does not expose --tenant-id; ServiceTree tagging is not required for
-        // publish since it targets Dataverse environments rather than Microsoft corp tenants.
+        // session, so publish does not expose --tenant-id. Publish now registers the A365 proxy and
+        // Public Clients Entra apps in the operator's own tenant (via az login) — which may be a
+        // ServiceTree-enrolled Microsoft corp tenant — so it exposes --service-tree-id and
+        // --secret-lifetime-months, mirroring register.
         var optionNames = options.Select(o => o.Name).ToList();
         optionNames.Should().Contain("environment-id");
         optionNames.Should().Contain("server-name");
@@ -135,10 +137,16 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases()
             "tenant-id",
             because: "tenant id is auto-detected from the current 'az login' session; exposing " +
                      "--tenant-id would imply per-publish tenant targeting that the executor does not support.");
-        optionNames.Should().NotContain(
+        optionNames.Should().Contain(
             "service-tree-id",
-            because: "publish targets a customer's Dataverse env, not a Microsoft corp tenant — " +
-                     "the ServiceTree tagging that --service-tree-id provides is not applicable here.");
+            because: "publish creates Entra app registrations in the operator's own tenant, which may " +
+                     "be ServiceTree-enrolled; those registrations are rejected without a " +
+                     "serviceManagementReference, so --service-tree-id must be available (reviewer request on #499, same as #496).");
+        optionNames.Should().Contain(
+            "secret-lifetime-months",
+            because: "the A365 proxy app's client secret must fit under the tenant's appManagementPolicies " +
+                     "lifetime cap or publish fails in strict tenants; --secret-lifetime-months lets the " +
+                     "operator set a compliant lifetime, mirroring register.");
         optionNames.Should().Contain("dry-run");
 
         // Verify critical aliases for Azure CLI compliance
@@ -153,6 +161,11 @@ public void PublishSubcommand_HasCorrectOptionsWithAliases()
 
         var displayNameOption = options.FirstOrDefault(o => o.Name == "display-name");
         displayNameOption!.Aliases.Should().Contain("-d");
+
+        var secretLifetimeOption = options.FirstOrDefault(o => o.Name == "secret-lifetime-months");
+        secretLifetimeOption!.Aliases.Should().Contain(
+            "-l",
+            because: "register exposes --secret-lifetime-months as -l; publish must use the same alias for consistency.");
     }
 
     [Fact]