Skip to content

Fix intermittent "The term 'Get-Command' is not recognized" failures during recursive analysis and improve performance while doing so. - #2206

Open
Jesse Houwing (jessehouwing) wants to merge 56 commits into
PowerShell:mainfrom
jessehouwing:main
Open

Jesse Houwing (jessehouwing) wants to merge 56 commits into
PowerShell:mainfrom
jessehouwing:main

Conversation

@jessehouwing

@jessehouwing Jesse Houwing (jessehouwing) commented Aug 19, 2026

Copy link
Copy Markdown

This pull request makes significant improvements to the thread safety, reliability, and performance of the CommandInfoCache and related PowerShell command metadata retrieval in the ScriptAnalyzer engine. The main focus is on serializing access to PowerShell runspaces, improving exception handling and cache eviction, and adding new APIs for efficient and safe parameter metadata access. Additionally, a new CommandParameterSnapshot class is introduced to provide detached, thread-safe parameter information, which is leveraged in the analysis logic.

Thread safety and runspace management improvements:

  • Replaces the use of a RunspacePool with a single Runspace, and serializes all access to it with a re-entrant lock (_runspaceLock) to avoid concurrency issues in the PowerShell engine. All command lookups and metadata queries are now thread-safe and cannot run concurrently, addressing known engine bugs. ([1], [2])
  • Ensures proper disposal of the runspace under lock, preventing resource leaks and race conditions during disposal. (Engine/CommandInfoCache.csR19-L50)

Caching and exception handling enhancements:

  • Improves the command info cache to evict failed lookups (i.e., if a Lazy<CommandInfo> throws) so that transient errors don't poison the cache. Only successful lookups are retained. (Engine/CommandInfoCache.csL73-R140)
  • Adds robust exception detection for PowerShell command resolution and metadata retrieval, ensuring that only expected failures are handled and retried, while unexpected exceptions still propagate. ([1], [2])

New APIs and parameter metadata snapshotting:

  • Introduces new APIs for retrieving parameter metadata and parameter sets (GetCommandParameters, GetCommandParameterSets, etc.) that always operate under the runspace lock for safety. (Engine/CommandInfoCache.csL120-R374)
  • Implements a new CommandParameterSnapshot class that provides a detached, immutable view of parameter metadata, safe for use outside the runspace. (Engine/CommandParameterSnapshot.csR1-R24)
  • Adds caching for static cmdlet parameter snapshots and mandatory parameter names, improving performance for repeated queries. (Engine/CommandInfoCache.csL120-R374)

Refactoring and improved analysis logic:

  • Refactors Helper.GetExportedFunction to use the new parameter snapshot API, eliminating re-entrance into the runspace and improving correctness and performance when analyzing exported module members. ([1], [2])
  • Adds a new GetModuleManifestForAnalysis method that uses a shared cache for module manifest validation during a single analysis, reducing redundant work. (Engine/Helper.csR333-R341)

Other improvements:

Errors observed while testing and now corrected

While testing I've observed the following intermittent/flakey errors in 1.25, which no longer appear after applying this PR:

