Add a365 network gsa enable|disable|status

[Lala Sushant Srivastava (lasrivas)]

Adds a365 network gsa enable|disable|status, which turns Global Secure Access on or off for the tenant's Agent 365 environment.

Global Secure Access is a per-environment Power Platform setting, so configuring it normally needs the environment's id. Agent 365 does not publish that id, so the CLI asks the platform to apply the setting to the environment it resolves for the tenant. The platform half shipped in bic/MCP-Platform#3686.

Design

docs/superpowers/specs/2026-09-15-gsa-environment-setting-design.md.

Notable decisions

• NotConfigured is reported distinctly from Disabled. A tenant that has never set the value has not turned it off, and the distinction changes what an admin does next, so status says so in as many words.
• Still pending is not a failure. The platform accepts the change and the environment catches up; a non-zero exit would break scripts that chain on success. --wait polls for the requested value, with a ten-minute ceiling armed on the request token so an in-flight poll cannot overshoot it.
• No --tenant-id. The commands authenticate against the tenant of your current az login. The command resolves that account once and hands it to the service, so --wait cannot confirm one tenant and change another, and a transient az failure part-way through a wait does not abort a change that already succeeded.
• enable and disable prompt before applying, naming the tenant, and take --yes to skip. status is read-only and does not prompt.

Scope

This PR no longer carries the virtual network change set. It was originally branched to include #494's work, which duplicated roughly 1400 lines across the two PRs and meant every VNet review finding had to be answered twice. GSA never depended on it — GsaService talks to the platform directly and shares only the tenant-resolution and confirmation helpers in NetworkCommand. Removed in d923c2a.

Both branches now define the network root command and those shared helpers, so whichever merges second will conflict there. That is a smaller price than reviewing the same code twice.

Review rounds

Three rounds of review are folded in. The first (16ef241) covered ARM-shape validation, NotStarted as in-flight, tenant-targeted tokens, --yes confirmation on enable and disable, handler-body test coverage and the CHANGELOG entry. The second (71a6570, ae03dfe) armed the --wait ceiling on the request token and corrected the az login prerequisite in the docs.

Third round (94e71aa, a15fac1)

• The account is resolved once and threaded through. The handler resolved az account show to name the tenant in the confirmation prompt, then GsaService resolved it again to authenticate. An az account set between the two, or any mutation of CLI state mid-run, meant confirming tenant A and changing tenant B. IGsaService now takes the resolved AzureAccountInfo as its first parameter, which also let the service drop its IAzureCliService dependency and its account cache -- that cache was itself unsound, because AzureAccountInfo.TenantId defaults to string.Empty rather than null, so a ??= cached a blank-tenant account forever instead of retrying.
• SendAsync no longer rethrows HttpClient's own timeout as cancellation. It surfaces as an OperationCanceledException with no token cancelled, so a bare enable, disable or status threw at the caller instead of returning the documented null and logging the failure. Now rethrows only when the supplied token is actually cancelled, which still covers Ctrl+C and the wait ceiling firing on its linked token.
• A per-request HttpClient no longer disposes a handler it does not own. HttpClientFactory.CreateAuthenticatedClient built the client as new HttpClient(handler), which defaults to disposeHandler: true, so a service that holds one handler as a field and builds a client per request lost that handler to the first client's disposal. Every later request, including every poll after the first, would have failed with ObjectDisposedException against any handler with real disposal semantics. A supplied handler now stays owned by the supplier.
• docs/commands/network-gsa.md prerequisite paragraph rewritten; the split in d923c2a had left a duplicated fragment mid-sentence.

Tests

Full suite: 2068 passed, 0 failed, 12 skipped.

[github-actions]

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[Copilot]

🟡 Changes recommended

Unresolved tenant-targeting, cancellation, validation, and VNet polling issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds tenant-level GSA enable, disable, and status commands alongside supporting VNet networking functionality.

Changes:

• Adds GSA services, models, authentication, and convergence polling.
• Registers network commands, handlers, and dependency injection.
• Adds tests, documentation, and release notes.

File summaries

