Skip to content

Fix incorrect parser errors from unloaded assemblies #5450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andyleejordan wants to merge 1 commit into
base: main
Choose a base branch
from
+29 −3
Open

Fix incorrect parser errors from unloaded assemblies #5450

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions src/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ export class SessionManager implements Middleware {
private versionDetails: IPowerShellVersionDetails | undefined;
private traceLogLevelHandler?: vscode.Disposable;

// Promise-based gate resolved when the session reaches Running status.
// Used by waitUntilStarted() and the didOpen()/didChange() notifications.
// eslint-disable-next-line @typescript-eslint/no-invalid-void-type
private started = Promise.withResolvers<void>();
Comment on lines +131 to +132
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

The eslint-disable @typescript-eslint/no-invalid-void-type is only needed because the generic is void. If you switch the deferred to use undefined (or a small Deferred<T> helper type), you can avoid the lint suppression and keep the type intent the same (a promise that resolves with no value).

Suggested change
// eslint-disable-next-line @typescript-eslint/no-invalid-void-type
private started = Promise.withResolvers<void>();
private started = Promise.withResolvers<undefined>();

Copilot uses AI. Check for mistakes.

constructor(
private extensionContext: vscode.ExtensionContext,
private sessionSettings: Settings,
Expand Down Expand Up @@ -295,6 +300,7 @@ export class SessionManager implements Middleware {
`Started PowerShell v${this.versionDetails.version}.`,
);
this.setSessionRunningStatus(); // Yay, we made it!
this.started.resolve(); // Release didOpen()/didChange() notifications and waitUntilStarted() gate

await this.writePidIfInDevMode(this.languageServerProcess);

Expand Down Expand Up @@ -328,6 +334,8 @@ export class SessionManager implements Middleware {
}

this.languageClient = undefined;
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

stop() replaces this.started with a new resolver without resolving/rejecting the previous promise. Any callers already awaiting the old this.started.promise (e.g., waitUntilStarted() or an in-flight middleware didOpen/didChange) can hang forever, even after a successful restart. Consider making the gate lifecycle explicit: create a new deferred at the beginning of each start() attempt, resolve it on Running, and reject/resolve it on stop/failure before replacing it so existing awaiters are unblocked.

Suggested change
this.languageClient = undefined;
this.languageClient = undefined;
const started = this.started;
started.resolve();

Copilot uses AI. Check for mistakes.
// eslint-disable-next-line @typescript-eslint/no-invalid-void-type
this.started = Promise.withResolvers<void>();

// Stop and dispose the PowerShell process(es).
this.debugSessionProcess?.dispose();
Expand Down Expand Up @@ -497,9 +505,27 @@ export class SessionManager implements Middleware {
}

public async waitUntilStarted(): Promise<void> {
while (this.sessionStatus !== SessionStatus.Running) {
await utils.sleep(200);
}
await this.started.promise;
}

// Middleware hooks to delay document sync notifications until the server
// is fully initialized. This prevents stale parser diagnostics (e.g.
// unresolved custom attribute types) that would otherwise appear because
// textDocument/didOpen is sent before the server's type resolution is ready.
public async didOpen(
document: vscode.TextDocument,
next: (document: vscode.TextDocument) => Promise<void>,
): Promise<void> {
await this.started.promise;
return next(document);
}

public async didChange(
event: vscode.TextDocumentChangeEvent,
next: (event: vscode.TextDocumentChangeEvent) => Promise<void>,
): Promise<void> {
await this.started.promise;
return next(event);
}

// TODO: Is this used by the magic of "Middleware" in the client library?
Expand Down
Loading