Skip to content

Commit 4a09091

Browse files
vadimLuzyaninqwencoder
authored andcommitted
fix(core): add LSP diagnostics caching and document refresh fallback
Add publishDiagnostics notification handler in LspServerManager that caches diagnostics and supports pending diagnostic promises. When textDocument/diagnostic pull fails in NativeLspService, fall back to force-refreshing the document (didClose + didOpen) and reading from the cached diagnostics. Also fix review findings: - Clear cache/pending maps on server restart in resetHandle() - Clean up pending diagnostics entries on timeout - Re-track URI in openedDocuments on refresh failure - Filter workspaceDiagnostics fallback by workspace root URI Fixes #3029 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
1 parent f208801 commit 4a09091

File tree

3 files changed

+149
-0
lines changed

3 files changed

+149
-0
lines changed

packages/core/src/lsp/LspServerManager.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import type {
2828
LspServerStatus,
2929
LspSocketOptions,
3030
} from './types.js';
31+
import { isPublishDiagnosticsParams } from './types.js';
3132
import { createDebugLogger } from '../utils/debugLogger.js';
3233

3334
const debugLogger = createDebugLogger('LSP');
@@ -58,6 +59,8 @@ export class LspServerManager {
5859
this.serverHandles.set(config.name, {
5960
config,
6061
status: 'NOT_STARTED',
62+
cachedDiagnostics: new Map(),
63+
pendingDiagnostics: new Map(),
6164
});
6265
}
6366
}
@@ -264,6 +267,21 @@ export class LspServerManager {
264267
handle.connection = connection.connection;
265268
handle.process = connection.process;
266269

270+
handle.connection.onNotification((msg) => {
271+
if (
272+
msg &&
273+
msg.method === 'textDocument/publishDiagnostics' &&
274+
isPublishDiagnosticsParams(msg.params)
275+
) {
276+
handle.cachedDiagnostics.set(msg.params.uri, msg.params.diagnostics);
277+
const pending = handle.pendingDiagnostics.get(msg.params.uri);
278+
if (pending) {
279+
handle.pendingDiagnostics.delete(msg.params.uri);
280+
pending.resolve();
281+
}
282+
}
283+
});
284+
267285
// Initialize LSP server
268286
await this.initializeLspServer(connection, handle.config);
269287

@@ -370,6 +388,8 @@ export class LspServerManager {
370388
handle.error = undefined;
371389
handle.warmedUp = false;
372390
handle.stopRequested = false;
391+
handle.cachedDiagnostics?.clear();
392+
handle.pendingDiagnostics?.clear();
373393
}
374394

375395
private buildProcessEnv(
@@ -527,6 +547,8 @@ export class LspServerManager {
527547
references: { dynamicRegistration: true },
528548
documentSymbol: { dynamicRegistration: true },
529549
codeAction: { dynamicRegistration: true },
550+
// Signal acceptance of publishDiagnostics notifications from server
551+
publishDiagnostics: {},
530552
},
531553
workspace: {
532554
workspaceFolders: true,

packages/core/src/lsp/NativeLspService.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,6 +1064,87 @@ export class NativeLspService {
10641064
`LSP textDocument/diagnostic failed for ${name}:`,
10651065
error,
10661066
);
1067+
1068+
// Force-refresh the document: send didClose + didOpen to trigger fresh analysis
1069+
handle.cachedDiagnostics.delete(uri);
1070+
const openedForServer = this.openedDocuments.get(name);
1071+
if (openedForServer?.has(uri)) {
1072+
openedForServer.delete(uri);
1073+
try {
1074+
const filePath = fileURLToPath(uri);
1075+
const text = fs.readFileSync(filePath, 'utf-8');
1076+
const languageId =
1077+
this.resolveLanguageId(filePath, handle) ?? 'plaintext';
1078+
handle.connection.send({
1079+
jsonrpc: '2.0',
1080+
method: 'textDocument/didClose',
1081+
params: { textDocument: { uri } },
1082+
});
1083+
handle.connection.send({
1084+
jsonrpc: '2.0',
1085+
method: 'textDocument/didOpen',
1086+
params: {
1087+
textDocument: {
1088+
uri,
1089+
languageId,
1090+
version: Date.now(),
1091+
text,
1092+
},
1093+
},
1094+
});
1095+
await this.delay(DEFAULT_LSP_DOCUMENT_OPEN_DELAY_MS * 5);
1096+
openedForServer.add(uri);
1097+
} catch (err) {
1098+
debugLogger.warn(`Failed to refresh document:`, err);
1099+
openedForServer.add(uri);
1100+
}
1101+
}
1102+
// Check push diagnostics cache (freshly updated after didOpen)
1103+
const cache = handle.cachedDiagnostics;
1104+
if (cache) {
1105+
const cached = cache.get(uri);
1106+
if (cached && cached.length > 0) {
1107+
for (const item of cached) {
1108+
const normalized2 = this.normalizer.normalizeDiagnostic(
1109+
item,
1110+
name,
1111+
);
1112+
if (normalized2) {
1113+
allDiagnostics.push(normalized2);
1114+
}
1115+
}
1116+
continue;
1117+
}
1118+
}
1119+
// Await push diagnostics via pub/sub (Promise.race with 5s timeout)
1120+
if (handle.pendingDiagnostics) {
1121+
await Promise.race([
1122+
new Promise<void>((resolve) => {
1123+
handle.pendingDiagnostics!.set(uri, { resolve });
1124+
}),
1125+
new Promise<void>((resolve) => {
1126+
setTimeout(() => {
1127+
handle.pendingDiagnostics!.delete(uri);
1128+
resolve();
1129+
}, 5000);
1130+
}),
1131+
]);
1132+
}
1133+
// Read diagnostics after notification arrives or timeout
1134+
if (cache) {
1135+
const cached = cache.get(uri);
1136+
if (cached) {
1137+
for (const item of cached) {
1138+
const normalized = this.normalizer.normalizeDiagnostic(
1139+
item,
1140+
name,
1141+
);
1142+
if (normalized) {
1143+
allDiagnostics.push(normalized);
1144+
}
1145+
}
1146+
}
1147+
}
10671148
}
10681149
}
10691150

@@ -1112,6 +1193,32 @@ export class NativeLspService {
11121193
}
11131194
} catch (error) {
11141195
debugLogger.warn(`LSP workspace/diagnostic failed for ${name}:`, error);
1196+
1197+
if (handle.cachedDiagnostics) {
1198+
const workspaceRootUris = this.workspaceContext
1199+
.getDirectories()
1200+
.map((dir) => pathToFileURL(dir).toString());
1201+
for (const [uri, diagnostics] of handle.cachedDiagnostics) {
1202+
if (!workspaceRootUris.some((rootUri) => uri.startsWith(rootUri))) {
1203+
continue;
1204+
}
1205+
if (results.length >= limit) break;
1206+
if (diagnostics && diagnostics.length > 0) {
1207+
const normalizedDiagnostics = [];
1208+
for (const diag of diagnostics) {
1209+
const n = this.normalizer.normalizeDiagnostic(diag, name);
1210+
if (n) normalizedDiagnostics.push(n);
1211+
}
1212+
if (normalizedDiagnostics.length > 0) {
1213+
results.push({
1214+
uri,
1215+
diagnostics: normalizedDiagnostics,
1216+
serverName: name,
1217+
});
1218+
}
1219+
}
1220+
}
1221+
}
11151222
}
11161223