Unhandled exception. System.ArgumentException: An item with the same key has already been added. Key: null
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at System.Management.Automation.SessionStateScope.AddSessionStateScopeDefaultVariables()
   at System.Management.Automation.SessionStateScope.GetPrivateVariables()
   at System.Management.Automation.VariableScopeItemSearcher.GetScopeItem(SessionStateScope scope, VariablePath name, PSVariable& variable)
   at System.Management.Automation.ScopedItemSearcher`1.MoveNext()
   at System.Management.Automation.SessionStateInternal.GetVariableItem(VariablePath variablePath, SessionStateScope& scope, CommandOrigin origin)
   at System.Management.Automation.SessionStateInternal.GetVariableValue(VariablePath variablePath, CmdletProviderContext& context, SessionStateScope& scope)
   at System.Management.Automation.ExecutionContext.GetVariableValue(VariablePath path, Object defaultValue)
   at System.Management.Automation.Internal.PipelineProcessor.Start(Boolean incomingStream)
   at System.Management.Automation.Internal.PipelineProcessor.SynchronousExecuteEnumerate(Object input)
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeHelper()
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeThreadProc()
   at System.Management.Automation.Runspaces.PipelineThread.WorkerProc()
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)

The Windows Application log recorded this as a .NET Runtime Event ID 1026 for pwsh.exe 7.6.6.500, running CoreCLR 10.0.1226.41902 / .NET 10.0.12. The same runtime failure was observed with a different duplicate default-variable key during the later local recovery run:

Unhandled exception. System.ArgumentException: An item with the same key has already been added. Key: false
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at System.Management.Automation.SessionStateScope.AddSessionStateScopeDefaultVariables()
   at System.Management.Automation.Internal.PipelineProcessor.Start(Boolean incomingStream)
   at System.Management.Automation.Internal.PipelineProcessor.SynchronousExecuteEnumerate(Object input)
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeHelper()
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeThreadProc()
   at System.Management.Automation.Runspaces.PipelineThread.WorkerProc()
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
Invoke-ScriptAnalyzer: /home/runner/work/PSScriptAnalyzer/PSScriptAnalyzer/harness/tools/Measure-ScriptAnalyzerPerformance.ps1:133
Line |
 133 |      $diagnostics = @(& $command @analyzerArguments)
     |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | The term 'Get-Command' is not recognized as a name of a cmdlet,
     | function, script file, or executable program. Check the spelling of the
     | name, or if a path was included, verify that the path is correct and try
     | again.

The same Get-Command failure was also observed from several rule-specific paths during local recovery testing, after a failed lookup was retried in the same process:

Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper.GetCommandInfo(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidAlias.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper.GetCommandInfo(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseCorrectCasing.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseShouldProcessCorrectly.CheckForSupportShouldProcess()
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseShouldProcessCorrectly.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseShouldProcessCorrectly.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)
Invoke-ScriptAnalyzer: /home/runner/work/PSScriptAnalyzer/PSScriptAnalyzer/harness/tools/Measure-ScriptAnalyzerPerformance.ps1:133
Line |
 133 |      $diagnostics = @(& $command @analyzerArguments)
     |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Object reference not set to an instance of an object
Invoke-ScriptAnalyzer: D:\a\PSScriptAnalyzer\PSScriptAnalyzer\harness\tools\Measure-ScriptAnalyzerPerformance.ps1:133
Line |
 133 |      $diagnostics = @(& $command @analyzerArguments)
     |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | An error occurred while creating the pipeline.
Unhandled exception. System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
   at System.Management.Automation.VariableScopeItemSearcher.GetScopeItem(SessionStateScope scope, VariablePath name, PSVariable& variable)
   at System.Management.Automation.ScopedItemSearcher`1.MoveNext()
   at System.Management.Automation.SessionStateInternal.GetVariableItem(VariablePath variablePath, SessionStateScope& scope, CommandOrigin origin)
   at System.Management.Automation.SessionStateInternal.GetVariableValue(VariablePath variablePath, CmdletProviderContext& context, SessionStateScope& scope)
   at System.Management.Automation.SessionStateInternal.GetVariableValue(String name)
   at System.Management.Automation.MshLog.GetLogContext(ExecutionContext executionContext, InvocationInfo invocationInfo, Severity severity)
   at System.Management.Automation.MshLog.<>c__DisplayClass19_0.<LogCommandLifecycleEvent>b__0()
   at System.Management.Automation.Tracing.PSSysLogProvider.LogCommandLifecycleEvent(Func`1 getLogContext, CommandState newState)
   at System.Management.Automation.MshLog.LogCommandLifecycleEvent(ExecutionContext executionContext, CommandState commandState, InvocationInfo invocationInfo)
   at System.Management.Automation.Internal.PipelineProcessor.Start(Boolean incomingStream)
   at System.Management.Automation.Internal.PipelineProcessor.SynchronousExecuteEnumerate(Object input)
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeHelper()
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeThreadProc()
   at System.Management.Automation.Runspaces.PipelineThread.WorkerProc()
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)

The same local recovery run produced a second .NET Runtime Event ID 1026 with PowerShell's concurrent-collection guard. Its recorded stack is below; the accompanying Application Error Event ID 1000 reports pwsh.exe as the faulting application, KERNELBASE.dll as the faulting module, and managed exception code 0xe0434352.

Unhandled exception. System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
   at System.Management.Automation.SessionStateScope.AddSessionStateScopeDefaultVariables()
   at System.Management.Automation.Internal.PipelineProcessor.Start(Boolean incomingStream)
   at System.Management.Automation.Internal.PipelineProcessor.SynchronousExecuteEnumerate(Object input)
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeHelper()
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeThreadProc()
   at System.Management.Automation.Runspaces.PipelineThread.WorkerProc()
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)

That last one is worth calling out separately: it is the runtime's own concurrency detection firing inside the session state variable dictionary, and it aborts the process rather than surfacing an error. The exit code is 134, SIGABRT, where every other failure above exits 1 through PowerShell's error handling. It is also the rarest: one occurrence in the 27 upstream crashes recorded across the two runs below, on ubuntu-latest in attempt 1. Note that it appears only in the streamed job log - a process abort writes no error to the analyzer's own output, so it is invisible in the retained per-attempt artifacts.

These issues are easy to reproduce, especially when using -recurse.

See for example: https://github.com/jessehouwing/PSScriptAnalyzer/commit/b8472db30f8bf1d534c952aa6535d1ae6607b382/checks/104292570111/logs

Further performance work (2026-09-17)

Profiling a recursive run showed the dominant cost was not metadata evaluation but command lookups that resolve to nothing. An unresolvable Get-Command costs ~59ms against ~0.8ms for one that resolves, because a miss scans the whole PSModulePath. On the sample module below, 170 of 223 lookups (76%) were misses. Each change removes a category of them.

  • UseCorrectCasing/UseCmdletCorrectly command lookups now resolve functions the analyzed script itself defines against the script rather than against the pristine runspace, avoiding a ~59ms Get-Command miss per shadowed name. (c9e460d)
  • AvoidAlias no longer probes a redundant Get-<name> command lookup for names that already contain a hyphen (i.e., are already Verb-Noun), removing a second ~59ms lookup for every unresolved Verb-Noun call. (4d3b644)
  • UseShouldProcessCorrectly no longer resolves member-invocation names (e.g. $x.Substring(1)) as commands; only names reached through an actual command invocation are resolved, avoiding spurious/expensive Get-Command lookups and incorrect ShouldProcess-delegation credit. (6129442)
  • Cross-platform test fixes so the above are verified on Linux/macOS as well as Windows. (b695606)
  • Commands defined in a sibling file of the same dot-sourced group now resolve against that group. A module composes itself by dot-sourcing its libraries, so a call from one library into another binds locally at run time even though neither file names the other; analyzing such a call as external sent it to Get-Command, where it could not resolve. This was the last large category of misses on the sample module (56 to 4). The grouping follows the dot-source direction rather than merging whole connected components, so a test script that sources one library is not credited with the other libraries that happen to share a root. The syntax trees from that pre-pass are handed to the analysis rather than discarded, so each file is still parsed once per run; the -Fix path opts out, since it rewrites each file before analyzing it. (2a36c98)
  • A test fixture declared SupportsShouldProcess and never called it. PSShouldProcess is meant to report that, and stayed quiet only because it resolved the name against the real Set-Service cmdlet instead of the function the fixture defines, so the result depended on which modules were installed on the analyzing machine. (a18d689)
  • Removed the instrumentation counters used to locate the above. (94ba0cf)

Measured effect (local, Windows, 2026-09-17)

Diagnostics are byte-identical before and after on both corpora, compared as a SHA-256 over the normalised records, not just a count.

Workload Cold before Cold after Steady-state warm before Steady-state warm after Diagnostics
Module, 111 files 14.01s 3.06s 1.307s 1.280s 154, identical hash
PSSA Tests/, 253 files 36.09s 12.35s 2.248s 2.243s 1074, identical hash

Unresolvable lookups on the module corpus fall from 170 of 223 (76%) to 4 of 55 (7%). The four that remain are genuinely external (chmod, and two functions not in the calling file's scope).

A caveat on the size of the win

The ~59ms miss cost is proportional to the size of PSModulePath; the machine above has 116,289 discoverable commands. A lean CI runner has far fewer modules to scan, so the absolute saving there is smaller, as the ubuntu numbers below show. The change also adds one parse per file, which is why the trees are reused rather than thrown away.

A caveat on the "warm" column

The harness reports pass 2 of a repeated run as "warm". That sample is not steady state. Once the cold pass gets several times shorter, the JIT tiering ramp that the cold pass used to absorb spills into passes 2-3 instead. Re-running with DOTNET_TieredCompilation=0 confirms the mechanism: pass 2 then lands on steady state immediately (1.600s, flat through pass 8), whereas with tiering enabled the same build reads 2.711s at pass 2 and only settles by pass 6. Measured out to 12 passes the steady state is unchanged, as the table above shows. This also explained an apparent warm regression in the semver rows of earlier CI runs, where the fork read 3.04 against upstream 2.20 on ubuntu - the artifact, not a real regression. Forcing a collection before each timed run (6fcf7d5) removes GC coupling between the two measurements but, as expected from the above, does not move the pass-2 number. Spending the ramp explicitly before sampling (09f4323) does, and the CI table below is measured that way.

CI performance and determinism

Measured on GitHub-hosted runners, 5 fresh-process samples per cell, both builds measured by the same harness in the same run: run 35216818822, attempt 2 (green).

OS Workload Cold upstream Cold fork Warm upstream Warm fork
ubuntu-latest powershell 6.62 2.80 0.10 0.11
ubuntu-latest semver 12.92 5.21 1.96 1.82
windows-latest powershell 8.49 2.60 0.10 0.07
windows-latest semver 12.96 6.52 2.29 2.23

Cold runs are 2.0x to 3.3x faster. Warm is at or slightly ahead of upstream everywhere; the earlier apparent warm regression was the measurement artifact described above, and is gone now that the harness spends the tiering ramp before sampling.

Determinism

The workflow retries a sample in a fresh process when the analyzer exits nonzero, but only for upstream: a nonzero exit from the fork fails the job outright. So the retry counts below are a direct count of analyzer crashes.

Run Workload Upstream crashes Fork crashes Distinct upstream fingerprints Distinct fork fingerprints
attempt 1 ubuntu semver 10 0 2 — 154 and 157 findings 1 — 154
attempt 1 windows semver 9 0 2 — 154 and 155 findings 1 — 154
attempt 2 ubuntu semver 6 0 1 — 154 1 — 154
attempt 2 windows semver 2 0 1 — 154 1 — 154

Upstream crashed 27 times across the two runs while analyzing a 111-file module, and in attempt 1 the surviving samples did not even agree with each other: the same input yielded 154, 155 or 157 findings depending on the run. That attempt failed the workflow's diagnostic-equivalence gate, which is the failure this PR exists to fix. Attempt 2 passed only because the retries happened to land on agreeing samples; upstream still crashed 8 times in it.

The fork crashed zero times in either run and produced one fingerprint, 154 findings, across all 40 measurements. The powershell workload never triggered the race on either build, which is consistent with it being timing-dependent rather than input-dependent.

Two honest caveats on these numbers. Retried timings keep only the successful attempts, so upstream's cold medians are if anything flattered by discarding the runs that died. And the two builds run on separate hosts, so this is a like-for-like comparison of medians rather than a controlled statistical test.

The fork is consistently faster than upstream on cold-start (the most common real-world scenario), with steady-state warm performance unchanged and without the nondeterminism. I'd propose evaluating these changes as the new baseline.

Additional local Windows stress and recovery evidence (2026-09-17)

The CI evidence above is retained as the primary cross-platform result. The following is an additional local Windows run against the same 111-file actions-semver-action workload. It uses fresh, non-interactive processes, eight concurrent analyses, and a 120-second process bound. This is a single local sample, not a controlled benchmark.

Build Host Fresh runs Successful Caught error / process abort Timeout
upstream f6cecef PowerShell 7.6.6 100 59 41 0
upstream f6cecef Windows PowerShell 5.1.26100.9444 100 55 43 2
this PR 09f4323 PowerShell 7.6.6 100 100 0 0
this PR 09f4323 Windows PowerShell 5.1.26100.9444 100 100 0 0

Across the 400 fresh-process analyses, upstream had 86 terminal failures (43%): 58 Get-Command command-not-found errors, 22 null-reference errors, two pipeline-creation errors, one concurrent-dictionary process abort, one duplicate-null-key process abort, and two no-output timeouts. The PR build had zero caught errors, process aborts, or timeouts in its 200 runs. The prior CI and development observations above remain valid even where their error classes did not occur in this particular local sample.

Recovery behavior after a caught cold failure

A separate upstream-only recovery smoke test started a fresh process, ran one cold analysis, then retried immediately in that same process only after a caught error. The process was allowed to continue until success, crash, or timeout. The preserved initial sample contains eight caught Get-Command failures: one from session 1 before its host timed out, and seven consecutive failures from session 2 before the test was stopped. This shows that a failed cold lookup does not necessarily self-heal in a warm host; the retained Lazy lookup can continue to throw from whichever rule touches it next.

These are the distinct ScriptAnalyzer tail signatures observed in those eight failures. Counts are occurrences, not unique root causes; all have the same CommandNotFoundException root message shown earlier.

1 occurrence
Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper.GetCommandInfo(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.AvoidAlias.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)

2 occurrences
Microsoft.Windows.PowerShell.ScriptAnalyzer.Helper.GetCommandInfo(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseCorrectCasing.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)

4 occurrences
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseShouldProcessCorrectly.CheckForSupportShouldProcess()
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseShouldProcessCorrectly.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)

1 occurrence
Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules.UseShouldProcessCorrectly.AnalyzeScript(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)
Microsoft.Windows.PowerShell.ScriptAnalyzer.ScriptAnalyzer.<AnalyzeSyntaxTree>(...)

The recovery test also now mirrors caught exceptions to each session's stderr file as well as retaining full structured exception text in its JSONL result record. Empty stderr is therefore meaningful only for a successful session or a process that stopped before producing managed error output.

Parallel PowerShell 7 recovery run (2026-09-17)

The earlier recovery smoke established the behavior. A subsequent, larger upstream-only run used 100 fresh PowerShell 7.6.6 hosts in waves of 10 concurrent hosts. Every host ran one cold analysis; after a caught failure it immediately retried in that same process until success, an unhandled process abort, or the 120-second host bound. This run produced 718 managed analysis attempts in total.

First-attempt / terminal outcome Hosts
Cold success 60
Recovered after one or more caught failures 12
Continued to fail until the 120-second bound 23
Timed out before producing a managed result 3
Aborted before producing a managed result 2

Of the 35 hosts that produced a caught first-attempt failure, 12 (34%) recovered in the same warm process and 23 (66%) did not recover before the bound. Therefore a warm retry is not a reliable recovery mechanism: an initial failed lookup can leave the process in a state where subsequent analyses keep failing through different rule paths.

Caught exception details

Exception / message Occurrences across 718 managed attempts
CommandNotFoundException: Get-Command is not recognized 548
CmdletInvocationException: object reference not set 83
NullReferenceException: object reference not set 15

The two unhandled process aborts both occurred while PowerShell initialized default variables in SessionStateScope.AddSessionStateScopeDefaultVariables(): one was the duplicate-key error with key false shown above, and one was the runtime concurrent-collection-corruption InvalidOperationException shown earlier. Three timeout hosts produced no managed result or stderr; 23 other hosts had managed failures but did not recover before their 120-second bounds.

Rules reached by caught failures

This table counts a rule once for each failed managed attempt whose ScriptAnalyzer stack contained that rule. The counts are paths reached after failure, not separate root causes; all four rules consume command metadata and can encounter the poisoned lookup state.

Rule Failed attempts Distinct hosts
UseShouldProcessCorrectly 366 18
UseCorrectCasing 197 27
UseCmdletCorrectly 41 9
AvoidAlias 40 11
No built-in rule frame retained 8 8

The exception-to-rule breakdown was:

Rule Get-Command command-not-found Wrapped null reference Raw null reference
UseShouldProcessCorrectly 366 0 0
UseCorrectCasing 125 57 15
UseCmdletCorrectly 22 19 0
AvoidAlias 35 5 0
No built-in rule frame retained 6 2 0

Tested against

All have safe cold and warm runs and do not cause any errors.

Linked issues

Directly addressed

  • Fixes #2205, the exact Get-Command command-not-found failure from CommandInfoCache. Failed lookups are now evicted rather than retained by Lazy<T>, and runspace access is serialized.
  • Fixes #1708, the intermittent UseCorrectCasing CommandInfo.Parameters null reference. The issue was still reported on PSScriptAnalyzer 1.25.0 with PowerShell 7.6.4. This PR retrieves detached parameter snapshots while holding the runspace lock, rather than reading live metadata from rule worker threads.

Likely resolved or materially improved

  • Likely resolves #1351, a transient recursive-analysis CommandInfo.ResolveParameter null reference reached through Helper.GetExportedFunction by ProvideCommentHelp and AvoidReservedCharInCmdlet. This PR routes that exported-function metadata through the same locked, detached snapshot path.
  • Likely materially improves #1189, slow recursive scans caused by repeated Get-Command calls implicitly scanning/loading modules. The local module run reduces unresolved lookups from 170 of 223 to 4 of 55, with cold execution falling from 14.01s to 3.06s. The original workload may retain unrelated costs, so this is not claimed as a complete fix.

Related, but not claimed as fixed

  • #959 reports a generic concurrency exception while debugging PowerShell Editor Services. It may involve overlapping external analyzer calls, which is outside the specific internal command-runspace access covered here.
  • #1030 documents historic command-cache lock contention and sporadic failures. It supports the same concurrency diagnosis but has no distinct current reproduction to close from this evidence.
  • #1867 reports intermittent null references without a stack trace. It may overlap with the fixed metadata races but cannot be attributed confidently.
  • #1881 is a malformed attribute-constructor metadata error, not one of the command-resolution or concurrency failure signatures addressed by this PR.

Copilot AI and others added 9 commits August 19, 2026 13:54
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
… path

Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
…issue-2205

Fix intermittent "The term 'Get-Command' is not recognized" failures during recursive analysis

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens PSScriptAnalyzer’s command-resolution path to avoid intermittent PowerShell engine/runspace-affinity failures (notably Get-Command resolution and CommandInfo.Parameters access) from causing a cascading, process-long failure during recursive/parallel analysis.

Changes:

  • Adds retry logic for transient Get-Command resolution failures and avoids permanently poisoning the CommandInfoCache when a cached Lazy<CommandInfo> faults.
  • Makes UseCorrectCasing resilient to InvalidOperationException/NullReferenceException from CommandInfo.Parameters, retrying via a fresh lookup and skipping only parameter-casing when parameters can’t be determined.
  • Adds a Linux-only regression test exercising recursive analysis with the reported settings under -ErrorAction Stop.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
Tests/Rules/Issue2205/PSScriptAnalyzerSettings.psd1 Adds the settings file used by the regression test scenario.
Tests/Rules/Issue2205.tests.ps1 Adds a regression test for recursive analysis under -ErrorAction Stop (Linux).
Rules/UseCorrectCasing.cs Adds a retry-and-skip path for parameter casing when parameter metadata can’t be reliably retrieved.
Engine/CommandInfoCache.cs Adds retry logic for transient Get-Command resolution failures and evicts faulted cached entries to prevent permanent poisoning.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Tests/Rules/Issue2205.tests.ps1 Outdated
Comment on lines +5 to +10
It "does not fail the analysis when a command lookup hits the runspace affinity problem" -Skip:(-not $IsLinux) {
$settingsPath = Join-Path $PSScriptRoot 'Issue2205/PSScriptAnalyzerSettings.psd1'
# $PSScriptRoot is <repo>/Tests/Rules, so two levels up is the repository root.
$repositoryRoot = (Resolve-Path (Join-Path $PSScriptRoot '..' '..')).Path

Invoke-ScriptAnalyzer -Path $repositoryRoot -Recurse -Settings $settingsPath -ErrorAction Stop | Out-Null
Comment thread Tests/Rules/Issue2205.tests.ps1 Outdated
@@ -0,0 +1,12 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
Copilot AI and others added 3 commits August 19, 2026 15:33
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
@jessehouwing

Copy link
Copy Markdown
Author

A more permanent solution may en to serialize access to the runspace. At the repo sizes I work at this causes minimal perf overhead and stabilizes the output from the script analyzer

jessehouwing#2

…lls-to-sequential

Serialize CommandInfo lookups onto a single dedicated runspace

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

Engine/CommandInfoCache.cs:99

  • GetOrAdd(key, value) eagerly allocates a new Lazy on every lookup, even when the key is already cached. Since cache hits are expected to dominate (and are intentionally lock-free), this adds avoidable allocations/GC pressure in the hot path. Use the valueFactory overload so the Lazy is only created on cache misses.
            var lazyCommandInfo = _commandInfoCache.GetOrAdd(key, new Lazy<CommandInfo>(() => GetCommandInfoInternal(commandName, commandTypes)));

Engine/CommandInfoCache.cs:62

  • This comment refers to a “finalizer path”, but CommandInfoCache does not define a finalizer; Dispose(bool) is only called from Dispose() unless a derived type adds a finalizer. The wording is misleading for readers trying to reason about disposal semantics.
            // Always take the lock, also on the finalizer path, so that 'disposed' is never
            // published without the runspace being disposed along with it and so that the runspace
            // cannot be disposed while a lookup is in flight.

Tests/Engine/CommandInfoCacheConcurrency.tests.ps1:35

  • Task.WaitAll(tasks) has no timeout; if a regression causes a deadlock/hang, the test run can stall indefinitely. Add a bounded wait and fail fast on timeout to keep CI reliable.
        Task.WaitAll(tasks);

@bergmeister

Copy link
Copy Markdown
Collaborator

Jesse Houwing (@jessehouwing) Thanks for your initiative to take this on. There have been a few issues with CommandIssuecache concurrency where PSSA errored. As far as I could track it down it's even root caused in PowerShell engine internals itself not being thread safe. I attempted a fix in PowerShell, which is a good read on related PowerShell and PSSA issues.
Regarding your fix, I am most concerned about performance because when I previously optimized locks I found performance degraded a lot if concurrency was reduced. I am willing to give your PR a chance:
In the past I've used analyzing this script as a good performance test: https://github.com/PowerShell/PowerShell/blob/master/build.psm1
Can you please report the time to run Invoke-ScriptAnalyzer on just this file please? Once in your branch and once on main. One measurement for cold run (analysing file first time in new Shell) and once warm (after that) please.

Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
…-workflow

Add isolated cross-platform PSScriptAnalyzer performance benchmarks
@jessehouwing

Jesse Houwing (jessehouwing) commented Sep 14, 2026

Copy link
Copy Markdown
Author

In my tests so far I haven't been able to get clean results once from v1.25. And there are multiple bugs happening all at once.

I may need to go over all the changes to clean some stuff up. I've been iterating over it using copilot and it introduced a few "retry loops" that may no longer be needed. And I added in a telemetry thingy, that may need to be ripped out again.

It's now also faster when running warm. And got the cold perf to 2x on windows, still 3x on ubuntu.

The issues are much easier to reproduce when using -resurse...

@jessehouwing

Copy link
Copy Markdown
Author

Here's al log where the issues reproduce very consistently:
https://github.com/jessehouwing/PSScriptAnalyzer/commit/7b223219b072517e87b04a9448868dc6eb9e48c3/checks/104150058905/logs

Somehow Ubuntu-latest seems to more consistently hit the problem.

Copilot AI and others added 13 commits September 14, 2026 21:26
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Make engine retries verifiable and centralize metadata recovery
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
Expand perf workflow to benchmark `perf` with and without `DISABLE_ENGINE_RETRIES`
Co-authored-by: jessehouwing <4173387+jessehouwing@users.noreply.github.com>
…isons

Restore benchmark comparisons and explicit retry variants
- Drop retry loops and DISABLE_ENGINE_RETRIES flag from CommandInfoCache;
  failed lookups evict the cache entry and return null gracefully
- Delete PerformanceTelemetry and all metrics collection from the engine,
  benchmark harness, workflows and tests
- Refactor GetCommandMetadata to Func<CommandInfo, T>
- Add lookup-count and module-qualified resolution tests
- Rework Issue2205 repro into generic ParallelRuleExecution test with a
  bounded synthetic workload, no Linux filter

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Benchmark now compares upstream@main against fork@main only, in
preparation for merging the perf branch to main. Also makes source a
proper matrix dimension.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jessehouwing

Copy link
Copy Markdown
Author

I think this is as much as I can squeeze out of this today:

OS Workload Source Cold median (s) Warm median (s) Status
ubuntu-latest powershell upstream 6.966 0.102 Verified
ubuntu-latest powershell fork 5.846 0.083 Verified
ubuntu-latest semver upstream 13.913 1.985 Verified; ⚠️ retried samples
ubuntu-latest semver fork 14.193 1.934 Verified
windows-latest powershell upstream 7.391 0.105 Verified
windows-latest powershell fork 11.623 0.090 Verified
windows-latest semver upstream 10.138 3.184 Verified; ⚠️ retried samples
windows-latest semver fork 14.321 2.626 Verified

The numbers are so close now, that I'd propose evaluating these changes as the new baseline.

Windows PowerShell 5.1 compiles Add-Type definitions with the legacy
C# 5 compiler, which rejects the null-conditional operator and
expression-bodied members used by the new test fixtures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jessehouwing

Copy link
Copy Markdown
Author

While testing I've observed the following intermittent/flakey errors in 1.25, which no longer appear after applying this PR:

Unhandled exception. System.ArgumentException: An item with the same key has already been added. Key: null
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at System.Management.Automation.SessionStateScope.AddSessionStateScopeDefaultVariables()
   at System.Management.Automation.SessionStateScope.GetPrivateVariables()
   at System.Management.Automation.VariableScopeItemSearcher.GetScopeItem(SessionStateScope scope, VariablePath name, PSVariable& variable)
   at System.Management.Automation.ScopedItemSearcher`1.MoveNext()
   at System.Management.Automation.SessionStateInternal.GetVariableItem(VariablePath variablePath, SessionStateScope& scope, CommandOrigin origin)
   at System.Management.Automation.SessionStateInternal.GetVariableValue(VariablePath variablePath, CmdletProviderContext& context, SessionStateScope& scope)
   at System.Management.Automation.ExecutionContext.GetVariableValue(VariablePath path, Object defaultValue)
   at System.Management.Automation.Internal.PipelineProcessor.Start(Boolean incomingStream)
   at System.Management.Automation.Internal.PipelineProcessor.SynchronousExecuteEnumerate(Object input)
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeHelper()
   at System.Management.Automation.Runspaces.LocalPipeline.InvokeThreadProc()
   at System.Management.Automation.Runspaces.PipelineThread.WorkerProc()
   at System.Threading.Thread.StartHelper.Callback(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
Invoke-ScriptAnalyzer: /home/runner/work/PSScriptAnalyzer/PSScriptAnalyzer/harness/tools/Measure-ScriptAnalyzerPerformance.ps1:133
Line |
 133 |      $diagnostics = @(& $command @analyzerArguments)
     |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | The term 'Get-Command' is not recognized as a name of a cmdlet,
     | function, script file, or executable program. Check the spelling of the
     | name, or if a path was included, verify that the path is correct and try
     | again.
Invoke-ScriptAnalyzer: /home/runner/work/PSScriptAnalyzer/PSScriptAnalyzer/harness/tools/Measure-ScriptAnalyzerPerformance.ps1:133
Line |
 133 |      $diagnostics = @(& $command @analyzerArguments)
     |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Object reference not set to an instance of an object

These issues are easy to reproduce, especially when using -recurse.

See for example: https://github.com/jessehouwing/PSScriptAnalyzer/commit/b8472db30f8bf1d534c952aa6535d1ae6607b382/checks/104292570111/logs

AvoidAlias probed 'Get-{name}' for every name that failed to resolve, so a call to a Verb-Noun function the session does not know produced a second lookup for names like 'Get-Test-ActionInput' or 'Get-Get-GitHubRelease'.

A Get-Command miss costs ~59ms because it scans the whole PSModulePath, against ~0.8ms for a hit, so the redundant probe doubled the cost of every such call. The check targets bare nouns such as 'process' standing in for 'Get-Process'; a name that already contains a hyphen is a Verb-Noun name and 'Get-' + it never resolves. No command matching 'Get-<x>-<y>' exists among the 116289 commands discoverable on the test machine.
Command lookups run against a pristine runspace, so a function the script defines itself was answered by whatever module happened to be installed on the analyzing machine. A script defining 'function Get-ChildItem { param($PATH) }' had its call to that function validated against the real cmdlet, and UseCorrectCasing offered to rewrite -PATH to -Path to match a command PowerShell would never bind. Results therefore depended on the host rather than the script.

Collect the FunctionDefinitionAst names once per analysis and answer lookups for those names locally. Output changes only where a script shadows an installed command; a script-defined name that is not installed already resolved to null, just 59ms slower per name, which is where most of the analysis time went.

Also removes a duplicated nested CommandLookupKey and adds lookup counters used to locate the bottleneck.
The call graph adds a vertex named after the member of every invocation, so '$x.Substring(1)' became a vertex 'Substring'. CheckForSupportShouldProcess then resolved every such name as a command, which is both wrong and expensive: a Get-Command miss scans the whole PSModulePath at ~59ms.

'mkdir' is a function that declares SupportsShouldProcess, so a script calling '$dir.mkdir()' was credited with delegating ShouldProcess and its missing implementation went unreported. Mark vertices reached through an actual command invocation and only resolve those.

Vertices are keyed on name alone, so the flag is merged when an existing vertex is reused; tests cover a name used as both a command and a member in either order.
The tests relied on mkdir declaring SupportsShouldProcess, which holds only on Windows, where mkdir is a PowerShell function. On Linux and macOS it is the native binary, so a real command call was no longer credited and three tests failed.

A quoted member name may contain a hyphen, so $o.'Remove-Item'() is a member invocation whose name also belongs to a cmdlet that declares SupportsShouldProcess everywhere. That keeps the tests running on every platform instead of being skipped.
The Set-Service function in this fixture declares SupportsShouldProcess but
never calls it, which the PSShouldProcess rule is meant to report. The rule
stayed quiet only because it resolved the name against the real Set-Service
cmdlet instead of the function the fixture defines, so the fixture's own
latent violation was masked by which modules happened to be installed.
A module composes itself by dot-sourcing its libraries, so a call from one
library to a function in another binds locally at run time even though neither
file mentions the other. Analyzing such a call as an external command sent it
to Get-Command, and a name that resolves nowhere costs a full PSModulePath
scan, so these were the most expensive lookups in a run.

Group the files of a run by dot-source relationship and give each file the
functions of the closures that reach it. The grouping follows the dot-source
direction rather than merging whole connected components: a test script that
sources one library must not be credited with the other libraries that happen
to share a root.

Hand the syntax trees from that pre-pass to the analysis instead of discarding
them, so each file is still parsed once per run. The -Fix path opts out, since
it rewrites each file before analyzing it.

On a 111-file module this cuts a cold run from 13.5s to 2.9s and leaves warm
runs unchanged, with identical diagnostics.
These counters were added to find where lookup time was going. The answer is
now encoded in the fixes and their tests, so the counters are dead weight on a
hot path.
The cold run canonicalizes its diagnostics into JSON before the warm run
starts, so the garbage from that could be collected on the warm run's clock.
Collecting first makes the two measurements symmetric.
@jessehouwing

Jesse Houwing (jessehouwing) commented Sep 17, 2026

Copy link
Copy Markdown
Author

Latest perf stats after most recent changes:

OS Workload Source Cold median (s) Warm median (s) Status
ubuntu-latest powershell upstream 6.235 0.102 Verified
ubuntu-latest powershell fork 2.424 0.121 Verified
ubuntu-latest semver upstream 10.496 1.613 Verified; ⚠️ retried samples
ubuntu-latest semver fork 8.655 3.546 Verified
windows-latest powershell upstream 7.963 0.091 Verified
windows-latest powershell fork 2.970 0.118 Verified
windows-latest semver upstream 11.528 2.447 Verified; ⚠️ retried samples
windows-latest semver fork 6.047 2.187 Verified

Latest run

The warm sample was the analysis immediately following the cold one, which is
not steady state: tiered compilation promotes hot methods over the first few
iterations, and shortening the cold pass pushed that ramp into the warm sample
rather than removing it. There is no way to wait for the tiering queue to
drain, so run and discard a few analyses first.

Measured with tiering disabled, the second analysis already sits at steady
state, which is what identified the ramp as the cause.
@jessehouwing Jesse Houwing (jessehouwing) changed the title Fix intermittent "The term 'Get-Command' is not recognized" failures during recursive analysis Fix intermittent "The term 'Get-Command' is not recognized" failures during recursive analysis and improve performance while doing so. Sep 17, 2026
@jessehouwing

Copy link
Copy Markdown
Author

Andy Jordan (@andyleejordan) / Christoph Bergmeister (@bergmeister)

I think I've exhausted this area of investigation. And I'm pretty stoked with the results, even though it adds some complexity to the analysis process.

I've tested against a couple of internal projects, my own open source project where I originally observed the issues, the script analyzer's own build process and the /tests folder.

I'm still receiving consistent results, comparable to the current main.

I'm no longer observing any crashes, results are consistent and performance is way up from the original implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants