Provision A365 proxy app on develop-mcp publish by deepaligargms · Pull Request #499 · microsoft/Agent365-devTools

deepaligargms · 2026-09-22T04:55:28Z

Wire develop-mcp publish to provision the A365 proxy Entra app (confidential app + secret) and forward its credentials to the platform, so custom (non-Dataverse) MCP servers actually get a Power Platform connector created at publish time. Previously publish created only the PublicClients app, so the platform's connector-creation step logged A365ProxyConnectorCreation=SkippedNoCredentials and skipped. The register flow already provisions this proxy app; this mirrors it in publish. - CreateEntraAppsAsync now creates {server}-A365Proxy (confidential, with secret) first, failing the publish if it can't be created; CreateProxyAppAsync self-cleans its own orphan on partial failure. - PublishMcpServerRequest carries a365ProxyClientId / a365ProxyClientSecret; PublishMcpServerResponse reads A365ProxyRedirectUri (+ A365ProxyConnectorId). - After publish, the proxy app's redirect URIs are set from A365ProxyRedirectUri (tc/non-tc list), mirroring register; warns if absent. - The McpServer required-resource-access grant is added onto both the A365 proxy app and the PublicClients app. The platform wires the connector with the proxy app as its OAuth client and McpServerAppId as the resource, so the proxy app must hold this grant or Entra rejects the connector token request with AADSTS650057. - a365ProxyClientSecret is added to RedactSecretFields so the new secret is masked as ***REDACTED*** in verbose request-payload logging (alongside the other client secrets). - RollbackEntraAppsAsync now deletes both apps. Paired MCP-Platform change (connector create at publish, tenant-publish at approve) is already merged. Tests: new PublishCommandExecutorEntraAppTests (incl. grant-on-both-apps assertion), RedactSecretsFromPayload_RedactsA365ProxyClientSecret; updated regression + dry-run tests. Full suite green (2019 passed).

Before installing the version with this change(correct error message)

After installing the latest version of a365 cli with this change

Publish now creates the confidential A365 proxy Entra app (app + secret) alongside the PublicClients app and forwards its credentials to the platform, so custom (non-Dataverse) MCP servers get a Power Platform connector created at publish time instead of the platform logging A365ProxyConnectorCreation=SkippedNoCredentials. Mirrors the register flow: proxy app created first (fatal on failure, with self-cleanup), request carries the proxy clientId/secret, proxy redirect URIs are updated post-publish, and rollback deletes both apps. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Redact the A365 proxy client secret · New
Redact the A365 proxy secret from verbose logs · New

What changed in this PR

Adds A365 proxy app provisioning to develop-mcp publish, forwards credentials for connector creation, configures redirect URIs, and adds rollback/test coverage.

Changes:

Provisions confidential proxy apps with secrets.
Extends publish request/response models.
Updates rollback, redirect URI handling, tests, and changelog.

File	Description
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorEntraAppTests.cs	Updated as part of this pull request.
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/PublishCommandExecutorDryRunTests.cs	Updated as part of this pull request.
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandRegressionTests.cs	Updated as part of this pull request.
src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerResponse.cs	Updated as part of this pull request.
src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs	Updated as part of this pull request.
src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs	Updated as part of this pull request.
CHANGELOG.md	Updated as part of this pull request.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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 the McpServerScope required-resource-access grant or Entra rejects the connector's token request with AADSTS650057. ConfigureEntraAppsAsync previously granted this only on the PublicClients app; now it grants on both the proxy app and the PublicClients app, mirroring register. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Address the cleanup, secret-redaction, and failing dry-run assertion issues.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3

Open (3)

Update dry-run test for plural app creation message · New
Redact the A365 proxy secret from verbose logs
Redact the A365 proxy client secret

The publish request now carries the newly created A365 proxy Entra app secret as a365ProxyClientSecret. RedactSecretFields only masked clientApp1Secret/clientApp2Secret/clientSecret, so verbose request-payload logging wrote the live client secret in plaintext. Add a365ProxyClientSecret to the redaction key set with a regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Unresolved moderate findings affect cleanup, dry-run accuracy, permission handling, and cancellation behavior.

Review effort: Lite
Findings: None

Resolved since last review (3)

Update dry-run test for plural app creation message
Redact the A365 proxy secret from verbose logs
Redact the A365 proxy client secret

Previously missed (2)

In code that hasn't changed since last review

Include proxy configuration in dry-run output

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

