Fix AD provider change detection for group membership operations

[Copilot]

Fix PruneEntitlements unstable behavior with AD provider

Problem Analysis

The PruneEntitlements step with AD provider always returns Changed: False even when the user has groups that should be removed.

Root Cause: The RemoveGroupMember and AddGroupMember methods in the AD adapter returned $true whenever the AD cmdlets succeeded, relying on them to throw errors for no-op scenarios. However, in newer versions of the ActiveDirectory PowerShell module (or certain AD configurations), Remove-ADGroupMember and Add-ADGroupMember are idempotent and don't error when removing a non-member or adding an existing member. This caused incorrect Changed status reporting.

Solution Implemented

Modified membership operations in /src/IdLE.Provider.AD/Private/New-IdleADAdapter.ps1:

1. Extracted shared helper method TestGroupMembership:

   • Returns $true if user is a member, $false if not
   • Returns $null if check fails (signals fail-forward to authoritative operation)
   • Includes Write-Verbose logging (fixes PSScriptAnalyzer empty catch block warning)
   • Uses Select-Object -First 1 to short-circuit for performance on large groups

2. Refactored AddGroupMember and RemoveGroupMember:

   • Use shared TestGroupMembership helper
   • Eliminated code duplication and drift risk
   • Return accurate change status based on actual state changes

3. Added comprehensive unit tests (New-IdleADAdapter.MembershipChecks.Tests.ps1):

   • TestGroupMembership method behavior
   • AddGroupMember change detection (no-op vs actual change)
   • RemoveGroupMember change detection (no-op vs actual change)
   • Fail-forward behavior when membership check fails
   • Idempotency across multiple calls
   • Performance test with large groups

Changes Made

• Added TestGroupMembership helper method (lines 670-703)
• Refactored AddGroupMember method (lines 705-736)
• Refactored RemoveGroupMember method (lines 738-770)
• Created comprehensive unit test file (12 tests, all passing)
• No changes to public APIs or provider contracts
• Maintains backward compatibility

Testing & Validation

• All 112 AD provider tests pass
• All 33 PruneEntitlements step tests pass
• All 12 new membership check unit tests pass
• Code review completed with no issues
• Addresses all PR feedback:
  • ✅ Empty catch blocks fixed with Write-Verbose
  • ✅ Performance optimization with Select-Object -First 1
  • ✅ Code duplication eliminated via shared helper
  • ✅ Unit tests added for change detection behavior

Impact

This fix ensures that:

• PruneEntitlements correctly reports Changed: True when groups are actually removed
• PruneEntitlements correctly reports Changed: False when no changes are needed
• Works reliably across different ActiveDirectory module versions
• Provides accurate change tracking for audit and compliance purposes
• Code is more maintainable with eliminated duplication

Original prompt

---

This section details on the original issue you should resolve

<issue_title>PruneEntitlements unstable with AD</issue_title>
<issue_description>## Description

Although during branch testing all situations worked out after PR merge to main this isn't the case anymore for AD provider. A given prune step that should just remove all groups is always in Changed: False

Steps to Reproduce

0. Having a user with multiple groups
1. Step with PruneEntitlements

@{
            Name = 'AD - Prune Groups'
            Type = 'IdLE.Step.PruneEntitlements'
            With = @{
                IdentityKey = '{{Request.IdentityKeys.sAMAccountName}}'
                Provider = 'AD'
                AuthSessionName = 'AD'
                Kind = 'Group'
            }
        }

2. Plan + Invoke

Expected Behavior

In the case of the given test user: remove all groups except primary group (AD).

Actual Behavior

Nothing is done.

Environment

• PowerShell version: 7.5.4
• OS: Windows Server 2019
• IdLE version / commit: main
• Execution context (CLI / Service / CI): CLI, AD Provider

Additional Context

• the user definetly has groups to remove
• it doesnt make a different if a keep entry is added</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes PruneEntitlements unstable with AD #255

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[Copilot]

Pull request overview

This PR fixes change detection in the Active Directory provider’s group membership operations so steps like PruneEntitlements correctly report Changed: True when group membership is actually modified, even on newer idempotent ActiveDirectory module versions.

Changes:

• Added a pre-check in AddGroupMember and RemoveGroupMember to detect current membership state before calling Add-ADGroupMember / Remove-ADGroupMember.
• Updated return semantics to $true only when membership changes and $false for no-op calls.
• Implemented a “fail-forward” pattern: if the membership check fails, proceed to the cmdlet call for an authoritative error.

[github-actions]

Code Coverage Report

Overall Project	73.57%	🍏

There is no coverage information present for the Files changed

[ntt-matthias-fleschuetz]

@copilot
I observed the following:

PS C:\Users\adm-mafl-server\Downloads\idle> $plan = New-IdlePlan -WorkflowPath .\leaver.psd1 -Request $request -Providers $providers
PS C:\Users\adm-mafl-server\Downloads\idle> $result = Invoke-IdlePlan -Plan $plan 
PS C:\Users\adm-mafl-server\Downloads\idle> $result.Status
Blocked 
PS C:\Users\adm-mafl-server\Downloads\idle> $result = Invoke-IdlePlan -Plan $plan 
PS C:\Users\adm-mafl-server\Downloads\idle> $result.Status
Blocked 
PS C:\Users\adm-mafl-server\Downloads\idle> $plan = New-IdlePlan -WorkflowPath .\leaver.psd1 -Request $request -Providers $providers 
PS C:\Users\adm-mafl-server\Downloads\idle> $result = Invoke-IdlePlan -Plan $plan 
PS C:\Users\adm-mafl-server\Downloads\idle> $result.Status
Completed 

The run was as following:

1. first Invoke with $result.Status = Blocked was correct, as user was still in BYOD group.
2. before second invoke BYOD group was definetly renived at user -> direct invoke without new plan still had $result.Status = Blocked.
3. Having a new plan and then invoke had status $result.Status = Completed.

My suspicion is that data somewhat gets cached in between? Does that make sense? Is there anything internally that we can improve or is it just relying on more waiting time between changes. But having a new plan and invoke after direct change somehow implies it is maybe more on IdLE side then on server side?

[Copilot]

@copilot
I observed the following:

...

This is expected behavior, not a bug. The plan captures identity/entitlement data as a snapshot during New-IdlePlan (via ContextResolvers). This snapshot is stored in $plan.Request.Context and used for the entire plan execution to ensure deterministic, repeatable runs.

What's happening:

1. First New-IdlePlan: Captures entitlements including BYOD group → Invoke-IdlePlan correctly shows Blocked
2. You remove BYOD manually in AD
3. Second Invoke-IdlePlan (same plan): Still uses the original snapshot with BYOD → Blocked
4. Second New-IdlePlan: Captures fresh data without BYOD → Invoke-IdlePlan shows Completed

Why this design:

• Plans are immutable, auditable artifacts
• Re-running the same plan should produce consistent results
• Prevents race conditions during multi-step operations

Recommendation:
Always create a fresh plan (New-IdlePlan) before execution if you need to react to external changes. The plan creation phase is when all identity/entitlement queries happen. You can inspect the captured data with $plan.Request.Context | ConvertTo-Json -Depth 5.

No code changes needed - this is working as designed per the planning/execution separation documented in docs/about/concepts.md.