File	Reviewed change
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/VNetLinkServiceTests.cs	VNet service tests
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GsaServiceTests.cs	GSA service tests
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/ArmApiServiceTests.cs	ARM policy lookup tests
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/NetworkCommandTests.cs	Network command tests
src/Microsoft.Agents.A365.DevTools.Cli/Services/VNetLinkService.cs	VNet operations and polling
src/Microsoft.Agents.A365.DevTools.Cli/Services/IVNetLinkService.cs	VNet service contract
src/Microsoft.Agents.A365.DevTools.Cli/Services/IGsaService.cs	GSA service contract
src/Microsoft.Agents.A365.DevTools.Cli/Services/GsaService.cs	GSA API operations and polling
src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs	Enterprise policy resolution
src/Microsoft.Agents.A365.DevTools.Cli/Program.cs	Command and service registration
src/Microsoft.Agents.A365.DevTools.Cli/Models/VNetModels.cs	VNet models
src/Microsoft.Agents.A365.DevTools.Cli/Models/GsaModels.cs	GSA response models
src/Microsoft.Agents.A365.DevTools.Cli/Constants/CommandNames.cs	Network command constant
src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs	Network, VNet, and GSA command tree
docs/commands/README.md	Command index entries
docs/commands/network.md	VNet command documentation
docs/commands/network-gsa.md	GSA command documentation
CHANGELOG.md	Release notes

Review details

Suppressed comments (7)

src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs:260

• The new enable/disable handlers are not exercised through Command.InvokeAsync; the tests only parse gsa enable and call ReportGsaAsync directly. A wiring regression could leave SetAsync uncalled or lose the handler's exit-code propagation while all tests stay green; add invocation tests for both verbs covering service calls and success/failure results.

            var result = await gsaService.SetAsync(enabled, ct);
            context.ExitCode = await ReportGsaAsync(logger, gsaService, result, wait, enabled, ct);

src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs:287

• The new GSA status handler is also only covered by parse/tree assertions; no test invokes it to verify that GetStatusAsync is called and that null responses produce exit code 1. Add an invocation test for the success and request-error branches so handler wiring is covered, not just LogGsaStatus/ReportGsaAsync.

            var status = await gsaService.GetStatusAsync(ct);
            if (status == null)
            {
                context.ExitCode = 1;
                return;
            }

            LogGsaStatus(logger, status);
            context.ExitCode = 0;

src/Microsoft.Agents.A365.DevTools.Cli/Commands/NetworkCommand.cs:111