11171224
if (results.length >= limit) {

packages/core/src/lsp/types.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,10 @@ export interface LspServerHandle {
494494
restartAttempts?: number;
495495
/** Lock to prevent concurrent startup attempts */
496496
startingPromise?: Promise<void>;
497+
/** Cache of diagnostics keyed by document URI */
498+
cachedDiagnostics: Map<string, Array<Record<string, unknown>>>;
499+
/** Pending diagnostic notification resolvers keyed by document URI */
500+
pendingDiagnostics: Map<string, { resolve: () => void }>;
497501
}
498502

499503
/**
@@ -521,3 +525,19 @@ export interface LspConnectionResult {
521525
/** Send initialize request */
522526
initialize: (params: unknown) => Promise<unknown>;
523527
}
528+
529+
/**
530+
* Type guard for publishDiagnostics notification params.
531+
*/
532+
export function isPublishDiagnosticsParams(
533+
value: unknown,
534+
): value is { uri: string; diagnostics: Array<Record<string, unknown>> } {
535+
return (
536+
typeof value === 'object' &&
537+
value !== null &&
538+
'uri' in value &&
539+
typeof value['uri'] === 'string' &&
540+
'diagnostics' in value &&
541+
Array.isArray(value['diagnostics'])
542+
);
543+
}

0 commit comments

Comments
 (0)