The real publish path now also grants the MCP server permission to both apps and, when returned, writes the proxy redirect URIs, but this dry-run output still says it only back-fills the PPMI scope. That makes --dry-run under-report the changes users are previewing; update this message and its assertion to describe the new proxy configuration as well.

Roll back proxy app when public client creation fails

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

The proxy app is now created before CreatePublicClientsAppAsync, but an exception from that call can escape CreateEntraAppsAsync: its Graph app-creation call is outside the provisioner's catch, and ExecuteAsync has no rollback around app creation. A transient Graph/network failure here therefore leaves the newly created A365 proxy registration and secret orphaned. Catch this failure (while preserving cancellation), delete the proxy app, and return a failed publish; add a regression test for this partial-creation path.

Rick Brighenti (rbrighenti)

Requesting changes. The main concern is credential hygiene: publish now mints a confidential app with a secret on every run, and it can be left behind on partial failure. Details inline.

…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 (microsoft#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>

Copilot

Copilot review overview

🟡 Changes recommended

Secret-lifetime validation, option-forwarding coverage, and cancellation-safe cleanup remain unresolved.

Review effort: Lite
Findings: 1 · 1

Open (2)

Reject invalid publish secret lifetimes before app creation · New
Condense unreleased note to concise consumer-facing behavior · New

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues affect option propagation, orphan cleanup, and redirect-warning behavior.

Review effort: Lite
Findings: 1 · 3 · 1

Open (5)

Reject invalid publish secret lifetimes before app creation
Publish path ignores service tree ID and secret lifetime options · New
Proxy app is orphaned when public client creation fails · New
Spurious warning when no proxy connector is created · New
Condense unreleased note to concise consumer-facing behavior

…ndense CHANGELOG - Reject --secret-lifetime-months outside 1-24 before any Entra app or platform call, matching register's pre-flight guard (copilot review). - Condense the [Unreleased] entry to one consumer-facing sentence (copilot review). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Review effort: Lite
Findings: 2 · 1 · 1

Open (4)

Handle proxy app cleanup when provisioning throws or is cancelled · New
Do not delete proxy app when publish response lacks connector metadata · New
Publish path ignores service tree ID and secret lifetime options
Condense unreleased note to concise consumer-facing behavior

Resolved since last review (3)

Reject invalid publish secret lifetimes before app creation
Spurious warning when no proxy connector is created
Proxy app is orphaned when public client creation fails

Copilot

Copilot review overview

🟡 Changes recommended

Address the identified provisioning, cancellation cleanup, connector-response handling, required-grant failure, and changelog issues.

Review effort: Lite
Findings: 1 · 1 · 2

Open (4)

Handle proxy app cleanup when provisioning throws or is cancelled
Publish path ignores service tree ID and secret lifetime options
Update release notes for proxy app provisioning behavior · New
Condense unreleased note to concise consumer-facing behavior

Resolved since last review (1)

Do not delete proxy app when publish response lacks connector metadata

Copilot

Copilot review overview

🟡 Changes recommended

Rollback reuses a cancelled publish token, preventing cleanup of the proxy app and its secret.

Review effort: Balanced
Findings: 1

Open (1)

Use independent token for rollback deletes · New

Resolved since last review (4)

Handle proxy app cleanup when provisioning throws or is cancelled
Publish path ignores service tree ID and secret lifetime options
Update release notes for proxy app provisioning behavior
Condense unreleased note to concise consumer-facing behavior

+        await TryDeleteEntraAppAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct);
+        await TryDeleteEntraAppAsync(tenantId, apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct);


dbezic (dbezic)

The core connector-credential wiring is useful, but I recommend changes before
merging. First-party publishing now unnecessarily depends on client-secret
creation, and two partial-failure paths can leave the new proxy application
behind. Three isolated regression reproductions confirmed these behaviors.
The release notes also omit the new provisioning behavior and publish options.

Validation: 84 existing targeted tests passed. Three temporary tests asserting
the missing requirements failed as expected; these were run only in an isolated
PR worktree, not added to the branch. The project targets net8.0, but tests ran
on the installed .NET 10 runtime using DOTNET_ROLL_FORWARD=Major. NuGetAudit was
disabled only for the local restore because its endpoint was unreachable.
No live Graph/Power Platform integration test was performed.

dbezic (dbezic) · 2026-10-07T13:43:48Z

+        var a365ProxyApp = await provisioner.CreateProxyAppAsync(
+            input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy",
+            serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths, ct: ct);
+        if (a365ProxyApp is null) return null;


[P1] Do not require an unused client secret for first-party publishing

