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

[Jesse Houwing (jessehouwing)]

Invoke-ScriptAnalyzer -Path './' -Recurse -Settings ... intermittently fails on Linux/CI with CommandNotFoundException: The term 'Get-Command' is not recognized and NullReferenceException at CommandInfo.get_Parameters(). The reported settings file is valid — this is an engine concurrency bug, not a configuration error.

Fixes: #2205

Root cause

CommandInfoCache resolves commands through a shared RunspacePool while ScriptAnalyzer.AnalyzeSyntaxTree runs script rules on parallel tasks. Occasionally the PowerShell engine fails to resolve Get-Command itself in a pooled runspace — a manifestation of PowerShell/PowerShell#4003.

The transient blip then became permanent: the cache is a ConcurrentDictionary<CommandLookupKey, Lazy<CommandInfo>> using LazyThreadSafetyMode.ExecutionAndPublication, which caches exceptions for the lifetime of the process. One failure poisoned that command's entry forever, cascading into hundreds of errors. Since rule exceptions surface via WriteError, -ErrorAction Stop (the norm in GitHub Actions) turns them into a hard failure.

Diagnostics confirmed transience: at failure time a fresh runspace worked, a fresh pool worked, and immediate retries on the same pool succeeded.

Changes

• Engine/CommandInfoCache.cs

  • GetCommandInfoInternal retries the Get-Command invocation up to MaxLookupAttempts (3) on CommandNotFoundException, returning null if it still fails. The catch is precisely scoped: Get-Command is invoked with -ErrorAction SilentlyContinue, so this exception can only mean Get-Command itself was unresolvable.

  • GetCommandInfo no longer lets a faulted Lazy<T> poison the cache — the entry is evicted before rethrowing. Eviction is value-specific so a replacement added concurrently by another thread isn't dropped:

    catch
    {
        ((ICollection<KeyValuePair<CommandLookupKey, Lazy<CommandInfo>>>)_commandInfoCache)
            .Remove(new KeyValuePair<CommandLookupKey, Lazy<CommandInfo>>(key, lazyCommandInfo));
        throw;
    }

• Rules/UseCorrectCasing.cs

  • The existing catch (InvalidOperationException) around parameter lookup now also covers NullReferenceException (see UseCorrectCasing gets NullReferenceException from CommandInfo.get_Parameters #1708), retries once via a cache-bypassing lookup, and skips only the parameter-casing check when parameters still can't be determined — instead of failing the rule.

• Tests/Rules/Issue2205.tests.ps1 — regression test performing a full recursive analysis of the repository with the reported settings under -ErrorAction Stop.

Note for reviewers

This is a defensive workaround for an upstream engine defect, consistent with the earlier precedent in #1523. If the underlying runspace-affinity issue is ever fixed, the retry loop becomes dead weight and can be removed.

More extensive fix applied:

Follow-up to #2206 / issue #2205. CommandInfoCache resolved commands through a RunspacePool(1, 10), allowing up to ten Get-Command invocations to run concurrently across separate runspaces. The PowerShell engine's command discovery state is not thread safe (PowerShell#4003), which surfaced as intermittent CommandNotFoundException for Get-Command itself and non-deterministic diagnostic counts.

A runspace pool doesn't help here: it already guarantees a given runspace is used by only one PowerShell instance at a time, and that per-runspace serialization is precisely the configuration that fails. The unsafe state is engine-global, so the gate has to be global too — at which point additional pooled runspaces are never concurrently active.

Changes

• Engine/CommandInfoCache.cs

  • Replaced the runspace pool with a single long-lived Runspace.
  • Added a re-entrant instance monitor around the engine invocation, including the retry loop. A monitor rather than a SemaphoreSlim so a future re-entrant lookup can't self-deadlock.
  • Cache hits remain lock-free — only misses reach the engine and serialize.
  • Dispose acquires the same lock; a lookup that acquires it after disposal returns null rather than touching a disposed runspace.
  • Retained the retry / CommandNotFoundException fallback from Fix intermittent "The term 'Get-Command' is not recognized" failures during recursive analysis #2206 as a safety net for hosts that drive the engine from other threads.

• Tests/Engine/CommandInfoCacheConcurrency.tests.ps1 (new)

  • Resolves 40 commands from thread pool threads via a small C# driver and asserts each lookup resolves. The driver is C# because invoking a script block on a thread pool thread introduces runspace affinity problems of its own.
  • Runs the analyzer once first: touching Helper.Instance before the cmdlet does installs a helper with no command invocation context, which breaks every later analysis in the process. Pre-existing engine fragility, called out here so the test doesn't trip it.

// Only cache misses reach this; hits are served from the ConcurrentDictionary lock-free.
lock (_runspaceLock)
{
    if (disposed) { return null; }

    using (var ps = System.Management.Automation.PowerShell.Create())
    {
        ps.Runspace = _runspace;
        // ...
    }
}

Performance

Invoke-ScriptAnalyzer -Path Tests -Recurse, warm, alternating runs:

	before	after
steady state	15.0s / 14.5s	15.1 – 15.8s
outliers	81s, 106s, 116s	none
diagnostics	4187 – 4199	4199

Steady-state cost is within noise. The pre-change build also produced large outliers and varying diagnostic counts, so serializing appears to trade little or no throughput for determinism.

[Copilot]

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.

[Jesse Houwing (jessehouwing)]

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

[Copilot]

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);