Repository navigation
Avoid redundant Az module root on GitHub-hosted runners - #648
Draft
Nathan McNulty (nathanmcnulty) wants to merge 1 commit into
Draft
Nathan McNulty (nathanmcnulty) wants to merge 1 commit into
Nathan McNulty (nathanmcnulty) wants to merge 1 commit into
Conversation
Copilot started reviewing on behalf of
Nathan McNulty (nathanmcnulty)
October 11, 2026 05:50
View session
There was a problem hiding this comment.
🟡 Changes recommended
Raw path comparisons miss equivalent trailing-separator and Windows case variants, retaining the targeted slowdown.
1 open finding
What changed in this PR
Optimizes PowerShell module discovery on GitHub-hosted runners by avoiding redundant broad module roots.
Changes:
- Detects reachable
Az.Accountsmodule paths before prepending. - Preserves existing behavior for non-hosted runners.
- Adds hosted, self-hosted, and path-boundary tests.
| File | Description |
|---|---|
src/PowerShell/AzPSUtils.ts |
Adds the hosted-runner path guard. |
__tests__/PowerShell/AzPSUtils.test.ts |
Tests path handling and runner environments. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+45
to
+50
| const entries = (process.env.PSModulePath || '').split(path.delimiter); | ||
| if (isGitHubHosted && entries.some(entry => entry === psModulePath || | ||
| (path.dirname(entry) === psModulePath && fs.existsSync(path.join(entry, AzPSConstants.AzAccounts))))) { | ||
| core.debug(`Skip setting the default PowerShell module path, ${psModulePath} is already reachable.`); | ||
| return; | ||
| } |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With
enable-AzPSSessionenabled, Azure/login prepends the broad default module root even when GitHub-hosted images already expose the installed Az directories directly. Skip that prepend on GitHub-hosted runners when the root or an existing direct child containingAz.Accountsis already onPSModulePath. Preserve the existing prepend and root precedence for self-hosted, unknown, and unspecified runner environments. PowerShell module discovery and manifest-version selection remain unchanged.Adds tests for private-runner precedence and direct-child boundaries. Validation of this exact source-only candidate:
lib/output is omitted under the repository's source-only policy.Az.Accounts5.5.3, including inherited paths from Windows PowerShell 5.1. No fresh live Azure authentication was performed.tsc --noEmit --skipLibCheckpasses. Plaintsc --noEmitreports identical existing TypeScript 4.9 /@types/node24 declaration errors on baseline and candidate; this PR does not change dependencies.The September reproduction measured first
pwshstartup at 4.48 s mean with image defaults versus 25.40 s with/usr/shareprepended, across ten fresh Ubuntu jobs per arm. These historical measurements motivate the guard; they are not fresh timing results for this revised commit.Fixes #646.