Skip to content

fix(pwsh): skip streaming in PowerShell Editor Services - #7810

Closed
SergiorCode wants to merge 2 commits into
JanDeDobbeleer:mainfrom
SergiorCode:fix/streaming-editor-services
Closed

fix(pwsh): skip streaming in PowerShell Editor Services#7810
SergiorCode wants to merge 2 commits into
JanDeDobbeleer:mainfrom
SergiorCode:fix/streaming-editor-services

Conversation

@SergiorCode

Copy link
Copy Markdown

Prerequisites

  • I have read and understood the contributing guide.
  • The commit message follows the conventional commits guidelines.
  • Tests for the changes have been added (for bug fixes / features).
  • Docs have been added/updated (for bug fixes / features).

Description

Fixes #7809, following the primary fix proposed in the issue analysis.

Problem. streaming drives its async repaints from a PowerShell.OnIdle subscriber that calls InvokePrompt() — a real pipeline invocation. That is safe on the stock console host, where PSReadLine's ReadKey() loop serializes all idle-time work on one thread. PowerShell Editor Services (the host behind the VS Code PowerShell extension and Neovim's PSES client) breaks the assumption: it installs its own idle hook that ReadKey() calls into as PsesInternalHost.OnPowerShellIdle(), and runs its own synchronous pipelines from there. Two idle-triggered pipeline invokers then race on the same runspace at roughly the same ~300ms cadence, and DoConcurrentCheck throws PipelineConcurrentInvokeNotAllowed, which PSReadLine surfaces as a crash dump.

Fix. Detect a PSES-hosted session in Enable-PoshStreaming and return before setting $global:_ompStreaming, so the prompt renders synchronously there. Not registering the subscriber at all also keeps PSES's own pump out of the mode that makes this racy — it only pulses its event-processing pipeline while Runspace.Events.Subscribers is non-empty.

Detection is $psEditor (defined only by PSES) with $Host.Name as a fallback, read through Get-Variable so an unset variable doesn't throw under Set-StrictMode, matching the existing $global:_ompAsyncInit read. $env:TERM_PROGRAM -eq 'vscode' is deliberately not used: it also matches a plain pwsh session in VS Code's regular terminal, where streaming works fine and should stay on.

Guarding InvokePrompt() instead was considered and rejected — the throw originates inside PSES's own InvokePSCommand, so any script-side availability check is a TOCTOU race.

Not verified

I don't have a PSES environment to test against, so two things from the issue analysis are still open and worth a second pair of eyes:

  • the actual $psEditor / $Host.Name values under Neovim's PSES client specifically (VS Code's are well documented, the nvim LSP client's may differ)
  • confirmation on a real repro that the crash is gone with the subscriber simply never registered

The $psEditor check should hold regardless of host name, since PSES registers that variable itself, but a real-world confirmation would be better than my reasoning about it.

PSES invokes its own synchronous pipelines from the PSReadLine idle hook,
and only pumps them while the runspace has event subscribers. Registering
PowerShell.OnIdle for the async repaint therefore puts a second pipeline
invoker on the same runspace at the same ~300ms cadence, and
DoConcurrentCheck aborts whichever one loses with "Pipelines cannot be run
concurrently".

The throw happens inside the host's own invocation, so script code can
neither observe nor synchronize against it. Detect the host instead and
leave streaming off, which also keeps the PSES pump out of that mode in
the first place.

Fixes JanDeDobbeleer#7809
@mattcargile

Copy link
Copy Markdown
Contributor

It might be nice to have the oh-my-posh serve but not the streaming. Not sure if that is possible? Having that warm cache, I think makes it feel that extra bit snappy.

Skipping streaming entirely in PowerShell Editor Services also gave up
the persistent daemon, paying a full CLI spawn per prompt. The hazard in
PSES is only the PowerShell.OnIdle subscriber: the host runs its own
synchronous pipelines from the same idle hook, and a second idle-driven
pipeline invoker is what DoConcurrentCheck aborts (JanDeDobbeleer#7809).

Keep the daemon and drop only the repaints: under PSES, render requests
now use the protocol's wait mode (the cmd/Clink contract) - the daemon
resolves every segment before replying with exactly two records, primary
and transient, so there is nothing to repaint and the OnIdle subscriber
is never registered. Prompts keep print primary accuracy and the warm
daemon's speed.

Wait renders carry no client-side deadline - like print primary and
Clink, a slow render is simply waited out, and daemon death is the only
failure signal. Since a wait cycle cannot be aborted daemon-side, a
cycle the client abandoned (Ctrl+C mid-wait, or a torn request/abort
write) is recovered at the next prompt by killing and restarting the
daemon instead of writing into a stream that is no longer in sync.

Daemon failures fall back to a synchronous CLI render instead of the
legacy per-prompt stream, which would re-register OnIdle, and a third
strike now also shuts the daemon down instead of stranding it. Without
a daemon at all (Windows PowerShell 5.1, ConstrainedLanguage) the
prompt stays fully synchronous, as before.
@SergiorCode

Copy link
Copy Markdown
Author

Good call — it's possible, and the protocol already has the primitive for it: the wait render mode the cmd/Clink integration uses. The daemon resolves every segment before replying with exactly two records (primary + transient), so no incremental records exist and there's nothing to repaint.

The update does exactly that under PSES: the daemon is kept, render requests carry "wait":true, and the PowerShell.OnIdle subscriber — the actual hazard here — is never registered. You keep the warm daemon (in-memory caches, no per-prompt spawn) with print primary accuracy; what you give up is only the live repaints. Daemon failures fall back to a synchronous CLI render instead of the legacy per-prompt stream, which would re-register OnIdle.

One thing worth confirming on a live PSES session, since I still don't have a repro environment: PowerShell.Exiting stays registered so a daemon never outlives the session, which keeps PSES's event pump pulsing. That pulse runs serialized on PSES's own execution queue — the crash needed a second independent pipeline invoker, which only the OnIdle action supplied — so it should be safe, but real-world confirmation would be great.

@JanDeDobbeleer

Copy link
Copy Markdown
Owner

decided to simply remove support instead. We can add support when they offer compatibility on their end.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streaming - _PowerShell Editor Services_ Terminal OnIdle PSReadline Error

3 participants