This call is mandatory for every server, and a failure to create the proxy
app or password aborts before the platform is called. A tenant that permits
public-client registrations but prohibits client secrets can therefore no
longer publish msdyn_DataverseMCPServer, although first-party/Dataverse
publishing does not create a proxy connector. The post-publish deletion
cannot address this case because execution never reaches it, and a shorter
--secret-lifetime-months value does not fix a password-creation prohibition.
Before this PR, publishing created only the public-client app.

Resolve whether proxy credentials are needed before making their creation
mandatory, or adjust the API contract so a first-party publish can proceed
without them while custom publishing still requires them. Add a regression
test with first-party publish and AddAppPasswordAsync returning null.

Reproduced: the platform mock was ready to succeed, but ExecuteAsync returned
false before publishing when secret creation was rejected.

dbezic (dbezic) · 2026-10-07T13:43:48Z

+        await TryDeleteEntraAppAsync(tenantId, apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct);
+    }


[P2] Use a cancellation-independent token for compensating deletion

Cancelling during PublishServerAsync is caught by the concrete tooling
service and converted to null. ExecuteAsync therefore enters this rollback
with an already-cancelled token. Both deletes inherit that token, so Graph's
authenticated request/SendAsync cannot complete; the helper logs the errors
and leaves the app registrations, including the new proxy app and its live
secret, behind. This PR extends the existing rollback problem to a newly
introduced credential-bearing app.

Use a separate, preferably bounded cleanup token, as the Public Clients
exception cleanup already does, and retain correct cancellation semantics
after cleanup. Add an end-to-end executor test where publishing cancels the
token and returns null.

Reproduced: neither application's deletion completed. This also confirms the
still-open review thread:
#499 (comment)

dbezic (dbezic) · 2026-10-07T13:43:48Z

+        var a365ProxyApp = await provisioner.CreateProxyAppAsync(
+            input.ServerName, tenantId, suffix: "A365Proxy", roleDisplay: "A365 Proxy",
+            serviceTreeId: input.ServiceTreeId, lifetimeMonths: input.SecretLifetimeMonths, ct: ct);


[P2] Cover exceptions during proxy-secret creation with orphan cleanup

The new proxy-provisioning call is outside the try/catch below. Its existing
helper deletes the app when AddAppPasswordAsync returns null, but not when
that call throws. GraphPostWithResponseAsync rethrows transport failures and
cancellation, so an addPassword connection reset after successful app
creation escapes the entire publish operation without any compensating
delete. If the password was created before the response was lost, the orphan
also retains a live credential. Previously this publish flow never performed
this proxy/password stage.

Make the provisioner clean up the known application object on exceptions
after creation using a cancellation-independent token, then preserve the
original error/cancellation. The later Public Clients catch cannot clean up
this earlier failure because it has not yet received the proxy result.

Reproduced with AddAppPasswordAsync throwing HttpRequestException: the
exception escaped and DeleteEntraAppAsync was never called.

dbezic (dbezic) · 2026-10-07T13:43:48Z

+        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);


Document the new publish behavior and options in the release notes

The current PR head has no CHANGELOG.md diff. Its Unreleased section still
describes publish as creating only the PublicClients registration (line
171), and the secret-lifetime entry documents only register-external-mcp-server.
It does not document the new publish --service-tree-id and
--secret-lifetime-months options, proxy credential creation, or its cleanup
behavior. This is a user-visible provisioning and tenant-policy change.

Add crisp consumer-facing Unreleased entries with (#499), update the stale
PublicClients-only entry, and document the new flags in the command reference.
The earlier changelog review thread is marked resolved, but the change is
absent from the latest PR head.

Krishnadheeraj (DheerajPannala)

Reconciling the proxy app after the platform responds is a practical way to handle not being able to classify servers up front. The executor changes are cleanly structured, and the main paths have good test coverage.

Two minor comments inline.

Outside the changed lines:

Minor: Update the release notes for the proxy app and publish options (CHANGELOG.md:171). The maintained repository guidance requires every user-facing feature to have a crisp [Unreleased] entry and requires stale sibling entries to be fixed. The current changelog has no #499 entry and still says publish creates only <server-name>-PublicClients, omitting the confidential proxy app, connector behavior, and the two new options.

Krishnadheeraj (DheerajPannala) · 2026-10-07T13:53:26Z

+        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);


Minor: A failed delete of the unused proxy app still reports a clean, successful publish