• --tenant-id is an Option<string?>; an explicitly supplied empty or whitespace value enters this branch and is silently replaced with the current az tenant. That makes a malformed explicit target indistinguishable from omission and can apply a tenant-wide link to the wrong tenant. Detect whether the option was supplied and reject blank values with a targeted error before falling back.

            if (string.IsNullOrWhiteSpace(tenantId))
            {
                var account = await azureCliService.GetCurrentAccountAsync();
                tenantId = account?.TenantId;
                if (string.IsNullOrWhiteSpace(tenantId))

src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs:328

• When the command token is cancelled during the policy GET or response read, RetryHelper rethrows the cancellation, but this catch converts it to null. LinkAsync then reports an ordinary policy-resolution failure instead of honoring Ctrl+C; rethrow OperationCanceledException when ct is cancelled before the catch-all.

            catch (Exception ex)
            {
                if (NetworkHelper.IsConnectionResetByProxy(ex))
                    _logger.LogWarning(NetworkHelper.ConnectionResetWarning);
                else

src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs:276

• policyArmId is CLI-controlled and is concatenated directly into the ARM URL after only a whitespace check. A value containing query, fragment, or path-traversal syntax can alter the resource or api-version portion of the request; validate it as a well-formed /subscriptions/.../providers/Microsoft.PowerPlatform/enterprisePolicies/... resource ID before constructing this URL.

            var url = $"{ArmBaseUrl}{policyArmId}?api-version={apiVersion}";

src/Microsoft.Agents.A365.DevTools.Cli/Services/ArmApiService.cs:269

• tenantId is a required non-nullable parameter, but it is passed to EnsureArmHeadersAsync without a whitespace guard. A blank value can fall through to common-tenant authentication and defeat the tenant-targeted ARM read; reject it before the first use.

        string tenantId,
        CancellationToken ct = default)
    {
        if (string.IsNullOrWhiteSpace(policyArmId))
            throw new ArgumentException("Policy ARM id is required.", nameof(policyArmId));

        if (!await EnsureArmHeadersAsync(tenantId, ct))

src/Microsoft.Agents.A365.DevTools.Cli/Services/GsaService.cs:120

• The account lookup validates only TenantId; an az account show result with a missing user name is passed as an empty userId, which disables the user hint and lets the broker select another cached account. For a tenant-wide setting, reject a missing user name before token acquisition so the selected tenant and user are both enforced.

            if (account is null || string.IsNullOrWhiteSpace(account.TenantId))
            {
                _logger.LogError("Could not determine your Azure tenant. Run 'az login' and try again.");
                return null;

• Files reviewed: 18/18 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Rick Brighenti (rbrighenti)]

Requesting changes. The ARM URL construction lets the bearer token be sent to an arbitrary host and needs fixing before merge.

The PR also adds network vnet link/unlink/status and ARM enterprise policy resolution, which aren't in the title. Please retitle, or split the VNet part out, since most of the findings are there.

[Copilot]

Copilot review overview

🟡 Changes recommended

Moderate correctness and timeout/cancellation issues remain unresolved, along with changelog nits.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 1

Open (3)

• Preserve caller cancellation in RetryHelper · New
• Reuse account context throughout the wait operation · New
• Condense unreleased entry to one consumer-facing sentence · New

Resolved since last review (5)

• This token request omits the tenant that the command selected for the ARM read, so…
• network.md documents NotStarted as an in-flight status, but this predicate only treats…
• This only checks that gsa enable --wait parses; none of the three new SetHandler blocks is…
• CommandNames.Network was added for centralized command names, but this registration still…
• This Unreleased entry is several sentences and includes implementation details and rationale; it…

[Copilot]

Copilot review overview

🟡 Changes recommended

Reuse one Azure CLI account snapshot for confirmation and authentication to prevent tenant mismatches and transient failures.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

• Reuse account snapshot to prevent applying settings to the wrong tenant · New
• Avoid duplicate account lookup causing failures or tenant mismatch · New

Resolved since last review (3)

• Reuse account context throughout the wait operation
• Preserve caller cancellation in RetryHelper
• Condense unreleased entry to one consumer-facing sentence

[Lala Sushant Srivastava (lasrivas)]

The virtual network change set has been removed from this PR, so it now carries only the GSA work.

This branch was originally cut to include #494, which meant roughly 1400 lines appeared on both PRs and every VNet review finding had to be answered twice. GSA never depended on any of it: GsaService talks to the platform directly, and the only shared code is the tenant-resolution and confirmation helpers in NetworkCommand, which both features need.

Removed in d923c2a: VNetLinkService, IVNetLinkService, VNetModels and their tests; ArmApiService and its tests reverted to main; the vnet subcommand tree along with ReportAsync and LogStatus; the vnet entries in the CHANGELOG and the docs index. NetworkCommand.CreateCommand loses its IVNetLinkService parameter.

Review the VNet code on #494 instead; it is unchanged there.

One consequence worth flagging: both branches now define the network root command and the two shared helpers, so whichever merges second will conflict in NetworkCommand.cs and Program.cs. The conflict is additive and small, and it seemed the better trade against reviewing the same 1400 lines on two PRs.

2063 passed, 0 failed, 12 skipped.

[Copilot]

Copilot review overview

🟡 Changes recommended

Several moderate service reliability and error-handling issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 2 · 1

Open (5)

• Avoid duplicate account lookup causing failures or tenant mismatch
• Reuse account snapshot to prevent applying settings to the wrong tenant
• Validate tenant before caching Azure account · New
• Handle HttpClient timeouts as documented failures · New
• Correct inaccurate Azure CLI account lookup prerequisites · New

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate service-handling issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Shared HTTP handler is disposed after the first request · New

Resolved since last review (5)

• Avoid duplicate account lookup causing failures or tenant mismatch
• Reuse account snapshot to prevent applying settings to the wrong tenant
• Handle HttpClient timeouts as documented failures
• Validate tenant before caching Azure account
• Correct inaccurate Azure CLI account lookup prerequisites

[Copilot]

Copilot review overview

🔵 Needs a closer look

Moderate issues remain in URL error handling and wait-timeout behavior.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Shared HTTP handler is disposed after the first request

[Krishnadheeraj (DheerajPannala)]

Thanks, the account-once threading is a real fix and the GSA flow reads well. I checked the contract against the platform change this calls (MCP-Platform PR 3686, merged) and built and ran the suite on this branch and merged onto current main (builds; all tests pass). Details are inline; the summary:

Before merge

• --wait can report the old value as success (inline on NetworkCommand.cs): the platform's status response never sets pending, so after a timed-out wait the command prints the old value and exits 0 with no "still being applied" hint. The tests covering this path feed pending: true into status responses, which the platform does not send.
• Scopes (please confirm): the platform rejects any token without the exact scope (AgentTools.Gsa.Manage.All for enable/disable, AgentTools.Gsa.Read.All for status) with 403 insufficient_scope. The token is {Agent 365 Tools app}/.default from the Azure PowerShell public client (no client id is passed), so both scopes need to be exposed on the Agent 365 Tools app and pre-authorized for that client, in every cloud the CLI targets.
• Rebase onto main: needed for #478 (cloud-aware endpoints; inline on GsaService.cs). Also, this branch still carries #494's original commit 1318bd50 plus d923c2a, which deletes those files. A squash merge nets that out, but with a merge commit (enabled in this repo) the two branches share 1318bd50 as a common ancestor, so whichever merges second hits modify/delete conflicts on the VNet files, and resolving them toward this branch deletes the VNet feature. Rebasing onto main (ideally dropping that pair) removes the shared ancestor.
• Interaction with #494: with squash merges, whichever lands second conflicts in NetworkCommand.cs, Program.cs, NetworkCommandTests.cs, docs/commands/README.md and CHANGELOG.md (reproduced in both orders). The semantics also diverge under one network parent: vnet has --tenant-id and does not confirm a plain link; gsa has no --tenant-id and confirms both verbs.

Verified and looks right

• Resolving the az account once and passing it down removes the prompt/token tenant split, and the tests pin it.
• HttpClient's own timeout returns the documented null, while a caller's cancellation and the wait ceiling still propagate.
• disposeHandler: false in HttpClientFactory is the right ownership model.
• Routes and shapes match the platform (POST /agents/gsa/enable|disable, GET /agents/gsa/status; status/pending/reason; error), and NotConfigured stays distinct from Disabled.

Nit: Program.cs creates the logger with the literal "network", and CommandNames.Network exists.

[Krishnadheeraj (DheerajPannala)]

Medium-High: after WaitForStatusAsync, result comes from GET /agents/gsa/status, and the platform sets pending only on the 202 from enable/disable, never on status. So when the wait times out, result.Pending is false: this prints e.g. "Global Secure Access: Disabled", skips this hint, and exits 0, right after the admin ran enable --wait. For a security control that reads as "done".

Suggest comparing against the requested value once the wait is over, e.g.:

if (wait && !string.Equals(result.Status, expectedStatus, StringComparison.OrdinalIgnoreCase))
{
    logger.LogWarning(
        "Global Secure Access is not {Expected} yet; the change is still being applied. Check with: a365 network gsa status",
        expectedStatus);
    return 1;
}

The exit code is your call given the "pending is not a failure" principle, but with --wait the caller explicitly asked to block until the value applies.

[Krishnadheeraj (DheerajPannala)]

Medium (test): this stubs WaitForStatusAsync to return Pending = true, but that result is a status read, which never carries pending from the platform. The realistic timed-out result for an enable is { Status = "Disabled", Pending = false }, and with that input ReportGsaAsync currently exits 0 with no "still being applied" message. Suggest switching the stub to the realistic shape and asserting that the timeout is reported (see the comment on NetworkCommand.cs).

[Krishnadheeraj (DheerajPannala)]

Low (test): the response queued just above is a status read with pending: true, which the platform does not send (pending appears only on the 202 from enable/disable). The test still proves the budget logic, but the fixture teaches the wrong contract; pending: false (and asserting Pending is false) keeps it honest.

[Krishnadheeraj (DheerajPannala)]

Medium: one failed poll (null: any 5xx, gateway error or network blip) ends the wait, and ReportGsaAsync exits 1 even though the platform already accepted the change. That is the failure mode the account-once change set out to remove ("a transient failure part-way through a wait does not abort a change that already succeeded"). Suggest tolerating a few consecutive failed reads before giving up, and, when giving up, saying that the change was accepted and how to check on it.

[Krishnadheeraj (DheerajPannala)]

Medium (after rebase): since #478, every Agent 365 Tools call in Agent365ToolingService passes authorityHost (resolved from the configured environment). This call does not, so it always authenticates against the commercial cloud. The docs say "Public cloud only", but nothing enforces it, so a gcc/gcc-high/dod configuration would get a commercial token for a sovereign endpoint. Either thread authorityHost through like the other services or fail fast outside the commercial cloud.

Also on main: BuildBaseUrl below duplicates Agent365ToolingService.BuildAgent365ToolsBaseUrl (#494's VNetLinkService has a third copy), and ConfigConstants.GetAgent365ToolsOrigin(environment) now exists for exactly this.

Related (tests): GetAccessTokenAsync on main has a 9th optional parameter, authorityHost. FakeAuth in GsaServiceTests matches 8, which pins authorityHost == null, the same shape as the CancellationToken pinning fixed earlier. Once this call passes authorityHost, the setup stops matching: the success-path tests fail loudly, but the tests that expect null pass without exercising anything.

[Krishnadheeraj (DheerajPannala)]

Low: two small prompt issues:

• Every other confirmation in the CLI ends with a (y/N): or [y/N]: hint (e.g. CleanupCommand, ClientAppValidator, PermissionsSubcommand). This one ends with Continue? and no trailing space.
• The account is already resolved when this runs, so the prompt could name who is acting as well as the tenant id (e.g. ... for tenant {tenantId} as {account.User.Name}), which is easier for an admin to recognize than a GUID.

[Krishnadheeraj (DheerajPannala)]

Low (UX): on a 202 the platform returns the value it has not displaced yet, so enable without --wait prints "Enabling Global Secure Access..." followed by "Global Secure Access: Disabled". The hint follows, but the first thing the admin reads is the old value. Suggest labelling both, e.g. "Requested: Enabled (still being applied). Current: Disabled."

[Krishnadheeraj (DheerajPannala)]

Low (docs, optional design):

• Tenants that only have Microsoft 365 (no Azure subscription) need az login --allow-no-subscriptions; otherwise az login fails before these commands can read the account. Worth stating here, since a GSA admin may never have used Azure.
• These commands read no Azure resources; az only supplies the tenant and the login hint. With an explicit tenant the token request is pinned to that tenant (and AuthenticationService's tid check enforces it), so a --tenant-id like vnet has would not reintroduce the prompt/token mismatch, and it would remove the Azure CLI dependency for admins who never use Azure.

[Krishnadheeraj (DheerajPannala)]

Nit: on the question in the earlier thread: the one-sentence rule is written down in .github/copilot-instructions.md ("CHANGELOG": "Each entry is one crisp consumer-facing sentence") and in CLAUDE.md, and the section ships verbatim to nuget.org. This entry is four sentences including rationale, and no other entry uses a relative link (it will not resolve outside GitHub). Suggestion: "a365 network gsa enable|disable|status turns Global Secure Access on or off for your Agent 365 environment (#497)." Agreed that the older multi-sentence entries deserve a separate cleanup.

[Krishnadheeraj (DheerajPannala)]

Low: #494 adds its own | [network](network.md) | Configure tenant networking for Agent 365. | row, so after both merge the index has two identical network rows pointing at different pages. Consider a single network row (or a small landing page) plus the per-subcommand rows.

[Copilot]

Copilot review overview

🟡 Changes recommended

Empty successful responses currently produce an Unknown status with exit code 0 instead of reporting a failed status read.

Review effort: Balanced
Findings: 1

Open (1)

• Treat empty 2xx GSA status responses as failed reads · New