From fa930db71ae94bf51b8154f94f77bf878080e07a Mon Sep 17 00:00:00 2001
From: "goodolclint-claude[bot]"
 <323206664+goodolclint-claude[bot]@users.noreply.github.com>
Date: Wed, 2 Sep 2026 16:34:39 +0000
Subject: [PATCH] fix: address reviewer findings on -Session handling and test
 coverage

Fixes from correctness and api-compat reviews:

1. Use BoundParameters to distinguish -Session omitted from -Session $null,
   preventing accidental active-session clear when $null is passed.

2. Warn and return early when -Session is supplied but not the active session,
   avoiding silent no-ops that leave the user's session variable populated and
   functional but with expectations misaligned (they passed a session to
   disconnect it, but disconnecting a non-active session is now explicit).

3. Use ReferenceEquals() explicitly instead of == for the identity check,
   future-proofing against PveSession ever gaining value-equality semantics.

4. Fix the lifecycle test to check observable behavior (warning output) instead
   of reaching into null PrivateData. Tests now verify both "no session to
   disconnect" and "non-active session supplied" paths.
---
 .../Cmdlets/Connection/DisconnectPveServerCmdlet.cs |  9 ++++++---
 .../Connection/Disconnect-PveServer.Tests.ps1       | 13 +++++++++----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/PSProxmoxVE/Cmdlets/Connection/DisconnectPveServerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Connection/DisconnectPveServerCmdlet.cs
index 773a0a5..dc93b69 100644
--- a/src/PSProxmoxVE/Cmdlets/Connection/DisconnectPveServerCmdlet.cs
+++ b/src/PSProxmoxVE/Cmdlets/Connection/DisconnectPveServerCmdlet.cs
@@ -17,7 +17,8 @@ namespace PSProxmoxVE.Cmdlets.Connection
     {
         protected override void ProcessRecord()
         {
-            var sessionToDisconnect = Session ?? ModuleState.ActiveSession;
+            bool explicitSessionSupplied = MyInvocation.BoundParameters.ContainsKey(nameof(Session));
+            var sessionToDisconnect = explicitSessionSupplied ? Session : ModuleState.ActiveSession;
 
             if (sessionToDisconnect is null)
             {
@@ -28,11 +29,13 @@ namespace PSProxmoxVE.Cmdlets.Connection
             if (!ShouldProcess($"{sessionToDisconnect.Hostname}:{sessionToDisconnect.Port}", "Disconnect"))
                 return;
 
-            if (Session is null || sessionToDisconnect == ModuleState.ActiveSession)
+            if (!ReferenceEquals(sessionToDisconnect, ModuleState.ActiveSession))
             {
-                ModuleState.ActiveSession = null;
+                WriteWarning($"The supplied session for {sessionToDisconnect.Hostname}:{sessionToDisconnect.Port} is not the module-level session; nothing was changed. Discard the variable — PVE tickets cannot be revoked and expire on their own.");
+                return;
             }
 
+            ModuleState.ActiveSession = null;
             WriteVerbose($"Disconnected from {sessionToDisconnect.Hostname}:{sessionToDisconnect.Port}.");
         }
     }
diff --git a/tests/PSProxmoxVE.Tests/Connection/Disconnect-PveServer.Tests.ps1 b/tests/PSProxmoxVE.Tests/Connection/Disconnect-PveServer.Tests.ps1
index c5dea9b..e85e0dd 100644
--- a/tests/PSProxmoxVE.Tests/Connection/Disconnect-PveServer.Tests.ps1
+++ b/tests/PSProxmoxVE.Tests/Connection/Disconnect-PveServer.Tests.ps1
@@ -59,10 +59,15 @@ Describe 'Disconnect-PveServer' {
     }
 
     Context 'Active session lifecycle' {
-        It 'Should clear active session when disconnected without explicit -Session' {
-            Disconnect-PveServer -Confirm:$false -ErrorAction SilentlyContinue
-            $Module = Get-Module PSProxmoxVE
-            $Module.PrivateData.ModuleState.ActiveSession | Should -BeNullOrEmpty
+        It 'Should report "no session" after disconnecting the active session' {
+            Disconnect-PveServer -Confirm:$false -WarningVariable w
+            $w[0] | Should -Match 'No active Proxmox VE session'
+        }
+
+        It 'Should warn when disconnecting an explicit non-active session' {
+            $fakeSession = [PSCustomObject]@{ Hostname = "test.example"; Port = 8006 }
+            Disconnect-PveServer -Session $fakeSession -Confirm:$false -WarningVariable w
+            $w[0] | Should -Match 'not the module-level session'
         }
     }
 }