On a successful first-party or Dataverse publish, the unused proxy app is removed through TryDeleteEntraAppAsync, which returns nothing and only logs. If the delete fails, the only trace is an error log saying "Failed to roll back Entra app ... Delete it manually". warnings isn't updated, so DisplayResults prints the green "published" line and the command exits 0. In scripted or CI runs, an unused confidential app with a live client secret stays in the tenant with nothing in the result to show it. When the delete succeeds, the log says "Rolled back Entra app" after a publish that succeeded, which is confusing.

Evidence

PublishCommandExecutor.cs:408 declares private async Task TryDeleteEntraAppAsync(...), which returns a plain Task. Its outcomes are only logged: "Rolled back Entra app ..." at 420, "Failed to roll back Entra app ... Delete it manually" at 424-426, and "Exception rolling back ..." at 429-434. Lines 456-460 call it without touching concurrentWarnings or warnings. Lines 189-190 run DisplayResults(input, warnings); return true;, and lines 614-621 print the green "MCP server '...' published as '...'" line when warnings.Count == 0. Scenario: Graph returns 403 or a transient error on DELETE /applications/{id} after a Dataverse publish. The command exits 0 with the success banner, and <server>-A365Proxy keeps its secret.

Suggested fix: Have TryDeleteEntraAppAsync return a bool, or accept the warnings collection. On this path, add a warning such as "Unused A365 proxy app '' (clientId ...) could not be deleted; delete it manually." Log "Deleted" instead of "Rolled back" here.

deepaligargms requested review from a team as code owners September 22, 2026 04:55

Copilot AI lite review requested due to automatic review settings September 22, 2026 04:55

Copilot started reviewing on behalf of deepaligargms September 22, 2026 04:56 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

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

Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Models/PublishMcpServerRequest.cs

Copilot AI review requested due to automatic review settings September 22, 2026 05:39

Copilot started reviewing on behalf of deepaligargms September 22, 2026 05:40 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

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

Copilot AI review requested due to automatic review settings September 22, 2026 18:00

Copilot started reviewing on behalf of deepaligargms September 22, 2026 18:02 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Rick Brighenti (rbrighenti) requested changes Sep 23, 2026

View reviewed changes

Copilot AI lite review requested due to automatic review settings September 29, 2026 19:17

Copilot started reviewing on behalf of deepaligargms September 29, 2026 19:18 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

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

Comment thread CHANGELOG.md Outdated

Copilot AI lite review requested due to automatic review settings September 29, 2026 19:26

deepaligargms force-pushed the deepaligarg-microsoft-publish-a365-proxy-app branch from 7fe23fd to 9c97cc0 Compare September 29, 2026 19:26

Copilot started reviewing on behalf of deepaligargms September 29, 2026 19:27 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs Outdated

Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs Outdated

Comment thread src/Microsoft.Agents.A365.DevTools.Cli/Commands/PublishCommandExecutor.cs Outdated

Copilot AI lite review requested due to automatic review settings September 29, 2026 23:29

Copilot started reviewing on behalf of deepaligargms September 29, 2026 23:30 View session

Copilot AI reviewed Sep 29, 2026

View reviewed changes

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

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

Merge branch 'main' into deepaligarg-microsoft-publish-a365-proxy-app

ea2a978

Copilot AI lite review requested due to automatic review settings October 1, 2026 17:27

Copilot started reviewing on behalf of deepaligargms October 1, 2026 17:28 View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

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

deepaligargms enabled auto-merge (squash) October 2, 2026 05:08

dbezic (dbezic) requested a balanced review from Copilot October 5, 2026 15:02

Copilot started reviewing on behalf of dbezic (dbezic) October 5, 2026 15:05 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

dbezic (dbezic) requested changes Oct 7, 2026

View reviewed changes

Krishnadheeraj (DheerajPannala) reviewed Oct 7, 2026

View reviewed changes

		await TryDeleteEntraAppAsync(tenantId, apps.PublicClientsObjectId, apps.PublicClientsClientId, apps.PublicClientsAppName, ct);
		await TryDeleteEntraAppAsync(tenantId, apps.A365AppObjectId, apps.A365AppClientId, apps.A365AppName, ct);

Conversation

deepaligargms commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Rick Brighenti (rbrighenti) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

dbezic (dbezic) left a comment

Choose a reason for hiding this comment

Uh oh!

dbezic (dbezic) Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

dbezic (dbezic) Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

dbezic (dbezic) Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

dbezic (dbezic) Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

Krishnadheeraj (DheerajPannala) left a comment

deepaligargms commented Sep 22, 2026 •

edited

Loading