Registered server commands - Leave the connection of the caller alone

[andreasjordan]

Type of Change

• Bug fix (non-breaking change, fixes Registered server commands disconnect the connection of the caller, including the Get- commands #10572)
• Ran manual Pester test and has passed
• Pester test is included

Purpose

Seven sites in the registered server commands closed a connection they did not open. Two of them are Get- commands, so merely reading registered servers ended the session of the connection that was handed in - temp tables, SET options, session context - and the connection was silently reopened afterwards, so nothing looked wrong.

Measured on SQL Server 2019 with a temp table as the marker and Get-DbaDatabase as a control:

Get-DbaRegServerGroup                      session survived: False
Get-DbaRegServer                           session survived: False
Add-DbaRegServerGroup                      session survived: False
Move-DbaRegServerGroup                     session survived: False
Move-DbaRegServer                          session survived: False
Remove-DbaRegServer                        session survived: False
Remove-DbaRegServerGroup                   session survived: False
Get-DbaRegServerStore (control)            session survived: True
Get-DbaDatabase (control)                  session survived: True

This is the family that the inventory in #10554 missed, because the grep behind it matched Disconnect-DbaInstance and ConnectionContext.Disconnect() but not the generic .Disconnect() these use.

Approach

One fix, not seven. Removing the disconnect in Add-DbaRegServerGroup alone would change nothing, because it calls Get-DbaRegServerGroup, which disconnects as well - anything short of a decision covering the whole family leaves the symptom in place.

Get-DbaRegServerStore is where the connection is obtained, so it is where the answer is known. It asks Connect-DbaInstance through IsNewConnectionReference and records the result as IsNewConnection on the store.

A note property is safe here, unlike on a server object: the store is built fresh on every call and is never handed back to Connect-DbaInstance, so nothing can re-stamp it. That was the objection to a note property in #10554, and it does not apply to this object.

Disconnect-Regserver becomes the single place that acts on it. It already walked up the parents to find the connection; it now also accepts the store itself, and closes the connection only when the store says we opened it:

$i = 0
while ($null -ne $Server -and $null -eq $Server.ServerConnection -and $i++ -le 20) {
    $Server = $Server.Parent
}

if ($Server.ServerConnection -and $Server.IsNewConnection) {
    $Server.ServerConnection.Disconnect()
}

The six sites that closed the connection by hand now go through it too, so there is one place left in the family that can close a connection.

Commands to test

$server = Connect-DbaInstance -SqlInstance $instance -NonPooledConnection
$null = $server.ConnectionContext.ExecuteNonQuery("CREATE TABLE #marker (id INT)")

$null = Get-DbaRegServerGroup -SqlInstance $server -Id 1

$server.ConnectionContext.ExecuteScalar("SELECT COUNT(*) FROM #marker")   # 0, was: Invalid object name '#marker'

Tests

One regression context per fixed command, all on InstanceSingle, in the shape used for #10554: connect with -NonPooledConnection, create a temp table, call the command with that server object, assert the command still did its work and the temp table is still there. A pooled connection passes either way, so the tests have to be non-pooled.

Get-DbaRegServerStore.Tests.ps1 covers the mechanism itself: IsNewConnection is $true for an instance name and $false for a connection of the caller.

For the commands that take an -InputObject, the marker is created after the lookup on purpose. Get-DbaRegServer and Get-DbaRegServerGroup close the connection too, so a marker created before the lookup would measure them instead of the command under test.

53 tests across the eight files, all passing. Against development every one of the eight files fails on exactly its new assertion.

🤖 Generated with Claude Code