Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines dependency/module loading to prevent macOS session DLL/ALC conflicts between Microsoft Graph and Az modules, and improves cross-platform behavior/messaging during connection setup.
Changes:
- Adjusts Connect-ZtAssessment module ordering logic and adds a clearer warning when Graph auth fails due to MSAL (Microsoft.Identity.Client) DLL conflicts.
- Skips SharePoint Online connection on non-Windows and removes a nonstandard console glyph from messaging.
- Updates Initialize-Dependencies to use a cross-platform path for Get-ModuleImportOrder and adds logic to pre-load only the shared low-level DLL (Microsoft.IdentityModel.Abstractions.dll).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/powershell/public/Connect-ZtAssessment.ps1 | Updates connection/import ordering, improves MSAL-conflict warnings, and gates SharePoint Online connect on Windows. |
| src/powershell/Initialize-Dependencies.ps1 | Fixes macOS helper script path and adds shared DLL pre-load logic focused on IdentityModel.Abstractions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Collect all module base directories from the import order candidates | ||
| $moduleBaseDirs = $msalToLoadInOrder | ForEach-Object { Split-Path -Path $_.DLLPath -Parent } | ||
| # Also include all candidate modules' full ModuleBase for broader search | ||
| $allModuleCandidates = $msalToLoadInOrder | ForEach-Object { | ||
| $candidateModule = Get-Module -Name $_.Name -ListAvailable | Select-Object -First 1 | ||
| if ($candidateModule) { $candidateModule.ModuleBase } | ||
| } | ||
| $searchDirs = @($moduleBaseDirs) + @($allModuleCandidates) | Where-Object { $_ } | Select-Object -Unique | ||
|
|
There was a problem hiding this comment.
$msalToLoadInOrder objects already include the selected module's ModuleBase. Re-querying with Get-Module -Name $_.Name -ListAvailable | Select-Object -First 1 can pick a different version/path than the import candidate, which can make the shared-DLL scan look in the wrong place (and potentially preload the wrong copy). Prefer using the existing $_.ModuleBase (or otherwise tie the search to the import-candidate module path) and avoid the extra Get-Module call.
| # Collect all module base directories from the import order candidates | |
| $moduleBaseDirs = $msalToLoadInOrder | ForEach-Object { Split-Path -Path $_.DLLPath -Parent } | |
| # Also include all candidate modules' full ModuleBase for broader search | |
| $allModuleCandidates = $msalToLoadInOrder | ForEach-Object { | |
| $candidateModule = Get-Module -Name $_.Name -ListAvailable | Select-Object -First 1 | |
| if ($candidateModule) { $candidateModule.ModuleBase } | |
| } | |
| $searchDirs = @($moduleBaseDirs) + @($allModuleCandidates) | Where-Object { $_ } | Select-Object -Unique | |
| # Collect all module base directories from the import order candidates (based on the selected DLL paths) | |
| $moduleBaseDirs = $msalToLoadInOrder | ForEach-Object { Split-Path -Path $_.DLLPath -Parent } | |
| # Also include each candidate module's ModuleBase directly from the import candidates | |
| $moduleBases = $msalToLoadInOrder | ForEach-Object { $_.ModuleBase } | Where-Object { $_ } | |
| # Build the final set of search directories tied to the actual import-candidate modules | |
| $searchDirs = @($moduleBaseDirs) + @($moduleBases) | Where-Object { $_ } | Select-Object -Unique |
| $allCopies = foreach ($dir in $searchDirs) { | ||
| $searchRoot = $dir | ||
| # Walk up to the module version root to search all subdirectories (e.g., Dependencies/, netCore/, lib/) | ||
| $parentDir = Split-Path -Path $dir -Parent | ||
| if ($parentDir -and (Test-Path $parentDir)) { $searchRoot = $parentDir } | ||
|
|
||
| Get-ChildItem -Path $searchRoot -Filter $dllName -File -Recurse -Force -ErrorAction SilentlyContinue | ||
| } |
There was a problem hiding this comment.
The shared-DLL discovery does a recursive Get-ChildItem -Recurse for each entry in $searchDirs, and each iteration can expand the search root to the parent directory. This can traverse the same directory trees multiple times during module import (ScriptsToProcess), which can noticeably slow startup on large module folders. Consider precomputing a unique set of search roots (after the parent adjustment) and/or restricting recursion to the import-candidate ModuleBase paths to avoid redundant scans.
| $connectionOrder = @($OrderedImport.Name) | ||
| foreach ($name in $moduleNamesForImportOrder) { | ||
| if ($name -notin $connectionOrder) { | ||
| $connectionOrder += $name | ||
| } |
There was a problem hiding this comment.
If Get-ModuleImportOrder returns no results (e.g., dependencies not installed yet or detection fails), $OrderedImport.Name is $null and @($OrderedImport.Name) produces an array with a null element. That leads to a leading empty entry in the verbose "Import Order" output and an extra no-op iteration in the switch. Filter out null/empty names when building $connectionOrder (e.g., only keep truthy names).
gaelcolas
left a comment
gaelcolas
left a comment
There was a problem hiding this comment.
Setting as request changes, because I haven't finished looking into the other parts.
I'd rather not merge this as-is.
| } | ||
| elseif (-not $SkipModuleInstallation.IsPresent) | ||
|
|
||
| if (-not $SkipModuleInstallation.IsPresent) |
There was a problem hiding this comment.
To me, that's the only change valid.
Everything else does not seem necessary.
I'm still trying to work it out...
|
@gaelcolas feel free to create a new pr and close this one off. The module loading did trip me. I think you can test on a linux vm/container for the cross platform testing. |
Root cause
Our previous fix pre-loaded three shared DLLs into the default AssemblyLoadContext:
Az.Accounts uses its own Assembly Load Context (Microsoft.Azure.PowerShell.AuthenticationAssemblyLoadContext.dll registers AzAssemblyLoadContextInitializer.RegisterAzSharedAssemblyLoadContext()). This ALC isolates Az's DLLs from the default context. When we pre-loaded Azure.Identity.dll (Graph's v1.13.2.0) and Azure.Core.dll (Az's v1.50.0.0) into the default context, it interfered with Az's ALC type resolution, causing the "Entry point was not found" error during Connect-AzAccount.
Fix
Removed Azure.Identity.dll and Azure.Core.dll from the pre-load list. Now we only pre-load Microsoft.IdentityModel.Abstractions.dll — the specific low-level type-definition DLL that caused the original Graph MissingMethodException. This DLL defines the IIdentityLogger interface that all modules share in the default context, so loading the newest version (8.6.1.0 from Graph) is both necessary and safe.
Both Graph and Azure connections should now work.